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

feat: refactor country disable logic into the Embargo app #36202

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

Conversation

hassan-raza-1
Copy link

@hassan-raza-1 hassan-raza-1 commented Feb 3, 2025

Description

This PR refactors the existing implementation of DISABLED_COUNTRIES, which previously controlled country visibility in the Legacy registration form and restrict users from switching to disabled countries. Validation errors were implemented separately in Authn MFE and Account MFE due to their different country listing mechanisms. The new approach moves the disabled countries logic from environment settings to a database-driven model within the Embargo Django App. As part of this refactor, the DISABLED_COUNTRIES setting will be deprecated, and all related functionality will be transitioned to the new model-based system.

Related links

Internal 2U Ticket: https://2u-internal.atlassian.net/browse/INF-1760

Testing instructions

  1. Enable FEATURES['EMBARGO'] in lms/envs/production.py.
  2. Using Django Admin, add Russia (RU) to the GlobalRestrictedCountry model in the Embargo app.
  3. Navigate to the Legacy User Registration page—Russia should not be visible in the country selection field.
  4. Go to the Authn MFE, fill out the registration form, select Russia as the country, and click Save a validation error should be displayed.
  5. Log in as any user and navigate to the Accounts MFE. Attempt to change the country to Russia and click Save a validation error should be displayed.

@hassan-raza-1 hassan-raza-1 requested review from a team as code owners February 3, 2025 08:48
Copy link
Member

@deborahgu deborahgu left a comment

Choose a reason for hiding this comment

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

does this PR correspond to a specific scope of work? Could you populate the PR description to explain the purpose and implementations?

@deborahgu deborahgu added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Feb 3, 2025
@hassan-raza-1 hassan-raza-1 changed the title Hraza/add embargo restricted country feat: Refactor Country Disable Logic into the Embargo App Feb 3, 2025
@hassan-raza-1 hassan-raza-1 changed the title feat: Refactor Country Disable Logic into the Embargo App feat: refactor country disable logic into the Embargo app Feb 3, 2025
@hassan-raza-1 hassan-raza-1 force-pushed the hraza/add_embargo_restricted_country branch from b786611 to 682e58e Compare February 4, 2025 12:33
@deborahgu deborahgu added waiting for eng review PR is ready for review. Review and merge it, or suggest changes. and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Feb 4, 2025
Copy link
Member

@deborahgu deborahgu left a comment

Choose a reason for hiding this comment

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

looks great to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for eng review PR is ready for review. Review and merge it, or suggest changes.
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

3 participants