From eaeaa124b8fb2160dad92dd9a6cf26a858e73512 Mon Sep 17 00:00:00 2001 From: Syed Sajjad Hussain Shah Date: Thu, 25 Jan 2024 12:28:09 +0500 Subject: [PATCH] fix: add setting for tests --- cms/envs/test.py | 7 +++++++ lms/djangoapps/course_api/tests/test_views.py | 2 +- .../course_home_api/course_metadata/tests/test_views.py | 1 + openedx/core/djangoapps/safe_sessions/middleware.py | 9 ++++++++- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/cms/envs/test.py b/cms/envs/test.py index 118d7e27a79c..9d65525768ed 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -223,6 +223,13 @@ FEATURES['DISABLE_SET_JWT_COOKIES_FOR_TESTS'] = True +# Skip setting user emails in session for specific unit tests. This is necessary +# because certain views raise Http404() exceptions, and since Django wraps all +# tests in transactions, modifying the session and attempting to save it after +# a transaction failure due to Http404() can result in test failures. This is +# because errors within a transaction affect subsequent DB operations. +FEATURES['DISABLE_SET_EMAIL_IN_SESSION_FOR_TESTS'] = False + FEATURES['ENABLE_SERVICE_STATUS'] = True # Toggles embargo on for testing diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index c39f808478ef..4065c54a983b 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -434,7 +434,7 @@ def test_too_many_courses(self): self.setup_user(self.audit_user) # These query counts were found empirically - query_counts = [50, 46, 46, 46, 46, 46, 46, 46, 46, 46, 16] + query_counts = [53, 46, 46, 46, 46, 46, 46, 46, 46, 46, 16] ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])]) self.clear_caches() diff --git a/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py index 74b66fa5f068..e3b4c3f186ed 100644 --- a/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py +++ b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py @@ -85,6 +85,7 @@ def test_get_masqueraded_user(self): self.update_masquerade(username=self.user.username) assert self.client.get(self.url).data['username'] == self.user.username + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_EMAIL_IN_SESSION_FOR_TESTS": True}) def test_get_unknown_course(self): url = reverse('course-home:course-metadata', args=['course-v1:unknown+course+2T2020']) # Django TestCase wraps every test in a transaction, so we must specifically wrap this when we expect an error diff --git a/openedx/core/djangoapps/safe_sessions/middleware.py b/openedx/core/djangoapps/safe_sessions/middleware.py index 71852ddf784b..e0e9143cc2d2 100644 --- a/openedx/core/djangoapps/safe_sessions/middleware.py +++ b/openedx/core/djangoapps/safe_sessions/middleware.py @@ -818,7 +818,14 @@ def process_response(self, request, response): # .. custom_attribute_description: Indicates that user's email was not # stored in the user's session. set_custom_attribute('session_with_no_email_found', True) - request.session['email'] = request.user.email + + # Skip setting user emails in session for specific unit tests. This is necessary + # because certain views raise Http404() exceptions, and since Django wraps all + # tests in transactions, modifying the session and attempting to save it after + # a transaction failure due to Http404() can result in test failures. This is + # because errors within a transaction affect subsequent DB operations. + if not settings.FEATURES.get('DISABLE_SET_EMAIL_IN_SESSION_FOR_TESTS', False): + request.session['email'] = request.user.email if request_cache.get_cached_response('email_change_requested').is_found: # Update the JWT cookies with new user email