Add Simple XML Sitemap defaults for content types, menu and vocabulary

The sitemap is an SEO thing, so this is probably under the purview of the SEO experts.

@phenaproxima need your suggestions
Here I have added simple sitemap for individual recipes
Basic page - drupal_cms_page
News item - drupal_cms_news
Case study - drupal_cms_case_study
Person profile - drupal_cms_person
Project - drupal_cms_project
I am not sure about Tags & Main navigation (from Custom menu link) on which recipe i have to add

I discussed this with @thejimbirch (SEO lead) and we have a new direction we'd like to take here.

Rather than have each content type recipe opt into the sitemap separately, the SEO Tools recipe should take advantage of core's createForEachIfNotExists config action to just opt all the existing content types into the sitemap in one fell swoop, like so:

config:
    actions:
        node.type.*:
            createForEachIfNotExists:
                simple_sitemap.bundle_settings.default.node.%bundle:
                    index: true
                    priority: '0.9'
                    changefreq: daily
                    include_images: false

(See the documentation for this config action → if you're unfamiliar with it.)

This means we can close all the sub-issues for individual content types. I will ensure credit is properly transferred into this issue.

This is a much lighter and more complete approach, and it keeps everything neatly bundled into the SEO Tools recipe.

phenaproxima → credited riyas_nr → .

phenaproxima → credited diwakar07 → .

phenaproxima → credited thejimbirch → .

I tried the config action createForEachIfNotExists for the recipe SEO tools and when installing the SEO Tools recipe, I encountered this error:
ConfigActionException: Cannot determine a config entity type from simple_sitemap.bundle_settings.default.node.blog
Debugging shows that in core/lib/Drupal/Core/Config/Action/Plugin/ConfigAction/EntityCreate.php, the line:

$entity_type_id = $this->configManager->getEntityTypeIdByName($configName);

returns null. Although the config entity types simple_sitemap.sitemap and simple_sitemap.type exist, getEntityTypeIdByName() isn’t returning a config entity type for simple_sitemap.bundle_settings.default.node.blog, which seems to be causing the exception.

Well, shit.

That means simple_sitemap.bundle_settings.whatever config is simple config, not a config entity type. I should have realized that before, honestly. We'll need to change our approach here.

I gave it a think and I have an idea that I think might work. It involves ECA.

What if we added a new ECA model to the SEO Tools recipe:

• Whenever a new content type is created, it creates the corresponding sitemap settings for it.
• Whenever the SEO Tools recipe is applied, it creates the sitemap settings for all existing content types.

I think that'd pretty much cover all cases.

Makes sense to me.

Started MR!502 which creates the simple sitemap config for new node bundles.

Next steps:

• Don't act on config import
• Update plugin IDs in the model
• Take action on all existing bundles when the SEO Tools recipe got applied

This'll need tests for sure but I'll handle those.

This is now implemented.

What's maybe remaining, is the boolean values currently get stored as integers. If that's an issue, we need to add a new feature into ECA to cast config values, that are always strings at this point, into boolean.

phenaproxima → changed the visibility of the branch 3494070-add-simple-sitemap-defaults-for-missing-content-type-menu-vocabulary to hidden.

I started writing tests for this, but in manual testing I discovered a few issues. Talked about it with @jurgenhaas in Slack: https://drupal.slack.com/archives/C07G41EUJP4/p1739455989742089

We agreed that the ECA model should exit early and do nothing for a given content type if any of the following are true:

• We're syncing config.
• We are creating a content type in the UI (i.e., the current path is /admin/structure/types/add).
• The sitemap settings for a given content type already exist.

Hmmm, found another problem (revealed by the tests):

1. Install Standard (drush si -y standard), which supplies content types out of the box
2. Apply this recipe (drush recipe ../recipes/drupal_cms_seo_tools)
3. Export config (drush cex -y)
4. Inspect the exported config and you'll see that the Page and Article content types don't have any sitemap settings

You won't reproduce this if ECA is already installed before you apply the SEO Tools recipe. That leads me to believe that ECA's event subscribers are not yet registered by the time the model runs.

Made a bit of progress today. All the conditions from #23 are implemented and working.

Haven't looked into the issue from #24 yet, will follow up soon.

There is one more issue I found, though: ECA finishes processing correctly, but prints an error message that it ran into a recursion. That's because we start with a config write event and later use actions to write config, which triggers the same event again. Of course, this is because of a different config entity, but the event itself is the same and ECA detects and prevents that from happening. So, without getting the error message, the behaviour would already be perfect. But ECA keeps that error to tell site builders that they should ideally resolve such issues in their models.

I can see various options on how to improve ECA to handle this better. The straightforward method would be to add some configuration to the ECA event that listens to write config so that we can filter out that the event should only listen on specific config being written. That would even be a tiny, but still a performance improvement. I think, that's what I'll implement in ECA top make this work.

BTW, while testing I figured that the news recipe comes with a simple_sitemap.bundle_settings.default.node.news.yml config file, which I've remove as part of the MR as well, as this setting will be set by this model when needed.

I've now optimized the ECA config save event so that we no longer get potential recursions. I believe that the performance gain might be even bigger than anticipated. Even the model itself got simplified as we no longer need 3 conditions after the event as all the prerequisites can now be configured when using the event.

I then also reviewed the issue described by @phenaproxima in #24 and I can (no longer) reproduce it. Not sure if it got solved by any of the other improvements I've made in the meantime. Here are the steps I followed:

drush si -y standard
drush recipe ../recipes/drupal_cms_seo_tools

drush cget simple_sitemap.bundle_settings.default.node.page
drush cget simple_sitemap.bundle_settings.default.node.news

For the first "cget" I'm getting the expected config values, for the second one I get an error message that the config does not exist. Both is as expected.

Assigning to @phenaproxima for the tests.

Started writing a test and there seems to be a bug here -- there are Simple Sitemap configurations being created for node types that don't even exist, like index and yoast_seo: https://git.drupalcode.org/project/drupal_cms/-/jobs/4414792#L42

Possible bug in the ECA model?

That was a head scratcher and I got really confused; until I realised that we didn't use the latest ECA enhancements yet. Changed the constraint to ECA 2.1.x-dev and now the tests are green. If nothing else comes up, we're ready to tag another release for ECA today or tomorrow.

Ah, phew! That's a relief. Back to NW for me to finish the test coverage. :)

Alright - that test coverage seems to be solid! Postponing on a new release of ECA

ECA 2.1.4 is now available and I've updated the constraint in all recipes as the others also use the improved "Config save" event plugin, so they all require at least that version. Back to NR.

Thanks for the new release. Marking as RTBC.

Merged into 1.x! Not backporting this one to 1.0.x, since it is definitely a feature addition.

@phenaproxima that's ok but the updated config of ECA models that use the improved "Config Save" event should be back ported. Shall I open a follow-up for this and deal with it as a bug?

Please do!

Done with 🐛 Update ECA models for version 2.1.4 Active

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

This will need a change record.

Created the change record, it just needs details filled in. https://git.drupalcode.org/project/drupal_cms/-/wikis/Committer-Info/Cha... can be used as a template.

Change record added.

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