feat: TNL-10136 Propagate changes from last PR

2022-11-25 07:36:43 -05:00
parent d5a5e25fef
commit 377edc2f4d
17 changed files with 47 additions and 58 deletions
--- a/common/djangoapps/student/models/init.py
+++ b/common/djangoapps/student/models/init.py
@@ -1,5 +1,6 @@
 '''
 Student models migrated to folder to tease out the course enrollment aspects from Student
 '''
-from .user import *
 from .course_enrollment import *
+from .user import *
+
--- a/common/djangoapps/student/models/course_enrollment.py
+++ b/common/djangoapps/student/models/course_enrollment.py
@@ -34,11 +34,6 @@ from requests.exceptions import HTTPError, RequestException
 from simple_history.models import HistoricalRecords

 from common.djangoapps.course_modes.models import CourseMode, get_cosmetic_verified_display_price
-from common.djangoapps.student.email_helpers import (
-    generate_proctoring_requirements_email_context,
-    should_send_proctoring_requirements_email,
-)
-from common.djangoapps.student.emails import send_proctoring_requirements_email
 from common.djangoapps.student.signals import ENROLL_STATUS_CHANGE, ENROLLMENT_TRACK_UPDATED, UNENROLL_DONE
 from common.djangoapps.track import contexts, segment
 from common.djangoapps.util.query import use_read_replica_if_available
@@ -512,6 +507,12 @@ class CourseEnrollment(models.Model):
                )

        if mode_changed:
+            from common.djangoapps.student.email_helpers import (
+                generate_proctoring_requirements_email_context,
+                should_send_proctoring_requirements_email,
+            )
+            from common.djangoapps.student.emails import send_proctoring_requirements_email
+
            # If mode changed to one that requires proctoring, send proctoring requirements email
            if should_send_proctoring_requirements_email(self.user.username, self.course_id):
                email_context = generate_proctoring_requirements_email_context(self.user, self.course_id)
--- a/common/djangoapps/student/models/user.py
+++ b/common/djangoapps/student/models/user.py
@@ -11,6 +11,7 @@ file and check it in at the same time as your model changes. To do that,
 3. Add the migration file created in edx-platform/common/djangoapps/student/migrations/
 """

+import crum
 import hashlib  # lint-amnesty, pylint: disable=wrong-import-order
 import json  # lint-amnesty, pylint: disable=wrong-import-order
 import logging  # lint-amnesty, pylint: disable=wrong-import-order
@@ -20,13 +21,12 @@ from functools import total_ordering  # lint-amnesty, pylint: disable=wrong-impo
 from importlib import import_module  # lint-amnesty, pylint: disable=wrong-import-order
 from urllib.parse import unquote, urlencode

-import crum
-
 from .course_enrollment import (
    ALLOWEDTOENROLL_TO_ENROLLED,
    CourseEnrollment,
    CourseEnrollmentAllowed,
-    ManualEnrollmentAudit
+    ManualEnrollmentAudit,
+    segment
 )

 from config_models.models import ConfigurationModel
@@ -54,7 +54,6 @@ from pytz import UTC, timezone
 from user_util import user_util

 import openedx.core.djangoapps.django_comment_common.comment_client as cc
-from common.djangoapps.track import segment
 from common.djangoapps.util.model_utils import emit_field_changed_events, get_changed_fields_dict
 from lms.djangoapps.courseware.toggles import streak_celebration_is_active
 from openedx.core.djangoapps.signals.signals import USER_ACCOUNT_ACTIVATED
--- a/common/djangoapps/student/tests/test_activate_account.py
+++ b/common/djangoapps/student/tests/test_activate_account.py
@@ -77,7 +77,7 @@ class TestActivateAccount(TestCase):
        assert self.user.is_active
        assert not mock_segment_identify.called

-    @patch('common.djangoapps.student.models.USER_ACCOUNT_ACTIVATED')
+    @patch('common.djangoapps.student.models.user.USER_ACCOUNT_ACTIVATED')
    def test_activation_signal(self, mock_signal):
        """
        Verify that USER_ACCOUNT_ACTIVATED is emitted upon account email activation.
