diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/authoring_grading.py b/cms/djangoapps/contentstore/rest_api/v0/views/authoring_grading.py index 7aabd8eea6b1..94f04a2c7055 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/authoring_grading.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/authoring_grading.py @@ -21,6 +21,9 @@ class AuthoringGradingView(DeveloperErrorViewMixin, APIView): """ View for getting and setting the advanced settings for a course. + + Deprecated: use ``PATCH /api/authoring/v3/courses/{course_key}/grading/`` + instead. See https://github.com/openedx/openedx-platform/issues/39128. """ @extend_schema( request=CourseGradingModelSerializer, @@ -42,6 +45,9 @@ def post(self, request: Request, course_key: CourseKey): """ Update a course's grading. + Deprecated: use ``PATCH /api/authoring/v3/courses/{course_key}/grading/``, + which accepts this body unchanged. See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** POST /api/contentstore/v0/course_grading/{course_id} diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/grading.py b/cms/djangoapps/contentstore/rest_api/v1/views/grading.py index a460ee07f2c2..8668965d7765 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/grading.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/grading.py @@ -25,6 +25,9 @@ class CourseGradingView(DeveloperErrorViewMixin, APIView): """ View for Course Grading policy configuration. + + Deprecated: use ``GET`` and ``PATCH /api/authoring/v3/courses/{course_key}/grading/`` + instead. See https://github.com/openedx/openedx-platform/issues/39128. """ @extend_schema( @@ -44,6 +47,10 @@ def get(self, request: Request, course_key: CourseKey): """ Get an object containing course grading settings with model. + Deprecated: use ``GET /api/authoring/v3/courses/{course_key}/grading/?view=full``, + which returns the same fields with the grading policy at the top level. + See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** GET /api/contentstore/v1/course_grading/{course_id} @@ -124,6 +131,9 @@ def post(self, request: Request, course_key: CourseKey): """ Update a course's grading. + Deprecated: use ``PATCH /api/authoring/v3/courses/{course_key}/grading/``, + which accepts this body unchanged. See https://github.com/openedx/openedx-platform/issues/39128. + **Example Request** POST /api/contentstore/v1/course_grading/{course_id} diff --git a/cms/djangoapps/contentstore/rest_api/v3/authoring_urls.py b/cms/djangoapps/contentstore/rest_api/v3/authoring_urls.py index 2d1db8f88434..86dd93e580b0 100644 --- a/cms/djangoapps/contentstore/rest_api/v3/authoring_urls.py +++ b/cms/djangoapps/contentstore/rest_api/v3/authoring_urls.py @@ -29,7 +29,7 @@ ), path( "courses//grading/", - AuthoringGradingViewSet.as_view({"patch": "partial_update"}), + AuthoringGradingViewSet.as_view({"get": "retrieve", "patch": "partial_update"}), name="course_grading", ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v3/grading_service.py b/cms/djangoapps/contentstore/rest_api/v3/grading_service.py new file mode 100644 index 000000000000..d52f96faf957 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/grading_service.py @@ -0,0 +1,83 @@ +"""Service layer for the v3 course grading endpoint.""" + +from django.conf import settings + +from cms.djangoapps.contentstore.rest_api.v3.serializers.grading import GRADING_POLICY_FIELDS +from cms.djangoapps.contentstore.utils import get_course_grading +from cms.djangoapps.models.settings.course_grading import CourseGradingModel +from openedx.core.djangoapps.credit.api import is_credit_course +from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements +from xmodule.modulestore.django import modulestore + + +def get_grading_policy(course_key): + """Return the stored grading policy of the course ``course_key``.""" + return CourseGradingModel.fetch(course_key) + + +def get_full_grading_settings(course_key): + """ + Return the grading policy of ``course_key`` with the context the Studio grading page shows beside it. + + The returned mapping holds the four policy fields plus the graded subsections per + assignment type, the default letter grade names, and the credit and proctoring context. + """ + with modulestore().bulk_operations(course_key): + show_credit_eligibility = is_credit_course(course_key) and settings.ENABLE_CREDIT_ELIGIBILITY + context = get_course_grading(course_key) + policy = context["course_details"] + return { + "graders": policy.graders, + "grade_cutoffs": policy.grade_cutoffs, + "grace_period": policy.grace_period, + "minimum_grade_credit": policy.minimum_grade_credit, + "course_assignment_lists": context["course_assignment_lists"], + "default_grade_designations": context["default_grade_designations"], + "is_credit_course": context["is_credit_course"], + "show_credit_eligibility": show_credit_eligibility, + "mfe_proctored_exam_settings_url": context["mfe_proctored_exam_settings_url"], + } + + +def update_grading_policy(course_key, changes, user): + """ + Apply ``changes`` to the grading policy of ``course_key`` and return the stored policy. + + Only the fields present in ``changes`` are written. A body holding all four fields is + applied exactly as a whole-policy update. When ``graders`` is absent the stored graders + are never written back: their weights are fractions that do not survive a round trip + through the percentages the API exchanges, so rewriting them would alter weights + nobody changed. Grade cutoffs are written, and a grading policy change announced, + only when they differ from the stored ones. + + The credit requirements of the course are refreshed whenever ``minimum_grade_credit`` + is present, whether or not its value changes. + """ + if "minimum_grade_credit" in changes: + update_credit_course_requirements.delay(str(course_key)) + + if all(field in changes for field in GRADING_POLICY_FIELDS): + return CourseGradingModel.update_from_json(course_key, changes, user) + + if "graders" in changes: + # The whole-policy update needs every field. The stored cutoffs leave them + # unchanged, and None leaves the grace period and credit minimum unchanged. + stored_cutoffs = modulestore().get_course(course_key).grade_cutoffs + policy = { + "grade_cutoffs": stored_cutoffs, + "grace_period": None, + "minimum_grade_credit": None, + **changes, + } + return CourseGradingModel.update_from_json(course_key, policy, user) + + if "grace_period" in changes: + CourseGradingModel.update_grace_period_from_json(course_key, changes["grace_period"], user) + if "minimum_grade_credit" in changes: + CourseGradingModel.update_minimum_grade_credit_from_json(course_key, changes["minimum_grade_credit"], user) + # Last, so the grading policy change is announced after every other field is stored, + # as the whole-policy update does. + if "grade_cutoffs" in changes: + if modulestore().get_course(course_key).grade_cutoffs != changes["grade_cutoffs"]: + CourseGradingModel.update_cutoffs_from_json(course_key, changes["grade_cutoffs"], user) + return CourseGradingModel.fetch(course_key) diff --git a/cms/djangoapps/contentstore/rest_api/v3/permissions.py b/cms/djangoapps/contentstore/rest_api/v3/permissions.py new file mode 100644 index 000000000000..a0f38f306c48 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/permissions.py @@ -0,0 +1,45 @@ +"""Permissions for the v3 contentstore API.""" + +from openedx_authz.constants.permissions import COURSES_EDIT_GRADING_SETTINGS, COURSES_VIEW_GRADING_SETTINGS +from rest_framework.permissions import BasePermission + +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission +from openedx.core.djangoapps.authz.decorators import user_has_course_permission + + +class _GradingSettingsPermission(BasePermission): + """ + Allow callers holding ``authz_permission`` on the course in the path. + + The course comes from the view's ``get_course_key()``, which refuses an + unparsable or unknown course with a 404 before permission is checked. On + courses using policy-based authoring permissions that policy decides; + elsewhere Studio read access to the course is required. + """ + + authz_permission = None + + def has_permission(self, request, view): + return user_has_course_permission( + request.user, + self.authz_permission, + view.get_course_key(), + LegacyAuthoringPermission.READ, + ) + + +class CanViewGradingSettings(_GradingSettingsPermission): + """Allow callers who may view the grading settings of the course in the path.""" + + authz_permission = COURSES_VIEW_GRADING_SETTINGS.identifier + + +class CanEditGradingSettings(_GradingSettingsPermission): + """ + Allow callers who may edit the grading settings of the course in the path. + + Where policy-based permissions are off, Studio read access is enough to + edit, as for every other grading endpoint in Studio. + """ + + authz_permission = COURSES_EDIT_GRADING_SETTINGS.identifier diff --git a/cms/djangoapps/contentstore/rest_api/v3/serializers/__init__.py b/cms/djangoapps/contentstore/rest_api/v3/serializers/__init__.py new file mode 100644 index 000000000000..205123e14271 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/serializers/__init__.py @@ -0,0 +1 @@ +"""Serializers for the v3 contentstore API.""" diff --git a/cms/djangoapps/contentstore/rest_api/v3/serializers/grading.py b/cms/djangoapps/contentstore/rest_api/v3/serializers/grading.py new file mode 100644 index 000000000000..f5ae93462940 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/serializers/grading.py @@ -0,0 +1,449 @@ +"""Serializers for a course's grading policy.""" + +import math +from collections.abc import Mapping +from datetime import timedelta + +from django.core.validators import MinLengthValidator +from drf_spectacular.types import OpenApiTypes +from drf_spectacular.utils import extend_schema_field +from rest_framework import serializers +from rest_framework.exceptions import ValidationError +from rest_framework.settings import api_settings +from rest_framework.utils import html + +GRADING_FULL_VIEW = "full" + + +class GraderSerializer(serializers.Serializer): # pylint: disable=abstract-method + """One assignment type of a course's grading policy.""" + + type = serializers.CharField( + allow_null=True, + help_text="Name of the assignment type, for example Homework, or null when the policy stores none.", + ) + min_count = serializers.IntegerField(help_text="Number of assignments of this type the course expects.") + drop_count = serializers.IntegerField( + help_text="Number of lowest-scoring assignments of this type left out of the grade.", + ) + short_label = serializers.CharField( + allow_null=True, + allow_blank=True, + help_text="Abbreviation shown with the assignment type, for example HW.", + ) + weight = serializers.IntegerField( + help_text=( + "Share of the course grade, in percent. The stored fraction is multiplied by 100 " + "and truncated to a whole number." + ), + ) + id = serializers.IntegerField(help_text="Position of the assignment type in the policy, starting at 0.") + + class Meta: + ref_name = "authoring_grading.Grader.v3" + + +class GracePeriodSerializer(serializers.Serializer): # pylint: disable=abstract-method + """How long after a due date a submission is still accepted.""" + + hours = serializers.IntegerField(help_text="Whole hours, with whole days counted as 24 hours.") + minutes = serializers.IntegerField(help_text="Minutes beyond the whole hours.") + seconds = serializers.IntegerField(help_text="Seconds beyond the whole minutes.") + + class Meta: + ref_name = "authoring_grading.GracePeriod.v3" + + +class CourseGradingSerializer(serializers.Serializer): # pylint: disable=abstract-method + """The grading policy of a course.""" + + graders = GraderSerializer( + many=True, + allow_empty=True, + help_text="The assignment types, in stored order.", + ) + grade_cutoffs = serializers.DictField( + child=serializers.FloatField(allow_null=True), + allow_null=True, + help_text=( + "Lowest score, from 0 to 1, that earns each letter grade, keyed by the grade's name. " + "The cutoffs, or a single cutoff, are null when the policy was stored that way." + ), + ) + grace_period = GracePeriodSerializer( + allow_null=True, + help_text="Grace period after each due date, or null when the course has none.", + ) + minimum_grade_credit = serializers.FloatField( + allow_null=True, + help_text="Lowest course score, from 0 to 1, that earns course credit, or null when none is set.", + ) + + class Meta: + ref_name = "authoring_grading.CourseGrading.v3" + + +class CourseGradingFullSerializer(CourseGradingSerializer): # pylint: disable=abstract-method + """The grading policy of a course, with the context the Studio grading page shows beside it.""" + + course_assignment_lists = serializers.DictField( + child=serializers.ListField(child=serializers.CharField()), + help_text=( + "For each assignment type in use, its graded subsections as '
- '. " + "Present only with view=full." + ), + ) + default_grade_designations = serializers.ListField( + child=serializers.CharField(), + help_text="Letter grade names offered for new grade cutoffs. Present only with view=full.", + ) + is_credit_course = serializers.BooleanField( + help_text="Whether the course offers course credit. Present only with view=full.", + ) + show_credit_eligibility = serializers.BooleanField( + help_text=( + "Whether the minimum grade for credit applies: true for a credit course when credit " + "eligibility is enabled on the platform. Present only with view=full." + ), + ) + mfe_proctored_exam_settings_url = serializers.CharField( + allow_blank=True, + help_text=( + "Path of the course's proctored exam settings page in the authoring app, or an empty " + "string when that page is not available. Present only with view=full." + ), + ) + + class Meta: + ref_name = "authoring_grading.CourseGradingFull.v3" + + +class GradingViewQuerySerializer(serializers.Serializer): # pylint: disable=abstract-method + """Query parameters of a grading policy read.""" + + view = serializers.ChoiceField( + choices=[GRADING_FULL_VIEW], + required=False, + help_text=( + "Set to full to add the graded subsections per assignment type, the default letter " + "grade names, and the credit and proctoring context. Omit it for the policy alone." + ), + ) + + +# The request fields below check each value with the conversion the grading +# model applies when it reads it, and the view then hands the model the body as +# sent. A value the model cannot convert or store is refused before anything is +# written. +# +# A body holding every policy field goes to the model's whole-policy update, +# which stores some values outside the documented shape: null or empty cutoffs, +# a grader type of any JSON type, an empty credit minimum. Such a body is +# validated by _WholeGradingPolicyUpdateSerializer, which accepts them. Any other +# body is held to the documented shape, because the model's per-field updates +# store null or empty cutoffs where they break the course's grading. + +GRADING_POLICY_FIELDS = ("graders", "grade_cutoffs", "grace_period", "minimum_grade_credit") + +# The modulestore stores integers in at most 8 bytes and fails part-way through +# a write on a wider one. +_STORABLE_INT_RANGE = range(-(2 ** 63), 2 ** 63) + + +def _storable(value): + """Whether the modulestore can store ``value``, at any depth.""" + if isinstance(value, bool): + return True + if isinstance(value, int): + return value in _STORABLE_INT_RANGE + if isinstance(value, Mapping): + return all(_storable(item) for item in value.values()) + if isinstance(value, list): + return all(_storable(item) for item in value) + return True + + +class _SentValueField(serializers.Field): + """A request value passed to the grading model as sent, after checking it converts and stores.""" + + convert = None + # False where the model stores only the converted value, or nothing. + stored_as_sent = True + default_error_messages = {"invalid": "Enter a valid value."} + + def to_internal_value(self, data): + if self.convert is not None: + try: + converted = self.convert(data) # pylint: disable=not-callable + except (TypeError, ValueError, OverflowError): + self.fail("invalid") + if isinstance(converted, float) and not math.isfinite(converted): + self.fail("invalid") + if not _storable(converted): + self.fail("invalid") + if self.stored_as_sent and not _storable(data): + self.fail("invalid") + return data + + def to_representation(self, value): + return value + + +@extend_schema_field(OpenApiTypes.STR) +class _NameField(_SentValueField): + """A name, which must be a string.""" + + default_error_messages = {"invalid": "A string is required."} + + def to_internal_value(self, data): + if not isinstance(data, str): + self.fail("invalid") + return super().to_internal_value(data) + + +@extend_schema_field(OpenApiTypes.STR) +class _TextField(_SentValueField): + """A label the grading model stores as sent, whatever its type.""" + + def validate_empty_values(self, data): + # The grading model stores null here, so it is accepted, though not published. + if data is None: + return (True, None) + return super().validate_empty_values(data) + + +@extend_schema_field(OpenApiTypes.INT) +class _CountField(_SentValueField): + """A count the grading model reads with ``int()``.""" + + convert = int + stored_as_sent = False + default_error_messages = {"invalid": "A whole number is required."} + + +@extend_schema_field(OpenApiTypes.DOUBLE) +class _NumberField(_SentValueField): + """A number the grading model reads with ``float()``.""" + + convert = float + default_error_messages = {"invalid": "A finite number is required."} + + +@extend_schema_field(OpenApiTypes.DOUBLE) +class _ScoreField(_NumberField): + """A score, which must be a JSON number.""" + + def to_internal_value(self, data): + if isinstance(data, bool) or not isinstance(data, (int, float)): + self.fail("invalid") + return super().to_internal_value(data) + + +@extend_schema_field(OpenApiTypes.DOUBLE) +class _CreditMinimumField(_NumberField): + """A credit minimum; an empty string is stored as no minimum.""" + + def to_internal_value(self, data): + if data == "": + return data + return super().to_internal_value(data) + + +@extend_schema_field(OpenApiTypes.INT) +class _PercentField(_NumberField): + """A whole percentage; the grading model also stores a fractional one as sent.""" + + stored_as_sent = False + + +@extend_schema_field(OpenApiTypes.INT) +class _GraderIdField(_TextField): + """A grader id, which the grading model ignores whatever its value.""" + + stored_as_sent = False + + +class GracePeriodUpdateSerializer(serializers.Serializer): # pylint: disable=abstract-method + """A grace period in an update. A unit left out counts as 0.""" + + hours = serializers.IntegerField(default=0, help_text="Whole hours.") + minutes = serializers.IntegerField(default=0, help_text="Minutes beyond the whole hours.") + seconds = serializers.IntegerField(default=0, help_text="Seconds beyond the whole minutes.") + + class Meta: + ref_name = "authoring_grading.GracePeriodUpdate.v3" + + +@extend_schema_field(GracePeriodUpdateSerializer) +class _GracePeriodField(serializers.Field): + """ + A grace period, in any of the units ``datetime.timedelta`` accepts. + + An object wrapped in a ``grace_period`` key is unwrapped first, as the + grading model does. + """ + + default_error_messages = { + "invalid": "Enter the grace period as an object of hours, minutes and seconds.", + } + + def to_internal_value(self, data): + try: + period = data["grace_period"] if "grace_period" in data else data + timedelta(**period) + except (TypeError, ValueError, OverflowError): + self.fail("invalid") + return data + + def to_representation(self, value): + return value + + +class _AnyGradeCutoffsField(serializers.DictField): + """Letter-grade cutoffs as the whole-policy update stores them: null, or an object of numbers or nulls.""" + + def __init__(self, **kwargs): + super().__init__(child=_NumberField(allow_null=True), **kwargs) + + def validate_empty_values(self, data): + if data is None: + return (True, None) + return super().validate_empty_values(data) + + +class _GraderListSerializer(serializers.ListSerializer): # pylint: disable=abstract-method + """ + Graders as a list. + + The grading model iterates whatever it is given, so it stores an empty + string or an empty object as an empty list, and both are accepted. + Anything else that is not a list is refused. + """ + + def to_internal_value(self, data): + if isinstance(data, (str, Mapping)): + data = list(data) + return super().to_internal_value(data) + + +class GraderUpdateSerializer(serializers.Serializer): # pylint: disable=abstract-method + """One assignment type in a grading policy update.""" + + type = _NameField(help_text="Name of the assignment type, for example Homework.") + min_count = _CountField( + required=False, + help_text="Number of assignments of this type the course expects. Defaults to 0.", + ) + drop_count = _CountField( + required=False, + help_text="Number of lowest-scoring assignments of this type left out of the grade. Defaults to 0.", + ) + short_label = _TextField( + required=False, + allow_null=True, + help_text="Abbreviation shown with the assignment type, for example HW.", + ) + weight = _PercentField( + required=False, + help_text="Share of the course grade, in percent. Defaults to 0.", + ) + id = _GraderIdField( + required=False, + help_text=( + "Ignored: a grader's position in the list is its id. Accepted so that graders " + "read from this endpoint can be sent back unchanged." + ), + ) + + class Meta: + ref_name = "authoring_grading.GraderUpdate.v3" + list_serializer_class = _GraderListSerializer + + +class CourseGradingUpdateSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + A change to a course's grading policy. + + Every field is optional, and a field left out keeps its stored value. + """ + + graders = GraderUpdateSerializer( + many=True, + required=False, + help_text="Replaces every assignment type, in the order given.", + ) + grade_cutoffs = serializers.DictField( + child=_ScoreField(), + allow_empty=False, + # Never reached, as allow_empty refuses an empty object first; it publishes the minimum. + validators=[MinLengthValidator(1)], + required=False, + help_text="Lowest score, from 0 to 1, that earns each letter grade, keyed by the grade's name.", + ) + grace_period = _GracePeriodField( + required=False, + allow_null=True, + help_text=( + "Grace period after each due date. Null leaves the stored grace period unchanged; " + "a grace period cannot be removed here." + ), + ) + minimum_grade_credit = _NumberField( + required=False, + allow_null=True, + help_text="Lowest course score, from 0 to 1, that earns course credit. Null leaves it unchanged.", + ) + + class Meta: + ref_name = "authoring_grading.CourseGradingUpdate.v3" + + def to_internal_value(self, data): + if html.is_html_input(data): + raise ValidationError({api_settings.NON_FIELD_ERRORS_KEY: ["Send the grading policy as a JSON object."]}) + return super().to_internal_value(data) + + +class _WholePolicyGraderUpdateSerializer(GraderUpdateSerializer): # pylint: disable=abstract-method + """One assignment type in a whole-policy update, whose type may be any value.""" + + type = _TextField(help_text="Name of the assignment type, for example Homework.") + + class Meta: + ref_name = "authoring_grading.WholePolicyGraderUpdate.v3" + list_serializer_class = _GraderListSerializer + + +class _WholeGradingPolicyUpdateSerializer(CourseGradingUpdateSerializer): # pylint: disable=abstract-method + """ + A grading policy update holding every policy field. + + The grading model's whole-policy update stores some values beyond the + documented shape, and this accepts them: null cutoffs or cutoff values, + empty cutoffs, a grader type of any JSON type, and an empty string as the + credit minimum, which stores no minimum. + """ + + graders = _WholePolicyGraderUpdateSerializer( + many=True, + required=False, + help_text="Replaces every assignment type, in the order given.", + ) + grade_cutoffs = _AnyGradeCutoffsField( + required=False, + help_text="Lowest score, from 0 to 1, that earns each letter grade, keyed by the grade's name.", + ) + minimum_grade_credit = _CreditMinimumField( + required=False, + allow_null=True, + help_text="Lowest course score, from 0 to 1, that earns course credit. Null leaves it unchanged.", + ) + + class Meta: + ref_name = "authoring_grading.WholeGradingPolicyUpdate.v3" + + +def grading_policy_update_serializer(data): + """Return the serializer that validates the grading policy update body ``data``.""" + if isinstance(data, Mapping) and all(field in data for field in GRADING_POLICY_FIELDS): + return _WholeGradingPolicyUpdateSerializer(data=data) + return CourseGradingUpdateSerializer(data=data) diff --git a/cms/djangoapps/contentstore/rest_api/v3/views/authoring_grading.py b/cms/djangoapps/contentstore/rest_api/v3/views/authoring_grading.py index 196d8aebcc1b..735051e67e85 100644 --- a/cms/djangoapps/contentstore/rest_api/v3/views/authoring_grading.py +++ b/cms/djangoapps/contentstore/rest_api/v3/views/authoring_grading.py @@ -1,187 +1,175 @@ -""" -API Views for course grading settings — v3. - -This module is the v3 incarnation of the v0 ``AuthoringGradingView`` endpoint, -restructured to apply the FC-0118 ADRs from the start: - - * ADR 0025 – ``serializer_class`` on the viewset - * ADR 0026 – explicit ``authentication_classes`` + ``permission_classes`` - * ADR 0027 – ``drf_spectacular`` for OpenAPI schema generation - * ADR 0028 – consolidated into a single DRF ``ViewSet`` registered via - ``DefaultRouter`` (replaces ``AuthoringGradingView`` ``APIView``) - * ADR 0029 – standardized error envelope via :class:`StandardizedErrorMixin` - (v3-scoped — does not change the project-wide DRF ``EXCEPTION_HANDLER`` - setting) - * ADR 0033 / OEP-68 – the URL kwarg, action parameter, and OpenAPI parameter - are named ``course_key`` (the OEP-68-standardized name) rather than the - legacy ``course_id``. Since this is a brand-new versioned API, no - deprecated alias is needed — clients on the v0 endpoint continue to use - ``course_id`` there. - * ADR 0036 – **largely out of scope.** The ``CourseGradingModelSerializer`` - response is a single top-level ``graders`` list of small fixed-shape - objects (type, min_count, drop_count, short_label, weight, id) — no - tree nesting, no embedded sub-objects, no ``children`` field, no wide - flat object that would benefit from ``?view=minimal`` / ``?fields=``. - - The one ADR 0036 concern is anti-pattern #3 (unbounded child list): the - ``graders`` array has no upper bound in the serializer. In practice each - course has typically ≤8 graders (Homework, Lab, Exam, etc.) and the - update flow is exercised only by course-authoring staff, so the - real-world payload is always small. A hard cap is enforced upstream of - this endpoint by :func:`CourseGradingModel.update_from_json`; we surface - that as a documentation note rather than re-implement the bound here. - * ADR 0034 – auth standardization (OEP-0042). - ``authentication_classes`` is ``(JwtAuthentication, SessionAuthenticationAllowInactiveUser)``; - ``BearerAuthenticationAllowInactiveUser`` has been removed per the - deprecation policy. ``SessionAuthenticationAllowInactiveUser`` is - retained (rather than relying on the platform-default - ``SessionAuthentication``) so Studio authors whose accounts are - temporarily inactive can still update grading. - -Permission model note: - PR #38363 proposed a class-level ``HasStudioReadAccess`` permission. The - current v0 view has since evolved to use the ``openedx_authz`` permission - framework (``COURSES_EDIT_GRADING_SETTINGS``), which is more specific to - grading and aligns with the platform-wide authz direction. - - The v3 viewset preserves the openedx_authz model via an *inline* - ``user_has_course_permission`` check inside the action body (rather than - the ``@authz_permission_required`` decorator). The decorator raises - ``DeveloperErrorResponseException`` — a plain ``Exception`` subclass that - does not flow through DRF's exception handler, so it would bypass - :class:`StandardizedErrorMixin` and surface as an unstructured 500. - Raising ``rest_framework.exceptions.PermissionDenied`` directly keeps the - ADR 0029 envelope intact. -""" - -from drf_spectacular.utils import OpenApiParameter, OpenApiRequest, OpenApiResponse, extend_schema +"""API views for a course's grading policy (v3).""" + +from drf_spectacular.utils import ( + OpenApiParameter, + OpenApiResponse, + PolymorphicProxySerializer, + 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.routers import COURSE_KEY_LOOKUP_REGEX -from openedx_authz.constants.permissions import COURSES_EDIT_GRADING_SETTINGS from rest_framework import viewsets -from rest_framework.exceptions import PermissionDenied from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request from rest_framework.response import Response -from cms.djangoapps.contentstore.rest_api.v0.serializers import CourseGradingModelSerializer -from cms.djangoapps.contentstore.rest_api.v3.utils import COMMON_ERROR_RESPONSES, resolve_course_key -from cms.djangoapps.models.settings.course_grading import CourseGradingModel -from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission -from openedx.core.djangoapps.authz.decorators import user_has_course_permission -from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements +from cms.djangoapps.contentstore.rest_api.v3.grading_service import ( + get_full_grading_settings, + get_grading_policy, + update_grading_policy, +) +from cms.djangoapps.contentstore.rest_api.v3.permissions import CanEditGradingSettings, CanViewGradingSettings +from cms.djangoapps.contentstore.rest_api.v3.serializers.grading import ( + GRADING_FULL_VIEW, + CourseGradingFullSerializer, + CourseGradingSerializer, + CourseGradingUpdateSerializer, + GradingViewQuerySerializer, + grading_policy_update_serializer, +) +from cms.djangoapps.contentstore.rest_api.v3.utils import resolve_course_key _COURSE_KEY_PARAMETER = OpenApiParameter( name="course_key", - description="OEP-68 course key (e.g. course-v1:org+course+run).", + description="Key of the course, for example course-v1:edX+DemoX+Demo_Course.", required=True, type=str, location=OpenApiParameter.PATH, ) +_VIEW_PARAMETER = OpenApiParameter( + name="view", + description=GradingViewQuerySerializer().fields["view"].help_text, + required=False, + type=str, + enum=[GRADING_FULL_VIEW], + location=OpenApiParameter.QUERY, +) + + +# The full shape is listed first: the default shape allows extra keys, so a +# client trying each shape in order would otherwise read a full response as the +# default one and lose the view=full fields. As the shapes overlap, the CMS +# schema hooks publish this component as anyOf (cms.lib.spectacular.ANY_OF_COMPONENTS). +_READ_RESPONSE = PolymorphicProxySerializer( + component_name="authoring_grading.CourseGradingRead.v3", + serializers=[CourseGradingFullSerializer, CourseGradingSerializer], + resource_type_field_name=None, +) + + +def _error(description): + return OpenApiResponse(response=ErrorResponseSerializer, description=description) + @extend_schema(tags=["openedx-platform-sdk"]) class AuthoringGradingViewSet(StandardizedErrorMixin, viewsets.ViewSet): """ - ViewSet for course grading settings (v3). Registered via DefaultRouter - (basename ``authoring_grading``). + The grading policy of one course: its assignment types and their weights, + letter-grade cutoffs, grace period, and minimum grade for course credit. - Router-generated URL:: + Reading the policy needs permission to view the course's grading settings, + changing it permission to edit them. The course is looked up first, so a + signed-in caller learns that a course does not exist (404) before being + refused (403). - PATCH /api/contentstore/v3/authoring_grading/{course_key}/ → partial_update - - Supersedes ``AuthoringGradingView`` at ``POST /api/contentstore/v0/grading/{course_id}``. + A plain ``ViewSet``: the policy is a settings field of the course in the + modulestore, not database rows. """ - # ADR 0034 — JWT + session-with-inactive-user (BearerAuthenticationAllowInactiveUser - # removed per OEP-0042). SessionAuthenticationAllowInactiveUser is retained - # (instead of relying on the platform-default SessionAuthentication) so Studio - # authors whose accounts are temporarily inactive can still update grading. - authentication_classes = ( - JwtAuthentication, - SessionAuthenticationAllowInactiveUser, - ) + # Studio authors whose account is not yet activated use this endpoint from + # the authoring app; the default session class would reject them. + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) permission_classes = (IsAuthenticated,) - serializer_class = CourseGradingModelSerializer + action_permission_classes = { + "retrieve": CanViewGradingSettings, + "partial_update": CanEditGradingSettings, + } + serializer_class = CourseGradingSerializer - # DefaultRouter lookup: matches course-v1:org+course+run (+ or / separators). - # OEP-68: the kwarg name is ``course_key`` (not the legacy ``course_id``). lookup_field = "course_key" lookup_value_regex = COURSE_KEY_LOOKUP_REGEX + _course_key = None + + def get_permissions(self): + """Add the permission of the requested action to the authentication check.""" + permissions = super().get_permissions() + action_permission = self.action_permission_classes.get(self.action) + if action_permission is not None: + permissions.append(action_permission()) + return permissions + + def get_course_key(self): + """Return the course in the path, parsed and checked to exist (404 otherwise).""" + if self._course_key is None: + self._course_key = resolve_course_key(self.kwargs["course_key"]) + return self._course_key + def get_serializer(self, *args, **kwargs): - """Instantiate and return the configured serializer class.""" + """Instantiate the policy serializer declared on this viewset.""" return self.serializer_class(*args, **kwargs) + @extend_schema( + summary="Get a course's grading settings", + description=( + "Returns the course's grading policy. With view=full the response also lists the graded " + "subsections of each assignment type, the default letter grade names, and the course's " + "credit and proctoring context." + ), + parameters=[_COURSE_KEY_PARAMETER, _VIEW_PARAMETER], + responses={ + 200: OpenApiResponse( + response=_READ_RESPONSE, + description="The grading policy, in the full shape with view=full and the default shape otherwise.", + ), + 400: _error("The view parameter has an unknown value."), + 401: _error("The requester is not authenticated."), + 403: _error("The requester cannot view the grading settings of this course."), + 404: _error("The course does not exist."), + }, + ) + # The legacy router also routes ".json" here, passing ``format``. + def retrieve( + self, request: Request, course_key: str, format=None, # pylint: disable=unused-argument,redefined-builtin + ): + """Return the grading policy, with the grading page's context when ``view=full``.""" + query = GradingViewQuerySerializer(data=request.query_params) + query.is_valid(raise_exception=True) + if query.validated_data.get("view") == GRADING_FULL_VIEW: + full = get_full_grading_settings(self.get_course_key()) + return Response(CourseGradingFullSerializer(full).data) + return Response(self.get_serializer(get_grading_policy(self.get_course_key())).data) + @extend_schema( summary="Update a course's grading settings", - description="Partially update the grading settings for the specified course.", - request=OpenApiRequest(request=CourseGradingModelSerializer), + description=( + "Changes the fields present in the body and leaves the others as stored. graders replaces " + "every assignment type. A null grace_period or minimum_grade_credit leaves that field " + "unchanged. Unknown fields are ignored. Returns the stored policy." + ), + request={"application/json": CourseGradingUpdateSerializer}, parameters=[_COURSE_KEY_PARAMETER], responses={ 200: OpenApiResponse( - response=CourseGradingModelSerializer, - description="Grading settings updated successfully.", + response=CourseGradingSerializer, + description="The grading policy as stored after the update.", ), - **COMMON_ERROR_RESPONSES, + 400: _error("The body is not a valid grading policy update."), + 401: _error("The requester is not authenticated."), + 403: _error("The requester cannot edit the grading settings of this course."), + 404: _error("The course does not exist."), }, ) - def partial_update(self, request: Request, course_key: str): - """ - Update a course's grading settings. - - **Example Request** - - PATCH /api/contentstore/v3/authoring_grading/{course_key}/ - - **PATCH Parameters** - - The request body should follow the ``CourseGradingModelSerializer`` - schema. Example:: - - { - "graders": [ - { - "type": "Homework", - "min_count": 1, - "drop_count": 0, - "short_label": "", - "weight": 100, - "id": 0 - } - ], - "grade_cutoffs": {"A": 0.75, "B": 0.63, "C": 0.57, "D": 0.5}, - "grace_period": {"hours": 12, "minutes": 0}, - "minimum_grade_credit": 0.7, - "is_credit_course": true - } - - **Response Values** - - If the request is successful, an HTTP 200 "OK" response is returned - with the updated grading data serialized via - :class:`CourseGradingModelSerializer`. - """ - parsed_course_key = resolve_course_key(course_key) - - # Per-action authorization (ADR 0026): kept inline rather than - # behind ``@authz_permission_required`` because that decorator - # raises ``DeveloperErrorResponseException`` (not a DRF exception), - # which bypasses :class:`StandardizedErrorMixin`. Raising - # ``PermissionDenied`` directly flows through the ADR 0029 envelope. - if not user_has_course_permission( - request.user, - COURSES_EDIT_GRADING_SETTINGS.identifier, - parsed_course_key, - LegacyAuthoringPermission.READ, - ): - raise PermissionDenied("You do not have permission to perform this action.") - - if "minimum_grade_credit" in request.data: - update_credit_course_requirements.delay(str(parsed_course_key)) - - updated_data = CourseGradingModel.update_from_json(parsed_course_key, request.data, request.user) - serializer = self.get_serializer(updated_data) - return Response(serializer.data) + def partial_update( + self, request: Request, course_key: str, format=None, # pylint: disable=unused-argument,redefined-builtin + ): + """Change the grading policy fields present in the body.""" + grading_policy_update_serializer(request.data).is_valid(raise_exception=True) + # The model receives the body as sent: it does its own conversions, and + # converting here first would change what it stores. + policy = update_grading_policy(self.get_course_key(), request.data, request.user) + return Response(self.get_serializer(policy).data) diff --git a/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_authoring_grading.py b/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_authoring_grading.py index 7503ee1ff6e9..5fedce281a49 100644 --- a/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_authoring_grading.py +++ b/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_authoring_grading.py @@ -1,360 +1,1331 @@ -""" -Unit tests for AuthoringGradingViewSet (v3). - -Single test module covering every ADR applied to the viewset: - * ADR 0025 / 0026 / 0027 / 0028 — action + permission + routing tests - (``TestAuthoringGradingViewSetPermissions``, ``TestAuthoringGradingViewSetUpdate``, - ``TestAuthoringGradingViewSetRouting``) - * ADR 0029 — standardized error envelope shape tests - (``TestAuthoringGradingViewSetErrorShape``) - -MongoDB-free: every service-layer call (``CourseGradingModel.update_from_json``, -``CourseOverview.course_exists``, ``update_credit_course_requirements.delay``, -and the ``openedx_authz`` permission lookup) is mocked, so these tests run -without a live modulestore or course-overview row. -""" +"""Tests for the v3 course grading API.""" + import json -from types import SimpleNamespace +from contextlib import contextmanager +from datetime import timedelta +from functools import cache +from importlib import import_module +from unittest import TestCase from unittest.mock import patch -from django.test import TestCase -from django.urls import resolve, reverse -from rest_framework import status -from rest_framework.test import APIClient, APITestCase +import ddt +import jsonschema +import pymongo +import pytest +from django.conf import settings +from django.db import connection +from django.test.utils import CaptureQueriesContext, override_settings +from django.urls import include, path, resolve, reverse +from drf_spectacular.generators import SchemaGenerator +from edx_rest_framework_extensions.testing import assert_error_envelope +from edx_toggles.toggles.testutils import override_waffle_flag +from openedx_authz.constants.roles import ( + COURSE_ADMIN, + COURSE_AUDITOR, + COURSE_DATA_RESEARCHER, + COURSE_EDITOR, + COURSE_STAFF, +) +from openedx_authz.models.engine import PolicyCacheControl +from rest_framework.test import APIClient +from cms.djangoapps.contentstore.rest_api.v3.serializers.grading import CourseGradingSerializer +from cms.djangoapps.contentstore.signals.signals import GRADING_POLICY_CHANGED from cms.djangoapps.contentstore.tests.utils import routed_actions +from cms.djangoapps.contentstore.toggles import LEGACY_STUDIO_EXAM_SETTINGS +from cms.djangoapps.models.settings.course_grading import GRADING_POLICY_CHANGED_EVENT_TYPE, CourseGradingModel +from cms.djangoapps.models.settings.waffle import MATERIAL_RECOMPUTE_ONLY_FLAG +from common.djangoapps.student.models import CourseAccessRole +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseLimitedStaffRole, + CourseStaffRole, + OrgStaffRole, +) from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin +from openedx.core.djangoapps.credit.tests.factories import CreditCourseFactory +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 SignalHandler, modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory -COURSE_ID = "course-v1:edX+ToyX+Toy_Course" - -# Minimal graders payload accepted by CourseGradingModelSerializer. -_GRADERS_PAYLOAD = [ - { - "type": "Homework", - "min_count": 1, - "drop_count": 0, - "short_label": "", - "weight": 100, - "id": 0, - }, +LEGACY = "legacy" +CONFORMING = "conforming" +MOUNTS = (LEGACY, CONFORMING) +UNKNOWN_COURSE_KEY = "course-v1:NoSuchOrg+NoSuchCourse+NoSuchRun" +CREDIT_TASK = "openedx.core.djangoapps.credit.tasks.update_credit_course_requirements.delay" +UPDATE_ITEM = "xmodule.modulestore.mixed.MixedModuleStore.update_item" + +# Stored on every seeded course. 0.29 does not survive the percentage round trip +# (it is shown as 28), so it reveals any write-back of graders nobody sent. +SEED_GRADERS = [ + {"type": "Homework", "min_count": 3, "drop_count": 1, "short_label": "HW", "weight": 0.29}, + {"type": "Lab", "min_count": 2, "drop_count": 0, "short_label": "Lab", "weight": 0.07}, + {"type": "Exam", "min_count": 1, "drop_count": 0, "short_label": None, "weight": 0.64}, ] +SEED_CUTOFFS = {"A": 0.9, "B": 0.8, "Pass": 0.5} +SEED_GRACE = timedelta(hours=2, minutes=5) +SEED_CREDIT = 0.8 -# Fake CourseGradingModel return value: only the field the serializer reads. -_MOCK_GRADING_MODEL = SimpleNamespace(graders=_GRADERS_PAYLOAD) +GRADERS = [ + {"type": "Homework", "min_count": 3, "drop_count": 1, "short_label": "HW", "weight": 28, "id": 0}, + {"type": "Lab", "min_count": 2, "drop_count": 0, "short_label": "Lab", "weight": 7, "id": 1}, + {"type": "Exam", "min_count": 1, "drop_count": 0, "short_label": "", "weight": 64, "id": 2}, +] +# The seeded graders exactly as stored: sending them changes no weight or count. +SEED_GRADERS_AS_SENT = [ + {"type": "Homework", "min_count": 3, "drop_count": 1, "short_label": "HW", "weight": 29}, + {"type": "Lab", "min_count": 2, "drop_count": 0, "short_label": "Lab", "weight": 7}, + {"type": "Exam", "min_count": 1, "drop_count": 0, "short_label": None, "weight": 64}, +] +CUTOFFS = {"A": 0.85, "B": 0.7, "Pass": 0.45} -# CourseOverview.course_exists is called inside ``resolve_course_key()`` (now in -# v3/utils.py), not directly in the view module — so the patch must target the -# utils module's binding, not the view module's. -MOCK_COURSE_EXISTS = ( - "cms.djangoapps.contentstore.rest_api.v3.utils.CourseOverview.course_exists" -) -MOCK_UPDATE_FROM_JSON = ( - "cms.djangoapps.contentstore.rest_api.v3.views.authoring_grading.CourseGradingModel.update_from_json" -) -MOCK_CREDIT_TASK = ( - "cms.djangoapps.contentstore.rest_api.v3.views.authoring_grading." - "update_credit_course_requirements.delay" -) -# Patch the local module-level binding (imported from openedx.core.djangoapps.authz.decorators) -# — patching the source module would not replace the already-bound reference inside the view. -MOCK_HAS_PERMISSION = ( - "cms.djangoapps.contentstore.rest_api.v3.views.authoring_grading.user_has_course_permission" + +def _full(**overrides): + """A whole-policy body, with ``overrides`` replacing its fields.""" + body = { + "graders": GRADERS, + "grade_cutoffs": CUTOFFS, + "grace_period": {"hours": 3, "minutes": 15}, + "minimum_grade_credit": 0.75, + } + body.update(overrides) + return body + + +# Whole-policy bodies the grading model accepts, picked by hand. Each must produce +# the same response bytes, stored policy, events and credit task as the +# whole-policy update. +HAND_PICKED_FULL_BODIES = { + "authoring_app": _full(), + "end_to_end_post": { + "graders": [{"type": "Homework", "min_count": 2, "drop_count": 0, "short_label": "HW", "weight": 100}], + "grade_cutoffs": {"Pass": 0.6}, + "grace_period": {"hours": 0, "minutes": 30}, + "minimum_grade_credit": 0.8, + "is_credit_course": False, + }, + "documented_example_with_credit_flag": { + "graders": [{"type": "Homework", "min_count": 1, "drop_count": 0, "short_label": "", "weight": 100, "id": 0}], + "grade_cutoffs": {"A": 0.75, "B": 0.63, "C": 0.57, "D": 0.5}, + "grace_period": {"hours": 12, "minutes": 0}, + "minimum_grade_credit": 0.7, + "is_credit_course": True, + }, + "graders_without_id": _full(graders=[{k: v for k, v in g.items() if k != "id"} for g in GRADERS]), + "numbers_as_strings": _full(graders=[ + {**g, "min_count": "1", "drop_count": "0", "weight": str(g["weight"])} for g in GRADERS + ]), + "grace_period_null": _full(grace_period=None), + "minimum_grade_credit_null": _full(minimum_grade_credit=None), + "grace_period_with_seconds": _full(grace_period={"hours": 1, "minutes": 2, "seconds": 3}), + "grace_period_with_days": _full(grace_period={"days": 1, "hours": 2}), + "grace_period_wrapped": _full(grace_period={"grace_period": {"hours": 2}}), + "grace_period_empty": _full(grace_period={}), + "grace_period_fractional": _full(grace_period={"hours": 1.5}), + "displayed_weights_resent": _full(graders=[{**GRADERS[0], "weight": 28}, GRADERS[1], GRADERS[2]]), + "stored_policy_resent": _full(graders=SEED_GRADERS_AS_SENT, grade_cutoffs=SEED_CUTOFFS), + "unknown_key": _full(foo="bar"), + "graders_empty_list": _full(graders=[]), + "graders_empty_object": _full(graders={}), + "graders_empty_string": _full(graders=""), + "grader_type_null": _full(graders=[{**GRADERS[0], "type": None}]), + "grader_type_number": _full(graders=[{**GRADERS[0], "type": 123}]), + "grader_count_fractional": _full(graders=[{**GRADERS[0], "min_count": 1.5}]), + "grader_count_boolean": _full(graders=[{**GRADERS[0], "min_count": True}]), + "grader_weight_boolean": _full(graders=[{**GRADERS[0], "weight": True}]), + "grader_label_number": _full(graders=[{**GRADERS[0], "short_label": 5}]), + "grader_extra_key": _full(graders=[{**GRADERS[0], "extra": 1}]), + "cutoffs_null": _full(grade_cutoffs=None), + "cutoffs_empty": _full(grade_cutoffs={}), + "cutoff_as_string": _full(grade_cutoffs={"A": "0.5"}), + "credit_as_string": _full(minimum_grade_credit="0.5"), + "credit_as_integer": _full(minimum_grade_credit=1), +} + +# Every position of a whole-policy body was sent each of these values, plus 10 ** 30. +# WHOLE_POLICY_VALUES lists, per position, the ones the whole-policy update stores +# and answers 200 for; every other combination fails inside the grading model. +_SCALARS = (True, False, 0, 1, -1, 0.5) +_ANY_JSON = ( + None, *_SCALARS, 1e308, "", "x", "1", "0.5", "inf", "nan", "1" * 30, [], ["a"], [1], {}, {"x": 1}, ) +GRACE_PERIOD_UNITS = ("hours", "minutes", "seconds", "days", "weeks", "milliseconds", "microseconds") +WHOLE_POLICY_VALUES = { + "graders": ([], {}, ""), + "grader.type": _ANY_JSON, + "grader.short_label": _ANY_JSON, + "grader.id": (*_ANY_JSON, 10 ** 30), + "grader.min_count": (*_SCALARS, "1"), + "grader.drop_count": (*_SCALARS, "1"), + "grader.weight": (*_SCALARS, 1e308, 10 ** 30, "1", "0.5", "1" * 30), + "grade_cutoffs": (None, {}, {"x": 1}), + "cutoff": (None, *_SCALARS, 1e308, "1", "0.5", "1" * 30), + "grace_period": (None, {}), + "grace_period.unit": _SCALARS, + "minimum_grade_credit": (None, *_SCALARS, 1e308, "", "1", "0.5", "1" * 30), + "unknown_field": (*_ANY_JSON, 10 ** 30), +} + + +def _whole_policy_bodies(): + """One whole-policy body per position and accepted value in WHOLE_POLICY_VALUES.""" + bodies = {} + for position, values in WHOLE_POLICY_VALUES.items(): + for value in values: + name = f"{position}={value!r}" + if position == "graders": + bodies[name] = _full(graders=value) + elif position.startswith("grader."): + field = position.split(".")[1] + bodies[name] = _full(graders=[{**GRADERS[0], field: value}, *GRADERS[1:]]) + elif position == "cutoff": + bodies[name] = _full(grade_cutoffs={"A": value, "Pass": 0.45}) + bodies[f"only_{name}"] = _full(grade_cutoffs={"A": value}) + elif position == "grace_period.unit": + for unit in GRACE_PERIOD_UNITS: + bodies[f"grace_period.{unit}={value!r}"] = _full(grace_period={unit: value}) + elif position == "unknown_field": + bodies[name] = _full(unknown=value) + else: + bodies[name] = _full(**{position: value}) + for field in ("min_count", "drop_count", "short_label", "weight", "id"): + bodies[f"grader_without_{field}"] = _full( + graders=[{key: value for key, value in GRADERS[0].items() if key != field}], + ) + bodies["cutoff_named_with_empty_string"] = _full(grade_cutoffs={"": 0.5}) + return bodies + + +GENERATED_FULL_BODIES = _whole_policy_bodies() +FULL_BODIES = {**HAND_PICKED_FULL_BODIES, **GENERATED_FULL_BODIES} + +# The whole-policy bodies replayed per mount. Both mounts route to one view, so +# each generated body is replayed on one of them, alternating; the hand-picked +# bodies are replayed on both. +FULL_BODY_CASES = [ + *((mount, name, body) for mount in MOUNTS for name, body in HAND_PICKED_FULL_BODIES.items()), + *((MOUNTS[index % len(MOUNTS)], name, body) for index, (name, body) in enumerate(GENERATED_FULL_BODIES.items())), +] + +# Bodies that make the grading model raise. Each is refused with a 400 naming the +# field, before anything is written. +REJECTED_BODIES = { + "graders_null": ({"graders": None}, "graders"), + "graders_not_a_list": ({"graders": 5}, "graders"), + "graders_a_string": ({"graders": "x"}, "graders"), + "grader_not_an_object": (_full(graders=["Homework"]), "graders"), + "grader_without_type": ({"graders": [{"min_count": 1, "weight": 100}]}, "graders"), + "grader_count_not_a_number": ({"graders": [{**GRADERS[0], "min_count": "many"}]}, "graders"), + "grader_count_fractional_string": (_full(graders=[{**GRADERS[0], "min_count": "1.5"}]), "graders"), + "grader_count_null": (_full(graders=[{**GRADERS[0], "min_count": None}]), "graders"), + "grader_drop_count_not_a_number": (_full(graders=[{**GRADERS[0], "drop_count": "x"}]), "graders"), + "grader_weight_not_a_number": (_full(graders=[{**GRADERS[0], "weight": "abc"}]), "graders"), + "grader_weight_null": (_full(graders=[{**GRADERS[0], "weight": None}]), "graders"), + "grader_weight_infinite": (_full(graders=[{**GRADERS[0], "weight": "inf"}]), "graders"), + "cutoffs_a_string": (_full(grade_cutoffs="x"), "grade_cutoffs"), + "cutoffs_a_list": (_full(grade_cutoffs=[]), "grade_cutoffs"), + "cutoff_not_a_number": ({"grade_cutoffs": {"A": "x"}}, "grade_cutoffs"), + "cutoff_an_object": (_full(grade_cutoffs={"A": {"x": 1}}), "grade_cutoffs"), + "grace_period_unknown_unit": (_full(grace_period={"foo": 1}), "grace_period"), + "grace_period_string_value": (_full(grace_period={"hours": "1"}), "grace_period"), + "grace_period_a_string": (_full(grace_period="x"), "grace_period"), + "grace_period_a_list": (_full(grace_period=[]), "grace_period"), + "grace_period_too_long": (_full(grace_period={"days": 10 ** 10}), "grace_period"), + "credit_not_a_number": (_full(minimum_grade_credit="x"), "minimum_grade_credit"), + "credit_a_list": (_full(minimum_grade_credit=[1]), "minimum_grade_credit"), + "body_a_list": ([1, 2], "non_field_errors"), + # A body without every policy field is held to the documented shape: a grader + # type is a string, and the cutoffs are at least one JSON number. + "partial_cutoffs_null": ({"grade_cutoffs": None}, "grade_cutoffs"), + "partial_cutoffs_empty": ({"grade_cutoffs": {}}, "grade_cutoffs"), + "partial_cutoff_null": ({"grade_cutoffs": {"A": None, "Pass": 0.5}}, "grade_cutoffs"), + "partial_only_cutoff_null": ({"grade_cutoffs": {"A": None}}, "grade_cutoffs"), + "partial_cutoff_a_numeric_string": ({"grade_cutoffs": {"A": "0.5"}}, "grade_cutoffs"), + "partial_cutoff_a_boolean": ({"grade_cutoffs": {"A": True}}, "grade_cutoffs"), + "partial_graders_and_cutoffs_null": ({"graders": GRADERS, "grade_cutoffs": None}, "grade_cutoffs"), + "partial_three_fields_cutoffs_empty": ( + {"graders": GRADERS, "grade_cutoffs": {}, "grace_period": {"hours": 1}}, "grade_cutoffs", + ), + "partial_grader_type_null": ({"graders": [{**GRADERS[0], "type": None}]}, "graders"), + "partial_grader_type_a_number": ({"graders": [{**GRADERS[0], "type": 123}]}, "graders"), + "partial_grader_type_a_list": ({"graders": [{**GRADERS[0], "type": ["a"]}]}, "graders"), + "partial_grader_type_an_object": ({"graders": [{**GRADERS[0], "type": {}}]}, "graders"), + "partial_credit_empty_string": ({"minimum_grade_credit": ""}, "minimum_grade_credit"), +} + +# Bodies holding an integer wider than the 8 bytes the modulestore can store, at a +# position where the grading model stores it. The whole-policy update fails part-way +# through its write on these and leaves the modulestore unable to read the course for +# the rest of the process, so they are not replayed against it here. +TOO_LARGE_BODIES = { + "grader_count_huge": (_full(graders=[{**GRADERS[0], "min_count": 10 ** 30}]), "graders"), + "grader_count_huge_float": (_full(graders=[{**GRADERS[0], "min_count": 1e308}]), "graders"), + "grader_count_huge_string": (_full(graders=[{**GRADERS[0], "min_count": "1" * 30}]), "graders"), + "grader_drop_count_huge": (_full(graders=[{**GRADERS[0], "drop_count": 10 ** 30}]), "graders"), + "grader_drop_count_huge_float": (_full(graders=[{**GRADERS[0], "drop_count": 1e308}]), "graders"), + "grader_drop_count_huge_string": (_full(graders=[{**GRADERS[0], "drop_count": "1" * 30}]), "graders"), + "grader_type_huge": (_full(graders=[{**GRADERS[0], "type": 10 ** 30}]), "graders"), + "grader_label_huge": (_full(graders=[{**GRADERS[0], "short_label": 10 ** 30}]), "graders"), + "grader_label_list_holding_huge": (_full(graders=[{**GRADERS[0], "short_label": [10 ** 30]}]), "graders"), + "cutoff_huge": (_full(grade_cutoffs={"A": 10 ** 30, "Pass": 0.45}), "grade_cutoffs"), + "credit_huge": (_full(minimum_grade_credit=10 ** 30), "minimum_grade_credit"), + "partial_grader_count_huge": ({"graders": [{**GRADERS[0], "min_count": 10 ** 30}]}, "graders"), + "partial_cutoff_huge": ({"grade_cutoffs": {"A": 10 ** 30}}, "grade_cutoffs"), + "partial_credit_huge": ({"minimum_grade_credit": 10 ** 30}, "minimum_grade_credit"), +} + + +def grading_url(mount, course_key): + """Return the address of the grading resource of ``course_key`` on ``mount``.""" + if mount == LEGACY: + return reverse( + "cms.djangoapps.contentstore:v3:authoring_grading-detail", kwargs={"course_key": str(course_key)}, + ) + return reverse("authoring_v3:course_grading", kwargs={"course_key": str(course_key)}) + + +def v0_url(course_key): + return reverse("cms.djangoapps.contentstore:v0:cms_api_update_grading", kwargs={"course_id": str(course_key)}) + + +def v1_url(course_key): + return reverse("cms.djangoapps.contentstore:v1:course_grading", kwargs={"course_id": str(course_key)}) + + +def _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 _patch(client, url, body): + return client.patch(url, data=json.dumps(body), content_type="application/json") + + +def _post(client, url, body): + return client.post(url, data=json.dumps(body), content_type="application/json") + + +def seed_course(user, **kwargs): + """Create a course holding the seeded grading policy.""" + course = CourseFactory.create(**kwargs) + course.raw_grader = [dict(grader) for grader in SEED_GRADERS] + course.grade_cutoffs = dict(SEED_CUTOFFS) + course.graceperiod = SEED_GRACE + course.minimum_grade_credit = SEED_CREDIT + modulestore().update_item(course, user.id) + return course + + +def stored_policy(course_key): + """The grading fields as the modulestore holds them, weights as raw fractions.""" + course = modulestore().get_course(course_key) + return { + "raw_grader": course.raw_grader, + "grade_cutoffs": course.grade_cutoffs, + "graceperiod": course.graceperiod, + "minimum_grade_credit": course.minimum_grade_credit, + } + + +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 + + +@cache +def _published_components(): + """The schema components the grading routes publish, generated from the real URLconfs.""" + # Importing the service URLconf registers the course key converter the authoring routes use. + import_module(settings.ROOT_URLCONF) + patterns = [ + path("api/contentstore/", include("cms.djangoapps.contentstore.rest_api.urls")), + path("api/authoring/v3/", include("cms.djangoapps.contentstore.rest_api.v3.authoring_urls")), + ] + schema = SchemaGenerator(patterns=patterns).get_schema(request=None, public=True) + return _as_json_schema(schema["components"]["schemas"]) + + +DEFAULT_SHAPE = "authoring_grading.CourseGrading.v3" +FULL_SHAPE = "authoring_grading.CourseGradingFull.v3" +READ_SHAPE = "authoring_grading.CourseGradingRead.v3" + + +def assert_matches_published_schema(body, component): + """Assert ``body`` is valid against the published schema component ``component``.""" + jsonschema.validate( + body, {"$ref": f"#/components/schemas/{component}", "components": {"schemas": _published_components()}}, + ) + + +@contextmanager +def capture_grading_effects(): + """Record grading-policy signals, grading-policy tracking events and credit tasks.""" + effects = {"signals": 0, "events": 0, "credit_tasks": []} + + def receiver(sender, **kwargs): # pylint: disable=unused-argument + effects["signals"] += 1 + + def emit(name, *args, **kwargs): # pylint: disable=unused-argument + if name == GRADING_POLICY_CHANGED_EVENT_TYPE: + effects["events"] += 1 + + GRADING_POLICY_CHANGED.connect(receiver, weak=False) + try: + with patch("eventtracking.tracker.emit", side_effect=emit), patch(CREDIT_TASK) as credit: + yield effects + effects["credit_tasks"] = [call.args for call in credit.call_args_list] + finally: + GRADING_POLICY_CHANGED.disconnect(receiver) + + +@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 + -_REQUIRED_ERROR_FIELDS = ("type", "title", "status", "detail", "instance") +def sql_writes(queries): + """``(statement, table)`` for each INSERT, UPDATE and DELETE among the captured ``queries``.""" + writes = [] + for query in queries.captured_queries: + words = query["sql"].split() + statement = words[0].upper() + if statement in ("INSERT", "DELETE"): + writes.append((statement, words[2].strip('"`'))) + elif statement == "UPDATE": + writes.append((statement, words[1].strip('"`'))) + return writes -# =========================================================================== -# ADR 0026 — permission boundary tests -# =========================================================================== -class TestAuthoringGradingViewSetPermissions(APITestCase): +# --------------------------------------------------------------------------- +# Who may call the endpoint +# --------------------------------------------------------------------------- + +OPERATIONS = ("get", "get_full", "patch_full", "patch_partial") +CALLS = [(mount, operation) for mount in MOUNTS for operation in OPERATIONS] + + +class _AccessMatrixMixin: """ - ADR 0026 — permission boundary tests for the partial_update action. + Calls every operation on both mounts and checks the status against the + expected one and against the decision v1 makes for the same caller. """ - def setUp(self): - super().setUp() - self.client = APIClient() - self.url = reverse( - "cms.djangoapps.contentstore:v3:authoring_grading-detail", - kwargs={"course_key": COURSE_ID}, + def _call(self, user, mount, operation, course_key): + """Call ``operation`` on ``mount`` as ``user``.""" + client = _client(user) + url = grading_url(mount, course_key) + if operation == "get": + return client.get(url) + if operation == "get_full": + return client.get(url, {"view": "full"}) + if operation == "patch_full": + return _patch(client, url, _full()) + return _patch(client, url, {"grade_cutoffs": CUTOFFS}) + + def _v1_status(self, user, operation, course_key): + client = _client(user) + if operation.startswith("get"): + return client.get(v1_url(course_key)).status_code + return _post(client, v1_url(course_key), _full()).status_code + + def assert_access(self, user, mount, operation, expected, course_key=None): + """Assert the status, the error envelope when refused, and agreement with v1.""" + course_key = course_key or self.course.id + with patch(CREDIT_TASK): + response = self._call(user, mount, operation, course_key) + v1_status = self._v1_status(user, operation, course_key) + assert response.status_code == expected, response.content + if expected != 200: + slug = {401: "authn", 403: "authz", 404: "not-found"}[expected] + assert_error_envelope(response, expected_status=expected, expected_type_slug=slug) + assert v1_status == expected, f"v1 decided {v1_status} for the same caller" + + +@ddt.ddt +class GradingAccessLegacyPermissionsTest(_AccessMatrixMixin, SharedModuleStoreTestCase): + """Who may read and change the policy 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(*CALLS) + @ddt.unpack + def test_anonymous_caller_is_refused_with_401(self, mount, operation): + self.assert_access(None, mount, operation, 401) + + @ddt.data(*CALLS) + @ddt.unpack + def test_user_without_a_role_is_refused_with_403(self, mount, operation): + self.assert_access(UserFactory(), mount, operation, 403) + + @ddt.data(*CALLS) + @ddt.unpack + def test_unknown_course_is_404_before_permission_is_checked(self, mount, operation): + self.assert_access(UserFactory(), mount, operation, 404, course_key=UNKNOWN_COURSE_KEY) + + @ddt.data(*CALLS) + @ddt.unpack + def test_course_staff_may_read_and_change(self, mount, operation): + self.assert_access(self._user_with(CourseStaffRole(self.course.id)), mount, operation, 200) + + @ddt.data(*CALLS) + @ddt.unpack + def test_course_instructor_may_read_and_change(self, mount, operation): + self.assert_access(self._user_with(CourseInstructorRole(self.course.id)), mount, operation, 200) + + @ddt.data(*CALLS) + @ddt.unpack + def test_org_staff_may_read_and_change(self, mount, operation): + self.assert_access(self._user_with(OrgStaffRole(self.course.id.org)), mount, operation, 200) + + @ddt.data(*CALLS) + @ddt.unpack + def test_global_staff_may_read_and_change(self, mount, operation): + self.assert_access(UserFactory(is_staff=True), mount, operation, 200) + + @ddt.data(*CALLS) + @ddt.unpack + def test_superuser_may_read_and_change(self, mount, operation): + self.assert_access(UserFactory(is_superuser=True), mount, operation, 200) + + @ddt.data(*CALLS) + @ddt.unpack + def test_limited_staff_is_refused(self, mount, operation): + self.assert_access(self._user_with(CourseLimitedStaffRole(self.course.id)), mount, operation, 403) + + @ddt.data(*CALLS) + @ddt.unpack + def test_staff_of_another_course_is_refused(self, mount, operation): + self.assert_access(self._user_with(CourseStaffRole(self.other_course.id)), mount, operation, 403) + + @ddt.data(*MOUNTS) + def test_inactive_user_with_a_session_may_read_and_change(self, mount): + """Session callers whose account is not activated are admitted.""" + user = UserFactory(is_staff=True, is_active=False, password="password") + client = APIClient() + assert client.login(username=user.username, password="password") + assert client.get(grading_url(mount, self.course.id)).status_code == 200 + response = _patch(client, grading_url(mount, self.course.id), {"grade_cutoffs": CUTOFFS}) + assert response.status_code == 200 + + @ddt.data(*MOUNTS) + def test_jwt_caller_may_read(self, mount): + user = UserFactory(is_staff=True) + response = APIClient().get( + grading_url(mount, self.course.id), HTTP_AUTHORIZATION=f"JWT {create_jwt_for_user(user)}", ) + assert response.status_code == 200 - def test_unauthenticated_patch_returns_401(self): - """Unauthenticated PATCH must return 401 (IsAuthenticated).""" - response = self.client.patch(self.url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_401_UNAUTHORIZED - - @patch(MOCK_HAS_PERMISSION, return_value=False) - @patch(MOCK_COURSE_EXISTS, return_value=True) - def test_authenticated_without_grading_permission_returns_403(self, mock_exists, mock_perm): # noqa: ARG002 - """Authenticated user without grading permission must receive 403.""" - user = UserFactory.create() - self.client.force_authenticate(user=user) - response = self.client.patch(self.url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_403_FORBIDDEN - - -# =========================================================================== -# ADR 0025 / 0028 — action body tests -# =========================================================================== -class TestAuthoringGradingViewSetUpdate(APITestCase): - """ - Action tests for the partial_update flow. + @ddt.data(*MOUNTS) + def test_bearer_token_is_refused(self, mount): + """Bearer tokens are not accepted here, though the v1 read takes them.""" + user = UserFactory(is_staff=True) + token = AccessTokenFactory(user=user, application=ApplicationFactory(user=user)).token + response = APIClient().get(grading_url(mount, self.course.id), HTTP_AUTHORIZATION=f"Bearer {token}") + assert_error_envelope(response, expected_status=401, expected_type_slug="authn") + assert APIClient().get(v1_url(self.course.id), HTTP_AUTHORIZATION=f"Bearer {token}").status_code == 200 - All service-layer interactions are mocked so the test exercises the - routing, serialization, and credit-task wiring without touching MongoDB - or modulestore. - """ + +@ddt.ddt +class GradingAccessAuthzTest(CourseAuthoringAuthzTestMixin, _AccessMatrixMixin, SharedModuleStoreTestCase): + """Who may read and change the policy when policy-based authoring permissions are on.""" + + @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 + + @ddt.data(*CALLS) + @ddt.unpack + def test_anonymous_caller_is_refused_with_401(self, mount, operation): + self.assert_access(None, mount, operation, 401) + + @ddt.data(*CALLS) + @ddt.unpack + def test_user_without_a_role_is_refused_with_403(self, mount, operation): + self.assert_access(self.unauthorized_user, mount, operation, 403) + + @ddt.data(*CALLS) + @ddt.unpack + def test_unknown_course_is_404_before_permission_is_checked(self, mount, operation): + self.assert_access(self.unauthorized_user, mount, operation, 404, course_key=UNKNOWN_COURSE_KEY) + + @ddt.data(*CALLS) + @ddt.unpack + def test_auditor_may_read_but_not_change(self, mount, operation): + expected = 200 if operation.startswith("get") else 403 + self.assert_access(self._user_with(COURSE_AUDITOR.external_key), mount, operation, expected) + + @ddt.data(*[(mount, operation, role) for mount, operation in CALLS for role in ( + COURSE_STAFF.external_key, COURSE_EDITOR.external_key, COURSE_ADMIN.external_key, + )]) + @ddt.unpack + def test_staff_editor_and_admin_may_read_and_change(self, mount, operation, role): + self.assert_access(self._user_with(role), mount, operation, 200) + + @ddt.data(*CALLS) + @ddt.unpack + def test_data_researcher_is_refused(self, mount, operation): + self.assert_access(self._user_with(COURSE_DATA_RESEARCHER.external_key), mount, operation, 403) + + @ddt.data(*CALLS) + @ddt.unpack + def test_legacy_course_staff_without_a_policy_role_is_refused(self, mount, operation): + user = UserFactory() + CourseAccessRole.objects.create(user=user, role="staff", course_id=self.course.id, org=self.course.id.org) + self.assert_access(user, mount, operation, 403) + + @ddt.data(*CALLS) + @ddt.unpack + def test_superuser_may_read_and_change(self, mount, operation): + self.assert_access(self.super_user, mount, operation, 200) + + +# --------------------------------------------------------------------------- +# Reading the policy +# --------------------------------------------------------------------------- + +def _flatten_v1(v1_body, grace_seconds): + """The v1 read with its policy lifted to the top level and the grace period's seconds added.""" + flat = {key: value for key, value in v1_body.items() if key != "course_details"} + flat.update(v1_body["course_details"]) + if flat["grace_period"] is not None: + flat["grace_period"] = {**flat["grace_period"], "seconds": grace_seconds} + return flat + + +@ddt.ddt +class GradingReadTest(ModuleStoreTestCase): + """The default and full representations.""" def setUp(self): super().setUp() - self.client = APIClient() - self.user = UserFactory.create(is_staff=True) - self.client.force_authenticate(user=self.user) - self.url = reverse( - "cms.djangoapps.contentstore:v3:authoring_grading-detail", - kwargs={"course_key": COURSE_ID}, - ) + self.staff = UserFactory(is_staff=True) + self.client = _client(self.staff) + self.course = seed_course(self.staff) + for name, assignment_type in (("One", "Homework"), ("Two", "Exam"), ("Three", "Homework")): + section = BlockFactory.create(parent=self.course, category="chapter", display_name=f"Section {name}") + BlockFactory.create( + parent=section, category="sequential", display_name=f"Sub {name}", format=assignment_type, graded=True, + ) - @patch(MOCK_CREDIT_TASK) - @patch(MOCK_UPDATE_FROM_JSON, return_value=_MOCK_GRADING_MODEL) - @patch(MOCK_HAS_PERMISSION, return_value=True) - @patch(MOCK_COURSE_EXISTS, return_value=True) - def test_patch_with_minimum_grade_credit_fires_credit_task( - self, mock_exists, mock_perm, mock_update, mock_credit_task, # noqa: ARG002 - ): - """``minimum_grade_credit`` in the payload triggers the credit-requirements Celery task.""" - body = { - "graders": _GRADERS_PAYLOAD, - "grade_cutoffs": {"A": 0.75, "B": 0.63, "C": 0.57, "D": 0.5}, - "grace_period": {"hours": 12, "minutes": 0}, - "minimum_grade_credit": 0.7, - "is_credit_course": True, + def _read(self, mount, **params): + response = self.client.get(grading_url(mount, self.course.id), params) + assert response.status_code == 200, response.content + return response.json() + + @ddt.data(*MOUNTS) + def test_default_view_is_the_stored_policy(self, mount): + expected = CourseGradingSerializer(CourseGradingModel.fetch(self.course.id)).data + assert self._read(mount) == expected + assert self._read(mount) == { + "graders": [ + {"type": "Homework", "min_count": 3, "drop_count": 1, "short_label": "HW", "weight": 28, "id": 0}, + {"type": "Lab", "min_count": 2, "drop_count": 0, "short_label": "Lab", "weight": 7, "id": 1}, + {"type": "Exam", "min_count": 1, "drop_count": 0, "short_label": None, "weight": 64, "id": 2}, + ], + "grade_cutoffs": SEED_CUTOFFS, + "grace_period": {"hours": 2, "minutes": 5, "seconds": 0}, + "minimum_grade_credit": SEED_CREDIT, } - response = self.client.patch(self.url, data=json.dumps(body), content_type="application/json") - assert response.status_code == status.HTTP_200_OK - mock_update.assert_called_once() - mock_credit_task.assert_called_once() - - @patch(MOCK_CREDIT_TASK) - @patch(MOCK_UPDATE_FROM_JSON, return_value=_MOCK_GRADING_MODEL) - @patch(MOCK_HAS_PERMISSION, return_value=True) - @patch(MOCK_COURSE_EXISTS, return_value=True) - def test_patch_without_minimum_grade_credit_skips_credit_task( - self, mock_exists, mock_perm, mock_update, mock_credit_task, # noqa: ARG002 - ): - """Absent ``minimum_grade_credit`` keeps the credit-requirements task unscheduled.""" - body = { - "graders": _GRADERS_PAYLOAD, - "grade_cutoffs": {"A": 0.75, "B": 0.63, "C": 0.57, "D": 0.5}, - "grace_period": {"hours": 12, "minutes": 0}, + + @ddt.data(*MOUNTS) + def test_default_view_matches_what_an_update_returns(self, mount): + with patch(CREDIT_TASK): + updated = _patch(self.client, grading_url(mount, self.course.id), {}).json() + assert self._read(mount) == updated + + @ddt.data(*MOUNTS) + def test_full_view_equals_the_v1_read_flattened(self, mount): + v1_body = self.client.get(v1_url(self.course.id)).json() + full = self._read(mount, view="full") + assert full == _flatten_v1(v1_body, grace_seconds=0) + assert full["course_assignment_lists"] == { + "Homework": ["Section One - Sub One", "Section Three - Sub Three"], + "Exam": ["Section Two - Sub Two"], } - response = self.client.patch(self.url, data=json.dumps(body), content_type="application/json") - assert response.status_code == status.HTTP_200_OK - mock_update.assert_called_once() - mock_credit_task.assert_not_called() + + @ddt.data(*[(mount, enabled) for mount in MOUNTS for enabled in (True, False)]) + @ddt.unpack + def test_full_view_credit_context_equals_v1(self, mount, eligibility_enabled): + CreditCourseFactory(course_key=self.course.id, enabled=True) + with override_settings(ENABLE_CREDIT_ELIGIBILITY=eligibility_enabled): + v1_body = self.client.get(v1_url(self.course.id)).json() + full = self._read(mount, view="full") + assert full == _flatten_v1(v1_body, grace_seconds=0) + assert full["is_credit_course"] is True + assert full["show_credit_eligibility"] is eligibility_enabled + + @ddt.data(*[(mount, legacy_exam_settings) for mount in MOUNTS for legacy_exam_settings in (True, False)]) + @ddt.unpack + def test_full_view_proctoring_link_equals_v1(self, mount, legacy_exam_settings): + with override_settings(COURSE_AUTHORING_MICROFRONTEND_URL="http://authoring.example"), \ + override_waffle_flag(LEGACY_STUDIO_EXAM_SETTINGS, active=legacy_exam_settings): + v1_body = self.client.get(v1_url(self.course.id)).json() + full = self._read(mount, view="full") + assert full == _flatten_v1(v1_body, grace_seconds=0) + expected = "" if legacy_exam_settings else f"/course/{self.course.id}/pages-and-resources/proctoring/settings" + assert full["mfe_proctored_exam_settings_url"] == expected + + @ddt.data(*MOUNTS) + def test_grace_period_seconds_are_shown_where_v1_drops_them(self, mount): + self.course.graceperiod = timedelta(hours=1, seconds=30) + modulestore().update_item(self.course, self.staff.id) + v1_body = self.client.get(v1_url(self.course.id)).json() + full = self._read(mount, view="full") + assert v1_body["course_details"]["grace_period"] == {"hours": 1, "minutes": 0} + assert full["grace_period"] == {"hours": 1, "minutes": 0, "seconds": 30} + assert full == _flatten_v1(v1_body, grace_seconds=30) + assert self._read(mount)["grace_period"] == {"hours": 1, "minutes": 0, "seconds": 30} + + @ddt.data(*MOUNTS) + def test_full_view_without_a_grace_period_equals_v1(self, mount): + course = CourseFactory.create() + v1_body = self.client.get(v1_url(course.id)).json() + full = self.client.get(grading_url(mount, course.id), {"view": "full"}).json() + assert full["grace_period"] is None + assert full == _flatten_v1(v1_body, grace_seconds=None) + + @ddt.data(*[(mount, value) for mount in MOUNTS for value in ("minimal", "bogus", "FULL")]) + @ddt.unpack + def test_unknown_view_is_refused_with_400(self, mount, value): + response = self.client.get(grading_url(mount, self.course.id), {"view": value}) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert list(envelope["errors"]) == ["view"] + + @ddt.data(*MOUNTS) + def test_empty_view_is_the_default_view(self, mount): + assert self._read(mount, view="") == self._read(mount) + + @ddt.data(*MOUNTS) + def test_reads_match_the_published_shapes(self, mount): + """ + Including policies stored with each value the schema marks nullable, and a + course never configured. The read publishes both shapes; a full response + also matches the default one. + """ + nullable_values = _full( + graders=[{**GRADERS[0], "type": None, "short_label": None}], + grade_cutoffs={"A": None, "Pass": 0.5}, + grace_period=None, + minimum_grade_credit="", + ) + cases = ( + (nullable_values, { + "graders": [{**GRADERS[0], "type": None, "short_label": None}], + "grade_cutoffs": {"A": None, "Pass": 0.5}, + "grace_period": {"hours": 2, "minutes": 5, "seconds": 0}, + "minimum_grade_credit": None, + }), + (_full(grade_cutoffs=None), { + "graders": GRADERS, + "grade_cutoffs": None, + "grace_period": {"hours": 3, "minutes": 15, "seconds": 0}, + "minimum_grade_credit": 0.75, + }), + ) + for body, expected in cases: + course = seed_course(self.staff) + with patch(CREDIT_TASK): + assert _patch(self.client, grading_url(mount, course.id), body).status_code == 200 + default = self.client.get(grading_url(mount, course.id)).json() + full = self.client.get(grading_url(mount, course.id), {"view": "full"}).json() + assert default == expected + assert {key: full[key] for key in default} == expected + assert_matches_published_schema(default, DEFAULT_SHAPE) + assert_matches_published_schema(full, FULL_SHAPE) + assert_matches_published_schema(default, READ_SHAPE) + assert_matches_published_schema(full, READ_SHAPE) + + never_configured = CourseFactory.create() + default = self.client.get(grading_url(mount, never_configured.id)).json() + full = self.client.get(grading_url(mount, never_configured.id), {"view": "full"}).json() + assert default["grace_period"] is None + assert_matches_published_schema(default, DEFAULT_SHAPE) + assert_matches_published_schema(full, FULL_SHAPE) + assert_matches_published_schema(default, READ_SHAPE) + assert_matches_published_schema(full, READ_SHAPE) + with pytest.raises(jsonschema.ValidationError): + assert_matches_published_schema(default, FULL_SHAPE) + + @ddt.data(*MOUNTS) + def test_head_answers_without_a_body(self, mount): + response = self.client.head(grading_url(mount, self.course.id)) + assert response.status_code == 200 + assert response.content == b"" -# =========================================================================== -# ADR 0028 — routing checks -# =========================================================================== -class TestAuthoringGradingViewSetRouting(TestCase): +# The one write a database's first role or permission lookup makes. +FIRST_LOOKUP_WRITES = [("INSERT", PolicyCacheControl._meta.db_table)] # pylint: disable=protected-access +READS = [(mount, method, params) for mount in MOUNTS + for method, params in (("get", {}), ("get", {"view": "full"}), ("head", {}))] + + +class _ReadIsSideEffectFreeMixin: """ - Routing checks — confirm the URL namespace and HTTP-method mapping are wired correctly. + Every read (default, full, HEAD, both mounts) on a credit and a non-credit + course writes no row and no Mongo document, stores nothing through the + modulestore, sends no grading or publishing signal, and emits no event + beyond the platform's request tracking. + + The reads start on a database that has never served a role or permission + lookup. The permission library keeps a one-row policy version table and + creates that row on its first lookup against a database, whichever API + makes it, so the first read may insert it and nothing else; the v1 read + inserts the same row. """ - def test_detail_url_resolves(self): - """v3 router exposes the viewset under ``v3:authoring_grading-detail``.""" - url = reverse( - "cms.djangoapps.contentstore:v3:authoring_grading-detail", - kwargs={"course_key": COURSE_ID}, - ) - assert "/api/contentstore/v3/authoring_grading/" in url - assert COURSE_ID in url + def reader_for(self, course): + """Return a user who may read ``course``, leaving the database as no lookup had touched it.""" + raise NotImplementedError - def test_post_not_allowed(self): - """The viewset only exposes PATCH (partial_update); POST must return 405.""" - client = APIClient() - client.force_authenticate(user=UserFactory.create(is_staff=True)) - url = reverse( - "cms.djangoapps.contentstore:v3:authoring_grading-detail", - kwargs={"course_key": COURSE_ID}, - ) - response = client.post(url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED + def _assert_reads_have_no_effects(self, course): + """Read ``course`` every way and assert nothing was written or announced.""" + published = [] + changed = [] + + def on_published(sender, **kwargs): # pylint: disable=unused-argument + published.append(kwargs) + + def on_changed(sender, **kwargs): # pylint: disable=unused-argument + changed.append(kwargs) + client = _client(self.reader_for(course)) + assert not PolicyCacheControl.objects.exists() + SignalHandler.course_published.connect(on_published, weak=False) + GRADING_POLICY_CHANGED.connect(on_changed, weak=False) + try: + for index, (mount, method, params) in enumerate(READS): + url = grading_url(mount, course.id) + with CaptureQueriesContext(connection) as queries, \ + count_mongo_calls() as mongo, \ + patch(UPDATE_ITEM) as update_item, \ + patch("eventtracking.tracker.emit") as emit: + response = getattr(client, method)(url, params) + request = (index, mount, method, params) + assert response.status_code == 200, (request, response.content) + assert sql_writes(queries) == (FIRST_LOOKUP_WRITES if index == 0 else []), request + assert mongo["writes"] == 0, request + assert update_item.call_count == 0, request + # Only the platform's per-request tracking event, as for any request. + assert [call.kwargs["name"] for call in emit.call_args_list] == [url], request + finally: + SignalHandler.course_published.disconnect(on_published) + GRADING_POLICY_CHANGED.disconnect(on_changed) + assert not published + assert not changed -# =========================================================================== -# ADR 0029 — standardized error-response envelope tests -# =========================================================================== -class TestAuthoringGradingViewSetErrorShape(APITestCase): + def test_reading_a_course_has_no_side_effects(self): + self._assert_reads_have_no_effects(seed_course(self.user)) + + def test_reading_a_credit_course_has_no_side_effects(self): + course = seed_course(self.user) + CreditCourseFactory(course_key=course.id, enabled=True) + with override_settings(ENABLE_CREDIT_ELIGIBILITY=True): + self._assert_reads_have_no_effects(course) + + def test_the_first_v1_read_writes_the_same_row(self): + course = seed_course(self.user) + client = _client(self.reader_for(course)) + assert not PolicyCacheControl.objects.exists() + with CaptureQueriesContext(connection) as queries, count_mongo_calls() as mongo: + response = client.get(v1_url(course.id)) + assert response.status_code == 200, response.content + assert sql_writes(queries) == FIRST_LOOKUP_WRITES + assert mongo["writes"] == 0 + + +class GradingReadSideEffectsLegacyPermissionsTest(_ReadIsSideEffectFreeMixin, ModuleStoreTestCase): """ - ADR 0029 — error response shape regression tests for AuthoringGradingViewSet. + Reads by course staff are side-effect free with policy-based authoring + permissions off. Course staff rather than global staff, whose access is + decided without a role lookup. + """ + + def reader_for(self, course): + reader = UserFactory() + CourseStaffRole(course.id).add_users(reader) + return reader - The envelope is wired in via - :class:`openedx.core.lib.api.mixins.StandardizedErrorMixin`, which overrides - DRF's per-view ``get_exception_handler`` to point at - ``openedx.core.lib.api.exceptions.standardized_error_exception_handler``. - Scoped to v3 — the project-wide DRF ``EXCEPTION_HANDLER`` setting is - unchanged, so v0 / v1 / v2 / v4 endpoints continue to return the legacy - error shape (locked in by ``test_v0_endpoint_unaffected_by_v3_envelope``). +class GradingReadSideEffectsAuthzTest(CourseAuthoringAuthzTestMixin, _ReadIsSideEffectFreeMixin, ModuleStoreTestCase): + """ + Reads by a course auditor are side-effect free with policy-based authoring + permissions on. A course auditor rather than a superuser, whom the policy + admits on the account's superuser flag without consulting role assignments. + + The test setup loads the policy and assigns roles through the permission + library, which creates the policy version row; it is deleted before the + reads so they start on a database no lookup has touched. + """ + + 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 + + +# --------------------------------------------------------------------------- +# Changing the policy +# --------------------------------------------------------------------------- + +class _UpdateBehaviourMixin: + """ + The update, checked against the v0 update as the reference: v0 hands any + body to the grading model's whole-policy update as sent, so a whole-policy + body must behave identically here. + + Each concrete class runs on one side of the flag that decides whether a + grading change with no new weights or counts is announced. """ + recompute_only = None + def setUp(self): super().setUp() - self.client = APIClient() - self.url = reverse( - "cms.djangoapps.contentstore:v3:authoring_grading-detail", - kwargs={"course_key": COURSE_ID}, - ) + self.enterContext(override_waffle_flag(MATERIAL_RECOMPUTE_ONLY_FLAG, active=self.recompute_only)) + self.staff = UserFactory(is_staff=True) + self.client = _client(self.staff) - def test_unauthenticated_patch_returns_standardized_401(self): - """Unauthenticated PATCH must return 401 with the ADR 0029 envelope.""" - response = self.client.patch(self.url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_401_UNAUTHORIZED - for field in _REQUIRED_ERROR_FIELDS: - assert field in response.data, f"ADR 0029: missing field '{field}'" - - def test_unauthenticated_401_type_uri(self): - """The ``type`` field for 401 must be the ADR 0029 authn URI.""" - response = self.client.patch(self.url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_401_UNAUTHORIZED - assert response.data.get("type") == "https://docs.openedx.org/errors/authn" - - @patch(MOCK_COURSE_EXISTS, return_value=True) - @patch(MOCK_HAS_PERMISSION, return_value=False) - def test_non_author_patch_returns_standardized_403(self, mock_perm, mock_exists): # noqa: ARG002 - """Authenticated non-author PATCH must return 403 with the ADR 0029 envelope.""" - non_author = UserFactory.create() - self.client.force_authenticate(user=non_author) - response = self.client.patch(self.url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_403_FORBIDDEN - for field in _REQUIRED_ERROR_FIELDS: - assert field in response.data, f"ADR 0029: missing field '{field}'" - - @patch(MOCK_COURSE_EXISTS, return_value=True) - @patch(MOCK_HAS_PERMISSION, return_value=False) - def test_non_author_403_type_uri(self, mock_perm, mock_exists): # noqa: ARG002 - """The ``type`` field for 403 must be the ADR 0029 authz URI.""" - non_author = UserFactory.create() - self.client.force_authenticate(user=non_author) - response = self.client.patch(self.url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_403_FORBIDDEN - assert response.data.get("type") == "https://docs.openedx.org/errors/authz" - - @patch(MOCK_COURSE_EXISTS, return_value=False) - def test_nonexistent_course_returns_standardized_404(self, mock_exists): # noqa: ARG002 - """PATCH for a non-existent course must return 404 with the ADR 0029 envelope.""" - staff = UserFactory.create(is_staff=True) - self.client.force_authenticate(user=staff) - response = self.client.patch(self.url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_404_NOT_FOUND - for field in _REQUIRED_ERROR_FIELDS: - assert field in response.data, f"ADR 0029: missing field '{field}'" - - @patch(MOCK_COURSE_EXISTS, return_value=False) - def test_not_found_type_uri(self, mock_exists): # noqa: ARG002 - """The ``type`` field for 404 must be the ADR 0029 not-found URI.""" - staff = UserFactory.create(is_staff=True) - self.client.force_authenticate(user=staff) - response = self.client.patch(self.url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_404_NOT_FOUND - assert response.data.get("type") == "https://docs.openedx.org/errors/not-found" - - def test_error_body_has_no_developer_message(self): - """Error responses must NOT contain old DeveloperErrorViewMixin fields.""" - response = self.client.patch(self.url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_401_UNAUTHORIZED - assert "developer_message" not in response.data - assert "error_code" not in response.data - - def test_instance_field_is_request_path(self): - """The ``instance`` field must equal the request path.""" - response = self.client.patch(self.url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_401_UNAUTHORIZED - assert response.data.get("instance") == self.url + def _twin_courses(self): + return seed_course(self.staff), seed_course(self.staff) - def test_v0_endpoint_unaffected_by_v3_envelope(self): + def _apply(self, request, course, body): + """Send ``body`` through ``request``; return status, bytes, policy before and after, and effects.""" + before = stored_policy(course.id) + with capture_grading_effects() as effects: + response = request(course, body) + effects["credit_tasks"] = [tuple(arg.replace(str(course.id), "") for arg in args) + for args in effects["credit_tasks"]] + content = response.content.replace(str(course.id).encode(), b"") + return response.status_code, content, before, stored_policy(course.id), effects + + def _v0(self, course, body): + return _post(self.client, v0_url(course.id), body) + + def _v3(self, mount): + return lambda course, body: _patch(self.client, grading_url(mount, course.id), body) + + def test_whole_policy_bodies_behave_as_the_v0_update(self): + for mount, name, body in FULL_BODY_CASES: + with self.subTest(mount=mount, body=name): + reference_course, course = self._twin_courses() + expected = self._apply(self._v0, reference_course, body) + actual = self._apply(self._v3(mount), course, body) + assert expected[0] == 200, expected[1] + assert actual[1] == expected[1] # response bytes + assert actual[3] == expected[3] # stored policy + assert actual[4] == expected[4] # signals, events, credit task + assert_matches_published_schema(json.loads(actual[1]), DEFAULT_SHAPE) + + def test_resending_the_stored_policy_is_announced_only_without_recompute_only(self): + """Pins which side of the flag this class runs on.""" + for mount in MOUNTS: + with self.subTest(mount=mount): + _, course = self._twin_courses() + effects = self._apply(self._v3(mount), course, FULL_BODIES["stored_policy_resent"])[4] + expected = 0 if self.recompute_only else 1 + assert (effects["signals"], effects["events"]) == (expected, expected) + + def test_displayed_weights_are_stored_truncated(self): + """A weight of 0.29 is shown as 28, so sending back what was shown stores 0.28.""" + _, course = self._twin_courses() + body = FULL_BODIES["displayed_weights_resent"] + status, content, _, after, _ = self._apply(self._v3(CONFORMING), course, body) + assert status == 200 + assert json.loads(content)["graders"][0]["weight"] == 28 + assert after["raw_grader"][0]["weight"] == 0.28 + + def test_partial_bodies_change_only_the_fields_sent(self): + seeded_weights = [grader["weight"] for grader in SEED_GRADERS] + cases = { + # name: (body, fields that change, grading signals) + "graders_only": ({"graders": GRADERS}, {"raw_grader"}, 1), + "stored_graders_only": ({"graders": SEED_GRADERS_AS_SENT}, set(), 0 if self.recompute_only else 1), + "cutoffs_only": ({"grade_cutoffs": CUTOFFS}, {"grade_cutoffs"}, 1), + "stored_cutoffs_only": ({"grade_cutoffs": SEED_CUTOFFS}, set(), 0), + "grace_period_only": ({"grace_period": {"hours": 5}}, {"graceperiod"}, 0), + "grace_period_null_only": ({"grace_period": None}, set(), 0), + "credit_only": ({"minimum_grade_credit": 0.6}, {"minimum_grade_credit"}, 0), + "credit_null_only": ({"minimum_grade_credit": None}, set(), 0), + "empty": ({}, set(), 0), + "graders_and_credit": ({"graders": GRADERS, "minimum_grade_credit": 0.55}, + {"raw_grader", "minimum_grade_credit"}, 1), + "cutoffs_and_grace_period": ({"grade_cutoffs": CUTOFFS, "grace_period": {"hours": 1}}, + {"grade_cutoffs", "graceperiod"}, 1), + "graders_cutoffs_and_grace_period": ({"graders": GRADERS, "grade_cutoffs": CUTOFFS, + "grace_period": {"minutes": 9}}, + {"raw_grader", "grade_cutoffs", "graceperiod"}, 1), + } + for mount in MOUNTS: + for name, (body, changed, signals) in cases.items(): + with self.subTest(mount=mount, body=name): + reference_course, course = self._twin_courses() + if set(body) != {"graders", "grade_cutoffs", "grace_period", "minimum_grade_credit"}: + # The whole-policy update cannot take a partial body. + assert self._apply(self._v0, reference_course, body)[0] == 500 + status, content, before, after, effects = self._apply(self._v3(mount), course, body) + assert status == 200, content + assert {field for field in before if before[field] != after[field]} == changed + if "graders" not in body: + assert [grader["weight"] for grader in after["raw_grader"]] == seeded_weights + assert (effects["signals"], effects["events"]) == (signals, signals) + expected_tasks = [("",)] if "minimum_grade_credit" in body else [] + assert effects["credit_tasks"] == expected_tasks + assert json.loads(content) == json.loads( + self.client.get(grading_url(mount, course.id)).content.replace( + str(course.id).encode(), b"") + ) + + def test_partial_bodies_without_graders_never_rewrite_stored_weights(self): """ - The ADR 0029 envelope must be scoped to v3 — hitting the legacy v0 - ``grading`` endpoint unauthenticated must NOT return the v3 envelope - (no ``type`` / ``instance`` keys). + A weight such as 0.1019 does not survive the model's own percentage round trip + (0.1019 * 100 / 100 == 0.10189999...), so any write-back of unsent graders shows. """ - v0_url = reverse( - "cms.djangoapps.contentstore:v0:cms_api_update_grading", - # v0 URL uses the legacy ``course_id`` named group; only v3 was renamed - # to ``course_key`` per OEP-68. - kwargs={"course_id": COURSE_ID}, + drifting = [{**SEED_GRADERS[0], "weight": 0.1019}, *SEED_GRADERS[1:]] + assert drifting[0]["weight"] * 100 / 100 != drifting[0]["weight"] + bodies = ( + {}, {"grade_cutoffs": CUTOFFS}, {"grade_cutoffs": SEED_CUTOFFS}, {"grace_period": {"hours": 1}}, + {"grace_period": None}, {"minimum_grade_credit": 0.6}, {"minimum_grade_credit": None}, ) - response = self.client.post(v0_url, data={}, content_type="application/json") - assert response.status_code == status.HTTP_401_UNAUTHORIZED - assert "type" not in response.data - assert "instance" not in response.data + for mount in MOUNTS: + for body in bodies: + with self.subTest(mount=mount, body=body): + course = seed_course(self.staff) + course.raw_grader = [dict(grader) for grader in drifting] + modulestore().update_item(course, self.staff.id) + status = self._apply(self._v3(mount), course, body)[0] + assert status == 200 + assert stored_policy(course.id)["raw_grader"] == drifting + def test_partial_graders_keep_the_stored_cutoffs_grace_period_and_credit_minimum(self): + _, course = self._twin_courses() + _, _, before, after, _ = self._apply(self._v3(LEGACY), course, {"graders": GRADERS}) + assert after["grade_cutoffs"] == SEED_CUTOFFS + assert after["graceperiod"] == SEED_GRACE + assert after["minimum_grade_credit"] == SEED_CREDIT + assert after["raw_grader"] != before["raw_grader"] -class TestAuthoringGradingViewSetUrlStructure(APITestCase): - """The conforming courses/{course_key}/grading/ route serves the same view as the legacy one.""" + def test_bodies_the_model_cannot_store_are_refused_before_anything_is_written(self): + for mount in MOUNTS: + for name, (body, field) in REJECTED_BODIES.items(): + with self.subTest(mount=mount, body=name): + reference_course, course = self._twin_courses() + assert self._apply(self._v0, reference_course, body)[0] == 500 + with capture_grading_effects() as effects: + response = _patch(self.client, grading_url(mount, course.id), body) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert list(envelope["errors"]) == [field] + assert effects == {"signals": 0, "events": 0, "credit_tasks": []} + _, fresh = self._twin_courses() + assert stored_policy(course.id) == stored_policy(fresh.id) + assert modulestore().get_course(course.id).lowest_passing_grade == min(SEED_CUTOFFS.values()) - def setUp(self): - super().setUp() - self.client = APIClient() - self.conforming_url = reverse( - "authoring_v3:course_grading", - kwargs={"course_key": COURSE_ID}, - ) - self.legacy_url = reverse( - "cms.djangoapps.contentstore:v3:authoring_grading-detail", - kwargs={"course_key": COURSE_ID}, + def test_numbers_too_large_to_store_are_refused_before_anything_is_written(self): + for mount in MOUNTS: + for name, (body, field) in TOO_LARGE_BODIES.items(): + with self.subTest(mount=mount, body=name): + course = seed_course(self.staff) + with capture_grading_effects() as effects: + response = _patch(self.client, grading_url(mount, course.id), body) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert list(envelope["errors"]) == [field] + assert effects == {"signals": 0, "events": 0, "credit_tasks": []} + assert stored_policy(course.id) == stored_policy(seed_course(self.staff).id) + + def test_a_form_encoded_body_is_refused(self): + _, course = self._twin_courses() + response = self.client.patch( + grading_url(CONFORMING, course.id), + data="graders[0]type=Homework&grade_cutoffs=x", + content_type="application/x-www-form-urlencoded", ) + envelope = assert_error_envelope(response, expected_status=400, expected_type_slug="validation") + assert list(envelope["errors"]) == ["non_field_errors"] + _, fresh = self._twin_courses() + assert stored_policy(course.id) == stored_policy(fresh.id) + + +class WholePolicyBodyCasesTest(TestCase): + """Every whole-policy body is replayed on at least one mount.""" + + def test_hand_picked_bodies_run_on_both_mounts_and_generated_bodies_on_one(self): + mounts_by_body = {} + for mount, name, _ in FULL_BODY_CASES: + mounts_by_body.setdefault(name, []).append(mount) + assert len(FULL_BODIES) == len(HAND_PICKED_FULL_BODIES) + len(GENERATED_FULL_BODIES) + assert set(mounts_by_body) == set(FULL_BODIES) + for name in HAND_PICKED_FULL_BODIES: + assert sorted(mounts_by_body[name]) == sorted(MOUNTS), name + for name in GENERATED_FULL_BODIES: + assert len(mounts_by_body[name]) == 1, name + per_mount = [sum(mounts_by_body[name] == [mount] for name in GENERATED_FULL_BODIES) for mount in MOUNTS] + assert max(per_mount) - min(per_mount) <= 1, per_mount + + +class GradingUpdateTest(_UpdateBehaviourMixin, ModuleStoreTestCase): + """Updates when every grading change is announced.""" + + recompute_only = False + + +class GradingUpdateRecomputeOnlyTest(_UpdateBehaviourMixin, ModuleStoreTestCase): + """Updates when only changes to weights or counts are announced.""" + + recompute_only = True + + +# --------------------------------------------------------------------------- +# Routing +# --------------------------------------------------------------------------- + +COURSE_KEY = "course-v1:edX+ToyX+Toy_Course" + + +@ddt.ddt +class GradingRoutingTest(SharedModuleStoreTestCase): + """Both addresses serve one view with the same actions.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def test_detail_url_resolves(self): + assert grading_url(LEGACY, COURSE_KEY) == f"/api/contentstore/v3/authoring_grading/{COURSE_KEY}/" def test_conforming_url_reverses_to_expected_path(self): - assert self.conforming_url == f"/api/authoring/v3/courses/{COURSE_ID}/grading/" + assert grading_url(CONFORMING, COURSE_KEY) == f"/api/authoring/v3/courses/{COURSE_KEY}/grading/" def test_conforming_and_legacy_routes_share_view_and_actions(self): - legacy = resolve(self.legacy_url).func - conforming = resolve(self.conforming_url).func + legacy = resolve(grading_url(LEGACY, COURSE_KEY)).func + conforming = resolve(grading_url(CONFORMING, COURSE_KEY)).func assert conforming.cls is legacy.cls - assert routed_actions(conforming) == routed_actions(legacy) + assert routed_actions(conforming) == routed_actions(legacy) == { + "get": "retrieve", + "patch": "partial_update", + } - def test_invalid_course_key_is_404_on_conforming_route(self): - # The shared course_key converter rejects unparseable keys with a - # routing-level 404. - response = self.client.patch( - "/api/authoring/v3/courses/not-a-course-key/grading/", - data={}, format="json", + def test_legacy_format_suffix_reads_and_updates_like_the_plain_address(self): + """The legacy router also routes ``.json`` to the same actions.""" + client = _client(UserFactory(is_staff=True)) + suffixed = reverse( + "cms.djangoapps.contentstore:v3:authoring_grading-detail", + kwargs={"course_key": str(self.course.id), "format": "json"}, + ) + assert suffixed == f"/api/contentstore/v3/authoring_grading/{self.course.id}.json" + assert resolve(suffixed).func.cls is resolve(grading_url(LEGACY, self.course.id)).func.cls + + read = client.get(suffixed) + assert read.status_code == 200, read.content + assert read.json() == client.get(grading_url(LEGACY, self.course.id)).json() + + with patch(CREDIT_TASK): + updated = _patch(client, suffixed, {"grace_period": {"hours": 7}}) + assert updated.status_code == 200, updated.content + assert updated.json()["grace_period"] == {"hours": 7, "minutes": 0, "seconds": 0} + assert updated.json() == client.get(grading_url(LEGACY, self.course.id)).json() + + # The error library does not catalogue MethodNotAllowed, ParseError, + # UnsupportedMediaType or NotAcceptable, so these four answer with the + # envelope typed internal. Change the expected type here once it does. + + @ddt.data(*[(mount, method) for mount in MOUNTS for method in ("post", "put", "delete")]) + @ddt.unpack + def test_other_methods_are_not_allowed(self, mount, method): + client = _client(UserFactory(is_staff=True)) + response = getattr(client, method)(grading_url(mount, self.course.id), data={}, format="json") + assert_error_envelope(response, expected_status=405, expected_type_slug="internal") + + @ddt.data(*MOUNTS) + def test_malformed_json_is_refused_with_400(self, mount): + client = _client(UserFactory(is_staff=True)) + response = client.patch( + grading_url(mount, self.course.id), data='{"graders": [', content_type="application/json", ) - assert response.status_code == status.HTTP_404_NOT_FOUND + assert_error_envelope(response, expected_status=400, expected_type_slug="internal") + + @ddt.data(*MOUNTS) + def test_unsupported_body_type_is_refused_with_415(self, mount): + client = _client(UserFactory(is_staff=True)) + response = client.patch(grading_url(mount, self.course.id), data="graders", content_type="text/plain") + assert_error_envelope(response, expected_status=415, expected_type_slug="internal") + + @ddt.data(*MOUNTS) + def test_unsupported_accept_is_refused_with_406(self, mount): + client = _client(UserFactory(is_staff=True)) + response = client.get(grading_url(mount, self.course.id), HTTP_ACCEPT="application/xml") + assert_error_envelope(response, expected_status=406, expected_type_slug="internal") + + # These addresses match no route, so the service's HTML 404 page answers them. + # Change these to the error envelope once a catch-all route under each mount + # answers unknown addresses. - def test_version_only_course_key_is_404_on_conforming_route(self): + @ddt.data(*[(key, method) for key in ("not-a-course-key", "edX/ToyX/Toy_Course") for method in ("get", "patch")]) + @ddt.unpack + def test_unparsable_or_slash_form_key_is_404_on_conforming_route(self, key, method): + client = _client(UserFactory(is_staff=True)) + response = getattr(client, method)(f"/api/authoring/v3/courses/{key}/grading/", data={}, format="json") + assert response.status_code == 404 + assert response["Content-Type"].startswith("text/html") + + @ddt.data("get", "patch") + def test_key_without_separators_is_404_on_legacy_route(self, method): + client = _client(UserFactory(is_staff=True)) + response = getattr(client, method)("/api/contentstore/v3/authoring_grading/not-a-key/", data={}, format="json") + assert response.status_code == 404 + assert response["Content-Type"].startswith("text/html") + + @ddt.data("get", "patch") + def test_version_only_course_key_is_404_on_conforming_route(self, method): # The converter accepts a key that names only a version. No course run # is behind it, so the view answers 404 rather than 500. - self.client.force_authenticate(user=UserFactory.create()) - response = self.client.patch( - "/api/authoring/v3/courses/course-v1:version@0123456789abcdef01234567/grading/", - data={}, format="json", + client = _client(UserFactory()) + response = getattr(client, method)( + "/api/authoring/v3/courses/course-v1:version@0123456789abcdef01234567/grading/", data={}, format="json", ) - assert response.status_code == status.HTTP_404_NOT_FOUND - - def test_unauthenticated_patch_returns_401(self): - response = self.client.patch(self.conforming_url, data={}, format="json") - assert response.status_code == status.HTTP_401_UNAUTHORIZED - - @patch(MOCK_CREDIT_TASK) - @patch(MOCK_UPDATE_FROM_JSON, return_value=_MOCK_GRADING_MODEL) - @patch(MOCK_HAS_PERMISSION, return_value=True) - @patch(MOCK_COURSE_EXISTS, return_value=True) - def test_patch_on_conforming_route_updates_grading( - self, mock_exists, mock_perm, mock_update, mock_credit, # noqa: ARG002 - ): - """Same contract on the conforming mount as on the legacy one.""" - user = UserFactory.create() - self.client.force_authenticate(user=user) - response = self.client.patch( - self.conforming_url, - data={"graders": _GRADERS_PAYLOAD}, - format="json", - ) - assert response.status_code == status.HTTP_200_OK - mock_update.assert_called_once() + assert_error_envelope(response, expected_status=404, expected_type_slug="not-found") + + def test_v0_endpoint_unaffected_by_v3_envelope(self): + """The frozen v0 update keeps its own error body.""" + response = _client().post(v0_url(COURSE_KEY), data={}, format="json") + assert response.status_code == 401 + assert "type" not in response.data + assert "instance" not in response.data + assert response.data["developer_message"] == "Authentication credentials were not provided." + + +# --------------------------------------------------------------------------- +# Query counts +# --------------------------------------------------------------------------- + +class _QueryCountMixin: + """ + SQL and Mongo calls per request, measured on the second request so request + caches are warm. A read costs no more than the v1 read, and the full view + costs the same. + """ + + reader = None + + def setUp(self): + super().setUp() + self.client = _client(self.reader) + self.course = CourseFactory.create() + for index in range(2): + section = BlockFactory.create(parent=self.course, category="chapter", display_name=f"S{index}") + BlockFactory.create(parent=section, category="sequential", format="Homework", graded=True) + + def _measure(self, 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"] + + def _v1_read(self): + return self._measure(lambda: self.client.get(v1_url(self.course.id))) + + def _read(self, mount, **params): + return self._measure(lambda: self.client.get(grading_url(mount, self.course.id), params)) + + def test_default_read_costs_no_more_than_the_v1_read(self): + v1_sql, v1_mongo_reads, v1_mongo_writes = self._v1_read() + for mount in MOUNTS: + sql, mongo_reads, mongo_writes = self._read(mount) + assert sql <= v1_sql + assert mongo_reads <= v1_mongo_reads + assert mongo_writes == v1_mongo_writes == 0 + + def test_full_read_costs_the_same_as_the_v1_read(self): + v1 = self._v1_read() + for mount in MOUNTS: + assert self._read(mount, view="full") == v1 + + +@ddt.ddt +class GradingQueryCountTest(_QueryCountMixin, ModuleStoreTestCase): + """ + Pinned counts with policy-based authoring permissions off: SQL queries, + Mongo reads and Mongo writes per request. + """ + + def setUp(self): + self.reader = UserFactory(is_staff=True) + super().setUp() + + @ddt.data(*MOUNTS) + def test_default_read(self, mount): + assert self._read(mount) == (12, 4, 0) + assert self._v1_read() == (14, 4, 0) + + @ddt.data(*MOUNTS) + def test_full_read(self, mount): + assert self._read(mount, view="full") == (14, 4, 0) + + @ddt.data(*MOUNTS) + def test_whole_policy_update(self, mount): + body = { + "graders": [ + {"type": "Homework", "min_count": 2, "drop_count": 0, "short_label": "HW", "weight": 60, "id": 0}, + {"type": "Exam", "min_count": 1, "drop_count": 0, "short_label": "Ex", "weight": 40, "id": 1}, + ], + "grade_cutoffs": {"Pass": 0.5}, + "grace_period": {"hours": 1, "minutes": 0}, + "minimum_grade_credit": 0.8, + } + counts = self._measure(lambda: _patch(self.client, grading_url(mount, self.course.id), body)) + assert counts == (33, 42, 6) + + +class GradingQueryCountAuthzTest(CourseAuthoringAuthzTestMixin, _QueryCountMixin, ModuleStoreTestCase): + """Read costs against the v1 read with policy-based authoring permissions on.""" + + def setUp(self): + super().setUp() + self.client = _client(self.super_user) diff --git a/cms/lib/spectacular.py b/cms/lib/spectacular.py index 662cf80c65f0..22adaf56f112 100644 --- a/cms/lib/spectacular.py +++ b/cms/lib/spectacular.py @@ -5,14 +5,16 @@ 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. +# The YouTube transcript routes make their final character optional, so both +# addresses resolve and each is listed in the truncated spelling as well as the +# full one: which one the generator emits depends on how it renders that regex. 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/contentstore/v0/grading/{course_id}", # → /api/authoring/v3/courses/{course_key}/grading/ + "/api/contentstore/v1/course_grading/{course_id}", # → /api/authoring/v3/courses/{course_key}/grading/ ) # Legacy addresses of APIs migrated to /api/authoring/, marked deprecated for @@ -39,6 +41,17 @@ "/api/authoring/v3/home/", ) +# Response components whose alternative shapes overlap, published as anyOf +# instead of drf-spectacular's oneOf. The default grading shape allows extra +# keys, so a full read matches both shapes and a strict oneOf validator would +# reject it. +ANY_OF_COMPONENTS = { + "authoring_grading.CourseGradingRead.v3": ( + "authoring_grading.CourseGradingFull.v3", + "authoring_grading.CourseGrading.v3", + ), +} + def cms_api_filter(endpoints): """ @@ -90,11 +103,25 @@ def get_operation_id(self): return operation_id +def _publish_overlapping_shapes_as_any_of(result): + """Turn the oneOf of each component in ANY_OF_COMPONENTS into anyOf, if it lists exactly the named shapes.""" + schemas = result.get("components", {}).get("schemas", {}) + for name, shapes in ANY_OF_COMPONENTS.items(): + component = schemas.get(name) + if not component or "oneOf" not in component: + continue + refs = [item.get("$ref") for item in component["oneOf"]] + if refs == [f"#/components/schemas/{shape}" for shape in shapes]: + component["anyOf"] = component.pop("oneOf") + + def cms_mark_migrated_paths(result, generator, request, public): # pylint: disable=unused-argument """ Post-processing hook: mark the legacy addresses of - migrated APIs ``deprecated: true`` and BFF surfaces ``x-internal``. + migrated APIs ``deprecated: true`` and BFF surfaces ``x-internal``, + and publish the components in ANY_OF_COMPONENTS as anyOf. """ + _publish_overlapping_shapes_as_any_of(result) for path, path_item in result.get("paths", {}).items(): legacy = path.startswith(LEGACY_MIGRATED_PATH_PREFIXES) internal = path.startswith(INTERNAL_BFF_PATH_PREFIXES) diff --git a/cms/lib/tests/test_spectacular.py b/cms/lib/tests/test_spectacular.py index 4581f23a6148..359ab46d0420 100644 --- a/cms/lib/tests/test_spectacular.py +++ b/cms/lib/tests/test_spectacular.py @@ -1,9 +1,13 @@ """Tests for the CMS drf-spectacular hooks and schema class.""" +from importlib import import_module from pathlib import Path from unittest import TestCase, mock +from django.conf import settings from django.test import SimpleTestCase +from django.test import TestCase as DjangoTestCase +from django.urls import include from django.urls import path as url_path from drf_spectacular.generators import SchemaGenerator from drf_spectacular.settings import spectacular_settings @@ -12,6 +16,7 @@ from rest_framework.settings import api_settings from cms.lib.spectacular import ( + ANY_OF_COMPONENTS, SUPERSEDED_PATHS, CmsAutoSchema, cms_api_filter, @@ -76,6 +81,8 @@ 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/grading/{course_id}", + "/api/contentstore/v1/course_grading/{course_id}", } def test_the_conforming_paths_are_left_unmarked(self): @@ -134,6 +141,42 @@ def test_paths_are_left_as_full_urls(self): assert list(result["paths"]) == ["/api/contentstore/v1/xblock/"] +def _alternatives(*names): + return [{"$ref": f"#/components/schemas/{name}"} for name in names] + + +class OverlappingShapesTest(SimpleTestCase): + """The post-processing hook publishes the listed overlapping response shapes as anyOf, and nothing else.""" + + READ = "authoring_grading.CourseGradingRead.v3" + SHAPES = ("authoring_grading.CourseGradingFull.v3", "authoring_grading.CourseGrading.v3") + + def _run(self, schemas): + result = {"paths": {}, "components": {"schemas": schemas}} + return cms_mark_migrated_paths(result, None, None, True)["components"]["schemas"] + + def test_only_the_grading_read_is_listed(self): + assert ANY_OF_COMPONENTS == {self.READ: self.SHAPES} + + def test_the_grading_read_becomes_any_of(self): + schemas = self._run({self.READ: {"oneOf": _alternatives(*self.SHAPES)}}) + assert schemas[self.READ] == {"anyOf": _alternatives(*self.SHAPES)} + + def test_other_one_of_components_are_left_alone(self): + other = {"oneOf": _alternatives("A", "B")} + schemas = self._run({"Other": other, self.READ: {"oneOf": _alternatives(*self.SHAPES)}}) + assert schemas["Other"] == {"oneOf": _alternatives("A", "B")} + + def test_the_grading_read_with_other_shapes_is_left_alone(self): + """A change to the shapes the read offers needs a fresh look at whether they overlap.""" + for shapes in (self.SHAPES[::-1], self.SHAPES[:1], (*self.SHAPES, "authoring_grading.Other.v3")): + schemas = self._run({self.READ: {"oneOf": _alternatives(*shapes)}}) + assert schemas[self.READ] == {"oneOf": _alternatives(*shapes)}, shapes + + def test_a_schema_without_components_is_accepted(self): + assert cms_mark_migrated_paths({"paths": {}}, None, None, True) == {"paths": {}} + + class SpectacularSettingsTest(TestCase): """ The service settings the hooks depend on. @@ -210,3 +253,139 @@ def test_only_the_colliding_legacy_address_is_suffixed(self): # Already distinct from its conforming twin's id, so it keeps its name. assert ids["/api/authoring/v4/courses/"] == "v4_courses_list" assert ids["/api/contentstore/v4/home/courses/"] == "v4_home_courses_list" + + +class GradingSchemaTest(DjangoTestCase): + """The grading operations as published, generated from the real contentstore and authoring routes.""" + + CONFORMING = "/api/authoring/v3/courses/{course_key}/grading/" + LEGACY_V3 = "/api/contentstore/v3/authoring_grading/{course_key}/" + V1 = "/api/contentstore/v1/course_grading/{course_id}" + V0 = "/api/contentstore/v0/grading/{course_id}" + + @classmethod + def setUpClass(cls): + super().setUpClass() + # Importing the service URLconf registers the course key converter the authoring routes use. + import_module(settings.ROOT_URLCONF) + patterns = [ + url_path("api/contentstore/", include("cms.djangoapps.contentstore.rest_api.urls")), + url_path("api/authoring/v3/", include("cms.djangoapps.contentstore.rest_api.v3.authoring_urls")), + ] + cls.schema = SchemaGenerator(patterns=patterns).get_schema(request=None, public=True) + + def _operation(self, path, method): + return self.schema["paths"][path][method] + + @staticmethod + def _ref(response_or_body): + return response_or_body["content"]["application/json"]["schema"]["$ref"].rsplit("/", 1)[-1] + + def test_the_superseded_operations_are_deprecated(self): + assert self._operation(self.V1, "get")["deprecated"] is True + assert self._operation(self.V1, "post")["deprecated"] is True + assert self._operation(self.V0, "post")["deprecated"] is True + + def test_the_read_on_the_conforming_address_is_published_for_the_sdk(self): + operation = self._operation(self.CONFORMING, "get") + assert "deprecated" not in operation + assert operation["operationId"] == "v3_courses_grading_retrieve" + assert operation["tags"] == ["openedx-platform-sdk"] + view = [p for p in operation["parameters"] if p["name"] == "view"] + assert len(view) == 1 + assert view[0]["in"] == "query" + assert "required" not in view[0] + assert view[0]["schema"]["enum"] == ["full"] + assert self._ref(operation["responses"]["200"]) == "authoring_grading.CourseGradingRead.v3" + for status in ("400", "401", "403", "404"): + assert self._ref(operation["responses"][status]) == "ErrorResponse", status + + def test_the_read_on_the_legacy_address_is_deprecated(self): + operation = self._operation(self.LEGACY_V3, "get") + assert operation["deprecated"] is True + assert operation["operationId"] == "v3_authoring_grading_retrieve" + + def test_the_update_takes_the_update_body_and_returns_the_policy(self): + for path in (self.CONFORMING, self.LEGACY_V3): + operation = self._operation(path, "patch") + assert self._ref(operation["requestBody"]) == "Patchedauthoring_grading.CourseGradingUpdate.v3", path + assert self._ref(operation["responses"]["200"]) == "authoring_grading.CourseGrading.v3", path + for status in ("400", "401", "403", "404"): + assert self._ref(operation["responses"][status]) == "ErrorResponse", (path, status) + assert "deprecated" not in self._operation(self.CONFORMING, "patch") + assert self._operation(self.LEGACY_V3, "patch")["deprecated"] is True + + def test_the_update_body_requires_nothing_but_a_type_per_grader(self): + components = self.schema["components"]["schemas"] + assert "required" not in components["Patchedauthoring_grading.CourseGradingUpdate.v3"] + assert components["authoring_grading.GraderUpdate.v3"]["required"] == ["type"] + assert "required" not in components["authoring_grading.GracePeriodUpdate.v3"] + + def test_the_update_body_is_json_only(self): + for path in (self.CONFORMING, self.LEGACY_V3): + assert list(self._operation(path, "patch")["requestBody"]["content"]) == ["application/json"], path + + def test_the_update_body_types(self): + components = self.schema["components"]["schemas"] + update = components["Patchedauthoring_grading.CourseGradingUpdate.v3"]["properties"] + assert update["grade_cutoffs"]["additionalProperties"] == {"type": "number", "format": "double"} + assert update["grade_cutoffs"]["minProperties"] == 1 + assert "nullable" not in update["grade_cutoffs"] + assert {key: update["minimum_grade_credit"][key] for key in ("type", "format", "nullable")} == { + "type": "number", "format": "double", "nullable": True, + } + assert "nullable" not in update["graders"] + assert update["grace_period"]["allOf"] == [ + {"$ref": "#/components/schemas/authoring_grading.GracePeriodUpdate.v3"}, + ] + assert update["grace_period"]["nullable"] is True + grace = components["authoring_grading.GracePeriodUpdate.v3"]["properties"] + assert {unit: (grace[unit]["type"], grace[unit]["default"]) for unit in grace} == { + "hours": ("integer", 0), "minutes": ("integer", 0), "seconds": ("integer", 0), + } + + def test_the_read_documents_the_full_and_the_default_shape(self): + components = self.schema["components"]["schemas"] + # anyOf, as a full response also matches the default shape. Full first, so a + # client trying each shape in order reads a full response as full. + assert components["authoring_grading.CourseGradingRead.v3"] == {"anyOf": [ + {"$ref": "#/components/schemas/authoring_grading.CourseGradingFull.v3"}, + {"$ref": "#/components/schemas/authoring_grading.CourseGrading.v3"}, + ]} + for path in (self.CONFORMING, self.LEGACY_V3): + response = self._operation(path, "get")["responses"]["200"] + assert self._ref(response) == "authoring_grading.CourseGradingRead.v3", path + policy = {"graders", "grade_cutoffs", "grace_period", "minimum_grade_credit"} + extras = { + "course_assignment_lists", "default_grade_designations", "is_credit_course", + "show_credit_eligibility", "mfe_proctored_exam_settings_url", + } + default = components["authoring_grading.CourseGrading.v3"] + full = components["authoring_grading.CourseGradingFull.v3"] + assert set(default["properties"]) == set(default["required"]) == policy + assert set(full["properties"]) == set(full["required"]) == policy | extras + + def test_the_policy_response_marks_what_is_always_present_and_what_can_be_null(self): + components = self.schema["components"]["schemas"] + + def nullable(component): + properties = components[component]["properties"] + return {name for name, schema in properties.items() if schema.get("nullable")} + + for component in ("authoring_grading.CourseGrading.v3", "authoring_grading.CourseGradingFull.v3"): + assert nullable(component) == {"grade_cutoffs", "grace_period", "minimum_grade_credit"}, component + cutoffs = components[component]["properties"]["grade_cutoffs"] + assert cutoffs["additionalProperties"] == {"type": "number", "format": "double", "nullable": True} + grader = components["authoring_grading.Grader.v3"] + assert set(grader["required"]) == set(grader["properties"]) == { + "type", "min_count", "drop_count", "short_label", "weight", "id", + } + assert nullable("authoring_grading.Grader.v3") == {"type", "short_label"} + grace = components["authoring_grading.GracePeriod.v3"] + assert grace["required"] == ["hours", "minutes", "seconds"] + assert nullable("authoring_grading.GracePeriod.v3") == set() + + def test_only_get_and_patch_are_published_on_the_grading_addresses(self): + for path in (self.CONFORMING, self.LEGACY_V3): + methods = {key for key in self.schema["paths"][path] if key != "parameters"} + assert methods == {"get", "patch"}, path