diff --git a/cms/djangoapps/contentstore/rest_api/pagination.py b/cms/djangoapps/contentstore/rest_api/pagination.py new file mode 100644 index 000000000000..cef20c7fe75a --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/pagination.py @@ -0,0 +1,56 @@ +"""Pagination shared by the contentstore REST API views.""" + +from edx_rest_framework_extensions.paginators import DefaultPagination + +ENVELOPE_KEYS = ("count", "num_pages", "current_page", "start", "next", "previous", "results") + + +class DocumentedDefaultPagination(DefaultPagination): + """ + The standard page envelope, with a response schema describing all seven of its keys. + + ``DefaultPagination`` returns ``count``, ``num_pages``, ``current_page``, + ``start``, ``next``, ``previous`` and ``results``, but inherits a response + schema that documents only four of them, so generated clients would drop + ``num_pages``, ``current_page`` and ``start``. Pages, page sizes and the + body are exactly ``DefaultPagination``'s; only the published schema + differs. Every member carries an example, so the schema generator can wrap + a view's example item in a whole page. Once ``DefaultPagination`` + documents all seven keys, use it directly instead. + """ + + def get_paginated_response_schema(self, schema): + """Describe a page whose ``results`` are items described by ``schema``.""" + return { + "type": "object", + "required": list(ENVELOPE_KEYS), + "properties": { + "count": {"type": "integer", "description": "Total number of items.", "example": 25}, + "num_pages": {"type": "integer", "description": "Total number of pages.", "example": 3}, + "current_page": { + "type": "integer", + "description": "Number of this page, starting at 1.", + "example": 2, + }, + "start": { + "type": "integer", + "description": "Zero-based index of the first item on this page.", + "example": 10, + }, + "next": { + "type": "string", + "format": "uri", + "nullable": True, + "description": "Address of the next page, or null on the last page.", + "example": "https://example.com/items/?page=3", + }, + "previous": { + "type": "string", + "format": "uri", + "nullable": True, + "description": "Address of the previous page, or null on the first page.", + "example": "https://example.com/items/?page=1", + }, + "results": schema, + }, + } diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py b/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py index c66663cd208d..ea119ea895e0 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/advanced_settings.py @@ -23,6 +23,9 @@ class AdvancedCourseSettingsView(DeveloperErrorViewMixin, APIView): """ View for getting and setting the advanced settings for a course. + + Deprecated: use ``GET`` and ``PATCH /api/authoring/v1/courses/{course_key}/advanced_settings/`` + instead. See https://github.com/openedx/openedx-platform/issues/39128. """ class FilterQuery(forms.Form): @@ -65,6 +68,12 @@ def get(self, request: Request, course_id: str): """ Get an object containing all the advanced settings in a course. + Deprecated: use ``GET /api/authoring/v1/courses/{course_key}/advanced_settings/``. + Equivalent v1 reads: without ``view`` = this endpoint with ``fetch_all=0``; + ``view=full`` = this endpoint's default; ``fields`` replaces ``filter_fields`` + and selects within the chosen view. + See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** GET /api/contentstore/v0/advanced_settings/{course_id} @@ -148,6 +157,9 @@ def patch(self, request: Request, course_id: str): """ Update a course's advanced settings. + Deprecated: use ``PATCH /api/authoring/v1/courses/{course_key}/advanced_settings/``, + which accepts this body unchanged. See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** PATCH /api/contentstore/v0/advanced_settings/{course_id} { diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py b/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py index 57664fdf44c8..5d2a7e5b85e9 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/tabs.py @@ -27,6 +27,9 @@ class CourseTabListView(DeveloperErrorViewMixin, APIView): """ API view to list course tabs. + + Deprecated: use ``GET /api/authoring/v1/custom_pages/?course_key={course_key}`` instead. + See https://github.com/openedx/openedx-platform/issues/39128. """ @extend_schema( @@ -43,6 +46,10 @@ def get(self, request: Request, course_id: str) -> Response: """ Get a list of all the tabs in a course including hidden tabs. + Deprecated: use ``GET /api/authoring/v1/custom_pages/?course_key={course_key}``, which + returns the same custom pages, paginated (``page``, ``page_size`` up to 100). + See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** GET /api/contentstore/v0/tabs/{course_id} @@ -110,6 +117,9 @@ def get(self, request: Request, course_id: str) -> Response: class CourseTabSettingsView(DeveloperErrorViewMixin, APIView): """ API view for course tabs settings. + + Deprecated: use ``PATCH /api/authoring/v1/courses/{course_key}/tabs/{tab_id}/`` instead. + See https://github.com/openedx/openedx-platform/issues/39128. """ def handle_exception(self, exc): @@ -139,6 +149,10 @@ def post(self, request: Request, course_id: str) -> Response: """ Change visibility of tabs in a course. + Deprecated: use ``PATCH /api/authoring/v1/courses/{course_key}/tabs/{tab_id}/`` with + the same body, which names the tab by ``tab_id`` in the path and returns the tab. + See https://github.com/openedx/openedx-platform/issues/39128. + **Example Requests** You can provide either a tab_id or a tab_location. @@ -191,6 +205,9 @@ def post(self, request: Request, course_id: str) -> Response: class CourseTabReorderView(DeveloperErrorViewMixin, APIView): """ API view for reordering course tabs. + + Deprecated: use ``PUT /api/authoring/v1/courses/{course_key}/custom_page_order/`` instead. + See https://github.com/openedx/openedx-platform/issues/39128. """ def handle_exception(self, exc: Exception) -> Response: @@ -218,6 +235,10 @@ def post(self, request: Request, course_id: str) -> Response: """ Reorder tabs in a course. + Deprecated: use ``PUT /api/authoring/v1/courses/{course_key}/custom_page_order/`` with + ``{"custom_pages": [, ...]}``, the usage keys this endpoint takes as + ``tab_locator``. See https://github.com/openedx/openedx-platform/issues/39128. + **Example Requests** Move course tabs: diff --git a/cms/djangoapps/contentstore/rest_api/v1/advanced_settings_service.py b/cms/djangoapps/contentstore/rest_api/v1/advanced_settings_service.py new file mode 100644 index 000000000000..3a641e488000 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/advanced_settings_service.py @@ -0,0 +1,90 @@ +"""Reads and updates of a course's advanced settings.""" + +import logging + +from rest_framework.exceptions import NotFound, ValidationError +from rest_framework.settings import api_settings + +from cms.djangoapps.contentstore.views.course import update_course_advanced_settings +from cms.djangoapps.contentstore.views.permissions import COURSE_NOT_FOUND_MESSAGE +from cms.djangoapps.models.settings.course_metadata import CourseMetadata +from xmodule.modulestore.django import modulestore + +log = logging.getLogger(__name__) + +# Reported under the name of each setting whose value the course refused. The +# validator's own explanation is logged, not returned. +INVALID_SETTING_MESSAGE = "The value is not valid for this setting." +# Reported when the settings could not be saved for a reason tied to no setting. +SETTINGS_NOT_SAVED_MESSAGE = "The settings could not be saved. No setting was changed." + + +def _course_block(course_key): + """Return the course block of ``course_key``, or raise ``NotFound`` when the modulestore has none.""" + course_block = modulestore().get_course(course_key) + if course_block is None: + raise NotFound(COURSE_NOT_FOUND_MESSAGE) + return course_block + + +def settings_authors_do_not_edit(course_key): + """Return the names of the course's settings that are not edited as advanced settings.""" + return frozenset(CourseMetadata.get_exclude_list_of_fields(course_key)) + + +def get_advanced_settings(course_key, full=False): + """ + Return the advanced settings of the course, keyed by setting name. + + By default only the settings course authors edit in Studio are returned; + with ``full`` every setting of the course is. + """ + course_block = _course_block(course_key) + if full: + return CourseMetadata.fetch_all(course_block) + return CourseMetadata.fetch(course_block) + + +def update_advanced_settings(course_key, changes, user): + """ + Apply ``changes`` (``{name: {"value": value}}``) to the course as ``user``. + + Every value is validated before any is stored; when one is refused, + nothing is stored and ``ValidationError`` maps each refused setting to + ``INVALID_SETTING_MESSAGE``. + Returns the settings course authors edit in Studio, as stored. + """ + course_block = _course_block(course_key) + try: + return update_course_advanced_settings(course_block, changes, user) + except ValidationError as error: + raise _errors_by_setting(course_key, error) from error + + +def _errors_by_setting(course_key, error): + """ + Return a ``ValidationError`` mapping each refused setting to a fixed message. + + The settings validator reports one entry per refused setting, carrying the + setting's name and its own explanation, which can be the text of a parsing + exception. The explanation is logged with the course and setting; the + caller gets ``INVALID_SETTING_MESSAGE`` under the setting's name. Any + failure not tied to a setting is logged and reported with + ``SETTINGS_NOT_SAVED_MESSAGE``. + """ + detail = error.detail if isinstance(error.detail, list) else [error.detail] + by_setting = {} + unattributed = [] + for entry in detail: + name = entry.get("key") if isinstance(entry, dict) else None + if name: + log.warning( + "Advanced setting %s of course %s was refused: %s", name, course_key, entry.get("message", ""), + ) + by_setting[str(name)] = [INVALID_SETTING_MESSAGE] + else: + unattributed.append(entry) + if unattributed: + log.warning("Advanced settings of course %s were not saved: %s", course_key, unattributed) + by_setting.setdefault(api_settings.NON_FIELD_ERRORS_KEY, []).append(SETTINGS_NOT_SAVED_MESSAGE) + return ValidationError(by_setting) diff --git a/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py b/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py index 5a53ae650413..50b5e590616a 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py @@ -7,6 +7,12 @@ YoutubeTranscriptChecksViewSet, YoutubeTranscriptImportsViewSet, ) +from cms.djangoapps.contentstore.rest_api.v1.views.advanced_settings import CourseAdvancedSettingsViewSet +from cms.djangoapps.contentstore.rest_api.v1.views.custom_pages import ( + CourseCustomPageOrderViewSet, + CourseCustomPagesViewSet, + CourseTabsViewSet, +) app_name = "authoring_v1" @@ -39,4 +45,25 @@ ), name="xblock_detail", ), + path( + "courses//advanced_settings/", + CourseAdvancedSettingsViewSet.as_view({"get": "retrieve", "patch": "partial_update"}), + name="course_advanced_settings", + ), + # The course is named by the required course_key query parameter. + path( + "custom_pages/", + CourseCustomPagesViewSet.as_view({"get": "list"}), + name="custom_page_list", + ), + path( + "courses//custom_page_order/", + CourseCustomPageOrderViewSet.as_view({"put": "update"}), + name="course_custom_page_order", + ), + path( + "courses//tabs//", + CourseTabsViewSet.as_view({"patch": "partial_update"}), + name="course_tab_detail", + ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v1/custom_pages_service.py b/cms/djangoapps/contentstore/rest_api/v1/custom_pages_service.py new file mode 100644 index 000000000000..325b90d33157 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/custom_pages_service.py @@ -0,0 +1,110 @@ +"""Reads and changes of a course's custom pages and tabs.""" + +import logging + +from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator +from rest_framework.exceptions import NotFound, ValidationError +from rest_framework.settings import api_settings + +from cms.djangoapps.contentstore.views.permissions import COURSE_NOT_FOUND_MESSAGE +from cms.djangoapps.contentstore.views.tabs import edit_tab_handler, get_course_tabs, reorder_tabs_handler +from xmodule.modulestore.django import modulestore +from xmodule.tabs import CourseTabList, StaticTab + +log = logging.getLogger(__name__) + +# Reported under custom_pages when an item names no custom page of the course. +UNKNOWN_CUSTOM_PAGE_MESSAGE = "Each item must be the usage key of a custom page of this course." +# Reported under custom_pages when the same custom page is named twice. +REPEATED_CUSTOM_PAGE_MESSAGE = "Each custom page may be listed only once." +# Reported when the course's tabs could not be saved in the new order. +ORDER_NOT_SAVED_MESSAGE = "The custom pages could not be reordered. Their order is unchanged." +# Reported when the course has no tab with the requested identifier. +TAB_NOT_FOUND_MESSAGE = "The requested tab does not exist." +# Reported under is_hidden when the tab may not be hidden or shown. +TAB_NOT_HIDEABLE_MESSAGE = "This tab cannot be hidden or shown." + + +def _course_block(course_key): + """Return the course block of ``course_key``, or raise ``NotFound`` when the modulestore has none.""" + course_block = modulestore().get_course(course_key) + if course_block is None: + raise NotFound(COURSE_NOT_FOUND_MESSAGE) + return course_block + + +def _custom_page_slug(course_key, usage_key): + """ + Return the ``url_slug`` a custom page of ``course_key`` named by ``usage_key`` would have, or None. + + The key may carry a branch or version of the course. A key of another + course or library, or of a block that is not a custom page, gives None. + """ + if not isinstance(usage_key, BlockUsageLocator) or usage_key.block_type != "static_tab": + return None + context = usage_key.course_key + if not isinstance(context, CourseLocator): + return None + if (context.org, context.course, context.run) != (course_key.org, course_key.course, course_key.run): + return None + return usage_key.block_id + + +def list_custom_pages(course_key, user): + """ + Return the course's custom pages in the order of the course's tabs. + + Each page carries its usage key as ``locator``. Built-in tabs are never + returned, whether or not hidden. + """ + course_block = _course_block(course_key) + return [tab for tab in get_course_tabs(course_block, user) if isinstance(tab, StaticTab)] + + +def reorder_custom_pages(course_key, usage_keys, user): + """ + Put the course's custom pages in the order of ``usage_keys``, as ``user``. + + Custom pages not named follow the named ones in their current order. As + when Studio reorders pages, the course's tabs are then sorted by kind, so + built-in tabs take their usual places around the custom pages. Raises + ``ValidationError`` when a usage key names no custom page of the course or + names one twice, and when the reordered tabs could not be saved; nothing + is stored then. + """ + course_block = _course_block(course_key) + pages = {tab.url_slug: tab for tab in course_block.tabs if isinstance(tab, StaticTab)} + named = [] + for usage_key in usage_keys: + page = pages.get(_custom_page_slug(course_key, usage_key)) + if page is None: + raise ValidationError({"custom_pages": [UNKNOWN_CUSTOM_PAGE_MESSAGE]}) + if any(page is other for other in named): + raise ValidationError({"custom_pages": [REPEATED_CUSTOM_PAGE_MESSAGE]}) + named.append(page) + rest = [page for page in pages.values() if all(page is not other for other in named)] + try: + reorder_tabs_handler(course_block, [{"tab_id": page.tab_id} for page in named + rest], user) + except ValidationError as error: + log.warning("The custom pages of course %s were not reordered: %s", course_key, error.detail) + raise ValidationError({api_settings.NON_FIELD_ERRORS_KEY: [ORDER_NOT_SAVED_MESSAGE]}) from error + + +def update_tab(course_key, tab_id, changes, user): + """ + Apply ``changes`` (``{"is_hidden": bool}`` or nothing) to the course tab ``tab_id``, as ``user``. + + Returns the tab as stored. Raises ``NotFound`` when the course has no such + tab, and ``ValidationError`` on ``is_hidden`` when the tab may not be + hidden or shown. + """ + course_block = _course_block(course_key) + tab = CourseTabList.get_tab_by_id(course_block.tabs, tab_id) + if tab is None: + raise NotFound(TAB_NOT_FOUND_MESSAGE) + if "is_hidden" not in changes: + return tab + if not tab.is_hideable: + raise ValidationError({"is_hidden": [TAB_NOT_HIDEABLE_MESSAGE]}) + edit_tab_handler(course_block, {"tab_id_locator": {"tab_id": tab_id}, "is_hidden": changes["is_hidden"]}, user) + return tab diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/advanced_settings.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/advanced_settings.py new file mode 100644 index 000000000000..7090efd864b1 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/advanced_settings.py @@ -0,0 +1,144 @@ +"""Serializers for a course's advanced settings.""" + +from collections.abc import Mapping + +from drf_spectacular.extensions import OpenApiSerializerExtension +from drf_spectacular.plumbing import build_object_type, get_doc +from rest_framework import serializers +from rest_framework.exceptions import ValidationError +from rest_framework.settings import api_settings + +MINIMAL_VIEW = "minimal" +FULL_VIEW = "full" + + +class AdvancedSettingSerializer(serializers.Serializer): # pylint: disable=abstract-method + """One advanced setting of a course, as Studio presents it for editing.""" + + value = serializers.JSONField( + allow_null=True, + help_text="Current value of the setting. Its type depends on the setting.", + ) + display_name = serializers.CharField(help_text="Name of the setting as Studio shows it.") + help = serializers.CharField( + allow_null=True, + help_text="Text explaining what the setting does, or null when the setting has none.", + ) + deprecated = serializers.BooleanField(help_text="Whether the setting is deprecated.") + hide_on_enabled_publisher = serializers.BooleanField( + help_text="Whether Studio hides the setting when the course is managed by a publishing service.", + ) + + class Meta: + ref_name = "AdvancedSetting.v1" + + +class AdvancedSettingUpdateSerializer(serializers.Serializer): # pylint: disable=abstract-method + """The new value of one advanced setting.""" + + value = serializers.JSONField( + allow_null=True, + help_text="New value of the setting. Its type depends on the setting.", + ) + + class Meta: + ref_name = "AdvancedSettingUpdate.v1" + + +class _SettingsByNameSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + An object whose members are settings keyed by setting name. + + The set of names depends on the course, so the members cannot be declared + as fields; each one is handled by ``setting_serializer_class``. Names given + in the ``ignored_settings`` context entry are dropped before any member is + validated. + """ + + setting_serializer_class = None + + def to_representation(self, instance): + return { + name: self.setting_serializer_class(setting).data + for name, setting in instance.items() + } + + def to_internal_value(self, data): + if not isinstance(data, Mapping): + message = self.error_messages["invalid"].format(datatype=type(data).__name__) + raise ValidationError({api_settings.NON_FIELD_ERRORS_KEY: [message]}, code="invalid") + ignored = self.context.get("ignored_settings", frozenset()) + validated, errors = {}, {} + for name, item in data.items(): + if name in ignored: + continue + setting = self.setting_serializer_class(data=item) + if setting.is_valid(): + validated[name] = dict(setting.validated_data) + else: + errors[name] = [str(message) for messages in setting.errors.values() for message in messages] + if errors: + raise ValidationError(errors) + return validated + + +class CourseAdvancedSettingsSerializer(_SettingsByNameSerializer): # pylint: disable=abstract-method + """A course's advanced settings, keyed by setting name in the order the course declares them.""" + + setting_serializer_class = AdvancedSettingSerializer + + class Meta: + ref_name = "CourseAdvancedSettings.v1" + + +class CourseAdvancedSettingsUpdateSerializer(_SettingsByNameSerializer): # pylint: disable=abstract-method + """ + New values for some of a course's advanced settings, keyed by setting name. + + Settings that authors do not edit are ignored, whatever their entry holds; + settings the course does not have are ignored once their entry is valid. + """ + + setting_serializer_class = AdvancedSettingUpdateSerializer + + class Meta: + ref_name = "CourseAdvancedSettingsUpdate.v1" + + +class AdvancedSettingsQuerySerializer(serializers.Serializer): # pylint: disable=abstract-method + """Query parameters of a read of a course's advanced settings.""" + + view = serializers.ChoiceField( + choices=[MINIMAL_VIEW, FULL_VIEW], + default=MINIMAL_VIEW, + help_text=( + "Which settings to return. 'minimal' returns the settings course authors edit in Studio; " + "'full' returns every setting of the course, including those Studio manages on other pages " + "and those of features that are turned off." + ), + ) + fields = serializers.CharField( + required=False, + allow_blank=True, + help_text=( + "Comma-separated names of the settings to return, out of those the view returns. " + "Names the view does not return are ignored." + ), + ) + + class Meta: + ref_name = "AdvancedSettingsQuery.v1" + + +class SettingsByNameSerializerExtension(OpenApiSerializerExtension): + """Publish a settings-by-name serializer as an object whose members are its setting component.""" + + target_class = _SettingsByNameSerializer + match_subclasses = True + + def map_serializer(self, auto_schema, direction): + setting = auto_schema.resolve_serializer(self.target.setting_serializer_class, direction) + return build_object_type( + description=get_doc(self.target.__class__), + additionalProperties=setting.ref, + ) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/custom_pages.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/custom_pages.py new file mode 100644 index 000000000000..6426ef553342 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/custom_pages.py @@ -0,0 +1,126 @@ +"""Serializers for a course's custom pages and tabs.""" + +from drf_spectacular.utils import extend_schema_field +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey, UsageKey +from rest_framework import serializers + +# Reported under custom_pages when an item is not a string holding a usage key. +NOT_A_USAGE_KEY_MESSAGE = "Each item must be the usage key of a custom page." +# Reported under course_key when the query parameter does not hold the key of a course. +NOT_A_COURSE_KEY_MESSAGE = "The value must be the key of a course, such as course-v1:edX+DemoX+2026." + +# Members of a tab that are returned as members of their own; the rest of the +# tab's stored entry is returned under ``settings``. +_TAB_MEMBERS = ("type", "title", "is_hideable", "is_hidden", "is_movable", "course_staff_only", "name", "tab_id") + + +class CourseTabSerializer(serializers.Serializer): # pylint: disable=abstract-method + """One tab of a course's navigation, as course authors manage it.""" + + type = serializers.CharField(help_text="Kind of tab, such as progress, wiki or static_tab for a custom page.") + title = serializers.CharField( + allow_null=True, + help_text="Default name of this kind of tab, or null for kinds without one, such as custom pages.", + ) + is_hideable = serializers.BooleanField(help_text="Whether course authors may hide the tab from learners.") + is_hidden = serializers.BooleanField(help_text="Whether the tab is hidden from learners.") + is_movable = serializers.BooleanField(help_text="Whether course authors may move the tab among the course's tabs.") + course_staff_only = serializers.BooleanField(help_text="Whether only course staff see the tab.") + name = serializers.CharField(help_text="Name of the tab as learners see it; it takes precedence over the title.") + tab_id = serializers.CharField(help_text="Identifier of the tab within the course, such as progress or wiki.") + settings = serializers.SerializerMethodField( + help_text="The tab's other stored settings, such as the url_slug of a custom page.", + ) + + class Meta: + ref_name = "CourseTab.v1" + + def get_settings(self, tab) -> dict: + """Return the members of the tab's stored entry not returned as members of their own.""" + return { + key: value for key, value in tab.tab_dict.items() + if key not in _TAB_MEMBERS and key != "link_func" + } + + +class CustomPageSerializer(CourseTabSerializer): # pylint: disable=abstract-method + """One custom page of a course: a tab whose content course authors write.""" + + id = serializers.CharField( + source="locator", + help_text="Usage key of the custom page. Use it to edit the page's content and to order the pages.", + ) + + class Meta: + ref_name = "CustomPage.v1" + + +class CustomPageListQuerySerializer(serializers.Serializer): # pylint: disable=abstract-method + """Query parameters of a listing of a course's custom pages.""" + + course_key = serializers.CharField( + help_text="Key of the course whose custom pages to list, such as course-v1:edX+DemoX+2026.", + ) + + class Meta: + ref_name = "CustomPageListQuery.v1" + + def validate_course_key(self, value): + """Parse the value as a course key; refuse a value that is not one, or is in the retired slash form.""" + try: + course_key = CourseKey.from_string(value) + except InvalidKeyError: + raise serializers.ValidationError(NOT_A_COURSE_KEY_MESSAGE) from None + if course_key.deprecated: + raise serializers.ValidationError(NOT_A_COURSE_KEY_MESSAGE) + return course_key + + +@extend_schema_field({"type": "string"}) +class _CustomPageItemField(serializers.JSONField): + """ + One item of a custom page order, accepted as any JSON value. + + Its type is checked by the list's own validation, so every unusable item + is refused with the same message instead of a per-item error. + """ + + +class CustomPageOrderSerializer(serializers.Serializer): # pylint: disable=abstract-method + """A new order for a course's custom pages.""" + + custom_pages = serializers.ListField( + child=_CustomPageItemField(allow_null=True, help_text="Usage key of one of the course's custom pages."), + allow_empty=True, + help_text=( + "Usage keys of the course's custom pages, in the order learners are to see them. Custom pages " + "left out keep their current order after the pages listed." + ), + ) + + class Meta: + ref_name = "CustomPageOrder.v1" + + def validate_custom_pages(self, values): + """Parse each item as a usage key; refuse the list when one is not a string holding one.""" + usage_keys = [] + for value in values: + if not isinstance(value, str): + raise serializers.ValidationError(NOT_A_USAGE_KEY_MESSAGE) + try: + usage_keys.append(UsageKey.from_string(value)) + except InvalidKeyError: + raise serializers.ValidationError(NOT_A_USAGE_KEY_MESSAGE) from None + return usage_keys + + +class CourseTabUpdateSerializer(serializers.Serializer): # pylint: disable=abstract-method + """Changes to one tab of a course.""" + + is_hidden = serializers.BooleanField( + help_text="True to hide the tab from learners, false to show it. Only hideable tabs accept it.", + ) + + class Meta: + ref_name = "CourseTabUpdate.v1" diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/advanced_settings.py b/cms/djangoapps/contentstore/rest_api/v1/views/advanced_settings.py new file mode 100644 index 000000000000..3fba8c7740bd --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/advanced_settings.py @@ -0,0 +1,203 @@ +"""API views for a course's advanced settings.""" + +from django.core.exceptions import ImproperlyConfigured +from drf_spectacular.utils import OpenApiExample, OpenApiParameter, OpenApiResponse, extend_schema +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from edx_rest_framework_extensions.errors import ErrorResponseSerializer +from edx_rest_framework_extensions.mixins import StandardizedErrorMixin +from edx_rest_framework_extensions.shaping import project +from edx_rest_framework_extensions.url_converters import CourseKeyConverter +from rest_framework import viewsets +from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response + +from cms.djangoapps.contentstore.rest_api.v1.advanced_settings_service import ( + INVALID_SETTING_MESSAGE, + get_advanced_settings, + settings_authors_do_not_edit, + update_advanced_settings, +) +from cms.djangoapps.contentstore.rest_api.v1.serializers.advanced_settings import ( + FULL_VIEW, + MINIMAL_VIEW, + AdvancedSettingsQuerySerializer, + CourseAdvancedSettingsSerializer, + CourseAdvancedSettingsUpdateSerializer, +) +from cms.djangoapps.contentstore.views.permissions import ( + CanManageAdvancedSettings, + CanViewAdvancedSettings, + IsExistingCourse, +) + +_COURSE_KEY_PARAMETER = OpenApiParameter( + name="course_key", + description="Key of the course, such as course-v1:edX+DemoX+2026.", + location=OpenApiParameter.PATH, + type=str, + pattern=f"^{CourseKeyConverter.regex}$", +) + + +def _query_parameter(name, **kwargs): + return OpenApiParameter( + name=name, + description=AdvancedSettingsQuerySerializer().fields[name].help_text, + required=False, + location=OpenApiParameter.QUERY, + **kwargs, + ) + + +def _error(description): + return OpenApiResponse(response=ErrorResponseSerializer, description=description) + + +_SETTINGS_EXAMPLE = OpenApiExample( + "Advanced settings", + summary="Two of a course's settings", + response_only=True, + status_codes=["200"], + value={ + "display_name": { + "value": "Demonstration Course", + "display_name": "Course Display Name", + "help": "Enter the name of the course as it should appear in the course list.", + "deprecated": False, + "hide_on_enabled_publisher": False, + }, + "days_early_for_beta": { + "value": None, + "display_name": "Days Early for Beta Users", + "help": "Enter the number of days before the start date that beta users can access the course.", + "deprecated": False, + "hide_on_enabled_publisher": False, + }, + }, +) + +_UPDATE_EXAMPLE = OpenApiExample( + "New values", + summary="Turn on the calculator and set the advanced modules", + request_only=True, + value={ + "show_calculator": {"value": True}, + "advanced_modules": {"value": ["poll", "survey"]}, + }, +) + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CourseAdvancedSettingsViewSet(StandardizedErrorMixin, viewsets.ViewSet): + """ + The advanced settings of one course: its policy settings keyed by name. + + By default (``view=minimal``) the settings course authors edit in Studio + are returned; with ``view=full`` every setting of the course is. A partial + update changes the values of the settings it names, all of them or none. + + Anyone who may view the course's advanced settings may read them; changing + them needs the permission to manage them. The course is looked up first, + so a signed-in caller learns that a course does not exist (404) before + being refused (403). + + A plain ``ViewSet``: the settings are fields of the course block in the + modulestore, not database rows. + """ + + # Kept from the legacy endpoint: an inactive account's browser session stays authenticated and + # the course permission decides (inactive course staff are refused, inactive global staff are not). + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated, IsExistingCourse) + action_permission_classes = { + "retrieve": (CanViewAdvancedSettings,), + "partial_update": (CanManageAdvancedSettings,), + } + # A method with no route (answered 405) and OPTIONS serve no settings, so + # only the course is checked for them. + unrouted_actions = (None, "metadata") + serializer_class = CourseAdvancedSettingsSerializer + + def get_permissions(self): + """ + Check the course, then the permission the requested action needs. + + An action with no permission declared in ``action_permission_classes`` + is refused with a server error instead of being served unchecked. + """ + if self.action in self.unrouted_actions: + return super().get_permissions() + if self.action not in self.action_permission_classes: + raise ImproperlyConfigured(f"No permission is declared for the {self.action!r} action.") + specific = self.action_permission_classes[self.action] + return [*super().get_permissions(), *(permission() for permission in specific)] + + def get_serializer(self, *args, **kwargs): + """Instantiate the settings serializer declared on this viewset.""" + return self.serializer_class(*args, **kwargs) + + @extend_schema( + summary="Get a course's advanced settings", + description=( + "Returns the course's advanced settings keyed by setting name, each with its value, its name " + "as Studio shows it, its help text and whether it is deprecated. By default (view=minimal) the " + "settings course authors edit in Studio are returned; view=full returns every setting of the " + "course. fields narrows the response to the named settings." + ), + parameters=[ + _COURSE_KEY_PARAMETER, + _query_parameter("view", type=str, enum=[MINIMAL_VIEW, FULL_VIEW], default=MINIMAL_VIEW), + _query_parameter("fields", type=str), + ], + responses={ + 200: OpenApiResponse( + response=CourseAdvancedSettingsSerializer, + description="The course's advanced settings.", + ), + 400: _error("A query parameter has an invalid value."), + 401: _error("The requester is not authenticated."), + 403: _error("The requester may not view this course's advanced settings."), + 404: _error("The course does not exist."), + }, + examples=[_SETTINGS_EXAMPLE], + ) + def retrieve(self, request, course_key): + """Return the course's advanced settings, narrowed to ``fields`` when given.""" + query = AdvancedSettingsQuerySerializer(data=request.query_params) + query.is_valid(raise_exception=True) + params = query.validated_data + current = get_advanced_settings(course_key, full=params["view"] == FULL_VIEW) + return Response(project(self.get_serializer(current).data, params.get("fields"))) + + @extend_schema( + summary="Change some of a course's advanced settings", + description=( + "Sets the value of each setting named in the body. Every value is validated before any is " + "stored: when one is refused nothing is changed, and the error's errors member maps the name " + f"of each refused setting to the message '{INVALID_SETTING_MESSAGE}'. Settings that authors do " + "not edit are ignored, and so are settings the course does not have. Returns the settings " + "course authors edit in Studio, as stored." + ), + parameters=[_COURSE_KEY_PARAMETER], + request={"application/json": CourseAdvancedSettingsUpdateSerializer}, + responses={ + 200: OpenApiResponse( + response=CourseAdvancedSettingsSerializer, + description="The settings course authors edit in Studio, after the change.", + ), + 400: _error("The body is not an object of settings, or a value was refused."), + 401: _error("The requester is not authenticated."), + 403: _error("The requester may not change this course's advanced settings."), + 404: _error("The course does not exist."), + }, + examples=[_SETTINGS_EXAMPLE, _UPDATE_EXAMPLE], + ) + def partial_update(self, request, course_key): + """Store the new values and return the editable settings as stored.""" + changes = CourseAdvancedSettingsUpdateSerializer( + data=request.data, context={"ignored_settings": settings_authors_do_not_edit(course_key)}, + ) + changes.is_valid(raise_exception=True) + stored = update_advanced_settings(course_key, changes.validated_data, request.user) + return Response(self.get_serializer(stored).data) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/custom_pages.py b/cms/djangoapps/contentstore/rest_api/v1/views/custom_pages.py new file mode 100644 index 000000000000..5b7374cbfe32 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/custom_pages.py @@ -0,0 +1,310 @@ +"""API views for a course's custom pages and tabs.""" + +from django.core.exceptions import ImproperlyConfigured +from drf_spectacular.utils import OpenApiExample, OpenApiParameter, OpenApiResponse, extend_schema +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from edx_rest_framework_extensions.errors import ErrorResponseSerializer +from edx_rest_framework_extensions.mixins import StandardizedErrorMixin +from edx_rest_framework_extensions.paginators import IterablePaginationMixin +from edx_rest_framework_extensions.url_converters import CourseKeyConverter +from rest_framework import status, viewsets +from rest_framework.permissions import BasePermission, IsAuthenticated +from rest_framework.response import Response + +from cms.djangoapps.contentstore.rest_api.pagination import DocumentedDefaultPagination +from cms.djangoapps.contentstore.rest_api.v1.custom_pages_service import ( + ORDER_NOT_SAVED_MESSAGE, + REPEATED_CUSTOM_PAGE_MESSAGE, + TAB_NOT_FOUND_MESSAGE, + TAB_NOT_HIDEABLE_MESSAGE, + UNKNOWN_CUSTOM_PAGE_MESSAGE, + list_custom_pages, + reorder_custom_pages, + update_tab, +) +from cms.djangoapps.contentstore.rest_api.v1.serializers.custom_pages import ( + NOT_A_USAGE_KEY_MESSAGE, + CourseTabSerializer, + CourseTabUpdateSerializer, + CustomPageListQuerySerializer, + CustomPageOrderSerializer, + CustomPageSerializer, +) +from cms.djangoapps.contentstore.views.permissions import ( + CanManagePagesAndResources, + CanViewPagesAndResources, + IsExistingCourse, +) + +_COURSE_KEY_PARAMETER = OpenApiParameter( + name="course_key", + description="Key of the course, such as course-v1:edX+DemoX+2026.", + location=OpenApiParameter.PATH, + type=str, + pattern=f"^{CourseKeyConverter.regex}$", +) + +_COURSE_KEY_QUERY_PARAMETER = OpenApiParameter( + name="course_key", + description=CustomPageListQuerySerializer().fields["course_key"].help_text, + location=OpenApiParameter.QUERY, + required=True, + type=str, +) + +_TAB_ID_PARAMETER = OpenApiParameter( + name="tab_id", + description="Identifier of the tab within the course, such as progress, dates or wiki.", + location=OpenApiParameter.PATH, + type=str, +) + + +def _error(description): + return OpenApiResponse(response=ErrorResponseSerializer, description=description) + + +_CUSTOM_PAGE = { + "type": "static_tab", + "title": None, + "is_hideable": False, + "is_hidden": False, + "is_movable": True, + "course_staff_only": False, + "name": "Syllabus", + "tab_id": "static_tab_4f1d7a2c9b3e4d5f8a6b7c8d9e0f1a2b", + "settings": {"url_slug": "4f1d7a2c9b3e4d5f8a6b7c8d9e0f1a2b"}, + "id": "block-v1:edX+DemoX+2026+type@static_tab+block@4f1d7a2c9b3e4d5f8a6b7c8d9e0f1a2b", +} + +_CUSTOM_PAGE_EXAMPLE = OpenApiExample( + "Custom page", + summary="One custom page of a course", + response_only=True, + status_codes=["200"], + value=_CUSTOM_PAGE, +) + +_ORDER_EXAMPLE = OpenApiExample( + "New order", + summary="Show the second custom page first", + request_only=True, + value={ + "custom_pages": [ + "block-v1:edX+DemoX+2026+type@static_tab+block@9e0f1a2b4f1d7a2c9b3e4d5f8a6b7c8d", + "block-v1:edX+DemoX+2026+type@static_tab+block@4f1d7a2c9b3e4d5f8a6b7c8d9e0f1a2b", + ], + }, +) + +_HIDE_EXAMPLE = OpenApiExample( + "Hide", summary="Hide the tab from learners", request_only=True, value={"is_hidden": True}, +) + +_TAB_EXAMPLE = OpenApiExample( + "Progress tab", + summary="The progress tab, hidden", + response_only=True, + status_codes=["200"], + value={ + "type": "progress", + "title": "Progress", + "is_hideable": True, + "is_hidden": True, + "is_movable": False, + "course_staff_only": False, + "name": "Progress", + "tab_id": "progress", + "settings": {}, + }, +) + + +class _CourseKeyFromQuery(BasePermission): + """ + Read the course from the required ``course_key`` query parameter into the view's ``course_key`` argument. + + The course checks that follow read that argument, as they do on addresses + that carry the course in the path. Listed after the authentication check, + so an anonymous caller is refused (401) before the query is read, and + before the course checks, so a missing or unparsable key is refused (400) + before any course is looked up. + """ + + def has_permission(self, request, view): + query = CustomPageListQuerySerializer(data=request.query_params) + query.is_valid(raise_exception=True) + view.kwargs["course_key"] = query.validated_data["course_key"] + return True + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CourseCustomPagesViewSet(StandardizedErrorMixin, IterablePaginationMixin, viewsets.ViewSet): + """ + The custom pages of courses: the tabs whose content course authors write, + each addressed by its own usage key, listed per course in the order of + the course's tabs. + + The course is named by the required ``course_key`` query parameter. + Built-in tabs (progress, dates, wiki and the like) are not custom pages + and are never listed; they are changed one at a time through + ``CourseTabsViewSet``. + + Anyone who may view the course's pages and resources may list them. The + course key is read first (400 when missing or invalid), then the course is + looked up, so a signed-in caller learns that a course does not exist (404) + before being refused (403). An action this viewset was not written for is + refused with a server error instead of being served with these checks. + + A plain ``ViewSet``: the pages are entries of the course's tab list in the + modulestore, not database rows. + """ + + # Kept from the legacy endpoint: inactive accounts' sessions are accepted and the course permission decides. + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated, _CourseKeyFromQuery, IsExistingCourse, CanViewPagesAndResources) + # The actions the permissions above were written for. A method with no + # route (answered 405) has no action; OPTIONS is ``metadata``. + checked_actions = (None, "list", "metadata") + serializer_class = CustomPageSerializer + pagination_class = DocumentedDefaultPagination + + def get_permissions(self): + """Return the viewset's permissions, or refuse an action they were not written for.""" + if self.action not in self.checked_actions: + raise ImproperlyConfigured(f"No permission is declared for the {self.action!r} action.") + return super().get_permissions() + + def get_serializer(self, *args, **kwargs): + """Instantiate the custom page serializer declared on this viewset.""" + return self.serializer_class(*args, **kwargs) + + @extend_schema( + summary="List a course's custom pages", + description=( + "Returns one page of the custom pages of the course named by course_key, in the order learners " + "see them among the course's tabs. Built-in tabs such as progress or wiki are not custom pages " + "and are not listed. A missing course_key, or one that is not the key of a course, is refused " + "with 400 and an error under course_key." + ), + parameters=[_COURSE_KEY_QUERY_PARAMETER], + responses={ + 200: OpenApiResponse(response=CustomPageSerializer, description="A page of the course's custom pages."), + 400: _error("course_key is missing or is not the key of a course."), + 401: _error("The requester is not authenticated."), + 403: _error("The requester may not view this course's pages and resources."), + 404: _error("The course does not exist, or the requested page is out of range."), + }, + examples=[_CUSTOM_PAGE_EXAMPLE], + ) + def list(self, request, **kwargs): + """Return one page of the custom pages of the course named in the query.""" + return self.paginate_iterable(request, list_custom_pages(self.kwargs["course_key"], request.user)) + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CourseCustomPageOrderViewSet(StandardizedErrorMixin, viewsets.ViewSet): + """ + The order of one course's custom pages. + + A reorder puts the pages it lists first, in the order listed; pages it + leaves out follow them in their current order. + + Anyone who may change the course's pages and resources may reorder them. + The course is looked up first, so a signed-in caller learns that a course + does not exist (404) before being refused (403). + + A plain ``ViewSet``: the order is that of the course's tab list in the + modulestore, not of database rows. + """ + + # Kept from the legacy endpoint: inactive accounts' sessions are accepted and the course permission decides. + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated, IsExistingCourse, CanManagePagesAndResources) + + @extend_schema( + summary="Reorder a course's custom pages", + description=( + "Puts the course's custom pages in the order of their usage keys in custom_pages. Custom pages " + "left out follow those listed, in their current order; built-in tabs keep their usual places. " + "When a request is refused, nothing is changed. Besides a body that is not an object holding " + "the list custom_pages, a request is refused with 400 and one of these messages: under " + f"custom_pages, '{NOT_A_USAGE_KEY_MESSAGE}' when an item is not a string holding a usage key, " + f"'{UNKNOWN_CUSTOM_PAGE_MESSAGE}' when an item names no custom page of this course, and " + f"'{REPEATED_CUSTOM_PAGE_MESSAGE}' when a page is named twice; under non_field_errors, " + f"'{ORDER_NOT_SAVED_MESSAGE}' when the course's tabs cannot be saved in the new order." + ), + parameters=[_COURSE_KEY_PARAMETER], + request={"application/json": CustomPageOrderSerializer}, + responses={ + 204: OpenApiResponse(description="The custom pages are in the new order."), + 400: _error("The body is invalid, names a page twice or one that is not a custom page of this " + "course, or the course's tabs cannot be saved in the new order."), + 401: _error("The requester is not authenticated."), + 403: _error("The requester may not change this course's pages and resources."), + 404: _error("The course does not exist."), + }, + examples=[_ORDER_EXAMPLE], + ) + def update(self, request, course_key): + """Store the new order of the course's custom pages.""" + order = CustomPageOrderSerializer(data=request.data) + order.is_valid(raise_exception=True) + reorder_custom_pages(course_key, order.validated_data["custom_pages"], request.user) + return Response(status=status.HTTP_204_NO_CONTENT) + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CourseTabsViewSet(StandardizedErrorMixin, viewsets.ViewSet): + """ + The tabs of one course's navigation, each addressed by its ``tab_id``. + + A partial update shows or hides one tab from learners. Only tabs whose + ``is_hideable`` is true accept it: built-in tabs such as progress, dates + and wiki do, custom pages do not. + + Anyone who may change the course's pages and resources may change a tab. + The course is looked up first, so a signed-in caller learns that a course + does not exist (404) before being refused (403). + + A plain ``ViewSet``: the tabs are entries of the course's tab list in the + modulestore, not database rows. + """ + + # Kept from the legacy endpoint: inactive accounts' sessions are accepted and the course permission decides. + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated, IsExistingCourse, CanManagePagesAndResources) + serializer_class = CourseTabSerializer + + def get_serializer(self, *args, **kwargs): + """Instantiate the tab serializer declared on this viewset.""" + return self.serializer_class(*args, **kwargs) + + @extend_schema( + summary="Show or hide one of a course's tabs", + description=( + "Hides the tab from learners when is_hidden is true and shows it when false. A body without " + "is_hidden changes nothing. Returns the tab as stored. A tab that may not be hidden or shown " + f"refuses is_hidden with the message '{TAB_NOT_HIDEABLE_MESSAGE}'; a tab_id the course does not " + f"have is answered with 404 and the message '{TAB_NOT_FOUND_MESSAGE}'. Custom pages are among " + "the course's tabs, under tab_id static_tab_ followed by their url_slug, but cannot be hidden " + "or shown here: is_hidden is refused for them." + ), + parameters=[_COURSE_KEY_PARAMETER, _TAB_ID_PARAMETER], + request={"application/json": CourseTabUpdateSerializer}, + responses={ + 200: OpenApiResponse(response=CourseTabSerializer, description="The tab, as stored."), + 400: _error("The body is invalid, or the tab may not be hidden or shown."), + 401: _error("The requester is not authenticated."), + 403: _error("The requester may not change this course's pages and resources."), + 404: _error("The course does not exist, or has no tab with this identifier."), + }, + examples=[_HIDE_EXAMPLE, _TAB_EXAMPLE], + ) + def partial_update(self, request, course_key, tab_id): + """Apply the change to the tab and return it as stored.""" + changes = CourseTabUpdateSerializer(data=request.data, partial=True) + changes.is_valid(raise_exception=True) + tab = update_tab(course_key, tab_id, changes.validated_data, request.user) + return Response(self.get_serializer(tab).data) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/group_configurations.py b/cms/djangoapps/contentstore/rest_api/v1/views/group_configurations.py index 78c03981f087..ec80c4b1e755 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/group_configurations.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/group_configurations.py @@ -20,6 +20,9 @@ class CourseGroupConfigurationsView(DeveloperErrorViewMixin, APIView): """ View for course's settings group configurations. + + Deprecated: use ``GET /api/authoring/v2/courses/{course_key}/group_configurations/`` instead. + See https://github.com/openedx/openedx-platform/issues/39128. """ @extend_schema( @@ -44,6 +47,14 @@ def get(self, request: Request, course_key: CourseKey): """ Get an object containing course's settings group configurations. + Deprecated: use ``GET /api/authoring/v2/courses/{course_key}/group_configurations/``, which + returns ``all_group_configurations`` followed by ``experiment_group_configurations`` as one + paginated list, each with ``can_manage``; ``usage`` only with ``view=full``. An + ``enrollment_track`` item is listed exactly when ``should_show_enrollment_track`` is true. + ``should_show_experiment_groups`` and ``mfe_proctored_exam_settings_url`` describe the + course, not its group configurations, and are not part of that resource. + See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** GET /api/contentstore/v1/group_configurations/{course_id} diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/helpers.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/helpers.py new file mode 100644 index 000000000000..08e7c5748e43 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/helpers.py @@ -0,0 +1,254 @@ +"""Shared helpers for the authoring v1 endpoint tests.""" + +from contextlib import contextmanager +from functools import cache +from unittest.mock import patch + +import jsonschema +import pymongo +from crum import signals as crum_signals +from django.core import signals as core_signals +from django.db import connection +from django.db.models import signals as model_signals +from django.dispatch import Signal +from django.test.utils import CaptureQueriesContext +from edx_rest_framework_extensions.testing import assert_error_envelope +from openedx_authz.models.engine import PolicyCacheControl +from openedx_events.tooling import OpenEdxPublicSignal +from rest_framework.test import APIClient + +from cms.djangoapps.contentstore.rest_api.v2.views.tests.helpers import generate_cms_schema +from xmodule.modulestore.mixed import MixedModuleStore + +COURSE_NOT_FOUND = "The requested course does not exist." +UNKNOWN_COURSE_KEY = "course-v1:NoSuchOrg+NoSuchCourse+NoSuchRun" +VERSION_ONLY_KEY = "course-v1:version@0123456789abcdef01234567" + + +# --------------------------------------------------------------------------- +# Clients and refusals +# --------------------------------------------------------------------------- + +def api_client(user=None): + """An API client that returns server errors as responses, authenticated as ``user`` if given.""" + client = APIClient() + client.raise_request_exception = False + if user is not None: + client.force_authenticate(user=user) + return client + + +def session_client(user): + """A client signed in to a browser session as ``user``.""" + user.set_password("password") + user.save() + client = APIClient() + assert client.login(username=user.username, password="password") + return client + + +def assert_refused(response, status): + """Assert ``response`` is the error envelope for ``status``; a 404 names the missing course.""" + slug = {401: "authn", 403: "authz", 404: "not-found"}[status] + envelope = assert_error_envelope(response, expected_status=status, expected_type_slug=slug) + if status == 404: + assert envelope["detail"] == COURSE_NOT_FOUND + return envelope + + +# --------------------------------------------------------------------------- +# The published schema +# --------------------------------------------------------------------------- + +@cache +def published_schema(): + """The CMS authoring schema, generated once per test process.""" + return generate_cms_schema() + + +def as_json_schema(node): + """An OpenAPI 3.0 schema as JSON Schema: ``nullable`` becomes a null alternative.""" + if isinstance(node, list): + return [as_json_schema(item) for item in node] + if not isinstance(node, dict): + return node + converted = {key: as_json_schema(value) for key, value in node.items() if key != "nullable"} + if node.get("nullable"): + return {"anyOf": [converted, {"type": "null"}]} + return converted + + +def assert_matches_published_schema(body, component): + """Assert ``body`` is valid against the published schema component ``component``.""" + components = as_json_schema(published_schema()["components"]["schemas"]) + jsonschema.validate(body, {"$ref": f"#/components/schemas/{component}", "components": {"schemas": components}}) + + +# --------------------------------------------------------------------------- +# Comparing two response bodies +# --------------------------------------------------------------------------- + +VOLATILE_FIELDS = {"instance"} +_ABSENT = object() + + +def flatten_json(obj, prefix=""): + """ + A ``{json_path: value}`` map of every leaf of ``obj``, without volatile + members. An empty object or array is a leaf, so a container that empties + or changes kind is seen. + """ + flat = {} + if isinstance(obj, dict) and obj: + for key in sorted(obj): + if key in VOLATILE_FIELDS: + continue + flat.update(flatten_json(obj[key], f"{prefix}.{key}" if prefix else key)) + elif isinstance(obj, list) and obj: + for index, item in enumerate(obj): + flat.update(flatten_json(item, f"{prefix}[{index}]")) + else: + flat[prefix] = obj + return flat + + +def same_json(a, b): + """Whether two leaves are equal and of the same JSON type: true is not 1, 1.0 is not 1, {} is not [].""" + return type(a) is type(b) and a == b + + +def changed_paths(legacy, new): + """The json paths whose leaf differs between the two bodies.""" + a, b = flatten_json(legacy), flatten_json(new) + return {key for key in set(a) | set(b) if not same_json(a.get(key, _ABSENT), b.get(key, _ABSENT))} + + +def undeclared_paths(changed, intentional_differences): + """The ``changed`` paths that no ``(path, reason)`` entry of ``intentional_differences`` covers.""" + declared = [path for path, _ in intentional_differences] + return { + key for key in changed + if not any(key == path or key.startswith(f"{path}.") or key.startswith(f"{path}[") for path in declared) + } + + +# --------------------------------------------------------------------------- +# Side effects and costs +# --------------------------------------------------------------------------- + +# The one write the permission library makes on a database's first role or +# permission lookup, and the model signals its save sends. +FIRST_LOOKUP_WRITES = [("INSERT", PolicyCacheControl._meta.db_table)] # pylint: disable=protected-access + + +FIRST_LOOKUP_SIGNALS = [("pre_save", "PolicyCacheControl"), ("post_save", "PolicyCacheControl")] + + +# Signals every request sends without writing anything. +NEUTRAL_SIGNALS = ( + core_signals.request_started, + core_signals.request_finished, + model_signals.pre_init, + model_signals.post_init, + crum_signals.current_user_getter, + crum_signals.current_user_setter, +) + + +MODEL_SIGNAL_NAMES = { + model_signals.pre_save: "pre_save", + model_signals.post_save: "post_save", + model_signals.pre_delete: "pre_delete", + model_signals.post_delete: "post_delete", + model_signals.m2m_changed: "m2m_changed", +} + + +@contextmanager +def count_mongo_calls(): + """Count Mongo reads and writes made inside the block.""" + reads = ("find", "find_one", "count_documents", "aggregate") + writes = ("insert_one", "insert_many", "replace_one", "update_one", "update_many", "bulk_write", + "delete_one", "delete_many", "find_one_and_update", "find_one_and_replace", "find_one_and_delete") + counts = {"reads": 0, "writes": 0} + + def counting(kind, original): + def wrapper(self, *args, **kwargs): + counts[kind] += 1 + return original(self, *args, **kwargs) + return wrapper + + collection = pymongo.collection.Collection + with patch.multiple(collection, **{ + name: counting("reads" if name in reads else "writes", getattr(collection, name)) for name in reads + writes + }): + yield counts + + +def sql_writes(queries): + """``(statement, table)`` for each INSERT, UPDATE and DELETE among the captured ``queries``.""" + found = [] + for query in queries.captured_queries: + words = query["sql"].split() + statement = words[0].upper() + if statement in ("INSERT", "DELETE"): + found.append((statement, words[2].strip('"`'))) + elif statement == "UPDATE": + found.append((statement, words[1].strip('"`'))) + return found + + +@contextmanager +def record_effects(test_case): + """ + Record, inside the block, SQL writes, Mongo writes, modulestore updates, + every Django signal other than the neutral ones (as ``(name, sender)``) and + every openedx-event. Commit callbacks registered inside the block are run, + so anything sent on commit is seen. + """ + effects = {"signals": [], "events": [], "update_item": []} + send, send_robust, send_event = Signal.send, Signal.send_robust, OpenEdxPublicSignal.send_event + update_item = MixedModuleStore.update_item + + def record(signal, sender): + if any(signal is neutral for neutral in NEUTRAL_SIGNALS): + return + name = MODEL_SIGNAL_NAMES.get(signal, repr(signal)) + effects["signals"].append((name, getattr(sender, "__name__", repr(sender)))) + + def spy_send(signal, sender, **named): + record(signal, sender) + return send(signal, sender, **named) + + def spy_send_robust(signal, sender, **named): + record(signal, sender) + return send_robust(signal, sender, **named) + + def spy_send_event(signal, *args, **kwargs): + effects["events"].append(signal.event_type) + return send_event(signal, *args, **kwargs) + + def spy_update_item(store, xblock, *args, **kwargs): + effects["update_item"].append(str(xblock.location)) + return update_item(store, xblock, *args, **kwargs) + + with CaptureQueriesContext(connection) as queries, \ + count_mongo_calls() as mongo, \ + patch.object(Signal, "send", spy_send), \ + patch.object(Signal, "send_robust", spy_send_robust), \ + patch.object(OpenEdxPublicSignal, "send_event", spy_send_event), \ + patch.object(MixedModuleStore, "update_item", spy_update_item): + # Innermost, so the callbacks run on its exit while every spy above is installed. + with test_case.captureOnCommitCallbacks(execute=True): + yield effects + effects["sql_writes"] = sql_writes(queries) + effects["mongo_writes"] = mongo["writes"] + + +def measure(request): + """SQL queries, Mongo reads and Mongo writes of the second of two identical requests.""" + request() + with CaptureQueriesContext(connection) as queries, count_mongo_calls() as mongo: + response = request() + assert response.status_code == 200, response.content + return len(queries.captured_queries), mongo["reads"], mongo["writes"] diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_advanced_settings.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_advanced_settings.py new file mode 100644 index 000000000000..27dded1e7e28 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_advanced_settings.py @@ -0,0 +1,1392 @@ +"""Tests for the v1 course advanced settings API.""" + +import json +import uuid +from datetime import UTC, datetime, timedelta +from types import SimpleNamespace +from unittest.mock import patch + +import ddt +import pytest +from django.core.exceptions import ImproperlyConfigured +from django.db import transaction +from django.dispatch import Signal +from django.test import SimpleTestCase, TestCase +from django.test.utils import override_settings +from django.urls import Resolver404, resolve, reverse +from edx_rest_framework_extensions.permissions import NotJwtRestrictedApplication +from edx_rest_framework_extensions.testing import assert_error_envelope +from opaque_keys.edx.keys import CourseKey +from openedx_authz.constants.roles import ( + COURSE_ADMIN, + COURSE_AUDITOR, + COURSE_BETA_TESTER, + COURSE_DATA_RESEARCHER, + COURSE_EDITOR, + COURSE_LIMITED_STAFF, + COURSE_STAFF, +) +from openedx_authz.models.engine import PolicyCacheControl +from rest_framework.permissions import IsAuthenticated +from rest_framework.test import APIClient, APIRequestFactory, force_authenticate +from rest_framework.utils.encoders import JSONEncoder + +from cms.djangoapps.contentstore.rest_api.v1.advanced_settings_service import ( + INVALID_SETTING_MESSAGE, + SETTINGS_NOT_SAVED_MESSAGE, +) +from cms.djangoapps.contentstore.rest_api.v1.views.advanced_settings import CourseAdvancedSettingsViewSet +from cms.djangoapps.contentstore.rest_api.v1.views.tests.helpers import ( + FIRST_LOOKUP_SIGNALS, + FIRST_LOOKUP_WRITES, + UNKNOWN_COURSE_KEY, + VERSION_ONLY_KEY, + api_client, + assert_matches_published_schema, + assert_refused, + changed_paths, + measure, + published_schema, + record_effects, + session_client, + undeclared_paths, +) +from cms.djangoapps.contentstore.tests.utils import routed_actions +from cms.djangoapps.contentstore.views.permissions import ( + CanManageAdvancedSettings, + CanViewAdvancedSettings, + IsExistingCourse, +) +from cms.djangoapps.models.settings.course_metadata import CourseMetadata +from cms.lib.spectacular import LEGACY_MIGRATED_PATH_PREFIXES, SUPERSEDED_PATHS +from common.djangoapps.student.auth import check_course_advanced_settings_access, has_studio_advanced_settings_access +from common.djangoapps.student.models import CourseAccessRole +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseLimitedStaffRole, + CourseStaffRole, + OrgInstructorRole, + OrgStaffRole, +) +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin +from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user +from openedx.core.djangoapps.oauth_dispatch.tests.factories import AccessTokenFactory, ApplicationFactory +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.mixed import MixedModuleStore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory +from xmodule.tabs import InvalidTabsException + +URL_NAME = "authoring_v1:course_advanced_settings" +LEGACY_URL_NAME = "cms.djangoapps.contentstore:v0:course_advanced_settings" +LEGACY_VIEW = ("cms.djangoapps.contentstore.rest_api.v0.views.advanced_settings", "AdvancedCourseSettingsView") +SERVICE_LOGGER = "cms.djangoapps.contentstore.rest_api.v1.advanced_settings_service" +FULL = {"view": "full"} +LEGACY_DEFAULT = {"fetch_all": "0"} +# An integer setting authors edit, so a non-numeric value is refused by its field. +INTEGER_SETTING = "max_student_enrollments_allowed" +SETTING_MEMBERS = {"value", "display_name", "help", "deprecated", "hide_on_enabled_publisher"} +ACTIONS = ("read", "update") + + +def url(course_key): + """Return the address of the advanced settings of ``course_key``.""" + return reverse(URL_NAME, kwargs={"course_key": str(course_key)}) + + +def legacy_url(course_key): + """Return the deprecated advanced settings address of ``course_key``.""" + return reverse(LEGACY_URL_NAME, kwargs={"course_id": str(course_key)}) + + +def stored_course(course_key): + """The stored course block of ``course_key`` (a key or its string form), or None when there is none.""" + return modulestore().get_course(CourseKey.from_string(str(course_key))) + + +def stored_display_name(course_key): + """The display name stored for the course.""" + return stored_course(course_key).display_name + + +def editable_settings(course_key): + """The settings authors edit, as the settings model returns them for the stored course.""" + return CourseMetadata.fetch(stored_course(course_key)) + + +def every_setting(course_key): + """Every setting of the stored course, as the settings model returns them.""" + return CourseMetadata.fetch_all(stored_course(course_key)) + + +# --------------------------------------------------------------------------- +# The published schema +# --------------------------------------------------------------------------- + + +# --------------------------------------------------------------------------- +# Who may call the endpoint +# --------------------------------------------------------------------------- + +class _AccessMixin: + """ + Checks a caller's status on a read or an update against the expected one, + and that v0 decides the same for the same caller. + """ + + def assert_read(self, user, expected, course_key=None): + """Assert the status of a read by ``user``, its body or envelope, and agreement with v0.""" + course_key = course_key or self.course.id + response = api_client(user).get(url(course_key)) + legacy = api_client(user).get(legacy_url(course_key), LEGACY_DEFAULT) + assert response.status_code == expected, response.content + if expected == 200: + assert list(response.json()) == list(editable_settings(course_key)) + else: + assert_refused(response, expected) + assert legacy.status_code == expected, f"v0 decided {legacy.status_code} for the same caller" + + def assert_update(self, user, expected, course_key=None): + """Assert the status of an update by ``user``, what it stored, and agreement with v0.""" + course_key = course_key or self.course.id + course = stored_course(course_key) + before = course.display_name if course is not None else None + new_name = f"Renamed {uuid.uuid4().hex[:8]}" + response = api_client(user).patch(url(course_key), {"display_name": {"value": new_name}}, format="json") + assert response.status_code == expected, response.content + if expected == 200: + assert response.json()["display_name"]["value"] == new_name + assert stored_display_name(course_key) == new_name + else: + assert_refused(response, expected) + if course is not None: + assert stored_display_name(course_key) == before + legacy_name = f"Legacy {uuid.uuid4().hex[:8]}" + legacy = api_client(user).patch(legacy_url(course_key), {"display_name": {"value": legacy_name}}, format="json") + assert legacy.status_code == expected, f"v0 decided {legacy.status_code} for the same caller" + + def assert_access(self, user, action, expected, course_key=None): + """Assert ``expected`` for ``action`` (``read`` or ``update``) by ``user``.""" + getattr(self, f"assert_{action}")(user, expected, course_key) + + +@ddt.ddt +class AdvancedSettingsAccessLegacyPermissionsTest(_AccessMixin, SharedModuleStoreTestCase): + """Who may read and change the settings when policy-based authoring permissions are off.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + cls.other_course = CourseFactory.create() + + def _user_with(self, role): + user = UserFactory() + role.add_users(user) + return user + + @ddt.data(*ACTIONS) + def test_anonymous_caller_is_refused_with_401(self, action): + self.assert_access(None, action, 401) + + @ddt.data(*ACTIONS) + def test_anonymous_caller_of_an_unknown_course_is_refused_with_401(self, action): + self.assert_access(None, action, 401, course_key=UNKNOWN_COURSE_KEY) + + @ddt.data(*ACTIONS) + def test_user_without_a_role_is_refused_with_403(self, action): + self.assert_access(UserFactory(), action, 403) + + @ddt.data(*ACTIONS) + def test_unknown_course_is_404_before_permission_is_checked(self, action): + self.assert_access(UserFactory(), action, 404, course_key=UNKNOWN_COURSE_KEY) + + @ddt.data(*ACTIONS) + def test_limited_staff_is_refused(self, action): + self.assert_access(self._user_with(CourseLimitedStaffRole(self.course.id)), action, 403) + + @ddt.data(*ACTIONS) + def test_staff_and_instructors_of_another_course_are_refused(self, action): + self.assert_access(self._user_with(CourseStaffRole(self.other_course.id)), action, 403) + self.assert_access(self._user_with(CourseInstructorRole(self.other_course.id)), action, 403) + + @ddt.data(*ACTIONS) + def test_course_staff_is_allowed(self, action): + self.assert_access(self._user_with(CourseStaffRole(self.course.id)), action, 200) + + @ddt.data(*ACTIONS) + def test_course_instructor_is_allowed(self, action): + self.assert_access(self._user_with(CourseInstructorRole(self.course.id)), action, 200) + + @ddt.data(*ACTIONS) + def test_org_staff_and_org_instructor_are_allowed(self, action): + self.assert_access(self._user_with(OrgStaffRole(self.course.id.org)), action, 200) + self.assert_access(self._user_with(OrgInstructorRole(self.course.id.org)), action, 200) + + @ddt.data(*ACTIONS) + def test_global_staff_is_allowed(self, action): + self.assert_access(UserFactory(is_staff=True), action, 200) + + @ddt.data(*ACTIONS) + def test_a_superuser_who_is_not_global_staff_is_allowed(self, action): + """The role checks consult the policy engine even here, and it admits superusers, as on v0.""" + self.assert_access(UserFactory(is_superuser=True, is_staff=False), action, 200) + + def test_a_version_only_course_key_is_404(self): + """The key parses but names no course run; v0 has no address that takes it.""" + user = UserFactory(is_staff=True) + assert_refused(api_client(user).get(url(VERSION_ONLY_KEY)), 404) + assert_refused( + api_client(user).patch(url(VERSION_ONLY_KEY), {"display_name": {"value": "x"}}, format="json"), 404, + ) + + def test_a_library_is_not_a_course(self): + library_key = LibraryFactory.create().location.library_key + user = UserFactory(is_staff=True) + assert_refused(api_client(user).get(url(library_key)), 404) + assert_refused(api_client(user).patch(url(library_key), {}, format="json"), 404) + + +@ddt.ddt +class AdvancedSettingsAccessAuthzTest(CourseAuthoringAuthzTestMixin, _AccessMixin, SharedModuleStoreTestCase): + """ + Who may read and change the settings when policy-based authoring + permissions are on: viewing needs the permission to view advanced + settings, changing them the permission to manage them. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + cls.other_course = CourseFactory.create() + + def _user_with(self, role, course_key=None): + user = UserFactory() + self.add_user_to_role_in_course(user, role, course_key or self.course.id) + return user + + @ddt.data(*ACTIONS) + def test_anonymous_caller_is_refused_with_401(self, action): + self.assert_access(None, action, 401) + + @ddt.data(*ACTIONS) + def test_user_without_a_role_is_refused_with_403(self, action): + self.assert_access(self.unauthorized_user, action, 403) + + @ddt.data(*ACTIONS) + def test_unknown_course_is_404_before_permission_is_checked(self, action): + self.assert_access(self.unauthorized_user, action, 404, course_key=UNKNOWN_COURSE_KEY) + + @ddt.data(COURSE_AUDITOR.external_key, COURSE_EDITOR.external_key) + def test_roles_that_may_only_view_may_read_but_not_update(self, role): + user = self._user_with(role) + self.assert_read(user, 200) + self.assert_update(user, 403) + + @ddt.data(*[(action, role) for action in ACTIONS for role in ( + COURSE_STAFF.external_key, COURSE_ADMIN.external_key, + )]) + @ddt.unpack + def test_roles_that_may_manage_may_read_and_update(self, action, role): + self.assert_access(self._user_with(role), action, 200) + + @ddt.data(*[(action, role) for action in ACTIONS for role in ( + COURSE_LIMITED_STAFF.external_key, COURSE_DATA_RESEARCHER.external_key, COURSE_BETA_TESTER.external_key, + )]) + @ddt.unpack + def test_other_roles_are_refused(self, action, role): + self.assert_access(self._user_with(role), action, 403) + + @ddt.data(*ACTIONS) + def test_a_role_on_another_course_is_refused(self, action): + user = self._user_with(COURSE_ADMIN.external_key, self.other_course.id) + self.assert_access(user, action, 200, course_key=self.other_course.id) + self.assert_access(user, action, 403) + + @ddt.data(*ACTIONS) + def test_legacy_course_staff_without_a_policy_role_is_refused(self, action): + user = UserFactory() + CourseAccessRole.objects.create(user=user, role="staff", course_id=self.course.id, org=self.course.id.org) + self.assert_access(user, action, 403) + + @ddt.data(*ACTIONS) + def test_staff_and_superuser_are_allowed_without_a_role(self, action): + self.assert_access(self.staff_user, action, 200) + self.assert_access(self.super_user, action, 200) + + def test_an_inactive_session_with_a_role_is_accepted(self): + """The policy does not look at whether the account is activated, on v1 as on v0.""" + user = self._user_with(COURSE_AUDITOR.external_key) + client = session_client(user) + user.is_active = False + user.save() + assert client.get(url(self.course.id)).status_code == 200 + assert client.get(legacy_url(self.course.id)).status_code == 200 + + +@override_settings(DISABLE_ADVANCED_SETTINGS=True) +class AdvancedSettingsSiteRestrictionLegacyPermissionsTest(_AccessMixin, SharedModuleStoreTestCase): + """ + With advanced settings restricted to staff site-wide and policy-based + permissions off, course staff still read and change them here, as on v0. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super().setUp() + self.course_staff = UserFactory() + CourseStaffRole(self.course.id).add_users(self.course_staff) + + def test_the_site_restriction_is_on(self): + """The Studio advanced settings page would refuse this caller.""" + assert has_studio_advanced_settings_access(self.course_staff) is False + assert check_course_advanced_settings_access(self.course_staff, self.course.id, "feature_restricted") is False + + def test_course_staff_may_still_read_and_update(self): + self.assert_read(self.course_staff, 200) + self.assert_update(self.course_staff, 200) + + def test_a_caller_without_a_role_is_still_refused(self): + self.assert_read(UserFactory(), 403) + self.assert_update(UserFactory(), 403) + + +@override_settings(DISABLE_ADVANCED_SETTINGS=True) +class AdvancedSettingsSiteRestrictionAuthzTest(CourseAuthoringAuthzTestMixin, _AccessMixin, SharedModuleStoreTestCase): + """ + With advanced settings restricted to staff site-wide and policy-based + permissions on, the policy alone still decides here, as on v0. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def _user_with(self, role): + user = UserFactory() + self.add_user_to_role_in_course(user, role, self.course.id) + return user + + def test_the_site_restriction_is_on(self): + """The restricted check that Studio pages use would refuse a course staff role.""" + user = self._user_with(COURSE_STAFF.external_key) + assert check_course_advanced_settings_access(user, self.course.id, "feature_restricted") is False + + def test_a_role_that_may_manage_may_still_read_and_update(self): + user = self._user_with(COURSE_STAFF.external_key) + self.assert_read(user, 200) + self.assert_update(user, 200) + + def test_a_role_that_may_only_view_still_may_not_update(self): + user = self._user_with(COURSE_EDITOR.external_key) + self.assert_read(user, 200) + self.assert_update(user, 403) + + +class AdvancedSettingsPermissionWiringTest(SimpleTestCase): + """Which permission each action checks, and which access each settings permission asks for.""" + + def test_each_action_checks_the_course_then_its_own_permission(self): + view = CourseAdvancedSettingsViewSet() + for action, expected in ( + ("retrieve", [IsAuthenticated, IsExistingCourse, CanViewAdvancedSettings]), + ("partial_update", [IsAuthenticated, IsExistingCourse, CanManageAdvancedSettings]), + (None, [IsAuthenticated, IsExistingCourse]), + ("metadata", [IsAuthenticated, IsExistingCourse]), + ): + view.action = action + # The JWT middleware appends NotJwtRestrictedApplication to views that take JWTs. + declared = [ + type(permission) for permission in view.get_permissions() + if not isinstance(permission, NotJwtRestrictedApplication) + ] + assert declared == expected, action + + def test_an_action_without_a_declared_permission_is_refused(self): + view = CourseAdvancedSettingsViewSet() + for action in ("destroy", "update", "list", "create"): + view.action = action + with pytest.raises(ImproperlyConfigured): + view.get_permissions() + + def test_a_routed_action_without_a_declared_permission_is_never_served(self): + """A request reaching such an action is answered with a server error, not the handler's body.""" + + class Unchecked(CourseAdvancedSettingsViewSet): + permission_classes = CourseAdvancedSettingsViewSet.permission_classes + + def destroy(self, request, course_key): # pylint: disable=unused-argument + raise AssertionError("the handler ran without a permission check") + + view = Unchecked.as_view({"delete": "destroy"}) + request = APIRequestFactory().delete(url(COURSE_KEY)) + force_authenticate(request, user=SimpleNamespace(is_authenticated=True, is_active=True)) + response = view(request, course_key=CourseKey.from_string(COURSE_KEY)) + assert_error_envelope(response, expected_status=500, expected_type_slug="internal") + + def test_the_settings_permissions_ask_for_read_and_write_access(self): + course_key = "course-v1:edX+Wiring+Run" + request = SimpleNamespace(user=object()) + view = SimpleNamespace(kwargs={"course_key": course_key}) + target = "cms.djangoapps.contentstore.views.permissions.check_course_advanced_settings_access" + for permission, access_type in ((CanViewAdvancedSettings, "read"), (CanManageAdvancedSettings, "write")): + for decision in (True, False): + with patch(target, return_value=decision) as check: + assert permission().has_permission(request, view) is decision + check.assert_called_once_with(request.user, course_key, access_type=access_type) + + def test_the_settings_permissions_refuse_a_view_without_a_course(self): + request = SimpleNamespace(user=object()) + view = SimpleNamespace(kwargs={}) + for permission in (CanViewAdvancedSettings, CanManageAdvancedSettings, IsExistingCourse): + assert permission().has_permission(request, view) is False + + +class AdvancedSettingsAuthenticationTest(SharedModuleStoreTestCase): + """How callers authenticate, with policy-based authoring permissions off.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def _course_staff(self): + user = UserFactory() + CourseStaffRole(self.course.id).add_users(user) + return user + + def test_a_jwt_caller_may_read_and_update(self): + header = {"HTTP_AUTHORIZATION": f"JWT {create_jwt_for_user(self._course_staff())}"} + assert APIClient().get(url(self.course.id), **header).status_code == 200 + response = APIClient().patch( + url(self.course.id), {"display_name": {"value": "By JWT"}}, format="json", **header, + ) + assert response.status_code == 200 + assert stored_display_name(self.course.id) == "By JWT" + + def test_a_session_caller_may_read_and_update(self): + client = session_client(self._course_staff()) + assert client.get(url(self.course.id)).status_code == 200 + response = client.patch(url(self.course.id), {"display_name": {"value": "By session"}}, format="json") + assert response.status_code == 200 + assert stored_display_name(self.course.id) == "By session" + + def test_an_inactive_session_is_authenticated(self): + """ + A session whose account is not activated is accepted, as on v0. Global + staff then pass; course staff are refused (403, not 401) because a + course role does not count while the account is inactive. + """ + for user, expected in ((UserFactory(is_staff=True), 200), (self._course_staff(), 403)): + client = session_client(user) + user.is_active = False + user.save() + response = client.get(url(self.course.id)) + assert response.status_code == expected, response.content + if expected == 403: + assert_refused(response, 403) + assert client.get(legacy_url(self.course.id)).status_code == expected + + def test_a_bearer_token_is_refused(self): + """Bearer tokens are not accepted here, though v0 takes them.""" + user = UserFactory(is_staff=True) + token = AccessTokenFactory(user=user, application=ApplicationFactory(user=user)).token + header = {"HTTP_AUTHORIZATION": f"Bearer {token}"} + assert_refused(APIClient().get(url(self.course.id), **header), 401) + assert APIClient().get(legacy_url(self.course.id), **header).status_code == 200 + + +# --------------------------------------------------------------------------- +# Reads +# --------------------------------------------------------------------------- + +@ddt.ddt +class AdvancedSettingsReadTest(SharedModuleStoreTestCase): + """What a read returns, by view and by named settings.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super().setUp() + reader = UserFactory() + CourseStaffRole(self.course.id).add_users(reader) + self.client = api_client(reader) + + def get(self, **params): + response = self.client.get(url(self.course.id), params) + assert response.status_code == 200, response.content + return response.json() + + def test_the_default_view_is_the_settings_authors_edit_in_course_order(self): + body = self.get() + assert list(body) == list(editable_settings(self.course.id)) + assert "display_name" in body + assert "tabs" not in body + + def test_the_full_view_is_every_setting_in_course_order(self): + body = self.get(**FULL) + assert list(body) == list(every_setting(self.course.id)) + assert "tabs" in body + + def test_the_default_view_leaves_out_exactly_the_excluded_settings(self): + default, full = self.get(), self.get(**FULL) + excluded = set(CourseMetadata.get_exclude_list_of_fields(self.course.id)) + assert set(full) - set(default) == set(full) & excluded + assert set(full) - set(default) != set() + + @ddt.data({}, FULL) + def test_every_setting_carries_the_five_members(self, params): + body = self.get(**params) + assert {name: set(setting) for name, setting in body.items()} == {name: SETTING_MEMBERS for name in body} + + @ddt.data({}, FULL) + def test_the_body_matches_the_published_schema(self, params): + assert_matches_published_schema(self.get(**params), "CourseAdvancedSettings.v1") + + def test_the_values_are_those_the_settings_model_reads(self): + body = self.get(**FULL) + assert {name: setting["value"] for name, setting in body.items()} == { + name: json.loads(json.dumps(setting["value"], cls=JSONEncoder)) + for name, setting in every_setting(self.course.id).items() + } + + def test_fields_narrows_to_the_named_settings_in_course_order(self): + named = {"days_early_for_beta", "display_name"} + body = self.get(fields="days_early_for_beta,display_name") + assert list(body) == [name for name in editable_settings(self.course.id) if name in named] + + def test_fields_ignores_names_outside_the_view(self): + assert self.get(fields="tabs,no_such_setting") == {} + assert list(self.get(fields="tabs,no_such_setting", **FULL)) == ["tabs"] + + def test_fields_ignores_spaces_around_names(self): + assert set(self.get(fields=" display_name , days_early_for_beta ")) == {"display_name", "days_early_for_beta"} + + def test_empty_fields_returns_the_whole_view(self): + assert list(self.get(fields="")) == list(editable_settings(self.course.id)) + + def test_the_minimal_view_is_the_default(self): + assert self.get(view="minimal") == self.get() + assert list(self.get(view="minimal")) == list(editable_settings(self.course.id)) + + def test_an_unknown_view_is_refused(self): + response = self.client.get(url(self.course.id), {"view": "default"}) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"view": ['"default" is not a valid choice.']} + + def test_the_v0_query_parameters_are_ignored(self): + """fetch_all and filter_fields have no meaning here; view and fields replace them.""" + for params in ({}, FULL): + expected = self.get(**params) + assert self.get(**params, fetch_all="0", filter_fields="display_name") == expected + assert self.get(**params, fetch_all="1") == expected + + def test_head_answers_without_a_body(self): + for params in ({}, FULL): + response = self.client.head(url(self.course.id), params) + assert response.status_code == 200 + assert response.content == b"" + + +# --------------------------------------------------------------------------- +# Updates +# --------------------------------------------------------------------------- + +class AdvancedSettingsUpdateTest(ModuleStoreTestCase): + """What an update stores, returns and refuses.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(display_name="Original") + writer = UserFactory() + CourseStaffRole(self.course.id).add_users(writer) + self.writer = writer + self.client = api_client(writer) + + def patch(self, body): + return self.client.patch(url(self.course.id), body, format="json") + + def test_named_settings_are_stored_and_the_editable_settings_returned(self): + response = self.patch({"display_name": {"value": "Renamed"}, "days_early_for_beta": {"value": 3.0}}) + assert response.status_code == 200, response.content + course = modulestore().get_course(self.course.id) + assert (course.display_name, course.days_early_for_beta) == ("Renamed", 3.0) + body = response.json() + assert list(body) == list(editable_settings(self.course.id)) + assert body["display_name"]["value"] == "Renamed" + assert body["days_early_for_beta"]["value"] == 3.0 + assert_matches_published_schema(body, "CourseAdvancedSettings.v1") + + def test_members_other_than_value_are_ignored(self): + response = self.patch({"display_name": {"value": "Renamed", "display_name": "Not stored", "deprecated": True}}) + assert response.status_code == 200, response.content + setting = response.json()["display_name"] + assert (setting["value"], setting["deprecated"]) == ("Renamed", False) + assert setting["display_name"] == str(editable_settings(self.course.id)["display_name"]["display_name"]) + + def test_unknown_settings_and_settings_authors_do_not_edit_are_ignored(self): + start = modulestore().get_course(self.course.id).start + response = self.patch({"no_such_setting": {"value": 1}, "start": {"value": "2030-01-01T00:00:00Z"}}) + assert response.status_code == 200, response.content + assert "no_such_setting" not in response.json() + assert "start" not in response.json() + assert modulestore().get_course(self.course.id).start == start + + def test_a_null_value_is_stored(self): + self.patch({"days_early_for_beta": {"value": 3.0}}) + response = self.patch({"days_early_for_beta": {"value": None}}) + assert response.status_code == 200, response.content + assert modulestore().get_course(self.course.id).days_early_for_beta is None + + def test_a_refused_value_is_reported_with_a_fixed_message_and_nothing_is_stored(self): + body = {INTEGER_SETTING: {"value": "lots"}, "display_name": {"value": "Not stored"}} + with self.assertLogs(SERVICE_LOGGER, "WARNING") as logs: + response = self.patch(body) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {INTEGER_SETTING: [INVALID_SETTING_MESSAGE]} + assert stored_display_name(self.course.id) == "Original" + legacy = self.client.patch(legacy_url(self.course.id), body, format="json") + assert legacy.status_code == 400 + (validator_text,) = [ + entry["message"] for entry in legacy.json()["developer_message"] if entry["key"] == INTEGER_SETTING + ] + assert validator_text != INVALID_SETTING_MESSAGE + assert validator_text not in response.content.decode() + (record,) = logs.output + assert validator_text in record + assert INTEGER_SETTING in record + assert str(self.course.id) in record + + def test_several_refused_settings_are_each_named(self): + body = {INTEGER_SETTING: {"value": "lots"}, "days_early_for_beta": {"value": "soon"}} + with self.assertLogs(SERVICE_LOGGER, "WARNING") as logs: + response = self.patch(body) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == { + INTEGER_SETTING: [INVALID_SETTING_MESSAGE], "days_early_for_beta": [INVALID_SETTING_MESSAGE], + } + assert len(logs.output) == 2 + + def test_a_body_that_is_not_an_object_is_refused(self): + envelope = assert_error_envelope(self.patch([1, 2]), expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"non_field_errors": ["Invalid data. Expected a dictionary, but got list."]} + + def test_each_entry_must_be_an_object_with_a_value(self): + response = self.patch({"display_name": "Renamed", "days_early_for_beta": {}}) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == { + "display_name": ["Invalid data. Expected a dictionary, but got str."], + "days_early_for_beta": ["This field is required."], + } + assert stored_display_name(self.course.id) == "Original" + + def test_an_entry_that_is_not_an_object_is_refused_by_v0_too(self): + body = {"display_name": "Renamed"} + assert self.patch(body).status_code == 400 + assert self.client.patch(legacy_url(self.course.id), body, format="json").status_code == 400 + assert stored_display_name(self.course.id) == "Original" + + def test_a_malformed_entry_of_a_setting_authors_do_not_edit_is_ignored(self): + start = stored_course(self.course.id).start + response = self.patch({"tabs": "not tabs", "start": {}, "display_name": {"value": "Renamed"}}) + assert response.status_code == 200, response.content + assert stored_display_name(self.course.id) == "Renamed" + assert stored_course(self.course.id).start == start + + def test_a_failure_saving_the_tabs_is_reported_without_its_text(self): + target = "cms.djangoapps.contentstore.views.course._refresh_course_tabs" + with ( + patch(target, side_effect=InvalidTabsException("tab internals")), + self.assertLogs(SERVICE_LOGGER, "WARNING"), + ): + response = self.patch({"display_name": {"value": "Not stored"}}) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"non_field_errors": [SETTINGS_NOT_SAVED_MESSAGE]} + assert b"tab internals" not in response.content + assert b"Tabs Exception" not in response.content + assert stored_display_name(self.course.id) == "Original" + with patch(target, side_effect=InvalidTabsException("tab internals")): + legacy = self.client.patch(legacy_url(self.course.id), {"display_name": {"value": "x"}}, format="json") + assert legacy.status_code == 400 + + def test_an_unexpected_validation_failure_is_reported_without_its_text(self): + target = "cms.djangoapps.models.settings.course_metadata.CourseMetadata.validate_and_update_from_json" + with patch(target, side_effect=TypeError("validator internals")), self.assertLogs(SERVICE_LOGGER, "WARNING"): + response = self.patch({"display_name": {"value": "Not stored"}}) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"non_field_errors": [SETTINGS_NOT_SAVED_MESSAGE]} + assert b"validator internals" not in response.content + assert stored_display_name(self.course.id) == "Original" + with patch(target, side_effect=TypeError("validator internals")): + legacy = self.client.patch(legacy_url(self.course.id), {"display_name": {"value": "x"}}, format="json") + assert legacy.status_code == 400 + assert b"validator internals" in legacy.content + + +# Requests v0 and v1 answer differently beyond the error body, as +# (request, v0 answer, v1 answer, test in AdvancedSettingsResponseDifferencesTest). +RESPONSE_DIFFERENCES = [ + ("read or update of a course the modulestore returns nothing for", "500", "404", + "test_a_course_the_modulestore_returns_nothing_for"), + ("update whose body is not an object", "500", "400", "test_an_update_whose_body_is_not_an_object"), + ("update entry without a value", "500", "400", "test_an_update_entry_without_a_value"), + ("read with view=default", "200", "400", "test_an_unknown_view"), + ("read or update with a Bearer token", "200", "401", "test_a_bearer_token"), + ("slash-separated course key", "404 JSON", "404 HTML", "test_a_slash_separated_course_key"), +] + + +class AdvancedSettingsResponseDifferencesTest(ModuleStoreTestCase): + """Each request v0 and v1 answer differently, both answers observed.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(display_name="Original") + writer = UserFactory() + CourseStaffRole(self.course.id).add_users(writer) + self.client = api_client(writer) + + def both(self, method, body=None, **params): + """The v0 and v1 responses to the same request.""" + call = getattr(self.client, method) + kwargs = {"format": "json"} if body is not None else {} + data = body if body is not None else params + return call(legacy_url(self.course.id), data, **kwargs), call(url(self.course.id), data, **kwargs) + + def test_every_listed_difference_has_its_test(self): + assert [name for *_, name in RESPONSE_DIFFERENCES if not hasattr(self, name)] == [] + + def test_a_course_the_modulestore_returns_nothing_for(self): + with patch.object(MixedModuleStore, "get_course", return_value=None): + for method, body in (("get", None), ("patch", {"display_name": {"value": "x"}})): + legacy, new = self.both(method, body) + assert legacy.status_code == 500, method + assert_refused(new, 404) + + def test_an_update_whose_body_is_not_an_object(self): + legacy, new = self.both("patch", [1, 2]) + assert legacy.status_code == 500 + envelope = assert_error_envelope(new, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"non_field_errors": ["Invalid data. Expected a dictionary, but got list."]} + + def test_an_update_entry_without_a_value(self): + legacy, new = self.both("patch", {"display_name": {}}) + assert legacy.status_code == 500 + envelope = assert_error_envelope(new, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"display_name": ["This field is required."]} + assert stored_display_name(self.course.id) == "Original" + + def test_an_unknown_view(self): + legacy, new = self.both("get", view="default") + assert legacy.status_code == 200 + assert_error_envelope(new, expected_status=400, expected_type_slug="validation") + + def test_a_bearer_token(self): + user = UserFactory(is_staff=True) + token = AccessTokenFactory(user=user, application=ApplicationFactory(user=user)).token + header = {"HTTP_AUTHORIZATION": f"Bearer {token}"} + assert APIClient().get(legacy_url(self.course.id), **header).status_code == 200 + assert_refused(APIClient().get(url(self.course.id), **header), 401) + body = {"display_name": {"value": "By Bearer"}} + assert APIClient().patch(legacy_url(self.course.id), body, format="json", **header).status_code == 200 + assert_refused(APIClient().patch(url(self.course.id), body, format="json", **header), 401) + + def test_a_slash_separated_course_key(self): + """v0 routes the slash form and reports the course unknown in JSON; v1 has no route for it.""" + legacy = self.client.get("/api/contentstore/v0/advanced_settings/edX/NoSuch/Course") + new = self.client.get("/api/authoring/v1/courses/edX/NoSuch/Course/advanced_settings/") + assert legacy.status_code == new.status_code == 404 + assert legacy.json()["error_code"] == "course_does_not_exist" + assert new["Content-Type"].startswith("text/html") + + +@ddt.ddt +@override_settings( + ENABLE_PROCTORED_EXAMS=True, + PROCTORING_BACKENDS={"DEFAULT": "test_proctoring_provider", "valid_provider": {}}, + PARTNER_SUPPORT_EMAIL="support@example.com", +) +class AdvancedSettingsProctoringProviderTest(ModuleStoreTestCase): + """ + Once a course has started, only global staff may change its proctoring + provider, on v1 as on v0. Before it starts, course staff may. + """ + + CHANGE = {"proctoring_provider": {"value": "valid_provider"}} + + def _course(self, started): + offset = timedelta(days=-1 if started else 30) + return CourseFactory.create(start=datetime.now(UTC) + offset) + + def _course_staff(self, course): + user = UserFactory() + CourseStaffRole(course.id).add_users(user) + return user + + def _patch(self, version, user, course): + address = url(course.id) if version == "v1" else legacy_url(course.id) + return api_client(user).patch(address, self.CHANGE, format="json") + + @ddt.data("v1", "v0") + def test_course_staff_may_not_change_it_after_the_course_starts(self, version): + course = self._course(started=True) + before = stored_course(course.id).proctoring_provider + assert before != "valid_provider" + response = self._patch(version, self._course_staff(course), course) + assert response.status_code == 400, response.content + if version == "v1": + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert list(envelope["errors"]) == ["proctoring_provider"] + else: + assert [entry["key"] for entry in response.json()["developer_message"]] == ["proctoring_provider"] + assert stored_course(course.id).proctoring_provider == before + + @ddt.data("v1", "v0") + def test_global_staff_may_change_it_after_the_course_starts(self, version): + course = self._course(started=True) + response = self._patch(version, UserFactory(is_staff=True), course) + assert response.status_code == 200, response.content + assert response.json()["proctoring_provider"]["value"] == "valid_provider" + assert stored_course(course.id).proctoring_provider == "valid_provider" + + @ddt.data("v1", "v0") + def test_course_staff_may_change_it_before_the_course_starts(self, version): + course = self._course(started=False) + response = self._patch(version, self._course_staff(course), course) + assert response.status_code == 200, response.content + assert stored_course(course.id).proctoring_provider == "valid_provider" + + +# --------------------------------------------------------------------------- +# Parity with v0 +# --------------------------------------------------------------------------- + +# Members of a response body that differ between v0 and v1 for the same +# request. The comparison asserts nothing else differs, and that each of these +# occurs in the error cases below. +INTENTIONAL_DIFFERENCES = [ + ("type", "errors use the standardized envelope"), + ("title", "errors use the standardized envelope"), + ("status", "errors use the standardized envelope"), + ("detail", "errors use the standardized envelope"), + ("errors", "a refused update maps the name of each refused setting to INVALID_SETTING_MESSAGE, where v0 " + "lists the validator's own text under developer_message"), + ("developer_message", "the v0 error body carries developer_message; the envelope does not"), + ("error_code", "the v0 not-found body carries error_code; the envelope does not"), +] + +# Reads of the two addresses that return the same settings. Their parameters +# differ by design: the default view is v0's fetch_all=0 response, view=full +# is v0's default, and fields replaces filter_fields. +EQUIVALENT_READS = [ + ({}, LEGACY_DEFAULT), + (FULL, {}), + ( + {"fields": "display_name,days_early_for_beta"}, + {**LEGACY_DEFAULT, "filter_fields": "display_name,days_early_for_beta"}, + ), + ({**FULL, "fields": "tabs,display_name"}, {"filter_fields": "tabs,display_name"}), +] + + +def _undeclared(changed): + """The changed paths no declared intentional difference covers.""" + return undeclared_paths(changed, INTENTIONAL_DIFFERENCES) + + +def test_the_comparison_sees_types_order_free_members_and_empty_containers(): + assert changed_paths({"a": {"b": []}}, {"a": {}}) == {"a", "a.b"} + assert changed_paths({"x": True}, {"x": 1}) == {"x"} + assert changed_paths({"x": 1.0}, {"x": 1}) == {"x"} + assert changed_paths({"x": {}}, {"x": []}) == {"x"} + assert changed_paths({"x": None, "y": 1}, {"y": 1}) == {"x"} + assert changed_paths({"instance": "/a"}, {"instance": "/b"}) == set() + assert changed_paths({"x": [1, {"y": False}]}, {"x": [1, {"y": False}]}) == set() + + +def test_only_declared_differences_are_tolerated(): + assert _undeclared({"type", "errors.display_name[0]", "developer_message[0].key"}) == set() + assert _undeclared({"display_name.value", "typed"}) == {"display_name.value", "typed"} + + +class _ParityMixin: + """ + v1 against v0 for the same course and caller: reads and updates return + identical bodies, and errors differ only by the declared differences. + """ + + def reader_for(self, course): + """Return a user who may read and change the course's settings.""" + raise NotImplementedError + + def outsider(self): + """Return a signed-in user with no access to the course.""" + raise NotImplementedError + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(display_name="Parity") + self.reader = self.reader_for(self.course) + self.observed = set() + + def compare(self, legacy, new, expected_status): + """Assert both statuses and that the bodies differ only as declared; return both bodies.""" + assert legacy.status_code == expected_status, legacy.content + assert new.status_code == expected_status, new.content + legacy_body, new_body = json.loads(legacy.content or b"null"), json.loads(new.content or b"null") + changed = changed_paths(legacy_body, new_body) + assert _undeclared(changed) == set(), f"undeclared differences: legacy={legacy_body} new={new_body}" + self.observed |= changed + return legacy_body, new_body + + def test_equivalent_reads_return_identical_bodies(self): + client = api_client(self.reader) + for new_params, legacy_params in EQUIVALENT_READS: + legacy_body, new_body = self.compare( + client.get(legacy_url(self.course.id), legacy_params), + client.get(url(self.course.id), new_params), + 200, + ) + assert changed_paths(legacy_body, new_body) == set(), (new_params, legacy_params) + assert list(new_body) == list(legacy_body) + assert new_body != {} + + def test_the_same_parameters_return_different_settings(self): + """The default view is not v0's default: the difference is exactly the excluded settings.""" + client = api_client(self.reader) + legacy_body = client.get(legacy_url(self.course.id)).json() + new_body = client.get(url(self.course.id)).json() + assert set(legacy_body) - set(new_body) == set(every_setting(self.course.id)) - set( + editable_settings(self.course.id) + ) + assert set(new_body) < set(legacy_body) + assert {name: legacy_body[name] for name in new_body} == new_body + + def test_an_update_returns_an_identical_body(self): + body = {"display_name": {"value": "Changed by both"}, "days_early_for_beta": {"value": 2.0}} + client = api_client(self.reader) + new = client.patch(url(self.course.id), body, format="json") + legacy = client.patch(legacy_url(self.course.id), body, format="json") + legacy_body, new_body = self.compare(legacy, new, 200) + assert changed_paths(legacy_body, new_body) == set() + assert new_body["display_name"]["value"] == "Changed by both" + + def _error_pairs(self): + """``(legacy, new, status)`` for each error both addresses produce.""" + refused = {INTEGER_SETTING: {"value": "lots"}} + cases = [ + (api_client(), self.course.id, "get", None, 401), + (api_client(self.outsider()), self.course.id, "get", None, 403), + (api_client(self.outsider()), UNKNOWN_COURSE_KEY, "get", None, 404), + (api_client(self.reader), self.course.id, "patch", refused, 400), + ] + for client, course_key, method, body, status in cases: + call = getattr(client, method) + kwargs = {"format": "json"} if body is not None else {} + legacy = call(legacy_url(course_key), body, **kwargs) + new = call(url(course_key), body, **kwargs) + yield legacy, new, status + + def test_errors_differ_only_by_the_envelope(self): + for legacy, new, status in self._error_pairs(): + self.compare(legacy, new, status) + + def test_a_refused_update_names_the_settings_v0_names_with_a_fixed_message(self): + *_, (legacy, new, status) = self._error_pairs() + legacy_body, new_body = self.compare(legacy, new, status) + validator_texts = {entry["key"]: entry["message"] for entry in legacy_body["developer_message"]} + assert new_body["errors"] == {name: [INVALID_SETTING_MESSAGE] for name in validator_texts} + assert INVALID_SETTING_MESSAGE not in validator_texts.values() + assert [text for text in validator_texts.values() if text in new.content.decode()] == [] + + def test_a_malformed_entry_of_a_setting_authors_do_not_edit_is_ignored_by_both(self): + client = api_client(self.reader) + for body in ({"tabs": "not tabs"}, {"start": {}}, {"tabs": "not tabs", "display_name": {"value": "Both"}}): + new = client.patch(url(self.course.id), body, format="json") + legacy = client.patch(legacy_url(self.course.id), body, format="json") + legacy_body, new_body = self.compare(legacy, new, 200) + assert changed_paths(legacy_body, new_body) == set(), body + + def test_every_declared_difference_occurs(self): + for legacy, new, status in self._error_pairs(): + self.compare(legacy, new, status) + missing = [ + path for path, _ in INTENTIONAL_DIFFERENCES + if not any(key == path or key.startswith(f"{path}.") or key.startswith(f"{path}[") for key in self.observed) + ] + assert missing == [] + + +class AdvancedSettingsParityLegacyPermissionsTest(_ParityMixin, ModuleStoreTestCase): + """Parity read and written by course staff, with policy-based authoring permissions off.""" + + def reader_for(self, course): + reader = UserFactory() + CourseStaffRole(course.id).add_users(reader) + return reader + + def outsider(self): + return UserFactory() + + +class AdvancedSettingsParityAuthzTest(_ParityMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase): + """Parity read and written by a course staff role, with policy-based authoring permissions on.""" + + def reader_for(self, course): + reader = UserFactory() + self.add_user_to_role_in_course(reader, COURSE_STAFF.external_key, course.id) + return reader + + def outsider(self): + return self.unauthorized_user + + +# --------------------------------------------------------------------------- +# Side effects of reads +# --------------------------------------------------------------------------- + + +READS = [("get", {}), ("get", FULL), ("get", {"fields": "display_name"}), ("head", {}), ("head", FULL)] + + +class _ReadsAreSideEffectFreeMixin: + """ + Every read (default, full, named settings, and HEAD on both views) writes + no row and no Mongo document, stores nothing through the modulestore and + sends no signal or event. The reads start on a database that has never + served a role or permission lookup, so the first read may insert the + permission library's one policy version row and nothing else; v0 inserts + the same row. + """ + + def reader_for(self, course): + """Return a user who may read ``course``, leaving the database as no lookup had touched it.""" + raise NotImplementedError + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + + def test_reads_have_no_side_effects(self): + client = api_client(self.reader_for(self.course)) + assert not PolicyCacheControl.objects.exists() + for index, (method, params) in enumerate(READS): + with record_effects(self) as effects: + response = getattr(client, method)(url(self.course.id), params) + read = (index, method, params) + first = index == 0 + assert response.status_code == 200, (read, response.content) + assert effects["sql_writes"] == (FIRST_LOOKUP_WRITES if first else []), read + assert effects["signals"] == (FIRST_LOOKUP_SIGNALS if first else []), read + assert effects["events"] == [], read + assert effects["mongo_writes"] == 0, read + assert effects["update_item"] == [], read + + def test_the_v0_read_makes_the_same_first_lookup_write(self): + client = api_client(self.reader_for(self.course)) + with record_effects(self) as effects: + response = client.get(legacy_url(self.course.id), LEGACY_DEFAULT) + assert response.status_code == 200, response.content + assert effects["sql_writes"] == FIRST_LOOKUP_WRITES + assert effects["signals"] == FIRST_LOOKUP_SIGNALS + assert effects["mongo_writes"] == 0 + + def test_the_recorder_sees_writes_signals_and_modulestore_updates(self): + """The checks above would notice a write: one made inside the recorder is seen.""" + with record_effects(self) as effects: + UserFactory() + course = modulestore().get_course(self.course.id) + modulestore().update_item(course, self.user.id) + assert ("INSERT", "auth_user") in effects["sql_writes"] + assert ("post_save", "User") in effects["signals"] + assert effects["mongo_writes"] > 0 + assert effects["update_item"] == [str(course.location)] + + def test_the_recorder_sees_what_commit_callbacks_do(self): + """Work deferred to the end of the transaction is recorded too.""" + probe = Signal() + + class Sender: + pass + + with record_effects(self) as effects: + transaction.on_commit(lambda: probe.send(sender=Sender)) + transaction.on_commit(lambda: UserFactory(username="made_on_commit")) + assert (repr(probe), "Sender") in effects["signals"] + assert ("INSERT", "auth_user") in effects["sql_writes"] + assert ("post_save", "User") in effects["signals"] + + +class AdvancedSettingsReadSideEffectsLegacyPermissionsTest(_ReadsAreSideEffectFreeMixin, ModuleStoreTestCase): + """Reads by course staff with policy-based authoring permissions off.""" + + def reader_for(self, course): + reader = UserFactory() + CourseStaffRole(course.id).add_users(reader) + PolicyCacheControl.objects.all().delete() + return reader + + +class AdvancedSettingsReadSideEffectsAuthzTest( + _ReadsAreSideEffectFreeMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase, +): + """ + Reads by a course auditor with policy-based authoring permissions on. The + role is assigned through the permission library, which creates the policy + version row; it is deleted before the reads. + """ + + def reader_for(self, course): + reader = UserFactory() + self.add_user_to_role_in_course(reader, COURSE_AUDITOR.external_key, course.id) + PolicyCacheControl.objects.all().delete() + return reader + + +# --------------------------------------------------------------------------- +# Query counts +# --------------------------------------------------------------------------- + + +class _QueryCountMixin: + """Each read costs no more SQL queries or Mongo reads than the v0 read returning the same settings.""" + + def reader_for(self, course): + raise NotImplementedError + + def test_reads_cost_no_more_than_the_equivalent_v0_reads(self): + course = CourseFactory.create() + client = api_client(self.reader_for(course)) + for new_params, legacy_params in EQUIVALENT_READS[:2]: + legacy = measure(lambda params=legacy_params: client.get(legacy_url(course.id), params)) + new = measure(lambda params=new_params: client.get(url(course.id), params)) + assert new[0] <= legacy[0], (new_params, new, legacy) + assert new[1] <= legacy[1], (new_params, new, legacy) + assert new[2] == legacy[2] == 0, (new_params, new, legacy) + + +class AdvancedSettingsQueryCountLegacyPermissionsTest(_QueryCountMixin, ModuleStoreTestCase): + """Costs read by course staff, with policy-based authoring permissions off.""" + + def reader_for(self, course): + reader = UserFactory() + CourseStaffRole(course.id).add_users(reader) + return reader + + +class AdvancedSettingsQueryCountAuthzTest(CourseAuthoringAuthzTestMixin, _QueryCountMixin, ModuleStoreTestCase): + """Costs read by a course auditor, with policy-based authoring permissions on.""" + + def reader_for(self, course): + reader = UserFactory() + self.add_user_to_role_in_course(reader, COURSE_AUDITOR.external_key, course.id) + return reader + + +# --------------------------------------------------------------------------- +# Addresses and methods +# --------------------------------------------------------------------------- + +COURSE_KEY = "course-v1:edX+DemoX+Demo_Course" + + +@ddt.ddt +class AdvancedSettingsRoutingTest(SharedModuleStoreTestCase): + """The address, the v0 address, and requests no route or method serves.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def test_the_address(self): + assert url(COURSE_KEY) == f"/api/authoring/v1/courses/{COURSE_KEY}/advanced_settings/" + match = resolve(url(COURSE_KEY)) + assert match.func.cls is CourseAdvancedSettingsViewSet + assert routed_actions(match.func) == {"get": "retrieve", "patch": "partial_update"} + + def test_the_v0_address_is_unchanged(self): + assert legacy_url(COURSE_KEY) == f"/api/contentstore/v0/advanced_settings/{COURSE_KEY}" + view = resolve(legacy_url(COURSE_KEY)).func.cls + assert (view.__module__, view.__name__) == LEGACY_VIEW + + def test_v0_still_takes_a_slash_separated_key_that_v1_does_not(self): + view = resolve("/api/contentstore/v0/advanced_settings/edX/DemoX/Demo_Course").func.cls + assert (view.__module__, view.__name__) == LEGACY_VIEW + with pytest.raises(Resolver404): + resolve("/api/authoring/v1/courses/edX/DemoX/Demo_Course/advanced_settings/") + + def test_the_url_names_are_unique_in_the_namespace(self): + # Imported here: at module import the URL converters are not registered yet. + from cms.djangoapps.contentstore.rest_api.v1 import authoring_urls # noqa: PLC0415 + names = [pattern.name for pattern in authoring_urls.urlpatterns] + assert len(names) == len(set(names)) + assert "course_advanced_settings" in names + + # The error library does not catalogue MethodNotAllowed or NotAcceptable, so + # these answer with the envelope typed internal. Change the expected type + # here once it does. + + @ddt.data("post", "put", "delete") + def test_other_methods_are_not_allowed(self, method): + response = getattr(api_client(UserFactory(is_staff=True)), method)(url(self.course.id), {}, format="json") + assert_error_envelope(response, expected_status=405, expected_type_slug="internal") + + def test_unsupported_accept_is_refused_with_406(self): + response = api_client(UserFactory(is_staff=True)).get(url(self.course.id), HTTP_ACCEPT="application/xml") + assert_error_envelope(response, expected_status=406, expected_type_slug="internal") + + # Nor ParseError or UnsupportedMediaType: a malformed or unsupported body is + # answered with its own status but typed internal. Change the expected type + # here once the library catalogues them. + + def test_a_malformed_json_body_is_a_400_typed_internal(self): + response = api_client(UserFactory(is_staff=True)).patch( + url(self.course.id), data="{", content_type="application/json", + ) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="internal") + assert envelope["title"] == "Internal Server Error" + + def test_an_unsupported_media_type_is_a_415_typed_internal(self): + response = api_client(UserFactory(is_staff=True)).patch( + url(self.course.id), data="display_name=Renamed", content_type="text/plain", + ) + envelope = assert_error_envelope(response, expected_status=415, expected_type_slug="internal") + assert envelope["title"] == "Internal Server Error" + assert stored_display_name(self.course.id) != "Renamed" + + # These addresses match no route, so the service's HTML 404 page answers + # them. Change these to the error envelope once a catch-all route under the + # mount answers unknown addresses. + + @ddt.data("not-a-course-key", "edX/DemoX/Demo_Course", "edX+DemoX+Demo_Course") + def test_an_unparsable_or_slash_form_key_is_an_html_404(self, key): + response = api_client(UserFactory(is_staff=True)).get(f"/api/authoring/v1/courses/{key}/advanced_settings/") + assert response.status_code == 404 + assert response["Content-Type"].startswith("text/html") + + def test_the_address_without_its_trailing_slash_redirects_to_it(self): + """No route answers the address without its slash; the service redirects a read to the slashed one.""" + address = f"/api/authoring/v1/courses/{self.course.id}/advanced_settings" + response = api_client(UserFactory(is_staff=True)).get(address) + assert response.status_code == 301 + assert response["Location"] == url(self.course.id) + with pytest.raises(Resolver404): + resolve(address) + + +# --------------------------------------------------------------------------- +# The published schema +# --------------------------------------------------------------------------- + +class AdvancedSettingsSchemaTest(TestCase): + """The read and the update as published in the CMS authoring schema.""" + + PATH = "/api/authoring/v1/courses/{course_key}/advanced_settings/" + LEGACY = "/api/contentstore/v0/advanced_settings/{course_id}" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.schema = published_schema() + cls.components = cls.schema["components"]["schemas"] + + @staticmethod + def _ref(node): + return node["content"]["application/json"]["schema"]["$ref"].rsplit("/", 1)[-1] + + def test_the_read_and_the_update_are_published_for_the_sdk(self): + operations = self.schema["paths"][self.PATH] + assert sorted(operations) == ["get", "patch"] + assert operations["get"]["operationId"] == "v1_courses_advanced_settings_retrieve" + assert operations["patch"]["operationId"] == "v1_courses_advanced_settings_partial_update" + for operation in operations.values(): + assert operation["tags"] == ["openedx-platform-sdk"] + assert "deprecated" not in operation + assert self._ref(operation["responses"]["200"]) == "CourseAdvancedSettings.v1" + for status in ("400", "401", "403", "404"): + assert self._ref(operation["responses"][status]) == "ErrorResponse", status + + def test_the_read_parameters(self): + parameters = {p["name"]: p for p in self.schema["paths"][self.PATH]["get"]["parameters"]} + assert set(parameters) == {"course_key", "view", "fields"} + assert parameters["course_key"]["in"] == "path" + assert parameters["course_key"]["schema"] == {"type": "string", "pattern": "^[^/]+$"} + assert parameters["view"]["in"] == parameters["fields"]["in"] == "query" + assert not parameters["view"].get("required") + assert not parameters["fields"].get("required") + assert parameters["view"]["schema"] == {"type": "string", "enum": ["full", "minimal"], "default": "minimal"} + assert parameters["fields"]["schema"] == {"type": "string"} + + def test_the_settings_are_an_object_keyed_by_setting_name(self): + settings = self.components["CourseAdvancedSettings.v1"] + assert settings["type"] == "object" + assert settings["additionalProperties"] == {"$ref": "#/components/schemas/AdvancedSetting.v1"} + assert "properties" not in settings + setting = self.components["AdvancedSetting.v1"] + assert set(setting["properties"]) == SETTING_MEMBERS + assert set(setting["required"]) == SETTING_MEMBERS + nullable = {name for name, field in setting["properties"].items() if field.get("nullable")} + assert nullable == {"value", "help"} + + def test_the_update_body_is_json_settings_each_with_a_value(self): + body = self.schema["paths"][self.PATH]["patch"]["requestBody"] + assert list(body["content"]) == ["application/json"] + assert self._ref(body) == "PatchedCourseAdvancedSettingsUpdate.v1" + update = self.components[self._ref(body)] + assert update["type"] == "object" + assert update["additionalProperties"] == {"$ref": "#/components/schemas/AdvancedSettingUpdate.v1"} + entry = self.components["AdvancedSettingUpdate.v1"] + assert set(entry["properties"]) == {"value"} + assert entry["required"] == ["value"] + assert entry["properties"]["value"]["nullable"] is True + + def test_every_member_is_described(self): + for name in ("CourseAdvancedSettings.v1", "AdvancedSetting.v1", "AdvancedSettingUpdate.v1"): + component = self.components[name] + assert component.get("description"), name + properties = component.get("properties", {}) + assert [field for field, spec in properties.items() if not spec.get("description")] == [], name + + def test_the_examples_are_valid_bodies(self): + get = self.schema["paths"][self.PATH]["get"] + patch_ = self.schema["paths"][self.PATH]["patch"] + response_example = get["responses"]["200"]["content"]["application/json"]["examples"]["AdvancedSettings"] + assert_matches_published_schema(response_example["value"], "CourseAdvancedSettings.v1") + request_example = patch_["requestBody"]["content"]["application/json"]["examples"]["NewValues"] + assert_matches_published_schema(request_example["value"], self._ref(patch_["requestBody"])) + + def test_the_v0_operations_are_deprecated_and_left_out_of_the_sdk(self): + operations = self.schema["paths"][self.LEGACY] + assert sorted(operations) == ["get", "patch"] + assert operations["get"]["operationId"] == "v0_advanced_settings_retrieve" + assert operations["patch"]["operationId"] == "v0_advanced_settings_partial_update" + for operation in operations.values(): + assert operation["deprecated"] is True + assert "openedx-platform-sdk" not in operation.get("tags", []) + + def test_only_superseded_or_migrated_addresses_are_deprecated(self): + """No operation is marked deprecated except by the legacy-address lists, so a stray mark is caught.""" + deprecated = { + (path, method) + for path, operations in self.schema["paths"].items() + for method, operation in operations.items() + if isinstance(operation, dict) and operation.get("deprecated") + } + assert {(self.LEGACY, "get"), (self.LEGACY, "patch")} <= deprecated + assert {(self.PATH, "get"), (self.PATH, "patch")} & deprecated == set() + stray = sorted( + (path, method) for path, method in deprecated + if path not in SUPERSEDED_PATHS and not path.startswith(LEGACY_MIGRATED_PATH_PREFIXES) + ) + assert stray == [] diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_custom_pages.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_custom_pages.py new file mode 100644 index 000000000000..9731376010da --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_custom_pages.py @@ -0,0 +1,1739 @@ +"""Tests for the v1 course custom pages and tabs API.""" + +import json +from types import SimpleNamespace +from unittest.mock import patch +from urllib.parse import urlencode + +import ddt +import pytest +from django.core.exceptions import ImproperlyConfigured +from django.db import connection +from django.test import SimpleTestCase, TestCase +from django.test.utils import CaptureQueriesContext +from django.urls import Resolver404, resolve, reverse +from edx_rest_framework_extensions.permissions import NotJwtRestrictedApplication +from edx_rest_framework_extensions.testing import assert_error_envelope +from edx_toggles.toggles.testutils import override_waffle_flag +from opaque_keys.edx.keys import CourseKey, UsageKey +from openedx_authz.constants.permissions import COURSES_MANAGE_PAGES_AND_RESOURCES, COURSES_VIEW_PAGES_AND_RESOURCES +from openedx_authz.constants.roles import ( + COURSE_ADMIN, + COURSE_AUDITOR, + COURSE_BETA_TESTER, + COURSE_DATA_RESEARCHER, + COURSE_EDITOR, + COURSE_LIMITED_STAFF, + COURSE_STAFF, +) +from openedx_authz.models.engine import PolicyCacheControl +from rest_framework.permissions import IsAuthenticated +from rest_framework.request import Request +from rest_framework.test import APIClient, APIRequestFactory, force_authenticate + +from cms.djangoapps.contentstore.rest_api.pagination import ENVELOPE_KEYS, DocumentedDefaultPagination +from cms.djangoapps.contentstore.rest_api.v1.custom_pages_service import ( + ORDER_NOT_SAVED_MESSAGE, + REPEATED_CUSTOM_PAGE_MESSAGE, + TAB_NOT_FOUND_MESSAGE, + TAB_NOT_HIDEABLE_MESSAGE, + UNKNOWN_CUSTOM_PAGE_MESSAGE, +) +from cms.djangoapps.contentstore.rest_api.v1.serializers.custom_pages import ( + NOT_A_COURSE_KEY_MESSAGE, + NOT_A_USAGE_KEY_MESSAGE, +) +from cms.djangoapps.contentstore.rest_api.v1.views.custom_pages import ( + CourseCustomPageOrderViewSet, + CourseCustomPagesViewSet, + CourseTabsViewSet, + _CourseKeyFromQuery, +) +from cms.djangoapps.contentstore.rest_api.v1.views.tests.helpers import ( + FIRST_LOOKUP_SIGNALS, + FIRST_LOOKUP_WRITES, + UNKNOWN_COURSE_KEY, + VERSION_ONLY_KEY, + api_client, + assert_matches_published_schema, + assert_refused, + changed_paths, + measure, + published_schema, + record_effects, + session_client, + undeclared_paths, +) +from cms.djangoapps.contentstore.tests.utils import routed_actions +from cms.djangoapps.contentstore.utils import get_pages_and_resources_url +from cms.djangoapps.contentstore.views.permissions import ( + CanManagePagesAndResources, + CanViewPagesAndResources, + IsExistingCourse, +) +from cms.lib.spectacular import LEGACY_MIGRATED_PATH_PREFIXES, SUPERSEDED_PATHS +from common.djangoapps.student.models import CourseAccessRole +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseLimitedStaffRole, + CourseStaffRole, + OrgInstructorRole, + OrgStaffRole, +) +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin +from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND +from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user +from openedx.core.djangoapps.oauth_dispatch.tests.factories import AccessTokenFactory, ApplicationFactory +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.mixed import MixedModuleStore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, LibraryFactory +from xmodule.tabs import CourseTabList, InvalidTabsException, StaticTab + +LIST_URL_NAME = "authoring_v1:custom_page_list" +ORDER_URL_NAME = "authoring_v1:course_custom_page_order" +TAB_URL_NAME = "authoring_v1:course_tab_detail" +LEGACY_LIST_URL_NAME = "cms.djangoapps.contentstore:v0:course_tab_list" +LEGACY_ORDER_URL_NAME = "cms.djangoapps.contentstore:v0:course_tab_reorder" +LEGACY_SETTINGS_URL_NAME = "cms.djangoapps.contentstore:v0:course_tab_settings" +LEGACY_MODULE = "cms.djangoapps.contentstore.rest_api.v0.views.tabs" +SERVICE_LOGGER = "cms.djangoapps.contentstore.rest_api.v1.custom_pages_service" +LEGACY_TABS_MODULE = "cms.djangoapps.contentstore.views.tabs" +COURSE_KEY = "course-v1:edX+DemoX+Demo_Course" +# A built-in tab course authors may hide, and one they may not. +HIDEABLE_TAB = "progress" +UNHIDEABLE_TAB = "courseware" +TAB_MEMBERS = { + "type", "title", "is_hideable", "is_hidden", "is_movable", "course_staff_only", "name", "tab_id", "settings", +} +PAGE_MEMBERS = TAB_MEMBERS | {"id"} +ACTIONS = ("list", "reorder", "hide") +# The status each action answers with on success, on v1 and on v0. +SUCCESS = {"list": 200, "reorder": 204, "hide": 200} +LEGACY_SUCCESS = {"list": 200, "reorder": 204, "hide": 204} +OK = "ok" + + +def list_url(course_key=None): + """Return the custom pages address, naming ``course_key`` in its query when given.""" + address = reverse(LIST_URL_NAME) + if course_key is None: + return address + return f"{address}?{urlencode({'course_key': str(course_key)})}" + + +def list_query(course_key, **params): + """ + The query of a listing of the custom pages of ``course_key``. A request + given query data ignores a query already in its address, so listings with + other parameters pass the course this way. + """ + return {"course_key": str(course_key), **params} + + +def order_url(course_key): + """Return the address of the order of the custom pages of ``course_key``.""" + return reverse(ORDER_URL_NAME, kwargs={"course_key": str(course_key)}) + + +def tab_url(course_key, tab_id=HIDEABLE_TAB): + """Return the address of the tab ``tab_id`` of ``course_key``.""" + return reverse(TAB_URL_NAME, kwargs={"course_key": str(course_key), "tab_id": tab_id}) + + +def legacy_list_url(course_key): + """Return the deprecated address listing the tabs of ``course_key``.""" + return reverse(LEGACY_LIST_URL_NAME, kwargs={"course_id": str(course_key)}) + + +def legacy_order_url(course_key): + """Return the deprecated address reordering the tabs of ``course_key``.""" + return reverse(LEGACY_ORDER_URL_NAME, kwargs={"course_id": str(course_key)}) + + +def legacy_settings_url(course_key, tab_id=HIDEABLE_TAB): + """Return the deprecated address changing the tab ``tab_id`` of ``course_key``.""" + return f"{reverse(LEGACY_SETTINGS_URL_NAME, kwargs={'course_id': str(course_key)})}?{urlencode({'tab_id': tab_id})}" + + +def stored_course(course_key): + """The stored course block of ``course_key`` (a key or its string form), or None when there is none.""" + return modulestore().get_course(CourseKey.from_string(str(course_key))) + + +def stored_tab_ids(course_key): + """The ``tab_id`` of each stored tab of the course, in order.""" + return [tab.tab_id for tab in stored_course(course_key).tabs] + + +def stored_tab(course_key, tab_id): + """The stored tab ``tab_id`` of the course.""" + return CourseTabList.get_tab_by_id(stored_course(course_key).tabs, tab_id) + + +def custom_page_keys(course_key): + """The usage keys of the course's stored custom pages, in order; none for a course that does not exist.""" + course = stored_course(course_key) + if course is None: + return [] + return [ + str(course.id.make_usage_key("static_tab", tab.url_slug)) for tab in course.tabs if isinstance(tab, StaticTab) + ] + + +def add_custom_pages(course, count): + """Add ``count`` custom pages to ``course`` and return the usage keys of all its pages, in order.""" + for index in range(count): + BlockFactory.create(parent_location=course.location, category="static_tab", display_name=f"Page {index}") + return custom_page_keys(course.id) + + +def add_custom_page_entries(course_key, count): + """ + Add ``count`` custom page entries to the course's tabs without creating + their content blocks, which listing does not read. + """ + course = stored_course(course_key) + course.tabs.extend(StaticTab(name=f"Entry {index}", url_slug=f"entry_{index:03d}") for index in range(count)) + modulestore().update_item(course, ModuleStoreEnum.UserID.test) + + +# --------------------------------------------------------------------------- +# Who may call the endpoints +# --------------------------------------------------------------------------- + +class _AccessMixin: + """ + Checks a caller's status on each action against the expected one, and + that v0 decides the same for the same caller. + """ + + def assert_list(self, user, expected, course_key=None): + """Assert the status of a listing by ``user``, its body or envelope, and agreement with v0.""" + course_key = course_key or self.course.id + response = api_client(user).get(list_url(), list_query(course_key, page_size=100)) + assert response.status_code == expected, response.content + if expected == 200: + assert [page["id"] for page in response.json()["results"]] == custom_page_keys(course_key) + else: + assert_refused(response, expected) + legacy = api_client(user).get(legacy_list_url(course_key)) + assert legacy.status_code == expected, f"v0 decided {legacy.status_code} for the same caller" + + def assert_reorder(self, user, expected, course_key=None): + """Assert the status of a reorder by ``user``, what it stored, and agreement with v0.""" + course_key = course_key or self.course.id + before = custom_page_keys(course_key) + wanted = before[::-1] + response = api_client(user).put(order_url(course_key), {"custom_pages": wanted}, format="json") + assert response.status_code == expected, response.content + if expected == 204: + assert response.content == b"" + assert custom_page_keys(course_key) == wanted + else: + assert_refused(response, expected) + assert custom_page_keys(course_key) == before + body = [{"tab_locator": key} for key in before] + legacy = api_client(user).post(legacy_order_url(course_key), body, format="json") + expected_legacy = LEGACY_SUCCESS["reorder"] if expected == 204 else expected + assert legacy.status_code == expected_legacy, f"v0 decided {legacy.status_code} for the same caller" + + def assert_hide(self, user, expected, course_key=None): + """Assert the status of a change of a tab's visibility by ``user``, what it stored, and agreement with v0.""" + course_key = course_key or self.course.id + course = stored_course(course_key) + before = stored_tab(course_key, HIDEABLE_TAB).is_hidden if course is not None else False + response = api_client(user).patch(tab_url(course_key), {"is_hidden": not before}, format="json") + assert response.status_code == expected, response.content + if expected == 200: + assert (response.json()["tab_id"], response.json()["is_hidden"]) == (HIDEABLE_TAB, not before) + assert stored_tab(course_key, HIDEABLE_TAB).is_hidden is (not before) + else: + assert_refused(response, expected) + if course is not None: + assert stored_tab(course_key, HIDEABLE_TAB).is_hidden is before + legacy = api_client(user).post(legacy_settings_url(course_key), {"is_hidden": before}, format="json") + expected_legacy = LEGACY_SUCCESS["hide"] if expected == 200 else expected + assert legacy.status_code == expected_legacy, f"v0 decided {legacy.status_code} for the same caller" + + def assert_access(self, user, action, expected, course_key=None): + """Assert ``expected`` (a status, or ``OK`` for the action's success) for ``action`` by ``user``.""" + status = SUCCESS[action] if expected == OK else expected + getattr(self, f"assert_{action}")(user, status, course_key) + + +@ddt.ddt +class CustomPagesAccessLegacyPermissionsTest(_AccessMixin, SharedModuleStoreTestCase): + """Who may list, reorder and change tabs when policy-based authoring permissions are off.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + add_custom_pages(cls.course, 3) + cls.other_course = CourseFactory.create() + + def _user_with(self, role): + user = UserFactory() + role.add_users(user) + return user + + @ddt.data(*ACTIONS) + def test_anonymous_caller_is_refused_with_401(self, action): + self.assert_access(None, action, 401) + + @ddt.data(*ACTIONS) + def test_anonymous_caller_of_an_unknown_course_is_refused_with_401(self, action): + self.assert_access(None, action, 401, course_key=UNKNOWN_COURSE_KEY) + + @ddt.data(*ACTIONS) + def test_user_without_a_role_is_refused_with_403(self, action): + self.assert_access(UserFactory(), action, 403) + + @ddt.data(*ACTIONS) + def test_unknown_course_is_404_before_permission_is_checked(self, action): + self.assert_access(UserFactory(), action, 404, course_key=UNKNOWN_COURSE_KEY) + + @ddt.data(*ACTIONS) + def test_limited_staff_is_refused(self, action): + self.assert_access(self._user_with(CourseLimitedStaffRole(self.course.id)), action, 403) + + @ddt.data(*ACTIONS) + def test_staff_and_instructors_of_another_course_are_refused(self, action): + self.assert_access(self._user_with(CourseStaffRole(self.other_course.id)), action, 403) + self.assert_access(self._user_with(CourseInstructorRole(self.other_course.id)), action, 403) + + @ddt.data(*ACTIONS) + def test_course_staff_is_allowed(self, action): + self.assert_access(self._user_with(CourseStaffRole(self.course.id)), action, OK) + + @ddt.data(*ACTIONS) + def test_course_instructor_is_allowed(self, action): + self.assert_access(self._user_with(CourseInstructorRole(self.course.id)), action, OK) + + @ddt.data(*ACTIONS) + def test_org_staff_and_org_instructor_are_allowed(self, action): + self.assert_access(self._user_with(OrgStaffRole(self.course.id.org)), action, OK) + self.assert_access(self._user_with(OrgInstructorRole(self.course.id.org)), action, OK) + + @ddt.data(*ACTIONS) + def test_global_staff_is_allowed(self, action): + self.assert_access(UserFactory(is_staff=True), action, OK) + + @ddt.data(*ACTIONS) + def test_a_superuser_who_is_not_global_staff_is_allowed(self, action): + """The role checks consult the policy engine even here, and it admits superusers, as on v0.""" + self.assert_access(UserFactory(is_superuser=True, is_staff=False), action, OK) + + def test_a_version_only_course_key_is_404(self): + """The key parses but names no course run; v0 has no address that takes it.""" + user = UserFactory(is_staff=True) + assert_refused(api_client(user).get(list_url(VERSION_ONLY_KEY)), 404) + assert_refused(api_client(user).put(order_url(VERSION_ONLY_KEY), {"custom_pages": []}, format="json"), 404) + assert_refused(api_client(user).patch(tab_url(VERSION_ONLY_KEY), {"is_hidden": True}, format="json"), 404) + + def test_a_library_is_not_a_course(self): + library_key = LibraryFactory.create().location.library_key + user = UserFactory(is_staff=True) + assert_refused(api_client(user).get(list_url(library_key)), 404) + assert_refused(api_client(user).put(order_url(library_key), {"custom_pages": []}, format="json"), 404) + assert_refused(api_client(user).patch(tab_url(library_key), {"is_hidden": True}, format="json"), 404) + + +@ddt.ddt +class CustomPagesAccessAuthzTest(CourseAuthoringAuthzTestMixin, _AccessMixin, SharedModuleStoreTestCase): + """ + Who may list, reorder and change tabs when policy-based authoring + permissions are on: listing needs the permission to view pages and + resources, reordering and changing a tab the permission to manage them. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + add_custom_pages(cls.course, 3) + cls.other_course = CourseFactory.create() + + def _user_with(self, role, course_key=None): + user = UserFactory() + self.add_user_to_role_in_course(user, role, course_key or self.course.id) + return user + + @ddt.data(*ACTIONS) + def test_anonymous_caller_is_refused_with_401(self, action): + self.assert_access(None, action, 401) + + @ddt.data(*ACTIONS) + def test_user_without_a_role_is_refused_with_403(self, action): + self.assert_access(self.unauthorized_user, action, 403) + + @ddt.data(*ACTIONS) + def test_unknown_course_is_404_before_permission_is_checked(self, action): + self.assert_access(self.unauthorized_user, action, 404, course_key=UNKNOWN_COURSE_KEY) + + def test_a_role_that_may_only_view_may_list_but_not_reorder_or_change_tabs(self): + user = self._user_with(COURSE_AUDITOR.external_key) + self.assert_list(user, 200) + self.assert_reorder(user, 403) + self.assert_hide(user, 403) + + @ddt.data(*[(action, role) for action in ACTIONS for role in ( + COURSE_EDITOR.external_key, COURSE_STAFF.external_key, COURSE_ADMIN.external_key, + )]) + @ddt.unpack + def test_roles_that_may_manage_may_do_everything(self, action, role): + self.assert_access(self._user_with(role), action, OK) + + @ddt.data(*[(action, role) for action in ACTIONS for role in ( + COURSE_LIMITED_STAFF.external_key, COURSE_DATA_RESEARCHER.external_key, COURSE_BETA_TESTER.external_key, + )]) + @ddt.unpack + def test_other_roles_are_refused(self, action, role): + self.assert_access(self._user_with(role), action, 403) + + @ddt.data(*ACTIONS) + def test_a_role_on_another_course_is_refused(self, action): + user = self._user_with(COURSE_ADMIN.external_key, self.other_course.id) + self.assert_access(user, action, 403) + + @ddt.data(*ACTIONS) + def test_legacy_course_staff_without_a_policy_role_is_refused(self, action): + user = UserFactory() + CourseAccessRole.objects.create(user=user, role="staff", course_id=self.course.id, org=self.course.id.org) + self.assert_access(user, action, 403) + + @ddt.data(*ACTIONS) + def test_staff_and_superuser_are_allowed_without_a_role(self, action): + self.assert_access(self.staff_user, action, OK) + self.assert_access(self.super_user, action, OK) + + def test_an_inactive_session_with_a_role_is_accepted(self): + """The policy does not look at whether the account is activated, on v1 as on v0.""" + user = self._user_with(COURSE_AUDITOR.external_key) + client = session_client(user) + user.is_active = False + user.save() + assert client.get(list_url(self.course.id)).status_code == 200 + assert client.get(legacy_list_url(self.course.id)).status_code == 200 + + +class CustomPagesPermissionWiringTest(SimpleTestCase): + """Which permissions each viewset checks, and what the page and resource permissions ask for.""" + + @staticmethod + def _declared(view): + # The JWT middleware appends NotJwtRestrictedApplication to views that take JWTs. + return [ + type(permission) for permission in view.get_permissions() + if not isinstance(permission, NotJwtRestrictedApplication) + ] + + def test_the_writes_check_the_course_then_the_permission_to_manage_for_every_action(self): + """These viewsets need the stronger permission, so any action, routed or added later, is checked by it.""" + for viewset in (CourseCustomPageOrderViewSet, CourseTabsViewSet): + view = viewset() + for action in ("list", "update", "partial_update", "retrieve", "create", "destroy", None, "metadata"): + view.action = action + assert self._declared(view) == [IsAuthenticated, IsExistingCourse, CanManagePagesAndResources], ( + viewset, action, + ) + + def test_the_listing_reads_the_course_from_the_query_then_checks_it_and_the_permission_to_view(self): + view = CourseCustomPagesViewSet() + for action in ("list", None, "metadata"): + view.action = action + assert self._declared(view) == [ + IsAuthenticated, _CourseKeyFromQuery, IsExistingCourse, CanViewPagesAndResources, + ], action + + def test_the_listing_refuses_any_other_action(self): + view = CourseCustomPagesViewSet() + for action in ("retrieve", "create", "update", "partial_update", "destroy"): + view.action = action + with pytest.raises(ImproperlyConfigured): + view.get_permissions() + + def test_a_routed_action_the_listing_does_not_check_is_never_served(self): + """A request reaching such an action is answered with a server error, not the handler's body.""" + + class Unchecked(CourseCustomPagesViewSet): + permission_classes = CourseCustomPagesViewSet.permission_classes + + def destroy(self, request, **kwargs): # pylint: disable=unused-argument + raise AssertionError("the handler ran without a permission check") + + request = APIRequestFactory().delete(list_url(COURSE_KEY)) + force_authenticate(request, user=SimpleNamespace(is_authenticated=True, is_active=True)) + response = Unchecked.as_view({"delete": "destroy"})(request) + assert_error_envelope(response, expected_status=500, expected_type_slug="internal") + + def test_the_page_and_resource_permissions_ask_for_their_policy_and_studio_access(self): + course_key = CourseKey.from_string("course-v1:edX+Wiring+Run") + request = SimpleNamespace(user=object()) + view = SimpleNamespace(kwargs={"course_key": course_key}) + target = "cms.djangoapps.contentstore.views.permissions.user_has_course_permission" + for permission, identifier, legacy in ( + (CanViewPagesAndResources, COURSES_VIEW_PAGES_AND_RESOURCES.identifier, LegacyAuthoringPermission.READ), + ( + CanManagePagesAndResources, + COURSES_MANAGE_PAGES_AND_RESOURCES.identifier, + LegacyAuthoringPermission.WRITE, + ), + ): + for decision in (True, False): + with patch(target, return_value=decision) as check: + assert permission().has_permission(request, view) is decision + check.assert_called_once_with(request.user, identifier, course_key, legacy) + + def test_the_manage_permission_refuses_a_view_without_a_course(self): + request = SimpleNamespace(user=object()) + target = "cms.djangoapps.contentstore.views.permissions.user_has_course_permission" + with patch(target, return_value=True) as check: + assert CanManagePagesAndResources().has_permission(request, SimpleNamespace(kwargs={})) is False + check.assert_not_called() + + +class CustomPagesAuthenticationTest(SharedModuleStoreTestCase): + """How callers authenticate, with policy-based authoring permissions off.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + add_custom_pages(cls.course, 2) + + def _course_staff(self): + user = UserFactory() + CourseStaffRole(self.course.id).add_users(user) + return user + + def _assert_every_action_succeeds(self, client, **headers): + assert client.get(list_url(self.course.id), **headers).status_code == 200 + wanted = custom_page_keys(self.course.id)[::-1] + response = client.put(order_url(self.course.id), {"custom_pages": wanted}, format="json", **headers) + assert response.status_code == 204, response.content + assert custom_page_keys(self.course.id) == wanted + hidden = not stored_tab(self.course.id, HIDEABLE_TAB).is_hidden + response = client.patch(tab_url(self.course.id), {"is_hidden": hidden}, format="json", **headers) + assert response.status_code == 200, response.content + assert stored_tab(self.course.id, HIDEABLE_TAB).is_hidden is hidden + + def test_a_jwt_caller_may_list_reorder_and_change_tabs(self): + header = {"HTTP_AUTHORIZATION": f"JWT {create_jwt_for_user(self._course_staff())}"} + self._assert_every_action_succeeds(APIClient(), **header) + + def test_a_session_caller_may_list_reorder_and_change_tabs(self): + self._assert_every_action_succeeds(session_client(self._course_staff())) + + def test_an_inactive_session_is_authenticated(self): + """ + A session whose account is not activated is accepted, as on v0. Global + staff then pass; course staff are refused (403, not 401) because a + course role does not count while the account is inactive. + """ + for user, expected in ((UserFactory(is_staff=True), 200), (self._course_staff(), 403)): + client = session_client(user) + user.is_active = False + user.save() + response = client.get(list_url(self.course.id)) + assert response.status_code == expected, response.content + if expected == 403: + assert_refused(response, 403) + assert client.get(legacy_list_url(self.course.id)).status_code == expected + + def test_a_bearer_token_is_refused(self): + """Bearer tokens are not accepted here, though v0 takes them.""" + user = UserFactory(is_staff=True) + token = AccessTokenFactory(user=user, application=ApplicationFactory(user=user)).token + header = {"HTTP_AUTHORIZATION": f"Bearer {token}"} + assert_refused(APIClient().get(list_url(self.course.id), **header), 401) + assert_refused(APIClient().put(order_url(self.course.id), {"custom_pages": []}, format="json", **header), 401) + assert_refused(APIClient().patch(tab_url(self.course.id), {"is_hidden": True}, format="json", **header), 401) + assert APIClient().get(legacy_list_url(self.course.id), **header).status_code == 200 + + +# --------------------------------------------------------------------------- +# Listing +# --------------------------------------------------------------------------- + +@ddt.ddt +class CustomPagesListTest(SharedModuleStoreTestCase): + """What a listing returns and how it pages.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + add_custom_pages(cls.course, 3) + cls.twelve_pages = CourseFactory.create() + add_custom_page_entries(cls.twelve_pages.id, 12) + cls.many_pages = CourseFactory.create() + add_custom_page_entries(cls.many_pages.id, 105) + cls.no_pages = CourseFactory.create() + + def setUp(self): + super().setUp() + self.client = api_client(UserFactory(is_staff=True)) + + def get(self, course_key=None, **params): + response = self.client.get(list_url(), list_query(course_key or self.course.id, **params)) + assert response.status_code == 200, response.content + return response.json() + + @ddt.data(True, False) + def test_only_the_custom_pages_are_listed_in_course_order(self, pages_and_resources_page): + """Whether Studio's pages and resources page is on changes which tabs Studio walks, not what is listed.""" + with override_waffle_flag(ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND, active=pages_and_resources_page): + assert bool(get_pages_and_resources_url(self.course.id)) is pages_and_resources_page + body = self.get(page_size=100) + assert [page["id"] for page in body["results"]] == custom_page_keys(self.course.id) + assert len(body["results"]) == 3 + listed = {page["tab_id"] for page in body["results"]} + assert {HIDEABLE_TAB, UNHIDEABLE_TAB} <= set(stored_tab_ids(self.course.id)) - listed + + def test_each_custom_page_carries_its_members(self): + course = stored_course(self.course.id) + slug = next(tab.url_slug for tab in course.tabs if isinstance(tab, StaticTab)) + first = self.get()["results"][0] + assert first == { + "type": "static_tab", + "title": None, + "is_hideable": False, + "is_hidden": False, + "is_movable": True, + "course_staff_only": False, + "name": "Static Tab", + "tab_id": f"static_tab_{slug}", + "settings": {"url_slug": slug}, + "id": str(course.id.make_usage_key("static_tab", slug)), + } + assert set(first) == PAGE_MEMBERS + + def test_the_envelope_has_the_seven_keys(self): + body = self.get() + assert set(body) == set(ENVELOPE_KEYS) + assert (body["count"], body["num_pages"], body["current_page"], body["start"]) == (3, 1, 1, 0) + assert (body["next"], body["previous"]) == (None, None) + + def test_page_size_splits_the_pages(self): + keys = custom_page_keys(self.course.id) + first = self.get(page_size=2) + assert [page["id"] for page in first["results"]] == keys[:2] + assert (first["count"], first["num_pages"], first["current_page"], first["start"]) == (3, 2, 1, 0) + assert "page=2" in first["next"] + assert first["previous"] is None + second = self.get(page_size=2, page=2) + assert [page["id"] for page in second["results"]] == keys[2:] + assert (second["current_page"], second["start"], second["next"]) == (2, 2, None) + assert second["previous"] is not None + + def test_the_default_page_size_is_ten(self): + body = self.get(self.twelve_pages.id) + assert (len(body["results"]), body["count"], body["num_pages"]) == (10, 12, 2) + + def test_the_page_size_is_at_most_one_hundred(self): + body = self.get(self.many_pages.id, page_size=1000) + assert (len(body["results"]), body["count"], body["num_pages"]) == (100, 105, 2) + assert [page["id"] for page in body["results"]] == custom_page_keys(self.many_pages.id)[:100] + last = self.get(self.many_pages.id, page_size=100, page=2) + assert [page["id"] for page in last["results"]] == custom_page_keys(self.many_pages.id)[100:] + + @ddt.data("99", "0", "abc") + def test_a_page_out_of_range_is_404(self, page): + response = self.client.get(list_url(), list_query(self.course.id, page=page)) + envelope = assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + assert envelope["detail"] == "Invalid page." + + def test_a_course_without_custom_pages_lists_none(self): + body = self.get(self.no_pages.id) + assert (body["results"], body["count"], body["num_pages"]) == ([], 0, 1) + + def test_the_body_matches_the_published_schema(self): + assert_matches_published_schema(self.get(page_size=2), "PaginatedCustomPage.v1List") + + def test_other_query_parameters_are_ignored(self): + assert self.get(view="full", ordering="-name", fields="id") == self.get() + + def test_head_answers_without_a_body(self): + response = self.client.head(list_url(self.course.id)) + assert response.status_code == 200 + assert response.content == b"" + + +@ddt.ddt +class CustomPagesCourseKeyQueryTest(SharedModuleStoreTestCase): + """How a listing reads the course from its query, and in which order it refuses a request.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + cls.keys = add_custom_pages(cls.course, 2) + cls.other_course = CourseFactory.create() + add_custom_page_entries(cls.other_course.id, 1) + cls.other_keys = custom_page_keys(cls.other_course.id) + + def assert_invalid_course_key(self, response, message): + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"course_key": [message]} + return envelope + + def test_each_course_lists_its_own_pages(self): + client = api_client(UserFactory(is_staff=True)) + for course, keys in ((self.course, self.keys), (self.other_course, self.other_keys)): + body = client.get(list_url(course.id)).json() + assert [page["id"] for page in body["results"]] == keys + + def test_a_missing_or_blank_course_key_is_refused(self): + client = api_client(UserFactory(is_staff=True)) + self.assert_invalid_course_key(client.get(list_url()), "This field is required.") + self.assert_invalid_course_key(client.get(list_url(), {"course_key": ""}), "This field may not be blank.") + + @ddt.data("not-a-course-key", "edX+DemoX+Demo_Course", "edX/DemoX/Demo_Course", "lib:edX:library") + def test_a_value_that_is_not_a_course_key_is_refused_without_echoing_it(self, value): + response = api_client(UserFactory(is_staff=True)).get(list_url(value)) + envelope = self.assert_invalid_course_key(response, NOT_A_COURSE_KEY_MESSAGE) + assert value not in envelope["detail"] + assert value not in json.dumps(envelope["errors"]) + + def test_a_version_only_or_library_key_is_an_unknown_course(self): + client = api_client(UserFactory(is_staff=True)) + assert_refused(client.get(list_url(VERSION_ONLY_KEY)), 404) + assert_refused(client.get(list_url(LibraryFactory.create().location.library_key)), 404) + + def test_the_caller_is_authenticated_before_the_course_key_is_read(self): + for address in (list_url(), list_url("not-a-course-key"), list_url(UNKNOWN_COURSE_KEY)): + assert_refused(api_client().get(address), 401) + + def test_the_course_key_is_read_before_the_course_is_looked_up_or_the_caller_checked(self): + outsider = api_client(UserFactory()) + self.assert_invalid_course_key(outsider.get(list_url("not-a-course-key")), NOT_A_COURSE_KEY_MESSAGE) + assert_refused(outsider.get(list_url(UNKNOWN_COURSE_KEY)), 404) + assert_refused(outsider.get(list_url(self.course.id)), 403) + + def test_paging_parameters_combine_with_the_course_key(self): + client = api_client(UserFactory(is_staff=True)) + body = client.get(list_url(), list_query(self.course.id, page_size=1)).json() + assert ([page["id"] for page in body["results"]], body["count"]) == (self.keys[:1], 2) + + +class SharedPageEnvelopeTest(SimpleTestCase): + """The shared paginator pages as the library's does and publishes all seven keys.""" + + def _page(self, items, **params): + paginator = DocumentedDefaultPagination() + request = Request(APIRequestFactory().get("/items/", params)) + page = paginator.paginate_queryset(items, request) + return paginator.get_paginated_response(page).data + + def test_the_page_sizes_are_the_librarys(self): + paginator = DocumentedDefaultPagination() + assert (paginator.page_size, paginator.max_page_size, paginator.page_size_query_param) == (10, 100, "page_size") + assert len(self._page(list(range(150)))["results"]) == 10 + assert len(self._page(list(range(150)), page_size=1000)["results"]) == 100 + + def test_the_schema_describes_exactly_the_keys_of_a_page(self): + schema = DocumentedDefaultPagination().get_paginated_response_schema({"type": "array"}) + assert list(schema["properties"]) == list(ENVELOPE_KEYS) + assert schema["required"] == list(ENVELOPE_KEYS) + assert set(self._page(list(range(30)), page=2)) == set(schema["properties"]) + assert schema["properties"]["results"] == {"type": "array"} + assert {name for name, spec in schema["properties"].items() if spec.get("nullable")} == {"next", "previous"} + + def test_every_key_but_the_results_is_described_with_an_example(self): + properties = DocumentedDefaultPagination().get_paginated_response_schema({})["properties"] + described = {name for name, spec in properties.items() if spec.get("description") and "example" in spec} + assert described == set(ENVELOPE_KEYS) - {"results"} + + +# --------------------------------------------------------------------------- +# Reordering +# --------------------------------------------------------------------------- + +class _RefusingTabList(CourseTabList): + """A tab list whose validation refuses every list, as for a course whose tabs became invalid.""" + + @classmethod + def validate_tabs(cls, tabs): + raise InvalidTabsException("tab internals") + + +@ddt.ddt +class CustomPageOrderTest(ModuleStoreTestCase): + """What a reorder stores and refuses.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.keys = add_custom_pages(self.course, 3) + writer = UserFactory() + CourseStaffRole(self.course.id).add_users(writer) + self.client = api_client(writer) + + def put(self, body): + return self.client.put(order_url(self.course.id), body, format="json") + + def assert_order_refused(self, body, errors): + """Assert ``body`` is refused with ``errors`` and nothing is stored.""" + before = stored_tab_ids(self.course.id) + envelope = assert_error_envelope(self.put(body), expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == errors + assert stored_tab_ids(self.course.id) == before + return envelope + + def test_the_pages_are_stored_in_the_new_order(self): + wanted = [self.keys[1], self.keys[2], self.keys[0]] + response = self.put({"custom_pages": wanted}) + assert response.status_code == 204, response.content + assert response.content == b"" + assert custom_page_keys(self.course.id) == wanted + listed = self.client.get(list_url(self.course.id)).json()["results"] + assert [page["id"] for page in listed] == wanted + + def test_the_pages_stay_together_and_the_course_still_opens_on_its_courseware(self): + self.put({"custom_pages": self.keys[::-1]}) + tab_ids = stored_tab_ids(self.course.id) + page_ids = [f"static_tab_{UsageKey.from_string(key).block_id}" for key in self.keys[::-1]] + start = tab_ids.index(page_ids[0]) + assert tab_ids[start:start + len(page_ids)] == page_ids + assert tab_ids[0] == UNHIDEABLE_TAB + assert {HIDEABLE_TAB, "dates", "wiki"} <= set(tab_ids) + + def test_pages_left_out_follow_in_their_current_order(self): + response = self.put({"custom_pages": [self.keys[2]]}) + assert response.status_code == 204, response.content + assert custom_page_keys(self.course.id) == [self.keys[2], self.keys[0], self.keys[1]] + + def test_an_empty_list_keeps_the_order(self): + assert self.put({"custom_pages": []}).status_code == 204 + assert custom_page_keys(self.course.id) == self.keys + + def test_a_key_naming_a_branch_of_the_course_names_the_same_page(self): + branched = str(UsageKey.from_string(self.keys[1]).for_branch("published-branch")) + assert branched != self.keys[1] + assert self.put({"custom_pages": [branched]}).status_code == 204 + assert custom_page_keys(self.course.id) == [self.keys[1], self.keys[0], self.keys[2]] + + def test_a_page_of_another_course_is_refused_even_with_the_same_url_slug(self): + other = CourseFactory.create() + (other_key,) = add_custom_pages(other, 1) + assert UsageKey.from_string(other_key).block_id == UsageKey.from_string(self.keys[0]).block_id + envelope = self.assert_order_refused({"custom_pages": [self.keys[0], other_key]}, + {"custom_pages": [UNKNOWN_CUSTOM_PAGE_MESSAGE]}) + assert str(other.id) not in json.dumps(envelope) + + def test_a_block_that_is_not_a_custom_page_is_refused(self): + chapter = BlockFactory.create(parent_location=self.course.location, category="chapter") + self.assert_order_refused( + {"custom_pages": [str(chapter.location)]}, {"custom_pages": [UNKNOWN_CUSTOM_PAGE_MESSAGE]}, + ) + missing = str(self.course.id.make_usage_key("static_tab", "no_such_page")) + self.assert_order_refused({"custom_pages": [missing]}, {"custom_pages": [UNKNOWN_CUSTOM_PAGE_MESSAGE]}) + + def test_a_page_listed_twice_is_refused(self): + self.assert_order_refused({"custom_pages": [self.keys[0], self.keys[0]]}, + {"custom_pages": [REPEATED_CUSTOM_PAGE_MESSAGE]}) + branched = str(UsageKey.from_string(self.keys[0]).for_branch("published-branch")) + self.assert_order_refused( + {"custom_pages": [self.keys[0], branched]}, {"custom_pages": [REPEATED_CUSTOM_PAGE_MESSAGE]}, + ) + + def test_an_item_that_is_not_a_usage_key_is_refused(self): + envelope = self.assert_order_refused({"custom_pages": ["not a usage key"]}, + {"custom_pages": [NOT_A_USAGE_KEY_MESSAGE]}) + assert "not a usage key" not in json.dumps(envelope) + + @ddt.data("", " ", None, 5, True, ["x"], {"id": "x"}) + def test_an_item_that_is_not_a_string_holding_a_usage_key_is_refused_with_the_same_message(self, item): + """Every unusable item gets the one fixed message under errors, never a per-item error or its rendering.""" + response = self.put({"custom_pages": [self.keys[0], item]}) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"custom_pages": [NOT_A_USAGE_KEY_MESSAGE]} + assert custom_page_keys(self.course.id) == self.keys + + def test_a_null_list_is_refused(self): + self.assert_order_refused({"custom_pages": None}, {"custom_pages": ["This field may not be null."]}) + + def test_the_body_must_be_an_object_naming_the_pages(self): + self.assert_order_refused({}, {"custom_pages": ["This field is required."]}) + self.assert_order_refused( + {"custom_pages": "abc"}, {"custom_pages": ['Expected a list of items but got type "str".']}, + ) + self.assert_order_refused( + [self.keys[0]], {"non_field_errors": ["Invalid data. Expected a dictionary, but got list."]}, + ) + + def test_a_failure_saving_the_order_is_reported_without_its_text(self): + with ( + patch(f"{LEGACY_TABS_MODULE}.CourseTabList", _RefusingTabList), + self.assertLogs(SERVICE_LOGGER, "WARNING") as logs, + ): + envelope = self.assert_order_refused({"custom_pages": self.keys[::-1]}, + {"non_field_errors": [ORDER_NOT_SAVED_MESSAGE]}) + assert "tab internals" not in json.dumps(envelope) + (record,) = logs.output + assert "tab internals" in record + assert str(self.course.id) in record + with patch(f"{LEGACY_TABS_MODULE}.CourseTabList", _RefusingTabList): + legacy = self.client.post(legacy_order_url(self.course.id), [], format="json") + assert legacy.status_code == 400 + assert b"tab internals" in legacy.content + + +# --------------------------------------------------------------------------- +# Changing a tab +# --------------------------------------------------------------------------- + +@ddt.ddt +class CourseTabUpdateTest(ModuleStoreTestCase): + """What a change to a tab stores, returns and refuses.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + (self.page_key,) = add_custom_pages(self.course, 1) + writer = UserFactory() + CourseStaffRole(self.course.id).add_users(writer) + self.client = api_client(writer) + + def patch(self, body, tab_id=HIDEABLE_TAB): + return self.client.patch(tab_url(self.course.id, tab_id), body, format="json") + + @ddt.data(HIDEABLE_TAB, "dates", "wiki") + def test_hiding_and_showing_a_tab_is_stored_and_the_tab_returned(self, tab_id): + for hidden in (True, False): + response = self.patch({"is_hidden": hidden}, tab_id) + assert response.status_code == 200, response.content + body = response.json() + assert set(body) == TAB_MEMBERS + assert (body["tab_id"], body["is_hideable"], body["is_hidden"]) == (tab_id, True, hidden) + assert stored_tab(self.course.id, tab_id).is_hidden is hidden + assert_matches_published_schema(body, "CourseTab.v1") + + def test_the_returned_tab_is_the_stored_tab(self): + body = self.patch({"is_hidden": True}).json() + tab = stored_tab(self.course.id, HIDEABLE_TAB) + assert (body["type"], body["name"], body["is_movable"], body["course_staff_only"]) == ( + tab.type, str(tab.name), tab.is_movable, tab.course_staff_only, + ) + + def test_the_same_value_again_is_accepted(self): + assert self.patch({"is_hidden": True}).status_code == 200 + response = self.patch({"is_hidden": True}) + assert (response.status_code, response.json()["is_hidden"]) == (200, True) + assert stored_tab(self.course.id, HIDEABLE_TAB).is_hidden is True + + @ddt.data(True, False) + def test_a_tab_that_cannot_be_hidden_is_refused(self, hidden): + custom_page_tab = f"static_tab_{UsageKey.from_string(self.page_key).block_id}" + for tab_id in (UNHIDEABLE_TAB, custom_page_tab): + before = stored_tab(self.course.id, tab_id).is_hidden + response = self.patch({"is_hidden": hidden}, tab_id) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"is_hidden": [TAB_NOT_HIDEABLE_MESSAGE]} + assert b"can not be hidden" not in response.content + assert stored_tab(self.course.id, tab_id).is_hidden is before + + @ddt.data({"is_hidden": True}, {}) + def test_a_tab_the_course_does_not_have_is_404(self, body): + response = self.patch(body, "no_such_tab") + envelope = assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + assert envelope["detail"] == TAB_NOT_FOUND_MESSAGE + assert "no_such_tab" not in envelope["detail"] + + def test_a_body_without_is_hidden_changes_nothing(self): + with patch.object(MixedModuleStore, "update_item", wraps=modulestore().update_item) as update_item: + response = self.patch({}) + renamed = self.patch({"name": "Renamed", "is_hideable": False}) + assert update_item.call_count == 0 + for answer in (response, renamed): + assert answer.status_code == 200, answer.content + assert (answer.json()["tab_id"], answer.json()["is_hideable"]) == (HIDEABLE_TAB, True) + assert renamed.json()["name"] == str(stored_tab(self.course.id, HIDEABLE_TAB).name) + + @ddt.data( + ({"is_hidden": None}, {"is_hidden": ["This field may not be null."]}), + ({"is_hidden": "abc"}, {"is_hidden": ["Must be a valid boolean."]}), + ([True], {"non_field_errors": ["Invalid data. Expected a dictionary, but got list."]}), + ) + @ddt.unpack + def test_an_invalid_body_is_refused(self, body, errors): + before = stored_tab(self.course.id, HIDEABLE_TAB).is_hidden + envelope = assert_error_envelope(self.patch(body), expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == errors + assert stored_tab(self.course.id, HIDEABLE_TAB).is_hidden is before + + +# --------------------------------------------------------------------------- +# Requests v0 and v1 answer differently +# --------------------------------------------------------------------------- + +# Requests v0 and v1 answer differently beyond the error body, as +# (request, v0 answer, v1 answer, test in CustomPagesResponseDifferencesTest). +RESPONSE_DIFFERENCES = [ + ("listing", "a bare array of every custom page", "a page of them in the standard envelope", + "test_the_list_is_paginated"), + ("change of a tab's visibility", "204 without a body", "200 with the tab", + "test_a_visibility_change_returns_the_tab"), + ("change of a tab the course does not have", "400", "404", "test_an_unknown_tab"), + ("change without is_hidden", "400", "200, nothing changed", "test_a_change_without_is_hidden"), + ("reorder naming a custom page that does not exist", "500", "400", "test_a_reorder_naming_no_page"), + ("reorder naming a page twice", "204, the page stored twice", "400", "test_a_reorder_naming_a_page_twice"), + ("any request for a course the modulestore returns nothing for", "500", "404", + "test_a_course_the_modulestore_returns_nothing_for"), + ("reorder leaving out two or more pages", "the pages left out in no particular order", + "the pages left out in their current order", "test_pages_left_out_of_a_reorder"), + ("reorder naming a page of another course with the same url_slug", "204, this course's page moved", "400", + "test_a_page_of_another_course_with_the_same_url_slug"), + ("OPTIONS by a signed-in caller without access, or for an unknown course", "200", "403 or 404", + "test_options"), + ("any request with a Bearer token", "2xx", "401", "test_a_bearer_token"), + ("listing for a slash-separated course key", "404 JSON", "400", "test_a_slash_separated_course_key"), +] + + +class CustomPagesResponseDifferencesTest(ModuleStoreTestCase): + """Each request v0 and v1 answer differently, both answers observed.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.keys = add_custom_pages(self.course, 3) + writer = UserFactory() + CourseStaffRole(self.course.id).add_users(writer) + self.client = api_client(writer) + + def test_every_listed_difference_has_its_test(self): + assert [name for *_, name in RESPONSE_DIFFERENCES if not hasattr(self, name)] == [] + + def test_the_list_is_paginated(self): + legacy = self.client.get(legacy_list_url(self.course.id)).json() + new = self.client.get(list_url(self.course.id)).json() + assert isinstance(legacy, list) + assert set(new) == set(ENVELOPE_KEYS) + assert [page["id"] for page in legacy] == [page["id"] for page in new["results"]] == self.keys + + def test_a_visibility_change_returns_the_tab(self): + legacy = self.client.post(legacy_settings_url(self.course.id), {"is_hidden": True}, format="json") + assert (legacy.status_code, legacy.content) == (204, b"") + new = self.client.patch(tab_url(self.course.id), {"is_hidden": False}, format="json") + assert (new.status_code, new.json()["tab_id"], new.json()["is_hidden"]) == (200, HIDEABLE_TAB, False) + + def test_an_unknown_tab(self): + address = legacy_settings_url(self.course.id, "no_such_tab") + legacy = self.client.post(address, {"is_hidden": True}, format="json") + assert legacy.status_code == 400 + assert b"no_such_tab" in legacy.content + new = self.client.patch(tab_url(self.course.id, "no_such_tab"), {"is_hidden": True}, format="json") + assert_error_envelope(new, expected_status=404, expected_type_slug="not-found") + + def test_a_change_without_is_hidden(self): + legacy = self.client.post(legacy_settings_url(self.course.id), {}, format="json") + assert legacy.status_code == 400 + new = self.client.patch(tab_url(self.course.id), {}, format="json") + assert new.status_code == 200, new.content + + def test_a_reorder_naming_no_page(self): + missing = str(self.course.id.make_usage_key("static_tab", "no_such_page")) + legacy = self.client.post(legacy_order_url(self.course.id), [{"tab_locator": missing}], format="json") + assert legacy.status_code == 500 + new = self.client.put(order_url(self.course.id), {"custom_pages": [missing]}, format="json") + assert_error_envelope(new, expected_status=400, expected_type_slug="validation") + + def test_a_reorder_naming_a_page_twice(self): + new = self.client.put(order_url(self.course.id), {"custom_pages": [self.keys[0]] * 2}, format="json") + assert_error_envelope(new, expected_status=400, expected_type_slug="validation") + assert custom_page_keys(self.course.id) == self.keys + body = [{"tab_locator": self.keys[0]}] * 2 + legacy = self.client.post(legacy_order_url(self.course.id), body, format="json") + assert legacy.status_code == 204 + assert custom_page_keys(self.course.id).count(self.keys[0]) == 2 + + def test_a_course_the_modulestore_returns_nothing_for(self): + with patch.object(MixedModuleStore, "get_course", return_value=None): + for legacy, new in ( + (self.client.get(legacy_list_url(self.course.id)), self.client.get(list_url(self.course.id))), + ( + self.client.post(legacy_order_url(self.course.id), [], format="json"), + self.client.put(order_url(self.course.id), {"custom_pages": []}, format="json"), + ), + ( + self.client.post(legacy_settings_url(self.course.id), {"is_hidden": True}, format="json"), + self.client.patch(tab_url(self.course.id), {"is_hidden": True}, format="json"), + ), + ): + assert legacy.status_code == 500, legacy.content + assert_refused(new, 404) + + def test_a_bearer_token(self): + user = UserFactory(is_staff=True) + token = AccessTokenFactory(user=user, application=ApplicationFactory(user=user)).token + header = {"HTTP_AUTHORIZATION": f"Bearer {token}"} + assert APIClient().get(legacy_list_url(self.course.id), **header).status_code == 200 + assert_refused(APIClient().get(list_url(self.course.id), **header), 401) + legacy = APIClient().post(legacy_order_url(self.course.id), [], format="json", **header) + assert legacy.status_code == 204 + assert_refused(APIClient().put(order_url(self.course.id), {"custom_pages": []}, format="json", **header), 401) + + def test_a_slash_separated_course_key(self): + """v0 routes the slash form and reports the course unknown in JSON; v1 refuses the key itself.""" + legacy = self.client.get("/api/contentstore/v0/tabs/edX/NoSuch/Course") + assert legacy.status_code == 404 + assert legacy.json()["error_code"] == "course_does_not_exist" + new = self.client.get(list_url("edX/NoSuch/Course")) + envelope = assert_error_envelope(new, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"course_key": [NOT_A_COURSE_KEY_MESSAGE]} + + def test_pages_left_out_of_a_reorder(self): + """Naming only the last page: v1 keeps the other two in their order; v0 puts them in no set order.""" + last, first_two = self.keys[2], self.keys[:2] + new = self.client.put(order_url(self.course.id), {"custom_pages": [last]}, format="json") + assert new.status_code == 204, new.content + assert custom_page_keys(self.course.id) == [last, *first_two] + self.client.put(order_url(self.course.id), {"custom_pages": self.keys}, format="json") + legacy = self.client.post(legacy_order_url(self.course.id), [{"tab_locator": last}], format="json") + assert legacy.status_code == 204 + stored = custom_page_keys(self.course.id) + assert stored[0] == last + assert sorted(stored[1:]) == sorted(first_two) + + def test_a_page_of_another_course_with_the_same_url_slug(self): + """v0 finds a page by its url_slug alone, so another course's page moves this course's page.""" + other = CourseFactory.create() + (other_key,) = add_custom_pages(other, 1) + assert UsageKey.from_string(other_key).block_id == UsageKey.from_string(self.keys[0]).block_id + reordered = [self.keys[1], self.keys[2], other_key] + new = self.client.put(order_url(self.course.id), {"custom_pages": reordered}, format="json") + assert_error_envelope(new, expected_status=400, expected_type_slug="validation") + assert custom_page_keys(self.course.id) == self.keys + legacy_body = [{"tab_locator": key} for key in reordered] + legacy = self.client.post(legacy_order_url(self.course.id), legacy_body, format="json") + assert legacy.status_code == 204 + assert custom_page_keys(self.course.id) == [self.keys[1], self.keys[2], self.keys[0]] + + def test_options(self): + """v0 answers OPTIONS to any signed-in caller; v1 checks the course and the caller first.""" + outsider = api_client(UserFactory()) + for legacy_address, new_address in ( + (legacy_list_url(self.course.id), list_url(self.course.id)), + (legacy_order_url(self.course.id), order_url(self.course.id)), + (legacy_settings_url(self.course.id), tab_url(self.course.id)), + ): + assert outsider.options(legacy_address).status_code == 200 + assert_refused(outsider.options(new_address), 403) + assert self.client.options(new_address).status_code == 200 + assert outsider.options(legacy_list_url(UNKNOWN_COURSE_KEY)).status_code == 200 + assert_refused(outsider.options(list_url(UNKNOWN_COURSE_KEY)), 404) + assert_refused(outsider.options(order_url(UNKNOWN_COURSE_KEY)), 404) + + +# --------------------------------------------------------------------------- +# Parity with v0 +# --------------------------------------------------------------------------- + +# Members of a response body that differ between v0 and v1 for the same +# request. The comparison asserts nothing else differs, and that each of these +# occurs in the error cases below. +INTENTIONAL_DIFFERENCES = [ + ("type", "errors use the standardized envelope"), + ("title", "errors use the standardized envelope"), + ("status", "errors use the standardized envelope"), + ("detail", "errors use the standardized envelope"), + ("errors", "a refused change names the refused member with a fixed message, where v0 returns the " + "helper's own text under developer_message"), + ("developer_message", "the v0 error body carries developer_message; the envelope does not"), + ("error_code", "the v0 not-found body carries error_code; the envelope does not"), +] + + +def _undeclared(changed): + """The changed paths no declared intentional difference of this module covers.""" + return undeclared_paths(changed, INTENTIONAL_DIFFERENCES) + + +def test_only_declared_differences_are_tolerated(): + assert _undeclared({"type", "errors.is_hidden[0]", "developer_message.error"}) == set() + assert _undeclared({"[0].name", "results", "typed"}) == {"[0].name", "results", "typed"} + + +class _ParityMixin: + """ + v1 against v0 for the same course and caller: listings return the same + pages, reorders and visibility changes store the same tabs, and errors + differ only by the declared differences. + """ + + def manager_for(self, course): + """Return a user who may view and change the course's pages and resources.""" + raise NotImplementedError + + def outsider(self): + """Return a signed-in user with no access to the course.""" + raise NotImplementedError + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.keys = add_custom_pages(self.course, 3) + self.manager = self.manager_for(self.course) + self.observed = set() + + def compare(self, legacy, new, expected_status): + """Assert both statuses and that the bodies differ only as declared; return both bodies.""" + assert legacy.status_code == expected_status, legacy.content + assert new.status_code == expected_status, new.content + legacy_body, new_body = json.loads(legacy.content or b"null"), json.loads(new.content or b"null") + changed = changed_paths(legacy_body, new_body) + assert _undeclared(changed) == set(), f"undeclared differences: legacy={legacy_body} new={new_body}" + self.observed |= changed + return legacy_body, new_body + + def assert_same_pages(self): + client = api_client(self.manager) + legacy = client.get(legacy_list_url(self.course.id)) + new = client.get(list_url(), list_query(self.course.id, page_size=100)) + assert legacy.status_code == new.status_code == 200, (legacy.content, new.content) + legacy_pages, envelope = legacy.json(), new.json() + assert changed_paths(legacy_pages, envelope["results"]) == set(), (legacy_pages, envelope["results"]) + assert [page["id"] for page in envelope["results"]] == self.keys + assert envelope["count"] == len(legacy_pages) == 3 + + def test_the_list_returns_the_same_custom_pages_on_both_sides_of_the_pages_and_resources_flag(self): + for pages_and_resources_page in (True, False): + with override_waffle_flag(ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND, active=pages_and_resources_page): + assert bool(get_pages_and_resources_url(self.course.id)) is pages_and_resources_page + self.assert_same_pages() + + def test_reorders_store_the_same_tabs(self): + """The full order, and an order naming all pages but one, which v0 also places deterministically.""" + client = api_client(self.manager) + for wanted in ([self.keys[2], self.keys[0], self.keys[1]], [self.keys[1], self.keys[2]]): + new = client.put(order_url(self.course.id), {"custom_pages": wanted}, format="json") + stored_by_v1 = stored_tab_ids(self.course.id) + original = [{"tab_locator": key} for key in self.keys] + restore = client.post(legacy_order_url(self.course.id), original, format="json") + assert restore.status_code == 204 + assert custom_page_keys(self.course.id) == self.keys + legacy = client.post(legacy_order_url(self.course.id), [{"tab_locator": k} for k in wanted], format="json") + self.compare(legacy, new, 204) + assert stored_tab_ids(self.course.id) == stored_by_v1 + assert custom_page_keys(self.course.id)[:len(wanted)] == wanted + client.put(order_url(self.course.id), {"custom_pages": self.keys}, format="json") + + def test_visibility_changes_store_the_same_tab(self): + client = api_client(self.manager) + assert client.patch(tab_url(self.course.id), {"is_hidden": True}, format="json").status_code == 200 + hidden_by_v1 = stored_tab(self.course.id, HIDEABLE_TAB).to_json() + assert client.patch(tab_url(self.course.id), {"is_hidden": False}, format="json").status_code == 200 + assert stored_tab(self.course.id, HIDEABLE_TAB).is_hidden is False + assert client.post(legacy_settings_url(self.course.id), {"is_hidden": True}, format="json").status_code == 204 + assert stored_tab(self.course.id, HIDEABLE_TAB).to_json() == hidden_by_v1 + assert hidden_by_v1["is_hidden"] is True + + def _error_pairs(self): + """``(legacy, new, status)`` for each error both versions produce.""" + missing_page = str(self.course.id.make_usage_key("static_tab", "no_such_page")) + cases = [ + (api_client(), "get", (legacy_list_url(self.course.id), None), (list_url(self.course.id), None), 401), + (api_client(self.outsider()), "get", (legacy_list_url(self.course.id), None), + (list_url(self.course.id), None), 403), + (api_client(self.outsider()), "get", (legacy_list_url(UNKNOWN_COURSE_KEY), None), + (list_url(UNKNOWN_COURSE_KEY), None), 404), + (api_client(self.outsider()), ("post", "put"), (legacy_order_url(self.course.id), []), + (order_url(self.course.id), {"custom_pages": []}), 403), + (api_client(self.outsider()), ("post", "patch"), (legacy_settings_url(self.course.id), {"is_hidden": True}), + (tab_url(self.course.id), {"is_hidden": True}), 403), + (api_client(self.manager), ("post", "put"), (legacy_order_url(self.course.id), [{"tab_id": "no_such_tab"}]), + (order_url(self.course.id), {"custom_pages": [missing_page]}), 400), + (api_client(self.manager), ("post", "patch"), + (legacy_settings_url(self.course.id, UNHIDEABLE_TAB), {"is_hidden": True}), + (tab_url(self.course.id, UNHIDEABLE_TAB), {"is_hidden": True}), 400), + ] + for client, methods, (legacy_address, legacy_body), (new_address, new_body), status in cases: + legacy_method, new_method = (methods, methods) if isinstance(methods, str) else methods + legacy = getattr(client, legacy_method)(legacy_address, legacy_body, **self._json(legacy_body)) + new = getattr(client, new_method)(new_address, new_body, **self._json(new_body)) + yield legacy, new, status + + @staticmethod + def _json(body): + """Send ``body`` as JSON when there is one.""" + return {"format": "json"} if body is not None else {} + + def test_errors_differ_only_by_the_envelope(self): + for legacy, new, status in self._error_pairs(): + self.compare(legacy, new, status) + + def test_a_refused_visibility_change_names_is_hidden_with_a_fixed_message(self): + *_, (legacy, new, status) = self._error_pairs() + legacy_body, new_body = self.compare(legacy, new, status) + assert new_body["errors"] == {"is_hidden": [TAB_NOT_HIDEABLE_MESSAGE]} + assert legacy_body["developer_message"] == {"error": f"Tab of type {UNHIDEABLE_TAB} can not be hidden"} + assert legacy_body["developer_message"]["error"] not in new.content.decode() + + def test_every_declared_difference_occurs(self): + for legacy, new, status in self._error_pairs(): + self.compare(legacy, new, status) + missing = [ + path for path, _ in INTENTIONAL_DIFFERENCES + if not any(key == path or key.startswith(f"{path}.") or key.startswith(f"{path}[") for key in self.observed) + ] + assert missing == [] + + +class CustomPagesParityLegacyPermissionsTest(_ParityMixin, ModuleStoreTestCase): + """Parity for course staff, with policy-based authoring permissions off.""" + + def manager_for(self, course): + manager = UserFactory() + CourseStaffRole(course.id).add_users(manager) + return manager + + def outsider(self): + return UserFactory() + + +class CustomPagesParityAuthzTest(_ParityMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase): + """Parity for a course staff role, with policy-based authoring permissions on.""" + + def manager_for(self, course): + manager = UserFactory() + self.add_user_to_role_in_course(manager, COURSE_STAFF.external_key, course.id) + return manager + + def outsider(self): + return self.unauthorized_user + + +# --------------------------------------------------------------------------- +# Side effects and cost of listing +# --------------------------------------------------------------------------- + +READS = [("get", {}), ("get", {"page_size": 2}), ("get", {"page_size": 2, "page": 2}), ("head", {})] + + +@ddt.ddt +class _ListingIsSideEffectFreeMixin: + """ + Every listing (default, paged, and HEAD), with Studio's pages and resources + page on and off, writes no row and no Mongo document, stores nothing + through the modulestore and sends no signal or event. The listings start + on a database that has never served a role or permission lookup, so the + first may insert the permission library's one policy version row and + nothing else; v0 inserts the same row. + """ + + def reader_for(self, course): + """Return a user who may view ``course``, leaving the database as no lookup had touched it.""" + raise NotImplementedError + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + add_custom_pages(self.course, 3) + + @ddt.data(True, False) + def test_listings_have_no_side_effects(self, pages_and_resources_page): + with override_waffle_flag(ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND, active=pages_and_resources_page): + assert bool(get_pages_and_resources_url(self.course.id)) is pages_and_resources_page + client = api_client(self.reader_for(self.course)) + assert not PolicyCacheControl.objects.exists() + for index, (method, params) in enumerate(READS): + with record_effects(self) as effects: + response = getattr(client, method)(list_url(), list_query(self.course.id, **params)) + read = (index, method, params) + first = index == 0 + assert response.status_code == 200, (read, response.content) + assert effects["sql_writes"] == (FIRST_LOOKUP_WRITES if first else []), read + assert effects["signals"] == (FIRST_LOOKUP_SIGNALS if first else []), read + assert effects["events"] == [], read + assert effects["mongo_writes"] == 0, read + assert effects["update_item"] == [], read + + def test_the_v0_listing_makes_the_same_first_lookup_write(self): + client = api_client(self.reader_for(self.course)) + with record_effects(self) as effects: + response = client.get(legacy_list_url(self.course.id)) + assert response.status_code == 200, response.content + assert effects["sql_writes"] == FIRST_LOOKUP_WRITES + assert effects["signals"] == FIRST_LOOKUP_SIGNALS + assert effects["mongo_writes"] == 0 + + +class CustomPagesListingSideEffectsLegacyPermissionsTest(_ListingIsSideEffectFreeMixin, ModuleStoreTestCase): + """Listings by course staff with policy-based authoring permissions off.""" + + def reader_for(self, course): + reader = UserFactory() + CourseStaffRole(course.id).add_users(reader) + PolicyCacheControl.objects.all().delete() + return reader + + +class CustomPagesListingSideEffectsAuthzTest( + _ListingIsSideEffectFreeMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase, +): + """ + Listings by a course auditor with policy-based authoring permissions on. + The role is assigned through the permission library, which creates the + policy version row; it is deleted before the listings. + """ + + def reader_for(self, course): + reader = UserFactory() + self.add_user_to_role_in_course(reader, COURSE_AUDITOR.external_key, course.id) + PolicyCacheControl.objects.all().delete() + return reader + + +class _ListingCostMixin: + """A listing costs no more SQL queries or Mongo reads than the v0 listing of the same pages.""" + + def reader_for(self, course): + raise NotImplementedError + + def test_a_listing_costs_no_more_than_the_v0_listing(self): + course = CourseFactory.create() + add_custom_pages(course, 3) + client = api_client(self.reader_for(course)) + for pages_and_resources_page in (True, False): + with override_waffle_flag(ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND, active=pages_and_resources_page): + legacy = measure(lambda: client.get(legacy_list_url(course.id))) + new = measure(lambda: client.get(list_url(), list_query(course.id, page_size=100))) + assert new[0] <= legacy[0], (pages_and_resources_page, new, legacy) + assert new[1] <= legacy[1], (pages_and_resources_page, new, legacy) + assert new[2] == legacy[2] == 0, (pages_and_resources_page, new, legacy) + + def test_the_sql_cost_does_not_grow_with_the_number_of_pages(self): + course = CourseFactory.create() + add_custom_page_entries(course.id, 3) + client = api_client(self.reader_for(course)) + + def queries(): + client.get(list_url(), list_query(course.id, page_size=100)) + with CaptureQueriesContext(connection) as captured: + assert client.get(list_url(), list_query(course.id, page_size=100)).status_code == 200 + return len(captured.captured_queries) + + few = queries() + add_custom_page_entries(course.id, 60) + assert queries() == few + + +class CustomPagesListingCostLegacyPermissionsTest(_ListingCostMixin, ModuleStoreTestCase): + """Costs for course staff, with policy-based authoring permissions off.""" + + def reader_for(self, course): + reader = UserFactory() + CourseStaffRole(course.id).add_users(reader) + return reader + + +class CustomPagesListingCostAuthzTest(CourseAuthoringAuthzTestMixin, _ListingCostMixin, ModuleStoreTestCase): + """Costs for a course auditor, with policy-based authoring permissions on.""" + + def reader_for(self, course): + reader = UserFactory() + self.add_user_to_role_in_course(reader, COURSE_AUDITOR.external_key, course.id) + return reader + + +# --------------------------------------------------------------------------- +# Addresses and methods +# --------------------------------------------------------------------------- + +ADDRESSES = {"list": list_url, "order": order_url, "tab": tab_url} + + +@ddt.ddt +class CustomPagesRoutingTest(SharedModuleStoreTestCase): + """The addresses, the v0 addresses, and requests no route or method serves.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + add_custom_pages(cls.course, 1) + + def setUp(self): + super().setUp() + self.client = api_client(UserFactory(is_staff=True)) + + def test_the_addresses(self): + assert list_url() == "/api/authoring/v1/custom_pages/" + assert list_url(COURSE_KEY) == ( + "/api/authoring/v1/custom_pages/?course_key=course-v1%3AedX%2BDemoX%2BDemo_Course" + ) + assert order_url(COURSE_KEY) == f"/api/authoring/v1/courses/{COURSE_KEY}/custom_page_order/" + assert tab_url(COURSE_KEY, "wiki") == f"/api/authoring/v1/courses/{COURSE_KEY}/tabs/wiki/" + for address, viewset, actions in ( + (list_url(), CourseCustomPagesViewSet, {"get": "list"}), + (order_url(COURSE_KEY), CourseCustomPageOrderViewSet, {"put": "update"}), + (tab_url(COURSE_KEY, "wiki"), CourseTabsViewSet, {"patch": "partial_update"}), + ): + match = resolve(address) + assert match.func.cls is viewset + assert routed_actions(match.func) == actions + assert resolve(tab_url(COURSE_KEY, "static_tab_abc")).kwargs["tab_id"] == "static_tab_abc" + + def test_the_v0_addresses_are_unchanged(self): + assert legacy_list_url(COURSE_KEY) == f"/api/contentstore/v0/tabs/{COURSE_KEY}" + assert legacy_order_url(COURSE_KEY) == f"/api/contentstore/v0/tabs/{COURSE_KEY}/reorder" + settings_address = reverse(LEGACY_SETTINGS_URL_NAME, kwargs={"course_id": COURSE_KEY}) + assert settings_address == f"/api/contentstore/v0/tabs/{COURSE_KEY}/settings" + for address, view_name in ( + (legacy_list_url(COURSE_KEY), "CourseTabListView"), + (legacy_order_url(COURSE_KEY), "CourseTabReorderView"), + (settings_address, "CourseTabSettingsView"), + ): + view = resolve(address).func.cls + assert (view.__module__, view.__name__) == (LEGACY_MODULE, view_name) + + def test_v0_still_takes_a_slash_separated_key_that_v1_does_not(self): + view = resolve("/api/contentstore/v0/tabs/edX/DemoX/Demo_Course").func.cls + assert (view.__module__, view.__name__) == (LEGACY_MODULE, "CourseTabListView") + with pytest.raises(Resolver404): + resolve("/api/authoring/v1/courses/edX/DemoX/Demo_Course/custom_pages/") + + def test_the_url_names_are_unique_in_the_namespace(self): + # Imported here: at module import the URL converters are not registered yet. + from cms.djangoapps.contentstore.rest_api.v1 import authoring_urls # noqa: PLC0415 + names = [pattern.name for pattern in authoring_urls.urlpatterns] + assert len(names) == len(set(names)) + assert {"custom_page_list", "course_custom_page_order", "course_tab_detail"} <= set(names) + + # The error library does not catalogue MethodNotAllowed or NotAcceptable, so + # these answer with the envelope typed internal. Change the expected type + # here once it does. + + @ddt.data( + *[("list", method) for method in ("post", "put", "patch", "delete")], + *[("order", method) for method in ("get", "post", "patch", "delete")], + *[("tab", method) for method in ("get", "put", "post", "delete")], + ) + @ddt.unpack + def test_other_methods_are_not_allowed(self, address, method): + response = getattr(self.client, method)(ADDRESSES[address](self.course.id), {}, format="json") + assert_error_envelope(response, expected_status=405, expected_type_slug="internal") + + def test_unsupported_accept_is_refused_with_406(self): + response = self.client.get(list_url(self.course.id), HTTP_ACCEPT="application/xml") + assert_error_envelope(response, expected_status=406, expected_type_slug="internal") + + # Nor ParseError or UnsupportedMediaType: a malformed or unsupported body is + # answered with its own status but typed internal. Change the expected type + # here once the library catalogues them. + + @ddt.data(("order", "put"), ("tab", "patch")) + @ddt.unpack + def test_a_malformed_json_body_is_a_400_typed_internal(self, address, method): + before = stored_tab_ids(self.course.id) + response = getattr(self.client, method)( + ADDRESSES[address](self.course.id), data="{", content_type="application/json", + ) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="internal") + assert envelope["title"] == "Internal Server Error" + assert stored_tab_ids(self.course.id) == before + + @ddt.data(("order", "put"), ("tab", "patch")) + @ddt.unpack + def test_an_unsupported_media_type_is_a_415_typed_internal(self, address, method): + before = stored_tab(self.course.id, HIDEABLE_TAB).is_hidden + response = getattr(self.client, method)( + ADDRESSES[address](self.course.id), data="is_hidden=true", content_type="text/plain", + ) + envelope = assert_error_envelope(response, expected_status=415, expected_type_slug="internal") + assert envelope["title"] == "Internal Server Error" + assert stored_tab(self.course.id, HIDEABLE_TAB).is_hidden is before + + # These addresses match no route, so the service's HTML 404 page answers + # them. Change these to the error envelope once a catch-all route under the + # mount answers unknown addresses. + + @ddt.data(*[ + (key, address) + for key in ("not-a-course-key", "edX/DemoX/Demo_Course", "edX+DemoX+Demo_Course") + for address in ("custom_page_order/", "tabs/progress/") + ]) + @ddt.unpack + def test_an_unparsable_or_slash_form_key_is_an_html_404(self, key, address): + response = self.client.get(f"/api/authoring/v1/courses/{key}/{address}") + assert response.status_code == 404 + assert response["Content-Type"].startswith("text/html") + + def test_the_list_address_without_its_trailing_slash_redirects_to_it(self): + """No route answers the address without its slash; the service redirects a read, query kept.""" + address = list_url(self.course.id).replace("custom_pages/?", "custom_pages?") + response = self.client.get(address) + assert response.status_code == 301 + assert response["Location"] == list_url(self.course.id) + with pytest.raises(Resolver404): + resolve("/api/authoring/v1/custom_pages") + + def test_the_course_is_no_longer_part_of_the_list_address(self): + with pytest.raises(Resolver404): + resolve(f"/api/authoring/v1/courses/{COURSE_KEY}/custom_pages/") + + +# --------------------------------------------------------------------------- +# The published schema +# --------------------------------------------------------------------------- + +class CustomPagesSchemaTest(TestCase): + """The listing, the reorder and the tab change as published in the CMS authoring schema.""" + + LIST_PATH = "/api/authoring/v1/custom_pages/" + ORDER_PATH = "/api/authoring/v1/courses/{course_key}/custom_page_order/" + TAB_PATH = "/api/authoring/v1/courses/{course_key}/tabs/{tab_id}/" + LEGACY = { + "/api/contentstore/v0/tabs/{course_id}": ("get", "v0_tabs_retrieve"), + "/api/contentstore/v0/tabs/{course_id}/reorder": ("post", "v0_tabs_reorder_create"), + "/api/contentstore/v0/tabs/{course_id}/settings": ("post", "v0_tabs_settings_create"), + } + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.schema = published_schema() + cls.components = cls.schema["components"]["schemas"] + + @staticmethod + def _ref(node): + return node["content"]["application/json"]["schema"]["$ref"].rsplit("/", 1)[-1] + + def _operation(self, path, method): + operations = self.schema["paths"][path] + assert sorted(operations) == [method] + return operations[method] + + def test_the_three_operations_are_published_for_the_sdk(self): + for path, method, operation_id, statuses in ( + (self.LIST_PATH, "get", "v1_custom_pages_list", ["200", "400", "401", "403", "404"]), + (self.ORDER_PATH, "put", "v1_courses_custom_page_order_update", ["204", "400", "401", "403", "404"]), + (self.TAB_PATH, "patch", "v1_courses_tabs_partial_update", ["200", "400", "401", "403", "404"]), + ): + operation = self._operation(path, method) + assert operation["operationId"] == operation_id + assert operation["tags"] == ["openedx-platform-sdk"] + assert "deprecated" not in operation + assert sorted(operation["responses"]) == statuses + for status in statuses: + if status.startswith("4"): + assert self._ref(operation["responses"][status]) == "ErrorResponse", (path, status) + + def test_the_list_publishes_the_seven_envelope_keys(self): + operation = self._operation(self.LIST_PATH, "get") + name = self._ref(operation["responses"]["200"]) + assert name == "PaginatedCustomPage.v1List" + envelope = self.components[name] + assert list(envelope["properties"]) == list(ENVELOPE_KEYS) + assert envelope["required"] == list(ENVELOPE_KEYS) + assert envelope["properties"]["results"] == { + "type": "array", "items": {"$ref": "#/components/schemas/CustomPage.v1"}, + } + + def test_the_list_parameters(self): + parameters = {p["name"]: p for p in self._operation(self.LIST_PATH, "get")["parameters"]} + assert set(parameters) == {"course_key", "page", "page_size"} + assert parameters["course_key"]["in"] == "query" + assert parameters["course_key"]["required"] is True + assert parameters["course_key"]["schema"] == {"type": "string"} + assert parameters["course_key"]["description"] + for name in ("page", "page_size"): + assert (parameters[name]["in"], parameters[name]["schema"]) == ("query", {"type": "integer"}) + assert not parameters[name].get("required") + + def test_the_reorder_documents_each_refusal_message(self): + description = self._operation(self.ORDER_PATH, "put")["description"] + for message in ( + NOT_A_USAGE_KEY_MESSAGE, UNKNOWN_CUSTOM_PAGE_MESSAGE, REPEATED_CUSTOM_PAGE_MESSAGE, ORDER_NOT_SAVED_MESSAGE, + ): + assert message in description, message + + def test_the_tab_change_says_custom_pages_cannot_be_hidden_there(self): + description = self._operation(self.TAB_PATH, "patch")["description"] + assert "Custom pages are among the course's tabs" in description + assert "is_hidden is refused for them" in description + assert TAB_NOT_HIDEABLE_MESSAGE in description + + def test_the_reorder_takes_json_naming_the_pages_and_answers_without_a_body(self): + operation = self._operation(self.ORDER_PATH, "put") + assert "content" not in operation["responses"]["204"] + body = operation["requestBody"] + assert list(body["content"]) == ["application/json"] + assert body["required"] is True + assert self._ref(body) == "CustomPageOrder.v1" + order = self.components["CustomPageOrder.v1"] + assert order["required"] == ["custom_pages"] + assert order["properties"]["custom_pages"]["type"] == "array" + assert order["properties"]["custom_pages"]["items"]["type"] == "string" + + def test_the_tab_change_takes_an_optional_is_hidden_and_returns_the_tab(self): + operation = self._operation(self.TAB_PATH, "patch") + parameters = {p["name"]: p for p in operation["parameters"]} + assert set(parameters) == {"course_key", "tab_id"} + assert parameters["tab_id"]["schema"] == {"type": "string"} + body = operation["requestBody"] + assert list(body["content"]) == ["application/json"] + assert self._ref(body) == "PatchedCourseTabUpdate.v1" + update = self.components["PatchedCourseTabUpdate.v1"] + assert list(update["properties"]) == ["is_hidden"] + assert update["properties"]["is_hidden"]["type"] == "boolean" + assert "required" not in update + assert self._ref(operation["responses"]["200"]) == "CourseTab.v1" + + def test_the_tab_and_page_components(self): + tab, page = self.components["CourseTab.v1"], self.components["CustomPage.v1"] + assert set(tab["properties"]) == set(tab["required"]) == TAB_MEMBERS + assert set(page["properties"]) == set(page["required"]) == PAGE_MEMBERS + for component in (tab, page): + assert {name for name, field in component["properties"].items() if field.get("nullable")} == {"title"} + assert component["properties"]["settings"]["type"] == "object" + + def test_every_member_is_described(self): + for name in ("CourseTab.v1", "CustomPage.v1", "CustomPageOrder.v1", "PatchedCourseTabUpdate.v1"): + component = self.components[name] + assert component.get("description"), name + assert [field for field, spec in component["properties"].items() if not spec.get("description")] == [], name + + def test_the_examples_are_valid_bodies(self): + listing = self._operation(self.LIST_PATH, "get")["responses"]["200"]["content"]["application/json"] + page = listing["examples"]["CustomPage"]["value"] + assert set(page) == set(ENVELOPE_KEYS) + assert len(page["results"]) == 1 + assert_matches_published_schema(page, "PaginatedCustomPage.v1List") + order = self._operation(self.ORDER_PATH, "put")["requestBody"]["content"]["application/json"] + assert_matches_published_schema(order["examples"]["NewOrder"]["value"], "CustomPageOrder.v1") + change = self._operation(self.TAB_PATH, "patch") + hide = change["requestBody"]["content"]["application/json"]["examples"]["Hide"]["value"] + assert_matches_published_schema(hide, "PatchedCourseTabUpdate.v1") + tab = change["responses"]["200"]["content"]["application/json"]["examples"]["ProgressTab"]["value"] + assert_matches_published_schema(tab, "CourseTab.v1") + + def test_the_v0_operations_are_deprecated_and_left_out_of_the_sdk(self): + for path, (method, operation_id) in self.LEGACY.items(): + operation = self._operation(path, method) + assert operation["operationId"] == operation_id + assert operation["deprecated"] is True + assert "openedx-platform-sdk" not in operation.get("tags", []) + + def test_only_superseded_or_migrated_addresses_are_deprecated(self): + """No operation is marked deprecated except by the legacy-address lists, so a stray mark is caught.""" + deprecated = { + (path, method) + for path, operations in self.schema["paths"].items() + for method, operation in operations.items() + if isinstance(operation, dict) and operation.get("deprecated") + } + assert {(path, method) for path, (method, _) in self.LEGACY.items()} <= deprecated + new = {(self.LIST_PATH, "get"), (self.ORDER_PATH, "put"), (self.TAB_PATH, "patch")} + assert new & deprecated == set() + stray = sorted( + (path, method) for path, method in deprecated + if path not in SUPERSEDED_PATHS and not path.startswith(LEGACY_MIGRATED_PATH_PREFIXES) + ) + assert stray == [] diff --git a/cms/djangoapps/contentstore/rest_api/v2/authoring_urls.py b/cms/djangoapps/contentstore/rest_api/v2/authoring_urls.py index 1c0d693dec4f..20caccc17c45 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/authoring_urls.py +++ b/cms/djangoapps/contentstore/rest_api/v2/authoring_urls.py @@ -2,6 +2,7 @@ from django.urls import path +from cms.djangoapps.contentstore.rest_api.v2.views.group_configurations import CourseGroupConfigurationsViewSet from cms.djangoapps.contentstore.rest_api.v2.views.textbooks import CourseTextbooksViewSet from cms.djangoapps.contentstore.rest_api.v2.views.video_settings import CourseVideoSettingsViewSet @@ -18,4 +19,9 @@ CourseVideoSettingsViewSet.as_view({"get": "retrieve"}), name="course_video_settings", ), + path( + "courses//group_configurations/", + CourseGroupConfigurationsViewSet.as_view({"get": "list"}), + name="course_group_configuration_list", + ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v2/group_configurations_service.py b/cms/djangoapps/contentstore/rest_api/v2/group_configurations_service.py new file mode 100644 index 000000000000..60077318fb26 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/group_configurations_service.py @@ -0,0 +1,51 @@ +"""Reads of a course's group configurations.""" + +import logging + +from openedx_authz.constants.permissions import COURSES_MANAGE_GROUP_CONFIGURATIONS +from rest_framework.exceptions import NotFound + +from cms.djangoapps.contentstore.utils import get_group_configurations_context +from cms.djangoapps.contentstore.views.permissions import COURSE_NOT_FOUND_MESSAGE +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission +from openedx.core.djangoapps.authz.decorators import user_has_course_permission +from xmodule.modulestore.django import modulestore + +log = logging.getLogger(__name__) + + +def list_group_configurations(course_key, user): + """ + Return the group configurations of the course ``course_key`` that Studio shows, in Studio's order. + + First the configurations Studio's group configurations page lists in its + main section: the enrollment track configuration when the course has more + than one track, the content type gating configuration when Studio may set + it, then content groups and the other configurations in course order. A + course without content groups gets an unsaved content group configuration + there, with a new identifier for each call. The experiment configurations + follow, in course order, when the course has content experiments turned + on; otherwise none is returned. + + Each configuration is a dict as Studio stores it, with ``usage`` where + Studio reports where the configuration or its groups are used, and with + ``can_manage``: whether ``user`` may change the course's group + configurations. That is decided once, for every configuration. + + Raises ``NotFound`` when the course cannot be loaded from the modulestore. + """ + store = modulestore() + with store.bulk_operations(course_key): + course = store.get_course(course_key) + if course is None: + log.info("Group configurations requested for course %s, which is not in the modulestore.", course_key) + raise NotFound(COURSE_NOT_FOUND_MESSAGE) + context = get_group_configurations_context(course, store) + can_manage = user_has_course_permission( + user, + COURSES_MANAGE_GROUP_CONFIGURATIONS.identifier, + course_key, + LegacyAuthoringPermission.WRITE, + ) + configurations = [*context["all_group_configurations"], *(context["experiment_group_configurations"] or [])] + return [{**configuration, "can_manage": can_manage} for configuration in configurations] diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/group_configurations.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/group_configurations.py new file mode 100644 index 000000000000..5ae24e303dce --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/group_configurations.py @@ -0,0 +1,152 @@ +"""Serializers for a course's group configurations.""" + +from rest_framework import serializers + +MINIMAL_VIEW = "minimal" +FULL_VIEW = "full" + + +class _FullViewUsage: + """ + Leave out ``usage`` unless the serializer context asks for it. + + The context entry ``include_usage`` is read from the outermost serializer, + so one flag decides for configurations and their groups alike. + """ + + def to_representation(self, instance): + data = super().to_representation(instance) + if not self.context.get("include_usage"): + data.pop("usage", None) + return data + + +class GroupConfigurationValidationSerializer(serializers.Serializer): # pylint: disable=abstract-method + """A summary of the problems Studio finds in a content experiment.""" + + type = serializers.CharField(help_text="How serious the problems are: 'warning' or 'error'.") + text = serializers.CharField(help_text="Message describing the problems, as Studio shows it.") + + class Meta: + ref_name = "GroupConfigurationValidation.v2" + + +class GroupConfigurationUsageSerializer(serializers.Serializer): # pylint: disable=abstract-method + """One place in the course content that uses a configuration or one of its groups.""" + + label = serializers.CharField( + help_text="Where the content is, as ' / '.", + ) + url = serializers.CharField(help_text="Studio address of the page showing the content.") + validation = GroupConfigurationValidationSerializer( + required=False, + allow_null=True, + help_text=( + "Present only for the content experiments using an experiment configuration: a summary of the " + "experiment's problems, or null when it has none." + ), + ) + + class Meta: + ref_name = "GroupConfigurationUsage.v2" + + def to_representation(self, instance): + data = super().to_representation(instance) + if "validation" not in instance: + data.pop("validation") + return data + + +class GroupConfigurationGroupSerializer(_FullViewUsage, serializers.Serializer): # pylint: disable=abstract-method + """One group of a group configuration.""" + + id = serializers.IntegerField(help_text="Identifier of the group within its configuration.") + name = serializers.CharField(help_text="Name of the group.") + usage = GroupConfigurationUsageSerializer( + many=True, + required=False, + allow_null=True, + help_text=( + "Present only with view=full. The content restricted to this group, or null for the groups of " + "experiment configurations, whose use is reported for the whole configuration." + ), + ) + version = serializers.IntegerField(help_text="Version of the group's stored format.") + + class Meta: + ref_name = "GroupConfigurationGroup.v2" + + +class GroupConfigurationSerializer(_FullViewUsage, serializers.Serializer): # pylint: disable=abstract-method + """ + One group configuration of a course, with what the caller may do with it. + + The scheme tells the kinds apart: 'random' for the configurations of + content experiments, 'cohort' for content groups, and schemes such as + 'enrollment_track' or 'content_type_gate' for configurations the platform + maintains, which are read only. + """ + + id = serializers.IntegerField( + help_text=( + "Identifier of the configuration within the course. A course without content groups lists an " + "unsaved content group configuration whose identifier is chosen afresh for each request until " + "a group is added to it." + ), + ) + name = serializers.CharField(help_text="Name of the configuration.") + scheme = serializers.CharField( + help_text=( + "How learners are assigned to the groups, such as random, cohort, enrollment_track, " + "content_type_gate or team." + ), + ) + description = serializers.CharField(allow_blank=True, help_text="Description of the configuration.") + parameters = serializers.DictField( + help_text="Settings of the scheme, such as the course_id of an enrollment track configuration.", + ) + groups = GroupConfigurationGroupSerializer(many=True, help_text="The groups of the configuration.") + usage = GroupConfigurationUsageSerializer( + many=True, + required=False, + allow_null=True, + help_text=( + "Present only with view=full. The content experiments using an experiment configuration, or " + "null for other configurations, whose use is reported per group." + ), + ) + version = serializers.IntegerField(help_text="Version of the configuration's stored format.") + active = serializers.BooleanField(help_text="Whether the configuration is in use for learners.") + read_only = serializers.BooleanField( + required=False, + help_text=( + "Whether the configuration is maintained by the platform and cannot be changed by course authors. " + "Absent for experiment configurations and for the unsaved content group configuration." + ), + ) + can_manage = serializers.BooleanField( + help_text=( + "Whether the caller may create, change and delete the course's group configurations. It is the " + "same on every configuration of the course and does not override read_only: a read-only " + "configuration, such as the enrollment track configuration, cannot be changed even when this is true." + ), + ) + + class Meta: + ref_name = "GroupConfiguration.v2" + + +class GroupConfigurationListQuerySerializer(serializers.Serializer): # pylint: disable=abstract-method + """Query parameters of a listing of a course's group configurations.""" + + view = serializers.ChoiceField( + choices=[MINIMAL_VIEW, FULL_VIEW], + default=MINIMAL_VIEW, + help_text=( + "How much to return. 'minimal' leaves out of the response where each configuration and group is " + "used in the course content; 'full' includes it." + ), + ) + + class Meta: + ref_name = "GroupConfigurationListQuery.v2" diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/group_configurations.py b/cms/djangoapps/contentstore/rest_api/v2/views/group_configurations.py new file mode 100644 index 000000000000..afd81b870d43 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/group_configurations.py @@ -0,0 +1,166 @@ +"""API views for a course's group configurations.""" + +from django.core.exceptions import ImproperlyConfigured +from drf_spectacular.utils import OpenApiExample, OpenApiParameter, OpenApiResponse, extend_schema +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from edx_rest_framework_extensions.errors import ErrorResponseSerializer +from edx_rest_framework_extensions.mixins import StandardizedErrorMixin +from edx_rest_framework_extensions.paginators import IterablePaginationMixin +from edx_rest_framework_extensions.url_converters import CourseKeyConverter +from rest_framework import viewsets +from rest_framework.permissions import IsAuthenticated + +from cms.djangoapps.contentstore.rest_api.pagination import DocumentedDefaultPagination +from cms.djangoapps.contentstore.rest_api.v2.group_configurations_service import list_group_configurations +from cms.djangoapps.contentstore.rest_api.v2.serializers.group_configurations import ( + FULL_VIEW, + MINIMAL_VIEW, + GroupConfigurationListQuerySerializer, + GroupConfigurationSerializer, +) +from cms.djangoapps.contentstore.views.permissions import CanViewGroupConfigurations, IsExistingCourse + +_COURSE_KEY_PARAMETER = OpenApiParameter( + name="course_key", + description="Key of the course, such as course-v1:edX+DemoX+2026.", + location=OpenApiParameter.PATH, + type=str, + pattern=f"^{CourseKeyConverter.regex}$", +) + +_VIEW_PARAMETER = OpenApiParameter( + name="view", + description=GroupConfigurationListQuerySerializer().fields["view"].help_text, + required=False, + location=OpenApiParameter.QUERY, + type=str, + enum=[MINIMAL_VIEW, FULL_VIEW], + default=MINIMAL_VIEW, +) + + +def _error(description): + return OpenApiResponse(response=ErrorResponseSerializer, description=description) + + +_CONFIGURATION_EXAMPLE = OpenApiExample( + "Content groups", + summary="The content groups of a course, with view=full", + response_only=True, + status_codes=["200"], + value={ + "id": 1791848226, + "name": "Content Groups", + "scheme": "cohort", + "description": "The groups in this configuration can be mapped to cohorts in the Instructor Dashboard.", + "parameters": {}, + "groups": [ + { + "id": 593758473, + "name": "Honors", + "usage": [ + { + "label": "Week 1 / Honors problem", + "url": "/container/block-v1:edX+DemoX+2026+type@vertical+block@3f9a7c1e", + }, + ], + "version": 1, + }, + ], + "usage": None, + "version": 3, + "active": True, + "read_only": False, + "can_manage": True, + }, +) + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CourseGroupConfigurationsViewSet(StandardizedErrorMixin, IterablePaginationMixin, viewsets.ViewSet): + """ + The group configurations of one course, in the order Studio shows them. + + The configurations Studio lists on its group configurations page come + first: the enrollment track configuration when the course has more than + one track, the content type gating configuration when Studio may set it, + then content groups and the rest. A course without content groups lists an + unsaved content group configuration in their place, as Studio does, so + that groups can be added to it; its identifier is chosen afresh for each + request until then. Experiment configurations follow when the course has + content experiments turned on, and are left out otherwise. Flags that + describe the course rather than its group configurations, such as whether + content experiments are turned on, are not part of this resource. + + By default (``view=minimal``) where each configuration and group is used + in the course content is left out of the response; ``view=full`` includes + it. Each configuration says whether the caller may change the course's + group configurations. + + Anyone who may view the course's group configurations may list them. The + course is looked up first, so a signed-in caller learns that a course does + not exist (404) before being refused (403). An action this viewset was not + written for is refused with a server error instead of being served with + these checks. + + A plain ``ViewSet``: the configurations are user partitions stored with + the course in the modulestore, not database rows. + """ + + # An inactive account's browser session stays authenticated, so the course permission decides: staff status + # admits it, and its roles count only on courses where policy-based permissions are on. + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated, IsExistingCourse, CanViewGroupConfigurations) + # The actions the permissions above were written for. A method with no + # route (answered 405) has no action; OPTIONS is ``metadata``. + checked_actions = (None, "list", "metadata") + serializer_class = GroupConfigurationSerializer + pagination_class = DocumentedDefaultPagination + + def get_permissions(self): + """Return the viewset's permissions, or refuse an action they were not written for.""" + if self.action not in self.checked_actions: + raise ImproperlyConfigured(f"No permission is declared for the {self.action!r} action.") + return super().get_permissions() + + def get_serializer(self, *args, **kwargs): + """Instantiate the group configuration serializer declared on this viewset.""" + return self.serializer_class(*args, **kwargs) + + @extend_schema( + summary="List a course's group configurations", + description=( + "Returns one page of the course's group configurations, in the order Studio shows them: those of " + "Studio's group configurations page first, then the experiment configurations when the course " + "has content experiments turned on. A course without content groups lists an unsaved content " + "group configuration, whose id changes from request to request until a group is added to it. " + "Flags that describe the course rather than its group configurations, such as whether content " + "experiments are turned on, are not part of this resource. " + "With view=full, each configuration and each group also says where it is used in the course " + "content. can_manage, the same on every configuration, says whether the caller may change the " + "course's group configurations." + ), + parameters=[_COURSE_KEY_PARAMETER, _VIEW_PARAMETER], + responses={ + 200: OpenApiResponse( + response=GroupConfigurationSerializer, + description="A page of the course's group configurations.", + ), + 400: _error("view has an invalid value."), + 401: _error("The requester is not authenticated."), + 403: _error("The requester may not view this course's group configurations."), + 404: _error("The course does not exist, or the requested page is out of range."), + }, + examples=[_CONFIGURATION_EXAMPLE], + ) + def list(self, request, course_key): + """Return one page of the course's group configurations, with their usage when ``view=full``.""" + query = GroupConfigurationListQuerySerializer(data=request.query_params) + query.is_valid(raise_exception=True) + context = {"include_usage": query.validated_data["view"] == FULL_VIEW} + return self.paginate_iterable( + request, + list_group_configurations(course_key, request.user), + serialize=lambda page: self.get_serializer(page, many=True, context=context).data, + ) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_group_configurations.py new file mode 100644 index 000000000000..a51721b2b9d7 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_group_configurations.py @@ -0,0 +1,1390 @@ +"""Tests for the v2 course group configurations API.""" + +import json +from datetime import UTC, datetime +from types import SimpleNamespace +from unittest.mock import patch + +import ddt +import pytest +from django.contrib.auth import get_user_model +from django.core.exceptions import ImproperlyConfigured +from django.test import SimpleTestCase, TestCase +from django.urls import Resolver404, resolve, reverse +from edx_rest_framework_extensions.permissions import NotJwtRestrictedApplication +from edx_rest_framework_extensions.testing import assert_error_envelope +from opaque_keys.edx.keys import CourseKey +from openedx_authz.constants.permissions import COURSES_MANAGE_GROUP_CONFIGURATIONS, COURSES_VIEW_GROUP_CONFIGURATIONS +from openedx_authz.constants.roles import ( + COURSE_ADMIN, + COURSE_AUDITOR, + COURSE_BETA_TESTER, + COURSE_DATA_RESEARCHER, + COURSE_EDITOR, + COURSE_LIMITED_STAFF, + COURSE_STAFF, +) +from openedx_authz.models.engine import PolicyCacheControl +from rest_framework.permissions import IsAuthenticated +from rest_framework.test import APIClient, APIRequestFactory, force_authenticate + +from cms.djangoapps.contentstore.rest_api.pagination import ENVELOPE_KEYS +from cms.djangoapps.contentstore.rest_api.v1.views.tests.helpers import ( + FIRST_LOOKUP_SIGNALS, + FIRST_LOOKUP_WRITES, + UNKNOWN_COURSE_KEY, + VERSION_ONLY_KEY, + api_client, + assert_matches_published_schema, + assert_refused, + changed_paths, + measure, + published_schema, + record_effects, + session_client, + undeclared_paths, +) +from cms.djangoapps.contentstore.rest_api.v2.views.group_configurations import CourseGroupConfigurationsViewSet +from cms.djangoapps.contentstore.tests.utils import routed_actions +from cms.djangoapps.contentstore.utils import reverse_usage_url +from cms.djangoapps.contentstore.views.permissions import CanViewGroupConfigurations, IsExistingCourse +from cms.lib.spectacular import LEGACY_MIGRATED_PATH_PREFIXES, SUPERSEDED_PATHS +from common.djangoapps.course_modes.tests.factories import CourseModeFactory +from common.djangoapps.student.models import CourseAccessRole +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseLimitedStaffRole, + CourseStaffRole, + OrgInstructorRole, + OrgStaffRole, +) +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin +from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user +from openedx.core.djangoapps.oauth_dispatch.tests.factories import AccessTokenFactory, ApplicationFactory +from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel +from openedx.core.lib.teams_config import CONTENT_GROUPS_FOR_TEAMS, TeamsConfig +from openedx.features.content_type_gating.helpers import CONTENT_GATING_PARTITION_ID +from openedx.features.content_type_gating.models import ContentTypeGatingConfig +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.mixed import MixedModuleStore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, LibraryFactory +from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID, Group, UserPartition +from xmodule.partitions.partitions_service import get_all_partitions_for_course +from xmodule.split_test_block import SplitTestBlock + +URL_NAME = "authoring_v2:course_group_configuration_list" +LEGACY_URL_NAME = "cms.djangoapps.contentstore:v1:group_configurations" +LEGACY_MODULE = "cms.djangoapps.contentstore.rest_api.v1.views.group_configurations" +SERVICE_MODULE = "cms.djangoapps.contentstore.rest_api.v2.group_configurations_service" +# The placeholder content group configuration's identifier is drawn by this +# function each time the configurations are read. +ID_GENERATOR = "cms.djangoapps.contentstore.course_group_config.generate_int_id" +TEAMS_LOOKUP = "lms.djangoapps.teams.team_partition_scheme.get_teams_in_teamset" +COURSE_KEY = "course-v1:edX+DemoX+Demo_Course" + +CONTENT_GROUPS_ID = 10 +EXPERIMENT_ID = 20 +UNUSED_EXPERIMENT_ID = 21 +TEAM_PARTITION_ID = 30 +TEAM_SET_ID = "debaters" +FIRST_TEAM_ID = 501 +PLACEHOLDER_ID = 1234567 +FULL = {"view": "full"} +ALL = {"page_size": 100} +CONFIGURATION_MEMBERS = { + "id", "name", "scheme", "description", "parameters", "groups", "version", "active", "can_manage", +} +GROUP_MEMBERS = {"id", "name", "version"} + + +def url(course_key): + """Return the address of the group configurations of ``course_key``.""" + return reverse(URL_NAME, kwargs={"course_key": str(course_key)}) + + +def legacy_url(course_key): + """Return the deprecated address of the group configurations of ``course_key``.""" + return reverse(LEGACY_URL_NAME, kwargs={"course_id": str(course_key)}) + + +# --------------------------------------------------------------------------- +# Courses +# --------------------------------------------------------------------------- + +# The teams of each team-set, by course key and team-set id. Studio reads a +# team-set's teams from the teams app's tables, which the CMS test database +# does not have, so every test reads them from here. +TEAMS = {} + + +@pytest.fixture(autouse=True) +def _teams_from_memory(): + """Serve the teams of each team-set from ``TEAMS`` during a test, and forget them after it.""" + def teams_in(course_id, topic_id): + return TEAMS.get((course_id, topic_id), []) + + with patch(TEAMS_LOOKUP, side_effect=teams_in): + yield + TEAMS.clear() + + +def _stored_course(course_key): + return modulestore().get_course(CourseKey.from_string(str(course_key))) + + +def store_partitions(course_key, partitions): + """Store ``partitions`` as the course's own user partitions.""" + course = _stored_course(course_key) + course.user_partitions = partitions + modulestore().update_item(course, ModuleStoreEnum.UserID.test) + + +def turn_experiments(course_key, on): + """Turn content experiments on or off for the course, as Studio's advanced settings do.""" + course = _stored_course(course_key) + course.advanced_modules = ["split_test"] if on else [] + modulestore().update_item(course, ModuleStoreEnum.UserID.test) + + +def content_groups(partition_id=CONTENT_GROUPS_ID, name="Content Groups"): + return UserPartition( + partition_id, name, "Groups for honors content", [Group(1, "Honors"), Group(2, "Regular")], + scheme_id="cohort", + ) + + +def experiment(partition_id, name): + return UserPartition(partition_id, name, "Which variant learners see", [Group(0, "Blue"), Group(1, "Green")]) + + +def build_course( + *, groups=False, experiments=False, experiments_on=False, tracks=0, gating=False, teams=None, team_groups_on=False, +): + """ + Create a course and return its key and blocks. + + ``groups``: a content group configuration, one of whose groups a problem + is restricted to. ``experiments``: two experiment configurations, one used + by a content experiment. ``experiments_on``: content experiments turned on. + ``tracks``: how many enrollment tracks (0 leaves the default track only). + ``gating``: content type gating configurable in Studio, site-wide. + ``teams``: a team-set with a user partition and this many teams, the + first of which a problem is restricted to; ``None`` for no team-set. + ``team_groups_on``: content groups for teams turned on for the course. + """ + if teams is None: + course = CourseFactory.create() + else: + team_set = { + "id": TEAM_SET_ID, "name": "Debaters", "description": "Teams of the debate", + "user_partition_id": TEAM_PARTITION_ID, + } + course = CourseFactory.create(teams_configuration=TeamsConfig({"team_sets": [team_set]})) + TEAMS[(str(course.id), TEAM_SET_ID)] = [ + SimpleNamespace(id=FIRST_TEAM_ID + index, name=f"Debate team {index}") for index in range(teams) + ] + if team_groups_on: + WaffleFlagCourseOverrideModel.objects.create( + waffle_flag=CONTENT_GROUPS_FOR_TEAMS.name, course_id=course.id, enabled=True, + ) + partitions = [] + if groups: + partitions.append(content_groups()) + if experiments: + partitions += [experiment(EXPERIMENT_ID, "Button colour"), experiment(UNUSED_EXPERIMENT_ID, "Unused")] + store_partitions(course.id, partitions) + turn_experiments(course.id, experiments_on) + chapter = BlockFactory.create(parent_location=course.location, category="chapter", display_name="Week 1") + sequential = BlockFactory.create(parent_location=chapter.location, category="sequential", display_name="Lesson") + unit = BlockFactory.create(parent_location=sequential.location, category="vertical", display_name="Unit 1") + problem = split_test = None + if groups: + problem = BlockFactory.create( + parent_location=unit.location, category="problem", display_name="Honors problem", + group_access={CONTENT_GROUPS_ID: [1]}, + ) + if teams: + BlockFactory.create( + parent_location=unit.location, category="problem", display_name="Team problem", + group_access={TEAM_PARTITION_ID: [FIRST_TEAM_ID]}, + ) + if experiments: + split_test = BlockFactory.create( + parent_location=unit.location, category="split_test", display_name="Experiment", + user_partition_id=EXPERIMENT_ID, + ) + for mode in ("audit", "verified", "honor")[:tracks]: + CourseModeFactory.create(course_id=course.id, mode_slug=mode) + if gating: + ContentTypeGatingConfig.objects.create(enabled=False, studio_override_enabled=True) + return SimpleNamespace(id=course.id, unit=unit, problem=problem, split_test=split_test) + + +# Courses each read and comparison runs on: between them, every way the +# configurations listed can change. +SHAPES = { + # Only the unsaved content group configuration. + "bare": {}, + # Experiments turned on, none defined: still only the unsaved configuration. + "experiments_on_none_defined": {"experiments_on": True}, + # Experiment configurations defined, experiments turned off: not listed. + "experiments_off": {"experiments": True}, + # One enrollment track: its configuration is not listed. + "one_track": {"groups": True, "tracks": 1}, + # Everything listed: two tracks, Studio-configurable gating, content groups, experiments. + "rich": {"groups": True, "experiments": True, "experiments_on": True, "tracks": 2, "gating": True}, + # Content groups for teams on, and a team-set with two teams: its configuration is listed after content groups. + "team_groups": {"groups": True, "teams": 2, "team_groups_on": True}, +} + + +def legacy_items(body): + """The configurations of a v1 body in the order v2 lists them.""" + return [*body["all_group_configurations"], *(body["experiment_group_configurations"] or [])] + + +def without_usage(configurations): + """``configurations`` without ``usage``, on configurations and groups alike.""" + stripped = [] + for configuration in configurations: + item = {key: value for key, value in configuration.items() if key != "usage"} + item["groups"] = [{key: value for key, value in group.items() if key != "usage"} for group in item["groups"]] + stripped.append(item) + return stripped + + +def without_can_manage(configurations): + return [{key: value for key, value in item.items() if key != "can_manage"} for item in configurations] + + +# --------------------------------------------------------------------------- +# Who may call the endpoint +# --------------------------------------------------------------------------- + +def superuser_without_staff_status(): + """ + Return a superuser whose staff status is off. + + Saving a superuser turns its staff status on (a pre-save receiver of the + student app does), so the status is cleared with an update, which skips it. + """ + user = UserFactory(is_superuser=True) + get_user_model().objects.filter(pk=user.pk).update(is_staff=False) + user.refresh_from_db() + assert (user.is_superuser, user.is_staff) == (True, False) + return user + + +class _AccessMixin: + """Checks a caller's status against the expected one, and that v1 decides the same for the same caller.""" + + def assert_list(self, user, expected, course_key=None, can_manage=None): + """ + Assert the status of a listing by ``user``; on success that every + configuration carries ``can_manage``, as v1 reports it once. + """ + course_key = course_key or self.course.id + response = api_client(user).get(url(course_key), ALL) + legacy = api_client(user).get(legacy_url(course_key)) + assert response.status_code == expected, response.content + assert legacy.status_code == expected, f"v1 decided {legacy.status_code} for the same caller" + if expected != 200: + assert_refused(response, expected) + return + results = response.json()["results"] + assert [configuration["id"] for configuration in results] == [CONTENT_GROUPS_ID] + assert [configuration["can_manage"] for configuration in results] == [can_manage] + assert legacy.json()["can_manage"] is can_manage + + +@ddt.ddt +class GroupConfigurationsAccessLegacyPermissionsTest(_AccessMixin, SharedModuleStoreTestCase): + """Who may list group configurations when policy-based authoring permissions are off.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + store_partitions(cls.course.id, [content_groups()]) + cls.other_course = CourseFactory.create() + + def _user_with(self, role): + user = UserFactory() + role.add_users(user) + return user + + def test_anonymous_caller_is_refused_with_401(self): + self.assert_list(None, 401) + + def test_anonymous_caller_of_an_unknown_course_is_refused_with_401(self): + self.assert_list(None, 401, course_key=UNKNOWN_COURSE_KEY) + + def test_user_without_a_role_is_refused_with_403(self): + self.assert_list(UserFactory(), 403) + + def test_unknown_course_is_404_before_permission_is_checked(self): + self.assert_list(UserFactory(), 404, course_key=UNKNOWN_COURSE_KEY) + + def test_limited_staff_is_refused(self): + self.assert_list(self._user_with(CourseLimitedStaffRole(self.course.id)), 403) + + def test_staff_and_instructors_of_another_course_are_refused(self): + self.assert_list(self._user_with(CourseStaffRole(self.other_course.id)), 403) + self.assert_list(self._user_with(CourseInstructorRole(self.other_course.id)), 403) + + def test_course_staff_may_list_and_manage(self): + self.assert_list(self._user_with(CourseStaffRole(self.course.id)), 200, can_manage=True) + + def test_course_instructor_may_list_and_manage(self): + self.assert_list(self._user_with(CourseInstructorRole(self.course.id)), 200, can_manage=True) + + def test_org_staff_and_org_instructor_may_list_and_manage(self): + self.assert_list(self._user_with(OrgStaffRole(self.course.id.org)), 200, can_manage=True) + self.assert_list(self._user_with(OrgInstructorRole(self.course.id.org)), 200, can_manage=True) + + def test_global_staff_may_list_and_manage(self): + self.assert_list(UserFactory(is_staff=True), 200, can_manage=True) + + def test_a_superuser_saved_through_the_model_is_global_staff_and_may_list_and_manage(self): + """Saving a superuser turns its staff status on, so it passes as global staff, on v1 as on v2.""" + user = UserFactory(is_superuser=True, is_staff=False) + assert user.is_staff is True + self.assert_list(user, 200, can_manage=True) + + def test_a_superuser_without_staff_status_is_refused(self): + """ + Studio read access comes from staff status or a course or organization + role, and being a superuser is neither; v1 refuses the same caller. + """ + self.assert_list(superuser_without_staff_status(), 403) + + def test_a_version_only_course_key_is_404(self): + """The key parses but names no course run; v1 has no address that takes it.""" + assert_refused(api_client(UserFactory(is_staff=True)).get(url(VERSION_ONLY_KEY)), 404) + + def test_a_library_is_not_a_course(self): + library_key = LibraryFactory.create().location.library_key + assert_refused(api_client(UserFactory(is_staff=True)).get(url(library_key)), 404) + + +@ddt.ddt +class GroupConfigurationsAccessAuthzTest(CourseAuthoringAuthzTestMixin, _AccessMixin, SharedModuleStoreTestCase): + """ + Who may list group configurations when policy-based authoring permissions + are on: listing needs the permission to view them, and can_manage reports + the permission to manage them. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + store_partitions(cls.course.id, [content_groups()]) + cls.other_course = CourseFactory.create() + + def _user_with(self, role, course_key=None): + user = UserFactory() + self.add_user_to_role_in_course(user, role, course_key or self.course.id) + return user + + def test_anonymous_caller_is_refused_with_401(self): + self.assert_list(None, 401) + + def test_user_without_a_role_is_refused_with_403(self): + self.assert_list(self.unauthorized_user, 403) + + def test_unknown_course_is_404_before_permission_is_checked(self): + self.assert_list(self.unauthorized_user, 404, course_key=UNKNOWN_COURSE_KEY) + + def test_a_role_that_may_only_view_lists_without_can_manage(self): + self.assert_list(self._user_with(COURSE_AUDITOR.external_key), 200, can_manage=False) + + @ddt.data(COURSE_EDITOR.external_key, COURSE_STAFF.external_key, COURSE_ADMIN.external_key) + def test_roles_that_may_manage_list_with_can_manage(self, role): + self.assert_list(self._user_with(role), 200, can_manage=True) + + @ddt.data(COURSE_LIMITED_STAFF.external_key, COURSE_DATA_RESEARCHER.external_key, COURSE_BETA_TESTER.external_key) + def test_other_roles_are_refused(self, role): + self.assert_list(self._user_with(role), 403) + + def test_a_role_on_another_course_is_refused(self): + self.assert_list(self._user_with(COURSE_ADMIN.external_key, self.other_course.id), 403) + + def test_legacy_course_staff_without_a_policy_role_is_refused(self): + user = UserFactory() + CourseAccessRole.objects.create(user=user, role="staff", course_id=self.course.id, org=self.course.id.org) + self.assert_list(user, 403) + + def test_staff_and_superuser_may_list_and_manage_without_a_role(self): + """The mixin's superuser was saved through the model, which turned its staff status on.""" + self.assert_list(self.staff_user, 200, can_manage=True) + self.assert_list(self.super_user, 200, can_manage=True) + + def test_a_superuser_without_staff_status_may_list_and_manage_without_a_role(self): + """The permission policy admits superusers on a course whether or not they have staff status, as on v1.""" + self.assert_list(superuser_without_staff_status(), 200, can_manage=True) + + def test_an_inactive_session_with_a_role_is_accepted(self): + """The policy does not look at whether the account is activated, on v2 as on v1.""" + user = self._user_with(COURSE_AUDITOR.external_key) + client = session_client(user) + user.is_active = False + user.save() + assert client.get(url(self.course.id)).status_code == 200 + assert client.get(legacy_url(self.course.id)).status_code == 200 + + +class GroupConfigurationsPermissionWiringTest(SimpleTestCase): + """Which permissions the viewset checks, and what the view permission asks for.""" + + @staticmethod + def _declared(view): + # The JWT middleware appends NotJwtRestrictedApplication to views that take JWTs. + return [ + type(permission) for permission in view.get_permissions() + if not isinstance(permission, NotJwtRestrictedApplication) + ] + + def test_the_listing_checks_the_course_then_the_permission_to_view(self): + view = CourseGroupConfigurationsViewSet() + for action in ("list", None, "metadata"): + view.action = action + assert self._declared(view) == [IsAuthenticated, IsExistingCourse, CanViewGroupConfigurations], action + + def test_any_other_action_is_refused(self): + view = CourseGroupConfigurationsViewSet() + for action in ("retrieve", "create", "update", "partial_update", "destroy"): + view.action = action + with pytest.raises(ImproperlyConfigured): + view.get_permissions() + + def test_a_routed_action_the_viewset_does_not_check_is_never_served(self): + """A request reaching such an action is answered with a server error, not the handler's body.""" + + class Unchecked(CourseGroupConfigurationsViewSet): + permission_classes = CourseGroupConfigurationsViewSet.permission_classes + + def destroy(self, request, **kwargs): # pylint: disable=unused-argument + raise AssertionError("the handler ran without a permission check") + + request = APIRequestFactory().delete(url(COURSE_KEY)) + force_authenticate(request, user=SimpleNamespace(is_authenticated=True, is_active=True)) + response = Unchecked.as_view({"delete": "destroy"})(request, course_key=CourseKey.from_string(COURSE_KEY)) + assert_error_envelope(response, expected_status=500, expected_type_slug="internal") + + def test_the_view_permission_asks_for_its_policy_and_studio_read_access(self): + course_key = CourseKey.from_string("course-v1:edX+Wiring+Run") + request = SimpleNamespace(user=object()) + view = SimpleNamespace(kwargs={"course_key": course_key}) + target = "cms.djangoapps.contentstore.views.permissions.user_has_course_permission" + for decision in (True, False): + with patch(target, return_value=decision) as check: + assert CanViewGroupConfigurations().has_permission(request, view) is decision + check.assert_called_once_with( + request.user, COURSES_VIEW_GROUP_CONFIGURATIONS.identifier, course_key, LegacyAuthoringPermission.READ, + ) + + def test_the_view_permission_refuses_a_view_without_a_course(self): + target = "cms.djangoapps.contentstore.views.permissions.user_has_course_permission" + with patch(target, return_value=True) as check: + permission = CanViewGroupConfigurations() + assert permission.has_permission(SimpleNamespace(user=object()), SimpleNamespace(kwargs={})) is False + check.assert_not_called() + + +class GroupConfigurationsAuthenticationTest(SharedModuleStoreTestCase): + """How callers authenticate, with policy-based authoring permissions off.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def _course_staff(self): + user = UserFactory() + CourseStaffRole(self.course.id).add_users(user) + return user + + def test_a_jwt_caller_may_list(self): + header = {"HTTP_AUTHORIZATION": f"JWT {create_jwt_for_user(self._course_staff())}"} + response = APIClient().get(url(self.course.id), **header) + assert response.status_code == 200, response.content + assert response.json()["count"] == 1 + + def test_a_session_caller_may_list(self): + response = session_client(self._course_staff()).get(url(self.course.id)) + assert response.status_code == 200, response.content + assert response.json()["count"] == 1 + + def test_an_inactive_session_is_authenticated(self): + """ + A session whose account is not activated is accepted, as on v1. Global + staff then pass; course staff are refused (403, not 401) because a + course role does not count while the account is inactive. + """ + for user, expected in ((UserFactory(is_staff=True), 200), (self._course_staff(), 403)): + client = session_client(user) + user.is_active = False + user.save() + response = client.get(url(self.course.id)) + assert response.status_code == expected, response.content + if expected == 403: + assert_refused(response, 403) + assert client.get(legacy_url(self.course.id)).status_code == expected + + def test_a_bearer_token_is_refused(self): + """Bearer tokens are not accepted here, though v1 takes them.""" + user = UserFactory(is_staff=True) + token = AccessTokenFactory(user=user, application=ApplicationFactory(user=user)).token + header = {"HTTP_AUTHORIZATION": f"Bearer {token}"} + assert_refused(APIClient().get(url(self.course.id), **header), 401) + assert APIClient().get(legacy_url(self.course.id), **header).status_code == 200 + + +# --------------------------------------------------------------------------- +# Listing +# --------------------------------------------------------------------------- + +@ddt.ddt +class GroupConfigurationsListTest(ModuleStoreTestCase): + """What a listing returns, in which order, and how it pages.""" + + def setUp(self): + super().setUp() + self.client = api_client(UserFactory(is_staff=True)) + + def get(self, course_key, **params): + response = self.client.get(url(course_key), params) + assert response.status_code == 200, response.content + return response.json() + + def test_the_configurations_are_listed_in_studio_order(self): + """Enrollment tracks, content type gating, content groups, then the experiments in course order.""" + course = build_course(**SHAPES["rich"]) + results = self.get(course.id, **ALL)["results"] + assert [(c["id"], c["scheme"]) for c in results] == [ + (ENROLLMENT_TRACK_PARTITION_ID, "enrollment_track"), + (CONTENT_GATING_PARTITION_ID, "content_type_gate"), + (CONTENT_GROUPS_ID, "cohort"), + (EXPERIMENT_ID, "random"), + (UNUSED_EXPERIMENT_ID, "random"), + ] + assert [c["read_only"] for c in results[:3]] == [True, True, False] + assert ["read_only" in c for c in results[3:]] == [False, False] + + def test_the_default_view_leaves_out_usage(self): + course = build_course(**SHAPES["rich"]) + for configuration in self.get(course.id, **ALL)["results"]: + assert set(configuration) == CONFIGURATION_MEMBERS | ({"read_only"} & set(configuration)) + assert configuration["groups"] + assert [set(group) for group in configuration["groups"]] == [GROUP_MEMBERS] * len(configuration["groups"]) + + def test_the_minimal_view_is_the_default(self): + course = build_course(**SHAPES["rich"]) + assert self.get(course.id, view="minimal", **ALL) == self.get(course.id, **ALL) + + def test_the_full_view_reports_where_each_configuration_and_group_is_used(self): + course = build_course(**SHAPES["rich"]) + by_id = {c["id"]: c for c in self.get(course.id, **FULL, **ALL)["results"]} + unit_key = course.id.make_usage_key("vertical", course.unit.location.block_id) + unit_url = reverse_usage_url("container_handler", unit_key) + groups = by_id[CONTENT_GROUPS_ID] + assert groups["usage"] is None + assert [(group["id"], group["usage"]) for group in groups["groups"]] == [ + (1, [{"label": "Unit 1 / Honors problem", "url": unit_url}]), + (2, []), + ] + # The experiment has no content for its groups, so Studio reports a problem with it. + message = modulestore().get_item(course.split_test.location).general_validation_message() + assert by_id[EXPERIMENT_ID]["usage"] == [{ + "label": "Unit 1 / Experiment", + "url": reverse_usage_url( + "container_handler", course.id.make_usage_key("split_test", course.split_test.location.block_id), + ), + "validation": {"type": message.type, "text": str(message.text)}, + }] + assert [group["usage"] for group in by_id[EXPERIMENT_ID]["groups"]] == [None, None] + assert by_id[UNUSED_EXPERIMENT_ID]["usage"] == [] + tracks = by_id[ENROLLMENT_TRACK_PARTITION_ID] + assert tracks["usage"] is None + assert [group["usage"] for group in tracks["groups"]] == [[], []] + + def test_an_experiment_without_problems_has_a_null_validation(self): + course = build_course(**SHAPES["rich"]) + with patch.object(SplitTestBlock, "general_validation_message", return_value=None): + legacy = self.client.get(legacy_url(course.id)).json() + by_id = {c["id"]: c for c in self.get(course.id, **FULL, **ALL)["results"]} + assert [use["validation"] for use in by_id[EXPERIMENT_ID]["usage"]] == [None] + assert by_id[EXPERIMENT_ID]["usage"] == legacy["experiment_group_configurations"][0]["usage"] + + def test_whether_the_caller_may_manage_is_asked_once_per_request(self): + course = build_course(**SHAPES["rich"]) + with patch(f"{SERVICE_MODULE}.user_has_course_permission", return_value=False) as check: + response = self.client.get(url(course.id), {**FULL, **ALL}) + assert response.status_code == 200, response.content + assert [c["can_manage"] for c in response.json()["results"]] == [False] * 5 + check.assert_called_once() + user, permission, course_key, legacy = check.call_args.args + assert (permission, course_key, legacy) == ( + COURSES_MANAGE_GROUP_CONFIGURATIONS.identifier, course.id, LegacyAuthoringPermission.WRITE, + ) + assert user.is_staff is True + + def test_a_course_without_content_groups_lists_an_unsaved_content_group_configuration(self): + course = build_course() + with patch(ID_GENERATOR, side_effect=[111111, 222222]): + first = self.get(course.id)["results"] + second = self.get(course.id)["results"] + assert [(c["id"], c["scheme"], c["name"], c["groups"]) for c in first] == [ + (111111, "cohort", "Content Groups", []), + ] + assert [c["id"] for c in second] == [222222] + assert "read_only" not in first[0] + assert _stored_course(course.id).user_partitions == [] + + def test_saved_content_groups_replace_the_unsaved_configuration(self): + course = build_course(groups=True) + with patch(ID_GENERATOR) as generate: + results = self.get(course.id)["results"] + generate.assert_not_called() + assert [(c["id"], c["scheme"]) for c in results] == [(CONTENT_GROUPS_ID, "cohort")] + + @ddt.data(True, False) + def test_experiment_configurations_are_listed_only_when_experiments_are_on(self, on): + course = build_course(groups=True, experiments=True, experiments_on=on) + schemes = [c["scheme"] for c in self.get(course.id, **ALL)["results"]] + assert schemes == (["cohort", "random", "random"] if on else ["cohort"]) + + @ddt.data((0, False), (1, False), (2, True), (3, True)) + @ddt.unpack + def test_the_enrollment_track_configuration_is_listed_with_two_tracks_or_more(self, tracks, listed): + course = build_course(groups=True, tracks=tracks) + results = self.get(course.id)["results"] + assert any(c["scheme"] == "enrollment_track" for c in results) is listed + if listed: + assert len(results[0]["groups"]) == tracks + + @ddt.data(True, False) + def test_content_type_gating_is_listed_only_when_studio_may_set_it(self, studio_override): + """Gating is on for the course either way; Studio lists its configuration only when it may set it.""" + course = build_course(groups=True, tracks=2) + ContentTypeGatingConfig.objects.create( + enabled=True, enabled_as_of=datetime(2018, 1, 1, tzinfo=UTC), studio_override_enabled=studio_override, + ) + partitions = get_all_partitions_for_course(_stored_course(course.id)) + assert "content_type_gate" in [partition.scheme.name for partition in partitions] + schemes = [c["scheme"] for c in self.get(course.id)["results"]] + gating = ["content_type_gate"] if studio_override else [] + assert schemes == ["enrollment_track", *gating, "cohort"] + + @ddt.data((True, 2, True), (False, 2, False), (True, 0, False)) + @ddt.unpack + def test_a_team_set_is_listed_only_with_content_groups_for_teams_on_and_a_team_in_it(self, on, teams, listed): + """ + With content groups for teams off the team-set has no configuration; + with them on, its configuration is listed once it has a team. v1 lists + the same configurations. + """ + course = build_course(groups=True, teams=teams, team_groups_on=on) + legacy = self.client.get(legacy_url(course.id)) + assert legacy.status_code == 200, legacy.content + results = self.get(course.id, **FULL, **ALL)["results"] + listing = [(c["id"], c["scheme"]) for c in results] + assert listing == [(c["id"], c["scheme"]) for c in legacy_items(legacy.json())] + assert listing == [(CONTENT_GROUPS_ID, "cohort")] + ([(TEAM_PARTITION_ID, "team")] if listed else []) + if listed: + team_set = results[1] + assert (team_set["read_only"], team_set["parameters"]) == ( + True, {"course_id": str(course.id), "team_set_id": TEAM_SET_ID}, + ) + assert [(group["id"], group["name"]) for group in team_set["groups"]] == [ + (FIRST_TEAM_ID, "Debate team 0"), (FIRST_TEAM_ID + 1, "Debate team 1"), + ] + unit_key = course.id.make_usage_key("vertical", course.unit.location.block_id) + unit_url = reverse_usage_url("container_handler", unit_key) + assert [group["usage"] for group in team_set["groups"]] == [ + [{"label": "Unit 1 / Team problem", "url": unit_url}], [], + ] + + def test_the_envelope_has_the_seven_keys(self): + course = build_course(**SHAPES["rich"]) + body = self.get(course.id) + assert set(body) == set(ENVELOPE_KEYS) + assert (body["count"], body["num_pages"], body["current_page"], body["start"]) == (5, 1, 1, 0) + assert (body["next"], body["previous"]) == (None, None) + + def test_page_size_splits_the_pages_in_order(self): + course = build_course(**SHAPES["rich"]) + ids = [c["id"] for c in self.get(course.id, **ALL)["results"]] + first = self.get(course.id, page_size=2) + assert [c["id"] for c in first["results"]] == ids[:2] + assert (first["count"], first["num_pages"], first["current_page"], first["start"]) == (5, 3, 1, 0) + assert "page=2" in first["next"] + last = self.get(course.id, page_size=2, page=3, **FULL) + assert [c["id"] for c in last["results"]] == ids[4:] + assert (last["current_page"], last["start"], last["next"]) == (3, 4, None) + assert "usage" in last["results"][0] + + def test_the_default_page_size_is_ten(self): + course = build_course() + store_partitions(course.id, [content_groups(100 + index, f"Groups {index}") for index in range(12)]) + body = self.get(course.id) + assert (len(body["results"]), body["count"], body["num_pages"]) == (10, 12, 2) + assert [c["id"] for c in body["results"]] == list(range(100, 110)) + + @ddt.data("99", "0", "abc") + def test_a_page_out_of_range_is_404(self, page): + course = build_course() + response = self.client.get(url(course.id), {"page": page}) + envelope = assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + assert envelope["detail"] == "Invalid page." + + def test_an_unknown_view_is_refused(self): + course = build_course() + response = self.client.get(url(course.id), {"view": "default"}) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"view": ['"default" is not a valid choice.']} + + @ddt.data({}, FULL) + def test_the_body_matches_the_published_schema(self, params): + course = build_course(**SHAPES["rich"]) + for page in (1, 2): + body = self.get(course.id, page=page, page_size=3, **params) + assert_matches_published_schema(body, "PaginatedGroupConfiguration.v2List") + + def test_other_query_parameters_are_ignored(self): + course = build_course(**SHAPES["rich"]) + assert self.get(course.id, ordering="-name", fields="id", scheme="random") == self.get(course.id) + + def test_head_answers_without_a_body(self): + course = build_course() + for params in ({}, FULL): + response = self.client.head(url(course.id), params) + assert response.status_code == 200 + assert response.content == b"" + + +# --------------------------------------------------------------------------- +# Requests v1 and v2 answer differently +# --------------------------------------------------------------------------- + +# Requests v1 and v2 answer differently beyond the error body, as +# (request, v1 answer, v2 answer, test in GroupConfigurationsResponseDifferencesTest). +RESPONSE_DIFFERENCES = [ + ("any read", "an object holding all_group_configurations and experiment_group_configurations", + "a page of configurations, the experiment configurations last", "test_the_two_lists_become_one_page"), + ("any read", "can_manage once, beside the lists", "can_manage on every configuration", + "test_can_manage_is_on_every_configuration"), + ("read without view", "usage on every configuration and group", "no usage; view=full includes it", + "test_usage_is_returned_only_with_view_full"), + ("any read", "should_show_enrollment_track", + "not returned; the enrollment_track configuration is listed exactly when it was true", + "test_should_show_enrollment_track"), + ("any read", "should_show_experiment_groups", "not returned: it describes the course", + "test_should_show_experiment_groups"), + ("any read", "mfe_proctored_exam_settings_url", "not returned: it describes the course", + "test_mfe_proctored_exam_settings_url"), + ("read with view=default", "200, the parameter ignored", "400", "test_view_default"), + ("read with a page past the last, zero or not a number", "200, the parameter ignored", "404", + "test_an_invalid_page"), + ("any read for a course the modulestore returns nothing for", "500", "404", + "test_a_course_the_modulestore_returns_nothing_for"), + ("OPTIONS by a signed-in caller without access, or for an unknown course", "200", "403 or 404", + "test_options"), + ("any read with a Bearer token", "200", "401", "test_a_bearer_token"), + ("read for a slash-separated course key", "404 JSON", "HTML 404 at routing", "test_a_slash_separated_course_key"), +] + + +class GroupConfigurationsResponseDifferencesTest(ModuleStoreTestCase): + """Each request v1 and v2 answer differently, both answers observed.""" + + def setUp(self): + super().setUp() + self.course = build_course(**SHAPES["rich"]) + self.client = api_client(UserFactory(is_staff=True)) + + def both(self, course_key=None, **params): + """Read the course through v1 and v2 (with ``params``), assert both succeed and return both bodies.""" + course_key = course_key or self.course.id + legacy = self.client.get(legacy_url(course_key)) + new = self.client.get(url(course_key), {**ALL, **params}) + assert legacy.status_code == new.status_code == 200, (legacy.content, new.content) + return legacy.json(), new.json() + + def test_every_listed_difference_has_its_test(self): + assert [name for *_, name in RESPONSE_DIFFERENCES if not hasattr(self, name)] == [] + + def test_the_two_lists_become_one_page(self): + legacy, new = self.both(**FULL) + assert len(legacy["all_group_configurations"]) == 3 + assert len(legacy["experiment_group_configurations"]) == 2 + assert set(new) == set(ENVELOPE_KEYS) + assert [c["id"] for c in new["results"]] == [c["id"] for c in legacy_items(legacy)] + + def test_can_manage_is_on_every_configuration(self): + legacy, new = self.both() + assert legacy["can_manage"] is True + assert [c["can_manage"] for c in new["results"]] == [True] * 5 + assert all("can_manage" not in c for c in legacy_items(legacy)) + + def test_usage_is_returned_only_with_view_full(self): + legacy, new = self.both() + assert all("usage" in c for c in legacy_items(legacy)) + assert all("usage" in g for c in legacy["all_group_configurations"] for g in c["groups"]) + assert all("usage" not in c and all("usage" not in g for g in c["groups"]) for c in new["results"]) + _, full = self.both(**FULL) + assert without_can_manage(full["results"]) == legacy_items(legacy) + + def test_should_show_enrollment_track(self): + one_track = build_course(groups=True, tracks=1) + for course_key, shown in ((self.course.id, True), (one_track.id, False)): + legacy, new = self.both(course_key) + assert legacy["should_show_enrollment_track"] is shown + assert any(c["scheme"] == "enrollment_track" for c in new["results"]) is shown + assert "should_show_enrollment_track" not in new + + def test_should_show_experiment_groups(self): + """Experiments turned on with none defined lists the same as turned off; only v1's flag tells them apart.""" + on_none_defined = build_course(**SHAPES["experiments_on_none_defined"]) + off = build_course(**SHAPES["experiments_off"]) + with patch(ID_GENERATOR, return_value=PLACEHOLDER_ID): + for course_key, shown, experiments in ((on_none_defined.id, True, []), (off.id, False, None)): + legacy, new = self.both(course_key) + assert legacy["should_show_experiment_groups"] is shown + assert legacy["experiment_group_configurations"] == experiments + assert [c["id"] for c in new["results"]] == [PLACEHOLDER_ID] + assert "should_show_experiment_groups" not in new + + def test_mfe_proctored_exam_settings_url(self): + legacy, new = self.both() + assert "mfe_proctored_exam_settings_url" in legacy + assert "mfe_proctored_exam_settings_url" not in new + assert all("mfe_proctored_exam_settings_url" not in c for c in new["results"]) + + def test_view_default(self): + """v1 takes no view parameter and ignores it; v2 refuses a value it does not know.""" + legacy = self.client.get(legacy_url(self.course.id), {"view": "default"}) + assert legacy.status_code == 200, legacy.content + assert legacy.json() == self.client.get(legacy_url(self.course.id)).json() + response = self.client.get(url(self.course.id), {"view": "default"}) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"view": ['"default" is not a valid choice.']} + + def test_an_invalid_page(self): + """v1 is not paged and ignores page; v2 answers a page it does not have with 404.""" + whole = self.client.get(legacy_url(self.course.id)).json() + for page in ("99", "0", "abc"): + legacy = self.client.get(legacy_url(self.course.id), {"page": page}) + assert legacy.status_code == 200, (page, legacy.content) + assert legacy.json() == whole, page + response = self.client.get(url(self.course.id), {"page": page}) + envelope = assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + assert envelope["detail"] == "Invalid page.", page + + def test_a_course_the_modulestore_returns_nothing_for(self): + with patch.object(MixedModuleStore, "get_course", return_value=None): + legacy = self.client.get(legacy_url(self.course.id)) + new = self.client.get(url(self.course.id)) + assert legacy.status_code == 500, legacy.content + assert_refused(new, 404) + + def test_options(self): + """v1 answers OPTIONS to any signed-in caller; v2 checks the course and the caller first.""" + outsider = api_client(UserFactory()) + assert outsider.options(legacy_url(self.course.id)).status_code == 200 + assert_refused(outsider.options(url(self.course.id)), 403) + assert self.client.options(url(self.course.id)).status_code == 200 + assert outsider.options(legacy_url(UNKNOWN_COURSE_KEY)).status_code == 200 + assert_refused(outsider.options(url(UNKNOWN_COURSE_KEY)), 404) + + def test_a_bearer_token(self): + user = UserFactory(is_staff=True) + token = AccessTokenFactory(user=user, application=ApplicationFactory(user=user)).token + header = {"HTTP_AUTHORIZATION": f"Bearer {token}"} + assert APIClient().get(legacy_url(self.course.id), **header).status_code == 200 + assert_refused(APIClient().get(url(self.course.id), **header), 401) + + def test_a_slash_separated_course_key(self): + """v1 routes the slash form and reports the course unknown in JSON; v2 has no route for it.""" + legacy = self.client.get("/api/contentstore/v1/group_configurations/edX/NoSuch/Course") + assert legacy.status_code == 404 + assert legacy.json()["error_code"] == "course_does_not_exist" + new = self.client.get("/api/authoring/v2/courses/edX/NoSuch/Course/group_configurations/") + assert new.status_code == 404 + assert new["Content-Type"].startswith("text/html") + + +# --------------------------------------------------------------------------- +# Parity with v1 +# --------------------------------------------------------------------------- + +# Members of an error body that differ between v1 and v2 for the same +# request. The comparison asserts nothing else differs, and that each of these +# occurs in the error cases below. +INTENTIONAL_DIFFERENCES = [ + ("type", "errors use the standardized envelope"), + ("title", "errors use the standardized envelope"), + ("status", "errors use the standardized envelope"), + ("detail", "errors use the standardized envelope"), + ("developer_message", "the v1 error body carries developer_message; the envelope does not"), + ("error_code", "the v1 refusal and not-found bodies carry error_code; the envelope does not"), +] + + +def _undeclared(changed): + """The changed paths no declared intentional difference of this module covers.""" + return undeclared_paths(changed, INTENTIONAL_DIFFERENCES) + + +def test_only_declared_differences_are_tolerated(): + assert _undeclared({"type", "developer_message", "error_code"}) == set() + changed = {"[0].usage", "can_manage", "results", "errors"} + assert _undeclared(changed) == changed + + +def test_the_comparison_sees_a_usage_that_turns_null_or_is_dropped(): + """The comparison the parity checks rely on reports a usage list that becomes null, or is dropped.""" + used = [{"label": "a", "url": "b"}] + legacy = [{"id": 1, "usage": [], "groups": [{"id": 2, "usage": used}]}] + turned_null = [{"id": 1, "usage": None, "groups": [{"id": 2, "usage": used}]}] + dropped = [{"id": 1, "usage": [], "groups": [{"id": 2}]}] + assert changed_paths(legacy, turned_null) == {"[0].usage"} + assert changed_paths(legacy, dropped) == {"[0].groups[0].usage[0].label", "[0].groups[0].usage[0].url"} + + +@ddt.ddt +class _ParityMixin: + """ + v2 against v1 for the same course and caller: v2's configurations are v1's + two lists in order, each with v1's can_manage, identical with view=full + and without usage otherwise; errors differ only by the declared + differences. + """ + + def reader_for(self, course_key): + """Return a user who may view the course's group configurations.""" + raise NotImplementedError + + def outsider(self): + """Return a signed-in user with no access to the course.""" + raise NotImplementedError + + def setUp(self): + super().setUp() + self.observed = set() + + @ddt.data(*SHAPES) + def test_the_configurations_are_v1s_on_both_views(self, shape): + course = build_course(**SHAPES[shape]) + client = api_client(self.reader_for(course.id)) + # The unsaved configuration's identifier is drawn afresh by each read; fixed here so reads compare. + with patch(ID_GENERATOR, return_value=PLACEHOLDER_ID): + legacy = client.get(legacy_url(course.id)) + full = client.get(url(course.id), {**FULL, **ALL}) + minimal = client.get(url(course.id), ALL) + assert legacy.status_code == full.status_code == minimal.status_code == 200 + legacy, full, minimal = legacy.json(), full.json()["results"], minimal.json()["results"] + expected = legacy_items(legacy) + assert expected, shape + assert changed_paths(expected, without_can_manage(full)) == set(), (expected, full) + assert changed_paths(without_usage(expected), without_can_manage(minimal)) == set(), (expected, minimal) + assert {c["can_manage"] for c in full + minimal} == {legacy["can_manage"]} + + def _error_pairs(self, course_key): + """``(legacy, new, status)`` for each error both versions produce.""" + cases = [ + (api_client(), course_key, 401), + (api_client(self.outsider()), course_key, 403), + (api_client(self.outsider()), UNKNOWN_COURSE_KEY, 404), + ] + for client, key, status in cases: + yield client.get(legacy_url(key)), client.get(url(key)), status + + def compare(self, legacy, new, expected_status): + """Assert both statuses and that the bodies differ only as declared; return both bodies.""" + assert legacy.status_code == expected_status, legacy.content + assert new.status_code == expected_status, new.content + legacy_body, new_body = json.loads(legacy.content), json.loads(new.content) + changed = changed_paths(legacy_body, new_body) + assert _undeclared(changed) == set(), f"undeclared differences: legacy={legacy_body} new={new_body}" + self.observed |= changed + return legacy_body, new_body + + def test_errors_differ_only_by_the_envelope_and_every_declared_difference_occurs(self): + course = build_course(groups=True) + for legacy, new, status in self._error_pairs(course.id): + self.compare(legacy, new, status) + missing = [ + path for path, _ in INTENTIONAL_DIFFERENCES + if not any(key == path or key.startswith(f"{path}.") or key.startswith(f"{path}[") for key in self.observed) + ] + assert missing == [] + + +class GroupConfigurationsParityLegacyPermissionsTest(_ParityMixin, ModuleStoreTestCase): + """Parity for course staff, with policy-based authoring permissions off.""" + + def reader_for(self, course_key): + reader = UserFactory() + CourseStaffRole(course_key).add_users(reader) + return reader + + def outsider(self): + return UserFactory() + + +class GroupConfigurationsParityAuthzTest(_ParityMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase): + """Parity for a course auditor, with policy-based authoring permissions on.""" + + def reader_for(self, course_key): + reader = UserFactory() + self.add_user_to_role_in_course(reader, COURSE_AUDITOR.external_key, course_key) + return reader + + def outsider(self): + return self.unauthorized_user + + +# --------------------------------------------------------------------------- +# Side effects and cost of listing +# --------------------------------------------------------------------------- + +READS = [ + ("get", {}), ("get", FULL), ("get", {"page_size": 2, "page": 2, **FULL}), ("head", {}), ("head", FULL), +] + + +@ddt.ddt +class _ListingIsSideEffectFreeMixin: + """ + Every listing (default, full, paged, and HEAD on both views), on a course + listing only the unsaved content group configuration, on one listing + every kind and on one listing a team-set's configuration, writes no row + and no Mongo document, stores nothing through the modulestore and sends + no signal or event. The listings start on a + database that has never served a role or permission lookup, so the first + may insert the permission library's one policy version row and nothing + else; v1 inserts the same row. + """ + + def reader_for(self, course_key): + """Return a user who may view the course, leaving the database as no lookup had touched it.""" + raise NotImplementedError + + @ddt.data("bare", "rich", "team_groups") + def test_listings_have_no_side_effects(self, shape): + course = build_course(**SHAPES[shape]) + partitions = _stored_course(course.id).user_partitions + client = api_client(self.reader_for(course.id)) + assert not PolicyCacheControl.objects.exists() + for index, (method, params) in enumerate(READS): + with record_effects(self) as effects: + response = getattr(client, method)(url(course.id), params) + read = (shape, index, method, params) + first = index == 0 + assert response.status_code == 200, (read, response.content) + assert effects["sql_writes"] == (FIRST_LOOKUP_WRITES if first else []), read + assert effects["signals"] == (FIRST_LOOKUP_SIGNALS if first else []), read + assert effects["events"] == [], read + assert effects["mongo_writes"] == 0, read + assert effects["update_item"] == [], read + assert _stored_course(course.id).user_partitions == partitions + + def test_the_v1_listing_makes_the_same_first_lookup_write(self): + course = build_course(**SHAPES["rich"]) + client = api_client(self.reader_for(course.id)) + with record_effects(self) as effects: + response = client.get(legacy_url(course.id)) + assert response.status_code == 200, response.content + assert effects["sql_writes"] == FIRST_LOOKUP_WRITES + assert effects["signals"] == FIRST_LOOKUP_SIGNALS + assert effects["mongo_writes"] == 0 + + +class GroupConfigurationsSideEffectsLegacyPermissionsTest(_ListingIsSideEffectFreeMixin, ModuleStoreTestCase): + """Listings by course staff with policy-based authoring permissions off.""" + + def reader_for(self, course_key): + reader = UserFactory() + CourseStaffRole(course_key).add_users(reader) + PolicyCacheControl.objects.all().delete() + return reader + + +class GroupConfigurationsSideEffectsAuthzTest( + _ListingIsSideEffectFreeMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase, +): + """ + Listings by a course auditor with policy-based authoring permissions on. + The role is assigned through the permission library, which creates the + policy version row; it is deleted before the listings. + """ + + def reader_for(self, course_key): + reader = UserFactory() + self.add_user_to_role_in_course(reader, COURSE_AUDITOR.external_key, course_key) + PolicyCacheControl.objects.all().delete() + return reader + + +@ddt.ddt +class _ListingCostMixin: + """A listing, default or full, costs no more SQL queries or Mongo reads than the v1 read of the same course.""" + + def reader_for(self, course_key): + raise NotImplementedError + + @ddt.data("bare", "rich", "team_groups") + def test_a_listing_costs_no_more_than_the_v1_read(self, shape): + course = build_course(**SHAPES[shape]) + client = api_client(self.reader_for(course.id)) + legacy = measure(lambda: client.get(legacy_url(course.id))) + minimal = measure(lambda: client.get(url(course.id), ALL)) + full = measure(lambda: client.get(url(course.id), {**FULL, **ALL})) + for new in (minimal, full): + assert new[0] <= legacy[0], (shape, new, legacy) + assert new[1] <= legacy[1], (shape, new, legacy) + assert new[2] == legacy[2] == 0, (shape, new, legacy) + + def test_the_sql_cost_stays_flat_and_mongo_reads_stay_within_v1s_as_configurations_are_added(self): + """ + One content group configuration and eight cost the same SQL queries, + on both views; at each size, neither view makes more Mongo reads than + v1 does for the same course. + """ + course = build_course(groups=True) + client = api_client(self.reader_for(course.id)) + + def costs(): + legacy = measure(lambda: client.get(legacy_url(course.id))) + new = [ + measure(lambda params=params: client.get(url(course.id), params)) for params in (ALL, {**FULL, **ALL}) + ] + for _, mongo_reads, mongo_writes in new: + assert mongo_reads <= legacy[1], (new, legacy) + assert mongo_writes == 0, (new, legacy) + return [sql for sql, _, _ in new] + + few = costs() + store_partitions(course.id, [content_groups(100 + index, f"Groups {index}") for index in range(8)]) + assert costs() == few + + +class GroupConfigurationsCostLegacyPermissionsTest(_ListingCostMixin, ModuleStoreTestCase): + """Costs for course staff, with policy-based authoring permissions off.""" + + def reader_for(self, course_key): + reader = UserFactory() + CourseStaffRole(course_key).add_users(reader) + return reader + + +class GroupConfigurationsCostAuthzTest(CourseAuthoringAuthzTestMixin, _ListingCostMixin, ModuleStoreTestCase): + """Costs for a course auditor, with policy-based authoring permissions on.""" + + def reader_for(self, course_key): + reader = UserFactory() + self.add_user_to_role_in_course(reader, COURSE_AUDITOR.external_key, course_key) + return reader + + +# --------------------------------------------------------------------------- +# Addresses and methods +# --------------------------------------------------------------------------- + +@ddt.ddt +class GroupConfigurationsRoutingTest(SharedModuleStoreTestCase): + """The address, the v1 address, and requests no route or method serves.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super().setUp() + self.client = api_client(UserFactory(is_staff=True)) + + def test_the_address(self): + assert url(COURSE_KEY) == f"/api/authoring/v2/courses/{COURSE_KEY}/group_configurations/" + match = resolve(url(COURSE_KEY)) + assert match.func.cls is CourseGroupConfigurationsViewSet + assert routed_actions(match.func) == {"get": "list"} + assert match.kwargs == {"course_key": CourseKey.from_string(COURSE_KEY)} + + def test_the_v1_address_is_unchanged(self): + assert legacy_url(COURSE_KEY) == f"/api/contentstore/v1/group_configurations/{COURSE_KEY}" + view = resolve(legacy_url(COURSE_KEY)).func.cls + assert (view.__module__, view.__name__) == (LEGACY_MODULE, "CourseGroupConfigurationsView") + + def test_v1_still_takes_a_slash_separated_key_that_v2_does_not(self): + view = resolve("/api/contentstore/v1/group_configurations/edX/DemoX/Demo_Course").func.cls + assert (view.__module__, view.__name__) == (LEGACY_MODULE, "CourseGroupConfigurationsView") + with pytest.raises(Resolver404): + resolve("/api/authoring/v2/courses/edX/DemoX/Demo_Course/group_configurations/") + + def test_the_url_names_are_unique_in_the_namespace(self): + # Imported here: at module import the URL converters are not registered yet. + from cms.djangoapps.contentstore.rest_api.v2 import authoring_urls # noqa: PLC0415 + names = [pattern.name for pattern in authoring_urls.urlpatterns] + assert len(names) == len(set(names)) + assert "course_group_configuration_list" in names + + # The error library does not catalogue MethodNotAllowed or NotAcceptable, so + # these answer with the envelope typed internal. Change the expected type + # here once it does. + + @ddt.data("post", "put", "patch", "delete") + def test_other_methods_are_not_allowed(self, method): + response = getattr(self.client, method)(url(self.course.id), {}, format="json") + assert_error_envelope(response, expected_status=405, expected_type_slug="internal") + + def test_unsupported_accept_is_refused_with_406(self): + response = self.client.get(url(self.course.id), HTTP_ACCEPT="application/xml") + assert_error_envelope(response, expected_status=406, expected_type_slug="internal") + + # These addresses match no route, so the service's HTML 404 page answers + # them. Change these to the error envelope once a catch-all route under the + # mount answers unknown addresses. + + @ddt.data("not-a-course-key", "edX/DemoX/Demo_Course", "edX+DemoX+Demo_Course") + def test_an_unparsable_or_slash_form_key_is_an_html_404(self, key): + response = self.client.get(f"/api/authoring/v2/courses/{key}/group_configurations/") + assert response.status_code == 404 + assert response["Content-Type"].startswith("text/html") + + def test_the_address_without_its_trailing_slash_redirects_to_it(self): + """No route answers the address without its slash; the service redirects a read to the slashed one.""" + address = f"/api/authoring/v2/courses/{self.course.id}/group_configurations" + response = self.client.get(address) + assert response.status_code == 301 + assert response["Location"] == url(self.course.id) + with pytest.raises(Resolver404): + resolve(address) + + +# --------------------------------------------------------------------------- +# The published schema +# --------------------------------------------------------------------------- + +class GroupConfigurationsSchemaTest(TestCase): + """The listing as published in the CMS authoring schema.""" + + PATH = "/api/authoring/v2/courses/{course_key}/group_configurations/" + LEGACY = "/api/contentstore/v1/group_configurations/{course_id}" + COMPONENTS = ( + "GroupConfiguration.v2", "GroupConfigurationGroup.v2", "GroupConfigurationUsage.v2", + "GroupConfigurationValidation.v2", + ) + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.schema = published_schema() + cls.components = cls.schema["components"]["schemas"] + + @staticmethod + def _ref(node): + return node["content"]["application/json"]["schema"]["$ref"].rsplit("/", 1)[-1] + + def _operation(self): + operations = self.schema["paths"][self.PATH] + assert sorted(operations) == ["get"] + return operations["get"] + + def test_the_listing_is_published_for_the_sdk(self): + operation = self._operation() + assert operation["operationId"] == "v2_courses_group_configurations_list" + assert operation["tags"] == ["openedx-platform-sdk"] + assert "deprecated" not in operation + assert sorted(operation["responses"]) == ["200", "400", "401", "403", "404"] + for status in ("400", "401", "403", "404"): + assert self._ref(operation["responses"][status]) == "ErrorResponse", status + + def test_the_listing_publishes_the_seven_envelope_keys(self): + name = self._ref(self._operation()["responses"]["200"]) + assert name == "PaginatedGroupConfiguration.v2List" + envelope = self.components[name] + assert list(envelope["properties"]) == list(ENVELOPE_KEYS) + assert envelope["required"] == list(ENVELOPE_KEYS) + assert envelope["properties"]["results"] == { + "type": "array", "items": {"$ref": "#/components/schemas/GroupConfiguration.v2"}, + } + + def test_the_parameters(self): + parameters = {p["name"]: p for p in self._operation()["parameters"]} + assert set(parameters) == {"course_key", "view", "page", "page_size"} + assert parameters["course_key"]["in"] == "path" + assert parameters["course_key"]["schema"] == {"type": "string", "pattern": "^[^/]+$"} + assert parameters["view"]["in"] == "query" + assert not parameters["view"].get("required") + assert parameters["view"]["schema"] == {"type": "string", "enum": ["full", "minimal"], "default": "minimal"} + for name in ("page", "page_size"): + assert (parameters[name]["in"], parameters[name]["schema"]) == ("query", {"type": "integer"}) + + def test_usage_validation_and_read_only_are_optional_and_usage_is_nullable(self): + configuration = self.components["GroupConfiguration.v2"] + assert set(configuration["required"]) == CONFIGURATION_MEMBERS + assert set(configuration["properties"]) == CONFIGURATION_MEMBERS | {"usage", "read_only"} + group = self.components["GroupConfigurationGroup.v2"] + assert set(group["required"]) == GROUP_MEMBERS + assert set(group["properties"]) == GROUP_MEMBERS | {"usage"} + for component in (configuration, group): + assert component["properties"]["usage"]["nullable"] is True + assert component["properties"]["usage"]["items"] == { + "$ref": "#/components/schemas/GroupConfigurationUsage.v2", + } + usage = self.components["GroupConfigurationUsage.v2"] + assert set(usage["required"]) == {"label", "url"} + assert usage["properties"]["validation"]["nullable"] is True + assert set(self.components["GroupConfigurationValidation.v2"]["required"]) == {"type", "text"} + + def test_the_description_leaves_out_the_course_flags_and_says_the_placeholder_id_changes(self): + description = self._operation()["description"] + assert ( + "Flags that describe the course rather than its group configurations, such as whether content " + "experiments are turned on, are not part of this resource." + ) in description + assert "whose id changes from request to request" in description + assert "/settings/" not in description + + def test_every_member_is_described(self): + for name in self.COMPONENTS: + component = self.components[name] + assert component.get("description"), name + assert [field for field, spec in component["properties"].items() if not spec.get("description")] == [], name + + def test_the_example_is_a_valid_page(self): + content = self._operation()["responses"]["200"]["content"]["application/json"] + page = content["examples"]["ContentGroups"]["value"] + assert set(page) == set(ENVELOPE_KEYS) + assert len(page["results"]) == 1 + assert_matches_published_schema(page, "PaginatedGroupConfiguration.v2List") + + def test_the_v1_operation_is_deprecated_and_left_out_of_the_sdk(self): + operations = self.schema["paths"][self.LEGACY] + assert sorted(operations) == ["get"] + assert operations["get"]["operationId"] == "v1_group_configurations_retrieve" + assert operations["get"]["deprecated"] is True + assert "openedx-platform-sdk" not in operations["get"].get("tags", []) + + def test_only_superseded_or_migrated_addresses_are_deprecated(self): + """No operation is marked deprecated except by the legacy-address lists, so a stray mark is caught.""" + deprecated = { + (path, method) + for path, operations in self.schema["paths"].items() + for method, operation in operations.items() + if isinstance(operation, dict) and operation.get("deprecated") + } + assert (self.LEGACY, "get") in deprecated + assert (self.PATH, "get") not in deprecated + stray = sorted( + (path, method) for path, method in deprecated + if path not in SUPERSEDED_PATHS and not path.startswith(LEGACY_MIGRATED_PATH_PREFIXES) + ) + assert stray == [] diff --git a/cms/djangoapps/contentstore/views/permissions.py b/cms/djangoapps/contentstore/views/permissions.py index 6450a3bfcc3e..bd0718d837ad 100644 --- a/cms/djangoapps/contentstore/views/permissions.py +++ b/cms/djangoapps/contentstore/views/permissions.py @@ -2,14 +2,26 @@ Custom permissions for the content store views. """ -from openedx_authz.constants.permissions import COURSES_VIEW_PAGES_AND_RESOURCES +from openedx_authz.constants.permissions import ( + COURSES_MANAGE_PAGES_AND_RESOURCES, + COURSES_VIEW_GROUP_CONFIGURATIONS, + COURSES_VIEW_PAGES_AND_RESOURCES, +) +from rest_framework.exceptions import NotFound from rest_framework.permissions import BasePermission -from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access +from common.djangoapps.student.auth import ( + check_course_advanced_settings_access, + has_studio_read_access, + has_studio_write_access, +) from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission from openedx.core.djangoapps.authz.decorators import user_has_course_permission +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api.view_utils import validate_course_key +COURSE_NOT_FOUND_MESSAGE = "The requested course does not exist." + class HasStudioWriteAccess(BasePermission): """ @@ -58,3 +70,102 @@ def has_permission(self, request, view): if course_key is None: return False return has_studio_read_access(request.user, course_key) + + +class IsExistingCourse(BasePermission): + """ + Answer 404 when the course in the path does not exist. + + Reads the parsed ``course_key`` URL argument. A key that names only a + content version parses, but no course run is behind it, so it is answered + as an unknown course. List this class after the authentication check, so + an anonymous caller learns nothing about which courses exist, and before + any class that checks roles on the course, so a signed-in caller is told + that a course is missing before being refused. + """ + + def has_permission(self, request, view): + course_key = view.kwargs.get("course_key") + if course_key is None: + return False + if course_key.org is None or not CourseOverview.course_exists(course_key): + raise NotFound(COURSE_NOT_FOUND_MESSAGE) + return True + + +class CanViewAdvancedSettings(BasePermission): + """ + Allow callers who may view the advanced settings of the course in the path. + + Reads the parsed ``course_key`` URL argument. On courses where + policy-based authoring permissions are enabled, only that policy is + consulted; elsewhere the caller needs Studio read access to the course. + Advanced settings being restricted to staff site-wide does not apply here: + course teams also change page and resource toggles through these settings. + """ + + def has_permission(self, request, view): + course_key = view.kwargs.get("course_key") + if course_key is None: + return False + return check_course_advanced_settings_access(request.user, course_key, access_type="read") + + +class CanManageAdvancedSettings(BasePermission): + """ + Allow callers who may change the advanced settings of the course in the path. + + Reads the parsed ``course_key`` URL argument. On courses where + policy-based authoring permissions are enabled, only that policy is + consulted; elsewhere the caller needs Studio write access to the course. + Advanced settings being restricted to staff site-wide does not apply here, + as for ``CanViewAdvancedSettings``. + """ + + def has_permission(self, request, view): + course_key = view.kwargs.get("course_key") + if course_key is None: + return False + return check_course_advanced_settings_access(request.user, course_key, access_type="write") + + +class CanManagePagesAndResources(BasePermission): + """ + Allow callers who may change the pages and resources of the course in the path. + + Reads the parsed ``course_key`` URL argument. On courses where + policy-based authoring permissions are enabled, only that policy is + consulted; elsewhere the caller needs Studio write access to the course. + """ + + def has_permission(self, request, view): + course_key = view.kwargs.get("course_key") + if course_key is None: + return False + return user_has_course_permission( + request.user, + COURSES_MANAGE_PAGES_AND_RESOURCES.identifier, + course_key, + LegacyAuthoringPermission.WRITE, + ) + + +class CanViewGroupConfigurations(BasePermission): + """ + Allow callers who may view the group configurations of the course in the path. + + Reads the parsed ``course_key`` URL argument. On courses where + policy-based authoring permissions are enabled, only that policy is + consulted; elsewhere the caller needs Studio read access to the course. + """ + + def has_permission(self, request, view): + course_key = view.kwargs.get("course_key") + if course_key is None: + return False + return user_has_course_permission( + request.user, + COURSES_VIEW_GROUP_CONFIGURATIONS.identifier, + course_key, + LegacyAuthoringPermission.READ, + ) diff --git a/cms/lib/spectacular.py b/cms/lib/spectacular.py index 662cf80c65f0..83794a0340fc 100644 --- a/cms/lib/spectacular.py +++ b/cms/lib/spectacular.py @@ -4,15 +4,25 @@ from drf_spectacular.openapi import AutoSchema -# Legacy operations superseded by a conforming address under /api/authoring/. -# Each is listed in the truncated spelling as well as the full one: the routes -# make the final character optional, so both addresses resolve, and which one -# the generator emits depends on how it renders that regex. +# Legacy operations superseded by a conforming address under /api/authoring/, +# matched on the exact path. A route that makes its final character optional +# resolves with and without it, and the generator may emit either spelling, so +# such a route is listed in both. SUPERSEDED_PATHS = ( "/api/contentstore/v0/youtube_transcripts/{course_id}/chec", "/api/contentstore/v0/youtube_transcripts/{course_id}/check", "/api/contentstore/v0/youtube_transcripts/{course_id}/uploa", "/api/contentstore/v0/youtube_transcripts/{course_id}/upload", + # → /api/authoring/v1/courses/{course_key}/advanced_settings/ + "/api/contentstore/v0/advanced_settings/{course_id}", + # → /api/authoring/v1/custom_pages/?course_key={course_key} + "/api/contentstore/v0/tabs/{course_id}", + # → /api/authoring/v1/courses/{course_key}/custom_page_order/ + "/api/contentstore/v0/tabs/{course_id}/reorder", + # → /api/authoring/v1/courses/{course_key}/tabs/{tab_id}/ + "/api/contentstore/v0/tabs/{course_id}/settings", + # → /api/authoring/v2/courses/{course_key}/group_configurations/ + "/api/contentstore/v1/group_configurations/{course_id}", ) # Legacy addresses of APIs migrated to /api/authoring/, marked deprecated for diff --git a/cms/lib/tests/test_spectacular.py b/cms/lib/tests/test_spectacular.py index 4581f23a6148..41f078d1af9d 100644 --- a/cms/lib/tests/test_spectacular.py +++ b/cms/lib/tests/test_spectacular.py @@ -76,6 +76,11 @@ def test_both_spellings_of_each_superseded_path_are_listed(self): "/api/contentstore/v0/youtube_transcripts/{course_id}/check", "/api/contentstore/v0/youtube_transcripts/{course_id}/uploa", "/api/contentstore/v0/youtube_transcripts/{course_id}/upload", + "/api/contentstore/v0/advanced_settings/{course_id}", + "/api/contentstore/v0/tabs/{course_id}", + "/api/contentstore/v0/tabs/{course_id}/reorder", + "/api/contentstore/v0/tabs/{course_id}/settings", + "/api/contentstore/v1/group_configurations/{course_id}", } def test_the_conforming_paths_are_left_unmarked(self): @@ -83,8 +88,45 @@ def test_the_conforming_paths_are_left_unmarked(self): result = self._run({"paths": {conforming: {"get": {"operationId": "x"}}}}) assert result["paths"][conforming]["get"] == {"operationId": "x"} + def test_the_advanced_settings_operations_are_marked_and_their_successor_is_not(self): + legacy = "/api/contentstore/v0/advanced_settings/{course_id}" + conforming = "/api/authoring/v1/courses/{course_key}/advanced_settings/" + result = self._run({"paths": { + legacy: {"get": {}, "patch": {}}, + conforming: {"get": {}, "patch": {}}, + }}) + assert result["paths"][legacy] == {"get": {"deprecated": True}, "patch": {"deprecated": True}} + assert result["paths"][conforming] == {"get": {}, "patch": {}} + + def test_the_tab_operations_are_marked_and_their_successors_are_not(self): + legacy = { + "/api/contentstore/v0/tabs/{course_id}": "get", + "/api/contentstore/v0/tabs/{course_id}/reorder": "post", + "/api/contentstore/v0/tabs/{course_id}/settings": "post", + } + conforming = { + "/api/authoring/v1/custom_pages/": "get", + "/api/authoring/v1/courses/{course_key}/custom_page_order/": "put", + "/api/authoring/v1/courses/{course_key}/tabs/{tab_id}/": "patch", + } + paths = {path: {method: {}} for path, method in {**legacy, **conforming}.items()} + result = self._run({"paths": paths}) + assert {path: result["paths"][path] for path in legacy} == { + path: {method: {"deprecated": True}} for path, method in legacy.items() + } + assert {path: result["paths"][path] for path in conforming} == { + path: {method: {}} for path, method in conforming.items() + } + + def test_the_group_configurations_operation_is_marked_and_its_successor_is_not(self): + legacy = "/api/contentstore/v1/group_configurations/{course_id}" + conforming = "/api/authoring/v2/courses/{course_key}/group_configurations/" + result = self._run({"paths": {legacy: {"get": {}}, conforming: {"get": {}}}}) + assert result["paths"][legacy] == {"get": {"deprecated": True}} + assert result["paths"][conforming] == {"get": {}} + def test_an_unrelated_contentstore_path_is_left_unmarked(self): - other = "/api/contentstore/v0/advanced_settings/{course_id}" + other = "/api/contentstore/v0/heartbeat" result = self._run({"paths": {other: {"get": {}}}}) assert "deprecated" not in result["paths"][other]["get"]