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/course_optimizer.py b/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py index 2af5f148b49c..586a873cd85c 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/course_optimizer.py @@ -33,6 +33,9 @@ class LinkCheckView(DeveloperErrorViewMixin, APIView): """ View for queueing a celery task to scan a course for broken links. + + Deprecated: use ``POST /api/authoring/v1/courses/{course_key}/link_check/`` instead. + See https://github.com/openedx/openedx-platform/issues/39128. """ @extend_schema( parameters=[ @@ -50,6 +53,10 @@ def post(self, request: Request, course_id: str): """ Queue celery task to scan a course for broken links. + Deprecated: use ``POST /api/authoring/v1/courses/{course_key}/link_check/``, which answers + 202 with ``{"status": "Pending"}``. + See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** POST /api/contentstore/v0/link_check/{course_id} @@ -77,6 +84,9 @@ def post(self, request: Request, course_id: str): class LinkCheckStatusView(DeveloperErrorViewMixin, APIView): """ View for checking the status of the celery task and returning the results. + + Deprecated: use ``GET /api/authoring/v1/courses/{course_key}/link_check/`` instead. + See https://github.com/openedx/openedx-platform/issues/39128. """ @extend_schema( parameters=[ @@ -103,6 +113,11 @@ def get(self, request: Request, course_id: str): This function adds a status for when status from UserTaskStatus is None: 'Uninitiated' + Deprecated: use ``GET /api/authoring/v1/courses/{course_key}/link_check/``. Its default + view returns ``status``, ``created_at`` and ``error``; ``view=full`` adds ``output``, the + report this endpoint returns, with snake_case member names. + See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** GET /api/contentstore/v0/link_check_status/{course_id} @@ -233,6 +248,9 @@ def get(self, request: Request, course_id: str): class RerunLinkUpdateView(DeveloperErrorViewMixin, APIView): """ View for queueing a celery task to update course links to the latest re-run. + + Deprecated: use ``POST /api/authoring/v1/courses/{course_key}/previous_run_link_update/`` instead. + See https://github.com/openedx/openedx-platform/issues/39128. """ @extend_schema( @@ -255,6 +273,10 @@ def post(self, request: Request, course_id: str): """ Queue celery task to update course links to the latest re-run. + Deprecated: use ``POST /api/authoring/v1/courses/{course_key}/previous_run_link_update/``, + which takes this body and answers 202 with ``{"status": "Pending"}``. + See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request - Update All Links** POST /api/contentstore/v0/rerun_link_update/{course_id} ```json @@ -341,6 +363,9 @@ def post(self, request: Request, course_id: str): class RerunLinkUpdateStatusView(DeveloperErrorViewMixin, APIView): """ View for checking the status of the course link update task and returning the results. + + Deprecated: use ``GET /api/authoring/v1/courses/{course_key}/previous_run_link_update/`` instead. + See https://github.com/openedx/openedx-platform/issues/39128. """ @extend_schema( @@ -367,6 +392,10 @@ def get(self, request: Request, course_id: str): Possible statuses: 'pending', 'in_progress', 'completed', 'failed', 'uninitiated' + Deprecated: use ``GET /api/authoring/v1/courses/{course_key}/previous_run_link_update/``. + Its default view returns ``status``; ``view=full`` adds ``results``. + See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** GET /api/contentstore/v0/rerun_link_update_status/{course_id} 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..6ad8f3b36cf1 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py @@ -7,6 +7,11 @@ YoutubeTranscriptChecksViewSet, YoutubeTranscriptImportsViewSet, ) +from cms.djangoapps.contentstore.rest_api.v1.views.advanced_settings import CourseAdvancedSettingsViewSet +from cms.djangoapps.contentstore.rest_api.v1.views.course_optimizer import ( + CourseLinkCheckViewSet, + CoursePreviousRunLinkUpdateViewSet, +) app_name = "authoring_v1" @@ -39,4 +44,19 @@ ), name="xblock_detail", ), + path( + "courses//advanced_settings/", + CourseAdvancedSettingsViewSet.as_view({"get": "retrieve", "patch": "partial_update"}), + name="course_advanced_settings", + ), + path( + "courses//link_check/", + CourseLinkCheckViewSet.as_view({"get": "retrieve", "post": "create"}), + name="course_link_check", + ), + path( + "courses//previous_run_link_update/", + CoursePreviousRunLinkUpdateViewSet.as_view({"get": "retrieve", "post": "create"}), + name="course_previous_run_link_update", + ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v1/course_optimizer_service.py b/cms/djangoapps/contentstore/rest_api/v1/course_optimizer_service.py new file mode 100644 index 000000000000..671201d9bfe6 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/course_optimizer_service.py @@ -0,0 +1,139 @@ +"""Runs of the course optimizer: link checks and updates of links to a previous course run.""" + +import json + +from rest_framework.exceptions import NotFound +from user_tasks.models import UserTaskArtifact, UserTaskStatus + +from cms.djangoapps.contentstore.core.course_optimizer_provider import ( + _latest_course_link_update_task_status, + _latest_task_status, + generate_broken_links_descriptor, + get_course_link_update_data, + sort_course_sections, +) +from cms.djangoapps.contentstore.tasks import check_broken_links, update_course_rerun_links +from cms.djangoapps.contentstore.toggles import enable_course_optimizer_check_prev_run_links + +# The state of a resource no run of which the caller may see. +UNINITIATED = "Uninitiated" +# The stages the link update task reports while it runs. +SCANNING = "Scanning" +UPDATING = "Updating" + +# Every state a link check or a link update can report. Scanning and Updating +# are reported by link updates only. +RUN_STATES = [ + UNINITIATED, + UserTaskStatus.PENDING, + UserTaskStatus.IN_PROGRESS, + SCANNING, + UPDATING, + UserTaskStatus.SUCCEEDED, + UserTaskStatus.FAILED, + UserTaskStatus.CANCELED, + UserTaskStatus.RETRYING, +] + +ALL_LINKS = "all" +SINGLE_LINKS = "single" + +# Reported for a link check that failed or was canceled with an error recorded. +# The recorded text can be an exception's, so it is not returned. +LINK_CHECK_ERROR_MESSAGE = "The link check could not be completed." +# Reported for each link an update run could not change, in place of the +# recorded reason, which can be an exception's text. +LINK_NOT_UPDATED_MESSAGE = "The link could not be updated." +PREVIOUS_RUN_LINK_UPDATES_DISABLED_MESSAGE = "Updating links to a previous course run is not enabled for this course." + +_FAILED_STATES = (UserTaskStatus.FAILED, UserTaskStatus.CANCELED) + + +def start_link_check(course_key, user, language): + """Queue a check of the links in the course, as ``user``; return the state of the queued run.""" + check_broken_links.delay(user.id, str(course_key), language) + return {"status": UserTaskStatus.PENDING} + + +def link_check_status(request, course_key, full=False): + """ + Return the latest link check of the course that the requester may see. + + Callers see their own runs; superusers see everyone's. With ``full``, the + report of a run that succeeded is added under ``output`` (``None`` + otherwise). + """ + run = _latest_task_status(request, str(course_key)) + if run is None: + status = {"status": UNINITIATED, "created_at": None, "error": None} + else: + failed = run.state in _FAILED_STATES and ( + UserTaskArtifact.objects.filter(status=run, name="Error").exclude(text="").exists() + ) + status = { + "status": run.state, + "created_at": run.created, + "error": LINK_CHECK_ERROR_MESSAGE if failed else None, + } + if full: + succeeded = run is not None and run.state == UserTaskStatus.SUCCEEDED + status["output"] = _link_check_report(request.user, course_key, run) if succeeded else None + return status + + +def _link_check_report(user, course_key, run): + """The flagged links a run found, sections in course order.""" + artifact = UserTaskArtifact.objects.get(status=run, name="BrokenLinks") + with artifact.file as file: + links = json.loads(file.read()) + report = generate_broken_links_descriptor(links, user, course_key) + return sort_course_sections(course_key, {"LinkCheckOutput": report})["LinkCheckOutput"] + + +def ensure_previous_run_link_updates_enabled(course_key): + """Raise ``NotFound`` unless links to a previous run may be updated in the course.""" + if not enable_course_optimizer_check_prev_run_links(course_key): + raise NotFound(PREVIOUS_RUN_LINK_UPDATES_DISABLED_MESSAGE) + + +def start_previous_run_link_update(course_key, user, action, links, language): + """ + Queue an update of the course's links to its previous run, as ``user``. + + ``action`` is ``all`` (every such link the course has) or ``single`` (the + ``links`` given). Returns the state of the queued run. Callers check + ``ensure_previous_run_link_updates_enabled`` first. + """ + update_course_rerun_links.delay(user.id, str(course_key), action, [dict(link) for link in links], language) + return {"status": UserTaskStatus.PENDING} + + +def previous_run_link_update_status(request, course_key, full=False): + """ + Return the latest update of links to the previous run that the requester may see. + + Visibility is as for link checks. With ``full``, the outcome of each link + of a run that succeeded is added under ``results`` (empty otherwise). + Callers check ``ensure_previous_run_link_updates_enabled`` first. + """ + if not full: + run = _latest_course_link_update_task_status(request, str(course_key)) + return {"status": UNINITIATED if run is None else run.state} + data = get_course_link_update_data(request, str(course_key)) + return { + "status": UNINITIATED if data["status"] == "uninitiated" else data["status"], + "results": [_link_update_result(result) for result in data["results"]], + } + + +def _link_update_result(result): + """One link's outcome, with a fixed message in place of the recorded failure reason.""" + success = bool(result["success"]) + return { + "id": result["id"], + "type": result["type"], + "original_url": result.get("original_url"), + "new_url": result["new_url"], + "success": success, + "error_message": None if success else LINK_NOT_UPDATED_MESSAGE, + } 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/course_optimizer.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_optimizer.py new file mode 100644 index 000000000000..6190a77ad8d2 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_optimizer.py @@ -0,0 +1,279 @@ +"""Serializers for the course optimizer's link checks and previous-run link updates.""" + +from collections.abc import Mapping + +from rest_framework import serializers + +from cms.djangoapps.contentstore.rest_api.v1.course_optimizer_service import ALL_LINKS, RUN_STATES, SINGLE_LINKS + +MINIMAL_VIEW = "minimal" +FULL_VIEW = "full" + + +class QueuedRunSerializer(serializers.Serializer): # pylint: disable=abstract-method + """The state of a run that has just been queued.""" + + status = serializers.ChoiceField( + choices=RUN_STATES, + help_text="State of the run: always Pending, as the run waits to be picked up.", + ) + + class Meta: + ref_name = "CourseOptimizerQueuedRun.v1" + + +class PreviousRunLinkSerializer(serializers.Serializer): # pylint: disable=abstract-method + """A link to the previous run of the course, and whether it now points at this run.""" + + original_link = serializers.CharField(source="originalLink", help_text="The link as it points at the previous run.") + is_updated = serializers.BooleanField( + source="isUpdated", help_text="Whether the link has been changed to point at this run.", + ) + updated_link = serializers.CharField( + source="updatedLink", + allow_null=True, + help_text="The link as it points at this run, or null when it has not been changed.", + ) + + class Meta: + ref_name = "LinkCheckPreviousRunLink.v1" + + +class LinkCheckReportBlockSerializer(serializers.Serializer): # pylint: disable=abstract-method + """A component, course update, handouts page or custom page with the links found in it, by kind.""" + + id = serializers.CharField(help_text="Usage key of the component or page, or the id of the course update.") + display_name = serializers.CharField( + source="displayName", allow_blank=True, help_text="Name of the component or page as Studio shows it.", + ) + url = serializers.CharField(help_text="Path of the Studio page where the content is edited.") + broken_links = serializers.ListField( + source="brokenLinks", child=serializers.CharField(), help_text="Links that could not be reached.", + ) + locked_links = serializers.ListField( + source="lockedLinks", + child=serializers.CharField(), + help_text="Links to Studio content that refused access, such as locked files.", + ) + external_forbidden_links = serializers.ListField( + source="externalForbiddenLinks", + child=serializers.CharField(), + help_text="Links to other sites that refused access.", + ) + previous_run_links = PreviousRunLinkSerializer( + source="previousRunLinks", many=True, help_text="Links that point at the previous run of the course.", + ) + + class Meta: + ref_name = "LinkCheckReportBlock.v1" + + +class LinkCheckReportUnitSerializer(serializers.Serializer): # pylint: disable=abstract-method + """A unit holding components with flagged links.""" + + id = serializers.CharField(help_text="Usage key of the unit.") + display_name = serializers.CharField( + source="displayName", allow_blank=True, help_text="Name of the unit as Studio shows it.", + ) + blocks = LinkCheckReportBlockSerializer(many=True, help_text="Components of the unit with flagged links.") + + class Meta: + ref_name = "LinkCheckReportUnit.v1" + + +class LinkCheckReportSubsectionSerializer(serializers.Serializer): # pylint: disable=abstract-method + """A subsection holding units with flagged links.""" + + id = serializers.CharField(help_text="Usage key of the subsection.") + display_name = serializers.CharField( + source="displayName", allow_blank=True, help_text="Name of the subsection as Studio shows it.", + ) + units = LinkCheckReportUnitSerializer(many=True, help_text="Units of the subsection with flagged links.") + + class Meta: + ref_name = "LinkCheckReportSubsection.v1" + + +class LinkCheckReportSectionSerializer(serializers.Serializer): # pylint: disable=abstract-method + """A section holding subsections with flagged links.""" + + id = serializers.CharField(help_text="Usage key of the section.") + display_name = serializers.CharField( + source="displayName", allow_blank=True, help_text="Name of the section as Studio shows it.", + ) + subsections = LinkCheckReportSubsectionSerializer( + many=True, help_text="Subsections of the section with flagged links.", + ) + + class Meta: + ref_name = "LinkCheckReportSection.v1" + + +class LinkCheckReportSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + The links a check flagged, where they were found. + + Course content is reported as sections, subsections, units and components, + four levels and no more, in course order; only the parts holding flagged + links appear. Course updates (with the handouts page last) and custom + pages are listed whether or not they hold any. + """ + + sections = LinkCheckReportSectionSerializer(many=True, help_text="Sections with flagged links, in course order.") + course_updates = LinkCheckReportBlockSerializer( + many=True, help_text="Course updates and the handouts page, with the links found in each.", + ) + custom_pages = LinkCheckReportBlockSerializer( + many=True, help_text="Custom pages of the course, with the links found in each.", + ) + + class Meta: + ref_name = "LinkCheckReport.v1" + + +class LinkCheckSerializer(serializers.Serializer): # pylint: disable=abstract-method + """The latest link check of a course that the requester may see.""" + + status = serializers.ChoiceField( + choices=RUN_STATES, + help_text=( + "State of the run. Uninitiated when the requester may see no run of this course; " + "Scanning and Updating are not reported for link checks." + ), + ) + created_at = serializers.DateTimeField( + allow_null=True, help_text="When the run was queued, or null when there is no run.", + ) + error = serializers.CharField( + allow_null=True, + help_text="A fixed message when the run failed or was canceled with an error recorded; otherwise null.", + ) + output = LinkCheckReportSerializer( + required=False, + allow_null=True, + help_text="Only with view=full: the links the run flagged, or null unless the run succeeded.", + ) + + class Meta: + ref_name = "LinkCheck.v1" + + +class LinkToUpdateSerializer(serializers.Serializer): # pylint: disable=abstract-method + """A link to change so that it points at this run instead of the previous one.""" + + url = serializers.CharField(help_text="The link as it appears in the content.") + type = serializers.CharField( + help_text=( + "Where the link was found, which decides how it is changed: 'course_content' for a component " + "of the course outline, 'course_updates' for a course update (or the handouts page, when id names " + "it), 'handouts' for the handouts page, and 'custom_pages' for a custom page. Other values are " + "treated as 'course_content'." + ), + ) + id = serializers.CharField( + help_text="Usage key of the component or page holding the link, or the course update id.", + ) + + class Meta: + ref_name = "LinkToUpdate.v1" + + +class PreviousRunLinkUpdateRequestSerializer(serializers.Serializer): # pylint: disable=abstract-method + """Which links to the previous run of the course to change.""" + + action = serializers.ChoiceField( + choices=[ALL_LINKS, SINGLE_LINKS], + help_text="'all' changes every link to the previous run the course holds; 'single' changes the links in data.", + ) + data = LinkToUpdateSerializer( + many=True, + required=False, + help_text=( + "The links to change, as a link check reports them. Required, and not empty, when action is " + "'single'. Not read when action is 'all', so it is neither validated nor used then." + ), + ) + + class Meta: + ref_name = "PreviousRunLinkUpdateRequest.v1" + + def to_internal_value(self, data): + """Drop ``data`` before validation when every link is to be changed, as it is not read then.""" + if isinstance(data, Mapping) and data.get("action") == ALL_LINKS: + data = {key: value for key, value in data.items() if key != "data"} + return super().to_internal_value(data) + + def validate(self, attrs): + if attrs["action"] == SINGLE_LINKS and not attrs.get("data"): + raise serializers.ValidationError({"data": ["This field is required when action is 'single'."]}) + return attrs + + +class LinkUpdateResultSerializer(serializers.Serializer): # pylint: disable=abstract-method + """The outcome of changing one link.""" + + id = serializers.CharField( + allow_blank=True, help_text="Usage key of the component or page holding the link, or the course update id.", + ) + type = serializers.CharField(allow_blank=True, help_text="Where the link was found.") + original_url = serializers.CharField( + allow_blank=True, allow_null=True, help_text="The link before the change, or null when it was not recorded.", + ) + new_url = serializers.CharField( + allow_blank=True, help_text="The link after the change; the original link when it could not be changed.", + ) + success = serializers.BooleanField(help_text="Whether the link was changed.") + error_message = serializers.CharField( + allow_null=True, help_text="A fixed message when the link could not be changed; otherwise null.", + ) + + class Meta: + ref_name = "LinkUpdateResult.v1" + + +class PreviousRunLinkUpdateSerializer(serializers.Serializer): # pylint: disable=abstract-method + """The latest update of links to the previous run that the requester may see.""" + + status = serializers.ChoiceField( + choices=RUN_STATES, + help_text=( + "State of the run; Scanning and Updating are the stages of a run in progress. " + "Uninitiated when the requester may see no run of this course." + ), + ) + results = LinkUpdateResultSerializer( + many=True, + required=False, + help_text="Only with view=full: the outcome of each link of a run that succeeded; empty otherwise.", + ) + + class Meta: + ref_name = "PreviousRunLinkUpdate.v1" + + +def _view_field(full_view_adds): + return serializers.ChoiceField( + choices=[MINIMAL_VIEW, FULL_VIEW], + default=MINIMAL_VIEW, + help_text=( + f"Which representation to return. 'minimal' returns the state of the run; 'full' adds {full_view_adds}." + ), + ) + + +class LinkCheckQuerySerializer(serializers.Serializer): # pylint: disable=abstract-method + """Query parameters of a read of a course's latest link check.""" + + view = _view_field("the links the run flagged, which can be many") + + class Meta: + ref_name = "LinkCheckQuery.v1" + + +class PreviousRunLinkUpdateQuerySerializer(serializers.Serializer): # pylint: disable=abstract-method + """Query parameters of a read of a course's latest update of links to its previous run.""" + + view = _view_field("the outcome of each link the run changed") + + class Meta: + ref_name = "PreviousRunLinkUpdateQuery.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_optimizer.py b/cms/djangoapps/contentstore/rest_api/v1/views/course_optimizer.py new file mode 100644 index 000000000000..3b82f6636c77 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/course_optimizer.py @@ -0,0 +1,332 @@ +"""API views for the course optimizer's link checks and previous-run link updates.""" + +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 status, viewsets +from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response + +from cms.djangoapps.contentstore.rest_api.v1.course_optimizer_service import ( + LINK_CHECK_ERROR_MESSAGE, + LINK_NOT_UPDATED_MESSAGE, + ensure_previous_run_link_updates_enabled, + link_check_status, + previous_run_link_update_status, + start_link_check, + start_previous_run_link_update, +) +from cms.djangoapps.contentstore.rest_api.v1.serializers.course_optimizer import ( + FULL_VIEW, + MINIMAL_VIEW, + LinkCheckQuerySerializer, + LinkCheckSerializer, + PreviousRunLinkUpdateQuerySerializer, + PreviousRunLinkUpdateRequestSerializer, + PreviousRunLinkUpdateSerializer, + QueuedRunSerializer, +) +from cms.djangoapps.contentstore.views.permissions import CanEditCourseContent, CanStartLinkCheck, 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 _view_parameter(query_serializer_class): + return OpenApiParameter( + name="view", + description=query_serializer_class().fields["view"].help_text, + required=False, + location=OpenApiParameter.QUERY, + type=str, + enum=[MINIMAL_VIEW, FULL_VIEW], + default=MINIMAL_VIEW, + ) + + +def _error(description): + return OpenApiResponse(response=ErrorResponseSerializer, description=description) + + +_BLOCK_KEY = "block-v1:edX+DemoX+2026+type@{}+block@{}" +_REPORT_EXAMPLE = OpenApiExample( + "Full report", + summary="A link check that succeeded, read with view=full", + response_only=True, + status_codes=["200"], + value={ + "status": "Succeeded", + "created_at": "2026-10-25T14:32:01.294587Z", + "error": None, + "output": { + "sections": [{ + "id": _BLOCK_KEY.format("chapter", "week1"), + "display_name": "Week 1", + "subsections": [{ + "id": _BLOCK_KEY.format("sequential", "lesson"), + "display_name": "Lesson", + "units": [{ + "id": _BLOCK_KEY.format("vertical", "unit"), + "display_name": "Unit", + "blocks": [{ + "id": _BLOCK_KEY.format("html", "reading"), + "display_name": "Reading", + "url": "/course/course-v1:edX+DemoX+2026/editor/html/" + + _BLOCK_KEY.format("html", "reading"), + "broken_links": ["https://example.com/missing"], + "locked_links": ["/static/handout.pdf"], + "external_forbidden_links": [], + "previous_run_links": [{ + "original_link": "https://studio.example.com/course/course-v1:edX+DemoX+2025/info", + "is_updated": False, + "updated_link": None, + }], + }], + }], + }], + }], + "course_updates": [], + "custom_pages": [], + }, + }, +) + +_QUEUED = OpenApiResponse(response=QueuedRunSerializer, description="The run was queued.") +_UNAUTHENTICATED = _error("The requester is not authenticated.") +_COURSE_NOT_FOUND = _error("The course does not exist.") + + +class _CourseOptimizerViewSet(StandardizedErrorMixin, viewsets.ViewSet): + """ + Checks the course, then the permission the requested action needs. + + Subclasses restate the checks every request passes in + ``permission_classes``, map each action to its own in + ``action_permission_classes``, and name the members of the minimal view + in ``minimal_fields``. An action with no permission declared is refused + with a server error instead of being served unchecked. + """ + + # Kept from the legacy endpoints: an inactive account's browser session stays authenticated, + # and the course permission decides whether it may call. + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated, IsExistingCourse) + action_permission_classes = {} + # A method with no route (answered 405) and OPTIONS read nothing of the + # course's runs, so only the course is checked for them. + unrouted_actions = (None, "metadata") + + def get_permissions(self): + """Check the course, then the permission the requested action needs.""" + 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 response serializer declared on this viewset.""" + return self.serializer_class(*args, **kwargs) + + @staticmethod + def _full_view_requested(request, query_serializer_class): + """Validate the query parameters and return whether ``view=full`` was requested.""" + query = query_serializer_class(data=request.query_params) + query.is_valid(raise_exception=True) + return query.validated_data["view"] == FULL_VIEW + + def _shaped(self, run, full): + """Serialize ``run``, keeping only ``minimal_fields`` unless the full view was requested.""" + data = self.get_serializer(run).data + return data if full else project(data, self.minimal_fields) + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CourseLinkCheckViewSet(_CourseOptimizerViewSet): + """ + The latest check of the links in a course. + + Starting a check queues a run that follows every link in the course's + content, updates, handouts and custom pages, and records those that are + broken, refuse access, or point at the course's previous run. A read + returns the latest run the requester may see: their own, or anyone's for + a superuser. By default (``view=minimal``) only the state of the run is + returned; ``view=full`` adds the links it flagged. + + Both need the permission to edit course content. Where policy-based + authoring permissions are off, starting a check needs only Studio read + access to the course while reading one needs write access; the two checks + are kept separate on purpose. 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``: runs are queued as tasks and read from their recorded + status, not addressed as rows. + """ + + permission_classes = (IsAuthenticated, IsExistingCourse) + action_permission_classes = { + "create": (CanStartLinkCheck,), + "retrieve": (CanEditCourseContent,), + } + serializer_class = LinkCheckSerializer + minimal_fields = ("status", "created_at", "error") + + @extend_schema( + summary="Start a check of a course's links", + description=( + "Queues a check of every link in the course and returns the state of the queued run. " + "Read the result from this address." + ), + parameters=[_COURSE_KEY_PARAMETER], + request=None, + responses={ + 202: _QUEUED, + 401: _UNAUTHENTICATED, + 403: _error("The requester may not check this course's links."), + 404: _COURSE_NOT_FOUND, + }, + ) + def create(self, request, course_key): + """Queue a link check of the course as the requester.""" + queued = start_link_check(course_key, request.user, request.LANGUAGE_CODE) + return Response(QueuedRunSerializer(queued).data, status=status.HTTP_202_ACCEPTED) + + @extend_schema( + summary="Get the latest check of a course's links", + description=( + "Returns the state of the latest link check of the course that the requester may see: their own " + "runs, or anyone's for a superuser. The state is Uninitiated when there is none. The error member " + f"of a run that failed is the fixed message '{LINK_CHECK_ERROR_MESSAGE}'. view=full adds the links " + "the run flagged, for a run that succeeded." + ), + parameters=[_COURSE_KEY_PARAMETER, _view_parameter(LinkCheckQuerySerializer)], + responses={ + 200: OpenApiResponse(response=LinkCheckSerializer, description="The latest link check."), + 400: _error("A query parameter has an invalid value."), + 401: _UNAUTHENTICATED, + 403: _error("The requester may not read this course's link checks."), + 404: _COURSE_NOT_FOUND, + }, + examples=[_REPORT_EXAMPLE], + ) + def retrieve(self, request, course_key): + """Return the latest link check the requester may see, in the requested view.""" + full = self._full_view_requested(request, LinkCheckQuerySerializer) + return Response(self._shaped(link_check_status(request, course_key, full=full), full)) + + +_UPDATE_ALL_EXAMPLE = OpenApiExample( + "Every link", + summary="Change every link to the previous run", + request_only=True, + value={"action": "all"}, +) +_UPDATE_SINGLE_EXAMPLE = OpenApiExample( + "Some links", + summary="Change the links a link check reported", + request_only=True, + value={ + "action": "single", + "data": [ + { + "url": "https://studio.example.com/course/course-v1:edX+DemoX+2025/course", + "type": "course_updates", + "id": "1", + }, + ], + }, +) + + +@extend_schema(tags=["openedx-platform-sdk"]) +class CoursePreviousRunLinkUpdateViewSet(_CourseOptimizerViewSet): + """ + The latest update of a course's links that point at its previous run. + + Starting an update queues a run that changes such links to point at this + run: every one the course holds (``action`` ``all``) or those listed + (``single``). A read returns the latest run the requester may see, as for + link checks. By default (``view=minimal``) only the state of the run is + returned; ``view=full`` adds the outcome of each link. + + Both need the permission to edit course content (Studio write access + where policy-based authoring permissions are off). Where updating links + to a previous run is not enabled for the course, both answer 404 once the + caller's permission is established. + + A plain ``ViewSet``, as for link checks. + """ + + permission_classes = (IsAuthenticated, IsExistingCourse) + action_permission_classes = { + "create": (CanEditCourseContent,), + "retrieve": (CanEditCourseContent,), + } + serializer_class = PreviousRunLinkUpdateSerializer + minimal_fields = ("status",) + + @extend_schema( + summary="Start an update of a course's links to its previous run", + description=( + "Queues a run that changes links to the course's previous run so that they point at this run. " + "With action 'all' every such link is changed; with 'single', the links in data. Returns the " + "state of the queued run; read the result from this address." + ), + parameters=[_COURSE_KEY_PARAMETER], + request={"application/json": PreviousRunLinkUpdateRequestSerializer}, + responses={ + 202: _QUEUED, + 400: _error("The body names no valid action, or names 'single' without links."), + 401: _UNAUTHENTICATED, + 403: _error("The requester may not change this course's content."), + 404: _error("The course does not exist, or updating links to its previous run is not enabled."), + }, + examples=[_UPDATE_ALL_EXAMPLE, _UPDATE_SINGLE_EXAMPLE], + ) + def create(self, request, course_key): + """Queue an update of the course's links to its previous run as the requester.""" + ensure_previous_run_link_updates_enabled(course_key) + body = PreviousRunLinkUpdateRequestSerializer(data=request.data) + body.is_valid(raise_exception=True) + queued = start_previous_run_link_update( + course_key, + request.user, + body.validated_data["action"], + body.validated_data.get("data", []), + request.LANGUAGE_CODE, + ) + return Response(QueuedRunSerializer(queued).data, status=status.HTTP_202_ACCEPTED) + + @extend_schema( + summary="Get the latest update of a course's links to its previous run", + description=( + "Returns the state of the latest such update of the course that the requester may see: their own " + "runs, or anyone's for a superuser. The state is Uninitiated when there is none. view=full adds " + "the outcome of each link of a run that succeeded; a link that could not be changed carries the " + f"fixed message '{LINK_NOT_UPDATED_MESSAGE}'." + ), + parameters=[_COURSE_KEY_PARAMETER, _view_parameter(PreviousRunLinkUpdateQuerySerializer)], + responses={ + 200: OpenApiResponse(response=PreviousRunLinkUpdateSerializer, description="The latest update."), + 400: _error("A query parameter has an invalid value."), + 401: _UNAUTHENTICATED, + 403: _error("The requester may not read this course's link updates."), + 404: _error("The course does not exist, or updating links to its previous run is not enabled."), + }, + ) + def retrieve(self, request, course_key): + """Return the latest update the requester may see, in the requested view.""" + ensure_previous_run_link_updates_enabled(course_key) + full = self._full_view_requested(request, PreviousRunLinkUpdateQuerySerializer) + return Response(self._shaped(previous_run_link_update_status(request, course_key, full=full), full)) 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_course_optimizer.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_optimizer.py new file mode 100644 index 000000000000..6af0ffd328ad --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_optimizer.py @@ -0,0 +1,1907 @@ +"""Tests for the v1 course optimizer API: link checks and updates of links to a previous run.""" + +import json +import re +import uuid +from contextlib import contextmanager +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.core.files.base import ContentFile +from django.test import SimpleTestCase, TestCase +from django.urls import Resolver404, resolve, reverse +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.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 user_tasks.models import UserTaskArtifact, UserTaskStatus + +from cms.djangoapps.contentstore.rest_api.v1.course_optimizer_service import ( + LINK_CHECK_ERROR_MESSAGE, + LINK_NOT_UPDATED_MESSAGE, + PREVIOUS_RUN_LINK_UPDATES_DISABLED_MESSAGE, + RUN_STATES, +) +from cms.djangoapps.contentstore.rest_api.v1.views.course_optimizer import ( + CourseLinkCheckViewSet, + CoursePreviousRunLinkUpdateViewSet, +) +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.tests.helpers import resolve_ref +from cms.djangoapps.contentstore.tasks import CourseLinkCheckTask, CourseLinkUpdateTask, LinkState +from cms.djangoapps.contentstore.tests.utils import routed_actions +from cms.djangoapps.contentstore.toggles import ENABLE_COURSE_OPTIMIZER_CHECK_PREV_RUN_LINKS +from cms.djangoapps.contentstore.views.permissions import CanEditCourseContent, CanStartLinkCheck, 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 LEGACY_PERMISSION_HANDLER_MAP, LegacyAuthoringPermission +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +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 BlockFactory, CourseFactory, LibraryFactory + +LINK_CHECK = "authoring_v1:course_link_check" +LINK_UPDATE = "authoring_v1:course_previous_run_link_update" +LEGACY_NAMESPACE = "cms.djangoapps.contentstore:v0" +LEGACY_VIEWS = "cms.djangoapps.contentstore.rest_api.v0.views.course_optimizer" +SERVICE = "cms.djangoapps.contentstore.rest_api.v1.course_optimizer_service" +FULL = {"view": "full"} + + +def previous_run_links(active): + """Turn updating links to a previous run on or off for every course, as a decorator or a context.""" + return override_waffle_flag(ENABLE_COURSE_OPTIMIZER_CHECK_PREV_RUN_LINKS, active=active) + + +# Each operation as (HTTP method, v1 URL name, v0 URL name, default body, v1 success status). +OPERATIONS = { + "start_check": ("post", LINK_CHECK, "link_check", None, 202), + "read_check": ("get", LINK_CHECK, "link_check_status", None, 200), + "start_update": ("post", LINK_UPDATE, "rerun_link_update", {"action": "all"}, 202), + "read_update": ("get", LINK_UPDATE, "rerun_link_update_status", None, 200), +} +STARTS = ("start_check", "start_update") +TASK_OF = {"start_check": "check_broken_links", "start_update": "update_course_rerun_links"} +# What each operation answers a caller who has no run of the course. +EMPTY_BODIES = { + "start_check": {"status": "Pending"}, + "read_check": {"status": "Uninitiated", "created_at": None, "error": None}, + "start_update": {"status": "Pending"}, + "read_update": {"status": "Uninitiated"}, +} +LEGACY_EMPTY_BODIES = { + "start_check": {"LinkCheckStatus": "Pending"}, + "read_check": {"LinkCheckStatus": "Uninitiated"}, + "start_update": {"status": "Pending"}, + "read_update": {"status": "uninitiated", "results": []}, +} +# The states a link check reports; a link update also reports Scanning and Updating. +LINK_CHECK_STATES = [ + UserTaskStatus.PENDING, + UserTaskStatus.IN_PROGRESS, + UserTaskStatus.SUCCEEDED, + UserTaskStatus.FAILED, + UserTaskStatus.CANCELED, + UserTaskStatus.RETRYING, +] +LINK_UPDATE_STATES = [*LINK_CHECK_STATES, "Scanning", "Updating"] +# Text a failed run records, standing for an exception's message. +RECORDED_FAILURE = "{'raw_error_msg': 'connection to 10.0.0.7:5432 refused'}" +_DEFAULT = object() + + +def url(name, course_key): + """Return the v1 address ``name`` for ``course_key``.""" + return reverse(name, kwargs={"course_key": str(course_key)}) + + +def legacy_url(name, course_key): + """Return the v0 address ``name`` for ``course_key``.""" + return reverse(f"{LEGACY_NAMESPACE}:{name}", kwargs={"course_id": str(course_key)}) + + +def call(client, operation, course_key, legacy=False, params=None, body=_DEFAULT): + """Perform ``operation`` on ``course_key`` at the v1 address, or the v0 one with ``legacy``.""" + method, name, legacy_name, default_body, _ = OPERATIONS[operation] + address = legacy_url(legacy_name, course_key) if legacy else url(name, course_key) + if method in ("get", "head"): + return getattr(client, method)(address, params or {}) + data = default_body if body is _DEFAULT else body + if data is None: + return client.post(address) + return client.post(address, data, format="json") + + +def assert_feature_off(response): + """Assert ``response`` is the 404 answered where updating links to a previous run is not enabled.""" + envelope = assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + assert envelope["detail"] == PREVIOUS_RUN_LINK_UPDATES_DISABLED_MESSAGE + return envelope + + +def assert_invalid(response): + """Assert ``response`` is the 400 validation envelope, and return its ``errors`` member.""" + return assert_error_envelope(response, expected_status=400, expected_type_slug="validation")["errors"] + + +def record_link_check(user, course_key, state, links=None, error=None, created=None): + """Store a link check of ``course_key`` started by ``user``, as the task records one.""" + run = UserTaskStatus.objects.create( + user=user, + task_id=str(uuid.uuid4()), + task_class="cms.djangoapps.contentstore.tasks.check_broken_links", + name=CourseLinkCheckTask.generate_name({"course_key_string": str(course_key)}), + total_steps=1, + state=state, + ) + if links is not None: + UserTaskArtifact.objects.create( + status=run, name="BrokenLinks", file=ContentFile(json.dumps(links).encode(), name="links.json"), + ) + if error is not None: + UserTaskArtifact.objects.create(status=run, name="Error", text=error) + if created is not None: + UserTaskStatus.objects.filter(pk=run.pk).update(created=created) + return run + + +def record_link_update(user, course_key, state, results=None, created=None): + """Store an update of links to the previous run of ``course_key`` started by ``user``.""" + run = UserTaskStatus.objects.create( + user=user, + task_id=str(uuid.uuid4()), + task_class="cms.djangoapps.contentstore.tasks.update_course_rerun_links", + name=CourseLinkUpdateTask.generate_name({"course_id": str(course_key)}), + total_steps=2, + state=state, + ) + if results is not None: + UserTaskArtifact.objects.create( + status=run, name="LinkUpdateResults", file=ContentFile(json.dumps(results).encode(), name="results.json"), + ) + if created is not None: + UserTaskStatus.objects.filter(pk=run.pk).update(created=created) + return run + + +def _hours_ago(hours): + return datetime.now(UTC) - timedelta(hours=hours) + + +# v0 member names and the v1 names that replace them. +LEGACY_MEMBER_NAMES = { + "LinkCheckStatus": "status", + "LinkCheckCreatedAt": "created_at", + "LinkCheckOutput": "output", + "LinkCheckError": "error", + "displayName": "display_name", + "brokenLinks": "broken_links", + "lockedLinks": "locked_links", + "externalForbiddenLinks": "external_forbidden_links", + "previousRunLinks": "previous_run_links", + "originalLink": "original_link", + "isUpdated": "is_updated", + "updatedLink": "updated_link", +} + + +def in_v1_names(body): + """``body`` with every v0 member name replaced by its v1 name, at every depth.""" + if isinstance(body, dict): + return {LEGACY_MEMBER_NAMES.get(key, key): in_v1_names(value) for key, value in body.items()} + if isinstance(body, list): + return [in_v1_names(item) for item in body] + return body + + +def member_names(body): + """Every member name in ``body``, at every depth.""" + if isinstance(body, dict): + return set(body) | {name for value in body.values() for name in member_names(value)} + if isinstance(body, list): + return {name for item in body for name in member_names(item)} + return set() + + +def test_v0_names_are_translated_at_every_depth(): + body = {"LinkCheckOutput": {"sections": [{"displayName": "A", "x": [{"previousRunLinks": [{"isUpdated": True}]}]}]}} + assert in_v1_names(body) == { + "output": {"sections": [{"display_name": "A", "x": [{"previous_run_links": [{"is_updated": True}]}]}]}, + } + assert member_names(in_v1_names(body)) == {"output", "sections", "display_name", "x", "previous_run_links", + "is_updated"} + + +# --------------------------------------------------------------------------- +# Who may call the endpoints +# --------------------------------------------------------------------------- + +class _PatchedTasksMixin: + """Replace the link check and link update tasks with mocks, on both versions.""" + + def setUp(self): + super().setUp() + self.tasks = {} + for version, module in (("v1", SERVICE), ("v0", LEGACY_VIEWS)): + for task in TASK_OF.values(): + patcher = patch(f"{module}.{task}") + self.tasks[(version, task)] = patcher.start() + self.addCleanup(patcher.stop) + + def delay(self, version, operation): + """The mocked ``delay`` of the task ``operation`` queues on ``version``.""" + return self.tasks[(version, TASK_OF[operation])].delay + + def reset_tasks(self): + """Forget every call the task mocks have recorded.""" + for task in self.tasks.values(): + task.reset_mock() + + +class _AccessMixin(_PatchedTasksMixin): + """ + Checks a caller's status on an operation against the expected one, that a + run is queued exactly when the start is accepted, and that v0 decides the + same for the same caller and queues the same run. + """ + + def assert_access(self, user, operation, expected, course_key=None): + """Assert ``expected`` for ``operation`` by ``user`` on v1, and agreement with v0.""" + course_key = course_key or self.course.id + self.reset_tasks() + client = api_client(user) + response = call(client, operation, course_key) + assert response.status_code == expected, (operation, response.content) + if expected in (200, 202): + assert response.json() == EMPTY_BODIES[operation] + else: + assert_refused(response, expected) + if operation in STARTS: + delay = self.delay("v1", operation) + if expected == 202: + delay.assert_called_once() + assert delay.call_args.args[:2] == (user.id, str(course_key)) + else: + delay.assert_not_called() + if expected == 404 and operation not in STARTS: + # v0 does not look the course up on reads; ResponseDifferencesTest covers them. + return + legacy = call(client, operation, course_key, legacy=True) + legacy_expected = 200 if expected == 202 else expected + assert legacy.status_code == legacy_expected, f"v0 decided {legacy.status_code} for {operation}" + if expected in (200, 202): + assert legacy.json() == LEGACY_EMPTY_BODIES[operation] + if operation in STARTS: + assert self.delay("v0", operation).call_args == self.delay("v1", operation).call_args + + +@ddt.ddt +@previous_run_links(True) +class CourseOptimizerAccessLegacyPermissionsTest(_AccessMixin, SharedModuleStoreTestCase): + """Who may start and read runs 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(*OPERATIONS) + def test_anonymous_caller_is_refused_with_401(self, operation): + self.assert_access(None, operation, 401) + + @ddt.data(*OPERATIONS) + def test_anonymous_caller_of_an_unknown_course_is_refused_with_401(self, operation): + self.assert_access(None, operation, 401, course_key=UNKNOWN_COURSE_KEY) + + @ddt.data(*OPERATIONS) + def test_user_without_a_role_is_refused_with_403(self, operation): + self.assert_access(UserFactory(), operation, 403) + + @ddt.data(*OPERATIONS) + def test_unknown_course_is_404_before_permission_is_checked(self, operation): + self.assert_access(UserFactory(), operation, 404, course_key=UNKNOWN_COURSE_KEY) + + @ddt.data(*OPERATIONS) + def test_limited_staff_is_refused(self, operation): + self.assert_access(self._user_with(CourseLimitedStaffRole(self.course.id)), operation, 403) + + @ddt.data(*OPERATIONS) + def test_staff_and_instructors_of_another_course_are_refused(self, operation): + self.assert_access(self._user_with(CourseStaffRole(self.other_course.id)), operation, 403) + self.assert_access(self._user_with(CourseInstructorRole(self.other_course.id)), operation, 403) + + @ddt.data(*OPERATIONS) + def test_course_staff_is_allowed(self, operation): + self.assert_access(self._user_with(CourseStaffRole(self.course.id)), operation, OPERATIONS[operation][4]) + + @ddt.data(*OPERATIONS) + def test_course_instructor_is_allowed(self, operation): + user = self._user_with(CourseInstructorRole(self.course.id)) + self.assert_access(user, operation, OPERATIONS[operation][4]) + + @ddt.data(*OPERATIONS) + def test_org_staff_and_org_instructor_are_allowed(self, operation): + expected = OPERATIONS[operation][4] + self.assert_access(self._user_with(OrgStaffRole(self.course.id.org)), operation, expected) + self.assert_access(self._user_with(OrgInstructorRole(self.course.id.org)), operation, expected) + + @ddt.data(*OPERATIONS) + def test_global_staff_is_allowed(self, operation): + self.assert_access(UserFactory(is_staff=True), operation, OPERATIONS[operation][4]) + + @ddt.data(*OPERATIONS) + def test_a_superuser_who_is_not_global_staff_is_allowed_as_on_v0(self, operation): + """The course role lookup consults the permission library, which admits superusers, on v1 as on v0.""" + self.assert_access(UserFactory(is_superuser=True, is_staff=False), operation, OPERATIONS[operation][4]) + + @ddt.data(*OPERATIONS) + def test_a_version_only_course_key_is_404(self, operation): + """The key parses but names no course run; v0 has no address that takes it.""" + response = call(api_client(UserFactory(is_staff=True)), operation, VERSION_ONLY_KEY) + assert_refused(response, 404) + + @ddt.data(*OPERATIONS) + def test_a_library_is_not_a_course(self, operation): + library_key = LibraryFactory.create().location.library_key + assert_refused(call(api_client(UserFactory(is_staff=True)), operation, library_key), 404) + + def test_starting_a_check_needs_only_read_access_and_the_rest_write_access(self): + """ + With Studio write access refused and read access kept, a check may be + started but not read, and nothing about link updates is allowed; v0 + decides the same. + """ + staff = self._user_with(CourseStaffRole(self.course.id)) + refuse_write = {LegacyAuthoringPermission.WRITE: lambda *_: False} + with patch.dict(LEGACY_PERMISSION_HANDLER_MAP, refuse_write): + self.assert_access(staff, "start_check", 202) + for operation in ("read_check", "start_update", "read_update"): + self.assert_access(staff, operation, 403) + + def test_starting_a_check_is_refused_without_read_access(self): + staff = self._user_with(CourseStaffRole(self.course.id)) + refuse_read = {LegacyAuthoringPermission.READ: lambda *_: False} + with patch.dict(LEGACY_PERMISSION_HANDLER_MAP, refuse_read): + self.assert_access(staff, "start_check", 403) + for operation in ("read_check", "start_update", "read_update"): + self.assert_access(staff, operation, OPERATIONS[operation][4]) + + +@ddt.ddt +@previous_run_links(True) +class CourseOptimizerAccessAuthzTest(CourseAuthoringAuthzTestMixin, _AccessMixin, SharedModuleStoreTestCase): + """ + Who may start and read runs when policy-based authoring permissions are + on: every operation needs the permission to edit course content. + """ + + @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(*OPERATIONS) + def test_anonymous_caller_is_refused_with_401(self, operation): + self.assert_access(None, operation, 401) + + @ddt.data(*OPERATIONS) + def test_user_without_a_role_is_refused_with_403(self, operation): + self.assert_access(self.unauthorized_user, operation, 403) + + @ddt.data(*OPERATIONS) + def test_unknown_course_is_404_before_permission_is_checked(self, operation): + self.assert_access(self.unauthorized_user, operation, 404, course_key=UNKNOWN_COURSE_KEY) + + @ddt.data(*[(operation, role) for operation in OPERATIONS for role in ( + COURSE_EDITOR.external_key, COURSE_STAFF.external_key, COURSE_ADMIN.external_key, + )]) + @ddt.unpack + def test_roles_that_may_edit_content_are_allowed(self, operation, role): + self.assert_access(self._user_with(role), operation, OPERATIONS[operation][4]) + + @ddt.data(*[(operation, role) for operation in OPERATIONS for role in ( + COURSE_AUDITOR.external_key, + COURSE_LIMITED_STAFF.external_key, + COURSE_DATA_RESEARCHER.external_key, + COURSE_BETA_TESTER.external_key, + )]) + @ddt.unpack + def test_roles_that_may_not_edit_content_are_refused(self, operation, role): + """An auditor may view the course, but starting a check needs the editing permission too.""" + self.assert_access(self._user_with(role), operation, 403) + + @ddt.data(*OPERATIONS) + def test_a_role_on_another_course_is_refused(self, operation): + user = self._user_with(COURSE_STAFF.external_key, self.other_course.id) + self.assert_access(user, operation, OPERATIONS[operation][4], course_key=self.other_course.id) + self.assert_access(user, operation, 403) + + @ddt.data(*OPERATIONS) + def test_legacy_course_staff_without_a_policy_role_is_refused(self, operation): + user = UserFactory() + CourseAccessRole.objects.create(user=user, role="staff", course_id=self.course.id, org=self.course.id.org) + self.assert_access(user, operation, 403) + + @ddt.data(*OPERATIONS) + def test_staff_and_superuser_are_allowed_without_a_role(self, operation): + self.assert_access(self.staff_user, operation, OPERATIONS[operation][4]) + self.assert_access(self.super_user, operation, OPERATIONS[operation][4]) + + 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_EDITOR.external_key) + client = session_client(user) + user.is_active = False + user.save() + for operation in OPERATIONS: + assert call(client, operation, self.course.id).status_code == OPERATIONS[operation][4], operation + assert call(client, operation, self.course.id, legacy=True).status_code == 200, operation + + +class _FeatureOffMixin(_PatchedTasksMixin): + """ + Where updating links to a previous run is not enabled, both link update + operations answer 404 to a caller who may use them, and nothing is + queued; a caller who may not is still refused with 403. v0 answers 400 + and 403. Link checks are not affected. + """ + + def editor(self): + """Return a user who may edit the course's content.""" + raise NotImplementedError + + def outsider(self): + """Return a signed-in user with no access to the course.""" + raise NotImplementedError + + def test_link_updates_are_404_for_a_caller_who_may_use_them(self): + client = api_client(self.editor()) + for operation in ("start_update", "read_update"): + response = call(client, operation, self.course.id) + assert_feature_off(response) + legacy = call(client, operation, self.course.id, legacy=True) + assert legacy.status_code == 400 + assert legacy.json() == {"error": "Course optimizer check for previous run links is not enabled."} + self.delay("v1", "start_update").assert_not_called() + self.delay("v0", "start_update").assert_not_called() + + def test_the_feature_is_checked_before_the_body(self): + response = call(api_client(self.editor()), "start_update", self.course.id, body={"action": "neither"}) + assert_feature_off(response) + + def test_a_caller_who_may_not_use_them_is_still_refused_with_403(self): + client = api_client(self.outsider()) + for operation in ("start_update", "read_update"): + assert_refused(call(client, operation, self.course.id), 403) + assert call(client, operation, self.course.id, legacy=True).status_code == 403 + + def test_link_checks_are_not_affected(self): + client = api_client(self.editor()) + assert call(client, "start_check", self.course.id).status_code == 202 + self.delay("v1", "start_check").assert_called_once() + assert call(client, "read_check", self.course.id).json() == EMPTY_BODIES["read_check"] + + +@previous_run_links(False) +class CourseOptimizerFeatureOffLegacyPermissionsTest(_FeatureOffMixin, SharedModuleStoreTestCase): + """The feature off, with policy-based authoring permissions off.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def editor(self): + user = UserFactory() + CourseStaffRole(self.course.id).add_users(user) + return user + + def outsider(self): + return UserFactory() + + def test_instructors_and_global_staff_get_404_too(self): + instructor = UserFactory() + CourseInstructorRole(self.course.id).add_users(instructor) + for user in (instructor, UserFactory(is_staff=True)): + for operation in ("start_update", "read_update"): + response = call(api_client(user), operation, self.course.id) + assert_feature_off(response) + self.delay("v1", "start_update").assert_not_called() + + +@previous_run_links(False) +class CourseOptimizerFeatureOffAuthzTest(CourseAuthoringAuthzTestMixin, _FeatureOffMixin, SharedModuleStoreTestCase): + """The feature off, with policy-based authoring permissions on.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def editor(self): + user = UserFactory() + self.add_user_to_role_in_course(user, COURSE_EDITOR.external_key, self.course.id) + return user + + def outsider(self): + return self.unauthorized_user + + +class CourseOptimizerPermissionWiringTest(SimpleTestCase): + """Which permission each action checks, and which access each permission asks for.""" + + def test_each_action_checks_the_course_then_its_own_permission(self): + for viewset, action, expected in ( + (CourseLinkCheckViewSet, "create", [IsAuthenticated, IsExistingCourse, CanStartLinkCheck]), + (CourseLinkCheckViewSet, "retrieve", [IsAuthenticated, IsExistingCourse, CanEditCourseContent]), + (CoursePreviousRunLinkUpdateViewSet, "create", [IsAuthenticated, IsExistingCourse, CanEditCourseContent]), + (CoursePreviousRunLinkUpdateViewSet, "retrieve", [IsAuthenticated, IsExistingCourse, CanEditCourseContent]), + (CourseLinkCheckViewSet, None, [IsAuthenticated, IsExistingCourse]), + (CoursePreviousRunLinkUpdateViewSet, "metadata", [IsAuthenticated, IsExistingCourse]), + ): + view = viewset() + view.action = action + # The JWT middleware adds NotJwtRestrictedApplication to JWT views on their first request. + got = [type(p) for p in view.get_permissions() if type(p).__name__ != "NotJwtRestrictedApplication"] + assert got == expected, (viewset, action) + + def test_an_action_without_a_declared_permission_is_refused(self): + for viewset in (CourseLinkCheckViewSet, CoursePreviousRunLinkUpdateViewSet): + view = viewset() + for action in ("destroy", "update", "partial_update", "list"): + 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(CourseLinkCheckViewSet): + permission_classes = CourseLinkCheckViewSet.permission_classes + + def destroy(self, request, course_key): # pylint: disable=unused-argument + raise AssertionError("the handler ran without a permission check") + + course_key = "course-v1:edX+Wiring+Run" + view = Unchecked.as_view({"delete": "destroy"}) + request = APIRequestFactory().delete(url(LINK_CHECK, 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_permissions_ask_for_the_content_editing_permission_with_read_or_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.user_has_course_permission" + for permission, access in ( + (CanStartLinkCheck, LegacyAuthoringPermission.READ), + (CanEditCourseContent, 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, "courses.edit_course_content", course_key, access) + + def test_the_permissions_refuse_a_view_without_a_course(self): + request = SimpleNamespace(user=object()) + view = SimpleNamespace(kwargs={}) + for permission in (CanStartLinkCheck, CanEditCourseContent): + assert permission().has_permission(request, view) is False + + +@previous_run_links(True) +class CourseOptimizerAuthenticationTest(_PatchedTasksMixin, 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_start_and_read(self): + user = self._course_staff() + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"JWT {create_jwt_for_user(user)}") + for operation in OPERATIONS: + response = call(client, operation, self.course.id) + assert response.status_code == OPERATIONS[operation][4], (operation, response.content) + self.delay("v1", "start_check").assert_called_once() + assert self.delay("v1", "start_check").call_args.args[0] == user.id + + def test_a_session_caller_may_start_and_read(self): + user = self._course_staff() + client = session_client(user) + for operation in OPERATIONS: + assert call(client, operation, self.course.id).status_code == OPERATIONS[operation][4], operation + assert self.delay("v1", "start_update").call_args.args[0] == user.id + + 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, allowed in ((UserFactory(is_staff=True), True), (self._course_staff(), False)): + client = session_client(user) + user.is_active = False + user.save() + for operation in OPERATIONS: + expected = OPERATIONS[operation][4] if allowed else 403 + response = call(client, operation, self.course.id) + assert response.status_code == expected, (operation, response.content) + if not allowed: + assert_refused(response, 403) + legacy_expected = 200 if allowed else 403 + assert call(client, operation, self.course.id, legacy=True).status_code == legacy_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(LINK_CHECK, self.course.id), **header), 401) + assert_refused(APIClient().post(url(LINK_CHECK, self.course.id), **header), 401) + self.delay("v1", "start_check").assert_not_called() + assert APIClient().get(legacy_url("link_check_status", self.course.id), **header).status_code == 200 + + +# --------------------------------------------------------------------------- +# Whose runs a caller sees +# --------------------------------------------------------------------------- + +class _VisibilityMixin: + """ + A read returns the latest run of the course started by the caller, never + another user's; a superuser sees the latest run of anyone. v0 shows the + same run. + """ + + def editor(self): + """Return a new user who may start and read runs of the course.""" + raise NotImplementedError + + def superuser(self): + """Return a superuser who may read runs of the course.""" + raise NotImplementedError + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.author, self.other = self.editor(), self.editor() + + def read_status(self, user, operation): + """The state ``user`` reads on v1, after asserting v0 reads the same one.""" + client = api_client(user) + new = call(client, operation, self.course.id) + legacy = call(client, operation, self.course.id, legacy=True) + assert new.status_code == legacy.status_code == 200, (new.content, legacy.content) + state = new.json()["status"] + legacy_body = in_v1_names(legacy.json()) + legacy_state = "Uninitiated" if legacy_body["status"] == "uninitiated" else legacy_body["status"] + assert legacy_state == state + return state + + def _record(self, operation, user, state, created, course_key=None): + course_key = course_key or self.course.id + if operation == "read_check": + return record_link_check(user, course_key, state, created=created) + return record_link_update(user, course_key, state, created=created) + + def _assert_visibility(self, operation): + self._record(operation, self.other, UserTaskStatus.FAILED, created=_hours_ago(2)) + self._record(operation, self.author, UserTaskStatus.IN_PROGRESS, created=_hours_ago(1)) + assert self.read_status(self.author, operation) == UserTaskStatus.IN_PROGRESS + assert self.read_status(self.other, operation) == UserTaskStatus.FAILED + assert self.read_status(self.editor(), operation) == "Uninitiated" + assert self.read_status(self.superuser(), operation) == UserTaskStatus.IN_PROGRESS + + def test_each_caller_sees_only_their_own_link_checks(self): + self._assert_visibility("read_check") + + def test_each_caller_sees_only_their_own_link_updates(self): + self._assert_visibility("read_update") + + def test_a_run_of_another_course_is_not_seen(self): + other_course = CourseFactory.create() + for operation in ("read_check", "read_update"): + self._record(operation, self.author, UserTaskStatus.SUCCEEDED, _hours_ago(1), other_course.id) + assert call(api_client(self.author), operation, self.course.id).json()["status"] == "Uninitiated" + + def test_the_latest_of_the_callers_runs_is_returned(self): + for operation in ("read_check", "read_update"): + self._record(operation, self.author, UserTaskStatus.FAILED, created=_hours_ago(1)) + self._record(operation, self.author, UserTaskStatus.PENDING, created=_hours_ago(3)) + assert self.read_status(self.author, operation) == UserTaskStatus.FAILED + + +@previous_run_links(True) +class CourseOptimizerVisibilityLegacyPermissionsTest(_VisibilityMixin, ModuleStoreTestCase): + """Visibility with policy-based authoring permissions off.""" + + def editor(self): + user = UserFactory() + CourseStaffRole(self.course.id).add_users(user) + return user + + def superuser(self): + return UserFactory(is_superuser=True, is_staff=False) + + def test_global_staff_who_is_not_a_superuser_sees_only_their_own_runs(self): + for operation in ("read_check", "read_update"): + self._record(operation, self.author, UserTaskStatus.IN_PROGRESS, created=_hours_ago(1)) + assert self.read_status(UserFactory(is_staff=True), operation) == "Uninitiated" + + +@previous_run_links(True) +class CourseOptimizerVisibilityAuthzTest(_VisibilityMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase): + """Visibility with policy-based authoring permissions on: a superuser needs no role.""" + + def editor(self): + user = UserFactory() + self.add_user_to_role_in_course(user, COURSE_EDITOR.external_key, self.course.id) + return user + + def superuser(self): + return self.super_user + + +# --------------------------------------------------------------------------- +# What reads return +# --------------------------------------------------------------------------- + +def _reported_blocks(report): + """Every component reported under the sections of a link check report.""" + for section in report["sections"]: + for subsection in section["subsections"]: + for unit in subsection["units"]: + yield from unit["blocks"] + + +@ddt.ddt +@previous_run_links(True) +class CourseOptimizerReadTest(ModuleStoreTestCase): + """What a read returns, by state and by view.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.editor = UserFactory() + CourseStaffRole(self.course.id).add_users(self.editor) + self.client = api_client(self.editor) + + def read(self, operation, legacy=False, **params): + response = call(self.client, operation, self.course.id, legacy=legacy, params=params) + assert response.status_code == 200, response.content + return response.json() + + def _course_with_flagged_links(self): + """Give the course one component and record a successful check that flagged four links in it.""" + chapter = BlockFactory.create(parent_location=self.course.location, category="chapter", display_name="Week 1") + sequential = BlockFactory.create(parent_location=chapter.location, category="sequential", display_name="Lesson") + vertical = BlockFactory.create(parent_location=sequential.location, category="vertical", display_name="Unit") + html = BlockFactory.create(parent_location=vertical.location, category="html", display_name="Reading") + block_id = str(html.location) + links = [ + [block_id, "https://broken.example.com/page", LinkState.BROKEN], + [block_id, "/static/handout.pdf", LinkState.LOCKED], + [block_id, "https://forbidden.example.com/", LinkState.EXTERNAL_FORBIDDEN], + [block_id, "https://studio.example.com/course/course-v1:Old+Run+2025/info", LinkState.PREVIOUS_RUN], + ] + record_link_check(self.editor, self.course.id, UserTaskStatus.SUCCEEDED, links=links) + return chapter, sequential, vertical, html + + @ddt.data(*LINK_CHECK_STATES) + def test_a_link_check_reports_its_state_verbatim(self, state): + links = [] if state == UserTaskStatus.SUCCEEDED else None + run = record_link_check(self.editor, self.course.id, state, links=links) + body = self.read("read_check") + assert body["status"] == state + assert body["created_at"] == self.read("read_check", legacy=True)["LinkCheckCreatedAt"] + assert body["created_at"].startswith(run.created.strftime("%Y-%m-%dT%H:%M:%S")) + assert set(body) == {"status", "created_at", "error"} + + @ddt.data(*LINK_UPDATE_STATES) + def test_a_link_update_reports_its_state_verbatim(self, state): + record_link_update(self.editor, self.course.id, state) + assert self.read("read_update") == {"status": state} + assert self.read("read_update", legacy=True)["status"] == state + + def test_every_reported_state_is_published(self): + assert set(LINK_UPDATE_STATES) | {"Uninitiated"} == set(RUN_STATES) + + def test_a_failed_link_check_reports_a_fixed_message_not_the_recorded_text(self): + record_link_check(self.editor, self.course.id, UserTaskStatus.FAILED, error=RECORDED_FAILURE) + response = call(self.client, "read_check", self.course.id) + assert response.json()["error"] == LINK_CHECK_ERROR_MESSAGE + assert "10.0.0.7" not in response.content.decode() + assert self.read("read_check", legacy=True)["LinkCheckError"] == RECORDED_FAILURE + + def test_a_canceled_link_check_with_an_error_recorded_reports_the_fixed_message(self): + record_link_check(self.editor, self.course.id, UserTaskStatus.CANCELED, error=RECORDED_FAILURE) + assert self.read("read_check")["error"] == LINK_CHECK_ERROR_MESSAGE + + @ddt.data( + (UserTaskStatus.FAILED, None), + (UserTaskStatus.FAILED, ""), + (UserTaskStatus.IN_PROGRESS, RECORDED_FAILURE), + ) + @ddt.unpack + def test_no_error_is_reported_unless_a_stopped_run_recorded_one(self, state, error): + record_link_check(self.editor, self.course.id, state, error=error) + assert self.read("read_check")["error"] is None + assert "LinkCheckError" not in self.read("read_check", legacy=True) + + def test_the_full_view_reports_the_flagged_links_in_the_course(self): + chapter, sequential, vertical, html = self._course_with_flagged_links() + body = self.read("read_check", **FULL) + report = body["output"] + assert [section["display_name"] for section in report["sections"]] == ["Week 1"] + (section,) = report["sections"] + assert UsageKey.from_string(section["id"]).block_id == chapter.location.block_id + (subsection,) = section["subsections"] + assert UsageKey.from_string(subsection["id"]).block_id == sequential.location.block_id + (unit,) = subsection["units"] + assert (UsageKey.from_string(unit["id"]).block_id, unit["display_name"]) == (vertical.location.block_id, "Unit") + (block,) = unit["blocks"] + assert UsageKey.from_string(block["id"]).block_id == html.location.block_id + assert block["display_name"] == "Reading" + assert block["broken_links"] == ["https://broken.example.com/page"] + assert block["locked_links"] == ["/static/handout.pdf"] + assert block["external_forbidden_links"] == ["https://forbidden.example.com/"] + assert block["previous_run_links"] == [{ + "original_link": "https://studio.example.com/course/course-v1:Old+Run+2025/info", + "is_updated": False, + "updated_link": None, + }] + assert (report["course_updates"], report["custom_pages"]) == ([], []) + assert_matches_published_schema(body, "LinkCheck.v1") + + def test_the_full_report_is_v0s_report_in_snake_case(self): + self._course_with_flagged_links() + new = self.read("read_check", **FULL)["output"] + legacy = self.read("read_check", legacy=True)["LinkCheckOutput"] + assert new == in_v1_names(legacy) + assert list(_reported_blocks(new)) != [] + assert [name for name in member_names(new) if not re.fullmatch(r"[a-z][a-z0-9_]*", name)] == [] + + def test_the_default_view_does_not_build_the_report(self): + self._course_with_flagged_links() + with patch(f"{SERVICE}.generate_broken_links_descriptor", side_effect=AssertionError("report built")): + assert set(self.read("read_check")) == {"status", "created_at", "error"} + assert self.read("read_check", view="minimal")["status"] == UserTaskStatus.SUCCEEDED + assert call(self.client, "read_check", self.course.id, params=FULL).status_code == 500 + + @ddt.data(UserTaskStatus.PENDING, UserTaskStatus.FAILED) + def test_the_full_view_of_a_run_that_did_not_succeed_has_no_report(self, state): + record_link_check(self.editor, self.course.id, state, error=RECORDED_FAILURE) + body = self.read("read_check", **FULL) + assert body["output"] is None + assert_matches_published_schema(body, "LinkCheck.v1") + + def test_the_full_view_without_a_run(self): + assert self.read("read_check", **FULL) == {**EMPTY_BODIES["read_check"], "output": None} + assert self.read("read_update", **FULL) == {"status": "Uninitiated", "results": []} + + def _link_update_results(self): + return [ + { + "id": "1", + "type": "course_updates", + "original_url": "https://studio.example.com/course/course-v1:Old+Run+2025/info", + "new_url": f"https://studio.example.com/course/{self.course.id}/info", + "success": True, + }, + { + "id": "block-v1:Org+Course+Run+type@html+block@reading", + "type": "course_content", + "original_url": "https://studio.example.com/course/course-v1:Old+Run+2025/progress", + "new_url": "https://studio.example.com/course/course-v1:Old+Run+2025/progress", + "success": False, + "error_message": "KeyError: 'children' at /edx/app/edxapp/xblock.py:88", + }, + ] + + def test_the_full_view_reports_each_links_outcome_with_a_fixed_failure_message(self): + results = self._link_update_results() + record_link_update(self.editor, self.course.id, UserTaskStatus.SUCCEEDED, results=results) + response = call(self.client, "read_update", self.course.id, params=FULL) + body = response.json() + assert body == { + "status": UserTaskStatus.SUCCEEDED, + "results": [ + {**results[0], "error_message": None}, + {**results[1], "error_message": LINK_NOT_UPDATED_MESSAGE}, + ], + } + assert "xblock.py" not in response.content.decode() + assert_matches_published_schema(body, "PreviousRunLinkUpdate.v1") + legacy = self.read("read_update", legacy=True) + assert legacy["results"][1]["error_message"] == results[1]["error_message"] + + def test_the_default_view_of_a_link_update_is_its_state(self): + record_link_update(self.editor, self.course.id, UserTaskStatus.SUCCEEDED, results=self._link_update_results()) + assert self.read("read_update") == {"status": UserTaskStatus.SUCCEEDED} + assert self.read("read_update", view="minimal") == {"status": UserTaskStatus.SUCCEEDED} + + def test_the_default_view_of_a_link_update_does_not_read_its_results(self): + record_link_update(self.editor, self.course.id, UserTaskStatus.SUCCEEDED, results=self._link_update_results()) + with patch(f"{SERVICE}.get_course_link_update_data", side_effect=AssertionError("results read")): + assert self.read("read_update") == {"status": UserTaskStatus.SUCCEEDED} + assert call(self.client, "read_update", self.course.id, params=FULL).status_code == 500 + + def test_a_flagged_link_below_a_unit_child_fails_the_full_read_on_both_versions(self): + """ + A known gap of the report, kept as v0 has it: a link in a component + nested below a unit's child, such as one inside a library content + block, gives the report a fifth level the four-level shape cannot + hold, and the full read answers 500. The default view still works. + """ + chapter = BlockFactory.create(parent_location=self.course.location, category="chapter") + sequential = BlockFactory.create(parent_location=chapter.location, category="sequential") + vertical = BlockFactory.create(parent_location=sequential.location, category="vertical") + library_content = BlockFactory.create(parent_location=vertical.location, category="library_content") + html = BlockFactory.create(parent_location=library_content.location, category="html") + links = [[str(html.location), "https://broken.example.com/", LinkState.BROKEN]] + record_link_check(self.editor, self.course.id, UserTaskStatus.SUCCEEDED, links=links) + assert call(self.client, "read_check", self.course.id, params=FULL).status_code == 500 + assert call(self.client, "read_check", self.course.id, legacy=True).status_code == 500 + assert self.read("read_check")["status"] == UserTaskStatus.SUCCEEDED + + def test_a_link_update_that_succeeded_without_results_reports_none(self): + record_link_update(self.editor, self.course.id, UserTaskStatus.SUCCEEDED) + assert self.read("read_update", **FULL) == {"status": UserTaskStatus.SUCCEEDED, "results": []} + + @ddt.data("read_check", "read_update") + def test_an_unknown_view_is_refused(self, operation): + response = call(self.client, operation, self.course.id, params={"view": "default"}) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert envelope["errors"] == {"view": ['"default" is not a valid choice.']} + + @ddt.data("read_check", "read_update") + def test_head_answers_without_a_body(self, operation): + name = OPERATIONS[operation][1] + for params in ({}, FULL): + response = self.client.head(url(name, self.course.id), params) + assert response.status_code == 200 + assert response.content == b"" + + @ddt.data("read_check", "read_update") + def test_the_bodies_match_the_published_schema(self, operation): + component = {"read_check": "LinkCheck.v1", "read_update": "PreviousRunLinkUpdate.v1"}[operation] + for params in ({}, FULL): + assert_matches_published_schema(self.read(operation, **params), component) + + +# --------------------------------------------------------------------------- +# Starting runs +# --------------------------------------------------------------------------- + +@previous_run_links(True) +class CourseOptimizerStartTest(_PatchedTasksMixin, ModuleStoreTestCase): + """What starting a run queues, accepts and refuses.""" + + LINKS = [ + {"url": "https://studio.example.com/course/course-v1:Old+Run+2025/info", "type": "course_updates", "id": "1"}, + { + "url": "/course/course-v1:Old+Run+2025/progress", + "type": "custom_pages", + "id": "block-v1:Old+Run+2025+type@static_tab+block@syllabus", + }, + ] + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.editor = UserFactory() + CourseStaffRole(self.course.id).add_users(self.editor) + self.client = api_client(self.editor) + + def start(self, operation, body=_DEFAULT, legacy=False): + return call(self.client, operation, self.course.id, legacy=legacy, body=body) + + def test_a_link_check_is_queued_as_v0_queues_it(self): + response = self.start("start_check") + assert response.status_code == 202 + assert response.json() == {"status": UserTaskStatus.PENDING} + assert_matches_published_schema(response.json(), "CourseOptimizerQueuedRun.v1") + delay = self.delay("v1", "start_check") + delay.assert_called_once() + assert delay.call_args.args[:2] == (self.editor.id, str(self.course.id)) + assert self.start("start_check", legacy=True).status_code == 200 + assert self.delay("v0", "start_check").call_args == delay.call_args + + def test_a_link_check_ignores_its_body(self): + response = self.client.post(url(LINK_CHECK, self.course.id), data="{not json", content_type="application/json") + assert response.status_code == 202 + self.delay("v1", "start_check").assert_called_once() + + def test_every_link_is_queued_as_v0_queues_it(self): + response = self.start("start_update", {"action": "all"}) + assert response.status_code == 202 + assert response.json() == {"status": UserTaskStatus.PENDING} + delay = self.delay("v1", "start_update") + delay.assert_called_once() + assert delay.call_args.args[:4] == (self.editor.id, str(self.course.id), "all", []) + assert self.start("start_update", {"action": "all"}, legacy=True).status_code == 200 + assert self.delay("v0", "start_update").call_args == delay.call_args + + def test_the_named_links_are_queued_as_v0_queues_them(self): + body = {"action": "single", "data": self.LINKS} + assert self.start("start_update", body).status_code == 202 + delay = self.delay("v1", "start_update") + assert delay.call_args.args[:4] == (self.editor.id, str(self.course.id), "single", self.LINKS) + assert self.start("start_update", body, legacy=True).status_code == 200 + assert self.delay("v0", "start_update").call_args == delay.call_args + + def test_links_sent_with_action_all_are_not_read(self): + """Every link is changed then, so links in the body are neither validated nor queued, as on v0.""" + for links in ("not a list", [{"type": "course_updates"}], self.LINKS): + self.reset_tasks() + body = {"action": "all", "data": links} + assert self.start("start_update", body).status_code == 202, links + assert self.delay("v1", "start_update").call_args.args[2:4] == ("all", []) + assert self.start("start_update", body, legacy=True).status_code == 200, links + self.delay("v0", "start_update").assert_called_once() + + def _refused(self, body): + errors = assert_invalid(self.start("start_update", body)) + self.delay("v1", "start_update").assert_not_called() + return errors + + def test_an_action_is_required(self): + assert self._refused({}) == {"action": ["This field is required."]} + assert self.start("start_update", {}, legacy=True).status_code == 400 + + def test_an_unknown_action_is_refused(self): + assert self._refused({"action": "some"}) == {"action": ['"some" is not a valid choice.']} + + def test_single_needs_links(self): + expected = {"data": ["This field is required when action is 'single'."]} + assert self._refused({"action": "single"}) == expected + assert self._refused({"action": "single", "data": []}) == expected + assert self.start("start_update", {"action": "single"}, legacy=True).status_code == 400 + + # The error library renders the errors of each link as Python text rather + # than as an object of members. Change these to per-link objects once it + # normalizes nested errors. + + def test_each_link_needs_a_url_a_type_and_an_id(self): + errors = self._refused({"action": "single", "data": [{"type": "course_updates", "id": "1"}]}) + assert errors == {"data": ["{'url': [ErrorDetail(string='This field is required.', code='required')]}"]} + link, blank_type = self.LINKS[0], {**self.LINKS[0], "type": ""} + errors = self._refused({"action": "single", "data": [link, blank_type]}) + assert errors == { + "data": ["{}", "{'type': [ErrorDetail(string='This field may not be blank.', code='blank')]}"], + } + + def test_the_links_must_be_a_list(self): + errors = self._refused({"action": "single", "data": {"url": "x", "type": "y", "id": "z"}}) + assert errors == {"data": [ + "{'non_field_errors': [ErrorDetail(string='Expected a list of items but got type \"dict\".', " + "code='not_a_list')]}" + ]} + + def test_a_body_that_is_not_an_object_is_refused(self): + response = self.client.post(url(LINK_UPDATE, self.course.id), data="[1, 2]", content_type="application/json") + assert assert_invalid(response) == {"non_field_errors": ["Invalid data. Expected a dictionary, but got list."]} + self.delay("v1", "start_update").assert_not_called() + + +# --------------------------------------------------------------------------- +# Differences from v0 +# --------------------------------------------------------------------------- + +# Requests v0 and v1 answer differently beyond the error body and the member +# names, as (request, v0 answer, v1 answer, test in CourseOptimizerResponseDifferencesTest). +RESPONSE_DIFFERENCES = [ + ("start a link check or a link update", "200", "202", "test_starting_a_run_is_accepted"), + ("read a link check of an unknown course, as global staff", "500", "404", "test_reads_of_an_unknown_course"), + ("read a link update of an unknown course, as global staff", "200, uninitiated", "404", + "test_reads_of_an_unknown_course"), + ("read either of an unknown course, without a role", "403", "404", "test_reads_of_an_unknown_course"), + ("start or read a link update where the feature is off", "400", "404", "test_the_feature_off"), + ("start a link update naming a link with a missing or blank url, type or id", "200, queued", "400", + "test_a_link_missing_a_member"), + ("start a link update whose body is not a JSON object", "500", "400", "test_a_body_that_is_not_an_object"), + ("read with a view other than minimal or full", "200, view ignored", "400", "test_an_unknown_view"), + ("read a link check after a status was stored in the session", "the stored status", "Uninitiated", + "test_a_status_stored_in_the_session"), + ("read a link update with no run", "uninitiated, results []", "Uninitiated", "test_no_link_update_yet"), + ("start or read with a Bearer token", "200", "401", "test_a_bearer_token"), + ("slash-separated course key", "404 JSON", "404 HTML", "test_a_slash_separated_course_key"), +] + + +@previous_run_links(True) +class CourseOptimizerResponseDifferencesTest(_PatchedTasksMixin, ModuleStoreTestCase): + """Each request v0 and v1 answer differently, both answers observed.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.editor = UserFactory() + CourseStaffRole(self.course.id).add_users(self.editor) + self.client = api_client(self.editor) + + def both(self, operation, course_key=None, client=None, **kwargs): + """The v0 and v1 responses to the same request.""" + client, course_key = client or self.client, course_key or self.course.id + return call(client, operation, course_key, legacy=True, **kwargs), call(client, operation, course_key, **kwargs) + + def test_every_listed_difference_has_its_test(self): + assert [name for *_, name in RESPONSE_DIFFERENCES if not hasattr(self, name)] == [] + + def test_starting_a_run_is_accepted(self): + for operation in STARTS: + legacy, new = self.both(operation) + assert (legacy.status_code, new.status_code) == (200, 202) + + def test_reads_of_an_unknown_course(self): + """ + v0 does not look the course up on reads. For global staff its link + check read fails sorting the report by the course outline the + modulestore cannot find; its link update read reports no run. + """ + global_staff = api_client(UserFactory(is_staff=True)) + legacy, new = self.both("read_check", UNKNOWN_COURSE_KEY, client=global_staff) + assert legacy.status_code == 500 + assert_refused(new, 404) + legacy, new = self.both("read_update", UNKNOWN_COURSE_KEY, client=global_staff) + assert legacy.status_code == 200 + assert legacy.json() == {"status": "uninitiated", "results": []} + assert_refused(new, 404) + for operation in ("read_check", "read_update"): + legacy, new = self.both(operation, UNKNOWN_COURSE_KEY, client=api_client(UserFactory())) + assert legacy.status_code == 403 + assert_refused(new, 404) + + def test_the_feature_off(self): + with previous_run_links(False): + for operation in ("start_update", "read_update"): + legacy, new = self.both(operation) + assert legacy.status_code == 400 + assert_feature_off(new) + + def test_a_link_missing_a_member(self): + complete = {"url": "/course/course-v1:Old+Run+2025/info", "type": "course_updates", "id": "1"} + for link in ( + {"type": "course_updates", "id": "1"}, + {"url": complete["url"], "id": "1"}, + {"url": complete["url"], "type": "course_updates"}, + {**complete, "type": ""}, + {**complete, "id": ""}, + ): + self.reset_tasks() + legacy, new = self.both("start_update", body={"action": "single", "data": [link]}) + assert legacy.status_code == 200, link + self.delay("v0", "start_update").assert_called_once() + assert list(assert_invalid(new)) == ["data"], link + self.delay("v1", "start_update").assert_not_called() + + def test_a_body_that_is_not_an_object(self): + address = {"legacy": legacy_url("rerun_link_update", self.course.id), "new": url(LINK_UPDATE, self.course.id)} + legacy = self.client.post(address["legacy"], data="[1, 2]", content_type="application/json") + new = self.client.post(address["new"], data="[1, 2]", content_type="application/json") + assert legacy.status_code == 500 + assert_invalid(new) + + def test_an_unknown_view(self): + for operation in ("read_check", "read_update"): + legacy, new = self.both(operation, params={"view": "default"}) + assert legacy.status_code == 200, operation + assert assert_invalid(new) == {"view": ['"default" is not a valid choice.']} + + def test_a_status_stored_in_the_session(self): + client = session_client(self.editor) + session = client.session + session["link_check_status"] = {str(self.course.id): "Stored"} + session.save() + legacy, new = self.both("read_check", client=client) + assert legacy.json() == {"LinkCheckStatus": "Stored"} + assert new.json() == EMPTY_BODIES["read_check"] + + def test_no_link_update_yet(self): + legacy, new = self.both("read_update") + assert legacy.json() == {"status": "uninitiated", "results": []} + assert new.json() == {"status": "Uninitiated"} + + def test_a_bearer_token(self): + user = UserFactory(is_staff=True) + token = AccessTokenFactory(user=user, application=ApplicationFactory(user=user)).token + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}") + for operation in OPERATIONS: + legacy, new = self.both(operation, client=client) + assert legacy.status_code == 200, operation + assert_refused(new, 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.post("/api/contentstore/v0/link_check/edX/NoSuch/Course") + new = self.client.post("/api/authoring/v1/courses/edX/NoSuch/Course/link_check/") + 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 v0 +# --------------------------------------------------------------------------- + +# Members of an error body that differ between v0 and v1 for the same refused +# request. The comparison asserts nothing else differs, and that each occurs. +ENVELOPE_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 body maps each refused member to its messages"), + ("developer_message", "v0 error bodies carry developer_message; the envelope does not"), + ("error_code", "the v0 not-found body carries error_code; the envelope does not"), + ("error", "v0 refuses a link update body with {'error': ...}; the envelope replaces it"), +] +# Members of a successful body, in v1's names, that differ from v0's, each with +# the test asserting exactly where it occurs. +BODY_DIFFERENCES = [ + ("created_at", "always present, null without a run; v0 omits it", "test_reads_without_a_run"), + ("error", "always present: null, or a fixed message where v0 returns the recorded text", + "test_a_failed_link_check"), + ("output", "with view=full, null for a run that did not succeed; v0 omits it", "test_reads_without_a_run"), + ("status", "no link update yet is Uninitiated; v0 spells it uninitiated", "test_reads_without_a_run"), + ("results", "only with view=full; v0 always returns it", "test_reads_without_a_run"), + ("results[1].error_message", "a fixed message where v0 returns the recorded text", + "test_a_link_update_with_a_failed_link"), + ("results[0].error_message", "null for a link that was changed; v0 omits it", + "test_a_link_update_with_a_failed_link"), +] + + +def _occurs(path, observed): + """Whether ``path`` or a member below it is among the ``observed`` changed paths.""" + return any(key == path or key.startswith(f"{path}.") or key.startswith(f"{path}[") for key in observed) + + +def test_only_declared_differences_are_tolerated(): + assert undeclared_paths({"type", "errors.action[0]", "developer_message"}, ENVELOPE_DIFFERENCES) == set() + undeclared = undeclared_paths({"output.sections", "results[0].id"}, ENVELOPE_DIFFERENCES) + assert undeclared == {"output.sections", "results[0].id"} + + +class _ParityMixin(_PatchedTasksMixin): + """ + v1 against v0 for the same course and caller: bodies, with v0's member + names translated, differ only where a declared difference says, and + errors only by the envelope. + """ + + def editor_for(self, course): + """Return a user who may start and read runs of the 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.editor = self.editor_for(self.course) + self.observed = set() + + def pair(self, operation, client=None, course_key=None, **kwargs): + client, course_key = client or api_client(self.editor), course_key or self.course.id + return call(client, operation, course_key, legacy=True, **kwargs), call(client, operation, course_key, **kwargs) + + def changed(self, legacy, new, expected_status): + """The paths that differ between the bodies, v0's translated to v1's names, after checking statuses.""" + legacy_status = 200 if expected_status == 202 else expected_status + assert legacy.status_code == legacy_status, legacy.content + assert new.status_code == expected_status, new.content + changed = changed_paths(in_v1_names(json.loads(legacy.content or b"null")), json.loads(new.content or b"null")) + self.observed |= changed + return changed + + def test_starts_return_the_same_body(self): + for operation in STARTS: + assert self.changed(*self.pair(operation), 202) == set(), operation + + def test_reads_without_a_run(self): + assert self.changed(*self.pair("read_check"), 200) == {"created_at", "error"} + assert self.changed(*self.pair("read_check", params=FULL), 200) == {"created_at", "error", "output"} + assert self.changed(*self.pair("read_update"), 200) == {"status", "results"} + assert self.changed(*self.pair("read_update", params=FULL), 200) == {"status"} + + def test_reads_of_a_run_in_progress(self): + record_link_check(self.editor, self.course.id, UserTaskStatus.IN_PROGRESS) + record_link_update(self.editor, self.course.id, "Scanning") + assert self.changed(*self.pair("read_check"), 200) == {"error"} + assert self.changed(*self.pair("read_update"), 200) == {"results"} + assert self.changed(*self.pair("read_update", params=FULL), 200) == set() + + def test_a_failed_link_check(self): + record_link_check(self.editor, self.course.id, UserTaskStatus.FAILED, error=RECORDED_FAILURE) + assert self.changed(*self.pair("read_check"), 200) == {"error"} + + def test_a_successful_link_check_has_the_same_report(self): + record_link_check(self.editor, self.course.id, UserTaskStatus.SUCCEEDED, links=[]) + assert self.changed(*self.pair("read_check", params=FULL), 200) == {"error"} + + def test_a_link_update_with_a_failed_link(self): + results = [ + {"id": "1", "type": "course_updates", "original_url": "a", "new_url": "b", "success": True}, + {"id": "2", "type": "course_updates", "original_url": "c", "new_url": "c", "success": False, + "error_message": "Traceback internals"}, + ] + record_link_update(self.editor, self.course.id, UserTaskStatus.SUCCEEDED, results=results) + changed = self.changed(*self.pair("read_update", params=FULL), 200) + assert changed == {"results[0].error_message", "results[1].error_message"} + + def _error_pairs(self): + """``(legacy, new, status)`` for each refusal both addresses produce.""" + outsider = api_client(self.outsider()) + yield (*self.pair("read_check", client=api_client()), 401) + yield (*self.pair("start_update", client=api_client()), 401) + for operation in OPERATIONS: + yield (*self.pair(operation, client=outsider), 403) + for operation in STARTS: + yield (*self.pair(operation, client=outsider, course_key=UNKNOWN_COURSE_KEY), 404) + yield (*self.pair("start_update", body={"action": "neither"}), 400) + + def test_errors_differ_only_by_the_envelope(self): + for legacy, new, status in self._error_pairs(): + changed = self.changed(legacy, new, status) + assert undeclared_paths(changed, ENVELOPE_DIFFERENCES) == set(), (legacy.content, new.content) + + def test_every_declared_difference_occurs(self): + # In this order: the reads without a run come before any run is recorded. + names = [test for *_, test in BODY_DIFFERENCES] + ["test_errors_differ_only_by_the_envelope"] + for name in dict.fromkeys(names): + getattr(self, name)() + for path, *_ in BODY_DIFFERENCES + ENVELOPE_DIFFERENCES: + assert _occurs(path, self.observed), path + + +@previous_run_links(True) +class CourseOptimizerParityLegacyPermissionsTest(_ParityMixin, ModuleStoreTestCase): + """Parity for course staff, with policy-based authoring permissions off.""" + + def editor_for(self, course): + editor = UserFactory() + CourseStaffRole(course.id).add_users(editor) + return editor + + def outsider(self): + return UserFactory() + + +@previous_run_links(True) +class CourseOptimizerParityAuthzTest(_ParityMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase): + """Parity for a course editor role, with policy-based authoring permissions on.""" + + def editor_for(self, course): + editor = UserFactory() + self.add_user_to_role_in_course(editor, COURSE_EDITOR.external_key, course.id) + return editor + + def outsider(self): + return self.unauthorized_user + + +# --------------------------------------------------------------------------- +# Side effects of reads +# --------------------------------------------------------------------------- + +READS = [ + (operation, method, params) + for operation in ("read_check", "read_update") + for method in ("get", "head") + for params in ({}, FULL) +] + + +@contextmanager +def record_item_creations(): + """ + Record the block type of each modulestore item created inside the block. + A full link check read can create a missing course info block, so reads + are checked for creations as well as for the writes ``record_effects`` sees. + """ + created = [] + create_item = MixedModuleStore.create_item + + def spy_create_item(store, user_id, course_key, block_type, *args, **kwargs): + created.append(block_type) + return create_item(store, user_id, course_key, block_type, *args, **kwargs) + + with patch.object(MixedModuleStore, "create_item", spy_create_item): + yield created + + +class _ReadsAreSideEffectFreeMixin: + """ + Every read (both resources, both views, GET and HEAD) of runs that + succeeded writes no row and no Mongo document, creates or updates no + modulestore item 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's first read writes the same. + """ + + def editor_for(self, course): + """Return a user who may read ``course``'s runs.""" + raise NotImplementedError + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + chapter = BlockFactory.create(parent_location=self.course.location, category="chapter") + sequential = BlockFactory.create(parent_location=chapter.location, category="sequential") + vertical = BlockFactory.create(parent_location=sequential.location, category="vertical") + self.html = BlockFactory.create(parent_location=vertical.location, category="html") + + def _reader_with_runs(self): + reader = self.editor_for(self.course) + links = [[str(self.html.location), "https://broken.example.com/", LinkState.BROKEN]] + record_link_check(reader, self.course.id, UserTaskStatus.SUCCEEDED, links=links) + results = [{"id": "1", "type": "course_updates", "original_url": "a", "new_url": "b", "success": True}] + record_link_update(reader, self.course.id, UserTaskStatus.SUCCEEDED, results=results) + PolicyCacheControl.objects.all().delete() + return reader + + def test_reads_have_no_side_effects(self): + client = api_client(self._reader_with_runs()) + for index, (operation, method, params) in enumerate(READS): + with record_item_creations() as created, record_effects(self) as effects: + response = call(client, operation, self.course.id, params=params) if method == "get" else \ + client.head(url(OPERATIONS[operation][1], self.course.id), params) + read = (index, operation, method, params) + assert response.status_code == 200, (read, response.content) + if index == 0: + assert effects["sql_writes"] in ([], FIRST_LOOKUP_WRITES), read + assert effects["signals"] == (FIRST_LOOKUP_SIGNALS if effects["sql_writes"] else []), read + else: + assert effects["sql_writes"] == [], read + assert effects["signals"] == [], read + assert effects["events"] == [], read + assert effects["mongo_writes"] == 0, read + assert effects["update_item"] == [], read + assert created == [], read + + def test_the_first_v0_read_writes_what_the_first_v1_read_writes(self): + reader = self._reader_with_runs() + for operation in ("read_check", "read_update"): + seen = [] + for legacy in (True, False): + PolicyCacheControl.objects.all().delete() + with record_effects(self) as effects: + response = call(api_client(reader), operation, self.course.id, legacy=legacy) + assert response.status_code == 200, response.content + seen.append((effects["sql_writes"], effects["signals"], effects["mongo_writes"])) + assert seen[0] == seen[1], operation + + def test_the_creation_recorder_sees_an_item_created(self): + """The check above would notice a creation: one made inside the recorder is seen.""" + with record_item_creations() as created: + modulestore().create_item(self.user.id, self.course.id, "course_info", block_id="probe") + assert created == ["course_info"] + + +@previous_run_links(True) +class CourseOptimizerReadSideEffectsLegacyPermissionsTest(_ReadsAreSideEffectFreeMixin, ModuleStoreTestCase): + """Reads by course staff with policy-based authoring permissions off.""" + + def editor_for(self, course): + reader = UserFactory() + CourseStaffRole(course.id).add_users(reader) + return reader + + +@previous_run_links(True) +class CourseOptimizerReadSideEffectsAuthzTest( + _ReadsAreSideEffectFreeMixin, CourseAuthoringAuthzTestMixin, ModuleStoreTestCase, +): + """ + Reads by a course editor 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 editor_for(self, course): + reader = UserFactory() + self.add_user_to_role_in_course(reader, COURSE_EDITOR.external_key, course.id) + return reader + + +# --------------------------------------------------------------------------- +# Query counts +# --------------------------------------------------------------------------- + +class _QueryCountMixin: + """ + The default view of each read costs no more SQL queries than v0, and of a + link check fewer Mongo reads, since v0 always builds the report. The full + view costs at most one SQL query more than v0: v1 looks the course up, + which v0's reads do not. + """ + + def editor_for(self, course): + raise NotImplementedError + + def test_reads_cost_no_more_than_v0(self): + course = CourseFactory.create() + # With its overview built, the course lookup is one row read, as for a published course. + CourseOverview.get_from_id(course.id) + chapter = BlockFactory.create(parent_location=course.location, category="chapter") + sequential = BlockFactory.create(parent_location=chapter.location, category="sequential") + vertical = BlockFactory.create(parent_location=sequential.location, category="vertical") + html = BlockFactory.create(parent_location=vertical.location, category="html") + editor = self.editor_for(course) + links = [[str(html.location), "https://broken.example.com/", LinkState.BROKEN]] + record_link_check(editor, course.id, UserTaskStatus.SUCCEEDED, links=links) + results = [{"id": "1", "type": "course_updates", "original_url": "a", "new_url": "b", "success": True}] + record_link_update(editor, course.id, UserTaskStatus.SUCCEEDED, results=results) + client = api_client(editor) + for operation in ("read_check", "read_update"): + legacy = measure(lambda op=operation: call(client, op, course.id, legacy=True)) + minimal = measure(lambda op=operation: call(client, op, course.id)) + full = measure(lambda op=operation: call(client, op, course.id, params=FULL)) + assert minimal[0] <= legacy[0], (operation, minimal, legacy) + assert full[0] <= legacy[0] + 1, (operation, full, legacy) + assert minimal[1] <= full[1] <= legacy[1], (operation, minimal, full, legacy) + if operation == "read_check": + assert minimal[1] < legacy[1], (operation, minimal, legacy) + assert minimal[2] == full[2] == legacy[2] == 0, (operation, minimal, full, legacy) + + +@previous_run_links(True) +class CourseOptimizerQueryCountLegacyPermissionsTest(_QueryCountMixin, ModuleStoreTestCase): + """Costs read by course staff, with policy-based authoring permissions off.""" + + def editor_for(self, course): + editor = UserFactory() + CourseStaffRole(course.id).add_users(editor) + return editor + + +@previous_run_links(True) +class CourseOptimizerQueryCountAuthzTest(CourseAuthoringAuthzTestMixin, _QueryCountMixin, ModuleStoreTestCase): + """Costs read by a course editor, with policy-based authoring permissions on.""" + + def editor_for(self, course): + editor = UserFactory() + self.add_user_to_role_in_course(editor, COURSE_EDITOR.external_key, course.id) + return editor + + +# --------------------------------------------------------------------------- +# Addresses and methods +# --------------------------------------------------------------------------- + +COURSE_KEY = "course-v1:edX+DemoX+Demo_Course" +LEGACY_VIEW_NAMES = { + "link_check": "LinkCheckView", + "link_check_status": "LinkCheckStatusView", + "rerun_link_update": "RerunLinkUpdateView", + "rerun_link_update_status": "RerunLinkUpdateStatusView", +} + + +@ddt.ddt +@previous_run_links(True) +class CourseOptimizerRoutingTest(_PatchedTasksMixin, SharedModuleStoreTestCase): + """The addresses, the v0 addresses, and requests no route or method serves.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def test_the_addresses(self): + for name, path, viewset in ( + (LINK_CHECK, "link_check", CourseLinkCheckViewSet), + (LINK_UPDATE, "previous_run_link_update", CoursePreviousRunLinkUpdateViewSet), + ): + assert url(name, COURSE_KEY) == f"/api/authoring/v1/courses/{COURSE_KEY}/{path}/" + match = resolve(url(name, COURSE_KEY)) + assert match.func.cls is viewset + assert routed_actions(match.func) == {"get": "retrieve", "post": "create"} + + def test_the_v0_addresses_are_unchanged(self): + for name, view_name in LEGACY_VIEW_NAMES.items(): + assert legacy_url(name, COURSE_KEY) == f"/api/contentstore/v0/{name}/{COURSE_KEY}" + view = resolve(legacy_url(name, COURSE_KEY)).func.cls + assert (view.__module__, view.__name__) == (LEGACY_VIEWS, view_name) + + def test_v0_still_takes_a_slash_separated_key_that_v1_does_not(self): + view = resolve("/api/contentstore/v0/link_check/edX/DemoX/Demo_Course").func.cls + assert (view.__module__, view.__name__) == (LEGACY_VIEWS, "LinkCheckView") + for path in ("link_check", "previous_run_link_update"): + with pytest.raises(Resolver404): + resolve(f"/api/authoring/v1/courses/edX/DemoX/Demo_Course/{path}/") + + 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_link_check", "course_previous_run_link_update"} <= 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(*[(name, method) for name in (LINK_CHECK, LINK_UPDATE) for method in ("put", "patch", "delete")]) + @ddt.unpack + def test_other_methods_are_not_allowed(self, name, method): + response = getattr(api_client(UserFactory(is_staff=True)), method)(url(name, self.course.id), {}, format="json") + assert_error_envelope(response, expected_status=405, expected_type_slug="internal") + + @ddt.data(LINK_CHECK, LINK_UPDATE) + def test_unsupported_accept_is_refused_with_406(self, name): + response = api_client(UserFactory(is_staff=True)).get(url(name, 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)).post( + url(LINK_UPDATE, 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" + self.delay("v1", "start_update").assert_not_called() + + def test_an_unsupported_media_type_is_a_415_typed_internal(self): + response = api_client(UserFactory(is_staff=True)).post( + url(LINK_UPDATE, self.course.id), data="action=all", content_type="text/plain", + ) + envelope = assert_error_envelope(response, expected_status=415, expected_type_slug="internal") + assert envelope["title"] == "Internal Server Error" + self.delay("v1", "start_update").assert_not_called() + + # 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, path) for key in ("not-a-course-key", "edX/DemoX/Demo_Course", "edX+DemoX+Demo_Course") + for path in ("link_check", "previous_run_link_update")]) + @ddt.unpack + def test_an_unparsable_or_slash_form_key_is_an_html_404(self, key, path): + response = api_client(UserFactory(is_staff=True)).get(f"/api/authoring/v1/courses/{key}/{path}/") + 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}/link_check" + response = api_client(UserFactory(is_staff=True)).get(address) + assert response.status_code == 301 + assert response["Location"] == url(LINK_CHECK, self.course.id) + with pytest.raises(Resolver404): + resolve(address) + + +# --------------------------------------------------------------------------- +# The published schema +# --------------------------------------------------------------------------- + +LINK_CHECK_PATH = "/api/authoring/v1/courses/{course_key}/link_check/" +LINK_UPDATE_PATH = "/api/authoring/v1/courses/{course_key}/previous_run_link_update/" +LEGACY_PATHS = { + "/api/contentstore/v0/link_check/{course_id}": ("post", "v0_link_check_create"), + "/api/contentstore/v0/link_check_status/{course_id}": ("get", "v0_link_check_status_retrieve"), + "/api/contentstore/v0/rerun_link_update/{course_id}": ("post", "v0_rerun_link_update_create"), + "/api/contentstore/v0/rerun_link_update_status/{course_id}": ("get", "v0_rerun_link_update_status_retrieve"), +} +NEW_COMPONENTS = ( + "CourseOptimizerQueuedRun.v1", + "LinkCheck.v1", + "LinkCheckReport.v1", + "LinkCheckReportSection.v1", + "LinkCheckReportSubsection.v1", + "LinkCheckReportUnit.v1", + "LinkCheckReportBlock.v1", + "LinkCheckPreviousRunLink.v1", + "PreviousRunLinkUpdate.v1", + "LinkUpdateResult.v1", + "PreviousRunLinkUpdateRequest.v1", + "LinkToUpdate.v1", +) + + +class CourseOptimizerSchemaTest(TestCase): + """The four operations as published in the CMS authoring schema.""" + + @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] + + @staticmethod + def _enum_ref(node): + """The enum component a property refers to; a described property wraps the reference in allOf.""" + return node.get("$ref") or node["allOf"][0]["$ref"] + + def _enum(self, node): + return set(resolve_ref(self.schema, {"$ref": self._enum_ref(node)})["enum"]) + + def test_the_operations_are_published_for_the_sdk(self): + for path, name, ok in ( + (LINK_CHECK_PATH, "link_check", "LinkCheck.v1"), + (LINK_UPDATE_PATH, "previous_run_link_update", "PreviousRunLinkUpdate.v1"), + ): + operations = self.schema["paths"][path] + assert sorted(operations) == ["get", "post"] + assert operations["get"]["operationId"] == f"v1_courses_{name}_retrieve" + assert operations["post"]["operationId"] == f"v1_courses_{name}_create" + for operation in operations.values(): + assert operation["tags"] == ["openedx-platform-sdk"] + assert "deprecated" not in operation + assert self._ref(operations["get"]["responses"]["200"]) == ok + assert self._ref(operations["post"]["responses"]["202"]) == "CourseOptimizerQueuedRun.v1" + assert "200" not in operations["post"]["responses"] + for method, statuses in (("get", ("400", "401", "403", "404")), ("post", ("401", "403", "404"))): + for status in statuses: + assert self._ref(operations[method]["responses"][status]) == "ErrorResponse", (path, status) + assert self._ref(self.schema["paths"][LINK_UPDATE_PATH]["post"]["responses"]["400"]) == "ErrorResponse" + + def test_the_read_parameters(self): + for path in (LINK_CHECK_PATH, LINK_UPDATE_PATH): + parameters = {p["name"]: p for p in self.schema["paths"][path]["get"]["parameters"]} + assert set(parameters) == {"course_key", "view"} + assert parameters["course_key"]["in"] == "path" + assert parameters["course_key"]["schema"] == {"type": "string", "pattern": "^[^/]+$"} + assert parameters["view"]["in"] == "query" + assert not parameters["view"].get("required") + assert parameters["view"]["schema"] == {"type": "string", "enum": ["full", "minimal"], "default": "minimal"} + + def test_starting_a_link_check_takes_no_body(self): + assert "requestBody" not in self.schema["paths"][LINK_CHECK_PATH]["post"] + + def test_starting_a_link_update_takes_an_action_and_links(self): + body = self.schema["paths"][LINK_UPDATE_PATH]["post"]["requestBody"] + assert list(body["content"]) == ["application/json"] + assert self._ref(body) == "PreviousRunLinkUpdateRequest.v1" + request = self.components["PreviousRunLinkUpdateRequest.v1"] + assert request["required"] == ["action"] + assert self._enum(request["properties"]["action"]) == {"all", "single"} + assert request["properties"]["data"]["items"] == {"$ref": "#/components/schemas/LinkToUpdate.v1"} + assert sorted(self.components["LinkToUpdate.v1"]["required"]) == ["id", "type", "url"] + + def test_the_states_are_one_published_enum(self): + """Both reads and both starts publish the same states, as one component.""" + nodes = [ + self.components["LinkCheck.v1"]["properties"]["status"], + self.components["PreviousRunLinkUpdate.v1"]["properties"]["status"], + self.components["CourseOptimizerQueuedRun.v1"]["properties"]["status"], + ] + assert [self._enum(node) for node in nodes] == [set(RUN_STATES)] * 3 + assert {self._enum_ref(node) for node in nodes} == {"#/components/schemas/StatusEnum"} + + def test_the_minimal_and_full_members(self): + link_check = self.components["LinkCheck.v1"] + assert set(link_check["properties"]) == {"status", "created_at", "error", "output"} + assert sorted(link_check["required"]) == ["created_at", "error", "status"] + nullable = {name for name, field in link_check["properties"].items() if field.get("nullable")} + assert nullable == {"created_at", "error", "output"} + link_update = self.components["PreviousRunLinkUpdate.v1"] + assert set(link_update["properties"]) == {"status", "results"} + assert link_update["required"] == ["status"] + result = self.components["LinkUpdateResult.v1"] + nullable = {name for name, field in result["properties"].items() if field.get("nullable")} + assert nullable == {"original_url", "error_message"} + + def test_the_link_types_are_documented(self): + description = self.components["LinkToUpdate.v1"]["properties"]["type"]["description"] + for link_type in ("course_content", "course_updates", "handouts", "custom_pages"): + assert f"'{link_type}'" in description, link_type + + def test_every_member_is_described(self): + for name in NEW_COMPONENTS: + component = self.components[name] + assert component.get("description"), name + properties = component.get("properties", {}) + undescribed = [field for field, spec in properties.items() if not spec.get("description")] + assert undescribed == [], name + + def test_the_examples_are_valid_bodies(self): + content = self.schema["paths"][LINK_UPDATE_PATH]["post"]["requestBody"]["content"]["application/json"] + assert sorted(content["examples"]) == ["EveryLink", "SomeLinks"] + for example in content["examples"].values(): + assert_matches_published_schema(example["value"], "PreviousRunLinkUpdateRequest.v1") + content = self.schema["paths"][LINK_CHECK_PATH]["get"]["responses"]["200"]["content"]["application/json"] + assert sorted(content["examples"]) == ["FullReport"] + report = content["examples"]["FullReport"]["value"] + assert_matches_published_schema(report, "LinkCheck.v1") + assert set(report) == {"status", "created_at", "error", "output"} + + def test_the_v0_operations_are_deprecated_and_left_out_of_the_sdk(self): + for path, (method, operation_id) in LEGACY_PATHS.items(): + operations = self.schema["paths"][path] + assert sorted(operations) == [method], path + assert operations[method]["operationId"] == operation_id + assert operations[method]["deprecated"] is True + assert "openedx-platform-sdk" not in operations[method].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 LEGACY_PATHS.items()} <= deprecated + assert {(path, method) for path in (LINK_CHECK_PATH, LINK_UPDATE_PATH) for method in ("get", "post")} & \ + 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..3fa362fce959 100644 --- a/cms/djangoapps/contentstore/views/permissions.py +++ b/cms/djangoapps/contentstore/views/permissions.py @@ -2,14 +2,22 @@ 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_EDIT_COURSE_CONTENT, COURSES_VIEW_PAGES_AND_RESOURCES +from rest_framework.exceptions import NotFound from rest_framework.permissions import BasePermission -from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access +from common.djangoapps.student.auth import ( + check_course_advanced_settings_access, + has_studio_read_access, + has_studio_write_access, +) from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission from openedx.core.djangoapps.authz.decorators import user_has_course_permission +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api.view_utils import validate_course_key +COURSE_NOT_FOUND_MESSAGE = "The requested course does not exist." + class HasStudioWriteAccess(BasePermission): """ @@ -58,3 +66,104 @@ 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 CanStartLinkCheck(BasePermission): + """ + Allow callers who may start a check of the links in the course in the path. + + Reads the parsed ``course_key`` URL argument. On courses where + policy-based authoring permissions are enabled, only the permission to + edit course content is consulted; elsewhere Studio read access to the + course is enough, while reading a check's result needs write access + (``CanEditCourseContent``). + """ + + 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_EDIT_COURSE_CONTENT.identifier, + course_key, + LegacyAuthoringPermission.READ, + ) + + +class CanEditCourseContent(BasePermission): + """ + Allow callers who may edit the content 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_EDIT_COURSE_CONTENT.identifier, + course_key, + LegacyAuthoringPermission.WRITE, + ) diff --git a/cms/lib/spectacular.py b/cms/lib/spectacular.py index 662cf80c65f0..04d6a501d554 100644 --- a/cms/lib/spectacular.py +++ b/cms/lib/spectacular.py @@ -4,15 +4,23 @@ 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/courses/{course_key}/link_check/ + "/api/contentstore/v0/link_check/{course_id}", + "/api/contentstore/v0/link_check_status/{course_id}", + # → /api/authoring/v1/courses/{course_key}/previous_run_link_update/ + "/api/contentstore/v0/rerun_link_update/{course_id}", + "/api/contentstore/v0/rerun_link_update_status/{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..1c22feb3ccc6 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/link_check/{course_id}", + "/api/contentstore/v0/link_check_status/{course_id}", + "/api/contentstore/v0/rerun_link_update/{course_id}", + "/api/contentstore/v0/rerun_link_update_status/{course_id}", } def test_the_conforming_paths_are_left_unmarked(self): @@ -83,8 +88,38 @@ 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_course_optimizer_operations_are_marked_and_their_successors_are_not(self): + legacy = { + "/api/contentstore/v0/link_check/{course_id}": "post", + "/api/contentstore/v0/link_check_status/{course_id}": "get", + "/api/contentstore/v0/rerun_link_update/{course_id}": "post", + "/api/contentstore/v0/rerun_link_update_status/{course_id}": "get", + } + conforming = ( + "/api/authoring/v1/courses/{course_key}/link_check/", + "/api/authoring/v1/courses/{course_key}/previous_run_link_update/", + ) + result = self._run({"paths": { + **{path: {method: {}} for path, method in legacy.items()}, + **{path: {"get": {}, "post": {}} for path in conforming}, + }}) + for path, method in legacy.items(): + assert result["paths"][path] == {method: {"deprecated": True}}, path + for path in conforming: + assert result["paths"][path] == {"get": {}, "post": {}}, path + 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"]