--- a/common/djangoapps/student/tests/test_enrollment.py
+++ b/common/djangoapps/student/tests/test_enrollment.py
@@ -242,7 +242,7 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase, OpenEdxEventsTestMixin)
        requirements should be sent. The email should not be sent for non-verified modes.
        """
        with patch(
-            'common.djangoapps.student.models.send_proctoring_requirements_email',
+            'common.djangoapps.student.emails.send_proctoring_requirements_email',
            return_value=None
        ) as mock_send_email:
            # First enroll in a non-proctored course. This should not trigger the email.
@@ -259,7 +259,7 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase, OpenEdxEventsTestMixin)
        any proctored exams, they should not receive a proctoring requirements email.
        """
        with patch(
-            'common.djangoapps.student.models.send_proctoring_requirements_email',
+            'common.djangoapps.student.emails.send_proctoring_requirements_email',
            return_value=None
        ) as mock_send_email:
            CourseEnrollment.enroll(
@@ -274,7 +274,7 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase, OpenEdxEventsTestMixin)
        should be sent.
        """
        with patch(
-            'common.djangoapps.student.models.send_proctoring_requirements_email',
+            'common.djangoapps.student.emails.send_proctoring_requirements_email',
            return_value=None
        ) as mock_send_email:
            enrollment = CourseEnrollment.enroll(
@@ -293,7 +293,7 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase, OpenEdxEventsTestMixin)
        enroll in honor mode for a course with proctored exams.
        """
        with patch(
-            'common.djangoapps.student.models.send_proctoring_requirements_email',
+            'common.djangoapps.student.emails.send_proctoring_requirements_email',
            return_value=None
        ) as mock_send_email:
            course_honor_mode = CourseFactory(
--- a/common/djangoapps/student/tests/test_events.py
+++ b/common/djangoapps/student/tests/test_events.py
@@ -94,7 +94,7 @@ class TestUserProfileEvents(UserSettingsEventTestMixin, TestCase):
        self.profile.save()
        self.assert_no_events_were_emitted()

-    @mock.patch('common.djangoapps.student.models.UserProfile.save', side_effect=IntegrityError)
+    @mock.patch('common.djangoapps.student.models.user.UserProfile.save', side_effect=IntegrityError)
    def test_no_event_if_save_failed(self, _save_mock):
        """
        Verify no event is triggered if the save does not complete. Note that the pre_save
--- a/common/djangoapps/student/tests/test_models.py
+++ b/common/djangoapps/student/tests/test_models.py
@@ -846,7 +846,7 @@ class TestUserPostSaveCallback(SharedModuleStoreTestCase):
            last_name='Person',
            email='some.user@example.com',
        )
-        with mock.patch('common.djangoapps.student.models.course_enrollment.segment') as mock_segment:
+        with mock.patch('common.djangoapps.student.models.user.segment') as mock_segment:
            user._called_by_management_command = True  # pylint: disable=protected-access
            user.save()

--- a/common/djangoapps/student/tests/test_refunds.py
+++ b/common/djangoapps/student/tests/test_refunds.py
@@ -156,7 +156,8 @@ class RefundableTest(SharedModuleStoreTestCase):
            value=self.ORDER_NUMBER
        )

-        with patch('common.djangoapps.student.models.EnrollmentRefundConfiguration.current') as config:
+        CONFIG_METHOD_NAME = 'common.djangoapps.student.models.course_enrollment.EnrollmentRefundConfiguration.current'
+        with patch(CONFIG_METHOD_NAME) as config:
            instance = config.return_value
            instance.refund_window = refund_period
            assert self.enrollment.refund_cutoff_date() == (expected_date + refund_period)
--- a/common/djangoapps/student/tests/test_retirement.py
+++ b/common/djangoapps/student/tests/test_retirement.py
@@ -15,8 +15,6 @@ from django.urls import reverse

 from openedx.core.djangolib.testing.utils import skip_unless_lms
 from common.djangoapps.student.models import (
-    _get_all_retired_emails_by_email,
-    _get_all_retired_usernames_by_username,
    get_potentially_retired_user_by_username,
    get_potentially_retired_user_by_username_and_hash,
    get_retired_email_by_email,
@@ -112,7 +110,7 @@ def test_get_all_retired_usernames_by_username(retirement_user):  # lint-amnesty
    Check that all salts are used for this method and return expected
    formats.
    """
-    hashed_usernames = list(_get_all_retired_usernames_by_username(retirement_user.username))
+    hashed_usernames = list(get_all_retired_usernames_by_username(retirement_user.username))
    assert len(hashed_usernames) == len(settings.RETIRED_USER_SALTS)

    for hashed_username in hashed_usernames:
@@ -182,21 +180,6 @@ def test_get_retired_email_status_exists(retirement_user, retirement_status):  #
    assert retirement_status.retired_email == hashed_email


-def test_get_all_retired_email_by_email(retirement_user):  # lint-amnesty, pylint: disable=redefined-outer-name
-    """
-    Check that all salts are used for this method and return expected
-    formats.
-    """
-    hashed_emails = list(_get_all_retired_emails_by_email(retirement_user.email))
-    assert len(hashed_emails) == len(settings.RETIRED_USER_SALTS)
-
-    for hashed_email in hashed_emails:
-        check_email_against_fmt(hashed_email)
-
-    # Make sure hashes are unique
-    assert len(hashed_emails) == len(set(hashed_emails))
-
-
 def test_get_correct_user_varying_by_case_only(two_users_same_username_different_case):  # lint-amnesty, pylint: disable=redefined-outer-name
    """
    Check that two users - one retired, one active - with the same username except for case can be found.
--- a/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py
+++ b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py
@@ -100,7 +100,8 @@ class CourseHomeMetadataTests(BaseCourseHomeTests):
        """ Test that metadata endpoint returns data for the streak celebration """
        CourseEnrollment.enroll(self.user, self.course.id, 'audit')
        with override_waffle_flag(COURSEWARE_MFE_MILESTONES_STREAK_DISCOUNT, active=True):
-            with mock.patch('common.djangoapps.student.models.UserCelebration.perform_streak_updates', return_value=3):
+            UPDATES_METHOD_NAME = 'common.djangoapps.student.models.user.UserCelebration.perform_streak_updates'
+            with mock.patch(UPDATES_METHOD_NAME, return_value=3):
                response = self.client.get(self.url, content_type='application/json')
                celebrations = response.json()['celebrations']
                assert celebrations['streak_length_to_celebrate'] == 3
--- a/lms/djangoapps/discussion/django_comment_client/base/tests.py
+++ b/lms/djangoapps/discussion/django_comment_client/base/tests.py
@@ -238,7 +238,7 @@ class ViewsTestCaseMixin:

        # Patch the comment client user save method so it does not try
        # to create a new cc user when creating a django user
-        with patch('common.djangoapps.student.models.cc.User.save'):
+        with patch('common.djangoapps.student.models.user.cc.User.save'):
            uname = 'student'
            email = 'student@edx.org'
            self.password = 'test'
@@ -461,7 +461,7 @@ class ViewsTestCase(

        # Patch the comment client user save method so it does not try
        # to create a new cc user when creating a django user
-        with patch('common.djangoapps.student.models.cc.User.save'):
+        with patch('common.djangoapps.student.models.user.cc.User.save'):
            uname = 'student'
            email = 'student@edx.org'
            self.password = 'test'
--- a/lms/djangoapps/discussion/tests/test_tasks.py
+++ b/lms/djangoapps/discussion/tests/test_tasks.py
@@ -78,7 +78,7 @@ class TaskTestCase(ModuleStoreTestCase):  # lint-amnesty, pylint: disable=missin

        # Patch the comment client user save method so it does not try
        # to create a new cc user when creating a django user
-        with mock.patch('common.djangoapps.student.models.cc.User.save'):
+        with mock.patch('common.djangoapps.student.models.user.cc.User.save'):
            cls.thread_author = UserFactory(
                username='thread_author',
                password='password',
--- a/lms/djangoapps/discussion/tests/test_views.py
+++ b/lms/djangoapps/discussion/tests/test_views.py
@@ -91,7 +91,7 @@ class ViewsExceptionTestCase(UrlResetMixin, ModuleStoreTestCase):  # lint-amnest

        # Patch the comment client user save method so it does not try
        # to create a new cc user when creating a django user
-        with patch('common.djangoapps.student.models.cc.User.save'):
+        with patch('common.djangoapps.student.models.user.cc.User.save'):
            uname = 'student'
            email = 'student@edx.org'
            password = 'test'
@@ -110,8 +110,8 @@ class ViewsExceptionTestCase(UrlResetMixin, ModuleStoreTestCase):  # lint-amnest
        config.enabled = True
        config.save()

-    @patch('common.djangoapps.student.models.cc.User.from_django_user')
-    @patch('common.djangoapps.student.models.cc.User.active_threads')
+    @patch('common.djangoapps.student.models.user.cc.User.from_django_user')
+    @patch('common.djangoapps.student.models.user.cc.User.active_threads')
    def test_user_profile_exception(self, mock_threads, mock_from_django_user):

        # Mock the code that makes the HTTP requests to the cs_comment_service app
@@ -127,8 +127,8 @@ class ViewsExceptionTestCase(UrlResetMixin, ModuleStoreTestCase):  # lint-amnest
        response = self.client.get(url)
        assert response.status_code == 404

-    @patch('common.djangoapps.student.models.cc.User.from_django_user')
-    @patch('common.djangoapps.student.models.cc.User.subscribed_threads')
+    @patch('common.djangoapps.student.models.user.cc.User.from_django_user')
+    @patch('common.djangoapps.student.models.user.cc.User.subscribed_threads')
    def test_user_followed_threads_exception(self, mock_threads, mock_from_django_user):

        # Mock the code that makes the HTTP requests to the cs_comment_service app
@@ -1661,7 +1661,7 @@ class ForumDiscussionXSSTestCase(ForumsEnableMixin, UrlResetMixin, ModuleStoreTe
        assert self.client.login(username=username, password=password)

    @ddt.data('"><script>alert(1)</script>', '<script>alert(1)</script>', '</script><script>alert(1)</script>')
-    @patch('common.djangoapps.student.models.cc.User.from_django_user')
+    @patch('common.djangoapps.student.models.user.cc.User.from_django_user')
    def test_forum_discussion_xss_prevent(self, malicious_code, mock_user, mock_req):
        """
        Test that XSS attack is prevented
@@ -1677,8 +1677,8 @@ class ForumDiscussionXSSTestCase(ForumsEnableMixin, UrlResetMixin, ModuleStoreTe
        self.assertNotContains(resp, malicious_code)

    @ddt.data('"><script>alert(1)</script>', '<script>alert(1)</script>', '</script><script>alert(1)</script>')
-    @patch('common.djangoapps.student.models.cc.User.from_django_user')
-    @patch('common.djangoapps.student.models.cc.User.active_threads')
+    @patch('common.djangoapps.student.models.user.cc.User.from_django_user')
+    @patch('common.djangoapps.student.models.user.cc.User.active_threads')
    def test_forum_user_profile_xss_prevent(self, malicious_code, mock_threads, mock_from_django_user, mock_request):
        """
        Test that XSS attack is prevented
--- a/lms/djangoapps/instructor_analytics/tests/test_basic.py
+++ b/lms/djangoapps/instructor_analytics/tests/test_basic.py
@@ -182,7 +182,8 @@ class TestAnalyticsBasic(ModuleStoreTestCase):
            assert userreport['verification_status'] in ['N/A']
        # make sure that the user report respects whatever value
        # is returned by verification and enrollment code
-        with patch("common.djangoapps.student.models.course_enrollment.CourseEnrollment.enrollment_mode_for_user") as enrollment_patch:
+        MODE_MTHD_NAME = "common.djangoapps.student.models.course_enrollment.CourseEnrollment.enrollment_mode_for_user"
+        with patch(MODE_MTHD_NAME) as enrollment_patch:
            with patch(
                "lms.djangoapps.verify_student.services.IDVerificationService.verification_status_for_user"
            ) as verify_patch:
--- a/openedx/core/djangoapps/courseware_api/tests/test_views.py
+++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py
@@ -301,7 +301,8 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
        """ Test that metadata endpoint returns data for the streak celebration """
        CourseEnrollment.enroll(self.user, self.course.id, 'audit')
        with override_waffle_flag(COURSEWARE_MFE_MILESTONES_STREAK_DISCOUNT, active=True):
-            with mock.patch('common.djangoapps.student.models.UserCelebration.perform_streak_updates', return_value=3):
+            UPDATE_MTHD_NAME = 'common.djangoapps.student.models.user.UserCelebration.perform_streak_updates'
+            with mock.patch(UPDATE_MTHD_NAME, return_value=3):
                response = self.client.get(self.url, content_type='application/json')
                celebrations = response.json()['celebrations']
                assert celebrations['streak_length_to_celebrate'] == 3
@@ -311,7 +312,8 @@ class CourseApiTestViews(BaseCoursewareTests, MasqueradeMixin):
        """ Test that metadata endpoint does not return a discount and signal is not sent if flag is not set """
        CourseEnrollment.enroll(self.user, self.course.id, 'audit')
        with override_waffle_flag(COURSEWARE_MFE_MILESTONES_STREAK_DISCOUNT, active=False):
-            with mock.patch('common.djangoapps.student.models.UserCelebration.perform_streak_updates', return_value=3):
+            UPDATE_MTHD_NAME = 'common.djangoapps.student.models.user.UserCelebration.perform_streak_updates'
+            with mock.patch(UPDATE_MTHD_NAME, return_value=3):
                response = self.client.get(self.url, content_type='application/json')
                celebrations = response.json()['celebrations']
                assert celebrations['streak_length_to_celebrate'] == 3
--- a/openedx/core/djangoapps/profile_images/tests/test_views.py
+++ b/openedx/core/djangoapps/profile_images/tests/test_views.py
@@ -441,7 +441,7 @@ class ProfileImageViewDeleteTestCase(ProfileImageEndpointMixin, APITestCase):
        )
        self.check_remove_event_emitted()

-    @patch('common.djangoapps.student.models.UserProfile.save')
+    @patch('common.djangoapps.student.models.user.UserProfile.save')
    def test_remove_failure(self, user_profile_save, mock_log):
        """
        Test that when remove validation fails, the proper HTTP response and
--- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py
+++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py
@@ -92,7 +92,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase, OpenEdxEventsTestMixin):

    def test_login_success(self):
        response, mock_audit_log = self._login_response(
-            self.user_email, self.password, patched_audit_log='common.djangoapps.student.models.AUDIT_LOG'
+            self.user_email, self.password, patched_audit_log='common.djangoapps.student.models.user.AUDIT_LOG'
        )
        self._assert_response(response, success=True)
        self._assert_audit_log(mock_audit_log, 'info', ['Login success', self.user_email])
@@ -105,7 +105,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase, OpenEdxEventsTestMixin):
        self.user.is_active = False
        self.user.save()
        response, mock_audit_log = self._login_response(
-            self.user_email, self.password, patched_audit_log='common.djangoapps.student.models.AUDIT_LOG'
+            self.user_email, self.password, patched_audit_log='common.djangoapps.student.models.user.AUDIT_LOG'
        )
        self._assert_response(response, success=True)
        self._assert_audit_log(mock_audit_log, 'info', ['Login success', self.user_email])
