Remove unused cache, max_age configuration options in SitemapSyndicateBlock

Thank you! I had no idea this configuration even existed! 😅

Looking at \Drupal\sitemap\Plugin\Block\SitemapSyndicateBlock, I see several bugs...

1. The cache and max_age config don't actually appear to be used anywhere (but maybe I missed something)
2. I see a call to $this->configFactory->get('sitemap.settings')->get('rss_front'); but no mention of that key in config/schema/sitemap.schema.yml

... but I daresay these belong in follow-up issues because using-previously-unused config and/or adding a settings form for config and/or figuring out what rss_front is supposed to be sound like larger discussions. Thoughts, @phenaproxima?

@phenaproxima are you working on a patch, or would you like me to try? (just want to avoid duplicating our efforts)

Looking at the history of the rss_front configuration...

The rss_front configuration item used to control two things: the href for this RSS icon on the Syndicate (sitemap) block, and feed URL on what would become the frontpage plugin (i.e.: \Drupal\sitemap\Plugin\Sitemap\Frontpage).

A separate setting for the frontpage plugin was established in commit 52ab0d4 from 2017-03-22.

The rss_front control on the settings page was removed shortly thereafter in commit commit b35b30f from 2017-03-22.

While I doubt anyone actually uses this Syndicate block (I don't understand what it's supposed to do — seems like it exists only to output an RSS icon that links to the RSS feed?), I also feel like deleting it would be a backwards-compatibility break / regression, so I think I should just fix it and mark it deprecated so we can (hopefully) delete in a future 3.0.x version.

I've created #3545858: Migrate sitemap.settings.rss_front to SitemapSyndicateBlock's configuration; test, deprecate SitemapSyndicateBlock to capture the work identified in this comment.

Looking at the history of the cache and max_age configuration...

Looks like it came from the site_map-8.x-1.x-dev project, i.e. added to sitemap in commit 65d9a44 from 2015-11-08.

Switching over to the site_map repo, it looks like cache and max_age were introduced in #2366683: Update site_map to work with 8.0.x branch. As far as I can tell, that issue sought to reconcile the code already in the 8.x-1.x branch with the code in the 7.x-1.x branch. At this point, the 7.x-1.x branch had a $blocks['syndicate']['cache'] = DRUPAL_NO_CACHE line in hook_block_info() from #1098918: Fix block caching & theming; while the Syndicate block's plugin class in the 8.x-1.x branch had a 'cache' => DRUPAL_NO_CACHE default configuration (from the initial D8 port in #2139181: Drupal 8 version) but it was never used anywhere. I guess making it configurable was better than making it never-cacheable but it seems that the setting may have been a relic that never properly took advantage of D8+'s [dynamic_]page_cache features.

Regardless, it looks like the cache and max_age configuration never did anything in D8 at any point, so I think we can delete it (safely, with an update hook) without breaking backwards compatibility.

Adding tags to reflect what needs to be done.

In particular, I should update the issue summary so we know what is/is-not in scope. But I'm going to do that tomorrow.

Updating the issue summary.

Updating the issue title to reflect the new scope.

Thanks for your hard work, @phenaproxima!

@phenaproxima are you still working on this?

I've got something working and tested in #3545858: Migrate sitemap.settings.rss_front to SitemapSyndicateBlock's configuration; test, deprecate SitemapSyndicateBlock — which will likely conflict with some of your changes here, although with relatively trivial resolutions...

1. both branches create sitemap.post_update.php but create two different hooks with different names - both functions should exist in the final version
2. both branches modify the defaultConfiguration() function in src/Plugin/Block/SitemapSyndicateBlock.php. - this branch should continue deleting the cache key and friends; the other branch should continue creating the rss_feed_path key.

I'm happy to merge this one first and then fix the merge conflicts in the other branch if that's easiest for you, so I'm going to hold off on merging 3545858 until I hear back from you!.

Thanks!

@phenaproxima, if you could review and/or RTBC this, that would be great. Thank you!

@phenaproxima, okay!

I've added #3557990: Add tests for remove_cache_setting_from_syndicate_blocks post-update hook as a follow-up issue, so I'm deleting the "Needs tests" tag.

I also added a post-update hook between #12 and #13 so I'm removing the "Needs update path" tag.

I will merge this shortly.

@phenaproxima I'm feeling very tired today, so I'm not going to make a release today, but if you need a release soon, I'll look into making one in the coming days.

This fix has been released in sitemap-8.x-2.5.

(typo in issue summary)