Merge branch 'master' into fix/remove-nonfunctional-retirement-code

Author: e0d

lms/djangoapps/bulk_email/tests/test_course_optout.py

@@ -13,19 +13,25 @@
 from edx_ace.message import Message
 from edx_ace.policy import PolicyResult
 from edx_ace.recipient import Recipient
+from edx_toggles.toggles.testutils import override_waffle_flag
 
 from common.djangoapps.student.models import CourseEnrollment
 from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
 from lms.djangoapps.bulk_email.api import get_unsubscribed_link
 from lms.djangoapps.bulk_email.models import BulkEmailFlag
 from lms.djangoapps.bulk_email.policies import CourseEmailOptout
+from lms.djangoapps.instructor.toggles import LEGACY_INSTRUCTOR_DASHBOARD
 from xmodule.modulestore.tests.django_utils import (
     ModuleStoreTestCase,  # lint-amnesty, pylint: disable=wrong-import-order
 )
 from xmodule.modulestore.tests.factories import CourseFactory  # lint-amnesty, pylint: disable=wrong-import-order
 
 
 @patch('lms.djangoapps.bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message', autospec=True))  # lint-amnesty, pylint: disable=line-too-long
+# Tests for legacy views. When DEPR-38432 is picked up, these tests will require the following changes:
+# Either remove or leave the specific parts that reference the legacy instructor dashboard,
+# and remove the override_waffle_flag for LEGACY_INSTRUCTOR_DASHBOARD.
+@override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
 class TestOptoutCourseEmails(ModuleStoreTestCase):
     """
     Test that optouts are referenced in sending course email.

lms/djangoapps/bulk_email/tests/test_email.py

@@ -16,6 +16,7 @@
 from django.test.utils import override_settings
 from django.urls import reverse
 from django.utils.translation import get_language
+from edx_toggles.toggles.testutils import override_waffle_flag
 from markupsafe import escape
 
 from common.djangoapps.course_modes.models import CourseMode
@@ -29,6 +30,7 @@
 )
 from lms.djangoapps.bulk_email.messages import ACEEmail
 from lms.djangoapps.bulk_email.tasks import _get_course_email_context, _get_source_address
+from lms.djangoapps.instructor.toggles import LEGACY_INSTRUCTOR_DASHBOARD
 from lms.djangoapps.instructor_task.subtasks import update_subtask_status
 from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort
 from openedx.core.djangoapps.course_groups.models import CourseCohort
@@ -62,6 +64,10 @@ def mock_update_subtask_status(entry_id, current_task_id, new_subtask_status):
         return mock_update_subtask_status
 
 
+# Tests for legacy views. When DEPR-38432 is picked up, these tests will require the following changes:
+# Either remove or leave the specific parts that reference the legacy instructor dashboard,
+# and remove the override_waffle_flag for LEGACY_INSTRUCTOR_DASHBOARD.
+@override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
 class EmailSendFromDashboardTestCase(SharedModuleStoreTestCase):
     """
     Test that emails send correctly.

lms/djangoapps/bulk_email/tests/test_signals.py

@@ -9,19 +9,25 @@
 from django.core import mail
 from django.core.management import call_command
 from django.urls import reverse
+from edx_toggles.toggles.testutils import override_waffle_flag
 from opaque_keys.edx.keys import CourseKey
 
 from common.djangoapps.student.models import CourseEnrollment
 from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
 from lms.djangoapps.bulk_email.models import BulkEmailFlag, Optout
 from lms.djangoapps.bulk_email.signals import force_optout_all
+from lms.djangoapps.instructor.toggles import LEGACY_INSTRUCTOR_DASHBOARD
 from xmodule.modulestore.tests.django_utils import (
     ModuleStoreTestCase,  # lint-amnesty, pylint: disable=wrong-import-order
 )
 from xmodule.modulestore.tests.factories import CourseFactory  # lint-amnesty, pylint: disable=wrong-import-order
 
 
 @patch('lms.djangoapps.bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message', autospec=True))  # lint-amnesty, pylint: disable=line-too-long
