Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

href for alternate aren't generated correctly #343

Open
afuno opened this issue Mar 22, 2020 · 3 comments
Open

href for alternate aren't generated correctly #343

afuno opened this issue Mar 22, 2020 · 3 comments

Comments

@afuno
Copy link

afuno commented Mar 22, 2020

I describe the rule as shown in your documentation:

add(
  company_overview_path(:en, company),
  changefreq: 'monthly',
  priority: 0.5,
  alternate: {
    href: company_overview_path(:es, company),
    lang: :es,
    nofollow: false
  }
)

For the main URL all is well. The protocol and host for this URL will be taken from:

SitemapGenerator::Sitemap.default_host = "#{Settings.protocol}#{Settings.host}"

But the URL for alternate will look like this:

/es/esrt/overview

That is, there will be no part with the domain.

This is mistake? Just now I have to use a crutch:

DEFAULT_HOST = "#{Settings.protocol}#{Settings.host}"
SitemapGenerator::Sitemap.default_host = DEFAULT_HOST

# and

href: DEFAULT_HOST + company_overview_path(:es, company)
@kjvarga
Copy link
Owner

kjvarga commented Mar 25, 2020

Hi @afuno thanks for the well explained report. I can confirm that it is expecting a full HREF and doesn't modify the alternate value at all:

          self[:alternates].each do |alternate|
            rel = alternate[:nofollow] ? 'alternate nofollow' : 'alternate'
            attributes = { :rel => rel, :href => alternate[:href].to_s }
            attributes[:hreflang] = alternate[:lang].to_s if SitemapGenerator::Utilities.present?(alternate[:lang])
            attributes[:media] = alternate[:media].to_s if SitemapGenerator::Utilities.present?(alternate[:media])
            builder.xhtml :link, attributes
          end

I think probably the best thing to do here would be to prepend the default_host if the value is a path.

@kjvarga
Copy link
Owner

kjvarga commented Mar 25, 2020

In your case I wonder if your settings are simple enough that you can use company_overview_url rather than company_overview_path or if you do need to customize the domain using Settings?

@mvitale
Copy link

mvitale commented Jun 3, 2020

Came here looking to see if this issue had been reported. It's confusing to be expected to use *_path for main entries but *_url for alternates. Is there a reason for this inconsistency? I'd be happy to work on a patch if that would be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants