- 🇩🇪Germany internetter Erfurt, Thüringen
We also have a deployment for multiple stage servers with different baseurls. For other stage related config changes we use config split and put this in git repo. But because of this issue, we can not use config-split.
Overriding in settings.php is possible with:
$settings['xmlsitemap_base_url'] = 'http://[other-base-url]';
Other settings from xml sitemap are exported for config split. So it should also be with base-url.
- 🇨🇦Canada deviantintegral
We've run into this on two different projects recently, so I took the opportunity to talk to @davereid about this. On one of those projects, we set the URL dynamically with:
if (isset($GLOBALS['request'])) { $xmlsitemap_host = $GLOBALS['request']->headers->get('host'); $settings['xmlsitemap_base_url'] = 'https://' . $xmlsitemap_host; }
Here's my notes:
- Dave thought this used to be dynamic based on the request URL, but was changed because of how many support requests there were with users regenerating sitemaps with drush, but not specifying a URI.
- We're not sure exactly why this is settings + state instead of a config object. It may just be an artifact of an early Drupal 8 port when many developers were still getting their heads around the new config system.
- We think this can be made dynamic again, using the request object to automatically determine the base URL in most cases.
- When doing this, we should check if the hostname is
default
. If so, we can confidently throw an exception that the site URI must be specified in the drush command or site alias. This should help deal with any potential support load. - There's still value in being able to override or hardcode this. For example, a site may have a separate edit domain, and users still want to be able to log in through that but update the production sitemap.
- Instead of settings + site, the override option should be a config value that's optional, overriding the auto-detection behaviour above.
We may be able to get some additional dev effort on this, but I can't commit to it right now.
- 🇩🇰Denmark ressa Copenhagen
+1 for treating Default Base URL as regular configuration, overridable in settings.php.
Many users spend (needlessly IMO) time to figure out what the problem is, since the current set up is counter intuitive to the now well established Drupal 10 workflow.
- 🇺🇸United States will kirchheimer New Orleans
Thanks @internetter for the settings.php tweak.
I do think that this should be handled in config, and then managed in config_split.
It is still important to allow the override in settings.php. My issue is I have lots of ways to hit my sites: Public, Load balanced, IP, Login Enabled, Internal, Etc. So, I end up with polluted URLs.
I can use the settings.php approach, and it is fine, but it does feel like it should be in the Config split.
- 🇪🇸Spain guardiola86
I'm having the same issue, I have two domains and that's causing both domains to appear in sitemap.
- last update
almost 2 years ago 42 pass, 2 fail - 🇪🇸Spain guardiola86
Here's another patch but is not complete yet, I will probably use another module for now.
Currently I can only add one sitemap and I would need 2 in my case. It will also need to filter by domain when indexing. - last update
almost 2 years ago 42 pass, 2 fail - last update
almost 2 years ago 42 pass, 2 fail - last update
almost 2 years ago 30 pass, 5 fail - last update
almost 2 years ago 41 pass, 4 fail - last update
almost 2 years ago 42 pass, 2 fail - last update
almost 2 years ago 42 pass, 2 fail - Status changed to Needs work
almost 2 years ago 9:20am 4 July 2023 - First commit to issue fork.
- last update
almost 2 years ago 45 pass - @penyaskito opened merge request.
- last update
almost 2 years ago 45 pass - last update
almost 2 years ago 45 pass - last update
almost 2 years ago 46 pass - last update
almost 2 years ago 46 pass - 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
The PR covers the plan consensuated by @deviantintegral and @davereid at #10:
- Defaults is based on the request URL. Uses "base:/", which if we look at Url::fromUri:$url = new static($uri, [], $options); if ($uri_parts['scheme'] !== 'base') { $url->external = TRUE; $url->setOption('external', TRUE); } $url->setUnrouted();
will use the same mechanism that Drupal uses for getting the base_url. There's test coverage for this.
- There's still value in being able to override or hardcode this.
We can use
$settings['xmlsitemap_base_url']
(as of now). IMHO this should be removed in next major release, as you can already do that by using$config['xmlsitemap_base_url']
. In case both are present,$settings
takes precedence. There's test coverage for this.- When doing this, we should check if the hostname is default. If so, we can confidently throw an exception that the site URI must be specified in the drush command or site alias.
This part is the only piece still missing from the PR. Wondering if that should happen on generation, or in which stage. Will check this next.
- last update
almost 2 years ago 46 pass - last update
almost 2 years ago 47 pass - last update
almost 2 years ago 47 pass - 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
I'm a bit lost with the new docs system, but created a draft guide for xmlsitemap, so we can add a page for the linked docs for this issue:
https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... →
- last update
almost 2 years ago 47 pass - Status changed to Needs review
almost 2 years ago 1:31pm 5 July 2023 - 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
Created [#3372567] for the docs, and linked this in the error messages.
I think this is ready for review now. - last update
almost 2 years ago 47 pass - last update
almost 2 years ago 42 pass, 1 fail - last update
almost 2 years ago 47 pass - last update
almost 2 years ago 47 pass - last update
almost 2 years ago 47 pass - last update
almost 2 years ago 47 pass - last update
almost 2 years ago 47 pass - last update
almost 2 years ago 47 pass, 2 fail - 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
I've responded to all @deviantintegral suggestions. Added test for the post_update. I think this is ready for another review.
- last update
almost 2 years ago 47 pass, 2 fail - last update
almost 2 years ago 48 pass - 🇩🇰Denmark ressa Copenhagen
A year has passed since @penyaskito kindly updated the MR and requested a review ... it would be great if it could get a look-over, to check if the code looks good now.
- 🇮🇹Italy apaderno Brescia, 🇮🇹
It seems that the 8.x-1.x branch is the developed one: The last commit for the 8.x-1.x branch was done 15 hours ago, while the last commit for the 2.x branch was done eight months ago, and just merged the 8.x-1.x in it.
Probably a merge request would be merged if it were for the 8.x-1.x branch.
- 🇩🇰Denmark ressa Copenhagen
Thanks for looking into this @apaderno.
On a related note, it looks like custom links are also not included when exporting configuration, so I created an issue.
- 🇩🇰Denmark ressa Copenhagen
A globally available Base URL would have prevented a problem in this, and many other modules, so I created ✨ Allow defining a base URL Active .
- 🇩🇰Denmark ressa Copenhagen
Just adding a note, that https://www.drupal.org/project/simple_sitemap → works out of the box, and automatically uses the URL of the Drupal installation, switching from for example https://mysite.ddev.site/ during development, to https://example.org/ on the server, without any configuration.
Bonus tip: Simple XML sitemap also has multilingual support, whereas this module seems to add the default language prefix, for example https://example.org/en/sitemap.xml, which can be problematic, since that's not where crawlers look for it ...