+# Tests for legacy views. When DEPR-38432 is picked up, these tests will require the following changes:
+# Either remove or leave the specific parts that reference the legacy instructor dashboard,
+# and remove the override_waffle_flag for LEGACY_INSTRUCTOR_DASHBOARD.
+@override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
 class TestOptoutCourseEmailsBySignal(ModuleStoreTestCase):
     """
     Tests that the force_optout_all signal receiver opts the user out of course emails

lms/djangoapps/courseware/tests/test_view_authentication.py

@@ -8,6 +8,7 @@
 
 import pytz
 from django.urls import reverse
+from edx_toggles.toggles.testutils import override_waffle_flag
 
 from common.djangoapps.student.tests.factories import (
     BetaTesterFactory,
@@ -21,6 +22,7 @@
 )
 from lms.djangoapps.courseware.access import has_access
 from lms.djangoapps.courseware.tests.helpers import CourseAccessTestMixin, LoginEnrollmentTestCase
+from lms.djangoapps.instructor.toggles import LEGACY_INSTRUCTOR_DASHBOARD
 from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseTestConsentRequired
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -92,7 +94,11 @@ def _check_staff(self, course):
             for index in range(len(course.textbooks))
         ])
         for url in urls:
-            self.assert_request_status_code(200, url)
+            # Instructor dashboard returns 302 (MFE redirect) by default
+            if 'instructor' in url:
+                self.assert_request_status_code(302, url)
+            else:
+                self.assert_request_status_code(200, url)
 
         # The student progress tab is not accessible to a student
         # before launch, so the instructor view-as-student feature
@@ -109,6 +115,58 @@ def _check_staff(self, course):
         )
         self.assert_request_status_code(302, url)
 
+    def _check_staff_legacy(self, course):
+        """
+        Check that access is right for staff in course with legacy instructor dashboard enabled.
+        """
+        names = ['about_course', 'instructor_dashboard', 'progress']
+        urls = self._reverse_urls(names, course)
+        urls.extend([
+            reverse('book', kwargs={'course_id': str(course.id),
+                                    'book_index': index})
+            for index in range(len(course.textbooks))
+        ])
+        for url in urls:
+            # With legacy flag enabled, all URLs return 200 (instructor dashboard skips MFE redirect)
+            self.assert_request_status_code(200, url)
+
+        # The student progress tab behavior is affected by legacy flag in normal scenarios
+        url = reverse(
+            'student_progress',
+            kwargs={
+                'course_id': str(course.id),
+                'student_id': self.enrolled_user.id,
+            }
+        )
+        self.assert_request_status_code(200, url)
+
+    def _check_staff_legacy_dark_launch(self, course):
+        """
+        Check staff access during dark launch with legacy instructor dashboard enabled.
+        In dark launch scenarios, student progress URL still returns 302 even with legacy flag.
+        """
+        names = ['about_course', 'instructor_dashboard', 'progress']
+        urls = self._reverse_urls(names, course)
+        urls.extend([
+            reverse('book', kwargs={'course_id': str(course.id),
+                                    'book_index': index})
+            for index in range(len(course.textbooks))
+        ])
+        for url in urls:
+            # With legacy flag enabled, all URLs return 200 (instructor dashboard skips MFE redirect)
+            self.assert_request_status_code(200, url)
+
+        # In dark launch scenarios, student progress URL still returns 302 even with legacy flag
+        # because course access restrictions take precedence
+        url = reverse(
+            'student_progress',
+            kwargs={
+                'course_id': str(course.id),
+                'student_id': self.enrolled_user.id,
+            }
+        )
+        self.assert_request_status_code(302, url)
+
     def login(self, user):  # lint-amnesty, pylint: disable=arguments-differ
         return super().login(user.email, self.TEST_PASSWORD)
 
@@ -186,7 +244,7 @@ def test_staff_course_access(self):
 
         # Now should be able to get to self.course, but not  self.test_course
         url = reverse('instructor_dashboard', kwargs={'course_id': str(self.course.id)})
-        self.assert_request_status_code(200, url)
+        self.assert_request_status_code(302, url)
 
         url = reverse('instructor_dashboard', kwargs={'course_id': str(self.test_course.id)})
         self.assert_request_status_code(404, url)
@@ -200,7 +258,7 @@ def test_instructor_course_access(self):
 
         # Now should be able to get to self.course, but not  self.test_course
         url = reverse('instructor_dashboard', kwargs={'course_id': str(self.course.id)})
-        self.assert_request_status_code(200, url)
+        self.assert_request_status_code(302, url)
 
         url = reverse('instructor_dashboard', kwargs={'course_id': str(self.test_course.id)})
         self.assert_request_status_code(404, url)
@@ -212,10 +270,10 @@ def test_org_staff_access(self):
         """
         self.login(self.org_staff_user)
         url = reverse('instructor_dashboard', kwargs={'course_id': str(self.course.id)})
