When multiple sitemaps of type "Default hreflang" are defined, only one can be overwritten

This fixes the problem for me.

@daniel.bosen Thank you for reporting! MR has changes in the wrong place, but the main idea is correct. I fixed the problem in the above commit.

@gbyte This bug is a symptom of a bigger problem with SitemapGetterTrait. The list of set sitemaps is statically cached in the $sitemaps property. Sometimes this leads to non-obvious results if some code previously called the setSitemaps() method and specified a parameter other than NULL. In other words, in some cases the getSitemaps() method is expected to return a list of all compatible sitemaps, but instead it returns only those that were previously set.

Patch was not applying hence rebased but I can see the fix is added in dev, will test without this patch. Thank you @WalkingDexter!!

The bug really exists.
The functions setEntityInstanceSettings() and getEntityInstanceSettings() don't support multiple variants. And we also have a @todo in the code to fix it.
See https://git.drupalcode.org/project/simple_sitemap/-/blob/4.x/src/Manager...

I provided a quick solution for that.
Updated MR to fix it. Also added a patch to simplify the application of the solution.

But.. the tests must be updated too.

Readded the patch without tests for v4.1.7

@HitchShock

I have not had time to go through your code thoroughly, but please note the following:

When I informed you in 🐛 Entity Instance Settings - support multiple sitemap types Closed: outdated that the bug you are getting is probably rooted in a regression we've introduced in the last version, I meant it has already been fixed in this here issue in the dev version. The code todo you are referring to is about the setEntityInstanceSettings() method not handling multiple variants as other methods do, but that's just an implementation detail that has never stopped the module from working as intended. We've been working around this limitation by calling the method for each set variant instead of calling the method once.

I have tested dev and it seems to be working well without your patch. As I mentioned in the issue linked above, you may need to reapply all settings. What I meant by this is, you may need to disable sitemap support for the entity bundle in question and then set any overrides anew.

@WalkingDexter

@gbyte This bug is a symptom of a bigger problem with SitemapGetterTrait. The list of set sitemaps is statically cached in the $sitemaps property. Sometimes this leads to non-obvious results if some code previously called the setSitemaps() method and specified a parameter other than NULL. In other words, in some cases the getSitemaps() method is expected to return a list of all compatible sitemaps, but instead it returns only those that were previously set.

The getting and setting of sitemaps uses a clever mechanism that has a lot of advantages, but also this quite serious disadvantage. I don't intend to change the mechanism in a breaking manner, but I suggest we document this anti-pattern. How about adding a warning to the docblocks in question?

I am setting this issue to 'fixed' - if any of you guys encounter the above problem, let me know.

Hi @gbyte
Retested the issue with dev module version on my project more carefully and you are right. Seems like it was fixed by the last commit on the dev branch.
So, yes, the issue is fixed. Made a patch for the last commit to apply it on the 4.1.7 version

Sorry, the wrong file was uploaded

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

WalkingDexter → closed merge request !83