Aggregator RSS feed outputting empty feed items

This is a reroll of #20.

Something went wrong and the tests-only patch didn't get uploaded with #40.

The last submitted patch, 40: 2552037-40.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

The last submitted patch, 41: 2552037-40-tests-only.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

This needed another reroll due to recent changes in AggregatorRenderingTest. Also, the patch's test was written before the conversion from SimpleTest to PHPUnit. So that part of the test had to be updated as a result since Mink doesn't support XPath queries for XML.

The last submitted patch, 45: 2552037-45-test-only.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

NW for the update path

I added an update path with a test. I don't believe that we can safely update most sites' view without risking data loss. This isn't like most other view updates that I could find in Core that change something about a plugin setting or some other piece of well-defined configuration. But I wanted to try. The only way I could think of to do that is to compare a hash of the view in active configuration to its default_config_hash. If it's the same, then we're safe to update it. If not, then we display a message to the admin telling them that they need to do it manually (which also applies if they've deleted the view).

To be honest, I expect that the majority of sites will have altered configuration. Even on my sandbox the view somehow got altered, removing what looks like two unused language context keys. I don't know how or when that happened.

I need feedback on the content of the "you have to manually upgrade" message. Should we provide a documentation link? It could possibly be just a link to this issue. The version in the message, "2.0.x", will have to be updated on commit when it's clear which version the change will be released in.

Also, this uses the exact same dump file as 📌 Refactor aggregator to use processed_text Fixed . When one gets committed, the other will probably have to be rerolled.

This is a reroll without the fixture, which was committed in 📌 Add a fixture for database update tests Fixed . I'm not going to bother with an interdiff since that was the only change.

+++ b/aggregator.install
@@ -11,3 +11,77 @@
+  if (!aggregator_rss_view_config_is_default()) {

We have a pattern in core of using the a reusable class for handling updates to views config (see \Drupal\views\ViewsConfigUpdater)
We then call that code both from an update hook, and from a hook_view_presave.

The later is to handle the case of where an install profile has its own config and it is now invalid.

In this case I don't think this applies.

We're updating default config provided by the aggregator module only, rather than generic config possibly used in multiple places.

E.g. those type of updates are for when the config changes for e.g. a views filter plugin and there is not a known set of views to update.

So I think this approach is fine in this case.

Thank you to everyone who worked on this!

Automatically closed - issue fixed for 2 weeks with no activity.