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

cast _location() return value as string to remove i18n bug #15875

Closed
wants to merge 1 commit into from
Closed

cast _location() return value as string to remove i18n bug #15875

wants to merge 1 commit into from

Conversation

johnmathews
Copy link

Generating sitemaps for multiple languages does not work - the URL for the default language is simply repeated for every language.

The problem occurs in django.contrib.sitemaps.__init__.py when Sitemap._location() returns the output of another method self._get(). Casting this methods output to a string when returning it results in the expected behavior.

This bug fix was motivated by this SO question which contains additional details of my particular use case.

I don't really understand why I need to cast the output of the _location method to a string, if anyone can explain why then Id be interested to find out. Something about a lazy reference and a context processor that activates a language in _location(), perhaps.

@github-actions
Copy link

Hello @johnmathews! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

@felixxm
Copy link
Member

felixxm commented Jul 25, 2022

@johnmathews Thanks for this patch 👍 However, alternates contain language codes, see tests added in 16218c2.

If you believe it's an issue in Django, then please add a comment to the ticket-33418 with a sample project and follow our bug reporting guidelines.

@felixxm felixxm closed this Jul 25, 2022
@johnmathews
Copy link
Author

Sorry @felixxm, I shouldnt have mentioned my last comment (now deleted). Please ignore it. My first comment is relevant - i18n and sitemaps doesnt work, the patch makes it work. I guess I'll create a sample project.

@felixxm
Copy link
Member

felixxm commented Jul 25, 2022

Have you checked the ticket that I mentioned? It's exactly about i18n and sitemaps.

@felixxm
Copy link
Member

felixxm commented Jul 25, 2022

Tests added in 16218c2 work so I wonder why it doesn't work for you.

@adi-
Copy link

adi- commented Apr 11, 2023

Same here – doesn't work for me.

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