Geta / SEO.Sitemaps

Search engine sitemaps.xml for EPiServer CMS
Apache License 2.0
28 stars 42 forks source link

[Proposal] SiteUrl to be overwritten by the one from the Hosts #60

Closed BorisSokolov closed 6 years ago

BorisSokolov commented 7 years ago

Currently there is a code that uses SiteUrl instead of the one that is listed in a Hosts: AdminManageSitemap.aspx.cs

if (host.Name == "*" || host.Name.Equals(siteInformation.SiteUrl.Host,
StringComparison.InvariantCultureIgnoreCase))
{
                        continue;
}

It makes the code less configurable as the SiteUrl is bound to the EPiServer license

ruifrvaz commented 6 years ago

Hi @BorisSokolov . I will propose this to the team. Will let you know if it's going to be implemented or not.

marisks commented 6 years ago

@BorisSokolov I do not understand what you meant by this issue. Do you have an example how it should work? As I see in the code: https://github.com/Geta/SEO.Sitemaps/blob/master/src/Geta.SEO.Sitemaps/Modules/Geta.SEO.Sitemaps/AdminManageSitemap.aspx.cs#L339 This method returns all host URLs. It just adds the Host URL which matches SiteUrl as the first one to the result list: https://github.com/Geta/SEO.Sitemaps/blob/master/src/Geta.SEO.Sitemaps/Modules/Geta.SEO.Sitemaps/AdminManageSitemap.aspx.cs#L349 And then in the loop, it just skips the URL as it is already added.

BorisSokolov commented 6 years ago

@marisks - for our case we had "real" url with https and the license with http. So there is no option to make that code working without changing the license.

marisks commented 6 years ago

But if you add the real URL to hosts doesn't it show up? This code should add all URLs from hosts: https://github.com/Geta/SEO.Sitemaps/blob/master/src/Geta.SEO.Sitemaps/Modules/Geta.SEO.Sitemaps/AdminManageSitemap.aspx.cs#L351

marisks commented 6 years ago

I understood now. In the code snippet you posted, comparison ignores scheme. This causes two hostnames which differ only by the scheme to be added only once.

Will fix this.

marisks commented 6 years ago

The fix is in version 2.0.8. The new package is uploaded to Episerver's Nuget feed - should be available soon.