From cec7969ce81db37769901665cf999ace2157e1b6 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 17 Apr 2024 08:48:11 -0400 Subject: [PATCH 1/3] feat!: Remove the django-splash app. DEPR: https://github.com/openedx/public-engineering/issues/224 The django-splash repo was created 11 years ago to let the LMS redirect users to a splash screen when a user comes to the site for the first time. It works by looking for a configurable cookie value and redirecting from the middleware. This feature was never documented, has some edx.org hardcoded defaults, and is not compatible with MFEs. BREAKING CHANGE: The django splash feature will no longer be available. --- lms/envs/common.py | 5 ----- requirements/edx/kernel.in | 1 - 2 files changed, 6 deletions(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 30ba71f62aad..4fd9ba08ada5 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2286,8 +2286,6 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring 'openedx.core.djangoapps.cors_csrf.middleware.CorsCSRFMiddleware', 'openedx.core.djangoapps.cors_csrf.middleware.CsrfCrossDomainCookieMiddleware', - 'splash.middleware.SplashMiddleware', - 'openedx.core.djangoapps.geoinfo.middleware.CountryMiddleware', 'openedx.core.djangoapps.embargo.middleware.EmbargoMiddleware', @@ -3162,9 +3160,6 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring # Notes 'lms.djangoapps.edxnotes', - # Splash screen - 'splash', - # User API 'rest_framework', diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 0c2f110c84b9..253926e2b385 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -51,7 +51,6 @@ django-pipeline django-ratelimit django-sekizai django-simple-history -django-splash django-statici18n django-storages django-user-tasks From 83ace4d925b0857a89a87760365ae017b3662cd4 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 17 Apr 2024 09:02:32 -0400 Subject: [PATCH 2/3] chore: Run `make compile-requirements` to drop django-splash. --- requirements/edx/base.txt | 3 --- requirements/edx/development.txt | 5 ----- requirements/edx/doc.txt | 3 --- requirements/edx/testing.txt | 3 --- 4 files changed, 14 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index c6607f0dfc0f..20272c4d6e5b 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -197,7 +197,6 @@ django==4.2.10 # django-oauth-toolkit # django-sekizai # django-ses - # django-splash # django-statici18n # django-storages # django-user-tasks @@ -349,8 +348,6 @@ django-simple-history==3.4.0 # edx-organizations # edx-proctoring # ora2 -django-splash==1.3.0 - # via -r requirements/edx/kernel.in django-statici18n==2.4.0 # via # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 4461a06efc61..c227b95b219b 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -363,7 +363,6 @@ django==4.2.10 # django-oauth-toolkit # django-sekizai # django-ses - # django-splash # django-statici18n # django-storages # django-stubs @@ -567,10 +566,6 @@ django-simple-history==3.4.0 # edx-organizations # edx-proctoring # ora2 -django-splash==1.3.0 - # via - # -r requirements/edx/doc.txt - # -r requirements/edx/testing.txt django-statici18n==2.4.0 # via # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 49acea996069..93555de9ed8c 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -247,7 +247,6 @@ django==4.2.10 # django-oauth-toolkit # django-sekizai # django-ses - # django-splash # django-statici18n # django-storages # django-user-tasks @@ -415,8 +414,6 @@ django-simple-history==3.4.0 # edx-organizations # edx-proctoring # ora2 -django-splash==1.3.0 - # via -r requirements/edx/base.txt django-statici18n==2.4.0 # via # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 6d21bb61a4a7..0ba71290b123 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -276,7 +276,6 @@ django==4.2.10 # django-oauth-toolkit # django-sekizai # django-ses - # django-splash # django-statici18n # django-storages # django-user-tasks @@ -444,8 +443,6 @@ django-simple-history==3.4.0 # edx-organizations # edx-proctoring # ora2 -django-splash==1.3.0 - # via -r requirements/edx/base.txt django-statici18n==2.4.0 # via # -r requirements/edx/base.txt From 9bd4474f7dee7590a95a351cc755a8ff65e15c45 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 17 Apr 2024 10:02:16 -0400 Subject: [PATCH 3/3] test: Reduce query counts now that we dropped django-splash. --- .../certificates/apis/v0/tests/test_views.py | 2 +- lms/djangoapps/course_api/tests/test_views.py | 2 +- lms/djangoapps/courseware/tests/test_views.py | 8 ++++---- .../discussion/django_comment_client/base/tests.py | 4 ++-- .../djangoapps/user_api/accounts/tests/test_views.py | 12 ++++++------ .../tests/views/test_course_updates.py | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lms/djangoapps/certificates/apis/v0/tests/test_views.py b/lms/djangoapps/certificates/apis/v0/tests/test_views.py index fd31e3456961..efff97f54d5a 100644 --- a/lms/djangoapps/certificates/apis/v0/tests/test_views.py +++ b/lms/djangoapps/certificates/apis/v0/tests/test_views.py @@ -309,7 +309,7 @@ def test_no_certificate(self): def test_query_counts(self): # Test student with no certificates student_no_cert = UserFactory.create(password=self.user_password) - with self.assertNumQueries(21, table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(20, table_ignorelist=WAFFLE_TABLES): resp = self.get_response( AuthType.jwt, requesting_user=self.global_staff, diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index 4065c54a983b..aab9769661a2 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 = [53, 46, 46, 46, 46, 46, 46, 46, 46, 46, 16] + query_counts = [52, 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/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 1d6c358ffd19..ef3d4741769e 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -365,7 +365,7 @@ def test_index_query_counts(self): self.client.login(username=self.user.username, password=self.user_password) CourseEnrollment.enroll(self.user, course.id) - with self.assertNumQueries(178, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(177, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): with check_mongo_calls(3): url = reverse( 'courseware_section', @@ -1496,8 +1496,8 @@ def test_view_certificate_link(self): self.assertContains(resp, "earned a certificate for this course.") @ddt.data( - (True, 54), - (False, 54), + (True, 53), + (False, 53), ) @ddt.unpack def test_progress_queries_paced_courses(self, self_paced, query_count): @@ -1512,7 +1512,7 @@ def test_progress_queries(self): ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) self.setup_course() with self.assertNumQueries( - 54, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST + 53, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST ), check_mongo_calls(2): self._get_progress_page() diff --git a/lms/djangoapps/discussion/django_comment_client/base/tests.py b/lms/djangoapps/discussion/django_comment_client/base/tests.py index 7eb99020d608..6aa9c3e8e9fd 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/tests.py +++ b/lms/djangoapps/discussion/django_comment_client/base/tests.py @@ -410,7 +410,7 @@ def inner(self, default_store, block_count, mongo_calls, sql_queries, *args, **k return inner @ddt.data( - (ModuleStoreEnum.Type.split, 3, 8, 43), + (ModuleStoreEnum.Type.split, 3, 8, 42), ) @ddt.unpack @count_queries @@ -418,7 +418,7 @@ def test_create_thread(self, mock_request): self.create_thread_helper(mock_request) @ddt.data( - (ModuleStoreEnum.Type.split, 3, 6, 42), + (ModuleStoreEnum.Type.split, 3, 6, 41), ) @ddt.unpack @count_queries diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 0496f45ae2fc..ff0fb7abe4eb 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -232,7 +232,7 @@ def test_get_username(self): Test that a client (logged in) can get her own username. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - self._verify_get_own_username(19) + self._verify_get_own_username(18) def test_get_username_inactive(self): """ @@ -242,7 +242,7 @@ def test_get_username_inactive(self): self.client.login(username=self.user.username, password=TEST_PASSWORD) self.user.is_active = False self.user.save() - self._verify_get_own_username(19) + self._verify_get_own_username(18) def test_get_username_not_logged_in(self): """ @@ -251,7 +251,7 @@ def test_get_username_not_logged_in(self): """ # verify that the endpoint is inaccessible when not logged in - self._verify_get_own_username(12, expected_status=401) + self._verify_get_own_username(11, expected_status=401) @skip_unless_lms @@ -358,7 +358,7 @@ class TestAccountsAPI(FilteredQueryCountMixin, CacheIsolationTestCase, UserAPITe """ ENABLED_CACHES = ['default'] - TOTAL_QUERY_COUNT = 27 + TOTAL_QUERY_COUNT = 26 FULL_RESPONSE_FIELD_COUNT = 29 def setUp(self): @@ -811,7 +811,7 @@ def verify_get_own_information(queries): assert data['time_zone'] is None self.client.login(username=self.user.username, password=TEST_PASSWORD) - verify_get_own_information(self._get_num_queries(25)) + verify_get_own_information(self._get_num_queries(24)) # Now make sure that the user can get the same information, even if not active self.user.is_active = False @@ -831,7 +831,7 @@ def test_get_account_empty_string(self): legacy_profile.save() self.client.login(username=self.user.username, password=TEST_PASSWORD) - with self.assertNumQueries(self._get_num_queries(25), table_ignorelist=WAFFLE_TABLES): + with self.assertNumQueries(self._get_num_queries(24), table_ignorelist=WAFFLE_TABLES): response = self.send_get(self.client) for empty_field in ("level_of_education", "gender", "country", "state", "bio",): assert response.data[empty_field] is None diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index f26e1cd35ba2..379be52ed40f 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -49,7 +49,7 @@ def test_queries(self): # Fetch the view and verify that the query counts haven't changed # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(53, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): + with self.assertNumQueries(52, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): with check_mongo_calls(3): url = course_updates_url(self.course) self.client.get(url)