Merge branch 'master' into pwnage101/ENT-11563

Author: kiram15

common/djangoapps/student/models/course_enrollment.py

@@ -1597,7 +1597,9 @@ class CourseEnrollmentAllowed(DeletableByUserValue, models.Model):
     the object is marked with the student who enrolled, to prevent students from changing e-mails and
     enrolling many accounts through the same e-mail.
 
-    .. no_pii:
+    .. pii: Contains email, redacted then deleted in AccountRetirementView
+    .. pii_types: email_address
+    .. pii_retirement: local_api
     """
     email = models.CharField(max_length=255, db_index=True)
     course_id = CourseKeyField(max_length=255, db_index=True)
@@ -1646,6 +1648,13 @@ def may_enroll_and_unenrolled(cls, course_id):
         """
         return CourseEnrollmentAllowed.objects.filter(course_id=course_id, user__isnull=True)
 
+    @classmethod
+    def redact_before_delete_fields(cls):
+        """
+        Redact email before deleting records for downstream soft-delete systems.
+        """
+        return {'email': 'redacted-before-delete@safe.com'}
+
 
 class CourseEnrollmentAttribute(models.Model):
     """

common/djangoapps/student/models/user.py

@@ -904,14 +904,21 @@ class PendingEmailChange(DeletableByUserValue, models.Model):  # noqa: DJ008
     """
     This model keeps track of pending requested changes to a user's email address.
 
-    .. pii: Contains new_email, retired in AccountRetirementView
+    .. pii: Contains new_email, redacted then deleted in AccountRetirementView
     .. pii_types: email_address
     .. pii_retirement: local_api
     """
     user = models.OneToOneField(User, unique=True, db_index=True, on_delete=models.CASCADE)
     new_email = models.CharField(blank=True, max_length=255, db_index=True)
     activation_key = models.CharField(('activation key'), max_length=32, unique=True, db_index=True)
 
+    @classmethod
+    def redact_before_delete_fields(cls):
+        """
+        Redact PII fields before delete in downstream soft-delete systems.
+        """
+        return {'new_email': 'redacted-before-delete@safe.com'}
+
     def request_change(self, email):
         """Request a change to a user's email.
 

common/djangoapps/student/tests/test_models.py

@@ -10,8 +10,10 @@
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser, User  # pylint: disable=imported-auth-user
 from django.core.cache import cache
+from django.db import connection
 from django.db.models.functions import Lower
 from django.test import TestCase, override_settings
+from django.test.utils import CaptureQueriesContext
 from edx_toggles.toggles.testutils import override_waffle_flag
 from freezegun import freeze_time
 from opaque_keys.edx.keys import CourseKey
@@ -42,7 +44,7 @@
 from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
 from openedx.core.djangoapps.schedules.models import Schedule
 from openedx.core.djangoapps.user_api.preferences.api import set_user_preference
-from openedx.core.djangolib.testing.utils import skip_unless_lms
+from openedx.core.djangolib.testing.utils import assert_redact_before_delete, skip_unless_lms
 from xmodule.modulestore import ModuleStoreEnum  # pylint: disable=wrong-import-order
 from xmodule.modulestore.tests.django_utils import (  # pylint: disable=wrong-import-order
     ModuleStoreTestCase,
@@ -600,6 +602,22 @@ def test_delete_by_user_no_effect_for_user_with_no_email_change(self):
         assert not record_was_deleted
         assert 1 == len(PendingEmailChange.objects.all())
 
+    def test_delete_by_user_value_redacts_pending_email_before_deletion(self):
+        """
+        Verify that delete_by_user_value redacts new_email before deletion.
+        """
+        table = PendingEmailChange._meta.db_table
+        with CaptureQueriesContext(connection) as ctx:
+            record_was_deleted = PendingEmailChange.delete_by_user_value(self.user, field='user')
+
+        assert_redact_before_delete(
+            [q['sql'] for q in ctx],
+            table=table,
+            expected_redacted_value_list=['redacted-before-delete@safe.com'],
+        )
+        assert record_was_deleted
+        assert not PendingEmailChange.objects.filter(user=self.user).exists()
+
 
 class TestCourseEnrollmentAllowed(ModuleStoreTestCase):  # pylint: disable=missing-class-docstring
 
@@ -615,11 +633,18 @@ def setUp(self):
         )
 
     def test_retiring_user_deletes_record(self):
-        is_successful = CourseEnrollmentAllowed.delete_by_user_value(
-            value=self.email,
-            field='email'
-        )
+        with CaptureQueriesContext(connection) as ctx:
+            is_successful = CourseEnrollmentAllowed.delete_by_user_value(
+                value=self.email,
+                field='email'
+            )
+
         assert is_successful
+        assert_redact_before_delete(
+            [q['sql'] for q in ctx],
+            table=CourseEnrollmentAllowed._meta.db_table,
+            expected_redacted_value_list=['redacted-before-delete@safe.com'],
+        )
         user_search_results = CourseEnrollmentAllowed.objects.filter(
             email=self.email
         )

common/djangoapps/student/views/management.py