-        self.assert_request_status_code(200, url)
+        self.assert_request_status_code(302, url)
 
         url = reverse('instructor_dashboard', kwargs={'course_id': str(self.test_course.id)})
-        self.assert_request_status_code(200, url)
+        self.assert_request_status_code(302, url)
 
         url = reverse('instructor_dashboard', kwargs={'course_id': str(self.other_org_course.id)})
         self.assert_request_status_code(404, url)
@@ -227,10 +285,10 @@ def test_org_instructor_access(self):
         """
         self.login(self.org_instructor_user)
         url = reverse('instructor_dashboard', kwargs={'course_id': str(self.course.id)})
-        self.assert_request_status_code(200, url)
+        self.assert_request_status_code(302, url)
 
         url = reverse('instructor_dashboard', kwargs={'course_id': str(self.test_course.id)})
-        self.assert_request_status_code(200, url)
+        self.assert_request_status_code(302, url)
 
         url = reverse('instructor_dashboard', kwargs={'course_id': str(self.other_org_course.id)})
         self.assert_request_status_code(404, url)
@@ -242,12 +300,94 @@ def test_global_staff_access(self):
         self.login(self.global_staff_user)
 
         # and now should be able to load both
+        urls = [reverse('instructor_dashboard', kwargs={'course_id': str(self.course.id)}),
+                reverse('instructor_dashboard', kwargs={'course_id': str(self.test_course.id)})]
+
+        for url in urls:
+            self.assert_request_status_code(302, url)
+
+    # Legacy instructor dashboard tests (with waffle flag enabled, expect 200 responses)
+
+    @override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
+    def test_staff_course_access_legacy(self):
+        """
+        Verify staff can load the legacy instructor dashboard (expects 200 response).
+        """
+        self.login(self.staff_user)
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': str(self.course.id)})
+        self.assert_request_status_code(200, url)
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': str(self.test_course.id)})
+        self.assert_request_status_code(404, url)
+
+    @override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
+    def test_instructor_course_access_legacy(self):
+        """
+        Verify instructor can load the legacy instructor dashboard (expects 200 response).
+        """
+        self.login(self.instructor_user)
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': str(self.course.id)})
+        self.assert_request_status_code(200, url)
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': str(self.test_course.id)})
+        self.assert_request_status_code(404, url)
+
+    @override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
+    def test_org_staff_access_legacy(self):
+        """
+        Verify org staff can load the legacy instructor dashboard (expects 200 response).
+        """
+        self.login(self.org_staff_user)
+        url = reverse('instructor_dashboard', kwargs={'course_id': str(self.course.id)})
+        self.assert_request_status_code(200, url)
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': str(self.test_course.id)})
+        self.assert_request_status_code(200, url)
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': str(self.other_org_course.id)})
+        self.assert_request_status_code(404, url)
+
+    @override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
+    def test_org_instructor_access_legacy(self):
+        """
+        Verify org instructor can load the legacy instructor dashboard (expects 200 response).
+        """
+        self.login(self.org_instructor_user)
+        url = reverse('instructor_dashboard', kwargs={'course_id': str(self.course.id)})
+        self.assert_request_status_code(200, url)
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': str(self.test_course.id)})
+        self.assert_request_status_code(200, url)
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': str(self.other_org_course.id)})
+        self.assert_request_status_code(404, url)
+
+    @override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
+    def test_global_staff_access_legacy(self):
+        """
+        Verify the global staff user can access the legacy instructor dashboard (expects 200 response).
+        """
+        self.login(self.global_staff_user)
+
         urls = [reverse('instructor_dashboard', kwargs={'course_id': str(self.course.id)}),
                 reverse('instructor_dashboard', kwargs={'course_id': str(self.test_course.id)})]
 
         for url in urls:
             self.assert_request_status_code(200, url)
 
+    @override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
+    def test_staff_method_legacy(self):
+        """
+        Test the _check_staff_legacy helper method with legacy flag enabled (expects 200 response).
+        """
+        self.login(self.staff_user)
+        self.enroll(self.course, True)
+
+        # Test the _check_staff_legacy method which includes instructor dashboard checks
+        self._check_staff_legacy(self.course)
+
     @patch.dict('lms.djangoapps.courseware.access.settings.FEATURES', {'DISABLE_START_DATES': False})
     def test_dark_launch_enrolled_student(self):
         """
