Skip to content

Ignore edx failures variable usage#3260

Open
cp-at-mit wants to merge 21 commits intomainfrom
ignore-edx-failures-variable-usage
Open

Ignore edx failures variable usage#3260
cp-at-mit wants to merge 21 commits intomainfrom
ignore-edx-failures-variable-usage

Conversation

@cp-at-mit
Copy link
Contributor

@cp-at-mit cp-at-mit commented Jan 30, 2026

What are the relevant tickets?

NA

Description (What does it do?)

This adds checks and logic around various calls to edx based on the value of the IGNORE_EDX_FAILURES environment variable. Previously this environment variable was only implemented around a few edx calls.

How can this be tested?

Without edx running, you should be able to set IGNORE_EDX_FAILURES to TRue in your .env file and enroll into courses, update your user, and load the dashboard without any issues caused by the edx interactions that failed.

The create_run_enrollments function now defaults the keep_failed_enrollments parameter to the IGNORE_EDX_FAILURES feature flag if not explicitly set. Added tests to verify that the function respects both the parameter and the feature flag.
Change deactivate_run_enrollment so keep_failed_enrollments defaults to None and, when None, is set from the IGNORE_EDX_FAILURES feature flag (is_enabled(features.IGNORE_EDX_FAILURES)). Update the function docstring accordingly and add tests (test_deactivate_run_enrollment_feature_flag) to verify behavior for None and explicit True/False values by mocking is_enabled and unenroll_edx_course_run.
Replace direct calls to mitol.olposthog.features.is_enabled with settings.FEATURES.get(features.IGNORE_EDX_FAILURES, False) across courses serializers and views to read the IGNORE_EDX_FAILURES flag from Django settings. Remove unused is_enabled imports and add django.conf.settings imports where needed. In users.views, add logging and robust error handling around queuing edX tasks: failures are logged and will only propagate when the IGNORE_EDX_FAILURES flag is disabled. Add tests to verify behavior of user update when edX task queuing fails and the feature flag is toggled. Files changed include courses/api.py, courses/serializers/v1/courses.py, courses/views/v1/__init__.py, courses/views/v2/__init__.py, users/views.py and users/views_test.py.
Add logging and error handling around syncing enrollments with edX in UserEnrollmentsApiViewSet.list. The view now catches exceptions from sync_enrollments_with_edx, logs the failure, and will re-raise only if the FEATURES flag IGNORE_EDX_FAILURES is not set. Also add parameterized tests to verify behavior across combinations of SYNC_ON_DASHBOARD_LOAD, IGNORE_EDX_FAILURES, and sync failures.
@github-actions
Copy link

github-actions bot commented Jan 30, 2026

OpenAPI Changes

Show/hide ## Changes for v0.yaml:
## Changes for v0.yaml:
1 changes: 0 error, 0 warning, 1 info
info	[response-property-enum-value-removed] at head/openapi/specs/v0.yaml	
	in API POST /api/v0/b2b/enroll/{readable_id}/
		removed the 'b2b-error-not-enrollable' enum value from the 'result/allOf[#/components/schemas/ResultEnum]/' response property for the response status '200'



## Changes for v1.yaml:
1 changes: 0 error, 0 warning, 1 info
info	[response-property-enum-value-removed] at head/openapi/specs/v1.yaml	
	in API POST /api/v0/b2b/enroll/{readable_id}/
		removed the 'b2b-error-not-enrollable' enum value from the 'result/allOf[#/components/schemas/ResultEnum]/' response property for the response status '200'



## Changes for v2.yaml:
1 changes: 0 error, 0 warning, 1 info
info	[response-property-enum-value-removed] at head/openapi/specs/v2.yaml	
	in API POST /api/v0/b2b/enroll/{readable_id}/
		removed the 'b2b-error-not-enrollable' enum value from the 'result/allOf[#/components/schemas/ResultEnum]/' response property for the response status '200'



Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

pre-commit-ci bot and others added 14 commits January 30, 2026 21:01
Add import of mitol.olposthog.features.is_enabled to courses.views v1 and v2. Update tests: remove an obsolete 404 assertion in courses/views/v2/views_test.py, and change users/views_test.py to mock settings.FEATURES.get (with updated comment) instead of users.views.is_enabled. These changes align imports and test mocking with the app's feature-flag handling.
@cp-at-mit cp-at-mit marked this pull request as ready for review February 2, 2026 23:52
Add a "# noqa: PLR0913" annotation to the test_create_user_ignore_edx_failures_flag definition in openedx/api_test.py to silence the linter (too many parameters) without refactoring the test signature. This keeps the test semantics unchanged while addressing CI lint failures.
@annagav annagav self-requested a review February 6, 2026 14:02
Copy link
Contributor

@annagav annagav left a comment

Choose a reason for hiding this comment

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

👍 LGTM after conflict resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants