revert: Cannot rerun courses - authz role assignment expects rerun to already exist (#38973)

(cherry picked from commit 07f0986)
(cherry picked from commit 6aeb002)

Author: efortish

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)

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)

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(

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'])