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

Conversation

hassan-raza-1
Copy link
Contributor

@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.

@AhtishamShahid AhtishamShahid self-assigned this Mar 11, 2025
@deborahgu deborahgu added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed waiting for eng review PR is ready for review. Review and merge it, or suggest changes. labels Mar 11, 2025
@deborahgu
Copy link
Member

@hassan-raza-1 @AhtishamShahid looks like this just has a couple of conflicts to resolve and then we can merge it.

@hassan-raza-1 hassan-raza-1 force-pushed the hraza/add_embargo_restricted_country branch from 391e0d8 to 8a5b10f Compare March 14, 2025 11:12
@AhtishamShahid AhtishamShahid merged commit 72959ad into openedx:master Mar 17, 2025
49 checks passed
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants