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/course_team.py b/cms/djangoapps/contentstore/rest_api/v1/views/course_team.py index ef96ade1cf35..3afc71ce6553 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/course_team.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/course_team.py @@ -18,6 +18,10 @@ class CourseTeamView(DeveloperErrorViewMixin, APIView): """ View for getting data for course team. + + Deprecated: use ``GET /api/authoring/v2/courses/{course_key}/team_members/`` and + ``GET /api/authoring/v2/courses/{course_key}/team_members/me/`` instead. + See https://github.com/openedx/openedx-platform/issues/39128. """ @extend_schema( parameters=[ @@ -35,6 +39,13 @@ def get(self, request: Request, course_id: str): """ Get all CMS users who are editors for the specified course. + Deprecated: use ``GET /api/authoring/v2/courses/{course_key}/team_members/``, which + returns the same users, paginated (``page``, ``page_size`` up to 100), instructors first + and then staff, each by username. ``allow_actions`` and ``show_transfer_ownership_hint`` + are ``can_manage_team`` and ``is_only_instructor`` of + ``GET /api/authoring/v2/courses/{course_key}/team_members/me/``. + See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** GET /api/contentstore/v1/course_team/{course_id} 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/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..806757ce304a 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.team_members import CourseTeamMembersViewSet 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,14 @@ CourseVideoSettingsViewSet.as_view({"get": "retrieve"}), name="course_video_settings", ), + path( + "courses//team_members/", + CourseTeamMembersViewSet.as_view({"get": "list"}), + name="course_team_member_list", + ), + path( + "courses//team_members/me/", + CourseTeamMembersViewSet.as_view({"get": "retrieve"}), + name="course_team_member_me", + ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/team_members.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/team_members.py new file mode 100644 index 000000000000..30ae53875aba --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/team_members.py @@ -0,0 +1,54 @@ +"""Serializers for the members of a course's team.""" + +from rest_framework import serializers + +INSTRUCTOR_ROLE = "instructor" +STAFF_ROLE = "staff" +# The roles a member may hold, in the order members are listed. +TEAM_ROLES = (INSTRUCTOR_ROLE, STAFF_ROLE) + + +class CourseTeamMemberSerializer(serializers.Serializer): # pylint: disable=abstract-method + """One member of a course's team and the role they hold on it.""" + + id = serializers.IntegerField(help_text="Numeric identifier of the member's user account.") + username = serializers.CharField(help_text="Username of the member.") + email = serializers.CharField(help_text="Email address of the member.") + role = serializers.ChoiceField( + choices=TEAM_ROLES, + help_text="The member's role: instructor, or staff for members who are not instructors.", + ) + + class Meta: + ref_name = "CourseTeamMember.v2" + + +class CourseTeamCallerSerializer(serializers.Serializer): # pylint: disable=abstract-method + """The caller as the course's team sees them, and whether they may change the team.""" + + username = serializers.CharField(help_text="Username of the caller.") + email = serializers.CharField(help_text="Email address of the caller.") + role = serializers.ChoiceField( + choices=TEAM_ROLES, + allow_null=True, + help_text=( + "The caller's role on the team, or null when the caller is not a member, as for global staff " + "who hold no role on the course." + ), + ) + can_manage_team = serializers.BooleanField( + help_text=( + "Whether the caller may add members to the team, and remove other members or change their roles. " + "A caller without it may still remove themself from the team, or move themself from instructor " + "to staff." + ), + ) + is_only_instructor = serializers.BooleanField( + help_text=( + "Whether the caller is the course's only instructor. The only instructor cannot be removed " + "or made staff until another instructor is added." + ), + ) + + class Meta: + ref_name = "CourseTeamCaller.v2" diff --git a/cms/djangoapps/contentstore/rest_api/v2/team_members_service.py b/cms/djangoapps/contentstore/rest_api/v2/team_members_service.py new file mode 100644 index 000000000000..c61bae0b957c --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/team_members_service.py @@ -0,0 +1,41 @@ +"""Service layer for the v2 course team members endpoints.""" + +from cms.djangoapps.contentstore.rest_api.v2.serializers.team_members import TEAM_ROLES +from cms.djangoapps.contentstore.utils import get_course_team +from common.djangoapps.student.auth import get_user_permissions + + +def _course_team(course_key, user): + """Return Studio's course team context for ``user`` on the course ``course_key``.""" + return get_course_team(user, course_key, get_user_permissions(user, course_key)) + + +def list_team_members(course_key, user): + """ + Return the members of the team of the course ``course_key``, as seen by ``user``. + + Instructors come first, then the staff who are not instructors, each in + order of username. A user holding both roles is listed once, as an + instructor. + """ + members = _course_team(course_key, user)["users"] + return sorted(members, key=lambda member: (TEAM_ROLES.index(member["role"]), member["username"])) + + +def describe_caller(course_key, user): + """ + Return ``user`` as the team of the course ``course_key`` sees them. + + ``role`` is the role under which the user is listed among the members, or + None when they are not listed; ``can_manage_team`` and + ``is_only_instructor`` are Studio's own answers for the course. + """ + team = _course_team(course_key, user) + role = next((member["role"] for member in team["users"] if member["id"] == user.id), None) + return { + "username": user.username, + "email": user.email, + "role": role, + "can_manage_team": team["allow_actions"], + "is_only_instructor": team["show_transfer_ownership_hint"], + } diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/team_members.py b/cms/djangoapps/contentstore/rest_api/v2/views/team_members.py new file mode 100644 index 000000000000..d44d27911a89 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/team_members.py @@ -0,0 +1,136 @@ +"""API views for the members of a course's team.""" + +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 rest_framework.response import Response + +from cms.djangoapps.contentstore.rest_api.pagination import DocumentedDefaultPagination +from cms.djangoapps.contentstore.rest_api.v2.serializers.team_members import ( + CourseTeamCallerSerializer, + CourseTeamMemberSerializer, +) +from cms.djangoapps.contentstore.rest_api.v2.team_members_service import describe_caller, list_team_members +from cms.djangoapps.contentstore.views.permissions import CanViewCourseTeam, 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 _error(description): + return OpenApiResponse(response=ErrorResponseSerializer, description=description) + + +_MEMBER_EXAMPLE = OpenApiExample( + "Team member", + summary="One member of a course's team", + response_only=True, + status_codes=["200"], + value={"id": 3, "username": "edx", "email": "edx@example.com", "role": "instructor"}, +) + +_CALLER_EXAMPLE = OpenApiExample( + "Only instructor", + summary="A caller who is the course's only instructor", + response_only=True, + status_codes=["200"], + value={ + "username": "edx", + "email": "edx@example.com", + "role": "instructor", + "can_manage_team": True, + "is_only_instructor": True, + }, +) + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CourseTeamMembersViewSet(StandardizedErrorMixin, IterablePaginationMixin, viewsets.ViewSet): + """ + The team of one course: the users who hold the instructor or staff role on + it, and the caller as the team sees them (``me``). + + Members are listed instructors first, then staff, each in order of + username. ``me`` says which role the caller holds, if any, whether they may + change the team, and whether they are its only instructor. + + Anyone who may see the course's team may call both. 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 team is read through Studio's course role helpers, + not from a queryset this view owns. + """ + + # An inactive account's browser session stays authenticated, so the course permission decides: + # inactive global staff are admitted, inactive course staff are refused (403, not 401). + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated, IsExistingCourse, CanViewCourseTeam) + # 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", "retrieve", "metadata") + serializer_class = CourseTeamMemberSerializer + 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 team member serializer declared on this viewset.""" + return self.serializer_class(*args, **kwargs) + + @extend_schema( + summary="List the members of a course's team", + description=( + "Returns one page of the users who hold the instructor or staff role on the course: instructors " + "first, then staff, each in order of username. A user who holds both roles is listed once, as " + "an instructor. Global staff are not listed unless they hold a role on the course." + ), + parameters=[_COURSE_KEY_PARAMETER], + responses={ + 200: OpenApiResponse(response=CourseTeamMemberSerializer, description="A page of the team's members."), + 401: _error("The requester is not authenticated."), + 403: _error("The requester may not see this course's team."), + 404: _error("The course does not exist, or the requested page is out of range."), + }, + examples=[_MEMBER_EXAMPLE], + ) + def list(self, request, course_key): + """Return one page of the course's team members.""" + return self.paginate_iterable(request, list_team_members(course_key, request.user)) + + @extend_schema( + summary="Get the caller's place on a course's team", + description=( + "Returns the caller's username and email, the role they hold on the course's team (null when they " + "hold none, as for global staff), whether they may add, remove and change the roles of members, " + "and whether they are the course's only instructor." + ), + parameters=[_COURSE_KEY_PARAMETER], + responses={ + 200: OpenApiResponse(response=CourseTeamCallerSerializer, description="The caller, as the team sees them."), + 401: _error("The requester is not authenticated."), + 403: _error("The requester may not see this course's team."), + 404: _error("The course does not exist."), + }, + examples=[_CALLER_EXAMPLE], + ) + def retrieve(self, request, course_key): + """Return the caller as the course's team sees them.""" + return Response(CourseTeamCallerSerializer(describe_caller(course_key, request.user)).data) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_team_members.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_team_members.py new file mode 100644 index 000000000000..1ef42d6b75da --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_team_members.py @@ -0,0 +1,1275 @@ +"""Tests for the v2 course team members API.""" + +import json +from types import SimpleNamespace +from unittest.mock import patch + +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_rest_framework_extensions.url_converters import CourseKeyConverter +from opaque_keys.edx.keys import CourseKey +from openedx_authz.api.users import assign_role_to_user_in_scope +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.engine.enforcer import AuthzEnforcer +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, DocumentedDefaultPagination +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.team_members import CourseTeamMembersViewSet +from cms.djangoapps.contentstore.rest_api.v2.views.tests.helpers import resolve_ref +from cms.djangoapps.contentstore.tests.utils import routed_actions +from cms.djangoapps.contentstore.views.permissions import CanViewCourseTeam, IsExistingCourse +from cms.lib.spectacular import LEGACY_MIGRATED_PATH_PREFIXES, SUPERSEDED_PATHS +from common.djangoapps.student.auth import ( + STUDIO_EDIT_CONTENT, + STUDIO_EDIT_ROLES, + STUDIO_NO_PERMISSIONS, + STUDIO_VIEW_CONTENT, + STUDIO_VIEW_USERS, +) +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 import toggles as core_toggles +from openedx.core.djangoapps.authz.tests.fixtures import seed_policies +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.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory + +LIST_URL_NAME = "authoring_v2:course_team_member_list" +ME_URL_NAME = "authoring_v2:course_team_member_me" +LEGACY_URL_NAME = "cms.djangoapps.contentstore:v1:course_team" +LEGACY_MODULE = "cms.djangoapps.contentstore.rest_api.v1.views.course_team" +COURSE_KEY = "course-v1:edX+DemoX+Demo_Course" +ACTIONS = ("list", "me") +# The roles in the order members are listed. +LISTED_ORDER = ("instructor", "staff") +MEMBER_MEMBERS = {"id", "username", "email", "role"} +CALLER_MEMBERS = {"username", "email", "role", "can_manage_team", "is_only_instructor"} +OK = 200 + + +def list_url(course_key): + """Return the address of the members of the team of ``course_key``.""" + return reverse(LIST_URL_NAME, kwargs={"course_key": str(course_key)}) + + +def me_url(course_key): + """Return the address of the caller's place on the team of ``course_key``.""" + return reverse(ME_URL_NAME, kwargs={"course_key": str(course_key)}) + + +def legacy_url(course_key): + """Return the deprecated course team address of ``course_key``.""" + return reverse(LEGACY_URL_NAME, kwargs={"course_id": str(course_key)}) + + +def in_listed_order(members): + """``members`` in the order the team is listed: instructors, then staff, each by username.""" + return sorted(members, key=lambda member: (LISTED_ORDER.index(member["role"]), member["username"])) + + +def caller_from_legacy(legacy_body, user): + """The ``me`` body the legacy course team body implies for ``user``.""" + role = next((member["role"] for member in legacy_body["users"] if member["id"] == user.id), None) + return { + "username": user.username, + "email": user.email, + "role": role, + "can_manage_team": legacy_body["allow_actions"], + "is_only_instructor": legacy_body["show_transfer_ownership_hint"], + } + + +def member(user, role): + """The listing entry of ``user`` holding ``role``.""" + return {"id": user.id, "username": user.username, "email": user.email, "role": role} + + +def legacy_team(course_key, role): + """Add a new user to the team of ``course_key`` through Studio's course roles; ``role`` is the role class.""" + user = UserFactory() + role(course_key).add_users(user) + return user + + +# --------------------------------------------------------------------------- +# Who may call the endpoints +# --------------------------------------------------------------------------- + +class _AccessMixin: + """ + Checks a caller's status on each action against the expected one, and + that the legacy endpoint decides the same for the same caller. On success + the listing holds what the legacy endpoint holds, in listed order, and + ``me`` says what the legacy body says about the 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 the legacy endpoint.""" + course_key = course_key or self.course.id + client = api_client(user) + response = client.get(list_url(course_key), {"page_size": 100}) + legacy = client.get(legacy_url(course_key)) + assert response.status_code == expected, response.content + assert legacy.status_code == expected, f"the legacy endpoint decided {legacy.status_code} for the same caller" + if expected == OK: + results = response.json()["results"] + assert results == in_listed_order(legacy.json()["users"]) + assert {entry["username"] for entry in results} >= {self.instructor.username, self.staff.username} + else: + assert_refused(response, expected) + + def assert_me(self, user, expected, course_key=None): + """Assert the status of ``me`` for ``user``, its body or envelope, and agreement with the legacy endpoint.""" + course_key = course_key or self.course.id + client = api_client(user) + response = client.get(me_url(course_key)) + legacy = client.get(legacy_url(course_key)) + assert response.status_code == expected, response.content + assert legacy.status_code == expected, f"the legacy endpoint decided {legacy.status_code} for the same caller" + if expected == OK: + assert response.json() == caller_from_legacy(legacy.json(), user) + else: + assert_refused(response, expected) + + def assert_access(self, user, action, expected, course_key=None): + """Assert ``expected`` for ``action`` (``list`` or ``me``) by ``user``.""" + getattr(self, f"assert_{action}")(user, expected, course_key) + + def me(self, user): + """Return the ``me`` body for ``user`` on the course.""" + response = api_client(user).get(me_url(self.course.id)) + assert response.status_code == 200, response.content + return response.json() + + +@ddt.ddt +class TeamMembersAccessLegacyPermissionsTest(_AccessMixin, SharedModuleStoreTestCase): + """Who may see a course's team when policy-based authoring permissions are off.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + cls.other_course = CourseFactory.create() + + def setUp(self): + super().setUp() + self.instructor = legacy_team(self.course.id, CourseInstructorRole) + self.staff = legacy_team(self.course.id, CourseStaffRole) + + def _user_with(self, role): + user = UserFactory() + role.add_users(user) + return user + + def _user_with_policy_role(self, role): + """Return a new user holding ``role`` on the course through policy-based permissions only.""" + seed_policies() + self.addCleanup(AuthzEnforcer.get_enforcer().clear_policy) + user = UserFactory() + assign_role_to_user_in_scope(user.username, role, str(self.course.id)) + AuthzEnforcer.get_enforcer().load_policy() + assert not CourseAccessRole.objects.filter(user=user).exists() + return user + + @staticmethod + def _policy_permissions_off(): + return patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, "is_enabled", return_value=False) + + @ddt.data(*ACTIONS) + def test_anonymous_caller_is_refused_with_401(self, action): + self.assert_access(None, action, 401) + + @ddt.data(*ACTIONS) + def test_a_policy_course_staff_role_opens_the_team_without_listing_its_holder(self, action): + """ + A course staff role assigned through policy-based permissions counts as + Studio's staff role even while those permissions are off for the + course, as on the legacy endpoint. Its holder is not listed, because + the team is then read from Studio's own role records. + """ + user = self._user_with_policy_role(COURSE_STAFF.external_key) + with self._policy_permissions_off(): + self.assert_access(user, action, OK) + listed = api_client(user).get(list_url(self.course.id)).json()["results"] + assert listed == [member(self.instructor, "instructor"), member(self.staff, "staff")] + assert self.me(user) == { + "username": user.username, "email": user.email, "role": None, + "can_manage_team": False, "is_only_instructor": False, + } + + @ddt.data(*[(action, role) for action in ACTIONS for role in ( + COURSE_EDITOR.external_key, + COURSE_AUDITOR.external_key, + )]) + @ddt.unpack + def test_a_policy_role_without_a_studio_course_role_is_refused(self, action, role): + """The policy lets these roles view the team, but they stand for no Studio course role.""" + user = self._user_with_policy_role(role) + with self._policy_permissions_off(): + self.assert_access(user, action, 403) + + @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.staff, action, OK) + + @ddt.data(*ACTIONS) + def test_course_instructor_is_allowed(self, action): + self.assert_access(self.instructor, 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_is_allowed_as_global_staff(self, action): + """Saving a superuser makes them global staff, so one created without staff status still has it.""" + user = UserFactory(is_superuser=True, is_staff=False) + assert user.is_staff is True + self.assert_access(user, action, OK) + + def test_course_staff_may_see_but_not_manage_the_team(self): + assert self.me(self.staff) == { + "username": self.staff.username, + "email": self.staff.email, + "role": "staff", + "can_manage_team": False, + "is_only_instructor": False, + } + + def test_the_only_instructor_may_manage_the_team_and_is_told_so(self): + assert self.me(self.instructor) == { + "username": self.instructor.username, + "email": self.instructor.email, + "role": "instructor", + "can_manage_team": True, + "is_only_instructor": True, + } + + def test_one_of_two_instructors_is_not_the_only_one(self): + second = legacy_team(self.course.id, CourseInstructorRole) + for instructor in (self.instructor, second): + body = self.me(instructor) + assert (body["role"], body["can_manage_team"], body["is_only_instructor"]) == ("instructor", True, False) + + def test_global_staff_without_a_role_may_manage_the_team_and_holds_no_role_on_it(self): + user = UserFactory(is_staff=True) + assert self.me(user) == { + "username": user.username, "email": user.email, "role": None, + "can_manage_team": True, "is_only_instructor": False, + } + + def test_org_roles_are_not_team_roles(self): + """Org staff and instructors may see the team without being listed on it; only instructors may manage it.""" + for role, can_manage in ((OrgStaffRole, False), (OrgInstructorRole, True)): + user = self._user_with(role(self.course.id.org)) + body = self.me(user) + assert (body["role"], body["can_manage_team"], body["is_only_instructor"]) == (None, can_manage, False) + + def test_a_version_only_course_key_is_404(self): + """The key parses but names no course run; the legacy endpoint 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).get(me_url(VERSION_ONLY_KEY)), 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).get(me_url(library_key)), 404) + + +@ddt.ddt +class TeamMembersAccessAuthzTest(CourseAuthoringAuthzTestMixin, _AccessMixin, SharedModuleStoreTestCase): + """ + Who may see a course's team when policy-based authoring permissions are + on. The team is then the holders of the course admin and course staff + policy roles, and those roles count as the instructor and staff course + roles; no other policy role or permission opens the team. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + cls.other_course = CourseFactory.create() + + def setUp(self): + super().setUp() + self.instructor = self._user_with(COURSE_ADMIN.external_key) + self.staff = self._user_with(COURSE_STAFF.external_key) + + 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(*ACTIONS) + def test_the_course_admin_and_course_staff_roles_are_allowed(self, action): + self.assert_access(self.instructor, action, OK) + self.assert_access(self.staff, action, OK) + + def test_the_course_admin_role_is_listed_as_instructor_and_may_manage_the_team(self): + body = self.me(self.instructor) + assert (body["role"], body["can_manage_team"], body["is_only_instructor"]) == ("instructor", True, True) + body = self.me(self.staff) + assert (body["role"], body["can_manage_team"], body["is_only_instructor"]) == ("staff", False, False) + listed = api_client(self.staff).get(list_url(self.course.id)).json()["results"] + assert listed == [member(self.instructor, "instructor"), member(self.staff, "staff")] + + @ddt.data(*[(action, role) for action in ACTIONS for role in ( + COURSE_AUDITOR.external_key, + COURSE_EDITOR.external_key, + 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): + self.assert_access(self._user_with(COURSE_ADMIN.external_key, self.other_course.id), action, 403) + + @ddt.data(*ACTIONS) + def test_a_studio_staff_role_still_opens_the_team_without_listing_its_holder(self, action): + """Studio's own staff role still counts, as on the legacy endpoint, but only policy roles are listed.""" + user = UserFactory() + CourseAccessRole.objects.create(user=user, role="staff", course_id=self.course.id, org=self.course.id.org) + self.assert_access(user, action, OK) + listed = api_client(user).get(list_url(self.course.id)).json()["results"] + assert user.username not in {entry["username"] for entry in listed} + assert self.me(user)["role"] is None + + @ddt.data(*ACTIONS) + def test_staff_and_superuser_are_allowed_without_a_role(self, action): + """Global staff need no role; a superuser counts as global staff because saving one sets staff status.""" + assert self.super_user.is_staff is True + 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_refused(self): + """A course role does not count while the account is inactive, on v2 as on the legacy endpoint.""" + client = session_client(self.staff) + self.staff.is_active = False + self.staff.save() + assert_refused(client.get(list_url(self.course.id)), 403) + assert_refused(client.get(me_url(self.course.id)), 403) + assert client.get(legacy_url(self.course.id)).status_code == 403 + + +class TeamMembersPermissionWiringTest(SimpleTestCase): + """Which permissions the viewset checks, and what the team 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_both_actions_check_the_course_then_the_permission_to_see_the_team(self): + view = CourseTeamMembersViewSet() + for action in ("list", "retrieve", None, "metadata"): + view.action = action + assert self._declared(view) == [IsAuthenticated, IsExistingCourse, CanViewCourseTeam], action + + def test_any_other_action_is_refused(self): + view = CourseTeamMembersViewSet() + for action in ("create", "update", "partial_update", "destroy", "anything_else"): + 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(CourseTeamMembersViewSet): + permission_classes = CourseTeamMembersViewSet.permission_classes + + def destroy(self, request, **kwargs): # pylint: disable=unused-argument + raise AssertionError("the handler ran without a permission check") + + request = APIRequestFactory().delete(f"/api/authoring/v2/courses/{COURSE_KEY}/team_members/me/") + 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_team_permission_needs_studios_permission_to_see_users(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.get_user_permissions" + for bits, decision in ( + (STUDIO_EDIT_ROLES | STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT, True), + (STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT, True), + (STUDIO_VIEW_USERS | STUDIO_VIEW_CONTENT, True), + (STUDIO_EDIT_ROLES | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT, False), + (STUDIO_EDIT_CONTENT, False), + (STUDIO_NO_PERMISSIONS, False), + ): + with patch(target, return_value=bits) as check: + assert CanViewCourseTeam().has_permission(request, view) is decision, bits + check.assert_called_once_with(request.user, course_key) + + def test_the_team_permission_refuses_a_view_without_a_course(self): + target = "cms.djangoapps.contentstore.views.permissions.get_user_permissions" + request, view = SimpleNamespace(user=object()), SimpleNamespace(kwargs={}) + with patch(target, return_value=STUDIO_VIEW_USERS) as check: + assert CanViewCourseTeam().has_permission(request, view) is False + check.assert_not_called() + + +class TeamMembersAuthenticationTest(SharedModuleStoreTestCase): + """How callers authenticate, with policy-based authoring permissions off.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def _course_staff(self): + return legacy_team(self.course.id, CourseStaffRole) + + def _assert_both_actions_succeed(self, client, user, **headers): + """Assert that ``client`` lists ``user`` as the only staff member, and that ``me`` names them as staff.""" + listed = client.get(list_url(self.course.id), **headers) + assert listed.status_code == 200, listed.content + assert listed.json()["results"] == [member(user, "staff")] + me = client.get(me_url(self.course.id), **headers) + assert me.status_code == 200, me.content + assert (me.json()["username"], me.json()["role"]) == (user.username, "staff") + + def test_a_jwt_caller_may_list_and_see_themself(self): + user = self._course_staff() + header = {"HTTP_AUTHORIZATION": f"JWT {create_jwt_for_user(user)}"} + self._assert_both_actions_succeed(APIClient(), user, **header) + + def test_a_session_caller_may_list_and_see_themself(self): + user = self._course_staff() + self._assert_both_actions_succeed(session_client(user), user) + + def test_an_inactive_session_is_authenticated(self): + """ + A session whose account is not activated is accepted, as on the legacy + endpoint. 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() + for address in (list_url(self.course.id), me_url(self.course.id)): + response = client.get(address) + 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 the legacy endpoint 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().get(me_url(self.course.id), **header), 401) + assert APIClient().get(legacy_url(self.course.id), **header).status_code == 200 + + +# --------------------------------------------------------------------------- +# Listing +# --------------------------------------------------------------------------- + +class TeamMembersListTest(ModuleStoreTestCase): + """What a listing returns and how it pages.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.caller = UserFactory(is_staff=True) + self.client = api_client(self.caller) + + def _add(self, username, *roles): + user = UserFactory(username=username) + for role in roles: + role(self.course.id).add_users(user) + return user + + def get(self, course_key=None, **params): + response = self.client.get(list_url(course_key or self.course.id), params) + assert response.status_code == 200, response.content + return response.json() + + def test_instructors_come_first_then_staff_each_by_username(self): + zed = self._add("zed_instructor", CourseInstructorRole) + amy = self._add("amy_instructor", CourseInstructorRole) + both = self._add("carl_both", CourseStaffRole, CourseInstructorRole) + bob = self._add("bob_staff", CourseStaffRole) + aaron = self._add("aaron_staff", CourseStaffRole) + assert self.get()["results"] == [ + member(amy, "instructor"), + member(both, "instructor"), + member(zed, "instructor"), + member(aaron, "staff"), + member(bob, "staff"), + ] + + def test_each_member_carries_exactly_its_members(self): + user = self._add("only_member", CourseStaffRole) + (entry,) = self.get()["results"] + assert set(entry) == MEMBER_MEMBERS + assert entry == {"id": user.id, "username": "only_member", "email": user.email, "role": "staff"} + + def test_a_member_whose_account_is_inactive_is_still_listed(self): + user = self._add("dormant_staff", CourseStaffRole) + user.is_active = False + user.save() + assert self.get()["results"] == [member(user, "staff")] + + def test_global_staff_without_a_role_are_not_listed(self): + staff = self._add("listed_staff", CourseStaffRole) + assert self.caller.is_staff + assert self.get()["results"] == [member(staff, "staff")] + + def test_members_of_another_course_are_not_listed(self): + other = CourseFactory.create() + outsider = UserFactory() + CourseInstructorRole(other.id).add_users(outsider) + staff = self._add("this_course_staff", CourseStaffRole) + assert self.get()["results"] == [member(staff, "staff")] + + def test_the_envelope_has_the_seven_keys(self): + self._add("enveloped", CourseStaffRole) + body = self.get() + assert set(body) == set(ENVELOPE_KEYS) + assert (body["count"], body["num_pages"], body["current_page"], body["start"]) == (1, 1, 1, 0) + assert (body["next"], body["previous"]) == (None, None) + + def test_page_size_splits_the_pages(self): + users = [self._add(f"member_{index:02d}", CourseStaffRole) for index in range(12)] + first, second = self.get(page_size=5), self.get(page_size=5, page=3) + assert (first["count"], first["num_pages"], len(first["results"])) == (12, 3, 5) + assert [entry["username"] for entry in first["results"]] == [user.username for user in users[:5]] + assert (second["current_page"], second["start"]) == (3, 10) + assert [entry["username"] for entry in second["results"]] == [user.username for user in users[10:]] + assert second["next"] is None + + def test_the_default_page_size_is_ten(self): + for index in range(12): + self._add(f"member_{index:02d}", CourseStaffRole) + body = self.get() + assert (body["count"], len(body["results"]), body["num_pages"]) == (12, 10, 2) + + def test_the_page_size_is_at_most_one_hundred(self): + users = UserFactory.create_batch(101) + CourseStaffRole(self.course.id).add_users(*users) + body = self.get(page_size=500) + assert (body["count"], len(body["results"]), body["num_pages"]) == (101, 100, 2) + + def test_a_page_beyond_the_last_is_404(self): + self._add("single", CourseStaffRole) + response = self.client.get(list_url(self.course.id), {"page": 2}) + assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_a_course_without_a_team_lists_no_one(self): + body = self.get() + assert (body["count"], body["results"]) == (0, []) + + def test_the_body_matches_the_published_schema(self): + self._add("schema_instructor", CourseInstructorRole) + self._add("schema_staff", CourseStaffRole) + assert_matches_published_schema(self.get(), "PaginatedCourseTeamMember.v2List") + + def test_head_answers_without_a_body(self): + response = self.client.head(list_url(self.course.id)) + assert (response.status_code, response.content) == (200, b"") + + +class TeamMembersMeTest(ModuleStoreTestCase): + """What ``me`` returns.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + + def me(self, user): + response = api_client(user).get(me_url(self.course.id)) + assert response.status_code == 200, response.content + return response.json() + + def test_a_user_holding_both_roles_is_an_instructor(self): + user = UserFactory() + CourseStaffRole(self.course.id).add_users(user) + CourseInstructorRole(self.course.id).add_users(user) + body = self.me(user) + assert set(body) == CALLER_MEMBERS + assert (body["role"], body["can_manage_team"], body["is_only_instructor"]) == ("instructor", True, True) + + def test_global_staff_who_are_the_only_instructor_are_told_so(self): + user = UserFactory(is_staff=True) + CourseInstructorRole(self.course.id).add_users(user) + body = self.me(user) + assert (body["role"], body["can_manage_team"], body["is_only_instructor"]) == ("instructor", True, True) + + def test_the_body_matches_the_published_schema(self): + user = legacy_team(self.course.id, CourseStaffRole) + assert_matches_published_schema(self.me(user), "CourseTeamCaller.v2") + assert_matches_published_schema(self.me(UserFactory(is_staff=True)), "CourseTeamCaller.v2") + + def test_head_answers_without_a_body(self): + response = api_client(UserFactory(is_staff=True)).head(me_url(self.course.id)) + assert (response.status_code, response.content) == (200, b"") + + +# --------------------------------------------------------------------------- +# Requests the legacy endpoint and v2 answer differently +# --------------------------------------------------------------------------- + +# Requests the legacy endpoint and v2 answer differently beyond the error body, +# as (request, legacy answer, v2 answer, test in TeamMembersResponseDifferencesTest). +RESPONSE_DIFFERENCES = [ + ("listing", "one object holding every member and the caller's flags", "a page of members in the standard envelope", + "test_the_list_is_paginated"), + ("order of members holding the same role", "no particular order", "by username", + "test_members_holding_the_same_role_are_ordered_by_username"), + ("whether the caller may manage the team, and is its only instructor", + "allow_actions and show_transfer_ownership_hint on the team", "can_manage_team and is_only_instructor on me", + "test_the_caller_flags_are_on_me"), + ("listing with a page beyond the last", "200 with every member (no paging)", "404", + "test_a_page_beyond_the_last"), + ("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", "200", "401", "test_a_bearer_token"), + ("slash-separated course key", "404 JSON", "404 HTML", "test_a_slash_separated_course_key"), +] + + +class TeamMembersResponseDifferencesTest(ModuleStoreTestCase): + """Each request the legacy endpoint and v2 answer differently, both answers observed.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.instructor = legacy_team(self.course.id, CourseInstructorRole) + self.client = api_client(self.instructor) + + 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_url(self.course.id)).json() + new = self.client.get(list_url(self.course.id)).json() + assert set(legacy) == {"show_transfer_ownership_hint", "users", "allow_actions"} + assert set(new) == set(ENVELOPE_KEYS) + assert legacy["users"] == new["results"] == [member(self.instructor, "instructor")] + + def test_members_holding_the_same_role_are_ordered_by_username(self): + staff = [UserFactory(username=name) for name in ("staff_c", "staff_a", "staff_b")] + CourseStaffRole(self.course.id).add_users(*staff) + legacy = self.client.get(legacy_url(self.course.id)).json()["users"] + new = self.client.get(list_url(self.course.id)).json()["results"] + assert legacy[0] == new[0] == member(self.instructor, "instructor") + assert sorted(entry["username"] for entry in legacy[1:]) == ["staff_a", "staff_b", "staff_c"] + assert [entry["username"] for entry in new[1:]] == ["staff_a", "staff_b", "staff_c"] + + def test_the_caller_flags_are_on_me(self): + legacy = self.client.get(legacy_url(self.course.id)).json() + new = self.client.get(me_url(self.course.id)).json() + assert (legacy["allow_actions"], legacy["show_transfer_ownership_hint"]) == (True, True) + assert (new["can_manage_team"], new["is_only_instructor"]) == (True, True) + listed = self.client.get(list_url(self.course.id)).json() + assert "allow_actions" not in listed + assert "show_transfer_ownership_hint" not in listed + + def test_a_page_beyond_the_last(self): + legacy = self.client.get(legacy_url(self.course.id), {"page": 2}) + assert legacy.status_code == 200 + assert legacy.json()["users"] == [member(self.instructor, "instructor")] + new = self.client.get(list_url(self.course.id), {"page": 2}) + assert_error_envelope(new, expected_status=404, expected_type_slug="not-found") + + def test_options(self): + """The legacy endpoint 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 outsider.options(legacy_url(UNKNOWN_COURSE_KEY)).status_code == 200 + for address in (list_url, me_url): + assert_refused(outsider.options(address(self.course.id)), 403) + assert_refused(outsider.options(address(UNKNOWN_COURSE_KEY)), 404) + assert self.client.options(address(self.course.id)).status_code == 200 + + 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(list_url(self.course.id), **header), 401) + assert_refused(APIClient().get(me_url(self.course.id), **header), 401) + + def test_a_slash_separated_course_key(self): + """The legacy endpoint routes the slash form and reports the course unknown in JSON; v2 has no route for it.""" + legacy = self.client.get("/api/contentstore/v1/course_team/edX/NoSuch/Course") + new = self.client.get("/api/authoring/v2/courses/edX/NoSuch/Course/team_members/") + assert legacy.status_code == new.status_code == 404 + assert legacy.json()["error_code"] == "course_does_not_exist" + assert new["Content-Type"].startswith("text/html") + + +# --------------------------------------------------------------------------- +# Parity with the legacy endpoint +# --------------------------------------------------------------------------- + +# Members of an error body that differ between the legacy endpoint 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 legacy error body carries developer_message; the envelope does not"), + ("error_code", "the legacy 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", "developer_message", "error_code"}) == set() + assert _undeclared({"[0].role", "results", "users", "typed"}) == {"[0].role", "results", "users", "typed"} + + +class _ParityMixin: + """ + v2 against the legacy endpoint for the same course and caller: the listing + holds the legacy members, ``me`` says what the legacy body says about the + caller, and errors differ only by the declared differences. + """ + + def add_member(self, course, role): + """Return a new user holding ``role`` (``instructor`` or ``staff``) on ``course``.""" + 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.instructors = [self.add_member(self.course, "instructor") for _ in range(2)] + self.staff = [self.add_member(self.course, "staff") for _ in range(3)] + self.observed = set() + + def compare(self, legacy, new, expected_status): + """Assert both statuses and that the bodies differ only as declared.""" + 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 + + def test_the_listing_holds_the_legacy_members_in_listed_order(self): + for caller in (self.instructors[0], self.staff[0]): + client = api_client(caller) + legacy = client.get(legacy_url(self.course.id)) + new = client.get(list_url(self.course.id), {"page_size": 100}) + assert legacy.status_code == new.status_code == 200, (legacy.content, new.content) + members, envelope = legacy.json()["users"], new.json() + assert changed_paths(in_listed_order(members), envelope["results"]) == set() + assert envelope["count"] == len(members) == 5 + assert [entry["role"] for entry in envelope["results"]] == ["instructor"] * 2 + ["staff"] * 3 + + def test_me_says_what_the_legacy_body_says_about_the_caller(self): + for caller in (*self.instructors, *self.staff): + client = api_client(caller) + legacy = client.get(legacy_url(self.course.id)) + new = client.get(me_url(self.course.id)) + assert legacy.status_code == new.status_code == 200, (legacy.content, new.content) + assert new.json() == caller_from_legacy(legacy.json(), caller) + + def _error_pairs(self): + """``(legacy, new, status)`` for each error both endpoints produce, for the listing and for ``me``.""" + for address in (list_url, me_url): + for client, course_key, status in ( + (api_client(), self.course.id, 401), + (api_client(self.outsider()), self.course.id, 403), + (api_client(self.outsider()), UNKNOWN_COURSE_KEY, 404), + ): + yield client.get(legacy_url(course_key)), client.get(address(course_key)), 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_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 TeamMembersParityLegacyPermissionsTest(_ParityMixin, ModuleStoreTestCase): + """Parity for course instructors and staff, with policy-based authoring permissions off.""" + + def add_member(self, course, role): + return legacy_team(course.id, {"instructor": CourseInstructorRole, "staff": CourseStaffRole}[role]) + + def outsider(self): + return UserFactory() + + +class TeamMembersParityAuthzTest(_ParityMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase): + """Parity for course admins and staff, with policy-based authoring permissions on.""" + + def add_member(self, course, role): + user = UserFactory() + policy_role = {"instructor": COURSE_ADMIN, "staff": COURSE_STAFF}[role] + self.add_user_to_role_in_course(user, policy_role.external_key, course.id) + return user + + def outsider(self): + return self.unauthorized_user + + +# --------------------------------------------------------------------------- +# Side effects and cost of reading +# --------------------------------------------------------------------------- + +# Every read, by address and query, in the order the side-effects test makes them. +READS = [ + (list_url, "get", {}), + (list_url, "get", {"page_size": 1}), + (list_url, "get", {"page_size": 1, "page": 2}), + (list_url, "head", {}), + (me_url, "get", {}), + (me_url, "head", {}), +] + + +class _ReadsAreSideEffectFreeMixin: + """ + Every read (listing, paged listing, ``me``, and HEAD of both) 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 may insert the permission + library's one policy version row and nothing else; the legacy endpoint + inserts the same row. + """ + + def reader_for(self, course): + """Return a team member of ``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, (address, method, params) in enumerate(READS): + with record_effects(self) as effects: + response = getattr(client, method)(address(self.course.id), params) + read = (index, address.__name__, 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_legacy_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)) + 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 TeamMembersReadSideEffectsLegacyPermissionsTest(_ReadsAreSideEffectFreeMixin, ModuleStoreTestCase): + """Reads by course staff with policy-based authoring permissions off.""" + + def reader_for(self, course): + reader = legacy_team(course.id, CourseStaffRole) + legacy_team(course.id, CourseInstructorRole) + PolicyCacheControl.objects.all().delete() + return reader + + +class TeamMembersReadSideEffectsAuthzTest( + _ReadsAreSideEffectFreeMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase, +): + """ + Reads by a course staff role holder with policy-based authoring + permissions on. The roles are 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_STAFF.external_key, course.id) + self.add_user_to_role_in_course(UserFactory(), COURSE_ADMIN.external_key, course.id) + PolicyCacheControl.objects.all().delete() + return reader + + +class _ReadCostMixin: + """Reads cost no more SQL queries or Mongo reads than the legacy read, and do not grow with the team.""" + + def add_member(self, course, role): + """Return a new user holding ``role`` (``instructor`` or ``staff``) on ``course``.""" + raise NotImplementedError + + def test_reads_cost_no_more_than_the_legacy_read(self): + course = CourseFactory.create() + reader = self.add_member(course, "staff") + self.add_member(course, "instructor") + client = api_client(reader) + legacy = measure(lambda: client.get(legacy_url(course.id))) + for address in (list_url, me_url): + new = measure(lambda address=address: client.get(address(course.id))) + assert new[0] <= legacy[0], (address.__name__, new, legacy) + assert new[1] <= legacy[1], (address.__name__, new, legacy) + assert new[2] == legacy[2] == 0, (address.__name__, new, legacy) + + def test_the_sql_cost_does_not_grow_with_the_team(self): + course = CourseFactory.create() + client = api_client(self.add_member(course, "staff")) + + def queries(address): + client.get(address(course.id), {"page_size": 100}) + with CaptureQueriesContext(connection) as captured: + assert client.get(address(course.id), {"page_size": 100}).status_code == 200 + return len(captured.captured_queries) + + one = {address: queries(address) for address in (list_url, me_url)} + for index in range(19): + self.add_member(course, "instructor" if index % 4 == 0 else "staff") + assert {address: queries(address) for address in (list_url, me_url)} == one + assert client.get(list_url(course.id), {"page_size": 100}).json()["count"] == 20 + + +class TeamMembersReadCostLegacyPermissionsTest(_ReadCostMixin, ModuleStoreTestCase): + """Costs for course staff, with policy-based authoring permissions off.""" + + def add_member(self, course, role): + return legacy_team(course.id, {"instructor": CourseInstructorRole, "staff": CourseStaffRole}[role]) + + +class TeamMembersReadCostAuthzTest(CourseAuthoringAuthzTestMixin, _ReadCostMixin, ModuleStoreTestCase): + """Costs for a course staff role holder, with policy-based authoring permissions on.""" + + def add_member(self, course, role): + user = UserFactory() + policy_role = {"instructor": COURSE_ADMIN, "staff": COURSE_STAFF}[role] + self.add_user_to_role_in_course(user, policy_role.external_key, course.id) + return user + + +# --------------------------------------------------------------------------- +# Addresses and methods +# --------------------------------------------------------------------------- + +@ddt.ddt +class TeamMembersRoutingTest(SharedModuleStoreTestCase): + """The addresses, the legacy 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_addresses(self): + assert list_url(COURSE_KEY) == f"/api/authoring/v2/courses/{COURSE_KEY}/team_members/" + assert me_url(COURSE_KEY) == f"/api/authoring/v2/courses/{COURSE_KEY}/team_members/me/" + for address, actions in ((list_url(COURSE_KEY), {"get": "list"}), (me_url(COURSE_KEY), {"get": "retrieve"})): + match = resolve(address) + assert match.func.cls is CourseTeamMembersViewSet + assert routed_actions(match.func) == actions + assert match.kwargs == {"course_key": CourseKey.from_string(COURSE_KEY)} + + def test_the_legacy_address_is_unchanged(self): + assert legacy_url(COURSE_KEY) == f"/api/contentstore/v1/course_team/{COURSE_KEY}" + view = resolve(legacy_url(COURSE_KEY)).func.cls + assert (view.__module__, view.__name__) == (LEGACY_MODULE, "CourseTeamView") + + def test_the_legacy_address_still_takes_a_slash_separated_key_that_v2_does_not(self): + view = resolve("/api/contentstore/v1/course_team/edX/DemoX/Demo_Course").func.cls + assert (view.__module__, view.__name__) == (LEGACY_MODULE, "CourseTeamView") + with pytest.raises(Resolver404): + resolve("/api/authoring/v2/courses/edX/DemoX/Demo_Course/team_members/") + + def test_only_the_caller_is_addressable_as_a_member(self): + with pytest.raises(Resolver404): + resolve(f"/api/authoring/v2/courses/{COURSE_KEY}/team_members/someone/") + response = self.client.get(f"/api/authoring/v2/courses/{self.course.id}/team_members/someone/") + assert response.status_code == 404 + assert response["Content-Type"].startswith("text/html") + + 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_team_member_list", "course_team_member_me"} <= 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(*[(address, method) for address in ("list", "me") for method in ("post", "put", "patch", "delete")]) + @ddt.unpack + def test_other_methods_are_not_allowed(self, address, method): + target = {"list": list_url, "me": me_url}[address](self.course.id) + response = getattr(self.client, method)(target, {}, 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") + + # 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, suffix) + for key in ("not-a-course-key", "edX/DemoX/Demo_Course", "edX+DemoX+Demo_Course") + for suffix in ("team_members/", "team_members/me/") + ]) + @ddt.unpack + def test_an_unparsable_or_slash_form_key_is_an_html_404(self, key, suffix): + response = self.client.get(f"/api/authoring/v2/courses/{key}/{suffix}") + assert response.status_code == 404 + assert response["Content-Type"].startswith("text/html") + + @ddt.data(list_url, me_url) + def test_the_address_without_its_trailing_slash_redirects_to_it(self, address): + """No route answers the address without its slash; the service redirects a read to the slashed one.""" + slashless = address(self.course.id).rstrip("/") + response = self.client.get(slashless) + assert response.status_code == 301 + assert response["Location"] == address(self.course.id) + with pytest.raises(Resolver404): + resolve(slashless) + + +# --------------------------------------------------------------------------- +# The published schema +# --------------------------------------------------------------------------- + +class TeamMembersSchemaTest(TestCase): + """The listing and ``me`` as published in the CMS authoring schema.""" + + LIST_PATH = "/api/authoring/v2/courses/{course_key}/team_members/" + ME_PATH = "/api/authoring/v2/courses/{course_key}/team_members/me/" + LEGACY_PATH = "/api/contentstore/v1/course_team/{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 _operation(self, path): + operations = self.schema["paths"][path] + assert sorted(operations) == ["get"] + return operations["get"] + + def test_both_operations_are_published_for_the_sdk(self): + for path, operation_id in ( + (self.LIST_PATH, "v2_courses_team_members_list"), + (self.ME_PATH, "v2_courses_team_members_me_retrieve"), + ): + operation = self._operation(path) + assert operation["operationId"] == operation_id + assert operation["tags"] == ["openedx-platform-sdk"] + assert "deprecated" not in operation + assert sorted(operation["responses"]) == ["200", "401", "403", "404"] + for status in ("401", "403", "404"): + assert self._ref(operation["responses"][status]) == "ErrorResponse", (path, status) + + def test_the_list_publishes_the_seven_envelope_keys(self): + name = self._ref(self._operation(self.LIST_PATH)["responses"]["200"]) + assert name == "PaginatedCourseTeamMember.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/CourseTeamMember.v2"}, + } + + def test_the_parameters(self): + listing = {p["name"]: p for p in self._operation(self.LIST_PATH)["parameters"]} + assert set(listing) == {"course_key", "page", "page_size"} + for name in ("page", "page_size"): + assert (listing[name]["in"], listing[name]["schema"]) == ("query", {"type": "integer"}) + assert not listing[name].get("required") + me = {p["name"]: p for p in self._operation(self.ME_PATH)["parameters"]} + assert set(me) == {"course_key"} + for parameters in (listing, me): + course_key = parameters["course_key"] + assert (course_key["in"], course_key["required"], course_key["schema"]["type"]) == ("path", True, "string") + assert course_key["schema"]["pattern"] == f"^{CourseKeyConverter.regex}$" + + def test_the_member_and_caller_components(self): + member_component = self.components["CourseTeamMember.v2"] + caller_component = self.components["CourseTeamCaller.v2"] + assert self._ref(self._operation(self.ME_PATH)["responses"]["200"]) == "CourseTeamCaller.v2" + assert set(member_component["properties"]) == set(member_component["required"]) == MEMBER_MEMBERS + assert set(caller_component["properties"]) == set(caller_component["required"]) == CALLER_MEMBERS + (role,) = member_component["properties"]["role"]["allOf"] + assert resolve_ref(self.schema, role)["enum"] == list(LISTED_ORDER) + caller_role = caller_component["properties"]["role"] + assert caller_role["nullable"] is True + assert [resolve_ref(self.schema, option)["enum"] for option in caller_role["oneOf"]] == [ + list(LISTED_ORDER), [None], + ] + assert member_component["properties"]["id"]["type"] == "integer" + for flag in ("can_manage_team", "is_only_instructor"): + assert caller_component["properties"][flag]["type"] == "boolean" + nullable = { + name for component in (member_component, caller_component) + for name, field in component["properties"].items() if field.get("nullable") + } + assert nullable == {"role"} + + def test_every_member_is_described(self): + for name in ("CourseTeamMember.v2", "CourseTeamCaller.v2"): + 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)["responses"]["200"]["content"]["application/json"] + page = listing["examples"]["TeamMember"]["value"] + assert set(page) == set(ENVELOPE_KEYS) + assert len(page["results"]) == 1 + assert_matches_published_schema(page, "PaginatedCourseTeamMember.v2List") + me = self._operation(self.ME_PATH)["responses"]["200"]["content"]["application/json"] + assert_matches_published_schema(me["examples"]["OnlyInstructor"]["value"], "CourseTeamCaller.v2") + + def test_the_paginator_is_the_shared_one(self): + assert CourseTeamMembersViewSet.pagination_class is DocumentedDefaultPagination + + def test_the_legacy_operation_is_deprecated_and_left_out_of_the_sdk(self): + operation = self._operation(self.LEGACY_PATH) + assert operation["operationId"] == "v1_course_team_retrieve" + 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_PATH, "get") in deprecated + assert {(self.LIST_PATH, "get"), (self.ME_PATH, "get")} & 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/views/permissions.py b/cms/djangoapps/contentstore/views/permissions.py index 6450a3bfcc3e..4ff4d58ce00c 100644 --- a/cms/djangoapps/contentstore/views/permissions.py +++ b/cms/djangoapps/contentstore/views/permissions.py @@ -2,14 +2,24 @@ 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_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 ( + STUDIO_VIEW_USERS, + check_course_advanced_settings_access, + get_user_permissions, + 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 +68,103 @@ 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 CanViewCourseTeam(BasePermission): + """ + Allow callers who may see who is on the team of the course in the path. + + Reads the parsed ``course_key`` URL argument. Global staff, and the + instructors and staff of the course or of its organization, may; limited + staff may not. The decision rests on Studio's course roles whether or not + policy-based authoring permissions are enabled for the course. A role + assigned through those permissions counts only as the Studio course role + it stands for: course admin as instructor, course staff as staff. A + policy role that stands for no Studio course role, such as course editor + or course auditor, is refused even though the policy lets it view the + team. + """ + + def has_permission(self, request, view): + course_key = view.kwargs.get("course_key") + if course_key is None: + return False + return bool(get_user_permissions(request.user, course_key) & STUDIO_VIEW_USERS) diff --git a/cms/lib/spectacular.py b/cms/lib/spectacular.py index 662cf80c65f0..b06ef0824200 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}/team_members/ (and team_members/me/) + "/api/contentstore/v1/course_team/{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..76828680d985 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/course_team/{course_id}", } def test_the_conforming_paths_are_left_unmarked(self): @@ -83,8 +88,48 @@ 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_course_team_operation_is_marked_and_its_successors_are_not(self): + legacy = "/api/contentstore/v1/course_team/{course_id}" + conforming = ( + "/api/authoring/v2/courses/{course_key}/team_members/", + "/api/authoring/v2/courses/{course_key}/team_members/me/", + ) + result = self._run({"paths": {path: {"get": {}} for path in (legacy, *conforming)}}) + assert result["paths"][legacy] == {"get": {"deprecated": True}} + assert {path: result["paths"][path] for path in conforming} == {path: {"get": {}} for path in conforming} + 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"]