From 9f136a4149548e10a39b3cafc5c9cd88a5c8c73c Mon Sep 17 00:00:00 2001 From: ayesha waris <73840786+ayesha-waris@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:33:16 +0500 Subject: [PATCH 1/4] feat: added notifications when response is endorsed or answered (#34082) * feat: added notification when response is endorsed or answered * test: added and fixed test cases * fix: fixed lint errors * refactor: changed method name for readibility * feat: added notification when my response is endorsed * test: fixed failed test cases --- .../rest_api/discussions_notifications.py | 16 ++++ lms/djangoapps/discussion/rest_api/tasks.py | 19 ++++ .../discussion/rest_api/tests/test_tasks.py | 96 ++++++++++++++++++- lms/djangoapps/discussion/signals/handlers.py | 20 +++- lms/djangoapps/teams/tests/test_models.py | 14 ++- .../notifications/base_notification.py | 28 ++++++ .../core/djangoapps/notifications/models.py | 2 +- .../notifications/tests/test_views.py | 4 +- 8 files changed, 191 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py index a148fbdc9f..da5b430fce 100644 --- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py +++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py @@ -235,6 +235,22 @@ class DiscussionNotificationSender: } return {} + def send_response_endorsed_on_thread_notification(self): + """ + Sends a notification to the author of the thread + response on his thread has been endorsed + """ + context = { + "username": self.creator.username, + } + self._send_notification([self.thread.user_id], "response_endorsed_on_thread", context) + + def send_response_endorsed_notification(self): + """ + Sends a notification to the author of the response + """ + self._send_notification([self.creator.id], "response_endorsed") + def send_new_thread_created_notification(self): """ Send notification based on notification_type diff --git a/lms/djangoapps/discussion/rest_api/tasks.py b/lms/djangoapps/discussion/rest_api/tasks.py index 385e2a1f46..bd41e1078c 100644 --- a/lms/djangoapps/discussion/rest_api/tasks.py +++ b/lms/djangoapps/discussion/rest_api/tasks.py @@ -47,3 +47,22 @@ def send_response_notifications(thread_id, course_key_str, user_id, parent_id=No notification_sender.send_new_response_notification() notification_sender.send_new_comment_on_response_notification() notification_sender.send_response_on_followed_post_notification() + + +@shared_task +@set_code_owner_attribute +def send_response_endorsed_notifications(thread_id, course_key_str, comment_author_id): + """ + Send notifications when a response is marked answered/ endorsed + """ + course_key = CourseKey.from_string(course_key_str) + if not ENABLE_NOTIFICATIONS.is_enabled(course_key): + return + thread = Thread(id=thread_id).retrieve() + comment_author = User.objects.get(id=comment_author_id) + course = get_course_with_access(comment_author, 'load', course_key, check_if_enrolled=True) + notification_sender = DiscussionNotificationSender(thread, course, comment_author) + #sends notification to author of thread + notification_sender.send_response_endorsed_on_thread_notification() + #sends notification to author of response + notification_sender.send_response_endorsed_notification() diff --git a/lms/djangoapps/discussion/rest_api/tests/test_tasks.py b/lms/djangoapps/discussion/rest_api/tests/test_tasks.py index 9e5a5bc79a..acb8da294f 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_tasks.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_tasks.py @@ -13,7 +13,10 @@ from openedx_events.learning.signals import USER_NOTIFICATION_REQUESTED, COURSE_ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import StaffFactory, UserFactory from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory -from lms.djangoapps.discussion.rest_api.tasks import send_response_notifications, send_thread_created_notification +from lms.djangoapps.discussion.rest_api.tasks import ( + send_response_notifications, + send_thread_created_notification, + send_response_endorsed_notifications) from lms.djangoapps.discussion.rest_api.tests.utils import ThreadMock, make_minimal_cs_thread from openedx.core.djangoapps.course_groups.models import CohortMembership, CourseCohortsSettings from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory @@ -49,6 +52,7 @@ class TestNewThreadCreatedNotification(DiscussionAPIViewTestMixin, ModuleStoreTe """ Test cases related to new_discussion_post and new_question_post notification types """ + def setUp(self): """ Setup test case @@ -478,6 +482,7 @@ class TestSendCommentNotification(DiscussionAPIViewTestMixin, ModuleStoreTestCas """ Test case to send new_comment notification """ + def setUp(self): super().setUp() httpretty.reset() @@ -527,3 +532,92 @@ class TestSendCommentNotification(DiscussionAPIViewTestMixin, ModuleStoreTestCas handler.assert_called_once() context = handler.call_args[1]['notification_data'].context self.assertEqual(context['author_name'], 'their') + + +@override_waffle_flag(ENABLE_NOTIFICATIONS, active=True) +class TestResponseEndorsedNotifications(DiscussionAPIViewTestMixin, ModuleStoreTestCase): + """ + Test case to send response endorsed notifications + """ + + def setUp(self): + super().setUp() + httpretty.reset() + httpretty.enable() + + self.course = CourseFactory.create() + self.user_1 = UserFactory.create() + CourseEnrollment.enroll(self.user_1, self.course.id) + self.user_2 = UserFactory.create() + CourseEnrollment.enroll(self.user_2, self.course.id) + + def test_basic(self): + """ + Left empty intentionally. This test case is inherited from DiscussionAPIViewTestMixin + """ + + def test_not_authenticated(self): + """ + Left empty intentionally. This test case is inherited from DiscussionAPIViewTestMixin + """ + + def test_response_endorsed_notifications(self): + """ + Tests nresponse endorsed notifications + """ + + thread = ThreadMock(thread_id=1, creator=self.user_1, title='test thread') + response = ThreadMock(thread_id=2, creator=self.user_2, title='test response') + self.register_get_thread_response({ + 'id': thread.id, + 'course_id': str(self.course.id), + 'topic_id': 'abc', + "user_id": thread.user_id, + "username": thread.username, + "thread_type": 'discussion', + "title": thread.title, + }) + self.register_get_comment_response({ + 'id': response.id, + 'thread_id': thread.id, + 'user_id': response.user_id + }) + handler = mock.Mock() + USER_NOTIFICATION_REQUESTED.connect(handler) + send_response_endorsed_notifications(thread.id, str(self.course.id), self.user_2.id) + self.assertEqual(handler.call_count, 2) + + #Test response endorsed on thread notification + notification_data = handler.call_args_list[0][1]['notification_data'] + # Target only the thread author + self.assertEqual([int(user_id) for user_id in notification_data.user_ids], [int(thread.user_id)]) + self.assertEqual(notification_data.notification_type, 'response_endorsed_on_thread') + + expected_context = { + 'replier_name': response.username, + 'post_title': 'test thread', + 'course_name': self.course.display_name, + 'sender_id': int(response.user_id), + 'username': response.username, + } + self.assertDictEqual(notification_data.context, expected_context) + self.assertEqual(notification_data.content_url, _get_mfe_url(self.course.id, thread.id)) + self.assertEqual(notification_data.app_name, 'discussion') + self.assertEqual('response_endorsed_on_thread', notification_data.notification_type) + + #Test response endorsed notification + notification_data = handler.call_args_list[1][1]['notification_data'] + # Target only the response author + self.assertEqual([int(user_id) for user_id in notification_data.user_ids], [int(response.user_id)]) + self.assertEqual(notification_data.notification_type, 'response_endorsed') + + expected_context = { + 'replier_name': response.username, + 'post_title': 'test thread', + 'course_name': self.course.display_name, + 'sender_id': int(response.user_id), + } + self.assertDictEqual(notification_data.context, expected_context) + self.assertEqual(notification_data.content_url, _get_mfe_url(self.course.id, thread.id)) + self.assertEqual(notification_data.app_name, 'discussion') + self.assertEqual('response_endorsed', notification_data.notification_type) diff --git a/lms/djangoapps/discussion/signals/handlers.py b/lms/djangoapps/discussion/signals/handlers.py index f857e1aa8c..423bc5b2bc 100644 --- a/lms/djangoapps/discussion/signals/handlers.py +++ b/lms/djangoapps/discussion/signals/handlers.py @@ -15,7 +15,11 @@ from lms.djangoapps.discussion.toggles import ENABLE_REPORTED_CONTENT_NOTIFICATI from xmodule.modulestore.django import SignalHandler, modulestore from lms.djangoapps.discussion import tasks -from lms.djangoapps.discussion.rest_api.tasks import send_response_notifications, send_thread_created_notification +from lms.djangoapps.discussion.rest_api.tasks import ( + send_response_notifications, + send_thread_created_notification, + send_response_endorsed_notifications +) from openedx.core.djangoapps.django_comment_common import signals from openedx.core.djangoapps.site_configuration.models import SiteConfiguration from openedx.core.djangoapps.theming.helpers import get_current_site @@ -178,3 +182,17 @@ def create_comment_created_notification(*args, **kwargs): parent_id = comment.attributes['parent_id'] course_key_str = comment.attributes['course_id'] send_response_notifications.apply_async(args=[thread_id, course_key_str, user.id, parent_id]) + + +@receiver(signals.comment_endorsed) +def create_response_endorsed_on_thread_notification(*args, **kwargs): + """ + Creates a notification for thread author when response on thread is endorsed + and another notification for response author when response is endorsed + """ + comment = kwargs['post'] + comment_author_id = comment.attributes['user_id'] + thread_id = comment.attributes['thread_id'] + course_key_str = comment.attributes['course_id'] + + send_response_endorsed_notifications.apply_async(args=[thread_id, course_key_str, comment_author_id]) diff --git a/lms/djangoapps/teams/tests/test_models.py b/lms/djangoapps/teams/tests/test_models.py index 02e238bf2a..6019007600 100644 --- a/lms/djangoapps/teams/tests/test_models.py +++ b/lms/djangoapps/teams/tests/test_models.py @@ -320,8 +320,11 @@ class TeamSignalsTest(EventTestMixin, SharedModuleStoreTestCase): user = getattr(self, user) with patch('lms.djangoapps.discussion.rest_api.tasks.send_response_notifications.apply_async'): with patch('lms.djangoapps.discussion.rest_api.tasks.send_thread_created_notification.apply_async'): - signal = self.SIGNALS[signal_name] - signal.send(sender=None, user=user, post=self.mock_comment()) + with patch( + 'lms.djangoapps.discussion.rest_api.tasks.send_response_endorsed_notifications.apply_async' + ): + signal = self.SIGNALS[signal_name] + signal.send(sender=None, user=user, post=self.mock_comment()) @ddt.data('thread_voted', 'comment_voted') def test_vote_others_post(self, signal_name): @@ -339,5 +342,8 @@ class TeamSignalsTest(EventTestMixin, SharedModuleStoreTestCase): with self.assert_last_activity_updated(False): with patch('lms.djangoapps.discussion.rest_api.tasks.send_response_notifications.apply_async'): with patch('lms.djangoapps.discussion.rest_api.tasks.send_thread_created_notification.apply_async'): - signal = self.SIGNALS[signal_name] - signal.send(sender=None, user=self.user, post=self.mock_comment(context='course')) + with patch( + 'lms.djangoapps.discussion.rest_api.tasks.send_response_endorsed_notifications.apply_async' + ): + signal = self.SIGNALS[signal_name] + signal.send(sender=None, user=self.user, post=self.mock_comment(context='course')) diff --git a/openedx/core/djangoapps/notifications/base_notification.py b/openedx/core/djangoapps/notifications/base_notification.py index 9d620b5b61..946314cfa0 100644 --- a/openedx/core/djangoapps/notifications/base_notification.py +++ b/openedx/core/djangoapps/notifications/base_notification.py @@ -132,6 +132,34 @@ COURSE_NOTIFICATION_TYPES = { }, 'email_template': '', }, + 'response_endorsed_on_thread': { + 'notification_app': 'discussion', + 'name': 'response_endorsed_on_thread', + 'is_core': True, + 'info': '', + 'non_editable': [], + 'content_template': _('<{p}><{strong}>{username} response has been endorsed in your post ' + '<{strong}>{post_title}'), + 'content_context': { + 'post_title': 'Post title', + 'username': 'Response author name', + }, + 'email_template': '', + 'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE] + }, + 'response_endorsed': { + 'notification_app': 'discussion', + 'name': 'response_endorsed', + 'is_core': True, + 'info': '', + 'non_editable': [], + 'content_template': _('<{p}>{post_title}'), + 'content_context': { + 'post_title': 'Post title', + }, + 'email_template': '', + 'filters': [FILTER_AUDIT_EXPIRED_USERS_WITH_NO_ROLE] + } } COURSE_NOTIFICATION_APPS = { diff --git a/openedx/core/djangoapps/notifications/models.py b/openedx/core/djangoapps/notifications/models.py index c2c03f2023..1014683f06 100644 --- a/openedx/core/djangoapps/notifications/models.py +++ b/openedx/core/djangoapps/notifications/models.py @@ -21,7 +21,7 @@ log = logging.getLogger(__name__) NOTIFICATION_CHANNELS = ['web', 'push', 'email'] # Update this version when there is a change to any course specific notification type or app. -COURSE_NOTIFICATION_CONFIG_VERSION = 5 +COURSE_NOTIFICATION_CONFIG_VERSION = 6 def get_course_notification_preference_config(): diff --git a/openedx/core/djangoapps/notifications/tests/test_views.py b/openedx/core/djangoapps/notifications/tests/test_views.py index 815266235b..b094a974aa 100644 --- a/openedx/core/djangoapps/notifications/tests/test_views.py +++ b/openedx/core/djangoapps/notifications/tests/test_views.py @@ -236,7 +236,9 @@ class UserNotificationPreferenceAPITest(ModuleStoreTestCase): 'new_comment', 'new_response', 'response_on_followed_post', - 'comment_on_followed_post' + 'comment_on_followed_post', + 'response_endorsed_on_thread', + 'response_endorsed' ], 'notification_types': { 'core': { From 1fc2e8a771a85db9b9a7e0b8c2da925bf81ed5bb Mon Sep 17 00:00:00 2001 From: Muhammad Soban Javed <58461728+iamsobanjaved@users.noreply.github.com> Date: Thu, 1 Feb 2024 16:10:34 +0500 Subject: [PATCH 2/4] feat!: upgrade Django version to 4.2 (LTS) (#34162) * feat!: upgrade Django version to 4.2 (LTS) --------- Co-authored-by: iamsobanjaved --- .github/workflows/migrations-check.yml | 11 ++----- .github/workflows/unit-tests.yml | 1 - Makefile | 2 ++ cms/envs/common.py | 4 +-- lms/envs/common.py | 4 +-- .../tests/test_safe_cookie_data.py | 2 +- openedx/core/djangoapps/theming/storage.py | 32 +++++++++++++------ requirements/common_constraints.txt | 2 +- requirements/constraints.txt | 5 +++ requirements/edx/base.txt | 9 +++--- requirements/edx/development.txt | 9 +++--- requirements/edx/doc.txt | 9 +++--- requirements/edx/testing.txt | 9 +++--- 13 files changed, 58 insertions(+), 41 deletions(-) diff --git a/.github/workflows/migrations-check.yml b/.github/workflows/migrations-check.yml index 3929e7f10e..fb66cfe775 100644 --- a/.github/workflows/migrations-check.yml +++ b/.github/workflows/migrations-check.yml @@ -16,16 +16,11 @@ jobs: os: [ ubuntu-20.04 ] python-version: [ 3.8 ] # 'pinned' is used to install the latest patch version of Django - # within the global constraint i.e. Django==3.2.21 in current case + # within the global constraint i.e. Django==4.2.8 in current case # because we have global constraint of Django<4.2 - django-version: ["pinned", "4.2"] + django-version: ["pinned"] mongo-version: ["4"] - mysql-version: ["5.7", "8"] - # excluding mysql5.7 with Django 4.2 since Django 4.2 has - # dropped support for MySQL<8 - exclude: - - django-version: "4.2" - mysql-version: "5.7" + mysql-version: ["8"] services: mongo: image: mongo:${{ matrix.mongo-version }} diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index c3b1086c20..be7e122f2f 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -17,7 +17,6 @@ jobs: - "3.8" django-version: - "pinned" - - "4.2" # When updating the shards, remember to make the same changes in # .github/workflows/unit-tests-gh-hosted.yml shard_name: diff --git a/Makefile b/Makefile index cc6fa55913..92c89d844a 100644 --- a/Makefile +++ b/Makefile @@ -150,6 +150,8 @@ compile-requirements: pre-requirements $(COMMON_CONSTRAINTS_TXT) ## Re-compile * @# time someone tries to use the outputs. sed '/^django-simple-history==/d' requirements/common_constraints.txt > requirements/common_constraints.tmp mv requirements/common_constraints.tmp requirements/common_constraints.txt + sed 's/Django<4.0//g' requirements/common_constraints.txt > requirements/common_constraints.tmp + mv requirements/common_constraints.tmp requirements/common_constraints.txt pip-compile -v --allow-unsafe ${COMPILE_OPTS} -o requirements/pip.txt requirements/pip.in pip install -r requirements/pip.txt diff --git a/cms/envs/common.py b/cms/envs/common.py index 4ded7a9cff..8f974e73e5 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1105,8 +1105,8 @@ DATABASES = { } DEFAULT_AUTO_FIELD = 'django.db.models.AutoField' -# This will be overridden through CMS config -DEFAULT_HASHING_ALGORITHM = 'sha1' +DEFAULT_HASHING_ALGORITHM = 'sha256' + #################### Python sandbox ############################################ CODE_JAIL = { diff --git a/lms/envs/common.py b/lms/envs/common.py index 3da23a4ad8..f14ec39627 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1768,8 +1768,8 @@ DATABASES = { DEFAULT_AUTO_FIELD = 'django.db.models.AutoField' -# This will be overridden through LMS config -DEFAULT_HASHING_ALGORITHM = 'sha1' +DEFAULT_HASHING_ALGORITHM = 'sha256' + #################### Python sandbox ############################################ CODE_JAIL = { diff --git a/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py b/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py index 47efa626ec..650a8d157b 100644 --- a/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py +++ b/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py @@ -236,7 +236,7 @@ class TestSafeCookieData(TestSafeSessionsLogMixin, TestCase): "|HvGnjXf1b3jU" "|ImExZWZiNzVlZGFmM2FkZWZmYjM4YjI0ZmZkOWU4MzExODU0MTk4NmVlNGRiYzBlODdhYWUzOGM5MzVlNzk4NjUi" ":1m6Hve" - ":OMhY2FL2pudJjSSXChtI-zR8QVA" + ":Pra4iochviPvKUoIV33gdVZFDgG-cMDlIYfl8iFIMaY" ) @pytest.mark.skipif(django.VERSION[0] < 4, reason="For django42 default algorithm is sha256. No need for django32.") diff --git a/openedx/core/djangoapps/theming/storage.py b/openedx/core/djangoapps/theming/storage.py index e8e3eefcd3..4a48cbb6e0 100644 --- a/openedx/core/djangoapps/theming/storage.py +++ b/openedx/core/djangoapps/theming/storage.py @@ -192,33 +192,41 @@ class ThemeManifestFilesMixin(ManifestFilesMixin): This requires figuring out which files the matched URL resolves to and calling the url() method of the storage. """ - matched, url = matchobj.groups() + matches = matchobj.groupdict() + matched = matches["matched"] + url = matches["url"] # Ignore absolute/protocol-relative and data-uri URLs. - if re.match(r'^[a-z]+:', url): + if re.match(r"^[a-z]+:", url): return matched # Ignore absolute URLs that don't point to a static file (dynamic # CSS / JS?). Note that STATIC_URL cannot be empty. - if url.startswith('/') and not url.startswith(settings.STATIC_URL): + if url.startswith("/") and not url.startswith(settings.STATIC_URL): return matched # Strip off the fragment so a path-like fragment won't interfere. url_path, fragment = urldefrag(url) - if url_path.startswith('/'): + # Ignore URLs without a path + if not url_path: + return matched + + if url_path.startswith("/"): # Otherwise the condition above would have returned prematurely. assert url_path.startswith(settings.STATIC_URL) target_name = url_path[len(settings.STATIC_URL):] else: # We're using the posixpath module to mix paths and URLs conveniently. - source_name = name if os.sep == '/' else name.replace(os.sep, '/') + source_name = name if os.sep == "/" else name.replace(os.sep, "/") target_name = posixpath.join(posixpath.dirname(source_name), url_path) # Determine the hashed name of the target file with the storage backend. hashed_url = self._url( - self._stored_name, unquote(target_name), - force=True, hashed_files=hashed_files, + self._stored_name, + unquote(target_name), + force=True, + hashed_files=hashed_files, ) # NOTE: @@ -228,15 +236,19 @@ class ThemeManifestFilesMixin(ManifestFilesMixin): # The line is commented and not removed to make future django upgrade easier and show exactly what is # changed in this method override # - #transformed_url = '/'.join(url_path.split('/')[:-1] + hashed_url.split('/')[-1:]) + # transformed_url = "/".join( + # url_path.split("/")[:-1] + hashed_url.split("/")[-1:] + # ) + transformed_url = hashed_url # This line was added. # Restore the fragment that was stripped off earlier. if fragment: - transformed_url += ('?#' if '?#' in url else '#') + fragment + transformed_url += ("?#" if "?#" in url else "#") + fragment # Return the hashed version to the file - return template % unquote(transformed_url) + matches["url"] = unquote(transformed_url) + return template % matches return converter diff --git a/requirements/common_constraints.txt b/requirements/common_constraints.txt index 53dff4a22d..7313473a16 100644 --- a/requirements/common_constraints.txt +++ b/requirements/common_constraints.txt @@ -17,7 +17,7 @@ # using LTS django version -Django<4.0 + # elasticsearch>=7.14.0 includes breaking changes in it which caused issues in discovery upgrade process. # elastic search changelog: https://www.elastic.co/guide/en/enterprise-search/master/release-notes-7.14.0.html diff --git a/requirements/constraints.txt b/requirements/constraints.txt index ebf7f376de..55ce227b21 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -25,10 +25,15 @@ click>=8.0,<9.0 # for them. edx-enterprise==4.11.3 +# Stay on LTS version, remove once this is added to common constraint +Django<5.0 + # django-oauth-toolkit version >=2.0.0 has breaking changes. More details # mentioned on this issue https://github.com/openedx/edx-platform/issues/32884 django-oauth-toolkit==1.7.1 +# incremental upgrade +django-simple-history==3.4.0 # constrained in opaque_keys. migration guide here: https://pymongo.readthedocs.io/en/4.0/migrate-to-pymongo4.html # Major upgrade will be done in separate ticket. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 8a8980796e..5ebfd20703 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -56,6 +56,7 @@ backoff==1.10.0 backports-zoneinfo[tzdata]==0.2.1 # via # celery + # django # icalendar # kombu beautifulsoup4==4.12.3 @@ -173,9 +174,9 @@ defusedxml==0.7.1 # social-auth-core deprecated==1.2.14 # via jwcrypto -django==3.2.23 +django==4.2.9 # via - # -c requirements/edx/../common_constraints.txt + # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in # django-appconf # django-celery-results @@ -338,6 +339,7 @@ django-ses==3.5.2 # via -r requirements/edx/bundled.in django-simple-history==3.4.0 # via + # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in # edx-enterprise # edx-name-affirmation @@ -431,7 +433,7 @@ edx-ccx-keys==1.2.1 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -edx-celeryutils==1.2.3 +edx-celeryutils==1.2.5 # via # -r requirements/edx/kernel.in # edx-name-affirmation @@ -940,7 +942,6 @@ pytz==2023.4 # via # -r requirements/edx/kernel.in # babel - # django # django-ses # djangorestframework # drf-yasg diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 7f56649bc4..150ef1c2aa 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -116,6 +116,7 @@ backports-zoneinfo[tzdata]==0.2.1 # -r requirements/edx/testing.txt # backports-zoneinfo # celery + # django # icalendar # kombu beautifulsoup4==4.12.3 @@ -341,9 +342,9 @@ distlib==0.3.8 # via # -r requirements/edx/testing.txt # virtualenv -django==3.2.23 +django==4.2.9 # via - # -c requirements/edx/../common_constraints.txt + # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # django-appconf @@ -559,6 +560,7 @@ django-ses==3.5.2 # -r requirements/edx/testing.txt django-simple-history==3.4.0 # via + # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # edx-enterprise @@ -701,7 +703,7 @@ edx-ccx-keys==1.2.1 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # lti-consumer-xblock -edx-celeryutils==1.2.3 +edx-celeryutils==1.2.5 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1670,7 +1672,6 @@ pytz==2023.4 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # babel - # django # django-ses # djangorestframework # drf-yasg diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 7a3fc1bb9c..44655affcf 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -79,6 +79,7 @@ backports-zoneinfo[tzdata]==0.2.1 # -r requirements/edx/base.txt # backports-zoneinfo # celery + # django # icalendar # kombu beautifulsoup4==4.12.3 @@ -223,9 +224,9 @@ deprecated==1.2.14 # via # -r requirements/edx/base.txt # jwcrypto -django==3.2.23 +django==4.2.9 # via - # -c requirements/edx/../common_constraints.txt + # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # django-appconf # django-celery-results @@ -404,6 +405,7 @@ django-ses==3.5.2 # via -r requirements/edx/base.txt django-simple-history==3.4.0 # via + # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # edx-enterprise # edx-name-affirmation @@ -509,7 +511,7 @@ edx-ccx-keys==1.2.1 # via # -r requirements/edx/base.txt # lti-consumer-xblock -edx-celeryutils==1.2.3 +edx-celeryutils==1.2.5 # via # -r requirements/edx/base.txt # edx-name-affirmation @@ -1123,7 +1125,6 @@ pytz==2023.4 # via # -r requirements/edx/base.txt # babel - # django # django-ses # djangorestframework # drf-yasg diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 0e993cd220..8f1152d553 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -81,6 +81,7 @@ backports-zoneinfo[tzdata]==0.2.1 # -r requirements/edx/base.txt # backports-zoneinfo # celery + # django # icalendar # kombu beautifulsoup4==4.12.3 @@ -254,9 +255,9 @@ dill==0.3.8 # via pylint distlib==0.3.8 # via virtualenv -django==3.2.23 +django==4.2.9 # via - # -c requirements/edx/../common_constraints.txt + # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # django-appconf # django-celery-results @@ -435,6 +436,7 @@ django-ses==3.5.2 # via -r requirements/edx/base.txt django-simple-history==3.4.0 # via + # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # edx-enterprise # edx-name-affirmation @@ -535,7 +537,7 @@ edx-ccx-keys==1.2.1 # via # -r requirements/edx/base.txt # lti-consumer-xblock -edx-celeryutils==1.2.3 +edx-celeryutils==1.2.5 # via # -r requirements/edx/base.txt # edx-name-affirmation @@ -1251,7 +1253,6 @@ pytz==2023.4 # via # -r requirements/edx/base.txt # babel - # django # django-ses # djangorestframework # drf-yasg From 6b0c5c661e6f034cb7d6fbd9cd8d16cffa6f0cd2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 1 Feb 2024 17:42:51 +0500 Subject: [PATCH 3/4] feat: Upgrade Python dependency edx-enterprise (#34155) Added logs and remove cornerstone from management command Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: zamanafzal Co-authored-by: Zaman Afzal --- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 55ce227b21..7c702ae42d 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -23,7 +23,7 @@ click>=8.0,<9.0 # The team that owns this package will manually bump this package rather than having it pulled in automatically. # This is to allow them to better control its deployment and to do it in a process that works better # for them. -edx-enterprise==4.11.3 +edx-enterprise==4.11.5 # Stay on LTS version, remove once this is added to common constraint Django<5.0 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 5ebfd20703..3738e7d53f 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -477,7 +477,7 @@ edx-drf-extensions==10.1.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.11.3 +edx-enterprise==4.11.5 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 150ef1c2aa..8def54dd19 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -757,7 +757,7 @@ edx-drf-extensions==10.1.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.11.3 +edx-enterprise==4.11.5 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 44655affcf..4572b11fb5 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -555,7 +555,7 @@ edx-drf-extensions==10.1.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.11.3 +edx-enterprise==4.11.5 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 8f1152d553..553a099d95 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -581,7 +581,7 @@ edx-drf-extensions==10.1.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.11.3 +edx-enterprise==4.11.5 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From 23659d5ba84a4f5154c498d7ddacaf912caf7ac0 Mon Sep 17 00:00:00 2001 From: Usama Sadiq Date: Thu, 1 Feb 2024 19:29:56 +0500 Subject: [PATCH 4/4] Revert "feat!: upgrade Django version to 4.2 (LTS) (#34162)" (#34165) This reverts commit 1fc2e8a771a85db9b9a7e0b8c2da925bf81ed5bb. --- .github/workflows/migrations-check.yml | 11 +++++-- .github/workflows/unit-tests.yml | 1 + Makefile | 2 -- cms/envs/common.py | 4 +-- lms/envs/common.py | 4 +-- .../tests/test_safe_cookie_data.py | 2 +- openedx/core/djangoapps/theming/storage.py | 32 ++++++------------- requirements/common_constraints.txt | 2 +- requirements/constraints.txt | 5 --- requirements/edx/base.txt | 9 +++--- requirements/edx/development.txt | 9 +++--- requirements/edx/doc.txt | 9 +++--- requirements/edx/testing.txt | 9 +++--- 13 files changed, 41 insertions(+), 58 deletions(-) diff --git a/.github/workflows/migrations-check.yml b/.github/workflows/migrations-check.yml index fb66cfe775..3929e7f10e 100644 --- a/.github/workflows/migrations-check.yml +++ b/.github/workflows/migrations-check.yml @@ -16,11 +16,16 @@ jobs: os: [ ubuntu-20.04 ] python-version: [ 3.8 ] # 'pinned' is used to install the latest patch version of Django - # within the global constraint i.e. Django==4.2.8 in current case + # within the global constraint i.e. Django==3.2.21 in current case # because we have global constraint of Django<4.2 - django-version: ["pinned"] + django-version: ["pinned", "4.2"] mongo-version: ["4"] - mysql-version: ["8"] + mysql-version: ["5.7", "8"] + # excluding mysql5.7 with Django 4.2 since Django 4.2 has + # dropped support for MySQL<8 + exclude: + - django-version: "4.2" + mysql-version: "5.7" services: mongo: image: mongo:${{ matrix.mongo-version }} diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index be7e122f2f..c3b1086c20 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -17,6 +17,7 @@ jobs: - "3.8" django-version: - "pinned" + - "4.2" # When updating the shards, remember to make the same changes in # .github/workflows/unit-tests-gh-hosted.yml shard_name: diff --git a/Makefile b/Makefile index 92c89d844a..cc6fa55913 100644 --- a/Makefile +++ b/Makefile @@ -150,8 +150,6 @@ compile-requirements: pre-requirements $(COMMON_CONSTRAINTS_TXT) ## Re-compile * @# time someone tries to use the outputs. sed '/^django-simple-history==/d' requirements/common_constraints.txt > requirements/common_constraints.tmp mv requirements/common_constraints.tmp requirements/common_constraints.txt - sed 's/Django<4.0//g' requirements/common_constraints.txt > requirements/common_constraints.tmp - mv requirements/common_constraints.tmp requirements/common_constraints.txt pip-compile -v --allow-unsafe ${COMPILE_OPTS} -o requirements/pip.txt requirements/pip.in pip install -r requirements/pip.txt diff --git a/cms/envs/common.py b/cms/envs/common.py index 8f974e73e5..4ded7a9cff 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1105,8 +1105,8 @@ DATABASES = { } DEFAULT_AUTO_FIELD = 'django.db.models.AutoField' -DEFAULT_HASHING_ALGORITHM = 'sha256' - +# This will be overridden through CMS config +DEFAULT_HASHING_ALGORITHM = 'sha1' #################### Python sandbox ############################################ CODE_JAIL = { diff --git a/lms/envs/common.py b/lms/envs/common.py index f14ec39627..3da23a4ad8 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1768,8 +1768,8 @@ DATABASES = { DEFAULT_AUTO_FIELD = 'django.db.models.AutoField' -DEFAULT_HASHING_ALGORITHM = 'sha256' - +# This will be overridden through LMS config +DEFAULT_HASHING_ALGORITHM = 'sha1' #################### Python sandbox ############################################ CODE_JAIL = { diff --git a/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py b/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py index 650a8d157b..47efa626ec 100644 --- a/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py +++ b/openedx/core/djangoapps/safe_sessions/tests/test_safe_cookie_data.py @@ -236,7 +236,7 @@ class TestSafeCookieData(TestSafeSessionsLogMixin, TestCase): "|HvGnjXf1b3jU" "|ImExZWZiNzVlZGFmM2FkZWZmYjM4YjI0ZmZkOWU4MzExODU0MTk4NmVlNGRiYzBlODdhYWUzOGM5MzVlNzk4NjUi" ":1m6Hve" - ":Pra4iochviPvKUoIV33gdVZFDgG-cMDlIYfl8iFIMaY" + ":OMhY2FL2pudJjSSXChtI-zR8QVA" ) @pytest.mark.skipif(django.VERSION[0] < 4, reason="For django42 default algorithm is sha256. No need for django32.") diff --git a/openedx/core/djangoapps/theming/storage.py b/openedx/core/djangoapps/theming/storage.py index 4a48cbb6e0..e8e3eefcd3 100644 --- a/openedx/core/djangoapps/theming/storage.py +++ b/openedx/core/djangoapps/theming/storage.py @@ -192,41 +192,33 @@ class ThemeManifestFilesMixin(ManifestFilesMixin): This requires figuring out which files the matched URL resolves to and calling the url() method of the storage. """ - matches = matchobj.groupdict() - matched = matches["matched"] - url = matches["url"] + matched, url = matchobj.groups() # Ignore absolute/protocol-relative and data-uri URLs. - if re.match(r"^[a-z]+:", url): + if re.match(r'^[a-z]+:', url): return matched # Ignore absolute URLs that don't point to a static file (dynamic # CSS / JS?). Note that STATIC_URL cannot be empty. - if url.startswith("/") and not url.startswith(settings.STATIC_URL): + if url.startswith('/') and not url.startswith(settings.STATIC_URL): return matched # Strip off the fragment so a path-like fragment won't interfere. url_path, fragment = urldefrag(url) - # Ignore URLs without a path - if not url_path: - return matched - - if url_path.startswith("/"): + if url_path.startswith('/'): # Otherwise the condition above would have returned prematurely. assert url_path.startswith(settings.STATIC_URL) target_name = url_path[len(settings.STATIC_URL):] else: # We're using the posixpath module to mix paths and URLs conveniently. - source_name = name if os.sep == "/" else name.replace(os.sep, "/") + source_name = name if os.sep == '/' else name.replace(os.sep, '/') target_name = posixpath.join(posixpath.dirname(source_name), url_path) # Determine the hashed name of the target file with the storage backend. hashed_url = self._url( - self._stored_name, - unquote(target_name), - force=True, - hashed_files=hashed_files, + self._stored_name, unquote(target_name), + force=True, hashed_files=hashed_files, ) # NOTE: @@ -236,19 +228,15 @@ class ThemeManifestFilesMixin(ManifestFilesMixin): # The line is commented and not removed to make future django upgrade easier and show exactly what is # changed in this method override # - # transformed_url = "/".join( - # url_path.split("/")[:-1] + hashed_url.split("/")[-1:] - # ) - + #transformed_url = '/'.join(url_path.split('/')[:-1] + hashed_url.split('/')[-1:]) transformed_url = hashed_url # This line was added. # Restore the fragment that was stripped off earlier. if fragment: - transformed_url += ("?#" if "?#" in url else "#") + fragment + transformed_url += ('?#' if '?#' in url else '#') + fragment # Return the hashed version to the file - matches["url"] = unquote(transformed_url) - return template % matches + return template % unquote(transformed_url) return converter diff --git a/requirements/common_constraints.txt b/requirements/common_constraints.txt index 7313473a16..53dff4a22d 100644 --- a/requirements/common_constraints.txt +++ b/requirements/common_constraints.txt @@ -17,7 +17,7 @@ # using LTS django version - +Django<4.0 # elasticsearch>=7.14.0 includes breaking changes in it which caused issues in discovery upgrade process. # elastic search changelog: https://www.elastic.co/guide/en/enterprise-search/master/release-notes-7.14.0.html diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 7c702ae42d..476bde2d9c 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -25,15 +25,10 @@ click>=8.0,<9.0 # for them. edx-enterprise==4.11.5 -# Stay on LTS version, remove once this is added to common constraint -Django<5.0 - # django-oauth-toolkit version >=2.0.0 has breaking changes. More details # mentioned on this issue https://github.com/openedx/edx-platform/issues/32884 django-oauth-toolkit==1.7.1 -# incremental upgrade -django-simple-history==3.4.0 # constrained in opaque_keys. migration guide here: https://pymongo.readthedocs.io/en/4.0/migrate-to-pymongo4.html # Major upgrade will be done in separate ticket. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 3738e7d53f..f15dc4efd5 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -56,7 +56,6 @@ backoff==1.10.0 backports-zoneinfo[tzdata]==0.2.1 # via # celery - # django # icalendar # kombu beautifulsoup4==4.12.3 @@ -174,9 +173,9 @@ defusedxml==0.7.1 # social-auth-core deprecated==1.2.14 # via jwcrypto -django==4.2.9 +django==3.2.23 # via - # -c requirements/edx/../constraints.txt + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/kernel.in # django-appconf # django-celery-results @@ -339,7 +338,6 @@ django-ses==3.5.2 # via -r requirements/edx/bundled.in django-simple-history==3.4.0 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in # edx-enterprise # edx-name-affirmation @@ -433,7 +431,7 @@ edx-ccx-keys==1.2.1 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -edx-celeryutils==1.2.5 +edx-celeryutils==1.2.3 # via # -r requirements/edx/kernel.in # edx-name-affirmation @@ -942,6 +940,7 @@ pytz==2023.4 # via # -r requirements/edx/kernel.in # babel + # django # django-ses # djangorestframework # drf-yasg diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 8def54dd19..1ff871d3be 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -116,7 +116,6 @@ backports-zoneinfo[tzdata]==0.2.1 # -r requirements/edx/testing.txt # backports-zoneinfo # celery - # django # icalendar # kombu beautifulsoup4==4.12.3 @@ -342,9 +341,9 @@ distlib==0.3.8 # via # -r requirements/edx/testing.txt # virtualenv -django==4.2.9 +django==3.2.23 # via - # -c requirements/edx/../constraints.txt + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # django-appconf @@ -560,7 +559,6 @@ django-ses==3.5.2 # -r requirements/edx/testing.txt django-simple-history==3.4.0 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # edx-enterprise @@ -703,7 +701,7 @@ edx-ccx-keys==1.2.1 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # lti-consumer-xblock -edx-celeryutils==1.2.5 +edx-celeryutils==1.2.3 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1672,6 +1670,7 @@ pytz==2023.4 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # babel + # django # django-ses # djangorestframework # drf-yasg diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 4572b11fb5..17092277ab 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -79,7 +79,6 @@ backports-zoneinfo[tzdata]==0.2.1 # -r requirements/edx/base.txt # backports-zoneinfo # celery - # django # icalendar # kombu beautifulsoup4==4.12.3 @@ -224,9 +223,9 @@ deprecated==1.2.14 # via # -r requirements/edx/base.txt # jwcrypto -django==4.2.9 +django==3.2.23 # via - # -c requirements/edx/../constraints.txt + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.txt # django-appconf # django-celery-results @@ -405,7 +404,6 @@ django-ses==3.5.2 # via -r requirements/edx/base.txt django-simple-history==3.4.0 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # edx-enterprise # edx-name-affirmation @@ -511,7 +509,7 @@ edx-ccx-keys==1.2.1 # via # -r requirements/edx/base.txt # lti-consumer-xblock -edx-celeryutils==1.2.5 +edx-celeryutils==1.2.3 # via # -r requirements/edx/base.txt # edx-name-affirmation @@ -1125,6 +1123,7 @@ pytz==2023.4 # via # -r requirements/edx/base.txt # babel + # django # django-ses # djangorestframework # drf-yasg diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 553a099d95..3d2cc3e5da 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -81,7 +81,6 @@ backports-zoneinfo[tzdata]==0.2.1 # -r requirements/edx/base.txt # backports-zoneinfo # celery - # django # icalendar # kombu beautifulsoup4==4.12.3 @@ -255,9 +254,9 @@ dill==0.3.8 # via pylint distlib==0.3.8 # via virtualenv -django==4.2.9 +django==3.2.23 # via - # -c requirements/edx/../constraints.txt + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.txt # django-appconf # django-celery-results @@ -436,7 +435,6 @@ django-ses==3.5.2 # via -r requirements/edx/base.txt django-simple-history==3.4.0 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # edx-enterprise # edx-name-affirmation @@ -537,7 +535,7 @@ edx-ccx-keys==1.2.1 # via # -r requirements/edx/base.txt # lti-consumer-xblock -edx-celeryutils==1.2.5 +edx-celeryutils==1.2.3 # via # -r requirements/edx/base.txt # edx-name-affirmation @@ -1253,6 +1251,7 @@ pytz==2023.4 # via # -r requirements/edx/base.txt # babel + # django # django-ses # djangorestframework # drf-yasg