diff --git a/cms/djangoapps/contentstore/rest_api/v0/tests/test_assets.py b/cms/djangoapps/contentstore/rest_api/v0/tests/test_assets.py index 38429910edd1..46b636916df8 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/tests/test_assets.py +++ b/cms/djangoapps/contentstore/rest_api/v0/tests/test_assets.py @@ -5,12 +5,11 @@ not the underlying Xblock service. It checks that the assets_handler method of the Xblock service is called with the expected parameters. """ -from unittest.mock import patch +from unittest.mock import patch, MagicMock + from django.core.files import File from django.http import JsonResponse - from django.urls import reverse -from mock import MagicMock from rest_framework import status from rest_framework.test import APITestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py index 971b2b996ab9..66b5f46128c8 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py @@ -1,8 +1,9 @@ """ Unit tests for Contentstore Proctored Exam Settings. """ +from unittest.mock import patch + import ddt -from mock import patch from django.conf import settings from django.test.utils import override_settings from django.urls import reverse diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py index 2c867ad0947c..5365443c47d9 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py @@ -1,10 +1,11 @@ """ Unit tests for course settings views. """ +from unittest.mock import patch + import ddt from django.conf import settings from django.urls import reverse -from mock import patch from rest_framework import status from cms.djangoapps.contentstore.tests.utils import CourseTestCase diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_videos.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_videos.py index 993072e0717f..1d086cb9fda0 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_videos.py @@ -1,6 +1,8 @@ """ Unit tests for course settings views. """ +from unittest.mock import patch + import ddt from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage @@ -12,7 +14,6 @@ get_transcript_credentials_state_for_org, get_transcript_preferences, ) -from mock import patch from rest_framework import status from cms.djangoapps.contentstore.video_storage_handlers import get_all_transcript_languages diff --git a/cms/envs/common.py b/cms/envs/common.py index ee4aa6eb0818..45e3ccb42ce7 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -915,6 +915,7 @@ # Various monitoring middleware 'edx_django_utils.monitoring.CookieMonitoringMiddleware', 'edx_django_utils.monitoring.DeploymentMonitoringMiddleware', + 'edx_django_utils.monitoring.FrontendMonitoringMiddleware', 'edx_django_utils.monitoring.MonitoringMemoryMiddleware', # Before anything that looks at cookies, especially the session middleware @@ -2958,13 +2959,13 @@ def _should_send_learning_badge_events(settings): BEAMER_PRODUCT_ID = "" -################### Studio Search (alpha, using Meilisearch) ################### +################### Studio Search (beta), using Meilisearch ################### # Enable Studio search features (powered by Meilisearch) (beta, off by default) MEILISEARCH_ENABLED = False # Meilisearch URL that the python backend can use. Often points to another docker container or k8s service. MEILISEARCH_URL = "http://meilisearch" -# URL that browsers (end users) can user to reach Meilisearch. Should be HTTPS in production. +# URL that browsers (end users) can use to reach Meilisearch. Should be HTTPS in production. MEILISEARCH_PUBLIC_URL = "http://meilisearch.example.com" # To support multi-tenancy, you can prefix all indexes with a common key like "sandbox7-" # and use a restricted tenant token in place of an API key, so that this Open edX instance diff --git a/common/djangoapps/third_party_auth/migrations/0013_default_site_id_wrapper_function.py b/common/djangoapps/third_party_auth/migrations/0013_default_site_id_wrapper_function.py new file mode 100644 index 000000000000..250e5370854e --- /dev/null +++ b/common/djangoapps/third_party_auth/migrations/0013_default_site_id_wrapper_function.py @@ -0,0 +1,36 @@ +# Generated by Django 4.2.13 on 2024-05-13 19:22 + +import common.djangoapps.third_party_auth.models +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('sites', '0002_alter_domain_unique'), + ('third_party_auth', '0012_alter_ltiproviderconfig_site_and_more'), + ] + + operations = [ + migrations.AlterField( + model_name='ltiproviderconfig', + name='site', + field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this provider configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'), + ), + migrations.AlterField( + model_name='oauth2providerconfig', + name='site', + field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this provider configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'), + ), + migrations.AlterField( + model_name='samlconfiguration', + name='site', + field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this SAML configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'), + ), + migrations.AlterField( + model_name='samlproviderconfig', + name='site', + field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this provider configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'), + ), + ] diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index 1ea265e868f2..6d244d96eddd 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -88,6 +88,24 @@ def __str__(self): ) +def _get_site_id_from_settings() -> int: + """ + Simply return SITE_ID from settings. + + We define this function so the current SITE_ID can be used as the default value on a + couple fields in this module. We can't use `settings.SITE_ID` directly in the model class, + because then the migration file will contain whatever the value of SITE_ID was when the + migration was created. This value is usually 1, but in the event that a developer is + running a non-default site, they would get a value like 2 or 3, which will result in + a dirty migration state. By defining this wrapper function, the name + `_get_site_id_from_settings` will be serialized to the migration file as the default, + regardless of what any developer's active SITE_ID is. + + Reference: https://docs.djangoproject.com/en/dev/ref/models/fields/#default + """ + return settings.SITE_ID + + class ProviderConfig(ConfigurationModel): """ Abstract Base Class for configuring a third_party_auth provider @@ -143,7 +161,7 @@ class ProviderConfig(ConfigurationModel): ) site = models.ForeignKey( Site, - default=settings.SITE_ID, + default=_get_site_id_from_settings, related_name='%(class)ss', help_text=_( 'The Site that this provider configuration belongs to.' @@ -455,7 +473,7 @@ class SAMLConfiguration(ConfigurationModel): KEY_FIELDS = ('site_id', 'slug') site = models.ForeignKey( Site, - default=settings.SITE_ID, + default=_get_site_id_from_settings, related_name='%(class)ss', help_text=_( 'The Site that this SAML configuration belongs to.' diff --git a/common/test/utils.py b/common/test/utils.py index d772f61edd56..ceddf775691a 100644 --- a/common/test/utils.py +++ b/common/test/utils.py @@ -1,6 +1,7 @@ """ General testing utilities. """ +from unittest.mock import Mock, patch import functools @@ -9,7 +10,6 @@ from django.dispatch import Signal from markupsafe import escape -from mock import Mock, patch @contextmanager diff --git a/lms/djangoapps/course_goals/tests/test_user_activity.py b/lms/djangoapps/course_goals/tests/test_user_activity.py index 0946ba158b95..04eb267152d4 100644 --- a/lms/djangoapps/course_goals/tests/test_user_activity.py +++ b/lms/djangoapps/course_goals/tests/test_user_activity.py @@ -3,7 +3,7 @@ """ from datetime import datetime, timedelta -from unittest.mock import Mock +from unittest.mock import Mock, patch import ddt from django.contrib.auth import get_user_model @@ -12,7 +12,6 @@ from edx_django_utils.cache import TieredCache from edx_toggles.toggles.testutils import override_waffle_flag from freezegun import freeze_time -from mock import patch from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory 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 4b3524eb1ee2..23052e5e7d90 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 @@ -1,14 +1,13 @@ """ Tests for the Course Home Course Metadata API in the Course Home API """ +import json +from unittest.mock import patch import ddt -import json -import mock from django.db import transaction from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag -from unittest.mock import patch from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment @@ -108,7 +107,7 @@ def test_streak_data_in_response(self): CourseEnrollment.enroll(self.user, self.course.id, 'audit') with override_waffle_flag(COURSEWARE_MFE_MILESTONES_STREAK_DISCOUNT, active=True): UPDATES_METHOD_NAME = 'common.djangoapps.student.models.user.UserCelebration.perform_streak_updates' - with mock.patch(UPDATES_METHOD_NAME, return_value=3): + with 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 @@ -187,7 +186,7 @@ def test_course_access( self.update_masquerade(role=masquerade_role) consent_url = 'dump/consent/url' if dsc_required else None - with mock.patch('openedx.features.enterprise_support.api.get_enterprise_consent_url', return_value=consent_url): + with patch('openedx.features.enterprise_support.api.get_enterprise_consent_url', return_value=consent_url): response = self.client.get(self.url) self._assert_course_access_response(response, expect_course_access, error_code) diff --git a/lms/djangoapps/discussion/django_comment_client/base/tests.py b/lms/djangoapps/discussion/django_comment_client/base/tests.py index 6aa9c3e8e9fd..f8242efa0c9c 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/tests.py +++ b/lms/djangoapps/discussion/django_comment_client/base/tests.py @@ -6,15 +6,15 @@ import json import logging from contextlib import contextmanager +from unittest import mock +from unittest.mock import ANY, Mock, patch import ddt -import mock from django.contrib.auth.models import User from django.core.management import call_command from django.test.client import RequestFactory from django.urls import reverse from eventtracking.processors.exceptions import EventEmissionExit -from mock import ANY, Mock, patch from opaque_keys.edx.keys import CourseKey from openedx_events.learning.signals import FORUM_THREAD_CREATED, FORUM_THREAD_RESPONSE_CREATED, FORUM_RESPONSE_COMMENT_CREATED diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index 7244127dc2a3..19ccf26d19a4 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -279,7 +279,7 @@ def get_thread_list_url(request, course_key, topic_id_list=None, following=False return request.build_absolute_uri(urlunparse(("", "", path, "", urlencode(query_list), ""))) -def get_course(request, course_key): +def get_course(request, course_key, check_tab=True): """ Return general discussion information for the course. @@ -289,6 +289,7 @@ def get_course(request, course_key): determining the requesting user. course_key: The key of the course to get information for + check_tab: Whether to check if the discussion tab is enabled for the course Returns: @@ -322,7 +323,7 @@ def _format_datetime(dt): """ return dt.isoformat().replace('+00:00', 'Z') - course = _get_course(course_key, request.user) + course = _get_course(course_key, request.user, check_tab=check_tab) user_roles = get_user_role_names(request.user, course_key) course_config = DiscussionsConfiguration.get(course_key) EDIT_REASON_CODES = getattr(settings, "DISCUSSION_MODERATION_EDIT_REASON_CODES", {}) @@ -331,7 +332,7 @@ def _format_datetime(dt): course_config.posting_restrictions, course.get_discussion_blackout_datetimes() ) - + discussion_tab = CourseTabList.get_tab_by_type(course.tabs, 'discussion') return { "id": str(course_key), "is_posting_enabled": is_posting_enabled, @@ -370,7 +371,7 @@ def _format_datetime(dt): {"code": reason_code, "label": label} for (reason_code, label) in CLOSE_REASON_CODES.items() ], - + 'show_discussions': bool(discussion_tab and discussion_tab.is_enabled(course, request.user)), } diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index 18c4447a54c1..5e34b688d15f 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -189,6 +189,10 @@ def test_discussions_disabled(self): with pytest.raises(DiscussionDisabledError): get_course(self.request, _discussion_disabled_course_for(self.user).id) + def test_discussions_disabled_v2(self): + data = get_course(self.request, _discussion_disabled_course_for(self.user).id, False) + assert data['show_discussions'] is False + def test_basic(self): assert get_course(self.request, self.course.id) == { 'id': str(self.course.id), @@ -211,6 +215,7 @@ def test_basic(self): 'user_roles': {'Student'}, 'edit_reasons': [{'code': 'test-edit-reason', 'label': 'Test Edit Reason'}], 'post_close_reasons': [{'code': 'test-close-reason', 'label': 'Test Close Reason'}], + 'show_discussions': True, } @ddt.data( diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index af41e4b87174..d7bf6c0e139e 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -538,6 +538,7 @@ def test_basic(self): "user_roles": ["Student"], "edit_reasons": [{"code": "test-edit-reason", "label": "Test Edit Reason"}], "post_close_reasons": [{"code": "test-close-reason", "label": "Test Close Reason"}], + 'show_discussions': True, } ) diff --git a/lms/djangoapps/discussion/rest_api/urls.py b/lms/djangoapps/discussion/rest_api/urls.py index 13d06288d639..f8c5bb3255b0 100644 --- a/lms/djangoapps/discussion/rest_api/urls.py +++ b/lms/djangoapps/discussion/rest_api/urls.py @@ -16,6 +16,7 @@ CourseTopicsViewV2, CourseTopicsViewV3, CourseView, + CourseViewV2, LearnerThreadView, ReplaceUsernamesView, RetireUserView, @@ -64,6 +65,11 @@ CourseView.as_view(), name="discussion_course" ), + re_path( + fr"^v2/courses/{settings.COURSE_ID_PATTERN}", + CourseViewV2.as_view(), + name="discussion_course_v2" + ), re_path(r'^v1/accounts/retire_forum/?$', RetireUserView.as_view(), name="retire_discussion_user"), path('v1/accounts/replace_username', ReplaceUsernamesView.as_view(), name="replace_discussion_username"), re_path( diff --git a/lms/djangoapps/discussion/rest_api/views.py b/lms/djangoapps/discussion/rest_api/views.py index 905b4ab49038..2b0bf8c41e5b 100644 --- a/lms/djangoapps/discussion/rest_api/views.py +++ b/lms/djangoapps/discussion/rest_api/views.py @@ -119,6 +119,36 @@ def get(self, request, course_id): return Response(get_course(request, course_key)) +@view_auth_classes() +class CourseViewV2(DeveloperErrorViewMixin, APIView): + """ + General discussion metadata API v2. + """ + + @apidocs.schema( + parameters=[ + apidocs.string_parameter("course_id", apidocs.ParameterLocation.PATH, description="Course ID") + ], + responses={ + 200: CourseMetadataSerailizer(read_only=True, required=False), + 401: "The requester is not authenticated.", + 403: "The requester cannot access the specified course.", + 404: "The requested course does not exist.", + } + ) + def get(self, request, course_id): + """ + Retrieve general discussion metadata for a course. + + **Example Requests**: + GET /api/discussion/v2/courses/course-v1:ExampleX+Subject101+2015 + """ + course_key = CourseKey.from_string(course_id) + # Record user activity for tracking progress towards a user's course goals (for mobile app) + UserActivity.record_user_activity(request.user, course_key, request=request, only_if_mobile_app=True) + return Response(get_course(request, course_key, False)) + + @view_auth_classes() class CourseActivityStatsView(DeveloperErrorViewMixin, APIView): """ diff --git a/lms/djangoapps/instructor/tests/test_tools.py b/lms/djangoapps/instructor/tests/test_tools.py index 0141ac9e8381..b9f754dbfff1 100644 --- a/lms/djangoapps/instructor/tests/test_tools.py +++ b/lms/djangoapps/instructor/tests/test_tools.py @@ -7,6 +7,7 @@ import json import unittest from unittest import mock +from unittest.mock import MagicMock, patch import pytest from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user @@ -14,7 +15,6 @@ from django.test import TestCase from edx_when.api import get_dates_for_course, set_dates_for_course from edx_when.field_data import DateLookupFieldData -from mock.mock import MagicMock, patch from opaque_keys.edx.keys import CourseKey from pytz import UTC from xmodule.fields import Date diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py b/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py index 6c50f68d6811..51d9acba54cc 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py @@ -1,11 +1,11 @@ """ Tests for serializers for the Mobile Course Info """ +from unittest.mock import MagicMock, Mock, patch +from typing import Dict, List, Tuple, Union import ddt from django.test import TestCase -from mock import MagicMock, Mock, patch -from typing import Dict, List, Tuple, Union from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.mobile_api.course_info.serializers import ( diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_views.py b/lms/djangoapps/mobile_api/tests/test_course_info_views.py index 67d2c79f9017..25fe08980379 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -1,6 +1,7 @@ """ Tests for course_info """ +from unittest.mock import patch import ddt @@ -11,7 +12,6 @@ from django.urls import reverse from edx_toggles.toggles.testutils import override_waffle_flag from milestones.tests.utils import MilestonesTestCaseMixin -from mock import patch from rest_framework import status from common.djangoapps.student.tests.factories import UserFactory # pylint: disable=unused-import diff --git a/lms/envs/common.py b/lms/envs/common.py index 3f18c96acf92..a65a7ab195ce 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2256,6 +2256,7 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring 'edx_django_utils.monitoring.CodeOwnerMonitoringMiddleware', 'edx_django_utils.monitoring.CookieMonitoringMiddleware', 'edx_django_utils.monitoring.DeploymentMonitoringMiddleware', + 'edx_django_utils.monitoring.FrontendMonitoringMiddleware', # Before anything that looks at cookies, especially the session middleware 'openedx.core.djangoapps.cookie_metadata.middleware.CookieNameChange', diff --git a/openedx/core/djangoapps/catalog/tests/test_api.py b/openedx/core/djangoapps/catalog/tests/test_api.py index ed657d992d57..318111547e89 100644 --- a/openedx/core/djangoapps/catalog/tests/test_api.py +++ b/openedx/core/djangoapps/catalog/tests/test_api.py @@ -1,7 +1,7 @@ """ Tests for the Catalog apps `api.py` functions. """ -from mock import patch +from unittest.mock import patch from django.test import TestCase diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_api.py b/openedx/core/djangoapps/content/course_overviews/tests/test_api.py index d98c6f0d016f..568322f82d98 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_api.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_api.py @@ -1,7 +1,7 @@ """ course_overview api tests """ -from mock import patch +from unittest.mock import patch from django.http.response import Http404 from opaque_keys.edx.keys import CourseKey diff --git a/openedx/core/djangoapps/discussions/tests/test_tasks.py b/openedx/core/djangoapps/discussions/tests/test_tasks.py index fa04713ae426..00b715c945e6 100644 --- a/openedx/core/djangoapps/discussions/tests/test_tasks.py +++ b/openedx/core/djangoapps/discussions/tests/test_tasks.py @@ -1,8 +1,9 @@ """ Tests for discussions tasks. """ +from unittest import mock + import ddt -import mock from edx_toggles.toggles.testutils import override_waffle_flag from openedx_events.learning.data import DiscussionTopicContext diff --git a/openedx/core/djangoapps/theming/tests/test_commands.py b/openedx/core/djangoapps/theming/tests/test_commands.py deleted file mode 100644 index 7ca56f604a41..000000000000 --- a/openedx/core/djangoapps/theming/tests/test_commands.py +++ /dev/null @@ -1,24 +0,0 @@ -""" -Tests for Management commands of comprehensive theming. -""" - -from django.core.management import call_command -from django.test import TestCase, override_settings -from unittest.mock import patch - -import pavelib.assets - - -class TestUpdateAssets(TestCase): - """ - Test comprehensive theming helper functions. - """ - - @patch.object(pavelib.assets, 'sh') - @override_settings(COMPREHENSIVE_THEME_DIRS='common/test') - def test_deprecated_wrapper(self, mock_sh): - call_command('compile_sass', '--themes', 'fake-theme1', 'fake-theme2') - assert mock_sh.called_once_with( - "npm run compile-sass -- " + - "--theme-dir common/test --theme fake-theme-1 --theme fake-theme-2" - ) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 71e545abbc04..d0414ef8790a 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -436,7 +436,7 @@ edx-django-release-util==1.4.0 # edxval edx-django-sites-extensions==4.2.0 # via -r requirements/edx/kernel.in -edx-django-utils==5.13.0 +edx-django-utils==5.14.1 # via # -r requirements/edx/kernel.in # django-config-models diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 65597387ab5f..11ad4f85cd8a 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -713,7 +713,7 @@ edx-django-sites-extensions==4.2.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-django-utils==5.13.0 +edx-django-utils==5.14.1 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 71f4dee8a69f..1131386cf5f9 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -510,7 +510,7 @@ edx-django-release-util==1.4.0 # edxval edx-django-sites-extensions==4.2.0 # via -r requirements/edx/base.txt -edx-django-utils==5.13.0 +edx-django-utils==5.14.1 # via # -r requirements/edx/base.txt # django-config-models diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index b683b31895f2..9321ca67ff31 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -68,8 +68,8 @@ edx-completion edx-django-release-util # Release utils for the edx release pipeline edx-django-sites-extensions edx-codejail -# edx-django-utils 5.4.0 adds CSP middleware -edx-django-utils>=5.4.0 # Utilities for cache, monitoring, and plugins +# edx-django-utils 5.14.1 adds FrontendMonitoringMiddleware +edx-django-utils>=5.14.1 # Utilities for cache, monitoring, and plugins edx-drf-extensions edx-enterprise # edx-event-bus-kafka 5.6.0 adds support for putting client ids on event producers/consumers diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 617b4e8ed250..e8df4b1a0eb7 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -543,7 +543,7 @@ edx-django-release-util==1.4.0 # edxval edx-django-sites-extensions==4.2.0 # via -r requirements/edx/base.txt -edx-django-utils==5.13.0 +edx-django-utils==5.14.1 # via # -r requirements/edx/base.txt # django-config-models diff --git a/scripts/user_retirement/requirements/base.txt b/scripts/user_retirement/requirements/base.txt index 9945c989ae62..27e8d695c6b3 100644 --- a/scripts/user_retirement/requirements/base.txt +++ b/scripts/user_retirement/requirements/base.txt @@ -50,7 +50,7 @@ django-crum==0.7.9 # via edx-django-utils django-waffle==4.1.0 # via edx-django-utils -edx-django-utils==5.13.0 +edx-django-utils==5.14.1 # via edx-rest-api-client edx-rest-api-client==5.7.0 # via -r scripts/user_retirement/requirements/base.in diff --git a/scripts/user_retirement/requirements/testing.txt b/scripts/user_retirement/requirements/testing.txt index 67d1d9098845..2468d17662ea 100644 --- a/scripts/user_retirement/requirements/testing.txt +++ b/scripts/user_retirement/requirements/testing.txt @@ -70,7 +70,7 @@ django-waffle==4.1.0 # via # -r scripts/user_retirement/requirements/base.txt # edx-django-utils -edx-django-utils==5.13.0 +edx-django-utils==5.14.1 # via # -r scripts/user_retirement/requirements/base.txt # edx-rest-api-client diff --git a/scripts/user_retirement/tests/test_get_learners_to_retire.py b/scripts/user_retirement/tests/test_get_learners_to_retire.py index 480de92185db..557398a2720f 100644 --- a/scripts/user_retirement/tests/test_get_learners_to_retire.py +++ b/scripts/user_retirement/tests/test_get_learners_to_retire.py @@ -1,11 +1,11 @@ """ Test the get_learners_to_retire.py script """ +from unittest.mock import DEFAULT, patch import os from click.testing import CliRunner -from mock import DEFAULT, patch from requests.exceptions import HTTPError from scripts.user_retirement.get_learners_to_retire import get_learners_to_retire diff --git a/scripts/user_retirement/tests/test_retire_one_learner.py b/scripts/user_retirement/tests/test_retire_one_learner.py index a78b6b787acb..844325ea3ed0 100644 --- a/scripts/user_retirement/tests/test_retire_one_learner.py +++ b/scripts/user_retirement/tests/test_retire_one_learner.py @@ -1,9 +1,9 @@ """ Test the retire_one_learner.py script """ +from unittest.mock import DEFAULT, patch from click.testing import CliRunner -from mock import DEFAULT, patch from scripts.user_retirement.retire_one_learner import ( END_STATES, diff --git a/scripts/user_retirement/tests/test_retirement_archive_and_cleanup.py b/scripts/user_retirement/tests/test_retirement_archive_and_cleanup.py index 3a6a847e1d6e..aa6715ba7d6c 100644 --- a/scripts/user_retirement/tests/test_retirement_archive_and_cleanup.py +++ b/scripts/user_retirement/tests/test_retirement_archive_and_cleanup.py @@ -4,12 +4,12 @@ import datetime import os +from unittest.mock import DEFAULT, call, patch import boto3 import pytest from botocore.exceptions import ClientError from click.testing import CliRunner -from mock import DEFAULT, call, patch from moto import mock_ec2, mock_s3 from scripts.user_retirement.retirement_archive_and_cleanup import ( diff --git a/scripts/user_retirement/tests/test_retirement_bulk_status_update.py b/scripts/user_retirement/tests/test_retirement_bulk_status_update.py index d2a8bab60ba3..0cfbeeef3d7a 100644 --- a/scripts/user_retirement/tests/test_retirement_bulk_status_update.py +++ b/scripts/user_retirement/tests/test_retirement_bulk_status_update.py @@ -1,9 +1,9 @@ """ Test the retirement_bulk_status_update.py script """ +from unittest.mock import DEFAULT, patch from click.testing import CliRunner -from mock import DEFAULT, patch from scripts.user_retirement.retirement_bulk_status_update import ( ERR_BAD_CONFIG, diff --git a/scripts/user_retirement/tests/test_retirement_partner_report.py b/scripts/user_retirement/tests/test_retirement_partner_report.py index c9d05a974396..886f2d9e4073 100644 --- a/scripts/user_retirement/tests/test_retirement_partner_report.py +++ b/scripts/user_retirement/tests/test_retirement_partner_report.py @@ -8,9 +8,9 @@ import time import unicodedata from datetime import date +from unittest.mock import DEFAULT, patch from click.testing import CliRunner -from mock import DEFAULT, patch from six import PY2, itervalues from scripts.user_retirement.retirement_partner_report import \ diff --git a/scripts/user_retirement/tests/utils/test_edx_api.py b/scripts/user_retirement/tests/utils/test_edx_api.py index 1c1aa78c39bd..706b1ce6d4a7 100644 --- a/scripts/user_retirement/tests/utils/test_edx_api.py +++ b/scripts/user_retirement/tests/utils/test_edx_api.py @@ -2,12 +2,12 @@ Tests for edX API calls. """ import unittest +from unittest.mock import DEFAULT, patch from urllib.parse import urljoin import requests import responses from ddt import data, ddt, unpack -from mock import DEFAULT, patch from requests.exceptions import ConnectionError, HTTPError from responses import GET, PATCH, POST, matchers from responses.registries import OrderedRegistry diff --git a/scripts/user_retirement/tests/utils/test_jenkins.py b/scripts/user_retirement/tests/utils/test_jenkins.py index 8d32ee24c2df..bd51c0f5d779 100644 --- a/scripts/user_retirement/tests/utils/test_jenkins.py +++ b/scripts/user_retirement/tests/utils/test_jenkins.py @@ -6,11 +6,11 @@ import re import unittest from itertools import islice +from unittest.mock import Mock, call, mock_open, patch import backoff import ddt import requests_mock -from mock import Mock, call, mock_open, patch import scripts.user_retirement.utils.jenkins as jenkins from scripts.user_retirement.utils.exception import BackendError diff --git a/scripts/user_retirement/tests/utils/thirdparty_apis/test_salesforce.py b/scripts/user_retirement/tests/utils/thirdparty_apis/test_salesforce.py index 47770439533e..2267f38369b9 100644 --- a/scripts/user_retirement/tests/utils/thirdparty_apis/test_salesforce.py +++ b/scripts/user_retirement/tests/utils/thirdparty_apis/test_salesforce.py @@ -3,8 +3,8 @@ """ import logging from contextlib import contextmanager +from unittest import mock -import mock import pytest from simple_salesforce import SalesforceError diff --git a/scripts/user_retirement/tests/utils/thirdparty_apis/test_segment_api.py b/scripts/user_retirement/tests/utils/thirdparty_apis/test_segment_api.py index 8d413c7d3282..c52a5c5074b1 100644 --- a/scripts/user_retirement/tests/utils/thirdparty_apis/test_segment_api.py +++ b/scripts/user_retirement/tests/utils/thirdparty_apis/test_segment_api.py @@ -2,8 +2,8 @@ Tests for the Segment API functionality """ import json +from unittest import mock -import mock import pytest import requests from six import text_type diff --git a/xmodule/capa/tests/helpers.py b/xmodule/capa/tests/helpers.py index 41fd485aef46..540d459c5ab2 100644 --- a/xmodule/capa/tests/helpers.py +++ b/xmodule/capa/tests/helpers.py @@ -6,10 +6,10 @@ import os import os.path import xml.sax.saxutils as saxutils +from unittest.mock import MagicMock, Mock import fs.osfs from mako.lookup import TemplateLookup -from mock import MagicMock, Mock from path import Path from xmodule.capa.capa_problem import LoncapaProblem, LoncapaSystem diff --git a/xmodule/capa/tests/test_capa_problem.py b/xmodule/capa/tests/test_capa_problem.py index 88a06b4c7ed7..74cf4d096fd1 100644 --- a/xmodule/capa/tests/test_capa_problem.py +++ b/xmodule/capa/tests/test_capa_problem.py @@ -3,6 +3,7 @@ """ import textwrap import unittest +from unittest.mock import patch, MagicMock from django.conf import settings from django.test import override_settings @@ -10,7 +11,6 @@ import ddt from lxml import etree from markupsafe import Markup -from mock import patch, MagicMock from xmodule.capa.correctmap import CorrectMap from xmodule.capa.responsetypes import LoncapaProblemError diff --git a/xmodule/capa/tests/test_html_render.py b/xmodule/capa/tests/test_html_render.py index 91500a0fdf56..2a9a7867718f 100644 --- a/xmodule/capa/tests/test_html_render.py +++ b/xmodule/capa/tests/test_html_render.py @@ -6,9 +6,9 @@ import os import textwrap import unittest +from unittest import mock import ddt -import mock from lxml import etree from xmodule.capa.tests.helpers import new_loncapa_problem, test_capa_system from openedx.core.djangolib.markup import HTML diff --git a/xmodule/capa/tests/test_inputtypes.py b/xmodule/capa/tests/test_inputtypes.py index 1d7e43b650a2..e7cc09eb572e 100644 --- a/xmodule/capa/tests/test_inputtypes.py +++ b/xmodule/capa/tests/test_inputtypes.py @@ -23,12 +23,12 @@ import unittest import xml.sax.saxutils as saxutils from collections import OrderedDict +from unittest.mock import ANY, patch import pytest import six from lxml import etree from lxml.html import fromstring -from mock import ANY, patch from pyparsing import ParseException from six.moves import zip diff --git a/xmodule/capa/tests/test_responsetypes.py b/xmodule/capa/tests/test_responsetypes.py index 936093aa6e21..37a6647ce7d0 100644 --- a/xmodule/capa/tests/test_responsetypes.py +++ b/xmodule/capa/tests/test_responsetypes.py @@ -9,10 +9,10 @@ import unittest import zipfile from datetime import datetime +from unittest import mock import pytest import calc -import mock import pyparsing import random2 as random import requests diff --git a/xmodule/modulestore/__init__.py b/xmodule/modulestore/__init__.py index 57a22f5e99b3..f3aee2a58f24 100644 --- a/xmodule/modulestore/__init__.py +++ b/xmodule/modulestore/__init__.py @@ -121,6 +121,7 @@ def __init__(self): self._active_count = 0 self.has_publish_item = False self.has_library_updated_item = False + self._commit_callbacks = [] @property def active(self): @@ -148,6 +149,20 @@ def is_root(self): """ return self._active_count == 1 + def defer_until_commit(self, fn): + """ + Run some code when the changes from this bulk op are committed to the DB + """ + self._commit_callbacks.append(fn) + + def call_commit_callbacks(self): + """ + When the changes have been committed to the DB, call this to run any queued callbacks + """ + for fn in self._commit_callbacks: + fn() + self._commit_callbacks.clear() + class ActiveBulkThread(threading.local): """ @@ -290,15 +305,34 @@ def _end_bulk_operation(self, structure_key, emit_signals=True, ignore_case=Fals # So re-nest until the signals are sent. bulk_ops_record.nest() - if emit_signals and dirty: - self.send_bulk_published_signal(bulk_ops_record, structure_key) - self.send_bulk_library_updated_signal(bulk_ops_record, structure_key) + if dirty: + # Call any "on commit" callback, regardless of if this was "published" or is still draft: + bulk_ops_record.call_commit_callbacks() + # Call any "on publish" handlers - emit_signals is usually false for draft-only changes: + if emit_signals: + self.send_bulk_published_signal(bulk_ops_record, structure_key) + self.send_bulk_library_updated_signal(bulk_ops_record, structure_key) # Signals are sent. Now unnest and clear the bulk op for good. bulk_ops_record.unnest() self._clear_bulk_ops_record(structure_key) + def on_commit_changes_to(self, course_key, fn): + """ + Call some callback when the currently active bulk operation has saved + """ + # Check if a bulk op is active. If so, defer fn(); otherwise call it immediately. + # Note: calling _get_bulk_ops_record() here and then checking .active can have side-effects in some cases + # because it creates an entry in the defaultdict if none exists, so we check if the record is active using + # the same code as _clear_bulk_ops_record(), which doesn't modify the defaultdict. + # so we check it this way: + if course_key and course_key.for_branch(None) in self._active_bulk_ops.records: + bulk_ops_record = self._active_bulk_ops.records[course_key.for_branch(None)] + bulk_ops_record.defer_until_commit(fn) + else: + fn() # There is no active bulk operation - call fn() now. + def _is_in_bulk_operation(self, course_key, ignore_case=False): """ Return whether a bulk operation is active on `course_key`. diff --git a/xmodule/modulestore/mixed.py b/xmodule/modulestore/mixed.py index 9665772d2718..e1ea6640acc5 100644 --- a/xmodule/modulestore/mixed.py +++ b/xmodule/modulestore/mixed.py @@ -758,15 +758,19 @@ def create_item(self, user_id, course_key, block_type, block_id=None, fields=Non """ modulestore = self._verify_modulestore_support(course_key, 'create_item') xblock = modulestore.create_item(user_id, course_key, block_type, block_id=block_id, fields=fields, **kwargs) - # .. event_implemented_name: XBLOCK_CREATED - XBLOCK_CREATED.send_event( - time=datetime.now(timezone.utc), - xblock_info=XBlockData( - usage_key=xblock.location.for_branch(None), - block_type=block_type, - version=xblock.location + + def send_created_event(): + # .. event_implemented_name: XBLOCK_CREATED + XBLOCK_CREATED.send_event( + time=datetime.now(timezone.utc), + xblock_info=XBlockData( + usage_key=xblock.location.for_branch(None), + block_type=block_type, + version=xblock.location + ) ) - ) + + modulestore.on_commit_changes_to(course_key, send_created_event) return xblock @strip_key @@ -787,19 +791,24 @@ def create_child(self, user_id, parent_usage_key, block_type, block_id=None, fie fields (dict): A dictionary specifying initial values for some or all fields in the newly created block """ - modulestore = self._verify_modulestore_support(parent_usage_key.course_key, 'create_child') - xblock = modulestore.create_child( + course_key = parent_usage_key.course_key + store = self._verify_modulestore_support(course_key, 'create_child') + xblock = store.create_child( user_id, parent_usage_key, block_type, block_id=block_id, fields=fields, **kwargs ) - # .. event_implemented_name: XBLOCK_CREATED - XBLOCK_CREATED.send_event( - time=datetime.now(timezone.utc), - xblock_info=XBlockData( - usage_key=xblock.location.for_branch(None), - block_type=block_type, - version=xblock.location + + def send_created_event(): + # .. event_implemented_name: XBLOCK_CREATED + XBLOCK_CREATED.send_event( + time=datetime.now(timezone.utc), + xblock_info=XBlockData( + usage_key=xblock.location.for_branch(None), + block_type=block_type, + version=xblock.location + ) ) - ) + + store.on_commit_changes_to(course_key, send_created_event) return xblock @strip_key @@ -828,17 +837,22 @@ def update_item(self, xblock, user_id, allow_not_found=False, **kwargs): # lint Update the xblock persisted to be the same as the given for all types of fields (content, children, and metadata) attribute the change to the given user. """ - store = self._verify_modulestore_support(xblock.location.course_key, 'update_item') + course_key = xblock.location.course_key + store = self._verify_modulestore_support(course_key, 'update_item') xblock = store.update_item(xblock, user_id, allow_not_found, **kwargs) - # .. event_implemented_name: XBLOCK_UPDATED - XBLOCK_UPDATED.send_event( - time=datetime.now(timezone.utc), - xblock_info=XBlockData( - usage_key=xblock.location.for_branch(None), - block_type=xblock.location.block_type, - version=xblock.location + + def send_updated_event(): + # .. event_implemented_name: XBLOCK_UPDATED + XBLOCK_UPDATED.send_event( + time=datetime.now(timezone.utc), + xblock_info=XBlockData( + usage_key=xblock.location.for_branch(None), + block_type=xblock.location.block_type, + version=xblock.location + ) ) - ) + + store.on_commit_changes_to(course_key, send_updated_event) return xblock @strip_key @@ -846,16 +860,21 @@ def delete_item(self, location, user_id, **kwargs): # lint-amnesty, pylint: dis """ Delete the given item from persistence. kwargs allow modulestore specific parameters. """ - store = self._verify_modulestore_support(location.course_key, 'delete_item') + course_key = location.course_key + store = self._verify_modulestore_support(course_key, 'delete_item') item = store.delete_item(location, user_id=user_id, **kwargs) - # .. event_implemented_name: XBLOCK_DELETED - XBLOCK_DELETED.send_event( - time=datetime.now(timezone.utc), - xblock_info=XBlockData( - usage_key=location, - block_type=location.block_type, + + def send_deleted_event(): + # .. event_implemented_name: XBLOCK_DELETED + XBLOCK_DELETED.send_event( + time=datetime.now(timezone.utc), + xblock_info=XBlockData( + usage_key=location, + block_type=location.block_type, + ) ) - ) + + store.on_commit_changes_to(course_key, send_deleted_event) return item def revert_to_published(self, location, user_id): diff --git a/xmodule/tests/test_capa_block.py b/xmodule/tests/test_capa_block.py index 42494547822e..96ec64de61e5 100644 --- a/xmodule/tests/test_capa_block.py +++ b/xmodule/tests/test_capa_block.py @@ -6,12 +6,11 @@ import datetime import json -import mock import os import random import textwrap import unittest -from unittest.mock import DEFAULT, Mock, patch +from unittest.mock import DEFAULT, Mock, PropertyMock, patch import pytest import ddt @@ -820,7 +819,7 @@ def test_submit_problem_grading_method_disable_to_enable( # Disabled grading method with patch( 'xmodule.capa_block.ProblemBlock.is_grading_method_enabled', - new_callable=mock.PropertyMock, + new_callable=PropertyMock, return_value=False ): # First Attempt @@ -846,7 +845,7 @@ def test_submit_problem_grading_method_disable_to_enable( # Enabled grading method with patch( 'xmodule.capa_block.ProblemBlock.is_grading_method_enabled', - new_callable=mock.PropertyMock, + new_callable=PropertyMock, return_value=True ): # Third Attempt @@ -887,7 +886,7 @@ def test_submit_problem_grading_method_enable_to_disable( # Enabled grading method with patch( 'xmodule.capa_block.ProblemBlock.is_grading_method_enabled', - new_callable=mock.PropertyMock, + new_callable=PropertyMock, return_value=True ): # First Attempt @@ -913,7 +912,7 @@ def test_submit_problem_grading_method_enable_to_disable( # Disabled grading method with patch( 'xmodule.capa_block.ProblemBlock.is_grading_method_enabled', - new_callable=mock.PropertyMock, + new_callable=PropertyMock, return_value=False ): # Third Attempt @@ -1636,7 +1635,7 @@ def test_rescore_problem_grading_method_disable_to_enable(self, mock_publish_gra # Disabled grading method with patch( 'xmodule.capa_block.ProblemBlock.is_grading_method_enabled', - new_callable=mock.PropertyMock, + new_callable=PropertyMock, return_value=False ): # Score is the last score @@ -1652,12 +1651,12 @@ def test_rescore_problem_grading_method_disable_to_enable(self, mock_publish_gra # Enabled grading method with patch( 'xmodule.capa_block.ProblemBlock.is_grading_method_enabled', - new_callable=mock.PropertyMock, + new_callable=PropertyMock, return_value=True ): with patch( 'xmodule.capa.capa_problem.LoncapaProblem.is_grading_method_enabled', - new_callable=mock.PropertyMock, + new_callable=PropertyMock, return_value=True ): # Change grading method to 'first_score' @@ -1706,12 +1705,12 @@ def test_rescore_problem_grading_method_enable_to_disable(self, mock_publish_gra # Enabled grading method with patch( 'xmodule.capa_block.ProblemBlock.is_grading_method_enabled', - new_callable=mock.PropertyMock, + new_callable=PropertyMock, return_value=True ): with patch( 'xmodule.capa.capa_problem.LoncapaProblem.is_grading_method_enabled', - new_callable=mock.PropertyMock, + new_callable=PropertyMock, return_value=True ): # Grading method is 'last_score' @@ -1745,7 +1744,7 @@ def test_rescore_problem_grading_method_enable_to_disable(self, mock_publish_gra # Disabled grading method with patch( 'xmodule.capa_block.ProblemBlock.is_grading_method_enabled', - new_callable=mock.PropertyMock, + new_callable=PropertyMock, return_value=False ): block.rescore(only_if_higher=False)