Merge branch 'master' into shafqat/VAN-966
This commit is contained in:
@@ -2,6 +2,8 @@
|
||||
Settings for ace_common app.
|
||||
"""
|
||||
|
||||
ACE_ROUTING_KEY = 'edx.lms.core.default'
|
||||
|
||||
|
||||
def plugin_settings(settings): # lint-amnesty, pylint: disable=missing-function-docstring, missing-module-docstring
|
||||
settings.ACE_ENABLED_CHANNELS = [
|
||||
@@ -17,6 +19,6 @@ def plugin_settings(settings): # lint-amnesty, pylint: disable=missing-function
|
||||
settings.ACE_CHANNEL_DEFAULT_EMAIL = 'django_email'
|
||||
settings.ACE_CHANNEL_TRANSACTIONAL_EMAIL = 'django_email'
|
||||
|
||||
settings.ACE_ROUTING_KEY = 'edx.core.low'
|
||||
settings.ACE_ROUTING_KEY = ACE_ROUTING_KEY
|
||||
|
||||
settings.FEATURES['test_django_plugin'] = True
|
||||
|
||||
@@ -11,9 +11,12 @@ The following internal data structures are implemented:
|
||||
|
||||
|
||||
from copy import deepcopy
|
||||
from datetime import datetime
|
||||
from functools import partial
|
||||
from logging import getLogger
|
||||
|
||||
from dateutil.tz import tzlocal
|
||||
|
||||
from openedx.core.lib.graph_traversals import traverse_post_order, traverse_topologically
|
||||
|
||||
from .exceptions import TransformerException
|
||||
@@ -464,7 +467,8 @@ class BlockStructureBlockData(BlockStructure):
|
||||
not found.
|
||||
"""
|
||||
block_data = self._block_data_map.get(usage_key)
|
||||
return getattr(block_data, field_name, default) if block_data else default
|
||||
xblock_field = getattr(block_data, field_name, default) if block_data else default
|
||||
return self._make_datetime_field_compatible(xblock_field)
|
||||
|
||||
def override_xblock_field(self, usage_key, field_name, override_data):
|
||||
"""
|
||||
@@ -554,7 +558,8 @@ class BlockStructureBlockData(BlockStructure):
|
||||
transformer_data = self.get_transformer_block_data(usage_key, transformer)
|
||||
except KeyError:
|
||||
return default
|
||||
return getattr(transformer_data, key, default)
|
||||
field = getattr(transformer_data, key, default)
|
||||
return self._make_datetime_field_compatible(field)
|
||||
|
||||
def set_transformer_block_field(self, usage_key, transformer, key, value):
|
||||
"""
|
||||
@@ -763,6 +768,23 @@ class BlockStructureBlockData(BlockStructure):
|
||||
self._block_data_map[usage_key] = block_data
|
||||
return block_data
|
||||
|
||||
def _make_datetime_field_compatible(self, field):
|
||||
"""
|
||||
Creates a new datetime object to avoid issues occurring due to upgrading
|
||||
python-datetuil version from 2.4.0
|
||||
|
||||
More info: https://openedx.atlassian.net/browse/BOM-2245
|
||||
"""
|
||||
if isinstance(field, datetime):
|
||||
if isinstance(field.tzinfo, tzlocal) and not hasattr(field.tzinfo, '_hasdst'):
|
||||
|
||||
return datetime(
|
||||
year=field.year, month=field.month, day=field.day,
|
||||
hour=field.hour, minute=field.minute, second=field.second,
|
||||
tzinfo=tzlocal()
|
||||
)
|
||||
return field
|
||||
|
||||
|
||||
class BlockStructureModulestoreData(BlockStructureBlockData):
|
||||
"""
|
||||
|
||||
@@ -62,7 +62,7 @@ class CourseOverviewTestCase(CatalogIntegrationMixin, ModuleStoreTestCase, Cache
|
||||
None: None,
|
||||
}
|
||||
|
||||
COURSE_OVERVIEW_TABS = {'courseware', 'info', 'textbooks', 'discussion', 'wiki', 'progress', 'dates'}
|
||||
COURSE_OVERVIEW_TABS = {'courseware', 'textbooks', 'discussion', 'wiki', 'progress', 'dates'}
|
||||
|
||||
ENABLED_SIGNALS = ['course_deleted', 'course_published']
|
||||
|
||||
|
||||
@@ -426,6 +426,10 @@ def create_library(
|
||||
"""
|
||||
assert isinstance(collection_uuid, UUID)
|
||||
assert isinstance(org, Organization)
|
||||
assert not transaction.get_autocommit(), (
|
||||
"Call within a django.db.transaction.atomic block so that all created objects are rolled back on error."
|
||||
)
|
||||
|
||||
validate_unicode_slug(slug)
|
||||
# First, create the blockstore bundle:
|
||||
bundle = create_bundle(
|
||||
@@ -436,20 +440,16 @@ def create_library(
|
||||
)
|
||||
# Now create the library reference in our database:
|
||||
try:
|
||||
# Atomic transaction required because if this fails,
|
||||
# we need to delete the bundle in the exception handler.
|
||||
with transaction.atomic():
|
||||
ref = ContentLibrary.objects.create(
|
||||
org=org,
|
||||
slug=slug,
|
||||
type=library_type,
|
||||
bundle_uuid=bundle.uuid,
|
||||
allow_public_learning=allow_public_learning,
|
||||
allow_public_read=allow_public_read,
|
||||
license=library_license,
|
||||
)
|
||||
ref = ContentLibrary.objects.create(
|
||||
org=org,
|
||||
slug=slug,
|
||||
type=library_type,
|
||||
bundle_uuid=bundle.uuid,
|
||||
allow_public_learning=allow_public_learning,
|
||||
allow_public_read=allow_public_read,
|
||||
license=library_license,
|
||||
)
|
||||
except IntegrityError:
|
||||
delete_bundle(bundle.uuid)
|
||||
raise LibraryAlreadyExists(slug) # lint-amnesty, pylint: disable=raise-missing-from
|
||||
CONTENT_LIBRARY_CREATED.send(sender=None, library_key=ref.library_key)
|
||||
return ContentLibraryMetadata(
|
||||
|
||||
@@ -30,6 +30,7 @@ from openedx.core.djangoapps.content_libraries.tests.base import (
|
||||
)
|
||||
from openedx.core.djangoapps.content_libraries.constants import VIDEO, COMPLEX, PROBLEM, CC_4_BY, ALL_RIGHTS_RESERVED
|
||||
from openedx.core.djangolib.blockstore_cache import cache
|
||||
from openedx.core.lib import blockstore_api
|
||||
from common.djangoapps.student.tests.factories import UserFactory
|
||||
|
||||
|
||||
@@ -189,10 +190,22 @@ class ContentLibrariesTestMixin:
|
||||
You can't create a library with the same slug as an existing library,
|
||||
or an invalid slug.
|
||||
"""
|
||||
assert 0 == len(blockstore_api.get_bundles(text_search='some-slug'))
|
||||
self._create_library(slug="some-slug", title="Existing Library")
|
||||
self._create_library(slug="some-slug", title="Duplicate Library", expect_response=400)
|
||||
assert 1 == len(blockstore_api.get_bundles(text_search='some-slug'))
|
||||
|
||||
self._create_library(slug="Invalid Slug!", title="Library with Bad Slug", expect_response=400)
|
||||
# Try to create a library+bundle with a duplicate slug
|
||||
response = self._create_library(slug="some-slug", title="Duplicate Library", expect_response=400)
|
||||
assert response == {
|
||||
'slug': 'A library with that ID already exists.',
|
||||
}
|
||||
# The second bundle created with that slug is removed when the transaction rolls back.
|
||||
assert 1 == len(blockstore_api.get_bundles(text_search='some-slug'))
|
||||
|
||||
response = self._create_library(slug="Invalid Slug!", title="Library with Bad Slug", expect_response=400)
|
||||
assert response == {
|
||||
'slug': ['Enter a valid “slug” consisting of Unicode letters, numbers, underscores, or hyphens.'],
|
||||
}
|
||||
|
||||
@ddt.data(True, False)
|
||||
@patch("openedx.core.djangoapps.content_libraries.views.LibraryApiPagination.page_size", new=2)
|
||||
|
||||
@@ -5,7 +5,7 @@ import json
|
||||
from gettext import GNUTranslations
|
||||
|
||||
from completion.test_utils import CompletionWaffleTestMixin
|
||||
from django.db import connections
|
||||
from django.db import connections, transaction
|
||||
from django.test import LiveServerTestCase, TestCase
|
||||
from django.utils.text import slugify
|
||||
from organizations.models import Organization
|
||||
@@ -51,17 +51,18 @@ class ContentLibraryContentTestMixin:
|
||||
short_name="CL-TEST",
|
||||
)
|
||||
_, slug = self.id().rsplit('.', 1)
|
||||
self.library = library_api.create_library(
|
||||
collection_uuid=self.collection.uuid,
|
||||
library_type=COMPLEX,
|
||||
org=self.organization,
|
||||
slug=slugify(slug),
|
||||
title=(f"{slug} Test Lib"),
|
||||
description="",
|
||||
allow_public_learning=True,
|
||||
allow_public_read=False,
|
||||
library_license=ALL_RIGHTS_RESERVED,
|
||||
)
|
||||
with transaction.atomic():
|
||||
self.library = library_api.create_library(
|
||||
collection_uuid=self.collection.uuid,
|
||||
library_type=COMPLEX,
|
||||
org=self.organization,
|
||||
slug=slugify(slug),
|
||||
title=(f"{slug} Test Lib"),
|
||||
description="",
|
||||
allow_public_learning=True,
|
||||
allow_public_read=False,
|
||||
library_license=ALL_RIGHTS_RESERVED,
|
||||
)
|
||||
|
||||
|
||||
class ContentLibraryRuntimeTestMixin(ContentLibraryContentTestMixin):
|
||||
@@ -83,17 +84,18 @@ class ContentLibraryRuntimeTestMixin(ContentLibraryContentTestMixin):
|
||||
library_api.create_library_block_child(unit_block_key, "problem", "p1")
|
||||
library_api.publish_changes(self.library.key)
|
||||
# Now do the same in a different library:
|
||||
library2 = library_api.create_library(
|
||||
collection_uuid=self.collection.uuid,
|
||||
org=self.organization,
|
||||
slug="idolx",
|
||||
title=("Identical OLX Test Lib 2"),
|
||||
description="",
|
||||
library_type=COMPLEX,
|
||||
allow_public_learning=True,
|
||||
allow_public_read=False,
|
||||
library_license=CC_4_BY,
|
||||
)
|
||||
with transaction.atomic():
|
||||
library2 = library_api.create_library(
|
||||
collection_uuid=self.collection.uuid,
|
||||
org=self.organization,
|
||||
slug="idolx",
|
||||
title=("Identical OLX Test Lib 2"),
|
||||
description="",
|
||||
library_type=COMPLEX,
|
||||
allow_public_learning=True,
|
||||
allow_public_read=False,
|
||||
library_license=CC_4_BY,
|
||||
)
|
||||
unit_block2_key = library_api.create_library_block(library2.key, "unit", "u1").usage_key
|
||||
library_api.create_library_block_child(unit_block2_key, "problem", "p1")
|
||||
library_api.publish_changes(library2.key)
|
||||
|
||||
@@ -18,6 +18,7 @@ from django.contrib.auth import authenticate
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.contrib.auth import login
|
||||
from django.contrib.auth.models import Group
|
||||
from django.db.transaction import atomic
|
||||
from django.http import Http404
|
||||
from django.http import HttpResponseBadRequest
|
||||
from django.http import JsonResponse
|
||||
@@ -142,6 +143,7 @@ class LibraryRootView(APIView):
|
||||
Views to list, search for, and create content libraries.
|
||||
"""
|
||||
|
||||
@atomic
|
||||
@apidocs.schema(
|
||||
parameters=[
|
||||
*LibraryApiPagination.apidoc_params,
|
||||
@@ -184,6 +186,7 @@ class LibraryRootView(APIView):
|
||||
return paginator.get_paginated_response(serializer.data)
|
||||
return Response(serializer.data)
|
||||
|
||||
@atomic
|
||||
def post(self, request):
|
||||
"""
|
||||
Create a new content library.
|
||||
@@ -223,6 +226,7 @@ class LibraryDetailsView(APIView):
|
||||
"""
|
||||
Views to work with a specific content library
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def get(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -233,6 +237,7 @@ class LibraryDetailsView(APIView):
|
||||
result = api.get_library(key)
|
||||
return Response(ContentLibraryMetadataSerializer(result).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def patch(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -255,6 +260,7 @@ class LibraryDetailsView(APIView):
|
||||
result = api.get_library(key)
|
||||
return Response(ContentLibraryMetadataSerializer(result).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def delete(self, request, lib_key_str): # pylint: disable=unused-argument
|
||||
"""
|
||||
@@ -275,6 +281,7 @@ class LibraryTeamView(APIView):
|
||||
Note also the 'allow_public_' settings which can be edited by PATCHing the
|
||||
library itself (LibraryDetailsView.patch).
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def post(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -302,6 +309,7 @@ class LibraryTeamView(APIView):
|
||||
grant = api.get_library_user_permissions(key, user)
|
||||
return Response(ContentLibraryPermissionSerializer(grant).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def get(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -320,6 +328,7 @@ class LibraryTeamUserView(APIView):
|
||||
View to add/remove/edit an individual user's permissions for a content
|
||||
library.
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def put(self, request, lib_key_str, username):
|
||||
"""
|
||||
@@ -338,6 +347,7 @@ class LibraryTeamUserView(APIView):
|
||||
grant = api.get_library_user_permissions(key, user)
|
||||
return Response(ContentLibraryPermissionSerializer(grant).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def get(self, request, lib_key_str, username):
|
||||
"""
|
||||
@@ -351,6 +361,7 @@ class LibraryTeamUserView(APIView):
|
||||
raise NotFound
|
||||
return Response(ContentLibraryPermissionSerializer(grant).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def delete(self, request, lib_key_str, username):
|
||||
"""
|
||||
@@ -372,6 +383,7 @@ class LibraryTeamGroupView(APIView):
|
||||
"""
|
||||
View to add/remove/edit a group's permissions for a content library.
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def put(self, request, lib_key_str, group_name):
|
||||
"""
|
||||
@@ -386,6 +398,7 @@ class LibraryTeamGroupView(APIView):
|
||||
api.set_library_group_permissions(key, group, access_level=serializer.validated_data["access_level"])
|
||||
return Response({})
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def delete(self, request, lib_key_str, username):
|
||||
"""
|
||||
@@ -404,6 +417,7 @@ class LibraryBlockTypesView(APIView):
|
||||
"""
|
||||
View to get the list of XBlock types that can be added to this library
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def get(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -428,6 +442,7 @@ class LibraryLinksView(APIView):
|
||||
Links always point to a specific published version of the target bundle.
|
||||
Links are identified by a slug-like ID, e.g. "link1"
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def get(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -438,6 +453,7 @@ class LibraryLinksView(APIView):
|
||||
result = api.get_bundle_links(key)
|
||||
return Response(LibraryBundleLinkSerializer(result, many=True).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def post(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -462,6 +478,7 @@ class LibraryLinkDetailView(APIView):
|
||||
"""
|
||||
View to update/delete an existing library link
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def patch(self, request, lib_key_str, link_id):
|
||||
"""
|
||||
@@ -478,6 +495,7 @@ class LibraryLinkDetailView(APIView):
|
||||
api.update_bundle_link(key, link_id, version=serializer.validated_data['version'])
|
||||
return Response({})
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def delete(self, request, lib_key_str, link_id): # pylint: disable=unused-argument
|
||||
"""
|
||||
@@ -494,6 +512,7 @@ class LibraryCommitView(APIView):
|
||||
"""
|
||||
Commit/publish or revert all of the draft changes made to the library.
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def post(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -505,6 +524,7 @@ class LibraryCommitView(APIView):
|
||||
api.publish_changes(key)
|
||||
return Response({})
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def delete(self, request, lib_key_str): # pylint: disable=unused-argument
|
||||
"""
|
||||
@@ -522,6 +542,7 @@ class LibraryBlocksView(APIView):
|
||||
"""
|
||||
Views to work with XBlocks in a specific content library.
|
||||
"""
|
||||
@atomic
|
||||
@apidocs.schema(
|
||||
parameters=[
|
||||
*LibraryApiPagination.apidoc_params,
|
||||
@@ -560,6 +581,7 @@ class LibraryBlocksView(APIView):
|
||||
|
||||
return Response(LibraryXBlockMetadataSerializer(result, many=True).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def post(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -592,6 +614,7 @@ class LibraryBlockView(APIView):
|
||||
"""
|
||||
Views to work with an existing XBlock in a content library.
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def get(self, request, usage_key_str):
|
||||
"""
|
||||
@@ -602,6 +625,7 @@ class LibraryBlockView(APIView):
|
||||
result = api.get_library_block(key)
|
||||
return Response(LibraryXBlockMetadataSerializer(result).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def delete(self, request, usage_key_str): # pylint: disable=unused-argument
|
||||
"""
|
||||
@@ -628,6 +652,7 @@ class LibraryBlockLtiUrlView(APIView):
|
||||
|
||||
Returns 404 in case the block not found by the given key.
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def get(self, request, usage_key_str):
|
||||
"""
|
||||
@@ -647,6 +672,7 @@ class LibraryBlockOlxView(APIView):
|
||||
"""
|
||||
Views to work with an existing XBlock's OLX
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def get(self, request, usage_key_str):
|
||||
"""
|
||||
@@ -657,6 +683,7 @@ class LibraryBlockOlxView(APIView):
|
||||
xml_str = api.get_library_block_olx(key)
|
||||
return Response(LibraryXBlockOlxSerializer({"olx": xml_str}).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def post(self, request, usage_key_str):
|
||||
"""
|
||||
@@ -682,6 +709,7 @@ class LibraryBlockAssetListView(APIView):
|
||||
"""
|
||||
Views to list an existing XBlock's static asset files
|
||||
"""
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def get(self, request, usage_key_str):
|
||||
"""
|
||||
@@ -700,6 +728,7 @@ class LibraryBlockAssetView(APIView):
|
||||
"""
|
||||
parser_classes = (MultiPartParser, )
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def get(self, request, usage_key_str, file_path):
|
||||
"""
|
||||
@@ -713,6 +742,7 @@ class LibraryBlockAssetView(APIView):
|
||||
return Response(LibraryXBlockStaticFileSerializer(f).data)
|
||||
raise NotFound
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def put(self, request, usage_key_str, file_path):
|
||||
"""
|
||||
@@ -735,6 +765,7 @@ class LibraryBlockAssetView(APIView):
|
||||
raise ValidationError("Invalid file path") # lint-amnesty, pylint: disable=raise-missing-from
|
||||
return Response(LibraryXBlockStaticFileSerializer(result).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def delete(self, request, usage_key_str, file_path):
|
||||
"""
|
||||
@@ -757,6 +788,7 @@ class LibraryImportTaskViewSet(ViewSet):
|
||||
Import blocks from Courseware through modulestore.
|
||||
"""
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def list(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -775,6 +807,7 @@ class LibraryImportTaskViewSet(ViewSet):
|
||||
paginator.paginate_queryset(result, request)
|
||||
)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def create(self, request, lib_key_str):
|
||||
"""
|
||||
@@ -795,6 +828,7 @@ class LibraryImportTaskViewSet(ViewSet):
|
||||
import_task = api.import_blocks_create_task(library_key, course_key)
|
||||
return Response(ContentLibraryBlockImportTaskSerializer(import_task).data)
|
||||
|
||||
@atomic
|
||||
@convert_exceptions
|
||||
def retrieve(self, request, lib_key_str, pk=None):
|
||||
"""
|
||||
|
||||
@@ -13,7 +13,7 @@ from django.db.models.signals import pre_delete
|
||||
from django.dispatch import receiver
|
||||
|
||||
from opaque_keys.edx.django.models import CourseKeyField
|
||||
from openedx_filters.learning.filters import CohortChangeRequested
|
||||
from openedx_filters.learning.filters import CohortAssignmentRequested, CohortChangeRequested
|
||||
|
||||
from openedx.core.djangolib.model_mixins import DeletableByUserValue
|
||||
|
||||
@@ -31,6 +31,10 @@ class CohortChangeNotAllowed(CohortMembershipException):
|
||||
pass
|
||||
|
||||
|
||||
class CohortAssignmentNotAllowed(CohortMembershipException):
|
||||
pass
|
||||
|
||||
|
||||
class CourseUserGroup(models.Model):
|
||||
"""
|
||||
This model represents groups of users in a course. Groups may have different types,
|
||||
@@ -113,6 +117,13 @@ class CohortMembership(models.Model):
|
||||
cohort
|
||||
Returns CohortMembership, previous_cohort (if any)
|
||||
"""
|
||||
try:
|
||||
# .. filter_implemented_name: CohortAssignmentRequested
|
||||
# .. filter_type: org.openedx.learning.cohort.assignment.requested.v1
|
||||
user, cohort = CohortAssignmentRequested.run_filter(user=user, target_cohort=cohort)
|
||||
except CohortAssignmentRequested.PreventCohortAssignment as exc:
|
||||
raise CohortAssignmentNotAllowed(str(exc)) from exc
|
||||
|
||||
with transaction.atomic():
|
||||
membership, created = cls.objects.select_for_update().get_or_create(
|
||||
user__id=user.id,
|
||||
|
||||
@@ -3,12 +3,16 @@ Test that various filters are executed for models in the course_groups app.
|
||||
"""
|
||||
from django.test import override_settings
|
||||
from openedx_filters import PipelineStep
|
||||
from openedx_filters.learning.filters import CohortChangeRequested
|
||||
from openedx_filters.learning.filters import CohortAssignmentRequested, CohortChangeRequested
|
||||
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
|
||||
|
||||
from common.djangoapps.student.tests.factories import UserFactory
|
||||
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
|
||||
from openedx.core.djangoapps.course_groups.models import CohortChangeNotAllowed, CohortMembership
|
||||
from openedx.core.djangoapps.course_groups.models import (
|
||||
CohortAssignmentNotAllowed,
|
||||
CohortChangeNotAllowed,
|
||||
CohortMembership,
|
||||
)
|
||||
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
|
||||
@@ -31,6 +35,23 @@ class TestCohortChangeStep(PipelineStep):
|
||||
return {}
|
||||
|
||||
|
||||
class TestCohortAssignmentStep(PipelineStep):
|
||||
"""
|
||||
Utility function used when getting steps for pipeline.
|
||||
"""
|
||||
|
||||
def run_filter(self, user, target_cohort): # pylint: disable=arguments-differ
|
||||
"""Pipeline step that adds cohort info to the users profile."""
|
||||
user.profile.set_meta(
|
||||
{
|
||||
"cohort_info":
|
||||
f"User assigned to Cohort {str(target_cohort)}",
|
||||
}
|
||||
)
|
||||
user.profile.save()
|
||||
return {}
|
||||
|
||||
|
||||
class TestStopCohortChangeStep(PipelineStep):
|
||||
"""
|
||||
Utility function used when getting steps for pipeline.
|
||||
@@ -41,6 +62,16 @@ class TestStopCohortChangeStep(PipelineStep):
|
||||
raise CohortChangeRequested.PreventCohortChange("You can't change cohorts.")
|
||||
|
||||
|
||||
class TestStopAssignmentChangeStep(PipelineStep):
|
||||
"""
|
||||
Utility function used when getting steps for pipeline.
|
||||
"""
|
||||
|
||||
def run_filter(self, user, target_cohort, *args, **kwargs): # pylint: disable=arguments-differ
|
||||
"""Pipeline step that stops the cohort change process."""
|
||||
raise CohortAssignmentRequested.PreventCohortAssignment("You can't be assign to this cohort.")
|
||||
|
||||
|
||||
@skip_unless_lms
|
||||
class CohortFiltersTest(SharedModuleStoreTestCase):
|
||||
"""
|
||||
@@ -92,6 +123,33 @@ class CohortFiltersTest(SharedModuleStoreTestCase):
|
||||
cohort_membership.user.profile.get_meta(),
|
||||
)
|
||||
|
||||
@override_settings(
|
||||
OPEN_EDX_FILTERS_CONFIG={
|
||||
"org.openedx.learning.cohort.assignment.requested.v1": {
|
||||
"pipeline": [
|
||||
"openedx.core.djangoapps.course_groups.tests.test_filters.TestCohortAssignmentStep",
|
||||
],
|
||||
"fail_silently": False,
|
||||
},
|
||||
},
|
||||
)
|
||||
def test_cohort_assignment_filter_executed(self):
|
||||
"""
|
||||
Test whether the student cohort assignment filter is triggered before the user's
|
||||
assignment.
|
||||
|
||||
Expected result:
|
||||
- CohortAssignmentRequested is triggered and executes TestCohortAssignmentStep.
|
||||
- The user's profile meta contains cohort_info.
|
||||
"""
|
||||
|
||||
cohort_membership, _ = CohortMembership.assign(user=self.user, cohort=self.second_cohort, )
|
||||
|
||||
self.assertEqual(
|
||||
{"cohort_info": "User assigned to Cohort SecondCohort"},
|
||||
cohort_membership.user.profile.get_meta(),
|
||||
)
|
||||
|
||||
@override_settings(
|
||||
OPEN_EDX_FILTERS_CONFIG={
|
||||
"org.openedx.learning.cohort.change.requested.v1": {
|
||||
@@ -115,6 +173,27 @@ class CohortFiltersTest(SharedModuleStoreTestCase):
|
||||
with self.assertRaises(CohortChangeNotAllowed):
|
||||
CohortMembership.assign(cohort=self.second_cohort, user=self.user)
|
||||
|
||||
@override_settings(
|
||||
OPEN_EDX_FILTERS_CONFIG={
|
||||
"org.openedx.learning.cohort.assignment.requested.v1": {
|
||||
"pipeline": [
|
||||
"openedx.core.djangoapps.course_groups.tests.test_filters.TestStopAssignmentChangeStep",
|
||||
],
|
||||
"fail_silently": False,
|
||||
},
|
||||
},
|
||||
)
|
||||
def test_cohort_assignment_filter_prevent_move(self):
|
||||
"""
|
||||
Test prevent the user's cohort assignment through a pipeline step.
|
||||
|
||||
Expected result:
|
||||
- CohortAssignmentRequested is triggered and executes TestStopAssignmentChangeStep.
|
||||
- The user can't be assigned to the cohort.
|
||||
"""
|
||||
with self.assertRaises(CohortAssignmentNotAllowed):
|
||||
CohortMembership.assign(cohort=self.second_cohort, user=self.user)
|
||||
|
||||
@override_settings(OPEN_EDX_FILTERS_CONFIG={})
|
||||
def test_cohort_change_without_filter_configuration(self):
|
||||
"""
|
||||
@@ -129,3 +208,16 @@ class CohortFiltersTest(SharedModuleStoreTestCase):
|
||||
cohort_membership, _ = CohortMembership.assign(cohort=self.second_cohort, user=self.user)
|
||||
|
||||
self.assertEqual({}, cohort_membership.user.profile.get_meta())
|
||||
|
||||
@override_settings(OPEN_EDX_FILTERS_CONFIG={})
|
||||
def test_cohort_assignment_without_filter_configuration(self):
|
||||
"""
|
||||
Test usual cohort assignment process, without filter's intervention.
|
||||
|
||||
Expected result:
|
||||
- CohortAssignmentRequested does not have any effect on the cohort change process.
|
||||
- The cohort assignment process ends successfully.
|
||||
"""
|
||||
cohort_membership, _ = CohortMembership.assign(cohort=self.second_cohort, user=self.user)
|
||||
|
||||
self.assertEqual({}, cohort_membership.user.profile.get_meta())
|
||||
|
||||
@@ -26,7 +26,11 @@ from rest_framework.serializers import Serializer
|
||||
|
||||
from lms.djangoapps.courseware.courses import get_course, get_course_with_access
|
||||
from common.djangoapps.edxmako.shortcuts import render_to_response
|
||||
from openedx.core.djangoapps.course_groups.models import CohortMembership
|
||||
from openedx.core.djangoapps.course_groups.models import (
|
||||
CohortAssignmentNotAllowed,
|
||||
CohortChangeNotAllowed,
|
||||
CohortMembership,
|
||||
)
|
||||
from openedx.core.djangoapps.course_groups.permissions import IsStaffOrAdmin
|
||||
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
|
||||
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin
|
||||
@@ -321,6 +325,7 @@ def add_users_to_cohort(request, course_key_string, cohort_id):
|
||||
unknown = []
|
||||
preassigned = []
|
||||
invalid = []
|
||||
not_allowed = []
|
||||
for username_or_email in split_by_comma_and_whitespace(users):
|
||||
if not username_or_email:
|
||||
continue
|
||||
@@ -346,6 +351,8 @@ def add_users_to_cohort(request, course_key_string, cohort_id):
|
||||
invalid.append(username_or_email)
|
||||
except ValueError:
|
||||
present.append(username_or_email)
|
||||
except (CohortAssignmentNotAllowed, CohortChangeNotAllowed):
|
||||
not_allowed.append(username_or_email)
|
||||
|
||||
return json_http_response({'success': True,
|
||||
'added': added,
|
||||
@@ -353,7 +360,8 @@ def add_users_to_cohort(request, course_key_string, cohort_id):
|
||||
'present': present,
|
||||
'unknown': unknown,
|
||||
'preassigned': preassigned,
|
||||
'invalid': invalid})
|
||||
'invalid': invalid,
|
||||
'not_allowed': not_allowed})
|
||||
|
||||
|
||||
@ensure_csrf_cookie
|
||||
|
||||
@@ -4,8 +4,8 @@ Configurations to render Course Live Tab
|
||||
from django.utils.translation import gettext_lazy
|
||||
from lti_consumer.models import LtiConfiguration
|
||||
|
||||
from common.lib.xmodule.xmodule.course_module import CourseBlock
|
||||
from common.lib.xmodule.xmodule.tabs import TabFragmentViewMixin
|
||||
from xmodule.course_module import CourseBlock
|
||||
from xmodule.tabs import TabFragmentViewMixin
|
||||
from lms.djangoapps.courseware.tabs import EnrolledTab
|
||||
from openedx.core.djangoapps.course_live.config.waffle import ENABLE_COURSE_LIVE
|
||||
from openedx.core.djangoapps.course_live.models import CourseLiveConfiguration
|
||||
|
||||
@@ -5,11 +5,12 @@ from django.core.exceptions import ValidationError
|
||||
from lti_consumer.api import get_lti_pii_sharing_state_for_course
|
||||
from lti_consumer.models import LtiConfiguration
|
||||
from rest_framework import serializers
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
from lms.djangoapps.discussion.toggles import ENABLE_REPORTED_CONTENT_EMAIL_NOTIFICATIONS
|
||||
from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task
|
||||
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
|
||||
from openedx.core.lib.courses import get_course_by_id
|
||||
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
|
||||
from .models import DiscussionsConfiguration, Provider
|
||||
from .utils import available_division_schemes, get_divided_discussions
|
||||
|
||||
@@ -257,6 +258,7 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
|
||||
# have already been set
|
||||
instance = self._update_lti(instance, validated_data)
|
||||
instance.save()
|
||||
update_discussions_settings_from_course_task.delay(str(instance.context_key))
|
||||
return instance
|
||||
|
||||
def _update_lti(
|
||||
|
||||
@@ -236,17 +236,6 @@ Configuration Flags
|
||||
Configuring Schedule Creation
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
Self-paced Configuration
|
||||
^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
Schedules will only be created for a course if it is self-paced. A
|
||||
course can be configured to be self-paced by going to
|
||||
``<studio_url>/admin/self_paced/selfpacedconfiguration/`` and adding an
|
||||
enabled self paced config. Then, go to Studio settings for the course
|
||||
and change the Course Pacing value to “Self-Paced”. Note that the Course
|
||||
Start Date has to be set to sometime in the future in order to change
|
||||
the Course Pacing.
|
||||
|
||||
Configuring Upgrade Deadline on Schedule
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
|
||||
@@ -252,7 +252,7 @@ class TestCourseNextSectionUpdateResolver(SchedulesResolverTestMixin, ModuleStor
|
||||
def test_schedule_context(self):
|
||||
resolver = self.create_resolver()
|
||||
# using this to make sure the select_related stays intact
|
||||
with self.assertNumQueries(41):
|
||||
with self.assertNumQueries(38):
|
||||
sc = resolver.get_schedules()
|
||||
schedules = list(sc)
|
||||
|
||||
|
||||
@@ -1,11 +0,0 @@
|
||||
"""
|
||||
Admin site bindings for self-paced courses.
|
||||
"""
|
||||
|
||||
|
||||
from config_models.admin import ConfigurationModelAdmin
|
||||
from django.contrib import admin
|
||||
|
||||
from .models import SelfPacedConfiguration
|
||||
|
||||
admin.site.register(SelfPacedConfiguration, ConfigurationModelAdmin)
|
||||
@@ -1,27 +0,0 @@
|
||||
from django.db import migrations, models
|
||||
import django.db.models.deletion
|
||||
from django.conf import settings
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.CreateModel(
|
||||
name='SelfPacedConfiguration',
|
||||
fields=[
|
||||
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
|
||||
('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')),
|
||||
('enabled', models.BooleanField(default=False, verbose_name='Enabled')),
|
||||
('enable_course_home_improvements', models.BooleanField(default=False, verbose_name='Enable course home page improvements.')),
|
||||
('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')),
|
||||
],
|
||||
options={
|
||||
'ordering': ('-change_date',),
|
||||
'abstract': False,
|
||||
},
|
||||
),
|
||||
]
|
||||
@@ -1,21 +0,0 @@
|
||||
"""
|
||||
Configuration for self-paced courses.
|
||||
"""
|
||||
|
||||
|
||||
from config_models.models import ConfigurationModel
|
||||
from django.db.models import BooleanField
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
|
||||
|
||||
class SelfPacedConfiguration(ConfigurationModel):
|
||||
"""
|
||||
Configuration for self-paced courses.
|
||||
|
||||
.. no_pii:
|
||||
"""
|
||||
|
||||
enable_course_home_improvements = BooleanField(
|
||||
default=False,
|
||||
verbose_name=_("Enable course home page improvements.")
|
||||
)
|
||||
@@ -70,6 +70,7 @@ class Command(BaseCommand):
|
||||
def handle(self, *args, **options):
|
||||
site_id = options.get('site_id')
|
||||
domain = options.get('domain')
|
||||
name = domain
|
||||
configuration = options.get('configuration')
|
||||
config_file_data = options.get('config_file_data')
|
||||
|
||||
@@ -78,9 +79,18 @@ class Command(BaseCommand):
|
||||
if site_id is not None:
|
||||
site, created = Site.objects.get_or_create(id=site_id)
|
||||
else:
|
||||
name_max_length = Site._meta.get_field("name").max_length
|
||||
if name:
|
||||
if len(str(name)) > name_max_length:
|
||||
LOG.warning(
|
||||
f"The name {name} is too long, truncating to {name_max_length}"
|
||||
" characters. Please update site name in admin."
|
||||
)
|
||||
# trim name as the column has a limit of 50 characters
|
||||
name = name[:name_max_length]
|
||||
site, created = Site.objects.get_or_create(
|
||||
domain=domain,
|
||||
name=domain,
|
||||
name=name,
|
||||
)
|
||||
if created:
|
||||
LOG.info(f"Site does not exist. Created new site '{site.domain}'")
|
||||
|
||||
@@ -107,6 +107,19 @@ class CreateOrUpdateSiteConfigurationTest(TestCase):
|
||||
assert not site_configuration.site_values
|
||||
assert not site_configuration.enabled
|
||||
|
||||
def test_site_created_when_domain_longer_than_50_characters(self):
|
||||
"""
|
||||
Verify that a SiteConfiguration instance is created with name trimmed
|
||||
to 50 characters when domain is longer than 50 characters
|
||||
"""
|
||||
self.assert_site_configuration_does_not_exist()
|
||||
|
||||
domain = "studio.newtestserverwithlongname.development.opencraft.hosting"
|
||||
call_command(self.command, f"{domain}")
|
||||
site = Site.objects.filter(domain=domain)
|
||||
assert site.exists()
|
||||
assert site[0].name == domain[:50]
|
||||
|
||||
def test_both_enabled_disabled_flags(self):
|
||||
"""
|
||||
Verify the error on providing both the --enabled and --disabled flags.
|
||||
|
||||
@@ -124,8 +124,8 @@ class TestComprehensiveThemeLMS(TestCase):
|
||||
courses_url = reverse('courses')
|
||||
resp = self.client.get(courses_url)
|
||||
assert resp.status_code == 200
|
||||
# The courses.html template includes the info.html file, which is overriden in the theme.
|
||||
self.assertContains(resp, "This overrides the courseware/info.html template.")
|
||||
# The courses.html template includes the progress.html file, which is overriden in the theme.
|
||||
self.assertContains(resp, "This overrides the courseware/progress.html template.")
|
||||
|
||||
@with_comprehensive_theme("test-theme")
|
||||
def test_include_custom_template(self):
|
||||
|
||||
@@ -98,6 +98,7 @@ REQUIRED_FIELD_NAME_MSG = _("Enter your full name.")
|
||||
REQUIRED_FIELD_FIRST_NAME_MSG = _("Enter your first name.")
|
||||
REQUIRED_FIELD_LAST_NAME_MSG = _("Enter your last name.")
|
||||
REQUIRED_FIELD_CONFIRM_EMAIL_MSG = _("The email addresses do not match.")
|
||||
REQUIRED_FIELD_CONFIRM_EMAIL_TEXT_MSG = _("Enter your confirm email")
|
||||
REQUIRED_FIELD_COUNTRY_MSG = _("Select your country or region of residence.")
|
||||
REQUIRED_FIELD_PROFESSION_SELECT_MSG = _("Select your profession.")
|
||||
REQUIRED_FIELD_SPECIALTY_SELECT_MSG = _("Select your specialty.")
|
||||
|
||||
@@ -5,6 +5,7 @@ Programmatic integration point for User API Accounts sub-application
|
||||
|
||||
|
||||
import datetime
|
||||
import re
|
||||
|
||||
from django.conf import settings
|
||||
from django.core.exceptions import ObjectDoesNotExist
|
||||
@@ -403,7 +404,11 @@ def get_name_validation_error(name):
|
||||
:return: Validation error message.
|
||||
|
||||
"""
|
||||
return '' if name else accounts.REQUIRED_FIELD_NAME_MSG
|
||||
if name:
|
||||
regex = re.findall(r'https|http?://(?:[-\w.]|(?:%[\da-fA-F]{2}))+', name)
|
||||
return _('Enter a valid name') if bool(regex) else ''
|
||||
else:
|
||||
return accounts.REQUIRED_FIELD_NAME_MSG
|
||||
|
||||
|
||||
def get_username_validation_error(username):
|
||||
|
||||
@@ -347,3 +347,20 @@ def add_country_field(is_field_required=False):
|
||||
empty
|
||||
"""
|
||||
return {'name': 'country', 'error_message': is_field_required}
|
||||
|
||||
|
||||
def add_confirm_email_field(is_field_required=False):
|
||||
"""
|
||||
Returns a email confirmation field description
|
||||
"""
|
||||
# Translators: This label appears above a field on the registration form
|
||||
# meant to confirm the user's email address.
|
||||
|
||||
email_label = _("Confirm Email")
|
||||
|
||||
return {
|
||||
'name': 'confirm_email',
|
||||
'type': SUPPORTED_FIELDS_TYPES['TEXT'],
|
||||
'label': email_label,
|
||||
'error_message': accounts.REQUIRED_FIELD_CONFIRM_EMAIL_TEXT_MSG if is_field_required else '',
|
||||
}
|
||||
|
||||
@@ -15,8 +15,6 @@ class RegistrationFieldsContext(APIView):
|
||||
"""
|
||||
Registration Fields View used by optional and required fields view.
|
||||
"""
|
||||
# allow to define custom set of required/optional/hidden fields via configuration
|
||||
FIELD_TYPE = 'hidden'
|
||||
|
||||
EXTRA_FIELDS = [
|
||||
'confirm_email',
|
||||
@@ -58,8 +56,9 @@ class RegistrationFieldsContext(APIView):
|
||||
|
||||
return field_order
|
||||
|
||||
def __init__(self):
|
||||
def __init__(self, field_type='required'):
|
||||
super().__init__()
|
||||
self.field_type = field_type
|
||||
self._fields_setting = copy.deepcopy(configuration_helpers.get_value('REGISTRATION_EXTRA_FIELDS'))
|
||||
if not self._fields_setting:
|
||||
self._fields_setting = copy.deepcopy(settings.REGISTRATION_EXTRA_FIELDS)
|
||||
@@ -70,18 +69,18 @@ class RegistrationFieldsContext(APIView):
|
||||
ordered_extra_fields.remove('year_of_birth')
|
||||
|
||||
self.valid_fields = [
|
||||
field for field in ordered_extra_fields if self._fields_setting.get(field) == self.FIELD_TYPE
|
||||
field for field in ordered_extra_fields if self._fields_setting.get(field) == self.field_type
|
||||
]
|
||||
|
||||
custom_form = get_registration_extension_form()
|
||||
if custom_form:
|
||||
for field_name, field in custom_form.fields.items():
|
||||
# If the FIELD_TYPE is required make sure the custom field is required in the form and if the
|
||||
# FIELD_TYPE is optional only add field if it is not required. This is to make sure field is
|
||||
# If the field_type is required make sure the custom field is required in the form and if the
|
||||
# field_type is optional only add field if it is not required. This is to make sure field is
|
||||
# added only once either on Registration Page or Progressive Profiling page.
|
||||
if (
|
||||
field.required and self.FIELD_TYPE == 'required' or
|
||||
not field.required and self.FIELD_TYPE == 'optional'
|
||||
field.required and self.field_type == 'required' or
|
||||
not field.required and self.field_type == 'optional'
|
||||
):
|
||||
self.valid_fields.append(field_name)
|
||||
|
||||
@@ -95,11 +94,15 @@ class RegistrationFieldsContext(APIView):
|
||||
for field in self.valid_fields:
|
||||
if custom_form and field in custom_form.fields:
|
||||
response[field] = form_fields.add_extension_form_field(
|
||||
field, custom_form, custom_form.fields[field], self.FIELD_TYPE
|
||||
field, custom_form, custom_form.fields[field], self.field_type
|
||||
)
|
||||
else:
|
||||
field_handler = getattr(form_fields, f'add_{field}_field', None)
|
||||
if field_handler:
|
||||
response[field] = field_handler(self.FIELD_TYPE == 'required')
|
||||
if field == 'confirm_email':
|
||||
if self.field_type == 'required':
|
||||
response[field] = field_handler(self.field_type == 'required')
|
||||
else:
|
||||
response[field] = field_handler(self.field_type == 'required')
|
||||
|
||||
return response
|
||||
|
||||
@@ -1,106 +0,0 @@
|
||||
"""
|
||||
Tests for OptionalFieldsData View
|
||||
"""
|
||||
from django.conf import settings
|
||||
from django.test.utils import override_settings
|
||||
from django.urls import reverse
|
||||
from rest_framework import status
|
||||
from rest_framework.test import APITestCase
|
||||
|
||||
from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
from common.djangoapps.student.tests.factories import UserFactory
|
||||
|
||||
|
||||
@skip_unless_lms
|
||||
class OptionalFieldsDataViewTest(APITestCase):
|
||||
"""
|
||||
Tests for the end-point that returns optional fields.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
|
||||
self.user = UserFactory.create(username='test_user', password='password123')
|
||||
self.client.force_authenticate(user=self.user)
|
||||
self.url = reverse('optional_fields')
|
||||
|
||||
def test_unauthenticated_request_is_forbidden(self):
|
||||
"""
|
||||
Test that unauthenticated user should not be able to access the endpoint.
|
||||
"""
|
||||
self.client.logout()
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == status.HTTP_401_UNAUTHORIZED
|
||||
|
||||
@override_settings(REGISTRATION_EXTRA_FIELDS={"goals": "required", "level_of_education": "required"})
|
||||
def test_optional_fields_not_configured(self):
|
||||
"""
|
||||
Test that when no optional fields are configured in REGISTRATION_EXTRA_FIELDS
|
||||
settings, then API returns proper response.
|
||||
"""
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
||||
assert response.data.get('error_code') == 'optional_fields_configured_incorrectly'
|
||||
|
||||
@override_settings(REGISTRATION_EXTRA_FIELDS={"new_field_with_no_description": "optional", "goals": "optional"})
|
||||
def test_optional_field_has_no_description(self):
|
||||
"""
|
||||
Test that if a new optional field is added to REGISTRATION_EXTRA_FIELDS without
|
||||
adding field description then that field is omitted from the final response.
|
||||
"""
|
||||
expected_response = {
|
||||
'goals': {
|
||||
'name': 'goals',
|
||||
'type': 'textarea',
|
||||
'label': "Tell us why you're interested in {platform_name}".format(
|
||||
platform_name=settings.PLATFORM_NAME
|
||||
),
|
||||
'error_message': '',
|
||||
}
|
||||
}
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert response.data.get('fields') == expected_response
|
||||
|
||||
@with_site_configuration(
|
||||
configuration={
|
||||
'EXTRA_FIELD_OPTIONS': {'profession': ['Software Engineer', 'Teacher', 'Other']}
|
||||
}
|
||||
)
|
||||
@override_settings(REGISTRATION_EXTRA_FIELDS={'profession': 'optional', 'specialty': 'optional'})
|
||||
def test_configurable_select_option_fields(self):
|
||||
"""
|
||||
Test that if optional fields have configurable options present in EXTRA_FIELD_OPTIONS,
|
||||
they are returned in response as "select" fields otherwise as "text" field.
|
||||
"""
|
||||
expected_response = {
|
||||
'profession': {
|
||||
'name': 'profession',
|
||||
'label': 'Profession',
|
||||
'error_message': '',
|
||||
'type': 'select',
|
||||
'options': [('software engineer', 'Software Engineer'), ('teacher', 'Teacher'), ('other', 'Other')],
|
||||
},
|
||||
'specialty': {
|
||||
'name': 'specialty',
|
||||
'label': 'Specialty',
|
||||
'error_message': '',
|
||||
'type': 'text',
|
||||
}
|
||||
}
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert response.data.get('fields') == expected_response
|
||||
|
||||
@override_settings(
|
||||
REGISTRATION_EXTRA_FIELDS={'goals': 'optional', 'specialty': 'optional'},
|
||||
REGISTRATION_FIELD_ORDER=['specialty', 'goals'],
|
||||
)
|
||||
def test_field_order(self):
|
||||
"""
|
||||
Test that order of fields
|
||||
"""
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert list(response.data['fields'].keys()) == ['specialty', 'goals']
|
||||
@@ -16,6 +16,7 @@ from common.djangoapps.student.models import Registration
|
||||
from common.djangoapps.student.tests.factories import UserFactory
|
||||
from common.djangoapps.third_party_auth import pipeline
|
||||
from common.djangoapps.third_party_auth.tests.testutil import ThirdPartyAuthTestMixin, simulate_running_pipeline
|
||||
from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration
|
||||
from openedx.core.djangoapps.geoinfo.api import country_code_from_ip
|
||||
from openedx.core.djangoapps.user_api.tests.test_views import UserAPITestCase
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
@@ -34,6 +35,7 @@ class MFEContextViewTest(ThirdPartyAuthTestMixin, APITestCase):
|
||||
"""
|
||||
super().setUp()
|
||||
|
||||
self.user = UserFactory.create(username='test_user', password='password123')
|
||||
self.url = reverse('mfe_context')
|
||||
self.query_params = {'next': '/dashboard'}
|
||||
|
||||
@@ -103,6 +105,7 @@ class MFEContextViewTest(ThirdPartyAuthTestMixin, APITestCase):
|
||||
'countryCode': self.country_code
|
||||
},
|
||||
'registration_fields': {},
|
||||
'optional_fields': {},
|
||||
}
|
||||
|
||||
@patch.dict(settings.FEATURES, {'ENABLE_THIRD_PARTY_AUTH': False})
|
||||
@@ -194,7 +197,8 @@ class MFEContextViewTest(ThirdPartyAuthTestMixin, APITestCase):
|
||||
Test that when no required fields are configured in REGISTRATION_EXTRA_FIELDS
|
||||
settings, then API returns proper response.
|
||||
"""
|
||||
response = self.client.get(self.url)
|
||||
self.query_params.update({'is_registered': True})
|
||||
response = self.client.get(self.url, self.query_params)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert response.data['registration_fields']['fields'] == {}
|
||||
|
||||
@@ -203,13 +207,83 @@ class MFEContextViewTest(ThirdPartyAuthTestMixin, APITestCase):
|
||||
REGISTRATION_EXTRA_FIELDS={'state': 'required', 'last_name': 'required', 'first_name': 'required'},
|
||||
REGISTRATION_FIELD_ORDER=['first_name', 'last_name', 'state'],
|
||||
)
|
||||
def test_field_order(self):
|
||||
def test_required_field_order(self):
|
||||
"""
|
||||
Test that order of fields
|
||||
Test that order of required fields
|
||||
"""
|
||||
self.query_params.update({'is_registered': True})
|
||||
response = self.client.get(self.url, self.query_params)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert list(response.data['registration_fields']['fields'].keys()) == ['first_name', 'last_name', 'state']
|
||||
|
||||
@override_settings(
|
||||
ENABLE_DYNAMIC_REGISTRATION_FIELDS=True,
|
||||
REGISTRATION_EXTRA_FIELDS={"new_field_with_no_description": "optional", "goals": "optional"}
|
||||
)
|
||||
def test_optional_field_has_no_description(self):
|
||||
"""
|
||||
Test that if a new optional field is added to REGISTRATION_EXTRA_FIELDS without
|
||||
adding field description then that field is omitted from the final response.
|
||||
"""
|
||||
expected_response = {
|
||||
'goals': {
|
||||
'name': 'goals',
|
||||
'type': 'textarea',
|
||||
'label': "Tell us why you're interested in {platform_name}".format(
|
||||
platform_name=settings.PLATFORM_NAME
|
||||
),
|
||||
'error_message': '',
|
||||
}
|
||||
}
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert response.data['optional_fields']['fields'] == expected_response
|
||||
|
||||
@with_site_configuration(
|
||||
configuration={
|
||||
'EXTRA_FIELD_OPTIONS': {'profession': ['Software Engineer', 'Teacher', 'Other']}
|
||||
}
|
||||
)
|
||||
@override_settings(
|
||||
ENABLE_DYNAMIC_REGISTRATION_FIELDS=True,
|
||||
REGISTRATION_EXTRA_FIELDS={'profession': 'optional', 'specialty': 'optional'}
|
||||
)
|
||||
def test_configurable_select_option_fields(self):
|
||||
"""
|
||||
Test that if optional fields have configurable options present in EXTRA_FIELD_OPTIONS,
|
||||
they are returned in response as "select" fields otherwise as "text" field.
|
||||
"""
|
||||
expected_response = {
|
||||
'profession': {
|
||||
'name': 'profession',
|
||||
'label': 'Profession',
|
||||
'error_message': '',
|
||||
'type': 'select',
|
||||
'options': [('software engineer', 'Software Engineer'), ('teacher', 'Teacher'), ('other', 'Other')],
|
||||
},
|
||||
'specialty': {
|
||||
'name': 'specialty',
|
||||
'label': 'Specialty',
|
||||
'error_message': '',
|
||||
'type': 'text',
|
||||
}
|
||||
}
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert response.data['optional_fields']['fields'] == expected_response
|
||||
|
||||
@override_settings(
|
||||
ENABLE_DYNAMIC_REGISTRATION_FIELDS=True,
|
||||
REGISTRATION_EXTRA_FIELDS={'goals': 'optional', 'specialty': 'optional'},
|
||||
REGISTRATION_FIELD_ORDER=['specialty', 'goals'],
|
||||
)
|
||||
def test_optional_field_order(self):
|
||||
"""
|
||||
Test that order of optional fields
|
||||
"""
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert list(response.data['registration_fields']['fields'].keys()) == ['first_name', 'last_name', 'state']
|
||||
assert list(response.data['optional_fields']['fields'].keys()) == ['specialty', 'goals']
|
||||
|
||||
|
||||
@skip_unless_lms
|
||||
|
||||
@@ -5,7 +5,6 @@ from django.urls import path
|
||||
from openedx.core.djangoapps.user_authn.api.views import (
|
||||
MFEContextView,
|
||||
SendAccountActivationEmail,
|
||||
OptionalFieldsView,
|
||||
)
|
||||
urlpatterns = [
|
||||
path('third_party_auth_context', MFEContextView.as_view(), name='third_party_auth_context'),
|
||||
@@ -13,5 +12,4 @@ urlpatterns = [
|
||||
path('send_account_activation_email', SendAccountActivationEmail.as_view(),
|
||||
name='send_account_activation_email'
|
||||
),
|
||||
path('optional_fields', OptionalFieldsView.as_view(), name='optional_fields'),
|
||||
]
|
||||
|
||||
@@ -4,12 +4,10 @@ Authn API Views
|
||||
|
||||
from django.conf import settings
|
||||
|
||||
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
|
||||
from rest_framework import status
|
||||
from rest_framework.response import Response
|
||||
from rest_framework.throttling import AnonRateThrottle, UserRateThrottle
|
||||
from rest_framework.throttling import AnonRateThrottle
|
||||
from rest_framework.views import APIView
|
||||
from rest_framework.authentication import SessionAuthentication
|
||||
from rest_framework.permissions import IsAuthenticated
|
||||
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
|
||||
|
||||
@@ -18,7 +16,6 @@ from common.djangoapps.student.views import compose_and_send_activation_email
|
||||
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
|
||||
from openedx.core.djangoapps.user_authn.api.helper import RegistrationFieldsContext
|
||||
from openedx.core.djangoapps.user_authn.views.utils import get_mfe_context
|
||||
from openedx.core.lib.api.authentication import BearerAuthentication
|
||||
|
||||
|
||||
class MFEContextThrottle(AnonRateThrottle):
|
||||
@@ -28,17 +25,18 @@ class MFEContextThrottle(AnonRateThrottle):
|
||||
rate = settings.LOGISTRATION_API_RATELIMIT
|
||||
|
||||
|
||||
class MFEContextView(RegistrationFieldsContext):
|
||||
class MFEContextView(APIView):
|
||||
"""
|
||||
API to get third party auth providers, user country code and the currently running pipeline.
|
||||
API to get third party auth providers, optional fields, required fields,
|
||||
user country code and the currently running pipeline.
|
||||
"""
|
||||
FIELD_TYPE = 'required'
|
||||
throttle_classes = [MFEContextThrottle]
|
||||
|
||||
def get(self, request, **kwargs): # lint-amnesty, pylint: disable=unused-argument
|
||||
"""
|
||||
Returns
|
||||
- dynamic registration fields
|
||||
- dynamic optional fields
|
||||
- the context for third party auth providers
|
||||
- user country code
|
||||
- the currently running pipeline.
|
||||
@@ -48,22 +46,31 @@ class MFEContextView(RegistrationFieldsContext):
|
||||
is currently running.
|
||||
tpa_hint (string): An override flag that will return a matching provider
|
||||
as long as its configuration has been enabled
|
||||
is_register_page (boolen): Determine the call is from register or login page
|
||||
"""
|
||||
request_params = request.GET
|
||||
redirect_to = get_next_url_for_login_page(request)
|
||||
third_party_auth_hint = request_params.get('tpa_hint')
|
||||
|
||||
is_register_page = request_params.get('is_registered')
|
||||
context = {
|
||||
'context_data': get_mfe_context(request, redirect_to, third_party_auth_hint),
|
||||
'registration_fields': {},
|
||||
'optional_fields': {},
|
||||
}
|
||||
|
||||
if settings.ENABLE_DYNAMIC_REGISTRATION_FIELDS:
|
||||
registration_fields = self._get_fields()
|
||||
context['registration_fields'].update({
|
||||
'fields': registration_fields,
|
||||
'extended_profile': configuration_helpers.get_value('extended_profile_fields', []),
|
||||
})
|
||||
if is_register_page:
|
||||
registration_fields = RegistrationFieldsContext()._get_fields() # pylint: disable=protected-access
|
||||
context['registration_fields'].update({
|
||||
'fields': registration_fields,
|
||||
'extended_profile': configuration_helpers.get_value('extended_profile_fields', []),
|
||||
})
|
||||
optional_fields = RegistrationFieldsContext('optional')._get_fields() # pylint: disable=protected-access
|
||||
if optional_fields:
|
||||
context['optional_fields'].update({
|
||||
'fields': optional_fields,
|
||||
'extended_profile': configuration_helpers.get_value('extended_profile_fields', []),
|
||||
})
|
||||
|
||||
return Response(
|
||||
status=status.HTTP_200_OK,
|
||||
@@ -95,40 +102,3 @@ class SendAccountActivationEmail(APIView):
|
||||
return Response(
|
||||
status=status.HTTP_500_INTERNAL_SERVER_ERROR
|
||||
)
|
||||
|
||||
|
||||
class OptionalFieldsThrottle(UserRateThrottle):
|
||||
"""
|
||||
Setting rate limit for OptionalFieldsData API
|
||||
"""
|
||||
rate = settings.OPTIONAL_FIELD_API_RATELIMIT
|
||||
|
||||
|
||||
class OptionalFieldsView(RegistrationFieldsContext):
|
||||
"""
|
||||
Construct Registration forms and associated fields.
|
||||
"""
|
||||
FIELD_TYPE = 'optional'
|
||||
|
||||
throttle_classes = [OptionalFieldsThrottle]
|
||||
authentication_classes = (JwtAuthentication, BearerAuthentication, SessionAuthentication,)
|
||||
permission_classes = (IsAuthenticated,)
|
||||
|
||||
def get(self, request): # lint-amnesty, pylint: disable=unused-argument
|
||||
"""
|
||||
Returns Optional fields to be shown on Progressive Profiling page
|
||||
"""
|
||||
response = self._get_fields()
|
||||
if not self.valid_fields or not response:
|
||||
return Response(
|
||||
status=status.HTTP_400_BAD_REQUEST,
|
||||
data={'error_code': f'{self.FIELD_TYPE}_fields_configured_incorrectly'}
|
||||
)
|
||||
|
||||
return Response(
|
||||
status=status.HTTP_200_OK,
|
||||
data={
|
||||
'fields': response,
|
||||
'extended_profile': configuration_helpers.get_value('extended_profile_fields', []),
|
||||
},
|
||||
)
|
||||
|
||||
@@ -350,6 +350,7 @@ def _link_user_to_third_party_provider(
|
||||
def _track_user_registration(user, profile, params, third_party_provider, registration):
|
||||
""" Track the user's registration. """
|
||||
if hasattr(settings, 'LMS_SEGMENT_KEY') and settings.LMS_SEGMENT_KEY:
|
||||
is_marketable = params.get('marketing_emails_opt_in') in ['true', '1']
|
||||
traits = {
|
||||
'email': user.email,
|
||||
'username': user.username,
|
||||
@@ -361,10 +362,10 @@ def _track_user_registration(user, profile, params, third_party_provider, regist
|
||||
'address': profile.mailing_address,
|
||||
'gender': profile.gender_display,
|
||||
'country': str(profile.country),
|
||||
'is_marketable': params.get('marketing_emails_opt_in') == 'true'
|
||||
'is_marketable': is_marketable
|
||||
}
|
||||
if settings.MARKETING_EMAILS_OPT_IN and params.get('marketing_emails_opt_in'):
|
||||
email_subscribe = 'subscribed' if params.get('marketing_emails_opt_in') == 'true' else 'unsubscribed'
|
||||
email_subscribe = 'subscribed' if is_marketable else 'unsubscribed'
|
||||
traits['email_subscribe'] = email_subscribe
|
||||
|
||||
# .. pii: Many pieces of PII are sent to Segment here. Retired directly through Segment API call in Tubular.
|
||||
@@ -386,7 +387,7 @@ def _track_user_registration(user, profile, params, third_party_provider, regist
|
||||
}
|
||||
# VAN-738 - added below properties to experiment marketing emails opt in/out events on Braze.
|
||||
if params.get('marketing_emails_opt_in') and settings.MARKETING_EMAILS_OPT_IN:
|
||||
properties['marketing_emails_opt_in'] = params.get('marketing_emails_opt_in') == 'true'
|
||||
properties['marketing_emails_opt_in'] = is_marketable
|
||||
|
||||
# DENG-803: For segment events forwarded along to Hubspot, duplicate the `properties` section of
|
||||
# the event payload into the `traits` section so that they can be received. This is a temporary
|
||||
@@ -792,8 +793,11 @@ class RegistrationValidationView(APIView):
|
||||
def name_handler(self, request):
|
||||
""" Validates whether fullname is valid """
|
||||
name = request.data.get('name')
|
||||
validation_error = get_name_validation_error(name)
|
||||
if validation_error:
|
||||
return validation_error
|
||||
self.username_suggestions = generate_username_suggestions(name)
|
||||
return get_name_validation_error(name)
|
||||
return validation_error
|
||||
|
||||
def username_handler(self, request):
|
||||
""" Validates whether the username is valid. """
|
||||
|
||||
@@ -23,6 +23,7 @@ from xblock.runtime import KvsFieldData, MemoryIdManager, Runtime
|
||||
from xmodule.errortracker import make_error_tracker
|
||||
from xmodule.contentstore.django import contentstore
|
||||
from xmodule.modulestore.django import ModuleI18nService
|
||||
from xmodule.services import RebindUserService
|
||||
from xmodule.util.sandboxing import SandboxService
|
||||
from common.djangoapps.edxmako.services import MakoService
|
||||
from common.djangoapps.static_replace.services import ReplaceURLService
|
||||
@@ -30,6 +31,7 @@ from common.djangoapps.track import contexts as track_contexts
|
||||
from common.djangoapps.track import views as track_views
|
||||
from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
|
||||
from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache
|
||||
from lms.djangoapps.courseware import module_render
|
||||
from lms.djangoapps.grades.api import signals as grades_signals
|
||||
from openedx.core.djangoapps.xblock.apps import get_xblock_app_config
|
||||
from openedx.core.djangoapps.xblock.runtime.blockstore_field_data import BlockstoreChildrenData, BlockstoreFieldData
|
||||
@@ -37,7 +39,7 @@ from openedx.core.djangoapps.xblock.runtime.ephemeral_field_data import Ephemera
|
||||
from openedx.core.djangoapps.xblock.runtime.mixin import LmsBlockMixin
|
||||
from openedx.core.djangoapps.xblock.utils import get_xblock_id_for_anonymous_user
|
||||
from openedx.core.lib.cache_utils import CacheService
|
||||
from openedx.core.lib.xblock_utils import wrap_fragment, xblock_local_resource_url
|
||||
from openedx.core.lib.xblock_utils import wrap_fragment, xblock_local_resource_url, request_token
|
||||
|
||||
from .id_managers import OpaqueKeyReader
|
||||
from .shims import RuntimeShim, XBlockShim
|
||||
@@ -217,6 +219,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
|
||||
# TODO: Do these declarations actually help with anything? Maybe this check should
|
||||
# be removed from here and from XBlock.runtime
|
||||
declaration = block.service_declaration(service_name)
|
||||
context_key = block.scope_ids.usage_id.context_key
|
||||
if declaration is None:
|
||||
raise NoSuchServiceError(f"Service {service_name!r} was not requested.")
|
||||
# Most common service is field-data so check that first:
|
||||
@@ -230,7 +233,6 @@ class XBlockRuntime(RuntimeShim, Runtime):
|
||||
raise
|
||||
return self.block_field_datas[block.scope_ids]
|
||||
elif service_name == "completion":
|
||||
context_key = block.scope_ids.usage_id.context_key
|
||||
return CompletionService(user=self.user, context_key=context_key)
|
||||
elif service_name == "user":
|
||||
return DjangoXBlockUserService(
|
||||
@@ -253,6 +255,17 @@ class XBlockRuntime(RuntimeShim, Runtime):
|
||||
return CacheService(cache)
|
||||
elif service_name == 'replace_urls':
|
||||
return ReplaceURLService(xblock=block, lookup_asset_url=self._lookup_asset_url)
|
||||
elif service_name == 'rebind_user':
|
||||
# this service should ideally be initialized with all the arguments of get_module_system_for_user
|
||||
# but only the positional arguments are passed here as the other arguments are too
|
||||
# specific to the lms.module_render module
|
||||
return RebindUserService(
|
||||
self.user,
|
||||
context_key,
|
||||
module_render.get_module_system_for_user,
|
||||
track_function=make_track_function(),
|
||||
request_token=request_token(crum.get_current_request()),
|
||||
)
|
||||
|
||||
# Check if the XBlockRuntimeSystem wants to handle this:
|
||||
service = self.system.get_service(block, service_name)
|
||||
|
||||
@@ -97,14 +97,6 @@ class RuntimeShim:
|
||||
# TODO: Refactor capa to access this directly, don't bother the runtime. Then remove it from here.
|
||||
return False # Change this if/when we need to support unsafe courses in the new runtime.
|
||||
|
||||
@property
|
||||
def DEBUG(self):
|
||||
"""
|
||||
Should DEBUG mode (?) be used? This flag is only read by capa.
|
||||
"""
|
||||
# TODO: Refactor capa to access this directly, don't bother the runtime. Then remove it from here.
|
||||
return False
|
||||
|
||||
def get_python_lib_zip(self):
|
||||
"""
|
||||
A function returning a bytestring or None. The bytestring is the
|
||||
@@ -145,16 +137,6 @@ class RuntimeShim:
|
||||
)
|
||||
return self.resources_fs
|
||||
|
||||
@property
|
||||
def node_path(self):
|
||||
"""
|
||||
Get the path to Node.js
|
||||
|
||||
Seems only to be used by capa. Remove this if capa can be refactored.
|
||||
"""
|
||||
# TODO: Refactor capa to access this directly, don't bother the runtime. Then remove it from here.
|
||||
return getattr(settings, 'NODE_PATH', None) # Only defined in the LMS
|
||||
|
||||
def render_template(self, template_name, dictionary, namespace='main'):
|
||||
"""
|
||||
Render a mako template
|
||||
|
||||
Reference in New Issue
Block a user