@@ -355,6 +495,52 @@ def test_enrollment_period(self):
         self.login(self.global_staff_user)
         assert self.enroll(self.course)
 
+    @override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
+    @patch.dict('lms.djangoapps.courseware.access.settings.FEATURES', {'DISABLE_START_DATES': False})
+    def test_dark_launch_instructor_legacy(self):
+        """
+        Make sure that before course start instructors can access the
+        page for their course with legacy instructor dashboard enabled.
+        """
+        now = datetime.datetime.now(pytz.UTC)
+        tomorrow = now + datetime.timedelta(days=1)
+        self.course.start = tomorrow
+        self.test_course.start = tomorrow
+        self.course = self.update_course(self.course, self.user.id)
+        self.test_course = self.update_course(self.test_course, self.user.id)
+
+        self.login(self.instructor_user)
+        # Enroll in the classes---can't see courseware otherwise.
+        self.enroll(self.course, True)
+        self.enroll(self.test_course, True)
+
+        # should now be able to get to everything for self.course
+        self._check_staff_legacy_dark_launch(self.course)
+        self._check_non_staff_dark(self.test_course)
+
+    @override_waffle_flag(LEGACY_INSTRUCTOR_DASHBOARD, active=True)
+    @patch.dict('lms.djangoapps.courseware.access.settings.FEATURES', {'DISABLE_START_DATES': False})
+    def test_dark_launch_global_staff_legacy(self):
+        """
+        Make sure that before course start staff can access
+        course pages with legacy instructor dashboard enabled.
+        """
+        now = datetime.datetime.now(pytz.UTC)
+        tomorrow = now + datetime.timedelta(days=1)
+
+        self.course.start = tomorrow
+        self.test_course.start = tomorrow
+        self.course = self.update_course(self.course, self.user.id)
+        self.test_course = self.update_course(self.test_course, self.user.id)
+
+        self.login(self.global_staff_user)
+        self.enroll(self.course, True)
+        self.enroll(self.test_course, True)
+
+        # and now should be able to load both
+        self._check_staff_legacy_dark_launch(self.course)
+        self._check_staff_legacy_dark_launch(self.test_course)
+
 
 class TestBetatesterAccess(ModuleStoreTestCase, CourseAccessTestMixin):
     """