From 545aa7a21930db2d84ea5cf48874ee95f086efbf Mon Sep 17 00:00:00 2001 From: javoconsultant Date: Fri, 11 Sep 2026 14:48:13 +0500 Subject: [PATCH] feat: add read-only REST endpoint for CBE rule profiles --- projects/urls.py | 2 + src/openedx_learning/README.rst | 31 +++ src/openedx_learning/applets/cbe/api.py | 18 +- .../applets/cbe/rest_api/__init__.py | 0 .../applets/cbe/rest_api/paginators.py | 20 ++ .../applets/cbe/rest_api/urls.py | 14 + .../applets/cbe/rest_api/v1/__init__.py | 0 .../applets/cbe/rest_api/v1/permissions.py | 19 ++ .../applets/cbe/rest_api/v1/serializers.py | 56 ++++ .../applets/cbe/rest_api/v1/urls.py | 15 ++ .../applets/cbe/rest_api/v1/views.py | 46 ++++ src/openedx_learning/applets/cbe/rules.py | 57 +++++ src/openedx_learning/rules.py | 9 + src/openedx_learning/urls.py | 16 ++ .../openedx_learning/applets/cbe/conftest.py | 23 +- .../openedx_learning/applets/cbe/test_api.py | 60 ++++- .../applets/cbe/test_rules.py | 72 ++++++ .../applets/cbe/test_views.py | 240 ++++++++++++++++++ 18 files changed, 693 insertions(+), 5 deletions(-) create mode 100644 src/openedx_learning/applets/cbe/rest_api/__init__.py create mode 100644 src/openedx_learning/applets/cbe/rest_api/paginators.py create mode 100644 src/openedx_learning/applets/cbe/rest_api/urls.py create mode 100644 src/openedx_learning/applets/cbe/rest_api/v1/__init__.py create mode 100644 src/openedx_learning/applets/cbe/rest_api/v1/permissions.py create mode 100644 src/openedx_learning/applets/cbe/rest_api/v1/serializers.py create mode 100644 src/openedx_learning/applets/cbe/rest_api/v1/urls.py create mode 100644 src/openedx_learning/applets/cbe/rest_api/v1/views.py create mode 100644 src/openedx_learning/applets/cbe/rules.py create mode 100644 src/openedx_learning/rules.py create mode 100644 src/openedx_learning/urls.py create mode 100644 tests/openedx_learning/applets/cbe/test_rules.py create mode 100644 tests/openedx_learning/applets/cbe/test_views.py diff --git a/projects/urls.py b/projects/urls.py index 1e1f0d7c1..b9feea3b3 100644 --- a/projects/urls.py +++ b/projects/urls.py @@ -8,5 +8,7 @@ path("admin/doc/", include("django.contrib.admindocs.urls")), path("admin/", admin.site.urls), path("tagging/rest_api/", include("openedx_tagging.urls")), + # Mirrors the prefix openedx-platform mounts CBE at; a consuming project picks its own. + path("api/", include("openedx_learning.urls")), path('__debug__/', include('debug_toolbar.urls')), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) diff --git a/src/openedx_learning/README.rst b/src/openedx_learning/README.rst index ecceb6b2b..ba21bcfb8 100644 --- a/src/openedx_learning/README.rst +++ b/src/openedx_learning/README.rst @@ -9,3 +9,34 @@ Like ``openedx_content``, it is one Django app split into applets. Its first app In the layering that ``.importlinter`` enforces, this app sits above ``openedx_catalog``, ``openedx_content`` and ``openedx_tagging``. It may build on any of them; none may import it. + +Setting up a consuming project +------------------------------ + +A project that uses this app's REST API needs three things in place. Its URLconf includes +``openedx_learning.urls`` under whatever prefix it likes, and the CBE API is then served under +``cbe/v1/``. Its ``INSTALLED_APPS`` lists ``rules.apps.AutodiscoverRulesConfig``, not +plain ``rules``, because only the autodiscovering config imports each app's ``rules.py`` and so +registers the permissions declared there. Its ``AUTHENTICATION_BACKENDS`` includes +``rules.permissions.ObjectPermissionBackend``, the backend that answers ``has_perm`` from those +registered permissions. If either ``rules`` setting is missing, the API answers 403 to every +caller who is not a superuser, and nothing in the logs says why. Superusers pass every +``has_perm`` check regardless, so testing as one hides the problem. + +.. code-block:: python + + # settings + INSTALLED_APPS = [ + ..., + "rules.apps.AutodiscoverRulesConfig", + "openedx_learning", + ] + AUTHENTICATION_BACKENDS = [ + ..., + "rules.permissions.ObjectPermissionBackend", + ] + + # urls.py: serves the CBE API under api/cbe/v1/ + urlpatterns = [ + path("api/", include("openedx_learning.urls")), + ] diff --git a/src/openedx_learning/applets/cbe/api.py b/src/openedx_learning/applets/cbe/api.py index a137901e1..4aed9cba8 100644 --- a/src/openedx_learning/applets/cbe/api.py +++ b/src/openedx_learning/applets/cbe/api.py @@ -8,10 +8,11 @@ from openedx_tagging.api import create_taxonomy from openedx_tagging.models import Taxonomy -from .models import CompetencyTaxonomy +from .models import CompetencyRuleProfile, CompetencyTaxonomy __all__ = [ "create_competency_taxonomy", + "get_competency_rule_profiles", "is_competency_taxonomy", "select_competency_taxonomies", ] @@ -43,6 +44,21 @@ def create_competency_taxonomy( # pylint: disable=too-many-positional-arguments return taxonomy +def get_competency_rule_profiles() -> QuerySet[CompetencyRuleProfile]: + """ + Return every live CompetencyRuleProfile, in ascending ``id`` order. + + UNSTABLE: the rule profile family is incomplete, so the create, update, and archive entry + points still to come may change this function's shape without a deprecation cycle. + + Archived profiles are left out: retirement is archive-only. + + The ordering is part of the contract rather than a cosmetic detail: an unordered queryset + gives a paginating caller overlapping and skipped pages. + """ + return CompetencyRuleProfile.objects.filter(archived=False).order_by("id") + + def is_competency_taxonomy(taxonomy: Taxonomy) -> bool: """ Return True if ``taxonomy`` is competency-enabled, i.e. has a CompetencyTaxonomy row. diff --git a/src/openedx_learning/applets/cbe/rest_api/__init__.py b/src/openedx_learning/applets/cbe/rest_api/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/openedx_learning/applets/cbe/rest_api/paginators.py b/src/openedx_learning/applets/cbe/rest_api/paginators.py new file mode 100644 index 000000000..ea23f67c9 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/paginators.py @@ -0,0 +1,20 @@ +""" +Paginators for the CBE REST API. + +These sit outside the versioned package, as openedx_tagging's do, because a page size is +version independent. +""" +from edx_rest_framework_extensions.paginators import DefaultPagination # type: ignore[import] + + +class CompetencyRuleProfilePagination(DefaultPagination): + """ + Page size for the competency rule profile collection. + + Pinned here rather than inherited from the consuming project's DEFAULT_PAGINATION_CLASS, the + same way openedx_tagging pins TaxonomyPagination: this is a published library, so the page + size is part of its REST contract instead of varying by deployment. + """ + + page_size = 100 + max_page_size = 500 diff --git a/src/openedx_learning/applets/cbe/rest_api/urls.py b/src/openedx_learning/applets/cbe/rest_api/urls.py new file mode 100644 index 000000000..8b882d982 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/urls.py @@ -0,0 +1,14 @@ +""" +CBE API URLs. +""" + +from django.urls import include, path + +from .v1 import urls as v1_urls + +# Namespaces the whole CBE API, so routes reverse as "cbe:". Declared here rather than in +# v1/ because the namespace spans every version, and rather than on the app-root module because +# that one aggregates every applet. +app_name = "cbe" + +urlpatterns = [path("v1/", include(v1_urls))] diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/__init__.py b/src/openedx_learning/applets/cbe/rest_api/v1/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/permissions.py b/src/openedx_learning/applets/cbe/rest_api/v1/permissions.py new file mode 100644 index 000000000..2c3cf186f --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/v1/permissions.py @@ -0,0 +1,19 @@ +""" +Permissions for the CBE REST API, v1. +""" +from rest_framework.permissions import DjangoObjectPermissions + + +class CompetencyRuleProfilePermissions(DjangoObjectPermissions): + """ + Maps each REST API method to its corresponding CompetencyRuleProfile permission. + + Only the read methods are mapped, so DRF answers a write attempt with 405 rather than + checking a permission that no write endpoint would honor anyway. + """ + + perms_map = { + "GET": ["%(app_label)s.view_%(model_name)s"], + "OPTIONS": [], + "HEAD": ["%(app_label)s.view_%(model_name)s"], + } diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/serializers.py b/src/openedx_learning/applets/cbe/rest_api/v1/serializers.py new file mode 100644 index 000000000..02ee97927 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/v1/serializers.py @@ -0,0 +1,56 @@ +""" +Serializers for the CBE REST API, v1. +""" +from __future__ import annotations + +from rest_framework import serializers + +from ...models import CompetencyRuleProfile + + +class CompetencyRuleProfileSerializer(serializers.ModelSerializer): + """ + Read-only representation of a CompetencyRuleProfile. + + UNSTABLE: the rule profile family is incomplete, so the create, update, and archive + endpoints still to come may change this shape without a deprecation cycle. + + ``rule_payload`` is emitted verbatim as stored. For ``Grade``, the only rule type + supported in this phase, that shape is ``{"op": ..., "value": ..., "scale": ...}``: + ``op`` is one of ``gte``, ``lte``, or ``eq``, and ``value`` is a fraction between 0.0 + and 1.0 inclusive. ``GradeRulePayload`` and ``validate_rule_payload`` in the + ``rule_payloads`` module own that shape, so it is not renormalized here. ``scale`` is + always ``"percent"`` for now; it tells a client how to display ``value`` (``0.8`` means 80%). + + ``scope_code`` and the raw ``organization``, ``course``, and ``competency_taxonomy`` + columns are left out: ``scope_type`` below is what a client can act on, while the + others are internal, existing only to enforce the one-profile-per-scope constraint + (:ref:`openedx-learning-adr-0002` Decision 3). + """ + + scope_type = serializers.SerializerMethodField() + + class Meta: + model = CompetencyRuleProfile + fields = ["id", "scope_type", "rule_type", "rule_payload", "archived"] + # scope_type is absent here because DRF refuses a field that is both declared above and + # named in read_only_fields; a SerializerMethodField is read-only in any case. + read_only_fields = ["id", "rule_type", "rule_payload", "archived"] + + def get_scope_type(self, profile: CompetencyRuleProfile) -> str: + """ + Return which kind of scope ``profile`` applies to. + + All four kinds are recognized from the outset, even though only the system default can + exist today, so enabling a narrower scope needs no edit here. The scope columns are read + by their ``_id`` attributes so that no row costs a query, and the system default is + recognized by those columns being null rather than by matching the internal + ``scope_code`` string. + """ + if profile.competency_taxonomy_id is not None: + return "taxonomy" + if profile.course_id is not None: + return "course" + if profile.organization_id is not None: + return "organization" + return "system_default" diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/urls.py b/src/openedx_learning/applets/cbe/rest_api/v1/urls.py new file mode 100644 index 000000000..32b9fd778 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/v1/urls.py @@ -0,0 +1,15 @@ +""" +CBE API v1 URLs. +""" + +from django.urls import include, path +from rest_framework.routers import DefaultRouter + +from . import views + +router = DefaultRouter() +router.register("rule_profiles", views.CompetencyRuleProfileView, basename="rule_profile") + +urlpatterns = [ + path("", include(router.urls)), +] diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/views.py b/src/openedx_learning/applets/cbe/rest_api/v1/views.py new file mode 100644 index 000000000..574ec522a --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/v1/views.py @@ -0,0 +1,46 @@ +""" +Views for the CBE REST API, v1. +""" +from __future__ import annotations + +from django.db.models import QuerySet +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication # type: ignore[import] +from edx_rest_framework_extensions.auth.session.authentication import ( # type: ignore[import] + SessionAuthenticationAllowInactiveUser, +) +from rest_framework import mixins +from rest_framework.viewsets import GenericViewSet + +from ...api import get_competency_rule_profiles +from ...models import CompetencyRuleProfile +from ..paginators import CompetencyRuleProfilePagination +from .permissions import CompetencyRuleProfilePermissions +from .serializers import CompetencyRuleProfileSerializer + + +class CompetencyRuleProfileView(mixins.ListModelMixin, GenericViewSet): + """ + Read the rule profiles this instance defines. + + ``GET api/cbe/v1/rule_profiles/`` lists them to any signed-in caller, paginated by ``page`` + and ``page_size``; ``can_view_competency_rule_profile`` holds the gate. + + UNSTABLE: the rule profile family is incomplete, so the create, update, and archive + endpoints still to come may change this shape without a deprecation cycle. + + This is a collection even while the seeded system default is the only profile an instance + holds, and it is paginated from the first release: wrapping a bare array in an envelope later + would change the top-level JSON type. A viewset rather than a ListAPIView, so the deferred + create and detail routes can be added as further mixins without touching the URL module. + """ + + serializer_class = CompetencyRuleProfileSerializer + permission_classes = [CompetencyRuleProfilePermissions] + pagination_class = CompetencyRuleProfilePagination + # Set here rather than through openedx_tagging's view_auth_classes decorator, which lives in + # another app's REST internals rather than in an API this library publishes. + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + + def get_queryset(self) -> QuerySet[CompetencyRuleProfile]: + """Return the live rule profiles, filtered and ordered by the applet's public API.""" + return get_competency_rule_profiles() diff --git a/src/openedx_learning/applets/cbe/rules.py b/src/openedx_learning/applets/cbe/rules.py new file mode 100644 index 000000000..f5d536bf0 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rules.py @@ -0,0 +1,57 @@ +""" +Django rules-based permissions for Competency-Based Education (CBE). + +Registered with the ``rules`` permission registry on import. ``src/openedx_learning/rules.py`` +is what makes that import happen; see the note there. +""" +from __future__ import annotations + +import rules + +from openedx_tagging.rules import UserType + +from .models import CompetencyRuleProfile + +__all__ = [ + "can_view_competency_rule_profile", +] + + +@rules.predicate +def can_view_competency_rule_profile( + user: UserType, + profile: CompetencyRuleProfile | None = None, +) -> bool: + """ + Decide who may read a rule profile, by the profile's scope. + + Course authors, not only platform staff, need to read rule profiles, so the collection and + the system default are open to anyone signed in. A taxonomy-scoped profile is as visible as + its taxonomy, which is the gate the criteria tree endpoint uses too, through + ``oel_tagging.view_tag``; a profile on a disabled taxonomy stays admin-only. Course- and + organization-scoped profiles are refused to everyone until #645 decides who may see them. + + The signed-in requirement is added where the permission is registered, below, so ``has_perm`` + refuses an anonymous caller whatever view sits in front of it. This predicate alone does not + check it. + + The taxonomy case asks ``has_perm`` rather than calling ``can_view_taxonomy``: openedx-platform + replaces that rule with an org-aware one via ``rules.set_perm``, which a direct call skips. + """ + if profile is None: + # Listing. Any row-level filtering happens in the queryset. + return True + if profile.competency_taxonomy_id is not None: + return user.has_perm("oel_tagging.view_taxonomy", profile.competency_taxonomy) + if profile.course_id is None and profile.organization_id is None: + # The system default: anyone who is signed in may read it. + return True + # Course- and organization-scoped profiles: no authoring flow creates these yet, and neither + # tagging permission describes who may see them. Refused until #645 decides. + return False + + +rules.add_perm( + "openedx_learning.view_competencyruleprofile", + rules.is_authenticated & can_view_competency_rule_profile, +) diff --git a/src/openedx_learning/rules.py b/src/openedx_learning/rules.py new file mode 100644 index 000000000..8364ed0c8 --- /dev/null +++ b/src/openedx_learning/rules.py @@ -0,0 +1,9 @@ +""" +This is where the openedx_learning app's rules-based permissions are registered. + +``rules.apps.AutodiscoverRulesConfig`` imports ``.rules`` and nothing deeper, so +an applet's permissions reach the registry only by being pulled in from here. +""" +# This wildcard import is okay because the applet rules module declares __all__. +# pylint: disable=wildcard-import +from .applets.cbe.rules import * diff --git a/src/openedx_learning/urls.py b/src/openedx_learning/urls.py new file mode 100644 index 000000000..75aec0c5c --- /dev/null +++ b/src/openedx_learning/urls.py @@ -0,0 +1,16 @@ +""" +Learning API URLs. + +Maps each applet onto its own URL segment. A consuming project includes this module and never an +applet package directly, so ``applets/`` stays the internal layout detail that api.py and +models.py already exist to hide. + +No ``app_name`` here on purpose: each applet declares its own, so route names stay unique per +applet rather than sharing one flat namespace. +""" + +from django.urls import include, path + +from .applets.cbe.rest_api import urls as cbe_urls + +urlpatterns = [path("cbe/", include(cbe_urls))] diff --git a/tests/openedx_learning/applets/cbe/conftest.py b/tests/openedx_learning/applets/cbe/conftest.py index 8053fda18..19a04170c 100644 --- a/tests/openedx_learning/applets/cbe/conftest.py +++ b/tests/openedx_learning/applets/cbe/conftest.py @@ -1,10 +1,11 @@ -"""Shared fixtures for the CBE criteria test modules.""" +"""Shared fixtures for the CBE test modules: schema, deletion, tree-integration, status, and REST tests.""" from datetime import datetime, timezone import pytest from django.contrib.auth import get_user_model from organizations.api import ensure_organization from organizations.models import Organization +from rest_framework.test import APIClient from openedx_catalog.models import CatalogCourse, CourseRun from openedx_learning.models import CompetencyCriteriaGroup, CompetencyRuleProfile, CompetencyTaxonomy @@ -70,6 +71,12 @@ def _default_rule_profile() -> CompetencyRuleProfile: ) +@pytest.fixture(name="staff_user") +def _staff_user(): + """A user permitted to administer taxonomies, and so every competency rule profile.""" + return get_user_model().objects.create(username="staff", email="staff@example.com", is_staff=True) + + @pytest.fixture(name="user") def _user(): """Create a single learner for use in these tests.""" @@ -80,3 +87,17 @@ def _user(): def _now() -> datetime: """A single UTC timestamp shared by writes in a test.""" return datetime.now(timezone.utc) + + +@pytest.fixture(name="api_client") +def _api_client() -> APIClient: + """A REST client for a caller the system cannot identify.""" + return APIClient() + + +@pytest.fixture(name="staff_client") +def _staff_client(staff_user) -> APIClient: + """A REST client acting as `staff_user`.""" + client = APIClient() + client.force_authenticate(user=staff_user) + return client diff --git a/tests/openedx_learning/applets/cbe/test_api.py b/tests/openedx_learning/applets/cbe/test_api.py index 53ceff16a..b37d1b15a 100644 --- a/tests/openedx_learning/applets/cbe/test_api.py +++ b/tests/openedx_learning/applets/cbe/test_api.py @@ -2,13 +2,21 @@ Tests for the CBE public API surface (openedx_learning.api). """ import pytest - -from openedx_learning.api import create_competency_taxonomy, is_competency_taxonomy, select_competency_taxonomies -from openedx_learning.models import CompetencyTaxonomy +from organizations.models import Organization + +from openedx_learning.api import ( + create_competency_taxonomy, + get_competency_rule_profiles, + is_competency_taxonomy, + select_competency_taxonomies, +) +from openedx_learning.models import CompetencyRuleProfile, CompetencyTaxonomy, RuleType from openedx_tagging.models import Taxonomy pytestmark = pytest.mark.django_db +GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} + def test_create_competency_taxonomy_saves_both_rows() -> None: """ @@ -104,3 +112,49 @@ def test_select_competency_taxonomies_avoids_n_plus_1(django_assert_num_queries) assert results.count(True) == 2 assert results.count(False) == 1 + + +def test_get_competency_rule_profiles_returns_the_seeded_default( + default_rule_profile: CompetencyRuleProfile, +) -> None: + """get_competency_rule_profiles() returns the system default an instance starts with.""" + assert list(get_competency_rule_profiles()) == [default_rule_profile] + + +def test_get_competency_rule_profiles_excludes_archived( + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, +) -> None: + """get_competency_rule_profiles() leaves retired profiles out.""" + archived = CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, + rule_payload=GRADE_PAYLOAD, + competency_taxonomy=competency_taxonomy, + archived=True, + ) + + profiles = list(get_competency_rule_profiles()) + + assert archived not in profiles + assert profiles == [default_rule_profile] + + +def test_get_competency_rule_profiles_is_ordered_by_id( + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, + organization: Organization, +) -> None: + """ + get_competency_rule_profiles() returns profiles in ascending id order. + + Without a deterministic order, paginating the collection would repeat and skip rows. + """ + taxonomy_scoped = CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, rule_payload=GRADE_PAYLOAD, competency_taxonomy=competency_taxonomy + ) + organization_scoped = CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, rule_payload=GRADE_PAYLOAD, organization=organization + ) + + expected = [default_rule_profile, taxonomy_scoped, organization_scoped] + assert list(get_competency_rule_profiles()) == expected diff --git a/tests/openedx_learning/applets/cbe/test_rules.py b/tests/openedx_learning/applets/cbe/test_rules.py new file mode 100644 index 000000000..6d0bb95ea --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_rules.py @@ -0,0 +1,72 @@ +""" +Tests for the CBE permission predicates. + +Fixtures live in this directory's conftest.py. +""" +import pytest +from django.contrib.auth.models import AnonymousUser + +from openedx_learning.applets.cbe.rules import can_view_competency_rule_profile +from openedx_learning.models import CompetencyRuleProfile, CompetencyTaxonomy, RuleType +from openedx_tagging.rules import UserType + +pytestmark = pytest.mark.django_db + +VIEW_RULE_PROFILE = "openedx_learning.view_competencyruleprofile" + +# A valid payload for the rows these tests create themselves. +GRADE_PAYLOAD = {"op": "gte", "value": 0.6, "scale": "percent"} + + +@pytest.mark.parametrize("user_fixture", ["staff_user", "user"]) +def test_staff_and_non_staff_may_view_the_collection_and_the_system_default( + request: pytest.FixtureRequest, + default_rule_profile: CompetencyRuleProfile, + user_fixture: str, +) -> None: + caller = request.getfixturevalue(user_fixture) + + assert can_view_competency_rule_profile(caller) is True + assert can_view_competency_rule_profile(caller, default_rule_profile) is True + + +def test_a_profile_on_a_disabled_taxonomy_stays_administrator_only( + staff_user: UserType, + user: UserType, +) -> None: + disabled_taxonomy = CompetencyTaxonomy.objects.create(name="Retired", export_id="retired-v1", enabled=False) + profile = CompetencyRuleProfile.objects.create( + competency_taxonomy=disabled_taxonomy, + rule_type=RuleType.GRADE, + rule_payload=dict(GRADE_PAYLOAD), + ) + + assert can_view_competency_rule_profile(user, profile) is False + assert can_view_competency_rule_profile(staff_user, profile) is True + + +@pytest.mark.parametrize("scope_field, scope_fixture", [("course", "course_run"), ("organization", "organization")]) +def test_course_and_organization_scoped_profiles_are_refused_to_everyone( + request: pytest.FixtureRequest, + staff_user: UserType, + user: UserType, + scope_field: str, + scope_fixture: str, +) -> None: + profile = CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, + rule_payload=dict(GRADE_PAYLOAD), + **{scope_field: request.getfixturevalue(scope_fixture)}, + ) + + assert can_view_competency_rule_profile(user, profile) is False + assert can_view_competency_rule_profile(staff_user, profile) is False + + +def test_anonymous_callers_are_refused_by_the_registered_permission( + default_rule_profile: CompetencyRuleProfile, +) -> None: + anonymous = AnonymousUser() + + assert anonymous.has_perm(VIEW_RULE_PROFILE) is False + assert anonymous.has_perm(VIEW_RULE_PROFILE, default_rule_profile) is False diff --git a/tests/openedx_learning/applets/cbe/test_views.py b/tests/openedx_learning/applets/cbe/test_views.py new file mode 100644 index 000000000..b6524edbd --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_views.py @@ -0,0 +1,240 @@ +""" +Tests for the CBE REST API views. + +Fixtures live in this directory's conftest.py. Several scenarios here create rule profile rows +directly rather than through a live call, because no create or archive endpoint exists yet. +""" +import pytest +from django.urls import reverse +from organizations.models import Organization +from rest_framework import status +from rest_framework.test import APIClient + +from openedx_catalog.models import CourseRun +from openedx_learning.models import CompetencyRuleProfile, CompetencyTaxonomy, RuleType + +pytestmark = pytest.mark.django_db + +# What migration 0005 seeds the system default with. Asserted verbatim rather than imported, so +# that a change to the seed surfaces here as a failing contract instead of passing silently. +SEEDED_GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} + +# A different valid payload for rows these tests create, so no assertion about the seeded row +# can pass by accident against a row a test made itself. +FIXTURE_GRADE_PAYLOAD = {"op": "gte", "value": 0.6, "scale": "percent"} + +RESPONSE_FIELDS = {"id", "scope_type", "rule_type", "rule_payload", "archived"} + + +def rule_profiles_url() -> str: + """Return the collection's path, resolved through the router rather than hardcoded.""" + return reverse("cbe:rule_profile-list") + + +def make_profile(**scope) -> CompetencyRuleProfile: + """Create a live rule profile at `scope`, which is at most one of the three scope columns.""" + return CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, rule_payload=dict(FIXTURE_GRADE_PAYLOAD), **scope + ) + + +def test_collection_resolves_to_the_documented_path() -> None: + """ + The router and the mounts compose into the path the CBE tickets name. + + Pinned in one place so the rest of these tests can use reverse() instead. + """ + assert rule_profiles_url() == "/api/cbe/v1/rule_profiles/" + + +def test_read_the_rule_the_instance_requires_for_mastery( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """ + A permitted caller reads the instance-wide default, complete enough to act on. + + The threshold arrives with the scale it is expressed on, so a caller cannot read the + fraction as a percentage or the reverse. + """ + response = staff_client.get(rule_profiles_url()) + + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 1 + profile = response.data["results"][0] + assert profile["id"] == default_rule_profile.id + assert profile["scope_type"] == "system_default" + assert profile["rule_type"] == "Grade" + assert profile["rule_payload"] == SEEDED_GRADE_PAYLOAD + assert profile["archived"] is False + + +def test_response_omits_internal_scope_bookkeeping( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """The response describes scope in terms a caller outside this system can act on.""" + response = staff_client.get(rule_profiles_url()) + + profile = response.data["results"][0] + assert profile["id"] == default_rule_profile.id + assert set(profile.keys()) == RESPONSE_FIELDS + assert "scope_code" not in profile + assert "organization" not in profile + assert "course" not in profile + assert "competency_taxonomy" not in profile + + +def test_every_profile_reports_the_scope_it_applies_to( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, + course_run: CourseRun, + organization: Organization, +) -> None: + """ + A caller tells the instance-wide default apart from a narrower profile by scope alone. + + Each row here holds a different scope_code, which is what shows the derivation reads the + scope columns rather than matching that string. + """ + taxonomy_scoped = make_profile(competency_taxonomy=competency_taxonomy) + course_scoped = make_profile(course=course_run) + organization_scoped = make_profile(organization=organization) + + response = staff_client.get(rule_profiles_url()) + + assert response.status_code == status.HTTP_200_OK + scope_types = {row["id"]: row["scope_type"] for row in response.data["results"]} + assert scope_types == { + default_rule_profile.id: "system_default", + taxonomy_scoped.id: "taxonomy", + course_scoped.id: "course", + organization_scoped.id: "organization", + } + + +def test_retired_profiles_are_left_out( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, +) -> None: + """A retired profile is not returned, while the instance-wide default still is.""" + archived = make_profile(competency_taxonomy=competency_taxonomy, archived=True) + + response = staff_client.get(rule_profiles_url()) + + returned_ids = [row["id"] for row in response.data["results"]] + assert archived.id not in returned_ids + assert returned_ids == [default_rule_profile.id] + + +def test_instance_with_no_rule_profiles_reports_an_empty_collection(staff_client: APIClient) -> None: + """ + An instance holding no profiles is an empty collection, not a missing resource. + + Migration 0005 seeds the system default into the test database, so the rows are cleared + explicitly here rather than assuming an empty table. + """ + CompetencyRuleProfile.objects.all().delete() + + response = staff_client.get(rule_profiles_url()) + + assert response.status_code == status.HTTP_200_OK + assert response.data["count"] == 0 + assert response.data["results"] == [] + + +def test_response_is_the_paginated_envelope( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """The collection arrives in an envelope, never as a bare array.""" + response = staff_client.get(rule_profiles_url()) + + assert isinstance(response.data, dict) + assert {"count", "next", "previous", "results"} <= set(response.data.keys()) + assert isinstance(response.data["results"], list) + assert [row["id"] for row in response.data["results"]] == [default_rule_profile.id] + + +def test_a_collection_larger_than_one_response_arrives_whole( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, + course_run: CourseRun, + organization: Organization, +) -> None: + """ + Following the link to the remainder yields every profile once, with none skipped. + + The page size divides the four profiles unevenly on purpose, so a partial last page is + exercised rather than a run of exactly full ones. + """ + expected_ids = [ + default_rule_profile.id, + make_profile(competency_taxonomy=competency_taxonomy).id, + make_profile(course=course_run).id, + make_profile(organization=organization).id, + ] + + collected_ids: list[int] = [] + pages = 0 + next_url = f"{rule_profiles_url()}?page_size=3" + while next_url: + response = staff_client.get(next_url) + assert response.status_code == status.HTTP_200_OK + assert response.data["count"] == len(expected_ids) + collected_ids.extend(row["id"] for row in response.data["results"]) + next_url = response.data["next"] + pages += 1 + + assert pages == 2, "page_size=3 should have split four profiles across two responses" + assert collected_ids == expected_ids + + +def test_profiles_arrive_in_the_same_order_every_time( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, + course_run: CourseRun, + organization: Organization, +) -> None: + """Two requests with nothing changing in between return the profiles in the same order.""" + make_profile(competency_taxonomy=competency_taxonomy) + make_profile(course=course_run) + make_profile(organization=organization) + + first = [row["id"] for row in staff_client.get(rule_profiles_url()).data["results"]] + second = [row["id"] for row in staff_client.get(rule_profiles_url()).data["results"]] + + assert first == second + assert first == sorted(first) + assert default_rule_profile.id in first + + +@pytest.mark.parametrize( + "user_fixture, expected_status", + [ + (None, status.HTTP_401_UNAUTHORIZED), + ("user", status.HTTP_200_OK), + ("staff_user", status.HTTP_200_OK), + ], +) +def test_any_signed_in_caller_may_read_the_collection( + request: pytest.FixtureRequest, + api_client: APIClient, + default_rule_profile: CompetencyRuleProfile, + user_fixture: str | None, + expected_status: int, +) -> None: + if user_fixture is not None: + api_client.force_authenticate(user=request.getfixturevalue(user_fixture)) + + response = api_client.get(rule_profiles_url()) + + assert response.status_code == expected_status + if expected_status == status.HTTP_200_OK: + assert [row["id"] for row in response.data["results"]] == [default_rule_profile.id] + else: + assert "results" not in response.data