@@ -318,7 +318,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase, OpenEdxEventsTestMixin):
    @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True})
    def test_login_success_no_pii(self):
        response, mock_audit_log = self._login_response(
-            self.user_email, self.password, patched_audit_log='common.djangoapps.student.models.AUDIT_LOG'
+            self.user_email, self.password, patched_audit_log='common.djangoapps.student.models.user.AUDIT_LOG'
        )
        self._assert_response(response, success=True)
        self._assert_audit_log(mock_audit_log, 'info', ['Login success'])
@@ -330,7 +330,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase, OpenEdxEventsTestMixin):
        self.user.save()

        response, mock_audit_log = self._login_response(
-            unicode_email, self.password, patched_audit_log='common.djangoapps.student.models.AUDIT_LOG'
+            unicode_email, self.password, patched_audit_log='common.djangoapps.student.models.user.AUDIT_LOG'
        )
        self._assert_response(response, success=True)
        self._assert_audit_log(mock_audit_log, 'info', ['Login success', unicode_email])
@@ -477,7 +477,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase, OpenEdxEventsTestMixin):
        response, _ = self._login_response(self.user_email, self.password)
        self._assert_response(response, success=True)
        logout_url = reverse('logout')
-        with patch('common.djangoapps.student.models.AUDIT_LOG') as mock_audit_log:
+        with patch('common.djangoapps.student.models.user.AUDIT_LOG') as mock_audit_log:
            response = self.client.post(logout_url)
        assert response.status_code == 200
        self._assert_audit_log(mock_audit_log, 'info', ['Logout', 'test'])
@@ -537,7 +537,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase, OpenEdxEventsTestMixin):
        response, _ = self._login_response(self.user_email, self.password)
        self._assert_response(response, success=True)
        logout_url = reverse('logout')
-        with patch('common.djangoapps.student.models.AUDIT_LOG') as mock_audit_log:
+        with patch('common.djangoapps.student.models.user.AUDIT_LOG') as mock_audit_log:
            response = self.client.post(logout_url)
        assert response.status_code == 200
        self._assert_audit_log(mock_audit_log, 'info', ['Logout'])