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

Allows users to change language when not signed-in. thus, implementing proper language redirection as in Kolibri #3721

Merged
merged 7 commits into from
Nov 2, 2022

Conversation

ozer550
Copy link
Member

@ozer550 ozer550 commented Oct 6, 2022

Summary

The Following Pr adds the functionality to the change language while viewing the public resources page or any other pages when not being signed in. It also implements proper redirection according to session and cookie data as in Kolibri.
closes #3150

Description of the change(s) you made

  • Refactored set_language endpoint used for setting language
  • Added a dropdown component with sign-in and change language option when the user is not signed in.
  • Wrote tests to verify proper redirection.

Manual verification steps performed

  1. Click on Explore without an account
  2. Click on dropdown menu at top right.
  3. Select a desired language.
  4. open new tab (opens with selected language).

Screenshots (if applicable)

Before:
Screenshot from 2022-10-06 23-01-39

After:
Screenshot from 2022-10-06 22-54-22

Does this introduce any tech-debt items?

Reviewer guidance

How can a reviewer test these changes?

  1. Click on Explore without an account from login page
  2. Click on dropdown menu at top right.
  3. Select a desired language.
  4. open new tab (opens with selected language).

References

The set_language function is refactored from here

Contributor's Checklist

PR process:

Testing:

  • Code is clean and well-commented
  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Any new interactions have been added to the QA Sheet
  • Critical and brittle code paths are covered by unit tests

Reviewer's Checklist

This section is for reviewers to fill out.

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@ozer550 ozer550 marked this pull request as ready for review October 6, 2022 17:33
@ozer550 ozer550 requested a review from bjester October 6, 2022 17:36
@ozer550 ozer550 changed the title Fix change language Allows users to change language when not signed-in. thus, implementing proper redirection as Kolibri Oct 6, 2022
@ozer550 ozer550 changed the title Allows users to change language when not signed-in. thus, implementing proper redirection as Kolibri Allows users to change language when not signed-in. thus, implementing proper language redirection as in Kolibri Oct 6, 2022
Copy link
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

In my testing, when I'm on the sign-in page and I change the language, the page refreshes again after reloading. It seems related to the service worker, and I think it must be the pwa/manifest.json causing it because it loads the language.

Copy link
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

Looks good to me-- one comment about adding/updating the doc string

@@ -353,42 +352,54 @@ def set_language(request):
"""
Copy link
Member

Choose a reason for hiding this comment

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

Could you add/replace the doc string here noting where this code came from and what you changed?

@bjester bjester merged commit f2b9798 into learningequality:unstable Nov 2, 2022
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.

No option to change the language when not being signed-in
2 participants