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

fix: ignore_paths are now checked before we do any translation logic #241

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bstewart00
Copy link
Contributor

@bstewart00 bstewart00 commented Oct 30, 2023

Purpose/goal of Pull Request (Issue #)

https://wovnio.atlassian.net/browse/ST-2310

The original problem is on our dashboard, the user gets redirected back to English even if they have selected Japanese. This is fixed by adding the use_cookie_lang feature. However, this created a new issue where it messes up widget XHR session auth. Those requests should not be redirected.

I noticed a few related issues in this area

Comments

  1. ignore_paths should be checked before we do anything else. This is what every other backend does
    • use_cookie_lang
    • explicit default lang redirects
  2. Remove language condition on ignore_paths. ignore_paths should be literal and really means do nothing if the request
    matches it. This is consistent with other backends as well.
  3. To actually fix the problem, rather than require setting ignore_paths for every known widget/live editor request (error prone when we add more) I added a check for requests for X-Wovn-Widget header. We will ignore these requests automatically. Can't do this because it violates CORS limits

… It now applies to all languages as well

This is consistent with other backends.
@bstewart00 bstewart00 added the wip label Oct 30, 2023
@bstewart00 bstewart00 removed the wip label Oct 30, 2023
@bstewart00
Copy link
Contributor Author

bstewart00 commented Oct 30, 2023

In the end, I think this is not the best way to fix the original problem since we have to be very careful with /ignore_paths. The dashboard should instead be generating links in the correct language.

So, this change is still good for the bug fixes to ignore_paths but we will not turn on use_cookie_lang to solve the dashboard issue which is safer for the widget.

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.

2 participants