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

Fixes invalid redirection for /admin when browser is not set to English #267

Merged
merged 3 commits into from
May 10, 2023

Conversation

saevarom
Copy link
Contributor

Fixes #262.

Move the homepage lookup after response handling so that we have access to the url resolver. Then check if the requested path belongs to i18n_patterns so that we are not handling e.g. /admin as a localized URL.

To test this you will need to:

  1. Make sure the browser language is english
    1. Test that /admin works properly
    2. Test that / works properly and redirects to `/en-latest``
  2. Change browser language to Deutsch
    1. Test that /admin works properly
    2. Test that / redirects to /de-latest/ and shows a 404 page with the English menu

…o that we have access to the url resolver. Then check if the requested path belongs to i18n_patterns.
@saevarom saevarom changed the title Fixes #262. Fixes invalid redirection for /admin when browser is not set to English Nov 11, 2022
@allcaps
Copy link
Member

allcaps commented Nov 11, 2022

Great! Thank you! Is it okay if I write some tests for this?

@saevarom
Copy link
Contributor Author

Great! Thank you! Is it okay if I write some tests for this?

Sure

@allcaps allcaps self-assigned this Nov 11, 2022
@saevarom saevarom self-assigned this Nov 11, 2022
Copy link
Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 this seems to be working great.

@thibaudcolas
Copy link
Member

I see the tests are still in progress in #277, but the fix itself seems worthwhile to include already, so I’ll merge now.

@thibaudcolas thibaudcolas merged commit 6281544 into wagtail:main May 10, 2023
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

Successfully merging this pull request may close these issues.

Invalid redirect for Wagtail Admin when browser language is not English
3 participants