From 209ff5049e8fecbccfb3bdba5bb4dbe76a1c5914 Mon Sep 17 00:00:00 2001 From: KEVYN SUAREZ <91025555+efortish@users.noreply.github.com> Date: Wed, 19 Aug 2026 12:14:08 -0500 Subject: [PATCH] revert: Cannot rerun courses - authz role assignment expects rerun to already exist (#38973) (cherry picked from commit 07f09868c1f851dc038988ca190f915db4c81438) (cherry picked from commit 6aeb002e77ac091e8f2d4d9e684b799154dbb63d) --- cms/djangoapps/contentstore/tasks.py | 10 +--- .../contentstore/tests/test_clone_course.py | 55 ++----------------- .../tests/test_course_create_rerun.py | 27 +++------ cms/djangoapps/contentstore/views/course.py | 28 ++-------- 4 files changed, 18 insertions(+), 102 deletions(-) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index c97d3cba4209..b73800053288 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -56,7 +56,6 @@ from cms.djangoapps.contentstore.toggles import enable_course_optimizer_check_prev_run_links from cms.djangoapps.contentstore.utils import ( IMPORTABLE_FILE_TYPES, - add_instructor, contains_course_reference, create_course_info_usage_key, create_or_update_xblock_upstream_link, @@ -189,14 +188,7 @@ def rerun_course(source_course_key_string, destination_course_key_string, user_i update_unit_discussion_state_from_discussion_blocks(destination_course_key, user_id) # set initial permissions for the user to access the course. - # NOTE: add_instructor is called here (after clone_course) because when - # authz.enable_course_authoring is enabled, it cannot be called pre-task - # (CourseOverview doesn't exist yet). This is a temporary workaround until - # openedx/openedx-authz#352 is implemented. Once resolved, add_instructor - # can move back to the pre-task call site unconditionally. - user = User.objects.get(id=user_id) - add_instructor(destination_course_key, user, user) - initialize_permissions(destination_course_key, user) + initialize_permissions(destination_course_key, User.objects.get(id=user_id)) # update state: Succeeded CourseRerunState.objects.succeeded(course_key=destination_course_key) diff --git a/cms/djangoapps/contentstore/tests/test_clone_course.py b/cms/djangoapps/contentstore/tests/test_clone_course.py index de9860a80b5e..2449c9269288 100644 --- a/cms/djangoapps/contentstore/tests/test_clone_course.py +++ b/cms/djangoapps/contentstore/tests/test_clone_course.py @@ -14,11 +14,10 @@ from common.djangoapps.course_action_state.managers import CourseRerunUIStateManager from common.djangoapps.course_action_state.models import CourseRerunState from common.djangoapps.student.auth import has_course_author_access -from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole -from xmodule.contentstore.content import StaticContent # pylint: disable=wrong-import-order -from xmodule.contentstore.django import contentstore # pylint: disable=wrong-import-order -from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum # pylint: disable=wrong-import-order -from xmodule.modulestore.tests.factories import CourseFactory # pylint: disable=wrong-import-order +from xmodule.contentstore.content import StaticContent # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT @@ -142,49 +141,3 @@ def test_rerun_course(self): course_key=split_course4_id, state=CourseRerunUIStateManager.State.FAILED ) - - - def test_rerun_course_grants_instructor_access(self): - """ - Test that the rerun_course task grants instructor and staff access - to the user after cloning. This verifies add_instructor is called - inside the task (needed when authz.enable_course_authoring is enabled - and add_instructor cannot be called pre-task). - - TODO: This test covers a temporary workaround until openedx/openedx-authz#352 - is implemented. Once authz supports pre-assigning roles without a CourseOverview, - add_instructor can move back to the pre-task call site and this test can be - simplified. - """ - org = 'edX' - course_number = 'CS101' - course_run = '2025_Q1' - display_name = 'rerun_instructor_test' - fields = {'display_name': display_name} - - # Create a source course - source_course = CourseFactory.create( - org=org, - number=course_number, - run=course_run, - display_name=display_name, - default_store=ModuleStoreEnum.Type.split, - ) - - dest_course_id = CourseLocator(org=org, course=course_number, run="instructor_rerun") - CourseRerunState.objects.initiated( - source_course.id, dest_course_id, self.user, fields['display_name'] - ) - - result = rerun_course.delay( - str(source_course.id), - str(dest_course_id), - self.user.id, - json.dumps(fields, cls=EdxJSONEncoder), - ) - assert result.get() == "succeeded" - - # Verify the user has instructor and staff access on the new course - assert has_course_author_access(self.user, dest_course_id) - assert CourseInstructorRole(dest_course_id).has_user(self.user) - assert CourseStaffRole(dest_course_id).has_user(self.user) diff --git a/cms/djangoapps/contentstore/tests/test_course_create_rerun.py b/cms/djangoapps/contentstore/tests/test_course_create_rerun.py index 52359dd7c27a..b9a22cfaf46c 100644 --- a/cms/djangoapps/contentstore/tests/test_course_create_rerun.py +++ b/cms/djangoapps/contentstore/tests/test_course_create_rerun.py @@ -426,18 +426,10 @@ class TestCourseRerunAuthz( """ Tests for course rerun behavior when authz.enable_course_authoring is enabled. - Verifies that: - - Rerun succeeds without calling add_instructor pre-task (which would fail - because CourseOverview doesn't exist yet). - - add_instructor is called in the task after clone_course, when CourseOverview - can be resolved. - - The initiating user can see in-process rerun status via created_user check. - - TODO: These tests cover a temporary workaround needed while (1) the authz system - doesn't support pre-assigning roles without a CourseOverview, and (2) we support - both authz and legacy systems simultaneously. Once openedx/openedx-authz#352 is - implemented, this class can be simplified — the conditional skip of add_instructor - and the created_user visibility fallback will no longer be needed. + Verifies that rerun succeeds and grants proper access even though add_instructor + is called for the destination course key before its CourseOverview exists + (openedx-authz#352 makes role assignment succeed ahead of the object, backfilling + the link once clone_course creates it). """ def setUp(self): @@ -467,8 +459,8 @@ def setUp(self): def test_rerun_succeeds_with_authz_enabled(self): """ Test that course rerun completes successfully when authz.enable_course_authoring - is enabled. Previously this would fail with CourseOverview.DoesNotExist because - add_instructor was called before the course was cloned. + is enabled, granting the initiating user instructor/staff access on the + destination course before it's cloned. """ response = self.staff_client.ajax_post(self.url, { 'source_course_key': str(self.source_course_key), @@ -516,8 +508,7 @@ def test_rerun_does_not_create_legacy_roles_with_authz_enabled(self): def test_rerun_grants_authz_permissions_after_clone(self): """ Test that after a successful rerun with authz enabled, the user has proper - permissions via the authz layer (add_instructor is called in the task after - clone_course completes and CourseOverview is available). + permissions via the authz layer on the newly created destination course. """ response = self.staff_client.ajax_post(self.url, { 'source_course_key': str(self.source_course_key), @@ -537,8 +528,8 @@ def test_rerun_grants_authz_permissions_after_clone(self): def test_in_process_rerun_visible_to_initiating_user(self): """ Test that the user who initiated the rerun can see the in-process status - via the created_user check in get_in_process_course_actions, even when - authz permissions haven't been fully established yet. + in get_in_process_course_actions, via the instructor/staff access already + granted on the destination course. """ dest_course_key = CourseLocator(org='testorg', course='101', run='in_progress_rerun') CourseRerunState.objects.initiated( diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 2ee588195210..3c9c36bb00b9 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -429,19 +429,8 @@ def get_in_process_course_actions(request): exclude_args={'state': CourseRerunUIStateManager.State.SUCCEEDED}, should_display=True, ) - if ( - # The user who initiated the rerun can always see its status. - # This is needed because when the authz flag is enabled, permission - # checks require a CourseOverview which doesn't exist until the - # rerun task clones the course. - # TODO: This created_user fallback is a temporary workaround until - # openedx/openedx-authz#352 is implemented. Once authz supports - # pre-assigning roles without a CourseOverview, this check can be removed - # and the standard permission check will suffice. - course.created_user == request.user - or user_has_course_permission( - request.user, COURSES_VIEW_COURSE.identifier, course.course_key, LegacyAuthoringPermission.READ - ) + if user_has_course_permission( + request.user, COURSES_VIEW_COURSE.identifier, course.course_key, LegacyAuthoringPermission.READ ) ] @@ -1345,17 +1334,8 @@ def rerun_course(user, source_course_key, org, number, run, fields, background=T raise PermissionDenied() # Make sure user has instructor and staff access to the destination course - # so the user can see the updated status for that course. - # When authz is enabled, we skip this because the authz layer requires a - # CourseOverview (which doesn't exist until the course is cloned in the task). - # In that case, visibility of the rerun status is granted by checking - # created_user on CourseRerunState instead. - # TODO: This conditional is a temporary workaround until openedx/openedx-authz#352 - # is implemented (pre-assigning roles without a CourseOverview). Once resolved, - # add_instructor can be called unconditionally here and the created_user fallback - # in get_in_process_course_actions can be removed. - if not enable_authz_course_authoring(destination_course_key): - add_instructor(destination_course_key, user, user) + # so the user can see the updated status for that course + add_instructor(destination_course_key, user, user) # Mark the action as initiated CourseRerunState.objects.initiated(source_course_key, destination_course_key, user, fields['display_name'])