Problem/Motivation

The Sitemap module defines a block plugin with ID sitemap_syndicate (i.e.: \Drupal\sitemap\Plugin\Block\SitemapSyndicateBlock), which appears to exist only to display an RSS icon that links to an RSS feed.

Before commit b35b30f from 2017-03-22, the location of this RSS feed was customizable (i.e.: similar to the Feed URL config on the Frontpage sitemap plugin) — the location to link to was stored in the top-level configuration sitemap.settings.rss_front. But we deleted sitemap.settings.rss_front in \sitemap_update_8200()

While I doubt anyone actually uses the SitemapSyndicateBlock, deleting it would be a backwards-compatibility break / regression, so we should fix and deprecate it in 8.x-2.x so it can be removed in 3.0.x.

Proposed resolution

  1. Write a test for the SitemapSyndicateBlock. - done by @mparker17 by #4
  2. Update the SitemapSyndicateBlock configuration to have an rss_feed_path configuration (both in the schema and in the block plugin class). Update the test to check that it can be changed. - done by @mparker17 by #4
  3. Write an update hook to see if plugins.frontpage.settings.rss exists (i.e.: from the Frontpage sitemap plugin configuration), and if so, migrate it to the new configuration. If not, fallback to the default, /rss.xml.
  4. Mark everything related to SitemapSyndicateBlock as deprecated for removal in 3.0.x - done by @mparker17 by #4

Remaining tasks

  1. Write a MR - done by @mparker17 by #4
  2. Write a draft change record - done by @mparker17 by #4
  3. Add an update hook - done by @mparker17 by #7
  4. Review and feedback - skipped by @mparker17 in #7
  5. RTBC and feedback - done by @mparker17 in #7
  6. Commit - done by @mparker17 in #10
  7. Release - released by @mparker17 in sitemap-8.x-2.5

User interface changes

Adds the ability to set the RSS link destination in the SitemapSyndicateBlock settings.

API changes

None (only deprecations).

Data model changes

Introduces a block.block.sitemap_syndicate_stark.settings.rss_feed_path configuration.

Issue fork sitemap-3545858

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mparker17 created an issue. See original summary.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

Update the issue summary with slightly clearer wording.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Assigned: Unassigned » mparker17

Assigning to myself.

mparker17 opened merge request !56

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes
Status: Active » Needs work

Made some progress but still have to write the update hook to migrate from the old config.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

Turns out we deleted rss_front from the global settings in \sitemap_update_8200(), which explains why my test wasn't working.

Updating the issue summary to reflect the new scope.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community

Okay, this seems to be working now, and testbot is happy. Note that it will conflict with #3545794: Remove unused cache, max_age configuration options in SitemapSyndicateBlock.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

No objections here! This is partially blocking progress in Drupal CMS so let's ship it, I say!

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented

Okay, I'll merge this, and then fix any conflicts in #3545794: Remove unused cache, max_age configuration options in SitemapSyndicateBlock and put that one for review.

  • mparker17 committed 52bf16f1 on 8.x-2.x 
    fix: #3545858 Migrate sitemap.settings.rss_front to...
mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Assigned: mparker17 » Unassigned
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Merged! Thanks @phenaproxima! I will update this message when the other issue is ready for review, and also when this gets released.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

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

Status: Fixed » Closed (fixed)

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