From a19993cd3e68ba6774b331b9f7ef7d4a265af8ca Mon Sep 17 00:00:00 2001 From: salmannawaz Date: Thu, 24 Oct 2024 21:12:12 +0500 Subject: [PATCH 01/22] Update catalog-info file for release data (#35691) * chore: Update catalog.yaml file for release data * chore: Delete openedx.yaml file --- catalog-info.yaml | 2 ++ openedx.yaml | 14 -------------- 2 files changed, 2 insertions(+), 14 deletions(-) delete mode 100644 openedx.yaml diff --git a/catalog-info.yaml b/catalog-info.yaml index 885b879833..5059638c7b 100644 --- a/catalog-info.yaml +++ b/catalog-info.yaml @@ -10,6 +10,8 @@ metadata: - url: "https://docs.openedx.org" title: "Documentation" icon: "Web" + annotations: + openedx.org/release: "2u/release" spec: owner: group:wg-maintenance-edx-platform type: 'service' diff --git a/openedx.yaml b/openedx.yaml deleted file mode 100644 index 867c055959..0000000000 --- a/openedx.yaml +++ /dev/null @@ -1,14 +0,0 @@ -# This file describes this Open edX repo, as described in OEP-2: -# https://open-edx-proposals.readthedocs.io/en/latest/oep-0002-bp-repo-metadata.html#specification - -nick: edx -openedx-release: {ref: "2u/release"} - -oeps: - oep-2: true - oep-18: true - oep-7: true - -tags: - - core - - webservice From 8539287a15a12a1935c6c7b065098c35148c4567 Mon Sep 17 00:00:00 2001 From: "Adolfo R. Brandes" Date: Thu, 24 Oct 2024 13:21:18 -0300 Subject: [PATCH 02/22] chore: update release branch to 'master' (#35722) --- catalog-info.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/catalog-info.yaml b/catalog-info.yaml index 5059638c7b..c6b87b08db 100644 --- a/catalog-info.yaml +++ b/catalog-info.yaml @@ -11,7 +11,7 @@ metadata: title: "Documentation" icon: "Web" annotations: - openedx.org/release: "2u/release" + openedx.org/release: "master" spec: owner: group:wg-maintenance-edx-platform type: 'service' From ab6d8cb6c25e6a8d6daea784d1e6935d052f966d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 24 Oct 2024 17:07:57 +0000 Subject: [PATCH 03/22] feat: Upgrade Python dependency edx-enterprise (#35721) Upgrade `edx-enterprise` to 4.29.0 Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: brobro10000 <82611798+brobro10000@users.noreply.github.com> Co-authored-by: Hamzah Ullah --- 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 312b6b7865..6c8fe968b2 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -82,7 +82,7 @@ django-storages<1.14.4 # 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.28.3 +edx-enterprise==4.29.0 # Date: 2024-05-09 # This has to be constrained as well because newer versions of edx-i18n-tools need the diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 166a9dc01b..35106f1498 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -467,7 +467,7 @@ edx-drf-extensions==10.4.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.28.3 +edx-enterprise==4.29.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 3283d27e21..b6a4b48d04 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -741,7 +741,7 @@ edx-drf-extensions==10.4.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.28.3 +edx-enterprise==4.29.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 338ab221d4..e0f08601b0 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -547,7 +547,7 @@ edx-drf-extensions==10.4.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.28.3 +edx-enterprise==4.29.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 14d7a977d8..9cd37f6e4b 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -571,7 +571,7 @@ edx-drf-extensions==10.4.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.28.3 +edx-enterprise==4.29.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From e8cdb064104d3f2d972e28363dda6bc8a5b3cf93 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Fri, 25 Oct 2024 13:16:56 -0500 Subject: [PATCH 04/22] feat!: remove all references to content library types (#35726) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At one point, we envisioned having different kinds of libraries, e.g. a "Video" library would be distinct from a "Problem" library. Later on, we decided on a more generalized form of Libraries, where any given library can hold any combination of content–which would then be organized using collections and tagging. Due to this shift in perspective, these values haven't actually been used for a long time. This is just getting rid of them altogether. --- .../views/tests/test_clipboard_paste.py | 1 - .../content/search/tests/test_api.py | 1 - .../core/djangoapps/content_libraries/api.py | 33 ++---------- .../djangoapps/content_libraries/constants.py | 10 ---- ...ove_contentlibrary_bundle_uuid_and_more.py | 21 ++++++++ .../djangoapps/content_libraries/models.py | 15 +----- .../content_libraries/serializers.py | 14 +----- .../content_libraries/tests/base.py | 5 +- .../tests/test_content_libraries.py | 50 ++----------------- .../content_libraries/tests/test_models.py | 2 - .../content_libraries/tests/test_runtime.py | 3 +- .../content_libraries/tests/test_views_lti.py | 6 +-- .../djangoapps/content_libraries/views.py | 5 -- 13 files changed, 35 insertions(+), 131 deletions(-) create mode 100644 openedx/core/djangoapps/content_libraries/migrations/0011_remove_contentlibrary_bundle_uuid_and_more.py diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index a818da81d1..68771ceaec 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -408,7 +408,6 @@ class ClipboardPasteFromV2LibraryTestCase(ModuleStoreTestCase): self.store = modulestore() self.library = library_api.create_library( - library_type=library_api.COMPLEX, org=Organization.objects.create(name="Test Org", short_name="CL-TEST"), slug="lib", title="Library", diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index c89d84490e..990226f343 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -115,7 +115,6 @@ class TestSearchApi(ModuleStoreTestCase): # Create a content library: self.library = library_api.create_library( - library_type=library_api.COMPLEX, org=OrganizationFactory.create(short_name="org1"), slug="lib", title="Library", diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 58b99a390f..fdfe2ff0ef 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -114,7 +114,7 @@ from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_f from xmodule.modulestore.django import modulestore from . import permissions, tasks -from .constants import ALL_RIGHTS_RESERVED, COMPLEX +from .constants import ALL_RIGHTS_RESERVED from .models import ContentLibrary, ContentLibraryPermission, ContentLibraryBlockImportTask log = logging.getLogger(__name__) @@ -176,7 +176,6 @@ class ContentLibraryMetadata: description = attr.ib("") num_blocks = attr.ib(0) version = attr.ib(0) - type = attr.ib(default=COMPLEX) last_published = attr.ib(default=None, type=datetime) last_draft_created = attr.ib(default=None, type=datetime) last_draft_created_by = attr.ib(default=None, type=datetime) @@ -306,15 +305,13 @@ class LibraryXBlockType: # ============ -def get_libraries_for_user(user, org=None, library_type=None, text_search=None, order=None): +def get_libraries_for_user(user, org=None, text_search=None, order=None): """ Return content libraries that the user has permission to view. """ filter_kwargs = {} if org: filter_kwargs['org__short_name'] = org - if library_type: - filter_kwargs['type'] = library_type qs = ContentLibrary.objects.filter(**filter_kwargs) \ .select_related('learning_package', 'org') \ .order_by('org__short_name', 'slug') @@ -361,7 +358,6 @@ def get_metadata(queryset, text_search=None): ContentLibraryMetadata( key=lib.library_key, title=lib.learning_package.title if lib.learning_package else "", - type=lib.type, description="", version=0, allow_public_learning=lib.allow_public_learning, @@ -446,7 +442,6 @@ def get_library(library_key): return ContentLibraryMetadata( key=library_key, title=learning_package.title, - type=ref.type, description=ref.learning_package.description, num_blocks=num_blocks, version=version, @@ -474,7 +469,6 @@ def create_library( allow_public_learning=False, allow_public_read=False, library_license=ALL_RIGHTS_RESERVED, - library_type=COMPLEX, ): """ Create a new content library. @@ -491,8 +485,6 @@ def create_library( allow_public_read: Allow anyone to view blocks (including source) in Studio? - library_type: Deprecated parameter, not really used. Set to COMPLEX. - Returns a ContentLibraryMetadata instance. """ assert isinstance(org, Organization) @@ -502,7 +494,6 @@ def create_library( ref = ContentLibrary.objects.create( org=org, slug=slug, - type=library_type, allow_public_learning=allow_public_learning, allow_public_read=allow_public_read, license=library_license, @@ -526,7 +517,6 @@ def create_library( return ContentLibraryMetadata( key=ref.library_key, title=title, - type=library_type, description=description, num_blocks=0, version=0, @@ -611,7 +601,6 @@ def update_library( description=None, allow_public_learning=None, allow_public_read=None, - library_type=None, library_license=None, ): """ @@ -621,7 +610,7 @@ def update_library( A value of None means "don't change". """ lib_obj_fields = [ - allow_public_learning, allow_public_read, library_type, library_license + allow_public_learning, allow_public_read, library_license ] lib_obj_changed = any(field is not None for field in lib_obj_fields) learning_pkg_changed = any(field is not None for field in [title, description]) @@ -640,10 +629,6 @@ def update_library( content_lib.allow_public_learning = allow_public_learning if allow_public_read is not None: content_lib.allow_public_read = allow_public_read - if library_type is not None: - # TODO: Get rid of this field entirely, and remove library_type - # from any functions that take it as an argument. - content_lib.library_type = library_type if library_license is not None: content_lib.library_license = library_license content_lib.save() @@ -856,13 +841,6 @@ def validate_can_add_block_to_library( """ assert isinstance(library_key, LibraryLocatorV2) content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] - if content_library.type != COMPLEX: - if block_type != content_library.type: - raise IncompatibleTypesError( - _('Block type "{block_type}" is not compatible with library type "{library_type}".').format( - block_type=block_type, library_type=content_library.type, - ) - ) # If adding a component would take us over our max, return an error. component_count = authoring_api.get_all_drafts(content_library.learning_package.id).count() @@ -1288,10 +1266,7 @@ def get_allowed_block_types(library_key): # pylint: disable=unused-argument # TODO: return support status and template options # See cms/djangoapps/contentstore/views/component.py block_types = sorted(name for name, class_ in XBlock.load_classes()) - lib = get_library(library_key) - if lib.type != COMPLEX: - # Problem and Video libraries only permit XBlocks of the same name. - block_types = (name for name in block_types if name == lib.type) + info = [] for block_type in block_types: # TODO: unify the contentstore helper with the xblock.api version of diff --git a/openedx/core/djangoapps/content_libraries/constants.py b/openedx/core/djangoapps/content_libraries/constants.py index 9505d52d1c..a01e0fbdda 100644 --- a/openedx/core/djangoapps/content_libraries/constants.py +++ b/openedx/core/djangoapps/content_libraries/constants.py @@ -2,16 +2,6 @@ from django.utils.translation import gettext_lazy as _ -VIDEO = 'video' -COMPLEX = 'complex' -PROBLEM = 'problem' - -LIBRARY_TYPES = ( - (VIDEO, _('Video')), - (COMPLEX, _('Complex')), - (PROBLEM, _('Problem')), -) - # These are all the licenses we support so far. ALL_RIGHTS_RESERVED = '' CC_4_BY = 'CC:4.0:BY' diff --git a/openedx/core/djangoapps/content_libraries/migrations/0011_remove_contentlibrary_bundle_uuid_and_more.py b/openedx/core/djangoapps/content_libraries/migrations/0011_remove_contentlibrary_bundle_uuid_and_more.py new file mode 100644 index 0000000000..0c634c6488 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/migrations/0011_remove_contentlibrary_bundle_uuid_and_more.py @@ -0,0 +1,21 @@ +# Generated by Django 4.2.16 on 2024-10-24 20:21 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('content_libraries', '0010_contentlibrary_learning_package_and_more'), + ] + + operations = [ + migrations.RemoveField( + model_name='contentlibrary', + name='bundle_uuid', + ), + migrations.RemoveField( + model_name='contentlibrary', + name='type', + ), + ] diff --git a/openedx/core/djangoapps/content_libraries/models.py b/openedx/core/djangoapps/content_libraries/models.py index c1dea39e06..4a210223cc 100644 --- a/openedx/core/djangoapps/content_libraries/models.py +++ b/openedx/core/djangoapps/content_libraries/models.py @@ -54,8 +54,7 @@ from pylti1p3.grade import Grade from opaque_keys.edx.django.models import UsageKeyField from openedx.core.djangoapps.content_libraries.constants import ( - LIBRARY_TYPES, COMPLEX, LICENSE_OPTIONS, - ALL_RIGHTS_RESERVED, + LICENSE_OPTIONS, ALL_RIGHTS_RESERVED, ) from openedx_learning.api.authoring_models import LearningPackage from organizations.models import Organization # lint-amnesty, pylint: disable=wrong-import-order @@ -101,18 +100,6 @@ class ContentLibrary(models.Model): org = models.ForeignKey(Organization, on_delete=models.PROTECT, null=False) slug = models.SlugField(allow_unicode=True) - # We no longer use the ``bundle_uuid`` and ``type`` fields, but we'll leave - # them in the model until after the Redwood release, just in case someone - # out there was using v2 libraries. We don't expect this, since it wasn't in - # a usable state, but there's always a chance someone managed to do it and - # is still using it. By keeping the schema backwards compatible, the thought - # is that they would update to the latest version, notice their libraries - # aren't working correctly, and still have the ability to recover their data - # if the code was rolled back. - # TODO: Remove these fields after the Redwood release is cut. - bundle_uuid = models.UUIDField(unique=True, null=True, default=None) - type = models.CharField(max_length=25, default=COMPLEX, choices=LIBRARY_TYPES) - license = models.CharField(max_length=25, default=ALL_RIGHTS_RESERVED, choices=LICENSE_OPTIONS) learning_package = models.OneToOneField( LearningPackage, diff --git a/openedx/core/djangoapps/content_libraries/serializers.py b/openedx/core/djangoapps/content_libraries/serializers.py index b19d27bed3..d639ed63af 100644 --- a/openedx/core/djangoapps/content_libraries/serializers.py +++ b/openedx/core/djangoapps/content_libraries/serializers.py @@ -11,8 +11,6 @@ from opaque_keys import InvalidKeyError from openedx_learning.api.authoring_models import Collection from openedx.core.djangoapps.content_libraries.constants import ( - LIBRARY_TYPES, - COMPLEX, ALL_RIGHTS_RESERVED, LICENSE_OPTIONS, ) @@ -37,10 +35,8 @@ class ContentLibraryMetadataSerializer(serializers.Serializer): # begins with 'lib:'. (The numeric ID of the ContentLibrary object in MySQL # is not exposed via this API.) id = serializers.CharField(source="key", read_only=True) - type = serializers.ChoiceField(choices=LIBRARY_TYPES, default=COMPLEX) org = serializers.SlugField(source="key.org") slug = serializers.CharField(source="key.slug", validators=(validate_unicode_slug, )) - bundle_uuid = serializers.UUIDField(format='hex_verbose', read_only=True) title = serializers.CharField() description = serializers.CharField(allow_blank=True) num_blocks = serializers.IntegerField(read_only=True) @@ -86,7 +82,6 @@ class ContentLibraryUpdateSerializer(serializers.Serializer): description = serializers.CharField() allow_public_learning = serializers.BooleanField() allow_public_read = serializers.BooleanField() - type = serializers.ChoiceField(choices=LIBRARY_TYPES) license = serializers.ChoiceField(choices=LICENSE_OPTIONS) @@ -118,7 +113,7 @@ class ContentLibraryPermissionSerializer(ContentLibraryPermissionLevelSerializer group_name = serializers.CharField(source="group.name", allow_null=True, allow_blank=False, default=None) -class BaseFilterSerializer(serializers.Serializer): +class ContentLibraryFilterSerializer(serializers.Serializer): """ Base serializer for filtering listings on the content library APIs. """ @@ -127,13 +122,6 @@ class BaseFilterSerializer(serializers.Serializer): order = serializers.CharField(default=None, required=False) -class ContentLibraryFilterSerializer(BaseFilterSerializer): - """ - Serializer for filtering library listings. - """ - type = serializers.ChoiceField(choices=LIBRARY_TYPES, default=None, required=False) - - class CollectionMetadataSerializer(serializers.Serializer): """ Serializer for CollectionMetadata diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 33d5477da6..7f1664e6c7 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -9,7 +9,7 @@ from organizations.models import Organization from rest_framework.test import APITransactionTestCase, APIClient from common.djangoapps.student.tests.factories import UserFactory -from openedx.core.djangoapps.content_libraries.constants import COMPLEX, ALL_RIGHTS_RESERVED +from openedx.core.djangoapps.content_libraries.constants import ALL_RIGHTS_RESERVED from openedx.core.djangolib.testing.utils import skip_unless_cms # Define the URLs here - don't use reverse() because we want to detect @@ -124,7 +124,7 @@ class ContentLibrariesRestApiTest(APITransactionTestCase): self.client = old_client # pylint: disable=attribute-defined-outside-init def _create_library( - self, slug, title, description="", org=None, library_type=COMPLEX, + self, slug, title, description="", org=None, license_type=ALL_RIGHTS_RESERVED, expect_response=200, ): """ Create a library """ @@ -135,7 +135,6 @@ class ContentLibrariesRestApiTest(APITransactionTestCase): "slug": slug, "title": title, "description": description, - "type": library_type, "license": license_type, }, expect_response) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 8977464dd4..b74d16b678 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -25,7 +25,7 @@ from organizations.models import Organization from rest_framework.test import APITestCase from common.djangoapps.student.tests.factories import UserFactory -from openedx.core.djangoapps.content_libraries.constants import CC_4_BY, COMPLEX, PROBLEM, VIDEO +from openedx.core.djangoapps.content_libraries.constants import CC_4_BY from openedx.core.djangoapps.content_libraries.tests.base import ( URL_BLOCK_GET_HANDLER_URL, URL_BLOCK_METADATA_URL, @@ -100,7 +100,6 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix "title": "A Tést Lꜟطrary", "description": "Just Téstꜟng", "version": 0, - "type": COMPLEX, "license": CC_4_BY, "has_unpublished_changes": False, "has_unpublished_deletes": False, @@ -199,13 +198,13 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix Test the filters in the list libraries API """ self._create_library( - slug="test-lib-filter-1", title="Fob", description="Bar", library_type=VIDEO, + slug="test-lib-filter-1", title="Fob", description="Bar", ) self._create_library( slug="test-lib-filter-2", title="Library-Title-2", description="Bar-2", ) self._create_library( - slug="l3", title="Library-Title-3", description="Description", library_type=VIDEO, + slug="l3", title="Library-Title-3", description="Description", ) Organization.objects.get_or_create( @@ -215,7 +214,6 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix self._create_library( slug="l4", title="Library-Title-4", description="Library-Description", org='org-test', - library_type=VIDEO, ) self._create_library( slug="l5", title="Library-Title-5", description="Library-Description", @@ -225,14 +223,11 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix assert len(self._list_libraries()) == 5 assert len(self._list_libraries({'org': 'org-test'})) == 2 assert len(self._list_libraries({'text_search': 'test-lib-filter'})) == 2 - assert len(self._list_libraries({'text_search': 'test-lib-filter', 'type': VIDEO})) == 1 assert len(self._list_libraries({'text_search': 'library-title'})) == 4 - assert len(self._list_libraries({'text_search': 'library-title', 'type': VIDEO})) == 2 assert len(self._list_libraries({'text_search': 'bar'})) == 2 assert len(self._list_libraries({'text_search': 'org-test'})) == 2 assert len(self._list_libraries({'org': 'org-test', 'text_search': 'library-title-4'})) == 1 - assert len(self._list_libraries({'type': VIDEO})) == 3 self.assertOrderEqual( self._list_libraries({'order': 'title'}), @@ -496,27 +491,6 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix assert len(self._get_library_blocks(lib['id'], {'block_type': 'problem'})['results']) == 3 assert len(self._get_library_blocks(lib['id'], {'block_type': 'squirrel'})['results']) == 0 - @ddt.data( - ('video-problem', VIDEO, 'problem', 400), - ('video-video', VIDEO, 'video', 200), - ('problem-problem', PROBLEM, 'problem', 200), - ('problem-video', PROBLEM, 'video', 400), - ('complex-video', COMPLEX, 'video', 200), - ('complex-problem', COMPLEX, 'problem', 200), - ) - @ddt.unpack - def test_library_blocks_type_constrained(self, slug, library_type, block_type, expect_response): - """ - Test that type-constrained libraries enforce their constraint when adding an XBlock. - """ - lib = self._create_library( - slug=slug, title="A Test Library", description="Testing XBlocks", library_type=library_type, - ) - lib_id = lib["id"] - - # Add a 'problem' XBlock to the library: - self._add_block_to_library(lib_id, block_type, 'test-block', expect_response=expect_response) - def test_library_not_found(self): """Test that requests fail with 404 when the library does not exist""" valid_not_found_key = 'lb:valid:key:video:1' @@ -755,24 +729,6 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix # Second block should throw error self._add_block_to_library(lib_id, "problem", "problem1", expect_response=400) - @ddt.data( - ('complex-types', COMPLEX, False), - ('video-types', VIDEO, True), - ('problem-types', PROBLEM, True), - ) - @ddt.unpack - def test_block_types(self, slug, library_type, constrained): - """ - Test that the permitted block types listing for a library change based on type. - """ - lib = self._create_library(slug=slug, title='Test Block Types', library_type=library_type) - types = self._get_library_block_types(lib['id']) - if constrained: - assert len(types) == 1 - assert types[0]['block_type'] == library_type - else: - assert len(types) > 1 - def test_content_library_create_event(self): """ Check that CONTENT_LIBRARY_CREATED event is sent when a content library is created. diff --git a/openedx/core/djangoapps/content_libraries/tests/test_models.py b/openedx/core/djangoapps/content_libraries/tests/test_models.py index 81a5a8fa32..eded47a305 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_models.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_models.py @@ -15,7 +15,6 @@ from organizations.models import Organization from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 from ..models import ALL_RIGHTS_RESERVED -from ..models import COMPLEX from ..models import ContentLibrary from ..models import LtiGradedResource from ..models import LtiProfile @@ -35,7 +34,6 @@ class ContentLibraryTest(TestCase): return ContentLibrary.objects.create( org=org, slug='foobar', - type=COMPLEX, allow_public_learning=False, allow_public_read=False, license=ALL_RIGHTS_RESERVED, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_runtime.py b/openedx/core/djangoapps/content_libraries/tests/test_runtime.py index f79808a7ec..3b505d3111 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_runtime.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_runtime.py @@ -21,7 +21,7 @@ from openedx.core.djangoapps.content_libraries.tests.base import ( URL_BLOCK_FIELDS_URL, ) from openedx.core.djangoapps.content_libraries.tests.user_state_block import UserStateTestBlock -from openedx.core.djangoapps.content_libraries.constants import COMPLEX, ALL_RIGHTS_RESERVED +from openedx.core.djangoapps.content_libraries.constants import ALL_RIGHTS_RESERVED from openedx.core.djangoapps.dark_lang.models import DarkLangConfig from openedx.core.djangoapps.xblock import api as xblock_api from openedx.core.djangolib.testing.utils import skip_unless_lms, skip_unless_cms @@ -50,7 +50,6 @@ class ContentLibraryContentTestMixin: _, slug = self.id().rsplit('.', 1) with transaction.atomic(): self.library = library_api.create_library( - library_type=COMPLEX, org=self.organization, slug=slugify(slug), title=(f"{slug} Test Lib"), diff --git a/openedx/core/djangoapps/content_libraries/tests/test_views_lti.py b/openedx/core/djangoapps/content_libraries/tests/test_views_lti.py index a25d02761d..b1ee61ca50 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_views_lti.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_views_lti.py @@ -5,8 +5,6 @@ Tests for LTI views. from django.conf import settings from django.test import TestCase, override_settings -from openedx.core.djangoapps.content_libraries.constants import PROBLEM - from .base import ( ContentLibrariesRestApiTest, URL_LIB_LTI_JWKS, @@ -60,10 +58,10 @@ class LibraryBlockLtiUrlViewTestMixin: """ library = self._create_library( - slug="libgg", title="A Test Library", description="Testing library", library_type=PROBLEM, + slug="libgg", title="A Test Library", description="Testing library", ) - block = self._add_block_to_library(library['id'], PROBLEM, PROBLEM) + block = self._add_block_to_library(library['id'], 'problem', 'problem') usage_key = str(block['id']) url = f'/api/libraries/v2/blocks/{usage_key}/lti/' diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py index 9357d54ca7..6325d9c4ae 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -227,14 +227,12 @@ class LibraryRootView(GenericAPIView): serializer = ContentLibraryFilterSerializer(data=request.query_params) serializer.is_valid(raise_exception=True) org = serializer.validated_data['org'] - library_type = serializer.validated_data['type'] text_search = serializer.validated_data['text_search'] order = serializer.validated_data['order'] queryset = api.get_libraries_for_user( request.user, org=org, - library_type=library_type, text_search=text_search, order=order, ) @@ -259,7 +257,6 @@ class LibraryRootView(GenericAPIView): data = dict(serializer.validated_data) # Converting this over because using the reserved names 'type' and 'license' would shadow the built-in # definitions elsewhere. - data['library_type'] = data.pop('type') data['library_license'] = data.pop('license') key_data = data.pop("key") # Move "slug" out of the "key.slug" pseudo-field that the serializer added: @@ -313,8 +310,6 @@ class LibraryDetailsView(APIView): serializer.is_valid(raise_exception=True) data = dict(serializer.validated_data) # Prevent ourselves from shadowing global names. - if 'type' in data: - data['library_type'] = data.pop('type') if 'license' in data: data['library_license'] = data.pop('license') try: From ebe3dc5e12d584d7140acd07933e7de1408f63ce Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Mon, 28 Oct 2024 15:47:08 +0500 Subject: [PATCH 05/22] fix: fixed course update notification UI in notification tray (#35715) --- cms/djangoapps/contentstore/tests/test_utils.py | 15 +++++++++++++++ cms/djangoapps/contentstore/utils.py | 2 ++ 2 files changed, 17 insertions(+) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 2c5be351fc..a913797970 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -974,3 +974,18 @@ class CourseUpdateNotificationTests(ModuleStoreTestCase): assert Notification.objects.all().count() == 1 notification = Notification.objects.first() assert notification.content == "

content Sub content heading

" + + def test_if_html_unescapes(self): + """ + Tests if html unescapes when creating content of course update notification + """ + user = UserFactory() + CourseEnrollment.enroll(user=user, course_key=self.course.id) + assert Notification.objects.all().count() == 0 + content = "

<p> &nbsp;</p>
"\ + "<p>abcd</p>
"\ + "<p>&nbsp;</p>

" + send_course_update_notification(self.course.id, content, self.user) + assert Notification.objects.all().count() == 1 + notification = Notification.objects.first() + assert notification.content == "

abcd

" diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 964f0c57e7..df1d1e27b4 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -3,6 +3,7 @@ Common utility functions useful throughout the contentstore """ from __future__ import annotations import configparser +import html import logging import re from collections import defaultdict @@ -2258,6 +2259,7 @@ def clean_html_body(html_body): """ Get html body, remove tags and limit to 500 characters """ + html_body = html.unescape(html_body).strip() html_body = BeautifulSoup(Truncator(html_body).chars(500, html=True), 'html.parser') text_content = html_body.get_text(separator=" ").strip() text_content = text_content.replace('\n', '').replace('\r', '') From e1f31fbb456a90c15219bba6320dbd9524330806 Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Mon, 28 Oct 2024 15:48:34 +0500 Subject: [PATCH 06/22] feat: removed setting email cadence to Never when using one click unsubscribe (#35708) --- .../notifications/email/tests/test_utils.py | 13 +++---------- .../core/djangoapps/notifications/email/utils.py | 6 ++---- .../djangoapps/notifications/tests/test_views.py | 2 -- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/openedx/core/djangoapps/notifications/email/tests/test_utils.py b/openedx/core/djangoapps/notifications/email/tests/test_utils.py index 6e79c497a4..1f3da983a0 100644 --- a/openedx/core/djangoapps/notifications/email/tests/test_utils.py +++ b/openedx/core/djangoapps/notifications/email/tests/test_utils.py @@ -16,7 +16,6 @@ from openedx.core.djangoapps.notifications.base_notification import ( COURSE_NOTIFICATION_TYPES, ) from openedx.core.djangoapps.notifications.config.waffle import ENABLE_EMAIL_NOTIFICATIONS -from openedx.core.djangoapps.notifications.email_notifications import EmailCadence from openedx.core.djangoapps.notifications.models import CourseNotificationPreference, Notification from openedx.core.djangoapps.notifications.email.utils import ( add_additional_attributes_to_notifications, @@ -320,9 +319,7 @@ class TestUpdatePreferenceFromPatch(ModuleStoreTestCase): if channel == param_channel: assert type_prefs[channel] == new_value if channel == 'email': - cadence_value = EmailCadence.NEVER - if new_value: - cadence_value = self.get_default_cadence_value(app_name, noti_type) + cadence_value = self.get_default_cadence_value(app_name, noti_type) assert type_prefs['email_cadence'] == cadence_value else: default_app_json = self.default_json[app_name] @@ -381,9 +378,7 @@ class TestUpdatePreferenceFromPatch(ModuleStoreTestCase): if app_name == param_app_name: assert type_prefs[channel] == new_value if channel == 'email': - cadence_value = EmailCadence.NEVER - if new_value: - cadence_value = self.get_default_cadence_value(app_name, noti_type) + cadence_value = self.get_default_cadence_value(app_name, noti_type) assert type_prefs['email_cadence'] == cadence_value else: default_app_json = self.default_json[app_name] @@ -415,9 +410,7 @@ class TestUpdatePreferenceFromPatch(ModuleStoreTestCase): if noti_type == param_notification_type: assert type_prefs[channel] == new_value if channel == 'email': - cadence_value = EmailCadence.NEVER - if new_value: - cadence_value = self.get_default_cadence_value(app_name, noti_type) + cadence_value = self.get_default_cadence_value(app_name, noti_type) assert type_prefs['email_cadence'] == cadence_value else: default_app_json = self.default_json[app_name] diff --git a/openedx/core/djangoapps/notifications/email/utils.py b/openedx/core/djangoapps/notifications/email/utils.py index 582e867d62..d855494012 100644 --- a/openedx/core/djangoapps/notifications/email/utils.py +++ b/openedx/core/djangoapps/notifications/email/utils.py @@ -406,9 +406,7 @@ def update_user_preferences_from_patch(encrypted_username, encrypted_patch): continue if is_editable(app_name, noti_type, channel): type_prefs[channel] = pref_value - if channel == 'email': - cadence_value = get_default_cadence_value(app_name, noti_type)\ - if pref_value else EmailCadence.NEVER - type_prefs['email_cadence'] = cadence_value + if channel == 'email' and pref_value and type_prefs.get('email_cadence') == EmailCadence.NEVER: + type_prefs['email_cadence'] = get_default_cadence_value(app_name, noti_type) preference.save() notification_preference_unsubscribe_event(user) diff --git a/openedx/core/djangoapps/notifications/tests/test_views.py b/openedx/core/djangoapps/notifications/tests/test_views.py index b7bd0414a2..70e6fbc573 100644 --- a/openedx/core/djangoapps/notifications/tests/test_views.py +++ b/openedx/core/djangoapps/notifications/tests/test_views.py @@ -27,7 +27,6 @@ from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_MODERATOR ) from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS -from openedx.core.djangoapps.notifications.email_notifications import EmailCadence from openedx.core.djangoapps.notifications.models import ( CourseNotificationPreference, Notification, @@ -936,7 +935,6 @@ class UpdatePreferenceFromEncryptedDataView(ModuleStoreTestCase): for app_name, app_prefs in config.items(): for type_prefs in app_prefs['notification_types'].values(): assert type_prefs['email'] is False - assert type_prefs['email_cadence'] == EmailCadence.NEVER def test_if_config_version_is_updated(self): """ From 2373dd02f9ab4edce560f182847c663122dacccf Mon Sep 17 00:00:00 2001 From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> Date: Mon, 28 Oct 2024 10:31:53 -0400 Subject: [PATCH 07/22] feat: update preview url to direct to mfe (#35687) * feat: update preview url to direct to mfe * fix: use url builder instead of string formatter * fix: url redirect for never published units --- .../rest_api/v1/views/tests/test_home.py | 4 +- .../rest_api/v2/views/tests/test_home.py | 18 +- cms/djangoapps/contentstore/utils.py | 27 +- lms/djangoapps/courseware/tests/test_views.py | 86 ++---- lms/djangoapps/courseware/views/index.py | 3 + lms/djangoapps/courseware/views/views.py | 268 +++++++++--------- .../features/course_experience/url_helpers.py | 41 ++- xmodule/modulestore/search.py | 73 ++--- .../tests/test_mixed_modulestore.py | 2 +- xmodule/video_block/video_block.py | 1 - 10 files changed, 273 insertions(+), 250 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index 69eee52437..73e3fe5ec3 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -117,7 +117,7 @@ class HomePageCoursesViewTest(CourseTestCase): "courses": [{ "course_key": course_id, "display_name": self.course.display_name, - "lms_link": f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}', + "lms_link": f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}', "number": self.course.number, "org": self.course.org, "rerun_link": f'/course_rerun/{course_id}', @@ -144,7 +144,7 @@ class HomePageCoursesViewTest(CourseTestCase): OrderedDict([ ("course_key", course_id), ("display_name", self.course.display_name), - ("lms_link", f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}'), + ("lms_link", f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}'), ("number", self.course.number), ("org", self.course.org), ("rerun_link", f'/course_rerun/{course_id}'), diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py index c0ffa50903..6905de254f 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py @@ -62,7 +62,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): OrderedDict([ ("course_key", course_id), ("display_name", self.course.display_name), - ("lms_link", f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}'), + ("lms_link", f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}'), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.course.id)}'), ("number", self.course.number), ("org", self.course.org), @@ -76,7 +76,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): ("display_name", self.archived_course.display_name), ( "lms_link", - f'//{settings.LMS_BASE}/courses/{archived_course_id}/jump_to/{self.archived_course.location}' + f'{settings.LMS_ROOT_URL}/courses/{archived_course_id}/jump_to/{self.archived_course.location}' ), ( "cms_link", @@ -139,7 +139,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): self.assertEqual(response.data["results"]["courses"], [OrderedDict([ ("course_key", str(self.course.id)), ("display_name", self.course.display_name), - ("lms_link", f'//{settings.LMS_BASE}/courses/{str(self.course.id)}/jump_to/{self.course.location}'), + ("lms_link", f'{settings.LMS_ROOT_URL}/courses/{str(self.course.id)}/jump_to/{self.course.location}'), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.course.id)}'), ("number", self.course.number), ("org", self.course.org), @@ -164,7 +164,11 @@ class HomePageCoursesViewV2Test(CourseTestCase): ("display_name", self.archived_course.display_name), ( "lms_link", - f'//{settings.LMS_BASE}/courses/{str(self.archived_course.id)}/jump_to/{self.archived_course.location}', + '{url_root}/courses/{course_id}/jump_to/{location}'.format( + url_root=settings.LMS_ROOT_URL, + course_id=str(self.archived_course.id), + location=self.archived_course.location + ), ), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}'), ("number", self.archived_course.number), @@ -190,7 +194,11 @@ class HomePageCoursesViewV2Test(CourseTestCase): ("display_name", self.archived_course.display_name), ( "lms_link", - f'//{settings.LMS_BASE}/courses/{str(self.archived_course.id)}/jump_to/{self.archived_course.location}', + '{url_root}/courses/{course_id}/jump_to/{location}'.format( + url_root=settings.LMS_ROOT_URL, + course_id=str(self.archived_course.id), + location=self.archived_course.location + ), ), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}'), ("number", self.archived_course.number), diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index df1d1e27b4..c0f656ec70 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -9,7 +9,7 @@ import re from collections import defaultdict from contextlib import contextmanager from datetime import datetime, timezone -from urllib.parse import quote_plus +from urllib.parse import quote_plus, urlencode, urlunparse, urlparse from uuid import uuid4 from bs4 import BeautifulSoup @@ -193,31 +193,30 @@ def get_lms_link_for_item(location, preview=False): """ assert isinstance(location, UsageKey) - # checks LMS_BASE value in site configuration for the given course_org_filter(org) - # if not found returns settings.LMS_BASE + # checks LMS_ROOT_URL value in site configuration for the given course_org_filter(org) + # if not found returns settings.LMS_ROOT_URL lms_base = SiteConfiguration.get_value_for_org( location.org, - "LMS_BASE", - settings.LMS_BASE + "LMS_ROOT_URL", + settings.LMS_ROOT_URL ) + query_string = '' if lms_base is None: return None if preview: - # checks PREVIEW_LMS_BASE value in site configuration for the given course_org_filter(org) - # if not found returns settings.FEATURES.get('PREVIEW_LMS_BASE') - lms_base = SiteConfiguration.get_value_for_org( - location.org, - "PREVIEW_LMS_BASE", - settings.FEATURES.get('PREVIEW_LMS_BASE') - ) + params = {'preview': '1'} + query_string = urlencode(params) - return "//{lms_base}/courses/{course_key}/jump_to/{location}".format( - lms_base=lms_base, + url_parts = list(urlparse(lms_base)) + url_parts[2] = '/courses/{course_key}/jump_to/{location}'.format( course_key=str(location.course_key), location=str(location), ) + url_parts[4] = query_string + + return urlunparse(url_parts) def get_lms_link_for_certificate_web_view(course_key, mode): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index cde0e8a34e..2ff1b8c256 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -132,38 +132,6 @@ class TestJumpTo(ModuleStoreTestCase): """ Check the jumpto link for a course. """ - @ddt.data( - (True, False), # preview -> Legacy experience - (False, True), # no preview -> MFE experience - ) - @ddt.unpack - def test_jump_to_legacy_vs_mfe(self, preview_mode, expect_mfe): - """ - Test that jump_to and jump_to_id correctly choose which courseware frontend to redirect to. - - Can be removed when the MFE supports a preview mode. - """ - course = CourseFactory.create() - chapter = BlockFactory.create(category='chapter', parent_location=course.location) - if expect_mfe: - expected_url = f'http://learning-mfe/course/{course.id}/{chapter.location}' - else: - expected_url = f'/courses/{course.id}/courseware/{chapter.url_name}/' - - jumpto_url = f'/courses/{course.id}/jump_to/{chapter.location}' - with set_preview_mode(preview_mode): - response = self.client.get(jumpto_url) - assert response.status_code == 302 - # Check the response URL, but chop off the querystring; we don't care here. - assert response.url.split('?')[0] == expected_url - - jumpto_id_url = f'/courses/{course.id}/jump_to_id/{chapter.url_name}' - with set_preview_mode(preview_mode): - response = self.client.get(jumpto_id_url) - assert response.status_code == 302 - # Check the response URL, but chop off the querystring; we don't care here. - assert response.url.split('?')[0] == expected_url - @ddt.data( (False, ModuleStoreEnum.Type.split), (True, ModuleStoreEnum.Type.split), @@ -174,32 +142,34 @@ class TestJumpTo(ModuleStoreTestCase): with self.store.default_store(store_type): course = CourseFactory.create() location = course.id.make_usage_key(None, 'NoSuchPlace') - expected_redirect_url = ( - f'/courses/{course.id}/courseware?' + urlencode({'activate_block_id': str(course.location)}) + + expected_redirect_url = f'http://learning-mfe/course/{course.id}' + jumpto_url = ( + f'/courses/{course.id}/jump_to/{location}?preview=1' ) if preview_mode else ( - f'http://learning-mfe/course/{course.id}' + f'/courses/{course.id}/jump_to/{location}' ) + # This is fragile, but unfortunately the problem is that within the LMS we # can't use the reverse calls from the CMS - jumpto_url = f'/courses/{course.id}/jump_to/{location}' - with set_preview_mode(preview_mode): + with set_preview_mode(False): response = self.client.get(jumpto_url) assert response.status_code == 302 assert response.url == expected_redirect_url - @set_preview_mode(True) - def test_jump_to_legacy_from_sequence(self): + @set_preview_mode(False) + def test_jump_to_preview_from_sequence(self): with self.store.default_store(ModuleStoreEnum.Type.split): course = CourseFactory.create() chapter = BlockFactory.create(category='chapter', parent_location=course.location) sequence = BlockFactory.create(category='sequential', parent_location=chapter.location) - activate_block_id = urlencode({'activate_block_id': str(sequence.location)}) + jumpto_url = f'/courses/{course.id}/jump_to/{sequence.location}?preview=1' expected_redirect_url = ( - f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/?{activate_block_id}' + f'http://learning-mfe/preview/course/{course.id}/{sequence.location}' ) - jumpto_url = f'/courses/{course.id}/jump_to/{sequence.location}' response = self.client.get(jumpto_url) - self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302) + assert response.status_code == 302 + assert response.url == expected_redirect_url @set_preview_mode(False) def test_jump_to_mfe_from_sequence(self): @@ -214,8 +184,8 @@ class TestJumpTo(ModuleStoreTestCase): assert response.status_code == 302 assert response.url == expected_redirect_url - @set_preview_mode(True) - def test_jump_to_legacy_from_block(self): + @set_preview_mode(False) + def test_jump_to_preview_from_block(self): with self.store.default_store(ModuleStoreEnum.Type.split): course = CourseFactory.create() chapter = BlockFactory.create(category='chapter', parent_location=course.location) @@ -225,21 +195,21 @@ class TestJumpTo(ModuleStoreTestCase): block1 = BlockFactory.create(category='html', parent_location=vertical1.location) block2 = BlockFactory.create(category='html', parent_location=vertical2.location) - activate_block_id = urlencode({'activate_block_id': str(block1.location)}) + jumpto_url = f'/courses/{course.id}/jump_to/{block1.location}?preview=1' expected_redirect_url = ( - f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/1?{activate_block_id}' + f'http://learning-mfe/preview/course/{course.id}/{sequence.location}/{vertical1.location}' ) - jumpto_url = f'/courses/{course.id}/jump_to/{block1.location}' response = self.client.get(jumpto_url) - self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302) + assert response.status_code == 302 + assert response.url == expected_redirect_url - activate_block_id = urlencode({'activate_block_id': str(block2.location)}) + jumpto_url = f'/courses/{course.id}/jump_to/{block2.location}?preview=1' expected_redirect_url = ( - f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/2?{activate_block_id}' + f'http://learning-mfe/preview/course/{course.id}/{sequence.location}/{vertical2.location}' ) - jumpto_url = f'/courses/{course.id}/jump_to/{block2.location}' response = self.client.get(jumpto_url) - self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302) + assert response.status_code == 302 + assert response.url == expected_redirect_url @set_preview_mode(False) def test_jump_to_mfe_from_block(self): @@ -300,8 +270,12 @@ class TestJumpTo(ModuleStoreTestCase): def test_jump_to_id_invalid_location(self, preview_mode, store_type): with self.store.default_store(store_type): course = CourseFactory.create() - jumpto_url = f'/courses/{course.id}/jump_to/NoSuchPlace' - with set_preview_mode(preview_mode): + jumpto_url = ( + f'/courses/{course.id}/jump_to/NoSuchPlace?preview=1' + ) if preview_mode else ( + f'/courses/{course.id}/jump_to/NoSuchPlace' + ) + with set_preview_mode(False): response = self.client.get(jumpto_url) assert response.status_code == 404 @@ -3359,7 +3333,7 @@ class PreviewTests(BaseViewsTestCase): def test_preview_no_redirect(self): __, __, preview_url = self._get_urls() with set_preview_mode(True): - # Previews will not redirect to the mfe + # Previews server from PREVIEW_LMS_BASE will not redirect to the mfe course_staff = UserFactory.create(is_staff=False) CourseStaffRole(self.course_key).add_users(course_staff) self.client.login(username=course_staff.username, password=TEST_PASSWORD) diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index c123c7f71f..7a9242f595 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -27,6 +27,7 @@ from web_fragments.fragment import Fragment from xmodule.course_block import COURSE_VISIBILITY_PUBLIC from xmodule.modulestore.django import modulestore from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW +from xmodule.util.xmodule_django import get_current_request_hostname from common.djangoapps.edxmako.shortcuts import render_to_response, render_to_string from common.djangoapps.student.models import CourseEnrollment @@ -188,11 +189,13 @@ class CoursewareIndex(View): unit_key = None except InvalidKeyError: unit_key = None + is_preview = settings.FEATURES.get('PREVIEW_LMS_BASE') == get_current_request_hostname() url = make_learning_mfe_courseware_url( self.course_key, self.section.location if self.section else None, unit_key, params=self.request.GET, + preview=is_preview, ) return url diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 1c57b23d9b..4e89dc2965 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -51,6 +51,7 @@ from xmodule.course_block import ( COURSE_VISIBILITY_PUBLIC_OUTLINE, CATALOG_VISIBILITY_CATALOG_AND_ABOUT, ) +from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem from xmodule.tabs import CourseTabList @@ -439,10 +440,12 @@ def jump_to(request, course_id, location): except InvalidKeyError as exc: raise Http404("Invalid course_key or usage_key") from exc + staff_access = has_access(request.user, 'staff', course_key) try: redirect_url = get_courseware_url( usage_key=usage_key, request=request, + is_staff=staff_access, ) except (ItemNotFoundError, NoPathToItem): # We used to 404 here, but that's ultimately a bad experience. There are real world use cases where a user @@ -452,6 +455,7 @@ def jump_to(request, course_id, location): redirect_url = get_courseware_url( usage_key=course_location_from_key(course_key), request=request, + is_staff=staff_access, ) return redirect(redirect_url) @@ -1565,143 +1569,151 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True, disable_sta f"Rendering of the xblock view '{nh3.clean(requested_view)}' is not supported." ) - staff_access = has_access(request.user, 'staff', course_key) + staff_access = bool(has_access(request.user, 'staff', course_key)) + is_preview = request.GET.get('preview') == '1' - with modulestore().bulk_operations(course_key): - # verify the user has access to the course, including enrollment check - try: - course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=check_if_enrolled) - except CourseAccessRedirect: - raise Http404("Course not found.") # lint-amnesty, pylint: disable=raise-missing-from + store = modulestore() + branchType = ModuleStoreEnum.Branch.draft_preferred if is_preview else ModuleStoreEnum.Branch.published_only - # with course access now verified: - # assume masquerading role, if applicable. - # (if we did this *before* the course access check, then course staff - # masquerading as learners would often be denied access, since course - # staff are generally not enrolled, and viewing a course generally - # requires enrollment.) - _course_masquerade, request.user = setup_masquerade( - request, - course_key, - staff_access, - ) + if is_preview and not staff_access: + return HttpResponseBadRequest("You do not have access to preview this xblock") - # Record user activity for tracking progress towards a user's course goals (for mobile app) - UserActivity.record_user_activity( - request.user, usage_key.course_key, request=request, only_if_mobile_app=True - ) + with store.bulk_operations(course_key): + with store.branch_setting(branchType, course_key): + # verify the user has access to the course, including enrollment check + try: + course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=check_if_enrolled) + except CourseAccessRedirect: + raise Http404("Course not found.") # lint-amnesty, pylint: disable=raise-missing-from - # get the block, which verifies whether the user has access to the block. - recheck_access = request.GET.get('recheck_access') == '1' - block, _ = get_block_by_usage_id( - request, - str(course_key), - str(usage_key), - disable_staff_debug_info=disable_staff_debug_info, - course=course, - will_recheck_access=recheck_access, - ) - - student_view_context = request.GET.dict() - student_view_context['show_bookmark_button'] = request.GET.get('show_bookmark_button', '0') == '1' - student_view_context['show_title'] = request.GET.get('show_title', '1') == '1' - - is_learning_mfe = is_request_from_learning_mfe(request) - # Right now, we only care about this in regards to the Learning MFE because it results - # in a bad UX if we display blocks with access errors (repeated upgrade messaging). - # If other use cases appear, consider removing the is_learning_mfe check or switching this - # to be its own query parameter that can toggle the behavior. - student_view_context['hide_access_error_blocks'] = is_learning_mfe and recheck_access - is_mobile_app = is_request_from_mobile_app(request) - student_view_context['is_mobile_app'] = is_mobile_app - - enable_completion_on_view_service = False - completion_service = block.runtime.service(block, 'completion') - if completion_service and completion_service.completion_tracking_enabled(): - if completion_service.blocks_to_mark_complete_on_view({block}): - enable_completion_on_view_service = True - student_view_context['wrap_xblock_data'] = { - 'mark-completed-on-view-after-delay': completion_service.get_complete_on_view_delay_ms() - } - - missed_deadlines, missed_gated_content = dates_banner_should_display(course_key, request.user) - - # Some content gating happens only at the Sequence level (e.g. "has this - # timed exam started?"). - ancestor_sequence_block = enclosing_sequence_for_gating_checks(block) - if ancestor_sequence_block: - context = {'specific_masquerade': is_masquerading_as_specific_student(request.user, course_key)} - # If the SequenceModule feels that gating is necessary, redirect - # there so we can have some kind of error message at any rate. - if ancestor_sequence_block.descendants_are_gated(context): - return redirect( - reverse( - 'render_xblock', - kwargs={'usage_key_string': str(ancestor_sequence_block.location)} - ) - ) - - # For courses using an LTI provider managed by edx-exams: - # Access to exam content is determined by edx-exams and passed to the LMS using a - # JWT url param. There is no longer a need for exam gating or logic inside the - # sequence block or its render call. descendants_are_gated shoule not return true - # for these timed exams. Instead, sequences are assumed gated by default and we look for - # an access token on the request to allow rendering to continue. - if course.proctoring_provider == 'lti_external': - seq_block = ancestor_sequence_block if ancestor_sequence_block else block - if getattr(seq_block, 'is_time_limited', None): - if not _check_sequence_exam_access(request, seq_block.location): - return HttpResponseForbidden("Access to exam content is restricted") - - context = { - 'course': course, - 'block': block, - 'disable_accordion': True, - 'allow_iframing': True, - 'disable_header': True, - 'disable_footer': True, - 'disable_window_wrap': True, - 'enable_completion_on_view_service': enable_completion_on_view_service, - 'edx_notes_enabled': is_feature_enabled(course, request.user), - 'staff_access': staff_access, - 'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://your_xqa_server.com'), - 'missed_deadlines': missed_deadlines, - 'missed_gated_content': missed_gated_content, - 'has_ended': course.has_ended(), - 'web_app_course_url': get_learning_mfe_home_url(course_key=course.id, url_fragment='home'), - 'on_courseware_page': True, - 'verified_upgrade_link': verified_upgrade_deadline_link(request.user, course=course), - 'is_learning_mfe': is_learning_mfe, - 'is_mobile_app': is_mobile_app, - 'render_course_wide_assets': True, - } - - try: - # .. filter_implemented_name: RenderXBlockStarted - # .. filter_type: org.openedx.learning.xblock.render.started.v1 - context, student_view_context = RenderXBlockStarted.run_filter( - context=context, student_view_context=student_view_context + # with course access now verified: + # assume masquerading role, if applicable. + # (if we did this *before* the course access check, then course staff + # masquerading as learners would often be denied access, since course + # staff are generally not enrolled, and viewing a course generally + # requires enrollment.) + _course_masquerade, request.user = setup_masquerade( + request, + course_key, + staff_access, ) - except RenderXBlockStarted.PreventXBlockBlockRender as exc: - log.info("Halted rendering block %s. Reason: %s", usage_key_string, exc.message) - return render_500(request) - except RenderXBlockStarted.RenderCustomResponse as exc: - log.info("Rendering custom exception for block %s. Reason: %s", usage_key_string, exc.message) + + # Record user activity for tracking progress towards a user's course goals (for mobile app) + UserActivity.record_user_activity( + request.user, usage_key.course_key, request=request, only_if_mobile_app=True + ) + + # get the block, which verifies whether the user has access to the block. + recheck_access = request.GET.get('recheck_access') == '1' + block, _ = get_block_by_usage_id( + request, + str(course_key), + str(usage_key), + disable_staff_debug_info=disable_staff_debug_info, + course=course, + will_recheck_access=recheck_access, + ) + + student_view_context = request.GET.dict() + student_view_context['show_bookmark_button'] = request.GET.get('show_bookmark_button', '0') == '1' + student_view_context['show_title'] = request.GET.get('show_title', '1') == '1' + + is_learning_mfe = is_request_from_learning_mfe(request) + # Right now, we only care about this in regards to the Learning MFE because it results + # in a bad UX if we display blocks with access errors (repeated upgrade messaging). + # If other use cases appear, consider removing the is_learning_mfe check or switching this + # to be its own query parameter that can toggle the behavior. + student_view_context['hide_access_error_blocks'] = is_learning_mfe and recheck_access + is_mobile_app = is_request_from_mobile_app(request) + student_view_context['is_mobile_app'] = is_mobile_app + + enable_completion_on_view_service = False + completion_service = block.runtime.service(block, 'completion') + if completion_service and completion_service.completion_tracking_enabled(): + if completion_service.blocks_to_mark_complete_on_view({block}): + enable_completion_on_view_service = True + student_view_context['wrap_xblock_data'] = { + 'mark-completed-on-view-after-delay': completion_service.get_complete_on_view_delay_ms() + } + + missed_deadlines, missed_gated_content = dates_banner_should_display(course_key, request.user) + + # Some content gating happens only at the Sequence level (e.g. "has this + # timed exam started?"). + ancestor_sequence_block = enclosing_sequence_for_gating_checks(block) + if ancestor_sequence_block: + context = {'specific_masquerade': is_masquerading_as_specific_student(request.user, course_key)} + # If the SequenceModule feels that gating is necessary, redirect + # there so we can have some kind of error message at any rate. + if ancestor_sequence_block.descendants_are_gated(context): + return redirect( + reverse( + 'render_xblock', + kwargs={'usage_key_string': str(ancestor_sequence_block.location)} + ) + ) + + # For courses using an LTI provider managed by edx-exams: + # Access to exam content is determined by edx-exams and passed to the LMS using a + # JWT url param. There is no longer a need for exam gating or logic inside the + # sequence block or its render call. descendants_are_gated shoule not return true + # for these timed exams. Instead, sequences are assumed gated by default and we look for + # an access token on the request to allow rendering to continue. + if course.proctoring_provider == 'lti_external': + seq_block = ancestor_sequence_block if ancestor_sequence_block else block + if getattr(seq_block, 'is_time_limited', None): + if not _check_sequence_exam_access(request, seq_block.location): + return HttpResponseForbidden("Access to exam content is restricted") + + context = { + 'course': course, + 'block': block, + 'disable_accordion': True, + 'allow_iframing': True, + 'disable_header': True, + 'disable_footer': True, + 'disable_window_wrap': True, + 'enable_completion_on_view_service': enable_completion_on_view_service, + 'edx_notes_enabled': is_feature_enabled(course, request.user), + 'staff_access': staff_access, + 'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://your_xqa_server.com'), + 'missed_deadlines': missed_deadlines, + 'missed_gated_content': missed_gated_content, + 'has_ended': course.has_ended(), + 'web_app_course_url': get_learning_mfe_home_url(course_key=course.id, url_fragment='home'), + 'on_courseware_page': True, + 'verified_upgrade_link': verified_upgrade_deadline_link(request.user, course=course), + 'is_learning_mfe': is_learning_mfe, + 'is_mobile_app': is_mobile_app, + 'render_course_wide_assets': True, + } + + try: + # .. filter_implemented_name: RenderXBlockStarted + # .. filter_type: org.openedx.learning.xblock.render.started.v1 + context, student_view_context = RenderXBlockStarted.run_filter( + context=context, student_view_context=student_view_context + ) + except RenderXBlockStarted.PreventXBlockBlockRender as exc: + log.info("Halted rendering block %s. Reason: %s", usage_key_string, exc.message) + return render_500(request) + except RenderXBlockStarted.RenderCustomResponse as exc: + log.info("Rendering custom exception for block %s. Reason: %s", usage_key_string, exc.message) + context.update({ + 'fragment': Fragment(exc.response) + }) + return render_to_response('courseware/courseware-chromeless.html', context, request=request) + + fragment = block.render(requested_view, context=student_view_context) + optimization_flags = get_optimization_flags_for_content(block, fragment) + context.update({ - 'fragment': Fragment(exc.response) + 'fragment': fragment, + **optimization_flags, }) + return render_to_response('courseware/courseware-chromeless.html', context, request=request) - fragment = block.render(requested_view, context=student_view_context) - optimization_flags = get_optimization_flags_for_content(block, fragment) - - context.update({ - 'fragment': fragment, - **optimization_flags, - }) - - return render_to_response('courseware/courseware-chromeless.html', context, request=request) - def get_optimization_flags_for_content(block, fragment): """ diff --git a/openedx/features/course_experience/url_helpers.py b/openedx/features/course_experience/url_helpers.py index f62e879f09..cdcb0203fc 100644 --- a/openedx/features/course_experience/url_helpers.py +++ b/openedx/features/course_experience/url_helpers.py @@ -12,9 +12,10 @@ from django.http import HttpRequest from django.http.request import QueryDict from django.urls import reverse from opaque_keys.edx.keys import CourseKey, UsageKey -from six.moves.urllib.parse import urlencode, urlparse +from six.moves.urllib.parse import urlencode, urlparse, urlunparse from lms.djangoapps.courseware.toggles import courseware_mfe_is_active +from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.search import navigation_index, path_to_location # lint-amnesty, pylint: disable=wrong-import-order @@ -24,6 +25,7 @@ User = get_user_model() def get_courseware_url( usage_key: UsageKey, request: Optional[HttpRequest] = None, + is_staff: bool = False, ) -> str: """ Return the URL to the canonical learning experience for a given block. @@ -44,12 +46,13 @@ def get_courseware_url( get_url_fn = _get_new_courseware_url else: get_url_fn = _get_legacy_courseware_url - return get_url_fn(usage_key=usage_key, request=request) + return get_url_fn(usage_key=usage_key, request=request, is_staff=is_staff) def _get_legacy_courseware_url( usage_key: UsageKey, request: Optional[HttpRequest] = None, + is_staff: bool = None ) -> str: """ Return the URL to Legacy (LMS-rendered) courseware content. @@ -90,6 +93,7 @@ def _get_legacy_courseware_url( def _get_new_courseware_url( usage_key: UsageKey, request: Optional[HttpRequest] = None, + is_staff: bool = None, ) -> str: """ Return the URL to the "new" (Learning Micro-Frontend) experience for a given block. @@ -99,7 +103,13 @@ def _get_new_courseware_url( * NoPathToItem if we cannot build a path to the `usage_key`. """ course_key = usage_key.course_key.replace(version_guid=None, branch=None) - path = path_to_location(modulestore(), usage_key, request, full_path=True) + preview = request.GET.get('preview') if request and request.GET else False + branch_type = ( + ModuleStoreEnum.Branch.draft_preferred + ) if preview and is_staff else ModuleStoreEnum.Branch.published_only + + path = path_to_location(modulestore(), usage_key, request, full_path=True, branch_type=branch_type) + if len(path) <= 1: # Course-run-level block: # We have no Sequence or Unit to return. @@ -120,6 +130,7 @@ def _get_new_courseware_url( course_key=course_key, sequence_key=sequence_key, unit_key=unit_key, + preview=preview, params=request.GET if request and request.GET else None, ) @@ -129,6 +140,7 @@ def make_learning_mfe_courseware_url( sequence_key: Optional[UsageKey] = None, unit_key: Optional[UsageKey] = None, params: Optional[QueryDict] = None, + preview: bool = None, ) -> str: """ Return a str with the URL for the specified courseware content in the Learning MFE. @@ -159,7 +171,18 @@ def make_learning_mfe_courseware_url( strings. They're only ever used to concatenate a URL string. `params` is an optional QueryDict object (e.g. request.GET) """ - mfe_link = f'{settings.LEARNING_MICROFRONTEND_URL}/course/{course_key}' + mfe_link = f'/course/{course_key}' + get_params = params.copy() if params else None + query_string = '' + + if preview: + if len(get_params.keys()) > 1: + get_params.pop('preview') + else: + get_params = None + + if (unit_key or sequence_key): + mfe_link = f'/preview/course/{course_key}' if sequence_key: mfe_link += f'/{sequence_key}' @@ -167,10 +190,14 @@ def make_learning_mfe_courseware_url( if unit_key: mfe_link += f'/{unit_key}' - if params: - mfe_link += f'?{params.urlencode()}' + if get_params: + query_string = get_params.urlencode() - return mfe_link + url_parts = list(urlparse(settings.LEARNING_MICROFRONTEND_URL)) + url_parts[2] = mfe_link + url_parts[4] = query_string + + return urlunparse(url_parts) def get_learning_mfe_home_url( diff --git a/xmodule/modulestore/search.py b/xmodule/modulestore/search.py index 6031703982..fc06507896 100644 --- a/xmodule/modulestore/search.py +++ b/xmodule/modulestore/search.py @@ -12,7 +12,7 @@ from .exceptions import ItemNotFoundError, NoPathToItem LOGGER = getLogger(__name__) -def path_to_location(modulestore, usage_key, request=None, full_path=False): +def path_to_location(modulestore, usage_key, request=None, full_path=False, branch_type=None): ''' Try to find a course_id/chapter/section[/position] path to location in modulestore. The courseware insists that the first level in the course is @@ -82,46 +82,47 @@ def path_to_location(modulestore, usage_key, request=None, full_path=False): queue.append((parent, newpath)) with modulestore.bulk_operations(usage_key.course_key): - if not modulestore.has_item(usage_key): - raise ItemNotFoundError(usage_key) + with modulestore.branch_setting(branch_type, usage_key.course_key): + if not modulestore.has_item(usage_key): + raise ItemNotFoundError(usage_key) - path = find_path_to_course() - if path is None: - raise NoPathToItem(usage_key) + path = find_path_to_course() + if path is None: + raise NoPathToItem(usage_key) - if full_path: - return path + if full_path: + return path - n = len(path) - course_id = path[0].course_key - # pull out the location names - chapter = path[1].block_id if n > 1 else None - section = path[2].block_id if n > 2 else None - vertical = path[3].block_id if n > 3 else None - # Figure out the position - position = None + n = len(path) + course_id = path[0].course_key + # pull out the location names + chapter = path[1].block_id if n > 1 else None + section = path[2].block_id if n > 2 else None + vertical = path[3].block_id if n > 3 else None + # Figure out the position + position = None - # This block of code will find the position of a block within a nested tree - # of blocks. If a problem is on tab 2 of a sequence that's on tab 3 of a - # sequence, the resulting position is 3_2. However, no positional blocks - # (e.g. sequential) currently deal with this form of representing nested - # positions. This needs to happen before jumping to a block nested in more - # than one positional block will work. + # This block of code will find the position of a block within a nested tree + # of blocks. If a problem is on tab 2 of a sequence that's on tab 3 of a + # sequence, the resulting position is 3_2. However, no positional blocks + # (e.g. sequential) currently deal with this form of representing nested + # positions. This needs to happen before jumping to a block nested in more + # than one positional block will work. - if n > 3: - position_list = [] - for path_index in range(2, n - 1): - category = path[path_index].block_type - if category == 'sequential': - section_desc = modulestore.get_item(path[path_index]) - # this calls get_children rather than just children b/c old mongo includes private children - # in children but not in get_children - child_locs = get_child_locations(section_desc, request, course_id) - # positions are 1-indexed, and should be strings to be consistent with - # url parsing. - if path[path_index + 1] in child_locs: - position_list.append(str(child_locs.index(path[path_index + 1]) + 1)) - position = "_".join(position_list) + if n > 3: + position_list = [] + for path_index in range(2, n - 1): + category = path[path_index].block_type + if category == 'sequential': + section_desc = modulestore.get_item(path[path_index]) + # this calls get_children rather than just children b/c old mongo includes private children + # in children but not in get_children + child_locs = get_child_locations(section_desc, request, course_id) + # positions are 1-indexed, and should be strings to be consistent with + # url parsing. + if path[path_index + 1] in child_locs: + position_list.append(str(child_locs.index(path[path_index + 1]) + 1)) + position = "_".join(position_list) return (course_id, chapter, section, vertical, position, path[-1]) diff --git a/xmodule/modulestore/tests/test_mixed_modulestore.py b/xmodule/modulestore/tests/test_mixed_modulestore.py index 8adbfcb911..0928ab253b 100644 --- a/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -1752,7 +1752,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): for location, expected in should_work: # each iteration has different find count, pop this iter's find count with check_mongo_calls(num_finds.pop(0), num_sends), self.assertNumQueries(num_mysql.pop(0)): - path = path_to_location(self.store, location) + path = path_to_location(self.store, location, branch_type=ModuleStoreEnum.Branch.published_only) assert path == expected not_found = ( diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index ea9d1a4428..12f0b2fb2a 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -179,7 +179,6 @@ class VideoBlock( track_url = self.track elif sub or other_lang: track_url = self.runtime.handler_url(self, 'transcript', 'download').rstrip('/?') - transcript_language = self.get_default_transcript_language(transcripts, dest_lang) native_languages = {lang: label for lang, label in settings.LANGUAGES if len(lang) == 2} languages = { From 1283fdde8ebc8a43165aa0aab8f36346bc618a62 Mon Sep 17 00:00:00 2001 From: Muhammad Farhan Khan Date: Mon, 28 Oct 2024 20:22:06 +0500 Subject: [PATCH 08/22] Dropping Sass support from builtin annotatable block (#35716) * feat!: Dropping Sass support from builtin annotatable block, replacing with vanilla CSS --- xmodule/annotatable_block.py | 6 +- xmodule/assets/AnnotatableBlockDisplay.scss | 3 - xmodule/assets/AnnotatableBlockEditor.scss | 3 - xmodule/assets/annotatable/_display.scss | 197 -------------- .../AnnotatableBlockDisplay.css | 243 ++++++++++++++++++ .../AnnotatableBlockEditor.css | 5 + 6 files changed, 251 insertions(+), 206 deletions(-) delete mode 100644 xmodule/assets/AnnotatableBlockDisplay.scss delete mode 100644 xmodule/assets/AnnotatableBlockEditor.scss delete mode 100644 xmodule/assets/annotatable/_display.scss create mode 100644 xmodule/static/css-builtin-blocks/AnnotatableBlockDisplay.css create mode 100644 xmodule/static/css-builtin-blocks/AnnotatableBlockEditor.css diff --git a/xmodule/annotatable_block.py b/xmodule/annotatable_block.py index e41e2b17a5..cec677f6c5 100644 --- a/xmodule/annotatable_block.py +++ b/xmodule/annotatable_block.py @@ -11,7 +11,7 @@ from xblock.fields import Scope, String from openedx.core.djangolib.markup import HTML, Text from xmodule.editing_block import EditingMixin from xmodule.raw_block import RawMixin -from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment +from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_css_to_fragment from xmodule.xml_block import XmlMixin from xmodule.x_module import ( ResourceTemplates, @@ -180,7 +180,7 @@ class AnnotatableBlock( """ fragment = Fragment() fragment.add_content(self.get_html()) - add_sass_to_fragment(fragment, 'AnnotatableBlockDisplay.scss') + add_css_to_fragment(fragment, 'AnnotatableBlockDisplay.css') add_webpack_js_to_fragment(fragment, 'AnnotatableBlockDisplay') shim_xmodule_js(fragment, 'Annotatable') @@ -193,7 +193,7 @@ class AnnotatableBlock( fragment = Fragment( self.runtime.service(self, 'mako').render_cms_template(self.mako_template, self.get_context()) ) - add_sass_to_fragment(fragment, 'AnnotatableBlockEditor.scss') + add_css_to_fragment(fragment, 'AnnotatableBlockEditor.css') add_webpack_js_to_fragment(fragment, 'AnnotatableBlockEditor') shim_xmodule_js(fragment, self.studio_js_module_name) return fragment diff --git a/xmodule/assets/AnnotatableBlockDisplay.scss b/xmodule/assets/AnnotatableBlockDisplay.scss deleted file mode 100644 index 66e1e756f3..0000000000 --- a/xmodule/assets/AnnotatableBlockDisplay.scss +++ /dev/null @@ -1,3 +0,0 @@ -.xmodule_display.xmodule_AnnotatableBlock { - @import "annotatable/display.scss"; -} diff --git a/xmodule/assets/AnnotatableBlockEditor.scss b/xmodule/assets/AnnotatableBlockEditor.scss deleted file mode 100644 index 8f2852422d..0000000000 --- a/xmodule/assets/AnnotatableBlockEditor.scss +++ /dev/null @@ -1,3 +0,0 @@ -.xmodule_edit.xmodule_AnnotatableBlock { - @import "codemirror/codemirror.scss"; -} diff --git a/xmodule/assets/annotatable/_display.scss b/xmodule/assets/annotatable/_display.scss deleted file mode 100644 index 9aaa8649c6..0000000000 --- a/xmodule/assets/annotatable/_display.scss +++ /dev/null @@ -1,197 +0,0 @@ -/* TODO: move top-level variables to a common _variables.scss. - * NOTE: These variables were only added here because when this was integrated with the CMS, - * SASS compilation errors were triggered because the CMS didn't have the same variables defined - * that the LMS did, so the quick fix was to localize the LMS variables not shared by the CMS. - * -Abarrett and Vshnayder - */ -@import 'bourbon/bourbon'; -@import 'lms/theme/variables'; -@import 'bootstrap/scss/variables'; -@import 'lms/theme/variables-v1'; - -$annotatable--border-color: var(--gray-l3); -$annotatable--body-font-size: em(14); - -.annotatable-wrapper { - position: relative; -} - -.annotatable-header { - margin-bottom: 0.5em; -} - -.annotatable-section { - position: relative; - padding: 0.5em 1em; - border: 1px solid $annotatable--border-color; - border-radius: 0.5em; - margin-bottom: 0.5em; - - &.shaded { background-color: #ededed; } - - .annotatable-section-title { - font-weight: bold; - a { font-weight: normal; } - } - - .annotatable-section-body { - border-top: 1px solid $annotatable--border-color; - margin-top: 0.5em; - padding-top: 0.5em; - - @include clearfix(); - } - - ul.instructions-template { - list-style: disc; - margin-left: 4em; - b { font-weight: bold; } - i { font-style: italic; } - - code { - display: inline; - white-space: pre; - font-family: Courier New, monospace; - } - } -} - -.annotatable-toggle { - position: absolute; - right: 0; - margin: 2px 1em 2px 0; - &.expanded::after { content: " \2191"; } - &.collapsed::after { content: " \2193"; } -} - -.annotatable-span { - @extend %ui-fake-link; - - display: inline; - - $highlight_index: 0; - - @each $highlight in ( - (yellow rgba(255, 255,10, 0.3) rgba(255, 255,10, 0.9)), - (red rgba(178,19,16,0.3) rgba(178,19,16,0.9)), - (orange rgba(255,165,0, 0.3) rgba(255,165,0, 0.9)), - (green rgba(25,255,132,0.3) rgba(25,255,132,0.9)), - (blue rgba(35,163,255,0.3) rgba(35,163,255,0.9)), - (purple rgba(115,9,178,0.3) rgba(115,9,178,0.9))) { - - $highlight_index: $highlight_index + 1; - $marker: nth($highlight, 1); - $color: nth($highlight, 2); - $selected_color: nth($highlight, 3); - - @if $highlight_index == 1 { - &.highlight { - background-color: $color; - &.selected { background-color: $selected_color; } - } - } - - &.highlight-#{$marker} { - background-color: $color; - &.selected { background-color: $selected_color; } - } - } - - &.hide { - cursor: none; - background-color: inherit; - - .annotatable-icon { - display: none; - } - } - - .annotatable-comment { - display: none; - } -} - -.ui-tooltip.qtip.ui-tooltip { - font-size: $annotatable--body-font-size; - border: 1px solid #333; - border-radius: 1em; - background-color: rgba(0, 0, 0, 0.85); - color: var(--white); - -webkit-font-smoothing: antialiased; - - .ui-tooltip-titlebar { - font-size: em(16); - color: inherit; - background-color: transparent; - padding: calc((var(--baseline)/4)) calc((var(--baseline)/2)); - border: none; - - .ui-tooltip-title { - padding: calc((var(--baseline)/4)) 0; - border-bottom: 2px solid #333; - font-weight: bold; - } - - .ui-tooltip-icon { - right: 10px; - background: #333; - } - - .ui-state-hover { - color: inherit; - border: 1px solid var(--gray-l3); - } - } - - .ui-tooltip-content { - color: inherit; - font-size: em(14); - text-align: left; - font-weight: 400; - padding: 0 calc((var(--baseline)/2)) calc((var(--baseline)/2)) calc((var(--baseline)/2)); - background-color: transparent; - border-color: transparent; - } - - p { - color: inherit; - line-height: normal; - } -} - -.ui-tooltip.qtip.ui-tooltip-annotatable { - max-width: 375px; - - .ui-tooltip-content { - padding: 0 calc((var(--baseline)/2)); - - .annotatable-comment { - display: block; - margin: 0 0 calc((var(--baseline)/2)) 0; - max-height: 225px; - overflow: auto; - line-height: normal; - } - - .annotatable-reply { - display: block; - border-top: 2px solid #333; - padding: calc((var(--baseline)/4)) 0; - margin: 0; - text-align: center; - } - } - - &::after { - content: ''; - display: inline-block; - position: absolute; - bottom: -20px; - left: 50%; - height: 0; - width: 0; - margin-left: calc(-1 * (var(--baseline) / 4)); - border: 10px solid transparent; - border-top-color: rgba(0, 0, 0, 0.85); - } -} diff --git a/xmodule/static/css-builtin-blocks/AnnotatableBlockDisplay.css b/xmodule/static/css-builtin-blocks/AnnotatableBlockDisplay.css new file mode 100644 index 0000000000..45b395ec66 --- /dev/null +++ b/xmodule/static/css-builtin-blocks/AnnotatableBlockDisplay.css @@ -0,0 +1,243 @@ +@import url("https://fonts.googleapis.com/css?family=Open+Sans:300,400,400i,600,700"); + +.xmodule_display.xmodule_AnnotatableBlock { + /* TODO: move top-level variables to a common _variables.scss. + * NOTE: These variables were only added here because when this was integrated with the CMS, + * SASS compilation errors were triggered because the CMS didn't have the same variables defined + * that the LMS did, so the quick fix was to localize the LMS variables not shared by the CMS. + * -Abarrett and Vshnayder + */ + /* stylelint-disable-line */ + /* stylelint-disable-line */ +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-wrapper { + position: relative; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-header { + margin-bottom: 0.5em; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-section { + position: relative; + padding: 0.5em 1em; + border: 1px solid var(--gray-l3); + border-radius: 0.5em; + margin-bottom: 0.5em; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-section.shaded { + background-color: #ededed; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-section .annotatable-section-title { + font-weight: bold; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-section .annotatable-section-title a { + font-weight: normal; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-section .annotatable-section-body { + border-top: 1px solid var(--gray-l3); + margin-top: 0.5em; + padding-top: 0.5em; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-section .annotatable-section-body:after { + content: ""; + display: table; + clear: both; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-section ul.instructions-template { + list-style: disc; + margin-left: 4em; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-section ul.instructions-template b { + font-weight: bold; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-section ul.instructions-template i { + font-style: italic; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-section ul.instructions-template code { + display: inline; + white-space: pre; + font-family: Courier New, monospace; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-toggle { + position: absolute; + right: 0; + margin: 2px 1em 2px 0; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-toggle.expanded::after { + content: " \2191"; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-toggle.collapsed::after { + content: " \2193"; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span { + display: inline; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight { + background-color: rgba(255, 255, 10, 0.3); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight.selected { + background-color: rgba(255, 255, 10, 0.9); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-yellow { + background-color: rgba(255, 255, 10, 0.3); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-yellow.selected { + background-color: rgba(255, 255, 10, 0.9); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-red { + background-color: rgba(178, 19, 16, 0.3); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-red.selected { + background-color: rgba(178, 19, 16, 0.9); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-orange { + background-color: rgba(255, 165, 0, 0.3); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-orange.selected { + background-color: rgba(255, 165, 0, 0.9); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-green { + background-color: rgba(25, 255, 132, 0.3); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-green.selected { + background-color: rgba(25, 255, 132, 0.9); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-blue { + background-color: rgba(35, 163, 255, 0.3); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-blue.selected { + background-color: rgba(35, 163, 255, 0.9); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-purple { + background-color: rgba(115, 9, 178, 0.3); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.highlight-purple.selected { + background-color: rgba(115, 9, 178, 0.9); +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.hide { + cursor: none; + background-color: inherit; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span.hide .annotatable-icon { + display: none; +} + +.xmodule_display.xmodule_AnnotatableBlock .annotatable-span .annotatable-comment { + display: none; +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip { + font-size: 0.875em; + border: 1px solid #333; + border-radius: 1em; + background-color: rgba(0, 0, 0, 0.85); + color: var(--white); + -webkit-font-smoothing: antialiased; +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip .ui-tooltip-titlebar { + font-size: 1em; + color: inherit; + background-color: transparent; + padding: calc((var(--baseline) / 4)) calc((var(--baseline) / 2)); + border: none; +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip .ui-tooltip-titlebar .ui-tooltip-title { + padding: calc((var(--baseline) / 4)) 0; + border-bottom: 2px solid #333; + font-weight: bold; +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip .ui-tooltip-titlebar .ui-tooltip-icon { + right: 10px; + background: #333; +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip .ui-tooltip-titlebar .ui-state-hover { + color: inherit; + border: 1px solid var(--gray-l3); +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip .ui-tooltip-content { + color: inherit; + font-size: 0.875em; + text-align: left; + font-weight: 400; + padding: 0 calc((var(--baseline) / 2)) calc((var(--baseline) / 2)) calc((var(--baseline) / 2)); + background-color: transparent; + border-color: transparent; +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip p { + color: inherit; + line-height: normal; +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip-annotatable { + max-width: 375px; +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip-annotatable .ui-tooltip-content { + padding: 0 calc((var(--baseline) / 2)); +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip-annotatable .ui-tooltip-content .annotatable-comment { + display: block; + margin: 0 0 calc((var(--baseline) / 2)) 0; + max-height: 225px; + overflow: auto; + line-height: normal; +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip-annotatable .ui-tooltip-content .annotatable-reply { + display: block; + border-top: 2px solid #333; + padding: calc((var(--baseline) / 4)) 0; + margin: 0; + text-align: center; +} + +.xmodule_display.xmodule_AnnotatableBlock .ui-tooltip.qtip.ui-tooltip-annotatable::after { + content: ''; + display: inline-block; + position: absolute; + bottom: -20px; + left: 50%; + height: 0; + width: 0; + margin-left: calc(-1 * (var(--baseline) / 4)); + border: 10px solid transparent; + border-top-color: rgba(0, 0, 0, 0.85); +} diff --git a/xmodule/static/css-builtin-blocks/AnnotatableBlockEditor.css b/xmodule/static/css-builtin-blocks/AnnotatableBlockEditor.css new file mode 100644 index 0000000000..498cbda9ff --- /dev/null +++ b/xmodule/static/css-builtin-blocks/AnnotatableBlockEditor.css @@ -0,0 +1,5 @@ +.xmodule_edit.xmodule_AnnotatableBlock .CodeMirror { + background: #fff; + font-size: 13px; + color: #3c3c3c; +} From 7fce06a11b225676d3abace20a8c11dad7e05d27 Mon Sep 17 00:00:00 2001 From: farhan Date: Thu, 24 Oct 2024 15:51:46 +0500 Subject: [PATCH 09/22] feat!: Dropping Sass support from builtin word cloud block, replacing with vanilla CSS --- xmodule/assets/WordCloudBlockDisplay.scss | 3 -- xmodule/assets/word_cloud/_display.scss | 30 ----------------- .../WordCloudBlockDisplay.css | 32 +++++++++++++++++++ xmodule/word_cloud_block.py | 4 +-- 4 files changed, 34 insertions(+), 35 deletions(-) delete mode 100644 xmodule/assets/WordCloudBlockDisplay.scss delete mode 100644 xmodule/assets/word_cloud/_display.scss create mode 100644 xmodule/static/css-builtin-blocks/WordCloudBlockDisplay.css diff --git a/xmodule/assets/WordCloudBlockDisplay.scss b/xmodule/assets/WordCloudBlockDisplay.scss deleted file mode 100644 index 884112a480..0000000000 --- a/xmodule/assets/WordCloudBlockDisplay.scss +++ /dev/null @@ -1,3 +0,0 @@ -.xmodule_display.xmodule_WordCloudBlock { - @import "word_cloud/display.scss"; -} diff --git a/xmodule/assets/word_cloud/_display.scss b/xmodule/assets/word_cloud/_display.scss deleted file mode 100644 index 4f7320380a..0000000000 --- a/xmodule/assets/word_cloud/_display.scss +++ /dev/null @@ -1,30 +0,0 @@ -@import 'bourbon/bourbon'; -@import 'lms/theme/variables'; -@import 'bootstrap/scss/variables'; -@import 'lms/theme/variables-v1'; - -.input-cloud { - margin: calc((var(--baseline)/4)); -} - -.result_cloud_section { - display: none; - width: 0px; - height: 0px; -} - -.result_cloud_section.active { - display: block; - width: 100%; - height: auto; - margin-top: 1em; - - h3 { - font-size: 100%; - } -} - -.your_words{ - font-size: 0.85em; - display: block; -} diff --git a/xmodule/static/css-builtin-blocks/WordCloudBlockDisplay.css b/xmodule/static/css-builtin-blocks/WordCloudBlockDisplay.css new file mode 100644 index 0000000000..51050dcba9 --- /dev/null +++ b/xmodule/static/css-builtin-blocks/WordCloudBlockDisplay.css @@ -0,0 +1,32 @@ +@import url("https://fonts.googleapis.com/css?family=Open+Sans:300,400,400i,600,700"); + +.xmodule_display.xmodule_WordCloudBlock { + /* stylelint-disable-line */ + /* stylelint-disable-line */ +} + +.xmodule_display.xmodule_WordCloudBlock .input-cloud { + margin: calc((var(--baseline) / 4)); +} + +.xmodule_display.xmodule_WordCloudBlock .result_cloud_section { + display: none; + width: 0px; + height: 0px; +} + +.xmodule_display.xmodule_WordCloudBlock .result_cloud_section.active { + display: block; + width: 100%; + height: auto; + margin-top: 1em; +} + +.xmodule_display.xmodule_WordCloudBlock .result_cloud_section.active h3 { + font-size: 100%; +} + +.xmodule_display.xmodule_WordCloudBlock .your_words { + font-size: 0.85em; + display: block; +} diff --git a/xmodule/word_cloud_block.py b/xmodule/word_cloud_block.py index 26a2c38c5c..d678f2a9a9 100644 --- a/xmodule/word_cloud_block.py +++ b/xmodule/word_cloud_block.py @@ -15,7 +15,7 @@ from xblock.core import XBlock from xblock.fields import Boolean, Dict, Integer, List, Scope, String from xmodule.editing_block import EditingMixin from xmodule.raw_block import EmptyDataRawMixin -from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment +from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_css_to_fragment from xmodule.xml_block import XmlMixin from xmodule.x_module import ( ResourceTemplates, @@ -262,7 +262,7 @@ class WordCloudBlock( # pylint: disable=abstract-method 'num_inputs': self.num_inputs, 'submitted': self.submitted, })) - add_sass_to_fragment(fragment, 'WordCloudBlockDisplay.scss') + add_css_to_fragment(fragment, 'WordCloudBlockDisplay.css') add_webpack_js_to_fragment(fragment, 'WordCloudBlockDisplay') shim_xmodule_js(fragment, 'WordCloud') From 0a2e8a914a93e0693e2d15e6ad98be550c8cd42b Mon Sep 17 00:00:00 2001 From: Fatima Sohail <68312464+sohailfatima@users.noreply.github.com> Date: Mon, 28 Oct 2024 22:10:09 +0500 Subject: [PATCH 10/22] feat: updated bulk email context for updated footer (#35702) * feat: updated bulk email context for updated footer * refactor: remove unnecessary f-string --- lms/djangoapps/bulk_email/tasks.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index dc11894953..197a7c0f13 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -537,6 +537,11 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas email_context['course_id'] = str(course_email.course_id) email_context['unsubscribe_link'] = get_unsubscribed_link(current_recipient['username'], str(course_email.course_id)) + email_context['unsubscribe_text'] = 'Unsubscribe from course updates for this course' + email_context['disclaimer'] = ( + "You are receiving this email because you are enrolled in the " + f"{email_context['platform_name']} course {email_context['course_title']}" + ) if is_bulk_email_edx_ace_enabled(): message = ACEEmail(site, email_context) From afd1394112a60227a3392ea40455de355c192dc2 Mon Sep 17 00:00:00 2001 From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> Date: Mon, 28 Oct 2024 13:26:29 -0400 Subject: [PATCH 11/22] Revert "feat: update preview url to direct to mfe (#35687)" (#35732) This reverts commit 2373dd02f9ab4edce560f182847c663122dacccf. --- .../rest_api/v1/views/tests/test_home.py | 4 +- .../rest_api/v2/views/tests/test_home.py | 18 +- cms/djangoapps/contentstore/utils.py | 27 +- lms/djangoapps/courseware/tests/test_views.py | 86 +++--- lms/djangoapps/courseware/views/index.py | 3 - lms/djangoapps/courseware/views/views.py | 252 +++++++++--------- .../features/course_experience/url_helpers.py | 41 +-- xmodule/modulestore/search.py | 73 +++-- .../tests/test_mixed_modulestore.py | 2 +- xmodule/video_block/video_block.py | 1 + 10 files changed, 242 insertions(+), 265 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index 73e3fe5ec3..69eee52437 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -117,7 +117,7 @@ class HomePageCoursesViewTest(CourseTestCase): "courses": [{ "course_key": course_id, "display_name": self.course.display_name, - "lms_link": f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}', + "lms_link": f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}', "number": self.course.number, "org": self.course.org, "rerun_link": f'/course_rerun/{course_id}', @@ -144,7 +144,7 @@ class HomePageCoursesViewTest(CourseTestCase): OrderedDict([ ("course_key", course_id), ("display_name", self.course.display_name), - ("lms_link", f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}'), + ("lms_link", f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}'), ("number", self.course.number), ("org", self.course.org), ("rerun_link", f'/course_rerun/{course_id}'), diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py index 6905de254f..c0ffa50903 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py @@ -62,7 +62,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): OrderedDict([ ("course_key", course_id), ("display_name", self.course.display_name), - ("lms_link", f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}'), + ("lms_link", f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}'), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.course.id)}'), ("number", self.course.number), ("org", self.course.org), @@ -76,7 +76,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): ("display_name", self.archived_course.display_name), ( "lms_link", - f'{settings.LMS_ROOT_URL}/courses/{archived_course_id}/jump_to/{self.archived_course.location}' + f'//{settings.LMS_BASE}/courses/{archived_course_id}/jump_to/{self.archived_course.location}' ), ( "cms_link", @@ -139,7 +139,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): self.assertEqual(response.data["results"]["courses"], [OrderedDict([ ("course_key", str(self.course.id)), ("display_name", self.course.display_name), - ("lms_link", f'{settings.LMS_ROOT_URL}/courses/{str(self.course.id)}/jump_to/{self.course.location}'), + ("lms_link", f'//{settings.LMS_BASE}/courses/{str(self.course.id)}/jump_to/{self.course.location}'), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.course.id)}'), ("number", self.course.number), ("org", self.course.org), @@ -164,11 +164,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): ("display_name", self.archived_course.display_name), ( "lms_link", - '{url_root}/courses/{course_id}/jump_to/{location}'.format( - url_root=settings.LMS_ROOT_URL, - course_id=str(self.archived_course.id), - location=self.archived_course.location - ), + f'//{settings.LMS_BASE}/courses/{str(self.archived_course.id)}/jump_to/{self.archived_course.location}', ), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}'), ("number", self.archived_course.number), @@ -194,11 +190,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): ("display_name", self.archived_course.display_name), ( "lms_link", - '{url_root}/courses/{course_id}/jump_to/{location}'.format( - url_root=settings.LMS_ROOT_URL, - course_id=str(self.archived_course.id), - location=self.archived_course.location - ), + f'//{settings.LMS_BASE}/courses/{str(self.archived_course.id)}/jump_to/{self.archived_course.location}', ), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}'), ("number", self.archived_course.number), diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index c0f656ec70..df1d1e27b4 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -9,7 +9,7 @@ import re from collections import defaultdict from contextlib import contextmanager from datetime import datetime, timezone -from urllib.parse import quote_plus, urlencode, urlunparse, urlparse +from urllib.parse import quote_plus from uuid import uuid4 from bs4 import BeautifulSoup @@ -193,30 +193,31 @@ def get_lms_link_for_item(location, preview=False): """ assert isinstance(location, UsageKey) - # checks LMS_ROOT_URL value in site configuration for the given course_org_filter(org) - # if not found returns settings.LMS_ROOT_URL + # checks LMS_BASE value in site configuration for the given course_org_filter(org) + # if not found returns settings.LMS_BASE lms_base = SiteConfiguration.get_value_for_org( location.org, - "LMS_ROOT_URL", - settings.LMS_ROOT_URL + "LMS_BASE", + settings.LMS_BASE ) - query_string = '' if lms_base is None: return None if preview: - params = {'preview': '1'} - query_string = urlencode(params) + # checks PREVIEW_LMS_BASE value in site configuration for the given course_org_filter(org) + # if not found returns settings.FEATURES.get('PREVIEW_LMS_BASE') + lms_base = SiteConfiguration.get_value_for_org( + location.org, + "PREVIEW_LMS_BASE", + settings.FEATURES.get('PREVIEW_LMS_BASE') + ) - url_parts = list(urlparse(lms_base)) - url_parts[2] = '/courses/{course_key}/jump_to/{location}'.format( + return "//{lms_base}/courses/{course_key}/jump_to/{location}".format( + lms_base=lms_base, course_key=str(location.course_key), location=str(location), ) - url_parts[4] = query_string - - return urlunparse(url_parts) def get_lms_link_for_certificate_web_view(course_key, mode): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 2ff1b8c256..cde0e8a34e 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -132,6 +132,38 @@ class TestJumpTo(ModuleStoreTestCase): """ Check the jumpto link for a course. """ + @ddt.data( + (True, False), # preview -> Legacy experience + (False, True), # no preview -> MFE experience + ) + @ddt.unpack + def test_jump_to_legacy_vs_mfe(self, preview_mode, expect_mfe): + """ + Test that jump_to and jump_to_id correctly choose which courseware frontend to redirect to. + + Can be removed when the MFE supports a preview mode. + """ + course = CourseFactory.create() + chapter = BlockFactory.create(category='chapter', parent_location=course.location) + if expect_mfe: + expected_url = f'http://learning-mfe/course/{course.id}/{chapter.location}' + else: + expected_url = f'/courses/{course.id}/courseware/{chapter.url_name}/' + + jumpto_url = f'/courses/{course.id}/jump_to/{chapter.location}' + with set_preview_mode(preview_mode): + response = self.client.get(jumpto_url) + assert response.status_code == 302 + # Check the response URL, but chop off the querystring; we don't care here. + assert response.url.split('?')[0] == expected_url + + jumpto_id_url = f'/courses/{course.id}/jump_to_id/{chapter.url_name}' + with set_preview_mode(preview_mode): + response = self.client.get(jumpto_id_url) + assert response.status_code == 302 + # Check the response URL, but chop off the querystring; we don't care here. + assert response.url.split('?')[0] == expected_url + @ddt.data( (False, ModuleStoreEnum.Type.split), (True, ModuleStoreEnum.Type.split), @@ -142,34 +174,32 @@ class TestJumpTo(ModuleStoreTestCase): with self.store.default_store(store_type): course = CourseFactory.create() location = course.id.make_usage_key(None, 'NoSuchPlace') - - expected_redirect_url = f'http://learning-mfe/course/{course.id}' - jumpto_url = ( - f'/courses/{course.id}/jump_to/{location}?preview=1' + expected_redirect_url = ( + f'/courses/{course.id}/courseware?' + urlencode({'activate_block_id': str(course.location)}) ) if preview_mode else ( - f'/courses/{course.id}/jump_to/{location}' + f'http://learning-mfe/course/{course.id}' ) - # This is fragile, but unfortunately the problem is that within the LMS we # can't use the reverse calls from the CMS - with set_preview_mode(False): + jumpto_url = f'/courses/{course.id}/jump_to/{location}' + with set_preview_mode(preview_mode): response = self.client.get(jumpto_url) assert response.status_code == 302 assert response.url == expected_redirect_url - @set_preview_mode(False) - def test_jump_to_preview_from_sequence(self): + @set_preview_mode(True) + def test_jump_to_legacy_from_sequence(self): with self.store.default_store(ModuleStoreEnum.Type.split): course = CourseFactory.create() chapter = BlockFactory.create(category='chapter', parent_location=course.location) sequence = BlockFactory.create(category='sequential', parent_location=chapter.location) - jumpto_url = f'/courses/{course.id}/jump_to/{sequence.location}?preview=1' + activate_block_id = urlencode({'activate_block_id': str(sequence.location)}) expected_redirect_url = ( - f'http://learning-mfe/preview/course/{course.id}/{sequence.location}' + f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/?{activate_block_id}' ) + jumpto_url = f'/courses/{course.id}/jump_to/{sequence.location}' response = self.client.get(jumpto_url) - assert response.status_code == 302 - assert response.url == expected_redirect_url + self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302) @set_preview_mode(False) def test_jump_to_mfe_from_sequence(self): @@ -184,8 +214,8 @@ class TestJumpTo(ModuleStoreTestCase): assert response.status_code == 302 assert response.url == expected_redirect_url - @set_preview_mode(False) - def test_jump_to_preview_from_block(self): + @set_preview_mode(True) + def test_jump_to_legacy_from_block(self): with self.store.default_store(ModuleStoreEnum.Type.split): course = CourseFactory.create() chapter = BlockFactory.create(category='chapter', parent_location=course.location) @@ -195,21 +225,21 @@ class TestJumpTo(ModuleStoreTestCase): block1 = BlockFactory.create(category='html', parent_location=vertical1.location) block2 = BlockFactory.create(category='html', parent_location=vertical2.location) - jumpto_url = f'/courses/{course.id}/jump_to/{block1.location}?preview=1' + activate_block_id = urlencode({'activate_block_id': str(block1.location)}) expected_redirect_url = ( - f'http://learning-mfe/preview/course/{course.id}/{sequence.location}/{vertical1.location}' + f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/1?{activate_block_id}' ) + jumpto_url = f'/courses/{course.id}/jump_to/{block1.location}' response = self.client.get(jumpto_url) - assert response.status_code == 302 - assert response.url == expected_redirect_url + self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302) - jumpto_url = f'/courses/{course.id}/jump_to/{block2.location}?preview=1' + activate_block_id = urlencode({'activate_block_id': str(block2.location)}) expected_redirect_url = ( - f'http://learning-mfe/preview/course/{course.id}/{sequence.location}/{vertical2.location}' + f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/2?{activate_block_id}' ) + jumpto_url = f'/courses/{course.id}/jump_to/{block2.location}' response = self.client.get(jumpto_url) - assert response.status_code == 302 - assert response.url == expected_redirect_url + self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302) @set_preview_mode(False) def test_jump_to_mfe_from_block(self): @@ -270,12 +300,8 @@ class TestJumpTo(ModuleStoreTestCase): def test_jump_to_id_invalid_location(self, preview_mode, store_type): with self.store.default_store(store_type): course = CourseFactory.create() - jumpto_url = ( - f'/courses/{course.id}/jump_to/NoSuchPlace?preview=1' - ) if preview_mode else ( - f'/courses/{course.id}/jump_to/NoSuchPlace' - ) - with set_preview_mode(False): + jumpto_url = f'/courses/{course.id}/jump_to/NoSuchPlace' + with set_preview_mode(preview_mode): response = self.client.get(jumpto_url) assert response.status_code == 404 @@ -3333,7 +3359,7 @@ class PreviewTests(BaseViewsTestCase): def test_preview_no_redirect(self): __, __, preview_url = self._get_urls() with set_preview_mode(True): - # Previews server from PREVIEW_LMS_BASE will not redirect to the mfe + # Previews will not redirect to the mfe course_staff = UserFactory.create(is_staff=False) CourseStaffRole(self.course_key).add_users(course_staff) self.client.login(username=course_staff.username, password=TEST_PASSWORD) diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 7a9242f595..c123c7f71f 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -27,7 +27,6 @@ from web_fragments.fragment import Fragment from xmodule.course_block import COURSE_VISIBILITY_PUBLIC from xmodule.modulestore.django import modulestore from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW -from xmodule.util.xmodule_django import get_current_request_hostname from common.djangoapps.edxmako.shortcuts import render_to_response, render_to_string from common.djangoapps.student.models import CourseEnrollment @@ -189,13 +188,11 @@ class CoursewareIndex(View): unit_key = None except InvalidKeyError: unit_key = None - is_preview = settings.FEATURES.get('PREVIEW_LMS_BASE') == get_current_request_hostname() url = make_learning_mfe_courseware_url( self.course_key, self.section.location if self.section else None, unit_key, params=self.request.GET, - preview=is_preview, ) return url diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 4e89dc2965..1c57b23d9b 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -51,7 +51,6 @@ from xmodule.course_block import ( COURSE_VISIBILITY_PUBLIC_OUTLINE, CATALOG_VISIBILITY_CATALOG_AND_ABOUT, ) -from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem from xmodule.tabs import CourseTabList @@ -440,12 +439,10 @@ def jump_to(request, course_id, location): except InvalidKeyError as exc: raise Http404("Invalid course_key or usage_key") from exc - staff_access = has_access(request.user, 'staff', course_key) try: redirect_url = get_courseware_url( usage_key=usage_key, request=request, - is_staff=staff_access, ) except (ItemNotFoundError, NoPathToItem): # We used to 404 here, but that's ultimately a bad experience. There are real world use cases where a user @@ -455,7 +452,6 @@ def jump_to(request, course_id, location): redirect_url = get_courseware_url( usage_key=course_location_from_key(course_key), request=request, - is_staff=staff_access, ) return redirect(redirect_url) @@ -1569,151 +1565,143 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True, disable_sta f"Rendering of the xblock view '{nh3.clean(requested_view)}' is not supported." ) - staff_access = bool(has_access(request.user, 'staff', course_key)) - is_preview = request.GET.get('preview') == '1' + staff_access = has_access(request.user, 'staff', course_key) - store = modulestore() - branchType = ModuleStoreEnum.Branch.draft_preferred if is_preview else ModuleStoreEnum.Branch.published_only + with modulestore().bulk_operations(course_key): + # verify the user has access to the course, including enrollment check + try: + course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=check_if_enrolled) + except CourseAccessRedirect: + raise Http404("Course not found.") # lint-amnesty, pylint: disable=raise-missing-from - if is_preview and not staff_access: - return HttpResponseBadRequest("You do not have access to preview this xblock") + # with course access now verified: + # assume masquerading role, if applicable. + # (if we did this *before* the course access check, then course staff + # masquerading as learners would often be denied access, since course + # staff are generally not enrolled, and viewing a course generally + # requires enrollment.) + _course_masquerade, request.user = setup_masquerade( + request, + course_key, + staff_access, + ) - with store.bulk_operations(course_key): - with store.branch_setting(branchType, course_key): - # verify the user has access to the course, including enrollment check - try: - course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=check_if_enrolled) - except CourseAccessRedirect: - raise Http404("Course not found.") # lint-amnesty, pylint: disable=raise-missing-from + # Record user activity for tracking progress towards a user's course goals (for mobile app) + UserActivity.record_user_activity( + request.user, usage_key.course_key, request=request, only_if_mobile_app=True + ) - # with course access now verified: - # assume masquerading role, if applicable. - # (if we did this *before* the course access check, then course staff - # masquerading as learners would often be denied access, since course - # staff are generally not enrolled, and viewing a course generally - # requires enrollment.) - _course_masquerade, request.user = setup_masquerade( - request, - course_key, - staff_access, - ) + # get the block, which verifies whether the user has access to the block. + recheck_access = request.GET.get('recheck_access') == '1' + block, _ = get_block_by_usage_id( + request, + str(course_key), + str(usage_key), + disable_staff_debug_info=disable_staff_debug_info, + course=course, + will_recheck_access=recheck_access, + ) - # Record user activity for tracking progress towards a user's course goals (for mobile app) - UserActivity.record_user_activity( - request.user, usage_key.course_key, request=request, only_if_mobile_app=True - ) + student_view_context = request.GET.dict() + student_view_context['show_bookmark_button'] = request.GET.get('show_bookmark_button', '0') == '1' + student_view_context['show_title'] = request.GET.get('show_title', '1') == '1' - # get the block, which verifies whether the user has access to the block. - recheck_access = request.GET.get('recheck_access') == '1' - block, _ = get_block_by_usage_id( - request, - str(course_key), - str(usage_key), - disable_staff_debug_info=disable_staff_debug_info, - course=course, - will_recheck_access=recheck_access, - ) + is_learning_mfe = is_request_from_learning_mfe(request) + # Right now, we only care about this in regards to the Learning MFE because it results + # in a bad UX if we display blocks with access errors (repeated upgrade messaging). + # If other use cases appear, consider removing the is_learning_mfe check or switching this + # to be its own query parameter that can toggle the behavior. + student_view_context['hide_access_error_blocks'] = is_learning_mfe and recheck_access + is_mobile_app = is_request_from_mobile_app(request) + student_view_context['is_mobile_app'] = is_mobile_app - student_view_context = request.GET.dict() - student_view_context['show_bookmark_button'] = request.GET.get('show_bookmark_button', '0') == '1' - student_view_context['show_title'] = request.GET.get('show_title', '1') == '1' + enable_completion_on_view_service = False + completion_service = block.runtime.service(block, 'completion') + if completion_service and completion_service.completion_tracking_enabled(): + if completion_service.blocks_to_mark_complete_on_view({block}): + enable_completion_on_view_service = True + student_view_context['wrap_xblock_data'] = { + 'mark-completed-on-view-after-delay': completion_service.get_complete_on_view_delay_ms() + } - is_learning_mfe = is_request_from_learning_mfe(request) - # Right now, we only care about this in regards to the Learning MFE because it results - # in a bad UX if we display blocks with access errors (repeated upgrade messaging). - # If other use cases appear, consider removing the is_learning_mfe check or switching this - # to be its own query parameter that can toggle the behavior. - student_view_context['hide_access_error_blocks'] = is_learning_mfe and recheck_access - is_mobile_app = is_request_from_mobile_app(request) - student_view_context['is_mobile_app'] = is_mobile_app + missed_deadlines, missed_gated_content = dates_banner_should_display(course_key, request.user) - enable_completion_on_view_service = False - completion_service = block.runtime.service(block, 'completion') - if completion_service and completion_service.completion_tracking_enabled(): - if completion_service.blocks_to_mark_complete_on_view({block}): - enable_completion_on_view_service = True - student_view_context['wrap_xblock_data'] = { - 'mark-completed-on-view-after-delay': completion_service.get_complete_on_view_delay_ms() - } - - missed_deadlines, missed_gated_content = dates_banner_should_display(course_key, request.user) - - # Some content gating happens only at the Sequence level (e.g. "has this - # timed exam started?"). - ancestor_sequence_block = enclosing_sequence_for_gating_checks(block) - if ancestor_sequence_block: - context = {'specific_masquerade': is_masquerading_as_specific_student(request.user, course_key)} - # If the SequenceModule feels that gating is necessary, redirect - # there so we can have some kind of error message at any rate. - if ancestor_sequence_block.descendants_are_gated(context): - return redirect( - reverse( - 'render_xblock', - kwargs={'usage_key_string': str(ancestor_sequence_block.location)} - ) + # Some content gating happens only at the Sequence level (e.g. "has this + # timed exam started?"). + ancestor_sequence_block = enclosing_sequence_for_gating_checks(block) + if ancestor_sequence_block: + context = {'specific_masquerade': is_masquerading_as_specific_student(request.user, course_key)} + # If the SequenceModule feels that gating is necessary, redirect + # there so we can have some kind of error message at any rate. + if ancestor_sequence_block.descendants_are_gated(context): + return redirect( + reverse( + 'render_xblock', + kwargs={'usage_key_string': str(ancestor_sequence_block.location)} ) - - # For courses using an LTI provider managed by edx-exams: - # Access to exam content is determined by edx-exams and passed to the LMS using a - # JWT url param. There is no longer a need for exam gating or logic inside the - # sequence block or its render call. descendants_are_gated shoule not return true - # for these timed exams. Instead, sequences are assumed gated by default and we look for - # an access token on the request to allow rendering to continue. - if course.proctoring_provider == 'lti_external': - seq_block = ancestor_sequence_block if ancestor_sequence_block else block - if getattr(seq_block, 'is_time_limited', None): - if not _check_sequence_exam_access(request, seq_block.location): - return HttpResponseForbidden("Access to exam content is restricted") - - context = { - 'course': course, - 'block': block, - 'disable_accordion': True, - 'allow_iframing': True, - 'disable_header': True, - 'disable_footer': True, - 'disable_window_wrap': True, - 'enable_completion_on_view_service': enable_completion_on_view_service, - 'edx_notes_enabled': is_feature_enabled(course, request.user), - 'staff_access': staff_access, - 'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://your_xqa_server.com'), - 'missed_deadlines': missed_deadlines, - 'missed_gated_content': missed_gated_content, - 'has_ended': course.has_ended(), - 'web_app_course_url': get_learning_mfe_home_url(course_key=course.id, url_fragment='home'), - 'on_courseware_page': True, - 'verified_upgrade_link': verified_upgrade_deadline_link(request.user, course=course), - 'is_learning_mfe': is_learning_mfe, - 'is_mobile_app': is_mobile_app, - 'render_course_wide_assets': True, - } - - try: - # .. filter_implemented_name: RenderXBlockStarted - # .. filter_type: org.openedx.learning.xblock.render.started.v1 - context, student_view_context = RenderXBlockStarted.run_filter( - context=context, student_view_context=student_view_context ) - except RenderXBlockStarted.PreventXBlockBlockRender as exc: - log.info("Halted rendering block %s. Reason: %s", usage_key_string, exc.message) - return render_500(request) - except RenderXBlockStarted.RenderCustomResponse as exc: - log.info("Rendering custom exception for block %s. Reason: %s", usage_key_string, exc.message) - context.update({ - 'fragment': Fragment(exc.response) - }) - return render_to_response('courseware/courseware-chromeless.html', context, request=request) - fragment = block.render(requested_view, context=student_view_context) - optimization_flags = get_optimization_flags_for_content(block, fragment) + # For courses using an LTI provider managed by edx-exams: + # Access to exam content is determined by edx-exams and passed to the LMS using a + # JWT url param. There is no longer a need for exam gating or logic inside the + # sequence block or its render call. descendants_are_gated shoule not return true + # for these timed exams. Instead, sequences are assumed gated by default and we look for + # an access token on the request to allow rendering to continue. + if course.proctoring_provider == 'lti_external': + seq_block = ancestor_sequence_block if ancestor_sequence_block else block + if getattr(seq_block, 'is_time_limited', None): + if not _check_sequence_exam_access(request, seq_block.location): + return HttpResponseForbidden("Access to exam content is restricted") + context = { + 'course': course, + 'block': block, + 'disable_accordion': True, + 'allow_iframing': True, + 'disable_header': True, + 'disable_footer': True, + 'disable_window_wrap': True, + 'enable_completion_on_view_service': enable_completion_on_view_service, + 'edx_notes_enabled': is_feature_enabled(course, request.user), + 'staff_access': staff_access, + 'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://your_xqa_server.com'), + 'missed_deadlines': missed_deadlines, + 'missed_gated_content': missed_gated_content, + 'has_ended': course.has_ended(), + 'web_app_course_url': get_learning_mfe_home_url(course_key=course.id, url_fragment='home'), + 'on_courseware_page': True, + 'verified_upgrade_link': verified_upgrade_deadline_link(request.user, course=course), + 'is_learning_mfe': is_learning_mfe, + 'is_mobile_app': is_mobile_app, + 'render_course_wide_assets': True, + } + + try: + # .. filter_implemented_name: RenderXBlockStarted + # .. filter_type: org.openedx.learning.xblock.render.started.v1 + context, student_view_context = RenderXBlockStarted.run_filter( + context=context, student_view_context=student_view_context + ) + except RenderXBlockStarted.PreventXBlockBlockRender as exc: + log.info("Halted rendering block %s. Reason: %s", usage_key_string, exc.message) + return render_500(request) + except RenderXBlockStarted.RenderCustomResponse as exc: + log.info("Rendering custom exception for block %s. Reason: %s", usage_key_string, exc.message) context.update({ - 'fragment': fragment, - **optimization_flags, + 'fragment': Fragment(exc.response) }) - return render_to_response('courseware/courseware-chromeless.html', context, request=request) + fragment = block.render(requested_view, context=student_view_context) + optimization_flags = get_optimization_flags_for_content(block, fragment) + + context.update({ + 'fragment': fragment, + **optimization_flags, + }) + + return render_to_response('courseware/courseware-chromeless.html', context, request=request) + def get_optimization_flags_for_content(block, fragment): """ diff --git a/openedx/features/course_experience/url_helpers.py b/openedx/features/course_experience/url_helpers.py index cdcb0203fc..f62e879f09 100644 --- a/openedx/features/course_experience/url_helpers.py +++ b/openedx/features/course_experience/url_helpers.py @@ -12,10 +12,9 @@ from django.http import HttpRequest from django.http.request import QueryDict from django.urls import reverse from opaque_keys.edx.keys import CourseKey, UsageKey -from six.moves.urllib.parse import urlencode, urlparse, urlunparse +from six.moves.urllib.parse import urlencode, urlparse from lms.djangoapps.courseware.toggles import courseware_mfe_is_active -from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.search import navigation_index, path_to_location # lint-amnesty, pylint: disable=wrong-import-order @@ -25,7 +24,6 @@ User = get_user_model() def get_courseware_url( usage_key: UsageKey, request: Optional[HttpRequest] = None, - is_staff: bool = False, ) -> str: """ Return the URL to the canonical learning experience for a given block. @@ -46,13 +44,12 @@ def get_courseware_url( get_url_fn = _get_new_courseware_url else: get_url_fn = _get_legacy_courseware_url - return get_url_fn(usage_key=usage_key, request=request, is_staff=is_staff) + return get_url_fn(usage_key=usage_key, request=request) def _get_legacy_courseware_url( usage_key: UsageKey, request: Optional[HttpRequest] = None, - is_staff: bool = None ) -> str: """ Return the URL to Legacy (LMS-rendered) courseware content. @@ -93,7 +90,6 @@ def _get_legacy_courseware_url( def _get_new_courseware_url( usage_key: UsageKey, request: Optional[HttpRequest] = None, - is_staff: bool = None, ) -> str: """ Return the URL to the "new" (Learning Micro-Frontend) experience for a given block. @@ -103,13 +99,7 @@ def _get_new_courseware_url( * NoPathToItem if we cannot build a path to the `usage_key`. """ course_key = usage_key.course_key.replace(version_guid=None, branch=None) - preview = request.GET.get('preview') if request and request.GET else False - branch_type = ( - ModuleStoreEnum.Branch.draft_preferred - ) if preview and is_staff else ModuleStoreEnum.Branch.published_only - - path = path_to_location(modulestore(), usage_key, request, full_path=True, branch_type=branch_type) - + path = path_to_location(modulestore(), usage_key, request, full_path=True) if len(path) <= 1: # Course-run-level block: # We have no Sequence or Unit to return. @@ -130,7 +120,6 @@ def _get_new_courseware_url( course_key=course_key, sequence_key=sequence_key, unit_key=unit_key, - preview=preview, params=request.GET if request and request.GET else None, ) @@ -140,7 +129,6 @@ def make_learning_mfe_courseware_url( sequence_key: Optional[UsageKey] = None, unit_key: Optional[UsageKey] = None, params: Optional[QueryDict] = None, - preview: bool = None, ) -> str: """ Return a str with the URL for the specified courseware content in the Learning MFE. @@ -171,18 +159,7 @@ def make_learning_mfe_courseware_url( strings. They're only ever used to concatenate a URL string. `params` is an optional QueryDict object (e.g. request.GET) """ - mfe_link = f'/course/{course_key}' - get_params = params.copy() if params else None - query_string = '' - - if preview: - if len(get_params.keys()) > 1: - get_params.pop('preview') - else: - get_params = None - - if (unit_key or sequence_key): - mfe_link = f'/preview/course/{course_key}' + mfe_link = f'{settings.LEARNING_MICROFRONTEND_URL}/course/{course_key}' if sequence_key: mfe_link += f'/{sequence_key}' @@ -190,14 +167,10 @@ def make_learning_mfe_courseware_url( if unit_key: mfe_link += f'/{unit_key}' - if get_params: - query_string = get_params.urlencode() + if params: + mfe_link += f'?{params.urlencode()}' - url_parts = list(urlparse(settings.LEARNING_MICROFRONTEND_URL)) - url_parts[2] = mfe_link - url_parts[4] = query_string - - return urlunparse(url_parts) + return mfe_link def get_learning_mfe_home_url( diff --git a/xmodule/modulestore/search.py b/xmodule/modulestore/search.py index fc06507896..6031703982 100644 --- a/xmodule/modulestore/search.py +++ b/xmodule/modulestore/search.py @@ -12,7 +12,7 @@ from .exceptions import ItemNotFoundError, NoPathToItem LOGGER = getLogger(__name__) -def path_to_location(modulestore, usage_key, request=None, full_path=False, branch_type=None): +def path_to_location(modulestore, usage_key, request=None, full_path=False): ''' Try to find a course_id/chapter/section[/position] path to location in modulestore. The courseware insists that the first level in the course is @@ -82,47 +82,46 @@ def path_to_location(modulestore, usage_key, request=None, full_path=False, bran queue.append((parent, newpath)) with modulestore.bulk_operations(usage_key.course_key): - with modulestore.branch_setting(branch_type, usage_key.course_key): - if not modulestore.has_item(usage_key): - raise ItemNotFoundError(usage_key) + if not modulestore.has_item(usage_key): + raise ItemNotFoundError(usage_key) - path = find_path_to_course() - if path is None: - raise NoPathToItem(usage_key) + path = find_path_to_course() + if path is None: + raise NoPathToItem(usage_key) - if full_path: - return path + if full_path: + return path - n = len(path) - course_id = path[0].course_key - # pull out the location names - chapter = path[1].block_id if n > 1 else None - section = path[2].block_id if n > 2 else None - vertical = path[3].block_id if n > 3 else None - # Figure out the position - position = None + n = len(path) + course_id = path[0].course_key + # pull out the location names + chapter = path[1].block_id if n > 1 else None + section = path[2].block_id if n > 2 else None + vertical = path[3].block_id if n > 3 else None + # Figure out the position + position = None - # This block of code will find the position of a block within a nested tree - # of blocks. If a problem is on tab 2 of a sequence that's on tab 3 of a - # sequence, the resulting position is 3_2. However, no positional blocks - # (e.g. sequential) currently deal with this form of representing nested - # positions. This needs to happen before jumping to a block nested in more - # than one positional block will work. + # This block of code will find the position of a block within a nested tree + # of blocks. If a problem is on tab 2 of a sequence that's on tab 3 of a + # sequence, the resulting position is 3_2. However, no positional blocks + # (e.g. sequential) currently deal with this form of representing nested + # positions. This needs to happen before jumping to a block nested in more + # than one positional block will work. - if n > 3: - position_list = [] - for path_index in range(2, n - 1): - category = path[path_index].block_type - if category == 'sequential': - section_desc = modulestore.get_item(path[path_index]) - # this calls get_children rather than just children b/c old mongo includes private children - # in children but not in get_children - child_locs = get_child_locations(section_desc, request, course_id) - # positions are 1-indexed, and should be strings to be consistent with - # url parsing. - if path[path_index + 1] in child_locs: - position_list.append(str(child_locs.index(path[path_index + 1]) + 1)) - position = "_".join(position_list) + if n > 3: + position_list = [] + for path_index in range(2, n - 1): + category = path[path_index].block_type + if category == 'sequential': + section_desc = modulestore.get_item(path[path_index]) + # this calls get_children rather than just children b/c old mongo includes private children + # in children but not in get_children + child_locs = get_child_locations(section_desc, request, course_id) + # positions are 1-indexed, and should be strings to be consistent with + # url parsing. + if path[path_index + 1] in child_locs: + position_list.append(str(child_locs.index(path[path_index + 1]) + 1)) + position = "_".join(position_list) return (course_id, chapter, section, vertical, position, path[-1]) diff --git a/xmodule/modulestore/tests/test_mixed_modulestore.py b/xmodule/modulestore/tests/test_mixed_modulestore.py index 0928ab253b..8adbfcb911 100644 --- a/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -1752,7 +1752,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): for location, expected in should_work: # each iteration has different find count, pop this iter's find count with check_mongo_calls(num_finds.pop(0), num_sends), self.assertNumQueries(num_mysql.pop(0)): - path = path_to_location(self.store, location, branch_type=ModuleStoreEnum.Branch.published_only) + path = path_to_location(self.store, location) assert path == expected not_found = ( diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 12f0b2fb2a..ea9d1a4428 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -179,6 +179,7 @@ class VideoBlock( track_url = self.track elif sub or other_lang: track_url = self.runtime.handler_url(self, 'transcript', 'download').rstrip('/?') + transcript_language = self.get_default_transcript_language(transcripts, dest_lang) native_languages = {lang: label for lang, label in settings.LANGUAGES if len(lang) == 2} languages = { From 949378f63fef3a03454508c91e6699765e2c6262 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 29 Oct 2024 00:04:53 +0530 Subject: [PATCH 12/22] fix: do not open MFE editors automatically on block paste (#35728) --- .../contentstore/xblock_storage_handlers/view_handlers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index df8d8dc625..3b9ba5798a 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -558,6 +558,7 @@ def _create_block(request): "locator": str(created_xblock.location), "courseKey": str(created_xblock.location.course_key), "static_file_notices": asdict(notices), + "upstreamRef": str(created_xblock.upstream), }) category = request.json["category"] From 97449ef54fb38c516a6fc4f4767bf8befd9b4e45 Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Mon, 28 Oct 2024 16:34:15 -0400 Subject: [PATCH 13/22] feat: add more authentication information to swagger (#35674) * feat: add more authentication information to swagger * updates the `docs-settings` to make the generated swagger `securityDefinitions` include both JWT and CSRF methods, as well as basic. A few linter fixes happened as a side effect. * Put in wordier descriptions for all three, since we don't have great shared documentation about authn/authz. * Added CSRF to `login_session`, which also serves as a proof of concept for other endpoits * Also regenerated the swagger doc, which picked up some extra changes. Generated swagger now has help and allows extra auth methods so some preveiously unusable endpoints can be hit. FIXES: APER-3554 --- docs/docs_settings.py | 75 +++- docs/lms-openapi.yaml | 244 +++++++++-- .../core/djangoapps/user_authn/views/login.py | 397 ++++++++++-------- 3 files changed, 491 insertions(+), 225 deletions(-) diff --git a/docs/docs_settings.py b/docs/docs_settings.py index f12848876e..f791b2faaf 100644 --- a/docs/docs_settings.py +++ b/docs/docs_settings.py @@ -4,7 +4,7 @@ Basically the LMS devstack settings plus a few items needed to successfully import all the Studio code. """ - +from textwrap import dedent import os from openedx.core.lib.derived import derive_settings @@ -27,18 +27,71 @@ for key, value in FEATURES.items(): FEATURES[key] = True # Settings that will fail if we enable them, and we don't need them for docs anyway. -FEATURES['RUN_AS_ANALYTICS_SERVER_ENABLED'] = False -FEATURES['ENABLE_SOFTWARE_SECURE_FAKE'] = False -FEATURES['ENABLE_MKTG_SITE'] = False +FEATURES["RUN_AS_ANALYTICS_SERVER_ENABLED"] = False +FEATURES["ENABLE_SOFTWARE_SECURE_FAKE"] = False +FEATURES["ENABLE_MKTG_SITE"] = False -INSTALLED_APPS.extend([ - 'cms.djangoapps.contentstore.apps.ContentstoreConfig', - 'cms.djangoapps.course_creators', - 'cms.djangoapps.xblock_config.apps.XBlockConfig', - 'lms.djangoapps.lti_provider', -]) +INSTALLED_APPS.extend( + [ + "cms.djangoapps.contentstore.apps.ContentstoreConfig", + "cms.djangoapps.course_creators", + "cms.djangoapps.xblock_config.apps.XBlockConfig", + "lms.djangoapps.lti_provider", + ] +) + +# Swagger generation details +openapi_security_info_basic = ( + "Obtain with a `POST` request to `/user/v1/account/login_session/`. " + "If needed, copy the cookies from the response to your new call." +) +openapi_security_info_jwt = dedent( + """ + Obtain by making a `POST` request to `/oauth2/v1/access_token`. + + You will need to be logged in and have a client ID and secret already created. + + Your request should have the headers + + ``` + 'Content-Type': 'application/x-www-form-urlencoded' + ``` + + Your request should have the data payload + + ``` + 'grant_type': 'client_credentials' + 'client_id': [your client ID] + 'client_secret': [your client secret] + 'token_type': 'jwt' + ``` + + Your JWT will be returned in the response as `access_token`. Prefix with `JWT ` in your header. + """ +) +openapi_security_info_csrf = ( + "Obtain by making a `GET` request to `/csrf/api/v1/token`. The token will be in the response cookie `csrftoken`." +) +SWAGGER_SETTINGS["SECURITY_DEFINITIONS"] = { + "Basic": { + "type": "basic", + "description": openapi_security_info_basic, + }, + "jwt": { + "type": "apiKey", + "name": "Authorization", + "in": "header", + "description": openapi_security_info_jwt, + }, + "csrf": { + "type": "apiKey", + "name": "X-CSRFToken", + "in": "header", + "description": openapi_security_info_csrf, + }, +} -COMMON_TEST_DATA_ROOT = '' +COMMON_TEST_DATA_ROOT = "" derive_settings(__name__) diff --git a/docs/lms-openapi.yaml b/docs/lms-openapi.yaml index 5e9afcc6d3..8011f84b45 100644 --- a/docs/lms-openapi.yaml +++ b/docs/lms-openapi.yaml @@ -13,8 +13,44 @@ produces: securityDefinitions: Basic: type: basic + description: Obtain with a `POST` request to `/user/v1/account/login_session/`. If + needed, copy the cookies from the response to your new call. + jwt: + type: apiKey + name: Authorization + in: header + description: |2 + + Obtain by making a `POST` request to `/oauth2/v1/access_token`. + + You will need to be logged in and have a client ID and secret already created. + + Your request should have the headers + + ``` + 'Content-Type': 'application/x-www-form-urlencoded' + ``` + + Your request should have the data payload + + ``` + 'grant_type': 'client_credentials' + 'client_id': [your client ID] + 'client_secret': [your client secret] + 'token_type': 'jwt' + ``` + + Your JWT will be returned in the response as `access_token`. Prefix with `JWT ` in your header. + csrf: + type: apiKey + name: X-CSRFToken + in: header + description: Obtain by making a `GET` request to `/csrf/api/v1/token`. The token + will be in the response cookie `csrftoken`. security: - Basic: [] +- csrf: [] +- jwt: [] paths: /agreements/v1/integrity_signature/{course_id}: get: @@ -3975,6 +4011,7 @@ paths: "profile_name": "Jon Doe" "verification_attempt_id": (Optional) "proctored_exam_attempt_id": (Optional) + "platform_verification_attempt_id": (Optional) "status": (Optional) } parameters: @@ -4130,6 +4167,7 @@ paths: "profile_name": "Jon Doe" "verification_attempt_id": (Optional) "proctored_exam_attempt_id": (Optional) + "platform_verification_attempt_id": (Optional) "status": (Optional) } parameters: @@ -6788,6 +6826,59 @@ paths: in: path required: true type: string + /mobile/{api_version}/notifications/create-token/: + post: + operationId: mobile_notifications_create-token_create + summary: |- + **Use Case** + This endpoint allows clients to register a device for push notifications. + description: |- + If the device is already registered, the existing registration will be updated. + If setting PUSH_NOTIFICATIONS_SETTINGS is not configured, the endpoint will return a 501 error. + + **Example Request** + POST /api/mobile/{version}/notifications/create-token/ + **POST Parameters** + The body of the POST request can include the following parameters. + * name (optional) - A name of the device. + * registration_id (required) - The device token of the device. + * device_id (optional) - ANDROID_ID / TelephonyManager.getDeviceId() (always as hex) + * active (optional) - Whether the device is active, default is True. + If False, the device will not receive notifications. + * cloud_message_type (required) - You should choose FCM or GCM. Currently, only FCM is supported. + * application_id (optional) - Opaque application identity, should be filled in for multiple + key/certificate access. Should be equal settings.FCM_APP_NAME. + **Example Response** + ```json + { + "id": 1, + "name": "My Device", + "registration_id": "fj3j4", + "device_id": 1234, + "active": true, + "date_created": "2024-04-18T07:39:37.132787Z", + "cloud_message_type": "FCM", + "application_id": "my_app_id" + } + ``` + parameters: + - name: data + in: body + required: true + schema: + $ref: '#/definitions/GCMDevice' + responses: + '201': + description: '' + schema: + $ref: '#/definitions/GCMDevice' + tags: + - mobile + parameters: + - name: api_version + in: path + required: true + type: string /mobile/{api_version}/users/{username}: get: operationId: mobile_users_read @@ -8849,22 +8940,6 @@ paths: tags: - user parameters: [] - /user/v1/accounts/verifications/{attempt_id}/: - get: - operationId: user_v1_accounts_verifications_read - description: Get IDV attempt details by attempt_id. Only accessible by global - staff. - parameters: [] - responses: - '200': - description: '' - tags: - - user - parameters: - - name: attempt_id - in: path - required: true - type: string /user/v1/accounts/{username}: get: operationId: user_v1_accounts_read @@ -9423,22 +9498,57 @@ paths: - user post: operationId: user_account_login_session_create - summary: Log in a user. - description: |- - See `login_user` for details. - - Example Usage: - - POST /api/user/v1/login_session - with POST params `email`, `password`. - - 200 {'success': true} - parameters: [] + summary: POST /user/{api_version}/account/login_session/ + description: Returns 200 on success, and a detailed error message otherwise. + parameters: + - name: data + in: body + required: true + schema: + type: object + properties: + email: + type: string + password: + type: string responses: - '201': + '200': description: '' + schema: + type: object + properties: + success: + type: boolean + value: + type: string + error_code: + type: string + '400': + description: '' + schema: + type: object + properties: + success: + type: boolean + value: + type: string + error_code: + type: string + '403': + description: '' + schema: + type: object + properties: + success: + type: boolean + value: + type: string + error_code: + type: string tags: - user + security: + - csrf: [] parameters: - name: api_version in: path @@ -10047,6 +10157,7 @@ definitions: required: - celebrations - course_access + - studio_access - course_id - is_enrolled - is_self_paced @@ -10084,6 +10195,9 @@ definitions: additionalProperties: type: string x-nullable: true + studio_access: + title: Studio access + type: boolean course_id: title: Course id type: string @@ -11237,10 +11351,24 @@ definitions: title: Verification attempt id type: integer x-nullable: true + verification_attempt_status: + title: Verification attempt status + type: string + minLength: 1 + x-nullable: true proctored_exam_attempt_id: title: Proctored exam attempt id type: integer x-nullable: true + platform_verification_attempt_id: + title: Platform verification attempt id + type: integer + x-nullable: true + platform_verification_attempt_status: + title: Platform verification attempt status + type: string + minLength: 1 + x-nullable: true status: title: Status type: string @@ -11277,10 +11405,24 @@ definitions: title: Verification attempt id type: integer x-nullable: true + verification_attempt_status: + title: Verification attempt status + type: string + minLength: 1 + x-nullable: true proctored_exam_attempt_id: title: Proctored exam attempt id type: integer x-nullable: true + platform_verification_attempt_id: + title: Platform verification attempt id + type: integer + x-nullable: true + platform_verification_attempt_status: + title: Platform verification attempt status + type: string + minLength: 1 + x-nullable: true status: title: Status type: string @@ -11710,6 +11852,52 @@ definitions: title: Enddatetime type: string format: date-time + GCMDevice: + required: + - registration_id + type: object + properties: + id: + title: ID + type: integer + name: + title: Name + type: string + maxLength: 255 + x-nullable: true + registration_id: + title: Registration ID + type: string + minLength: 1 + device_id: + title: Device id + description: 'ANDROID_ID / TelephonyManager.getDeviceId() (e.g: 0x01)' + type: integer + x-nullable: true + active: + title: Is active + description: Inactive devices will not be sent notifications + type: boolean + date_created: + title: Creation date + type: string + format: date-time + readOnly: true + x-nullable: true + cloud_message_type: + title: Cloud Message Type + description: You should choose FCM, GCM is deprecated + type: string + enum: + - FCM + - GCM + application_id: + title: Application ID + description: Opaque application identity, should be filled in for multiple + key/certificate access + type: string + maxLength: 64 + x-nullable: true mobile_api.User: required: - username diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 2ad3c0ff18..6c7390406b 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -23,11 +23,14 @@ from django.views.decorators.csrf import csrf_exempt, csrf_protect, ensure_csrf_ from django.views.decorators.debug import sensitive_post_parameters from django.views.decorators.http import require_http_methods from django_ratelimit.decorators import ratelimit +from drf_yasg import openapi +from drf_yasg.utils import swagger_auto_schema from edx_django_utils.monitoring import set_custom_attribute from eventtracking import tracker from openedx_events.learning.data import UserData, UserPersonalData from openedx_events.learning.signals import SESSION_LOGIN_COMPLETED from openedx_filters.learning.filters import StudentLoginRequested +from rest_framework import status from rest_framework.views import APIView from common.djangoapps import third_party_auth @@ -49,7 +52,7 @@ from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError, Vulne from openedx.core.djangoapps.user_authn.tasks import check_pwned_password_and_send_track_event from openedx.core.djangoapps.user_authn.toggles import ( is_require_third_party_auth_enabled, - should_redirect_to_authn_microfrontend + should_redirect_to_authn_microfrontend, ) from openedx.core.djangoapps.user_authn.views.login_form import get_login_session_form from openedx.core.djangoapps.user_authn.views.password_reset import send_password_reset_email_for_user @@ -62,7 +65,7 @@ from openedx.features.enterprise_support.api import activate_learner_enterprise, log = logging.getLogger("edx.student") AUDIT_LOG = logging.getLogger("audit") USER_MODEL = get_user_model() -PASSWORD_RESET_INITIATED = 'edx.user.passwordreset.initiated' +PASSWORD_RESET_INITIATED = "edx.user.passwordreset.initiated" def _do_third_party_auth(request): @@ -70,9 +73,9 @@ def _do_third_party_auth(request): User is already authenticated via 3rd party, now try to find and return their associated Django user. """ running_pipeline = pipeline.get(request) - username = running_pipeline['kwargs'].get('username') - backend_name = running_pipeline['backend'] - third_party_uid = running_pipeline['kwargs']['uid'] + username = running_pipeline["kwargs"].get("username") + backend_name = running_pipeline["backend"] + third_party_uid = running_pipeline["kwargs"]["uid"] requested_provider = provider.Registry.get_from_pipeline(running_pipeline) platform_name = configuration_helpers.get_value("platform_name", settings.PLATFORM_NAME) @@ -81,26 +84,25 @@ def _do_third_party_auth(request): except USER_MODEL.DoesNotExist: AUDIT_LOG.info( "Login failed - user with username {username} has no social auth " - "with backend_name {backend_name}".format( - username=username, backend_name=backend_name) + "with backend_name {backend_name}".format(username=username, backend_name=backend_name) ) - message = Text(_( - "You've successfully signed in to your {provider_name} account, " - "but this account isn't linked with your {platform_name} account yet. {blank_lines}" - "Use your {platform_name} username and password to sign in to {platform_name} below, " - "and then link your {platform_name} account with {provider_name} from your dashboard. {blank_lines}" - "If you don't have an account on {platform_name} yet, " - "click {register_label_strong} at the top of the page." - )).format( - blank_lines=HTML('

'), + message = Text( + _( + "You've successfully signed in to your {provider_name} account, " + "but this account isn't linked with your {platform_name} account yet. {blank_lines}" + "Use your {platform_name} username and password to sign in to {platform_name} below, " + "and then link your {platform_name} account with {provider_name} from your dashboard. {blank_lines}" + "If you don't have an account on {platform_name} yet, " + "click {register_label_strong} at the top of the page." + ) + ).format( + blank_lines=HTML("

"), platform_name=platform_name, provider_name=requested_provider.name, - register_label_strong=HTML('{register_text}').format( - register_text=_('Register') - ) + register_label_strong=HTML("{register_text}").format(register_text=_("Register")), ) - raise AuthFailedError(message, error_code='third-party-auth-with-no-linked-account') # lint-amnesty, pylint: disable=raise-missing-from + raise AuthFailedError(message, error_code="third-party-auth-with-no-linked-account") # lint-amnesty, pylint: disable=raise-missing-from def _get_user_by_email(email): @@ -128,14 +130,14 @@ def _get_user_by_email_or_username(request, api_version): Finds a user object in the database based on the given request, ignores all fields except for email and username. """ is_api_v2 = api_version != API_V1 - login_fields = ['email', 'password'] + login_fields = ["email", "password"] if is_api_v2: - login_fields = ['email_or_username', 'password'] + login_fields = ["email_or_username", "password"] if any(f not in request.POST.keys() for f in login_fields): - raise AuthFailedError(_('There was an error receiving your login information. Please email us.')) + raise AuthFailedError(_("There was an error receiving your login information. Please email us.")) - email_or_username = request.POST.get('email', None) or request.POST.get('email_or_username', None) + email_or_username = request.POST.get("email", None) or request.POST.get("email_or_username", None) user = _get_user_by_email(email_or_username) if not user and is_api_v2: @@ -143,7 +145,7 @@ def _get_user_by_email_or_username(request, api_version): user = _get_user_by_username(email_or_username) if not user: - digest = hashlib.shake_128(email_or_username.encode('utf-8')).hexdigest(16) + digest = hashlib.shake_128(email_or_username.encode("utf-8")).hexdigest(16) AUDIT_LOG.warning(f"Login failed - Unknown user email or username {digest}") return user @@ -165,27 +167,30 @@ def _generate_locked_out_error_message(): """ locked_out_period_in_sec = settings.MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS - error_message = Text(_('To protect your account, it’s been temporarily ' - 'locked. Try again in {locked_out_period} minutes.' - '{li_start}To be on the safe side, you can reset your ' - 'password {link_start}here{link_end} before you try again.')).format( - link_start=HTML(''), - link_end=HTML(''), - li_start=HTML('
  • '), - li_end=HTML('
    • '), - locked_out_period=int(locked_out_period_in_sec / 60)) + error_message = Text( + _( + "To protect your account, it’s been temporarily " + "locked. Try again in {locked_out_period} minutes." + "{li_start}To be on the safe side, you can reset your " + "password {link_start}here{link_end} before you try again." + ) + ).format( + link_start=HTML(''), + link_end=HTML(""), + li_start=HTML("
  • "), + li_end=HTML("
    • "), + locked_out_period=int(locked_out_period_in_sec / 60), + ) raise AuthFailedError( error_message, - error_code='account-locked-out', - context={ - 'locked_out_period': int(locked_out_period_in_sec / 60) - } + error_code="account-locked-out", + context={"locked_out_period": int(locked_out_period_in_sec / 60)}, ) def _enforce_password_policy_compliance(request, user): # lint-amnesty, pylint: disable=missing-function-docstring try: - password_policy_compliance.enforce_compliance_on_login(user, request.POST.get('password')) + password_policy_compliance.enforce_compliance_on_login(user, request.POST.get("password")) except password_policy_compliance.NonCompliantPasswordWarning as e: # Allow login, but warn the user that they will be required to reset their password soon. PageLevelMessages.register_warning_message(request, HTML(str(e))) @@ -201,7 +206,7 @@ def _enforce_password_policy_compliance(request, user): # lint-amnesty, pylint: { "user_id": user.id, "source": "Policy Compliance", - } + }, ) send_password_reset_email_for_user(user, request) @@ -214,19 +219,17 @@ def _log_and_raise_inactive_user_auth_error(unauthenticated_user): Depending on Django version we can get here a couple of ways, but this takes care of logging an auth attempt by an inactive user, re-sending the activation email, and raising an error with the correct message. """ - AUDIT_LOG.warning( - f"Login failed - Account not active for user.id: {unauthenticated_user.id}, resending activation" - ) + AUDIT_LOG.warning(f"Login failed - Account not active for user.id: {unauthenticated_user.id}, resending activation") profile = UserProfile.objects.get(user=unauthenticated_user) compose_and_send_activation_email(unauthenticated_user, profile) raise AuthFailedError( - error_code='inactive-user', + error_code="inactive-user", context={ - 'platformName': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), - 'supportLink': configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK) - } + "platformName": configuration_helpers.get_value("PLATFORM_NAME", settings.PLATFORM_NAME), + "supportLink": configuration_helpers.get_value("SUPPORT_SITE_LINK", settings.SUPPORT_SITE_LINK), + }, ) @@ -234,9 +237,11 @@ def _authenticate_first_party(request, unauthenticated_user, third_party_auth_re """ Use Django authentication on the given request, using rate limiting if configured """ - should_be_rate_limited = getattr(request, 'limited', False) + should_be_rate_limited = getattr(request, "limited", False) if should_be_rate_limited: - raise AuthFailedError(_('Too many failed login attempts. Try again later.')) # lint-amnesty, pylint: disable=raise-missing-from + raise AuthFailedError( + _("Too many failed login attempts. Try again later.") + ) # lint-amnesty, pylint: disable=raise-missing-from # If the user doesn't exist, we want to set the username to an invalid username so that authentication is guaranteed # to fail and we can take advantage of the ratelimited backend @@ -248,12 +253,8 @@ def _authenticate_first_party(request, unauthenticated_user, third_party_auth_re if not third_party_auth_requested: _check_user_auth_flow(request.site, unauthenticated_user) - password = normalize_password(request.POST['password']) - return authenticate( - username=username, - password=password, - request=request - ) + password = normalize_password(request.POST["password"]) + return authenticate(username=username, password=password, request=request) def _handle_failed_authentication(user, authenticated_user): @@ -279,34 +280,37 @@ def _handle_failed_authentication(user, authenticated_user): if not LoginFailures.is_user_locked_out(user): max_failures_allowed = settings.MAX_FAILED_LOGIN_ATTEMPTS_ALLOWED remaining_attempts = max_failures_allowed - failure_count - error_message = Text(_('Email or password is incorrect.' - '{li_start}You have {remaining_attempts} more sign-in ' - 'attempts before your account is temporarily locked.{li_end}' - '{li_start}If you\'ve forgotten your password, click ' - '{link_start}here{link_end} to reset.{li_end}')).format( - link_start=HTML( - '' - ), - link_end=HTML(''), - li_start=HTML('
  • '), - li_end=HTML('
    • '), - remaining_attempts=remaining_attempts) + error_message = Text( + _( + "Email or password is incorrect." + "{li_start}You have {remaining_attempts} more sign-in " + "attempts before your account is temporarily locked.{li_end}" + "{li_start}If you've forgotten your password, click " + "{link_start}here{link_end} to reset.{li_end}" + ) + ).format( + link_start=HTML(''), + link_end=HTML(""), + li_start=HTML("
  • "), + li_end=HTML("
    • "), + remaining_attempts=remaining_attempts, + ) raise AuthFailedError( error_message, - error_code='failed-login-attempt', + error_code="failed-login-attempt", context={ - 'remaining_attempts': remaining_attempts, - 'allowed_failure_attempts': max_failures_allowed, - 'failure_count': failure_count, - } + "remaining_attempts": remaining_attempts, + "allowed_failure_attempts": max_failures_allowed, + "failure_count": failure_count, + }, ) _generate_locked_out_error_message() raise AuthFailedError( - _('Email or password is incorrect.'), - error_code='incorrect-email-or-password', - context={'failure_count': failure_count}, + _("Email or password is incorrect."), + error_code="incorrect-email-or-password", + context={"failure_count": failure_count}, ) @@ -352,25 +356,18 @@ def _track_user_login(user, request): # .. pii_retirement: third_party segment.identify( user.id, - { - 'email': user.email, - 'username': user.username - }, + {"email": user.email, "username": user.username}, { # Disable MailChimp because we don't want to update the user's email # and username in MailChimp on every page load. We only need to capture # this data on registration/activation. - 'MailChimp': False - } + "MailChimp": False + }, ) segment.track( user.id, "edx.bi.user.account.authenticated", - { - 'category': "conversion", - 'label': request.POST.get('course_id'), - 'provider': None - }, + {"category": "conversion", "label": request.POST.get("course_id"), "provider": None}, ) @@ -380,20 +377,22 @@ def _create_message(site, root_url, allowed_domain): to an allowed domain and not whitelisted then ask such users to login through allowed domain SSO provider. """ - msg = Text(_( - 'As {allowed_domain} user, You must login with your {allowed_domain} ' - '{link_start}{provider} account{link_end}.' - )).format( + msg = Text( + _( + "As {allowed_domain} user, You must login with your {allowed_domain} " + "{link_start}{provider} account{link_end}." + ) + ).format( allowed_domain=allowed_domain, link_start=HTML("").format( - root_url=root_url if root_url else '', - tpa_provider_link='{dashboard_url}?tpa_hint={tpa_hint}'.format( - dashboard_url=reverse('dashboard'), - tpa_hint=site.configuration.get_value('THIRD_PARTY_AUTH_ONLY_HINT'), - ) + root_url=root_url if root_url else "", + tpa_provider_link="{dashboard_url}?tpa_hint={tpa_hint}".format( + dashboard_url=reverse("dashboard"), + tpa_hint=site.configuration.get_value("THIRD_PARTY_AUTH_ONLY_HINT"), + ), ), - provider=site.configuration.get_value('THIRD_PARTY_AUTH_ONLY_PROVIDER'), - link_end=HTML("") + provider=site.configuration.get_value("THIRD_PARTY_AUTH_ONLY_PROVIDER"), + link_end=HTML(""), ) return msg @@ -404,13 +403,13 @@ def _check_user_auth_flow(site, user): then ask user to login through allowed domain SSO provider. """ if user and ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY.is_enabled(): - allowed_domain = site.configuration.get_value('THIRD_PARTY_AUTH_ONLY_DOMAIN', '').lower() - email_parts = user.email.split('@') + allowed_domain = site.configuration.get_value("THIRD_PARTY_AUTH_ONLY_DOMAIN", "").lower() + email_parts = user.email.split("@") if len(email_parts) != 2: # User has a nonstandard email so we record their id. # we don't record their e-mail in case there is sensitive info accidentally # in there. - set_custom_attribute('login_tpa_domain_shortcircuit_user_id', user.id) + set_custom_attribute("login_tpa_domain_shortcircuit_user_id", user.id) log.warning("User %s has nonstandard e-mail. Shortcircuiting THIRD_PART_AUTH_ONLY_DOMAIN check.", user.id) return user_domain = email_parts[1].strip().lower() @@ -422,19 +421,19 @@ def _check_user_auth_flow(site, user): raise AuthFailedError(msg) raise AuthFailedError( - error_code='allowed-domain-login-error', + error_code="allowed-domain-login-error", context={ - 'allowed_domain': allowed_domain, - 'provider': site.configuration.get_value('THIRD_PARTY_AUTH_ONLY_PROVIDER'), - 'tpa_hint': site.configuration.get_value('THIRD_PARTY_AUTH_ONLY_HINT'), - } + "allowed_domain": allowed_domain, + "provider": site.configuration.get_value("THIRD_PARTY_AUTH_ONLY_PROVIDER"), + "tpa_hint": site.configuration.get_value("THIRD_PARTY_AUTH_ONLY_HINT"), + }, ) @login_required -@require_http_methods(['GET']) +@require_http_methods(["GET"]) def finish_auth(request): - """ Following logistration (1st or 3rd party), handle any special query string params. + """Following logistration (1st or 3rd party), handle any special query string params. See FinishAuthView.js for details on the query string params. @@ -459,10 +458,13 @@ def finish_auth(request): GET /account/finish_auth/?course_id=course-v1:blah&enrollment_action=enroll """ - return render_to_response('student_account/finish_auth.html', { - 'disable_courseware_js': True, - 'disable_footer': True, - }) + return render_to_response( + "student_account/finish_auth.html", + { + "disable_courseware_js": True, + "disable_footer": True, + }, + ) def enterprise_selection_page(request, user, next_url): @@ -478,14 +480,14 @@ def enterprise_selection_page(request, user, next_url): response = get_enterprise_learner_data_from_api(user) if response and len(response) > 1: - redirect_url = reverse('enterprise_select_active') + '/?success_url=' + urllib.parse.quote(next_url) + redirect_url = reverse("enterprise_select_active") + "/?success_url=" + urllib.parse.quote(next_url) # Check to see if next url has an enterprise in it. In this case if user is associated with # that enterprise, activate that enterprise and bypass the selection page. if re.match(ENTERPRISE_ENROLLMENT_URL_REGEX, urllib.parse.unquote(next_url)): enterprise_in_url = re.search(UUID4_REGEX, next_url).group(0) for enterprise in response: - if enterprise_in_url == str(enterprise['enterprise_customer']['uuid']): + if enterprise_in_url == str(enterprise["enterprise_customer"]["uuid"]): is_activated_successfully = activate_learner_enterprise(request, user, enterprise_in_url) if is_activated_successfully: redirect_url = next_url @@ -495,20 +497,20 @@ def enterprise_selection_page(request, user, next_url): @ensure_csrf_cookie -@require_http_methods(['POST']) +@require_http_methods(["POST"]) @ratelimit( - key='openedx.core.djangoapps.util.ratelimit.request_post_email_or_username', + key="openedx.core.djangoapps.util.ratelimit.request_post_email_or_username", rate=settings.LOGISTRATION_PER_EMAIL_RATELIMIT_RATE, - method='POST', + method="POST", block=False, ) # lint-amnesty, pylint: disable=too-many-statements @ratelimit( - key='openedx.core.djangoapps.util.ratelimit.real_ip', + key="openedx.core.djangoapps.util.ratelimit.real_ip", rate=settings.LOGISTRATION_RATELIMIT_RATE, - method='POST', + method="POST", block=False, ) # lint-amnesty, pylint: disable=too-many-statements -def login_user(request, api_version='v1'): # pylint: disable=too-many-statements +def login_user(request, api_version="v1"): # pylint: disable=too-many-statements """ AJAX request to log in the user. @@ -542,10 +544,10 @@ def login_user(request, api_version='v1'): # pylint: disable=too-many-statement _parse_analytics_param_for_course_id(request) third_party_auth_requested = third_party_auth.is_enabled() and pipeline.running(request) - first_party_auth_requested = any(bool(request.POST.get(p)) for p in ['email', 'email_or_username', 'password']) + first_party_auth_requested = any(bool(request.POST.get(p)) for p in ["email", "email_or_username", "password"]) is_user_third_party_authenticated = False - set_custom_attribute('login_user_course_id', request.POST.get('course_id')) + set_custom_attribute("login_user_course_id", request.POST.get("course_id")) if is_require_third_party_auth_enabled() and not third_party_auth_requested: return HttpResponseForbidden( @@ -564,12 +566,12 @@ def login_user(request, api_version='v1'): # pylint: disable=too-many-statement try: user = _do_third_party_auth(request) is_user_third_party_authenticated = True - set_custom_attribute('login_user_tpa_success', True) + set_custom_attribute("login_user_tpa_success", True) except AuthFailedError as e: - set_custom_attribute('login_user_tpa_success', False) - set_custom_attribute('login_user_tpa_failure_msg', e.value) + set_custom_attribute("login_user_tpa_success", False) + set_custom_attribute("login_user_tpa_failure_msg", e.value) if e.error_code: - set_custom_attribute('login_error_code', e.error_code) + set_custom_attribute("login_error_code", e.error_code) # user successfully authenticated with a third party provider, but has no linked Open edX account response_content = e.get_response() @@ -585,7 +587,10 @@ def login_user(request, api_version='v1'): # pylint: disable=too-many-statement possibly_authenticated_user = StudentLoginRequested.run_filter(user=possibly_authenticated_user) except StudentLoginRequested.PreventLogin as exc: raise AuthFailedError( - str(exc), redirect_url=exc.redirect_to, error_code=exc.error_code, context=exc.context, + str(exc), + redirect_url=exc.redirect_to, + error_code=exc.error_code, + context=exc.context, ) from exc if not is_user_third_party_authenticated: @@ -599,82 +604,82 @@ def login_user(request, api_version='v1'): # pylint: disable=too-many-statement ): _handle_failed_authentication(user, possibly_authenticated_user) - pwned_properties = check_pwned_password_and_send_track_event( - user_id=user.id, - password=request.POST.get('password'), - internal_user=user.is_staff, - request_page='login' - ) if not is_user_third_party_authenticated else {} - # Set default for third party login - password_frequency = pwned_properties.get('frequency', -1) - if ( - settings.ENABLE_AUTHN_LOGIN_BLOCK_HIBP_POLICY and - password_frequency >= settings.HIBP_LOGIN_BLOCK_PASSWORD_FREQUENCY_THRESHOLD - ): - raise VulnerablePasswordError( - accounts.AUTHN_LOGIN_BLOCK_HIBP_POLICY_MSG, - 'require-password-change' + pwned_properties = ( + check_pwned_password_and_send_track_event( + user_id=user.id, + password=request.POST.get("password"), + internal_user=user.is_staff, + request_page="login", ) + if not is_user_third_party_authenticated + else {} + ) + # Set default for third party login + password_frequency = pwned_properties.get("frequency", -1) + if ( + settings.ENABLE_AUTHN_LOGIN_BLOCK_HIBP_POLICY + and password_frequency >= settings.HIBP_LOGIN_BLOCK_PASSWORD_FREQUENCY_THRESHOLD + ): + raise VulnerablePasswordError(accounts.AUTHN_LOGIN_BLOCK_HIBP_POLICY_MSG, "require-password-change") _handle_successful_authentication_and_login(possibly_authenticated_user, request) # The AJAX method calling should know the default destination upon success - redirect_url, finish_auth_url = None, '' + redirect_url, finish_auth_url = None, "" if third_party_auth_requested: running_pipeline = pipeline.get(request) - finish_auth_url = pipeline.get_complete_url(backend_name=running_pipeline['backend']) + finish_auth_url = pipeline.get_complete_url(backend_name=running_pipeline["backend"]) if is_user_third_party_authenticated: redirect_url = finish_auth_url elif should_redirect_to_authn_microfrontend(): next_url, root_url = get_next_url_for_login_page(request, include_host=True) redirect_url = get_redirect_url_with_host( - root_url, - enterprise_selection_page(request, possibly_authenticated_user, finish_auth_url or next_url) + root_url, enterprise_selection_page(request, possibly_authenticated_user, finish_auth_url or next_url) ) if ( - settings.ENABLE_AUTHN_LOGIN_NUDGE_HIBP_POLICY and - 0 <= password_frequency <= settings.HIBP_LOGIN_NUDGE_PASSWORD_FREQUENCY_THRESHOLD + settings.ENABLE_AUTHN_LOGIN_NUDGE_HIBP_POLICY + and 0 <= password_frequency <= settings.HIBP_LOGIN_NUDGE_PASSWORD_FREQUENCY_THRESHOLD ): raise VulnerablePasswordError( - accounts.AUTHN_LOGIN_NUDGE_HIBP_POLICY_MSG, - 'nudge-password-change', - redirect_url + accounts.AUTHN_LOGIN_NUDGE_HIBP_POLICY_MSG, "nudge-password-change", redirect_url ) - response = JsonResponse({ - 'success': True, - 'redirect_url': redirect_url, - }) + response = JsonResponse( + { + "success": True, + "redirect_url": redirect_url, + } + ) # Ensure that the external marketing site can # detect that the user is logged in. response = set_logged_in_cookies(request, response, possibly_authenticated_user) - set_custom_attribute('login_user_auth_failed_error', False) - set_custom_attribute('login_user_response_status', response.status_code) - set_custom_attribute('login_user_redirect_url', redirect_url) + set_custom_attribute("login_user_auth_failed_error", False) + set_custom_attribute("login_user_response_status", response.status_code) + set_custom_attribute("login_user_redirect_url", redirect_url) mark_user_change_as_expected(user.id) return response except AuthFailedError as error: response_content = error.get_response() log.exception(response_content) - error_code = response_content.get('error_code') + error_code = response_content.get("error_code") if error_code: - set_custom_attribute('login_error_code', error_code) - email_or_username_key = 'email' if api_version == API_V1 else 'email_or_username' + set_custom_attribute("login_error_code", error_code) + email_or_username_key = "email" if api_version == API_V1 else "email_or_username" email_or_username = request.POST.get(email_or_username_key, None) email_or_username = possibly_authenticated_user.email if possibly_authenticated_user else email_or_username - response_content['email'] = email_or_username + response_content["email"] = email_or_username except VulnerablePasswordError as error: response_content = error.get_response() log.exception(response_content) response = JsonResponse(response_content, status=400) - set_custom_attribute('login_user_auth_failed_error', True) - set_custom_attribute('login_user_response_status', response.status_code) + set_custom_attribute("login_user_auth_failed_error", True) + set_custom_attribute("login_user_response_status", response.status_code) return response @@ -683,10 +688,10 @@ def login_user(request, api_version='v1'): # pylint: disable=too-many-statement # to get a CSRF token before we need to refresh adds too much # complexity. @csrf_exempt -@require_http_methods(['POST']) +@require_http_methods(["POST"]) def login_refresh(request): # lint-amnesty, pylint: disable=missing-function-docstring if not request.user.is_authenticated or request.user.is_anonymous: - return JsonResponse('Unauthorized', status=401) + return JsonResponse("Unauthorized", status=401) try: return get_response_with_refreshed_jwt_cookies(request, request.user) @@ -700,33 +705,57 @@ def redirect_to_lms_login(request): This view redirect the admin/login url to the site's login page if waffle switch is on otherwise returns the admin site's login view. """ - return redirect('/login?next=/admin') + return redirect("/login?next=/admin") + + +login_user_schema = openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + "email": openapi.Schema(type=openapi.TYPE_STRING), + "password": openapi.Schema(type=openapi.TYPE_STRING), + }, +) + +login_user_return_schema = openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + "success": openapi.Schema(type=openapi.TYPE_BOOLEAN), + "value": openapi.Schema(type=openapi.TYPE_STRING), + "error_code": openapi.Schema(type=openapi.TYPE_STRING), + }, +) class LoginSessionView(APIView): - """HTTP end-points for logging in users. """ + """HTTP end-points for logging in users.""" # This end-point is available to anonymous users, # so do not require authentication. authentication_classes = [] + login_user_responses = { + status.HTTP_200_OK: login_user_return_schema, + status.HTTP_400_BAD_REQUEST: login_user_return_schema, + status.HTTP_403_FORBIDDEN: login_user_return_schema, + } + @method_decorator(ensure_csrf_cookie) def get(self, request, *args, **kwargs): return HttpResponse(get_login_session_form(request).to_json(), content_type="application/json") # lint-amnesty, pylint: disable=http-response-with-content-type-json + @swagger_auto_schema( + request_body=login_user_schema, + responses=login_user_responses, + security=[ + {"csrf": []}, + ], + ) @method_decorator(csrf_protect) def post(self, request, api_version): - """Log in a user. - - See `login_user` for details. - - Example Usage: - - POST /api/user/v1/login_session - with POST params `email`, `password`. - - 200 {'success': true} + """ + POST /user/{api_version}/account/login_session/ + Returns 200 on success, and a detailed error message otherwise. """ return login_user(request, api_version) @@ -736,19 +765,19 @@ class LoginSessionView(APIView): def _parse_analytics_param_for_course_id(request): - """ If analytics request param is found, parse and add course id as a new request param. """ + """If analytics request param is found, parse and add course id as a new request param.""" # Make a copy of the current POST request to modify. modified_request = request.POST.copy() if isinstance(request, HttpRequest): # Works for an HttpRequest but not a rest_framework.request.Request. # Note: This case seems to be used for tests only. request.POST = modified_request - set_custom_attribute('login_user_request_type', 'django') + set_custom_attribute("login_user_request_type", "django") else: # The request must be a rest_framework.request.Request. # Note: Only DRF seems to be used in Production. request._data = modified_request # pylint: disable=protected-access - set_custom_attribute('login_user_request_type', 'drf') + set_custom_attribute("login_user_request_type", "drf") # Include the course ID if it's specified in the analytics info # so it can be included in analytics events. @@ -758,9 +787,5 @@ def _parse_analytics_param_for_course_id(request): if "enroll_course_id" in analytics: modified_request["course_id"] = analytics.get("enroll_course_id") except (ValueError, TypeError): - set_custom_attribute('shim_analytics_course_id', 'parse-error') - log.error( - "Could not parse analytics object sent to user API: {analytics}".format( - analytics=analytics - ) - ) + set_custom_attribute("shim_analytics_course_id", "parse-error") + log.error("Could not parse analytics object sent to user API: {analytics}".format(analytics=analytics)) From e1ecf7296c1223108987ea56c4c40d1d5dee4f39 Mon Sep 17 00:00:00 2001 From: edX requirements bot <49161187+edx-requirements-bot@users.noreply.github.com> Date: Tue, 29 Oct 2024 03:17:40 -0400 Subject: [PATCH 14/22] chore: Upgrade Python requirements (#35735) --- requirements/common_constraints.txt | 4 ++ requirements/edx-sandbox/base.txt | 2 +- requirements/edx/base.txt | 26 ++++++------ requirements/edx/development.txt | 40 +++++++++---------- requirements/edx/doc.txt | 26 ++++++------ requirements/edx/semgrep.txt | 2 +- requirements/edx/testing.txt | 38 +++++++++--------- requirements/pip.txt | 4 +- scripts/user_retirement/requirements/base.txt | 6 +-- .../user_retirement/requirements/testing.txt | 8 ++-- 10 files changed, 81 insertions(+), 75 deletions(-) diff --git a/requirements/common_constraints.txt b/requirements/common_constraints.txt index f2ef0216ca..5188f37250 100644 --- a/requirements/common_constraints.txt +++ b/requirements/common_constraints.txt @@ -32,3 +32,7 @@ elasticsearch<7.14.0 # This can be unpinned once https://github.com/openedx/edx-platform/issues/34586 # has been resolved and edx-platform is running with pymongo>=4.4.0 + +# Cause: https://github.com/openedx/edx-lint/issues/458 +# This can be unpinned once https://github.com/openedx/edx-lint/issues/459 has been resolved. +pip<24.3 diff --git a/requirements/edx-sandbox/base.txt b/requirements/edx-sandbox/base.txt index 991ea0efcf..eb74898596 100644 --- a/requirements/edx-sandbox/base.txt +++ b/requirements/edx-sandbox/base.txt @@ -86,5 +86,5 @@ sympy==1.13.3 # via # -r requirements/edx-sandbox/base.in # openedx-calc -tqdm==4.66.5 +tqdm==4.66.6 # via nltk diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 35106f1498..a6af0a610c 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -70,13 +70,13 @@ bleach[css]==6.1.0 # xblock-poll boto==2.49.0 # via -r requirements/edx/kernel.in -boto3==1.35.46 +boto3==1.35.50 # via # -r requirements/edx/kernel.in # django-ses # fs-s3fs # ora2 -botocore==1.35.46 +botocore==1.35.50 # via # -r requirements/edx/kernel.in # boto3 @@ -144,7 +144,7 @@ code-annotations==1.8.0 # edx-toggles codejail-includes==1.0.0 # via -r requirements/edx/kernel.in -crowdsourcehinter-xblock==0.7 +crowdsourcehinter-xblock==0.8 # via -r requirements/edx/bundled.in cryptography==43.0.3 # via @@ -455,7 +455,7 @@ edx-django-utils==7.0.0 # openedx-events # ora2 # super-csv -edx-drf-extensions==10.4.0 +edx-drf-extensions==10.5.0 # via # -r requirements/edx/kernel.in # edx-completion @@ -471,7 +471,7 @@ edx-enterprise==4.29.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in -edx-event-bus-kafka==5.8.1 +edx-event-bus-kafka==6.0.0 # via -r requirements/edx/kernel.in edx-event-bus-redis==0.5.1 # via -r requirements/edx/kernel.in @@ -584,7 +584,7 @@ geoip2==4.8.0 # via -r requirements/edx/kernel.in glob2==0.7 # via -r requirements/edx/kernel.in -google-api-core[grpc]==2.21.0 +google-api-core[grpc]==2.22.0 # via # firebase-admin # google-api-python-client @@ -747,7 +747,7 @@ markupsafe==3.0.2 # xblock maxminddb==2.6.2 # via geoip2 -meilisearch==0.31.5 +meilisearch==0.31.6 # via # -r requirements/edx/kernel.in # edx-search @@ -1038,9 +1038,9 @@ pyyaml==6.0.2 # xblock random2==1.0.2 # via -r requirements/edx/kernel.in -recommender-xblock==2.2.1 +recommender-xblock==3.0.0 # via -r requirements/edx/bundled.in -redis==5.1.1 +redis==5.2.0 # via # -r requirements/edx/kernel.in # walrus @@ -1144,7 +1144,7 @@ slumber==0.7.1 # -r requirements/edx/kernel.in # edx-bulk-grades # edx-enterprise -snowflake-connector-python==3.12.2 +snowflake-connector-python==3.12.3 # via edx-enterprise social-auth-app-django==5.4.1 # via @@ -1191,7 +1191,7 @@ tinycss2==1.2.1 # via bleach tomlkit==0.13.2 # via snowflake-connector-python -tqdm==4.66.5 +tqdm==4.66.6 # via # nltk # openai @@ -1254,7 +1254,7 @@ webencodings==0.5.1 # bleach # html5lib # tinycss2 -webob==1.8.8 +webob==1.8.9 # via # -r requirements/edx/kernel.in # xblock @@ -1291,7 +1291,7 @@ xmlsec==1.3.13 # python3-saml xss-utils==0.6.0 # via -r requirements/edx/kernel.in -yarl==1.16.0 +yarl==1.17.0 # via aiohttp zipp==3.20.2 # via importlib-metadata diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index b6a4b48d04..49bbe5bf53 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -140,14 +140,14 @@ boto==2.49.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -boto3==1.35.46 +boto3==1.35.50 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # django-ses # fs-s3fs # ora2 -botocore==1.35.46 +botocore==1.35.50 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -282,7 +282,7 @@ coverage[toml]==7.6.4 # via # -r requirements/edx/testing.txt # pytest-cov -crowdsourcehinter-xblock==0.7 +crowdsourcehinter-xblock==0.8 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -576,7 +576,7 @@ django-stubs==1.16.0 # -c requirements/edx/../constraints.txt # -r requirements/edx/development.in # djangorestframework-stubs -django-stubs-ext==5.1.0 +django-stubs-ext==5.1.1 # via django-stubs django-user-tasks==3.2.0 # via @@ -728,7 +728,7 @@ edx-django-utils==7.0.0 # openedx-events # ora2 # super-csv -edx-drf-extensions==10.4.0 +edx-drf-extensions==10.5.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -746,7 +746,7 @@ edx-enterprise==4.29.0 # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-event-bus-kafka==5.8.1 +edx-event-bus-kafka==6.0.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -760,7 +760,7 @@ edx-i18n-tools==1.5.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # ora2 -edx-lint==5.4.0 +edx-lint==5.4.1 # via -r requirements/edx/testing.txt edx-milestones==0.6.0 # via @@ -879,11 +879,11 @@ execnet==2.1.1 # pytest-xdist factory-boy==3.3.1 # via -r requirements/edx/testing.txt -faker==30.8.0 +faker==30.8.1 # via # -r requirements/edx/testing.txt # factory-boy -fastapi==0.115.3 +fastapi==0.115.4 # via # -r requirements/edx/testing.txt # pact-python @@ -943,7 +943,7 @@ glob2==0.7 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -google-api-core[grpc]==2.21.0 +google-api-core[grpc]==2.22.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1236,7 +1236,7 @@ mccabe==0.7.0 # via # -r requirements/edx/testing.txt # pylint -meilisearch==0.31.5 +meilisearch==0.31.6 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1712,7 +1712,7 @@ pytest-metadata==1.8.0 # via # -r requirements/edx/testing.txt # pytest-json-report -pytest-randomly==3.15.0 +pytest-randomly==3.16.0 # via -r requirements/edx/testing.txt pytest-xdist[psutil]==3.6.1 # via -r requirements/edx/testing.txt @@ -1800,11 +1800,11 @@ random2==1.0.2 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -recommender-xblock==2.2.1 +recommender-xblock==3.0.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -redis==5.1.1 +redis==5.2.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1958,7 +1958,7 @@ snowballstemmer==2.2.0 # via # -r requirements/edx/doc.txt # sphinx -snowflake-connector-python==3.12.2 +snowflake-connector-python==3.12.3 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -2053,7 +2053,7 @@ staff-graded-xblock==2.3.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -starlette==0.41.0 +starlette==0.41.2 # via # -r requirements/edx/testing.txt # fastapi @@ -2101,7 +2101,7 @@ tomlkit==0.13.2 # snowflake-connector-python tox==4.23.2 # via -r requirements/edx/testing.txt -tqdm==4.66.5 +tqdm==4.66.6 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -2180,7 +2180,7 @@ vine==5.1.0 # amqp # celery # kombu -virtualenv==20.27.0 +virtualenv==20.27.1 # via # -r requirements/edx/testing.txt # tox @@ -2222,7 +2222,7 @@ webencodings==0.5.1 # bleach # html5lib # tinycss2 -webob==1.8.8 +webob==1.8.9 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -2280,7 +2280,7 @@ xss-utils==0.6.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -yarl==1.16.0 +yarl==1.17.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index e0f08601b0..64dc11f076 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -102,13 +102,13 @@ bleach[css]==6.1.0 # xblock-poll boto==2.49.0 # via -r requirements/edx/base.txt -boto3==1.35.46 +boto3==1.35.50 # via # -r requirements/edx/base.txt # django-ses # fs-s3fs # ora2 -botocore==1.35.46 +botocore==1.35.50 # via # -r requirements/edx/base.txt # boto3 @@ -194,7 +194,7 @@ code-annotations==1.8.0 # edx-toggles codejail-includes==1.0.0 # via -r requirements/edx/base.txt -crowdsourcehinter-xblock==0.7 +crowdsourcehinter-xblock==0.8 # via -r requirements/edx/base.txt cryptography==43.0.3 # via @@ -535,7 +535,7 @@ edx-django-utils==7.0.0 # openedx-events # ora2 # super-csv -edx-drf-extensions==10.4.0 +edx-drf-extensions==10.5.0 # via # -r requirements/edx/base.txt # edx-completion @@ -551,7 +551,7 @@ edx-enterprise==4.29.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt -edx-event-bus-kafka==5.8.1 +edx-event-bus-kafka==6.0.0 # via -r requirements/edx/base.txt edx-event-bus-redis==0.5.1 # via -r requirements/edx/base.txt @@ -683,7 +683,7 @@ gitpython==3.1.43 # via -r requirements/edx/doc.in glob2==0.7 # via -r requirements/edx/base.txt -google-api-core[grpc]==2.21.0 +google-api-core[grpc]==2.22.0 # via # -r requirements/edx/base.txt # firebase-admin @@ -891,7 +891,7 @@ maxminddb==2.6.2 # via # -r requirements/edx/base.txt # geoip2 -meilisearch==0.31.5 +meilisearch==0.31.6 # via # -r requirements/edx/base.txt # edx-search @@ -1247,9 +1247,9 @@ pyyaml==6.0.2 # xblock random2==1.0.2 # via -r requirements/edx/base.txt -recommender-xblock==2.2.1 +recommender-xblock==3.0.0 # via -r requirements/edx/base.txt -redis==5.1.1 +redis==5.2.0 # via # -r requirements/edx/base.txt # walrus @@ -1371,7 +1371,7 @@ smmap==5.0.1 # via gitdb snowballstemmer==2.2.0 # via sphinx -snowflake-connector-python==3.12.2 +snowflake-connector-python==3.12.3 # via # -r requirements/edx/base.txt # edx-enterprise @@ -1472,7 +1472,7 @@ tomlkit==0.13.2 # via # -r requirements/edx/base.txt # snowflake-connector-python -tqdm==4.66.5 +tqdm==4.66.6 # via # -r requirements/edx/base.txt # nltk @@ -1547,7 +1547,7 @@ webencodings==0.5.1 # bleach # html5lib # tinycss2 -webob==1.8.8 +webob==1.8.9 # via # -r requirements/edx/base.txt # xblock @@ -1586,7 +1586,7 @@ xmlsec==1.3.13 # python3-saml xss-utils==0.6.0 # via -r requirements/edx/base.txt -yarl==1.16.0 +yarl==1.17.0 # via # -r requirements/edx/base.txt # aiohttp diff --git a/requirements/edx/semgrep.txt b/requirements/edx/semgrep.txt index 6ad814b602..61db06bbf1 100644 --- a/requirements/edx/semgrep.txt +++ b/requirements/edx/semgrep.txt @@ -116,7 +116,7 @@ ruamel-yaml==0.17.40 # via semgrep ruamel-yaml-clib==0.2.12 # via ruamel-yaml -semgrep==1.92.0 +semgrep==1.93.0 # via -r requirements/edx/semgrep.in tomli==2.0.2 # via semgrep diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 9cd37f6e4b..7853f39c6f 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -102,13 +102,13 @@ bleach[css]==6.1.0 # xblock-poll boto==2.49.0 # via -r requirements/edx/base.txt -boto3==1.35.46 +boto3==1.35.50 # via # -r requirements/edx/base.txt # django-ses # fs-s3fs # ora2 -botocore==1.35.46 +botocore==1.35.50 # via # -r requirements/edx/base.txt # boto3 @@ -213,7 +213,7 @@ coverage[toml]==7.6.4 # via # -r requirements/edx/coverage.txt # pytest-cov -crowdsourcehinter-xblock==0.7 +crowdsourcehinter-xblock==0.8 # via -r requirements/edx/base.txt cryptography==43.0.3 # via @@ -559,7 +559,7 @@ edx-django-utils==7.0.0 # openedx-events # ora2 # super-csv -edx-drf-extensions==10.4.0 +edx-drf-extensions==10.5.0 # via # -r requirements/edx/base.txt # edx-completion @@ -575,7 +575,7 @@ edx-enterprise==4.29.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt -edx-event-bus-kafka==5.8.1 +edx-event-bus-kafka==6.0.0 # via -r requirements/edx/base.txt edx-event-bus-redis==0.5.1 # via -r requirements/edx/base.txt @@ -584,7 +584,7 @@ edx-i18n-tools==1.5.0 # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # ora2 -edx-lint==5.4.0 +edx-lint==5.4.1 # via -r requirements/edx/testing.in edx-milestones==0.6.0 # via -r requirements/edx/base.txt @@ -674,9 +674,9 @@ execnet==2.1.1 # via pytest-xdist factory-boy==3.3.1 # via -r requirements/edx/testing.in -faker==30.8.0 +faker==30.8.1 # via factory-boy -fastapi==0.115.3 +fastapi==0.115.4 # via pact-python fastavro==1.9.7 # via @@ -717,7 +717,7 @@ geoip2==4.8.0 # via -r requirements/edx/base.txt glob2==0.7 # via -r requirements/edx/base.txt -google-api-core[grpc]==2.21.0 +google-api-core[grpc]==2.22.0 # via # -r requirements/edx/base.txt # firebase-admin @@ -944,7 +944,7 @@ maxminddb==2.6.2 # geoip2 mccabe==0.7.0 # via pylint -meilisearch==0.31.5 +meilisearch==0.31.6 # via # -r requirements/edx/base.txt # edx-search @@ -1295,7 +1295,7 @@ pytest-metadata==1.8.0 # via # -r requirements/edx/testing.in # pytest-json-report -pytest-randomly==3.15.0 +pytest-randomly==3.16.0 # via -r requirements/edx/testing.in pytest-xdist[psutil]==3.6.1 # via -r requirements/edx/testing.in @@ -1365,9 +1365,9 @@ pyyaml==6.0.2 # xblock random2==1.0.2 # via -r requirements/edx/base.txt -recommender-xblock==2.2.1 +recommender-xblock==3.0.0 # via -r requirements/edx/base.txt -redis==5.1.1 +redis==5.2.0 # via # -r requirements/edx/base.txt # walrus @@ -1490,7 +1490,7 @@ slumber==0.7.1 # edx-enterprise sniffio==1.3.1 # via anyio -snowflake-connector-python==3.12.2 +snowflake-connector-python==3.12.3 # via # -r requirements/edx/base.txt # edx-enterprise @@ -1522,7 +1522,7 @@ sqlparse==0.5.1 # django staff-graded-xblock==2.3.0 # via -r requirements/edx/base.txt -starlette==0.41.0 +starlette==0.41.2 # via fastapi stevedore==5.3.0 # via @@ -1560,7 +1560,7 @@ tomlkit==0.13.2 # snowflake-connector-python tox==4.23.2 # via -r requirements/edx/testing.in -tqdm==4.66.5 +tqdm==4.66.6 # via # -r requirements/edx/base.txt # nltk @@ -1614,7 +1614,7 @@ vine==5.1.0 # amqp # celery # kombu -virtualenv==20.27.0 +virtualenv==20.27.1 # via tox voluptuous==0.15.2 # via @@ -1644,7 +1644,7 @@ webencodings==0.5.1 # bleach # html5lib # tinycss2 -webob==1.8.8 +webob==1.8.9 # via # -r requirements/edx/base.txt # xblock @@ -1685,7 +1685,7 @@ xmlsec==1.3.13 # python3-saml xss-utils==0.6.0 # via -r requirements/edx/base.txt -yarl==1.16.0 +yarl==1.17.0 # via # -r requirements/edx/base.txt # aiohttp diff --git a/requirements/pip.txt b/requirements/pip.txt index 346a0611f0..797974efa4 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -9,6 +9,8 @@ wheel==0.44.0 # The following packages are considered to be unsafe in a requirements file: pip==24.2 - # via -r requirements/pip.in + # via + # -c requirements/common_constraints.txt + # -r requirements/pip.in setuptools==75.2.0 # via -r requirements/pip.in diff --git a/scripts/user_retirement/requirements/base.txt b/scripts/user_retirement/requirements/base.txt index 5c2c300341..288d660b14 100644 --- a/scripts/user_retirement/requirements/base.txt +++ b/scripts/user_retirement/requirements/base.txt @@ -10,9 +10,9 @@ attrs==24.2.0 # via zeep backoff==2.2.1 # via -r scripts/user_retirement/requirements/base.in -boto3==1.35.46 +boto3==1.35.50 # via -r scripts/user_retirement/requirements/base.in -botocore==1.35.46 +botocore==1.35.50 # via # boto3 # s3transfer @@ -50,7 +50,7 @@ edx-django-utils==7.0.0 # via edx-rest-api-client edx-rest-api-client==6.0.0 # via -r scripts/user_retirement/requirements/base.in -google-api-core==2.21.0 +google-api-core==2.22.0 # via google-api-python-client google-api-python-client==2.149.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 a149a1d10c..da4ce1b039 100644 --- a/scripts/user_retirement/requirements/testing.txt +++ b/scripts/user_retirement/requirements/testing.txt @@ -14,11 +14,11 @@ attrs==24.2.0 # zeep backoff==2.2.1 # via -r scripts/user_retirement/requirements/base.txt -boto3==1.35.46 +boto3==1.35.50 # via # -r scripts/user_retirement/requirements/base.txt # moto -botocore==1.35.46 +botocore==1.35.50 # via # -r scripts/user_retirement/requirements/base.txt # boto3 @@ -72,7 +72,7 @@ edx-django-utils==7.0.0 # edx-rest-api-client edx-rest-api-client==6.0.0 # via -r scripts/user_retirement/requirements/base.txt -google-api-core==2.21.0 +google-api-core==2.22.0 # via # -r scripts/user_retirement/requirements/base.txt # google-api-python-client @@ -272,7 +272,7 @@ urllib3==1.26.20 # botocore # requests # responses -werkzeug==3.0.4 +werkzeug==3.0.6 # via moto xmltodict==0.14.2 # via moto From d54e3bd4c1b5780e43eb3ba6d073f96dcb848561 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Tue, 29 Oct 2024 11:50:43 -0400 Subject: [PATCH 15/22] feat: bump edx-enterprise to 4.30.0, extend system-wide roles for enrollment intentions feature (#35739) --- lms/envs/common.py | 7 +++++++ requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 6 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 2f47e006c6..c7e38441e3 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -50,6 +50,7 @@ from path import Path as path from django.utils.translation import gettext_lazy as _ from enterprise.constants import ( ENTERPRISE_ADMIN_ROLE, + ENTERPRISE_LEARNER_ROLE, ENTERPRISE_CATALOG_ADMIN_ROLE, ENTERPRISE_DASHBOARD_ADMIN_ROLE, ENTERPRISE_ENROLLMENT_API_ADMIN_ROLE, @@ -60,6 +61,7 @@ from enterprise.constants import ( SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE, PROVISIONING_ENTERPRISE_CUSTOMER_ADMIN_ROLE, PROVISIONING_PENDING_ENTERPRISE_CUSTOMER_ADMIN_ROLE, + DEFAULT_ENTERPRISE_ENROLLMENT_INTENTIONS_ROLE, ) from openedx.core.constants import COURSE_KEY_REGEX, COURSE_KEY_PATTERN, COURSE_ID_PATTERN @@ -4722,11 +4724,15 @@ ENTERPRISE_READONLY_ACCOUNT_FIELDS = [ ENTERPRISE_CUSTOMER_COOKIE_NAME = 'enterprise_customer_uuid' BASE_COOKIE_DOMAIN = 'localhost' SYSTEM_TO_FEATURE_ROLE_MAPPING = { + ENTERPRISE_LEARNER_ROLE: [ + DEFAULT_ENTERPRISE_ENROLLMENT_INTENTIONS_ROLE, + ], ENTERPRISE_ADMIN_ROLE: [ ENTERPRISE_DASHBOARD_ADMIN_ROLE, ENTERPRISE_CATALOG_ADMIN_ROLE, ENTERPRISE_ENROLLMENT_API_ADMIN_ROLE, ENTERPRISE_REPORTING_CONFIG_ADMIN_ROLE, + DEFAULT_ENTERPRISE_ENROLLMENT_INTENTIONS_ROLE, ], ENTERPRISE_OPERATOR_ROLE: [ ENTERPRISE_DASHBOARD_ADMIN_ROLE, @@ -4735,6 +4741,7 @@ SYSTEM_TO_FEATURE_ROLE_MAPPING = { ENTERPRISE_REPORTING_CONFIG_ADMIN_ROLE, ENTERPRISE_FULFILLMENT_OPERATOR_ROLE, ENTERPRISE_SSO_ORCHESTRATOR_OPERATOR_ROLE, + DEFAULT_ENTERPRISE_ENROLLMENT_INTENTIONS_ROLE, ], SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE: [ PROVISIONING_ENTERPRISE_CUSTOMER_ADMIN_ROLE, diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 6c8fe968b2..0292e08f61 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -82,7 +82,7 @@ django-storages<1.14.4 # 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.29.0 +edx-enterprise==4.30.0 # Date: 2024-05-09 # This has to be constrained as well because newer versions of edx-i18n-tools need the diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index a6af0a610c..a0117e5730 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -467,7 +467,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.29.0 +edx-enterprise==4.30.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 49bbe5bf53..485a57753a 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -741,7 +741,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.29.0 +edx-enterprise==4.30.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 64dc11f076..5f6f0e1162 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -547,7 +547,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.29.0 +edx-enterprise==4.30.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 7853f39c6f..fc60c467d4 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -571,7 +571,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.29.0 +edx-enterprise==4.30.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From 55aeca65c2f1382c6b4a3b7f3f7e9de9f6fe94bc Mon Sep 17 00:00:00 2001 From: farhan Date: Thu, 24 Oct 2024 16:28:10 +0500 Subject: [PATCH 16/22] feat!: Dropping Sass support from builtin poll block, replacing with vanilla CSS --- xmodule/assets/poll/_display.scss | 16 +- xmodule/poll_block.py | 4 +- .../css-builtin-blocks/PollBlockDisplay.css | 221 ++++++++++++++++++ 3 files changed, 231 insertions(+), 10 deletions(-) create mode 100644 xmodule/static/css-builtin-blocks/PollBlockDisplay.css diff --git a/xmodule/assets/poll/_display.scss b/xmodule/assets/poll/_display.scss index 7c07f89376..7c9b21bf20 100644 --- a/xmodule/assets/poll/_display.scss +++ b/xmodule/assets/poll/_display.scss @@ -20,13 +20,13 @@ div.poll_question { h3 { margin-top: 0; - margin-bottom: ($baseline*0.75); + margin-bottom: calc((var(--baseline)*0.75)); color: #fe57a1; font-size: 1.9em; &.problem-header { div.staff { - margin-top: ($baseline*1.5); + margin-top: calc((var(--baseline)*1.5)); font-size: 80%; } } @@ -44,7 +44,7 @@ div.poll_question { } .poll_answer { - margin-bottom: $baseline; + margin-bottom: var(--baseline); &.short { clear: both; @@ -107,7 +107,7 @@ div.poll_question { font-weight: bold; letter-spacing: normal; line-height: 25.59375px; - margin-bottom: ($baseline*0.75); + margin-bottom: calc((var(--baseline)*0.75)); margin: 0; padding: 0px; text-align: center; @@ -145,9 +145,9 @@ div.poll_question { width: 80%; text-align: left; min-height: 30px; - margin-left: $baseline; + margin-left: var(--baseline); height: auto; - margin-bottom: $baseline; + margin-bottom: var(--baseline); &.short { width: 100px; @@ -157,7 +157,7 @@ div.poll_question { .stats { min-height: 40px; - margin-top: $baseline; + margin-top: var(--baseline); clear: both; &.short { @@ -174,7 +174,7 @@ div.poll_question { border: 1px solid black; display: inline; float: left; - margin-right: ($baseline/2); + margin-right: calc((var(--baseline)/2)); &.short { width: 65%; diff --git a/xmodule/poll_block.py b/xmodule/poll_block.py index f09889f506..a1c9686f42 100644 --- a/xmodule/poll_block.py +++ b/xmodule/poll_block.py @@ -21,7 +21,7 @@ from xblock.fields import Boolean, Dict, List, Scope, String # lint-amnesty, py from openedx.core.djangolib.markup import Text, HTML from xmodule.mako_block import MakoTemplateBlockBase from xmodule.stringify import stringify_children -from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment +from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_css_to_fragment from xmodule.x_module import ( ResourceTemplates, shim_xmodule_js, @@ -136,7 +136,7 @@ class PollBlock( 'configuration_json': self.dump_poll(), } fragment.add_content(self.runtime.service(self, 'mako').render_lms_template('poll.html', params)) - add_sass_to_fragment(fragment, 'PollBlockDisplay.scss') + add_css_to_fragment(fragment, 'PollBlockDisplay.css') add_webpack_js_to_fragment(fragment, 'PollBlockDisplay') shim_xmodule_js(fragment, 'Poll') return fragment diff --git a/xmodule/static/css-builtin-blocks/PollBlockDisplay.css b/xmodule/static/css-builtin-blocks/PollBlockDisplay.css new file mode 100644 index 0000000000..f617dc960d --- /dev/null +++ b/xmodule/static/css-builtin-blocks/PollBlockDisplay.css @@ -0,0 +1,221 @@ +@import url("https://fonts.googleapis.com/css?family=Open+Sans:300,400,400i,600,700"); + +.xmodule_display.xmodule_PollBlock { + /* stylelint-disable-line */ + /* stylelint-disable-line */ +} + +@media print { + .xmodule_display.xmodule_PollBlock div.poll_question { + display: block; + width: auto; + padding: 0; + } + + .xmodule_display.xmodule_PollBlock div.poll_question canvas, .xmodule_display.xmodule_PollBlock div.poll_question img { + page-break-inside: avoid; + } +} + +.xmodule_display.xmodule_PollBlock div.poll_question .inline { + display: inline; +} + +.xmodule_display.xmodule_PollBlock div.poll_question h3 { + margin-top: 0; + margin-bottom: calc((var(--baseline) * 0.75)); + color: #fe57a1; + font-size: 1.9em; +} + +.xmodule_display.xmodule_PollBlock div.poll_question h3.problem-header div.staff { + margin-top: calc((var(--baseline) * 1.5)); + font-size: 80%; +} + +@media print { + .xmodule_display.xmodule_PollBlock div.poll_question h3 { + display: block; + width: auto; + border-right: 0; + } +} + +.xmodule_display.xmodule_PollBlock div.poll_question p { + text-align: justify; + font-weight: bold; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer { + margin-bottom: var(--baseline); +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer.short { + clear: both; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .question { + height: auto; + clear: both; + min-height: 30px; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .question.short { + clear: none; + width: 30%; + display: inline; + float: left; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .question .button { + -webkit-appearance: none; + -webkit-background-clip: padding-box; + -webkit-border-image: none; + -webkit-box-align: center; + -webkit-box-shadow: white 0px 1px 0px 0px inset; + -webkit-font-smoothing: antialiased; + -webkit-rtl-ordering: logical; + -webkit-user-select: text; + -webkit-writing-mode: horizontal-tb; + background-clip: padding-box; + background-color: #eeeeee; + background-image: -webkit-linear-gradient(top, #eeeeee, #d2d2d2); + border-bottom-color: #cacaca; + border-bottom-left-radius: 3px; + border-bottom-right-radius: 3px; + border-bottom-style: solid; + border-bottom-width: 1px; + border-left-color: #cacaca; + border-left-style: solid; + border-left-width: 1px; + border-right-color: #cacaca; + border-right-style: solid; + border-right-width: 1px; + border-top-color: #cacaca; + border-top-left-radius: 3px; + border-top-right-radius: 3px; + border-top-style: solid; + border-top-width: 1px; + box-shadow: white 0px 1px 0px 0px inset; + box-sizing: border-box; + color: #333333; + /* display: inline-block; */ + display: inline; + float: left; + font-family: 'Open Sans', Verdana, Geneva, sans-serif; + font-size: 13px; + font-style: normal; + font-variant: normal; + font-weight: bold; + letter-spacing: normal; + line-height: 25.59375px; + margin-bottom: calc((var(--baseline) * 0.75)); + margin: 0; + padding: 0px; + text-align: center; + text-decoration: none; + text-indent: 0px; + text-shadow: #f8f8f8 0px 1px 0px; + text-transform: none; + vertical-align: top; + white-space: pre-line; + width: 25px; + height: 25px; + word-spacing: 0px; + writing-mode: lr-tb; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .question .button.answered { + -webkit-box-shadow: #61b8e1 0px 1px 0px 0px inset; + background-color: #1d9dd9; + background-image: -webkit-linear-gradient(top, #1d9dd9, #0e7cb0); + border-bottom-color: #0d72a2; + border-left-color: #0d72a2; + border-right-color: #0d72a2; + border-top-color: #0d72a2; + box-shadow: #61b8e1 0px 1px 0px 0px inset; + color: white; + text-shadow: #076794 0px 1px 0px; + background-image: none; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .question .text { + display: inline; + float: left; + width: 80%; + text-align: left; + min-height: 30px; + margin-left: var(--baseline); + height: auto; + margin-bottom: var(--baseline); +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .question .text.short { + width: 100px; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .stats { + min-height: 40px; + margin-top: var(--baseline); + clear: both; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .stats.short { + margin-top: 0; + clear: none; + display: inline; + float: right; + width: 70%; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .stats .bar { + width: 75%; + height: 20px; + border: 1px solid black; + display: inline; + float: left; + margin-right: calc((var(--baseline) / 2)); +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .stats .bar.short { + width: 65%; + height: 20px; + margin-top: 3px; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .stats .bar .percent { + background-color: gray; + width: 0; + height: 20px; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .stats .number { + width: 80px; + display: inline; + float: right; + height: 28px; + text-align: right; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer .stats .number.short { + width: 120px; + height: auto; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .poll_answer.answered { + -webkit-box-shadow: #61b8e1 0 1px 0 0 inset; + background-color: #1d9dd9; + background-image: -webkit-linear-gradient(top, #1d9dd9, #0e7cb0); + border-bottom-color: #0d72a2; + border-left-color: #0d72a2; + border-right-color: #0d72a2; + border-top-color: #0d72a2; + box-shadow: #61b8e1 0 1px 0 0 inset; + color: white; + text-shadow: #076794 0 1px 0; +} + +.xmodule_display.xmodule_PollBlock div.poll_question .button.reset-button { + clear: both; + float: right; +} From 7f1611ed9039b425e5ed094f7160e68f518298c2 Mon Sep 17 00:00:00 2001 From: farhan Date: Fri, 25 Oct 2024 18:35:23 +0500 Subject: [PATCH 17/22] chore: remove scss files related to the poll block --- xmodule/assets/PollBlockDisplay.scss | 3 - xmodule/assets/poll/_display.scss | 226 --------------------------- 2 files changed, 229 deletions(-) delete mode 100644 xmodule/assets/PollBlockDisplay.scss delete mode 100644 xmodule/assets/poll/_display.scss diff --git a/xmodule/assets/PollBlockDisplay.scss b/xmodule/assets/PollBlockDisplay.scss deleted file mode 100644 index 85110778c7..0000000000 --- a/xmodule/assets/PollBlockDisplay.scss +++ /dev/null @@ -1,3 +0,0 @@ -.xmodule_display.xmodule_PollBlock { - @import "poll/display.scss"; -} diff --git a/xmodule/assets/poll/_display.scss b/xmodule/assets/poll/_display.scss deleted file mode 100644 index 7c9b21bf20..0000000000 --- a/xmodule/assets/poll/_display.scss +++ /dev/null @@ -1,226 +0,0 @@ -@import 'bourbon/bourbon'; -@import 'lms/theme/variables'; -@import 'bootstrap/scss/variables'; -@import 'lms/theme/variables-v1'; - -div.poll_question { - @media print { - display: block; - width: auto; - padding: 0; - - canvas, img { - page-break-inside: avoid; - } - } - - .inline { - display: inline; - } - - h3 { - margin-top: 0; - margin-bottom: calc((var(--baseline)*0.75)); - color: #fe57a1; - font-size: 1.9em; - - &.problem-header { - div.staff { - margin-top: calc((var(--baseline)*1.5)); - font-size: 80%; - } - } - - @media print { - display: block; - width: auto; - border-right: 0; - } - } - - p { - text-align: justify; - font-weight: bold; - } - - .poll_answer { - margin-bottom: var(--baseline); - - &.short { - clear: both; - } - - .question { - height: auto; - clear: both; - min-height: 30px; - - &.short { - clear: none; - width: 30%; - display: inline; - float: left; - } - - .button { - @extend %ui-fake-link; - - -webkit-appearance: none; - -webkit-background-clip: padding-box; - -webkit-border-image: none; - -webkit-box-align: center; - -webkit-box-shadow: rgb(255, 255, 255) 0px 1px 0px 0px inset; - -webkit-font-smoothing: antialiased; - -webkit-rtl-ordering: logical; - -webkit-user-select: text; - -webkit-writing-mode: horizontal-tb; - background-clip: padding-box; - background-color: rgb(238, 238, 238); - background-image: -webkit-linear-gradient(top, rgb(238, 238, 238), rgb(210, 210, 210)); - border-bottom-color: rgb(202, 202, 202); - border-bottom-left-radius: 3px; - border-bottom-right-radius: 3px; - border-bottom-style: solid; - border-bottom-width: 1px; - border-left-color: rgb(202, 202, 202); - border-left-style: solid; - border-left-width: 1px; - border-right-color: rgb(202, 202, 202); - border-right-style: solid; - border-right-width: 1px; - border-top-color: rgb(202, 202, 202); - border-top-left-radius: 3px; - border-top-right-radius: 3px; - border-top-style: solid; - border-top-width: 1px; - box-shadow: rgb(255, 255, 255) 0px 1px 0px 0px inset; - box-sizing: border-box; - color: rgb(51, 51, 51); - - /* display: inline-block; */ - display: inline; - float: left; - font-family: 'Open Sans', Verdana, Geneva, sans-serif; - font-size: 13px; - font-style: normal; - font-variant: normal; - font-weight: bold; - letter-spacing: normal; - line-height: 25.59375px; - margin-bottom: calc((var(--baseline)*0.75)); - margin: 0; - padding: 0px; - text-align: center; - text-decoration: none; - text-indent: 0px; - text-shadow: rgb(248, 248, 248) 0px 1px 0px; - text-transform: none; - vertical-align: top; - white-space: pre-line; - width: 25px; - height: 25px; - word-spacing: 0px; - writing-mode: lr-tb; - } - - .button.answered { - -webkit-box-shadow: rgb(97, 184, 225) 0px 1px 0px 0px inset; - background-color: rgb(29, 157, 217); - background-image: -webkit-linear-gradient(top, rgb(29, 157, 217), rgb(14, 124, 176)); - border-bottom-color: rgb(13, 114, 162); - border-left-color: rgb(13, 114, 162); - border-right-color: rgb(13, 114, 162); - border-top-color: rgb(13, 114, 162); - box-shadow: rgb(97, 184, 225) 0px 1px 0px 0px inset; - color: rgb(255, 255, 255); - text-shadow: rgb(7, 103, 148) 0px 1px 0px; - background-image: none; - } - - .text { - @extend %ui-fake-link; - - display: inline; - float: left; - width: 80%; - text-align: left; - min-height: 30px; - margin-left: var(--baseline); - height: auto; - margin-bottom: var(--baseline); - - &.short { - width: 100px; - } - } - } - - .stats { - min-height: 40px; - margin-top: var(--baseline); - clear: both; - - &.short { - margin-top: 0; - clear: none; - display: inline; - float: right; - width: 70%; - } - - .bar { - width: 75%; - height: 20px; - border: 1px solid black; - display: inline; - float: left; - margin-right: calc((var(--baseline)/2)); - - &.short { - width: 65%; - height: 20px; - margin-top: 3px; - } - - .percent { - background-color: gray; - width: 0; - height: 20px; - - &.short { } - } - } - - .number { - width: 80px; - display: inline; - float: right; - height: 28px; - text-align: right; - - &.short { - width: 120px; - height: auto; - } - } - } - } - - .poll_answer.answered { - -webkit-box-shadow: rgb(97, 184, 225) 0 1px 0 0 inset; - background-color: rgb(29, 157, 217); - background-image: -webkit-linear-gradient(top, rgb(29, 157, 217), rgb(14, 124, 176)); - border-bottom-color: rgb(13, 114, 162); - border-left-color: rgb(13, 114, 162); - border-right-color: rgb(13, 114, 162); - border-top-color: rgb(13, 114, 162); - box-shadow: rgb(97, 184, 225) 0 1px 0 0 inset; - color: rgb(255, 255, 255); - text-shadow: rgb(7, 103, 148) 0 1px 0; - } - - .button.reset-button { - clear: both; - float: right; - } -} From d5a768964774e845b0e0040d5e86295dc2ab6635 Mon Sep 17 00:00:00 2001 From: Fatima Sohail <68312464+sohailfatima@users.noreply.github.com> Date: Wed, 30 Oct 2024 14:11:59 +0500 Subject: [PATCH 18/22] fix: import font for notification digest email (#35720) * fix: import font for notification digest email * fix: font smoothing and size --- .../templates/notifications/digest_content.html | 5 +++-- .../notifications/edx_ace/email_digest/email/body.html | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/notifications/templates/notifications/digest_content.html b/openedx/core/djangoapps/notifications/templates/notifications/digest_content.html index f2e239bb7e..df1de6171a 100644 --- a/openedx/core/djangoapps/notifications/templates/notifications/digest_content.html +++ b/openedx/core/djangoapps/notifications/templates/notifications/digest_content.html @@ -28,8 +28,9 @@ style="max-height: 28px; max-width: 28px; margin: 0.75rem 1rem 0.75rem 0" /> - +
    + {{ notification.email_content | truncatechars_html:600 | safe }}
    {% if notification.details %} @@ -37,7 +38,7 @@ {{ notification.details | safe }} {% endif %} -
    +
    {{ notification.course_name }} {{ "·"|safe }} diff --git a/openedx/core/djangoapps/notifications/templates/notifications/edx_ace/email_digest/email/body.html b/openedx/core/djangoapps/notifications/templates/notifications/edx_ace/email_digest/email/body.html index 76658a0436..a15f4f6ebf 100644 --- a/openedx/core/djangoapps/notifications/templates/notifications/edx_ace/email_digest/email/body.html +++ b/openedx/core/djangoapps/notifications/templates/notifications/edx_ace/email_digest/email/body.html @@ -1,4 +1,8 @@ -
    + + + + +
    From 051eacb0244e438ae8b084b951a465b9e35b7f80 Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Fri, 18 Oct 2024 18:35:13 +0000 Subject: [PATCH 19/22] fix: certificate display behaiviour not showing date-picker for end-with-date --- cms/static/js/views/settings/main.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/cms/static/js/views/settings/main.js b/cms/static/js/views/settings/main.js index 4f3bf0bb27..5b7a25dbde 100644 --- a/cms/static/js/views/settings/main.js +++ b/cms/static/js/views/settings/main.js @@ -388,9 +388,6 @@ function(ValidatingView, CodeMirror, _, $, ui, DateUtils, FileUploadModel, Hides and clears the certificate available date field if a display behavior that doesn't use it is chosen. Because we are clearing it, toggling back to "end_with_date" will require re-entering the date */ - if (!this.useV2CertDisplaySettings) { - return; - } // eslint-disable-next-line prefer-const let showDatepicker = this.model.get('certificates_display_behavior') == 'end_with_date'; // eslint-disable-next-line prefer-const From fb5e26952bc581f72529be0f7daeacf47625321d Mon Sep 17 00:00:00 2001 From: Muhammad Anas Date: Fri, 18 Oct 2024 18:44:42 +0000 Subject: [PATCH 20/22] fix: removed all the usage of useV2CertDisplaySettings --- cms/static/js/factories/settings.js | 4 +- .../js/models/settings/course_details.js | 54 +++++++++---------- 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/cms/static/js/factories/settings.js b/cms/static/js/factories/settings.js index f8f927287c..472f3c874e 100644 --- a/cms/static/js/factories/settings.js +++ b/cms/static/js/factories/settings.js @@ -3,7 +3,7 @@ define([ ], function($, CourseDetailsModel, MainView) { 'use strict'; - return function(detailsUrl, showMinGradeWarning, showCertificateAvailableDate, upgradeDeadline, useV2CertDisplaySettings) { + return function(detailsUrl, showMinGradeWarning, showCertificateAvailableDate, upgradeDeadline) { var model; // highlighting labels when fields are focused in $('form :input') @@ -23,7 +23,6 @@ define([ model = new CourseDetailsModel(); model.urlRoot = detailsUrl; model.showCertificateAvailableDate = showCertificateAvailableDate; - model.useV2CertDisplaySettings = useV2CertDisplaySettings; model.set('upgrade_deadline', upgradeDeadline); model.fetch({ // eslint-disable-next-line no-shadow @@ -33,7 +32,6 @@ define([ model: model, showMinGradeWarning: showMinGradeWarning }); - editor.useV2CertDisplaySettings = useV2CertDisplaySettings; editor.render(); }, reset: true, diff --git a/cms/static/js/models/settings/course_details.js b/cms/static/js/models/settings/course_details.js index 302f214fd6..1714436d7d 100644 --- a/cms/static/js/models/settings/course_details.js +++ b/cms/static/js/models/settings/course_details.js @@ -84,35 +84,33 @@ function(Backbone, _, gettext, ValidationHelpers, DateUtils, StringUtils) { ); } - if (this.useV2CertDisplaySettings) { - if ( - newattrs.certificates_display_behavior - && !(Object.values(CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS).includes(newattrs.certificates_display_behavior)) - ) { - errors.certificates_display_behavior = StringUtils.interpolate( - gettext( - 'The certificate display behavior must be one of: {behavior_options}' - ), - { - behavior_options: Object.values(CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS).join(', ') - } - ); - } + if ( + newattrs.certificates_display_behavior + && !(Object.values(CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS).includes(newattrs.certificates_display_behavior)) + ) { + errors.certificates_display_behavior = StringUtils.interpolate( + gettext( + 'The certificate display behavior must be one of: {behavior_options}' + ), + { + behavior_options: Object.values(CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS).join(', ') + } + ); + } - // Throw error if there's a value for certificate_available_date - if ( - (newattrs.certificate_available_date && newattrs.certificates_display_behavior != CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS.END_WITH_DATE) - || (!newattrs.certificate_available_date && newattrs.certificates_display_behavior == CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS.END_WITH_DATE) - ) { - errors.certificates_display_behavior = StringUtils.interpolate( - gettext( - 'The certificates display behavior must be {valid_option} if certificate available date is set.' - ), - { - valid_option: CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS.END_WITH_DATE - } - ); - } + // Throw error if there's a value for certificate_available_date + if ( + (newattrs.certificate_available_date && newattrs.certificates_display_behavior != CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS.END_WITH_DATE) + || (!newattrs.certificate_available_date && newattrs.certificates_display_behavior == CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS.END_WITH_DATE) + ) { + errors.certificates_display_behavior = StringUtils.interpolate( + gettext( + 'The certificates display behavior must be {valid_option} if certificate available date is set.' + ), + { + valid_option: CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS.END_WITH_DATE + } + ); } if (newattrs.intro_video && newattrs.intro_video !== this.get('intro_video')) { From 338a0a1166b34f83edc64cfe89787e609df23b61 Mon Sep 17 00:00:00 2001 From: Alison Langston <46360176+alangsto@users.noreply.github.com> Date: Wed, 30 Oct 2024 08:52:43 -0400 Subject: [PATCH 21/22] feat: check course start date for courseware search (#35740) --- lms/djangoapps/courseware/tests/test_views.py | 20 +++++++++++++++++++ lms/djangoapps/courseware/views/views.py | 7 +++++++ 2 files changed, 27 insertions(+) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index cde0e8a34e..7a5e36e549 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -3814,6 +3814,26 @@ class TestCoursewareMFESearchAPI(SharedModuleStoreTestCase): self.assertEqual(response.status_code, 200) self.assertEqual(body, {'enabled': False}) + @patch.dict('django.conf.settings.FEATURES', {'COURSEWARE_SEARCH_INCLUSION_DATE': '2020'}) + @ddt.data( + (datetime(2013, 9, 18, 11, 30, 00), False), + (None, False), + (datetime(2024, 9, 18, 11, 30, 00), True), + ) + @ddt.unpack + def test_inclusion_date_greater_than_course_start(self, start_date, expected_enabled): + course_with_start = CourseFactory.create(start=start_date) + api_url = reverse('courseware_search_enabled_view', kwargs={'course_id': str(course_with_start.id)}) + + user_staff = UserFactory(is_staff=True) + + self.client.login(username=user_staff.username, password=TEST_PASSWORD) + response = self.client.get(api_url, content_type='application/json') + body = json.loads(response.content.decode('utf-8')) + + self.assertEqual(response.status_code, 200) + self.assertEqual(body, {'enabled': expected_enabled}) + class TestCoursewareMFENavigationSidebarTogglesAPI(SharedModuleStoreTestCase): """ diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 1c57b23d9b..b182202edd 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -2317,6 +2317,13 @@ def courseware_mfe_search_enabled(request, course_id=None): else: enabled = True + inclusion_date = settings.FEATURES.get('COURSEWARE_SEARCH_INCLUSION_DATE') + start_date = CourseOverview.get_from_id(course_key).start + + # only include courses that have a start date later than the setting-defined inclusion date + if inclusion_date: + enabled = enabled and (start_date and start_date.strftime('%Y-%m-%d') > inclusion_date) + payload = {"enabled": courseware_mfe_search_is_enabled(course_key) if enabled else False} return JsonResponse(payload) From 10a876ffbd38d790c8a72462cd69b0d673160f21 Mon Sep 17 00:00:00 2001 From: Kira Miller <31229189+kiram15@users.noreply.github.com> Date: Wed, 30 Oct 2024 09:12:35 -0600 Subject: [PATCH 22/22] fix: removing migration check after dropping column in table (#35742) --- common/djangoapps/util/tests/test_db.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/djangoapps/util/tests/test_db.py b/common/djangoapps/util/tests/test_db.py index 8d80991c41..4a16c2a20a 100644 --- a/common/djangoapps/util/tests/test_db.py +++ b/common/djangoapps/util/tests/test_db.py @@ -1,7 +1,6 @@ """Tests for util.db module.""" from io import StringIO -import unittest import ddt from django.core.management import call_command @@ -121,7 +120,6 @@ class MigrationTests(TestCase): Tests for migrations. """ - @unittest.skip('Skipping temporarily to drop column in table') @override_settings(MIGRATION_MODULES={}) def test_migrations_are_in_sync(self): """