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: changing languages on the enrollment success page #2179

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

lalver1
Copy link
Member

@lalver1 lalver1 commented Jun 20, 2024

Closes #2058

On the enrollment success page, the Django set_language redirect view was redirecting the user to the enrollment index page when the language was changed. Looking at the Django documentation, adding a next parameter in the POST data and explicitly including the enrollment success page url in the context seems to be what was missing.

For all other views, when changing languages, I think we don't see this behavior because the Referer request header contains the address of the same view, but for the enrollment success view, Referer is /enrollment/ (I think because it is called from /enrollment/ initially), so using the next parameter is required.

@lalver1 lalver1 self-assigned this Jun 20, 2024
@github-actions github-actions bot added deployment-dev [auto] Changes that will trigger a deploy if merged to dev back-end Django views, sessions, middleware, models, migrations etc. labels Jun 20, 2024
Copy link

github-actions bot commented Jun 20, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  benefits/enrollment
  views.py
Project Total  

This report was generated by python-coverage-comment-action

@lalver1 lalver1 changed the title Fix: changing languages on the enrollemnt success page Fix: changing languages on the enrollment success page Jun 20, 2024
@lalver1 lalver1 force-pushed the fix/spanish-button-sucess branch 4 times, most recently from 3a01aeb to b9bde4b Compare June 21, 2024 17:06
@lalver1 lalver1 marked this pull request as ready for review June 21, 2024 18:34
@lalver1 lalver1 requested a review from a team as a code owner June 21, 2024 18:34
angela-tran
angela-tran previously approved these changes Jun 24, 2024
Copy link
Member

@angela-tran angela-tran left a comment

Choose a reason for hiding this comment

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

Nice fix and explanation! Thanks for linking the Django docs

benefits/core/templates/core/includes/lang-selector.html Outdated Show resolved Hide resolved
Copy link
Member

@angela-tran angela-tran left a comment

Choose a reason for hiding this comment

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

🚀

@lalver1 lalver1 merged commit d49aa6b into dev Jun 24, 2024
15 checks passed
@lalver1 lalver1 deleted the fix/spanish-button-sucess branch June 24, 2024 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing languages on the success page takes user back to Enrollment Index
2 participants