@@ -963,14 +963,17 @@ def confirm_email_change(request, key):
 
         user.email = pec.new_email
         user.save()
-        pec.delete()
+
+        # Redact pending email before deletion.
+        PendingEmailChange.delete_by_user_value(user, field="user")
+
         # And send it to the new email...
-        msg.recipient = Recipient(user.id, pec.new_email)
+        msg.recipient = Recipient(user.id, user.email)
         try:
             ace.send(msg)
         except Exception:  # pylint: disable=broad-except
             log.warning('Unable to send confirmation email to new address', exc_info=True)
-            response = render_to_response("email_change_failed.html", {'email': pec.new_email})
+            response = render_to_response("email_change_failed.html", {'email': user.email})
             transaction.set_rollback(True)
             return response
 

common/djangoapps/util/tests/test_filters.py

@@ -3,6 +3,7 @@
 """
 from django.test import override_settings
 from openedx_filters import PipelineStep
+from openedx_filters.learning.filters import InstructorDashboardTabsRequested
 
 from common.djangoapps.util import course
 from openedx.core.djangolib.testing.utils import skip_unless_lms
@@ -76,3 +77,53 @@ def test_course_about_page_url_requested_without_filter_configuration(self):
         )
 
         self.assertEqual(expected_course_about, course_about_url)  # noqa: PT009
+
+
+class TestInstructorDashCustomTab(PipelineStep):
+    """
+    Utility class used when getting steps for pipeline.
+    """
+
+    def run_filter(self, tabs, user, course_key):  # pylint: disable=arguments-differ,unused-argument
+        """Pipeline step that appends a custom instructor dashboard tab."""
+        result = {
+            "tabs": tabs + [{
+                "tab_id": "custom",
+                "title": "Custom Tab",
+                "url": f"/courses/{course_key}/instructor/custom",
+                "sort_order": 999,
+            }],
+        }
+        return result
+
+
+class TestPreventTabsGenerationWithTabs(PipelineStep):
+    """
+    Pipeline step that raises PreventTabsGeneration with a custom tabs list.
+    Used to test that the exception handler in get_tabs uses exc.tabs when present.
+    """
+
+    def run_filter(self, tabs, user, course_key):  # pylint: disable=arguments-differ,unused-argument
+        """Pipeline step that raises PreventTabsGeneration with custom tabs."""
+        raise InstructorDashboardTabsRequested.PreventTabsGeneration(
+            "Preventing default tabs in favor of custom ones.",
+            tabs=[{
+                "tab_id": "plugin_tab",
+                "title": "Plugin Tab",
+                "url": f"/courses/{course_key}/instructor/plugin",
+                "sort_order": 5,
+            }],
+        )
+
+
+class TestPreventTabsGenerationWithoutTabs(PipelineStep):
+    """
+    Pipeline step that raises PreventTabsGeneration without a tabs list.
+    Used to test that the exception handler in get_tabs falls back to an empty list.
+    """
+
+    def run_filter(self, tabs, user, course_key):  # pylint: disable=arguments-differ,unused-argument
+        """Pipeline step that raises PreventTabsGeneration without providing tabs."""
+        raise InstructorDashboardTabsRequested.PreventTabsGeneration(
+            "Preventing all tabs from being generated."
+        )

common/static/data/geoip/GeoLite2-Country.mmdb

@@ -5,3 +5,5 @@ references/docstrings/common
 references/docstrings/lms
 references/docstrings/openedx
 references/docstrings/xmodule
+apps/index.rst
+decisions/app_decisions.rst

docs/.gitignore

@@ -338,7 +338,10 @@ def on_init(app):  # pylint: disable=redefined-outer-name, unused-argument
     avoid checking in the generated reStructuredText files.
     """
     repo_docs_build_path = f'{root}/docs/references/docs'
-    RepositoryDocs(root, repo_docs_build_path).build_rst_docs()
+    repo_docs = RepositoryDocs(root, repo_docs_build_path)
+    repo_docs.build_rst_docs()
+    repo_docs.build_apps_index(root / 'docs' / 'apps' / 'index.rst')
+    repo_docs.build_decisions_index(root / 'docs' / 'decisions' / 'app_decisions.rst')
 
     docs_path = root / 'docs'
     apidoc_path = 'sphinx-apidoc'

docs/conf.py

@@ -2,3 +2,4 @@
 
 .. toctree::
    0*
+   app_decisions

docs/decisions/index.rst

@@ -35,6 +35,7 @@ locations.
     concepts/index
     extensions/tinymce_plugins
     decisions/index
+    apps/index
 
 .. grid:: 1 2 2 2
    :gutter: 3
@@ -91,6 +92,18 @@ locations.
 
          Hooks Extensions Framework
 
+   .. grid-item-card:: App Documentation
+      :class-card: sd-shadow-md sd-p-2
+      :class-footer: sd-border-0
+
+      * :doc:`apps/index`
+      * :doc:`decisions/app_decisions`
+      +++
+      .. button-ref:: apps/index
+         :color: primary
+         :outline:
+         :expand:
+
 
 Change History
 **************