Skip to content

feat: serve course group configurations from authoring v2 - #39308

Draft
taimoor-ahmed-1 wants to merge 13 commits into
openedx:masterfrom
edly-io:feat/standardize-group-configurations-api
Draft

taimoor-ahmed-1 wants to merge 13 commits into
openedx:masterfrom
edly-io:feat/standardize-group-configurations-api

Conversation

@taimoor-ahmed-1

Copy link
Copy Markdown
Contributor

Stacked on #39305 (custom pages and tabs, which adds the shared paginator DocumentedDefaultPagination), which is itself stacked on #39303 (advanced settings, which adds IsExistingCourse and the shared test helpers). Until both merge this branch also carries their commits; only the last 3 commits belong to this PR.

Merge after #39307 (course settings, PR F) before any client moves to this endpoint: two course-level flags that v1 returns here are served by #39307's settings/ resource, not by this one. See Merge order.

Summary

Standardizes the group configurations resource of the Course Management area (#39059, part of #38137). This is PR G of the eight resource-group PRs for the area; #39303 (advanced settings) is PR A, #39305 (custom pages and tabs) is PR C, and #39307 (course settings and rerun defaults) is PR F.

One conforming operation replaces the one v1 group configurations endpoint:

v1 (unchanged, now deprecated: true) v2 URL name (authoring_v2:)
GET /api/contentstore/v1/group_configurations/{course_id} GET /api/authoring/v2/courses/{course_key}/group_configurations/ course_group_configuration_list

It applies ADRs 0025–0038 and OEP-69.

One list of configurations. v1 returns one object: two lists, all_group_configurations and experiment_group_configurations, next to four facts about the course and the caller (can_manage, should_show_enrollment_track, should_show_experiment_groups, mfe_proctored_exam_settings_url). v2 returns one paginated list:

  • The items are v1's, in v1's order: the configurations of all_group_configurations (the enrollment track configuration when the course has two tracks or more, the content type gating configuration when Studio may set it, then content groups and the other configurations), followed by the experiment configurations when the course has content experiments turned on. They come back in the shared seven-key page envelope, 10 per page by default and at most 100. v1 was unbounded.
  • view=minimal (default) leaves usage out, on configurations and on groups alike; view=full adds it, identical to v1's. The minimal view saves payload size only: the shared Studio helper computes usage either way (Known gaps).
  • can_manage is on every item. It answers for the whole course: whether the caller may change the course's group configurations. It is asked once per request, with the same permission call v1 makes, and copied onto every item. It does not override read_only: a configuration the platform maintains, such as the enrollment track configuration, stays read only even when can_manage is true. The schema says so.
  • The unsaved content group configuration is kept. A course without content groups lists one, as v1 does, so that groups can be added to it; Studio's content-group create flow posts its id. Its id is drawn afresh on each request until a group is added, on v1 and v2 alike, and the schema says the id is unstable.

The course-level flags are not part of this resource. should_show_enrollment_track is answered by the list itself: the enrollment_track configuration is listed exactly when it was true. should_show_experiment_groups and mfe_proctored_exam_settings_url describe the course, not its group configurations; they are served by the course settings resource of #39307 (content_experiments_enabled and mfe_proctored_exam_settings_url on GET /api/authoring/v2/courses/{course_key}/settings/). The list cannot stand in for should_show_experiment_groups: a course with experiments turned on and none defined lists the same configurations as one with experiments off.

Why a new version and not a fix in place. Each of these changes the contract for v1's callers (the Group Configurations page in frontend-app-authoring calls it), so they ride v2 (ADR 0037):

  • the two lists become one paginated list (one object becomes the seven-key envelope);
  • usage is opt-in with view=full;
  • can_manage moves onto every item, and the three course-level flags leave the response;
  • errors use the standard envelope;
  • Bearer tokens are refused;
  • addresses gain a trailing slash and a checked course key.

Merge order and the course flags

  • feat: serve course rerun defaults and settings from authoring v2 #39307 adds GET /api/authoring/v2/courses/{course_key}/settings/, which carries content_experiments_enabled (v1's should_show_experiment_groups, the same are_content_experiments_enabled check) and mfe_proctored_exam_settings_url. Its field descriptions point at this resource.
  • This PR's code and schema do not name that address, because it is not mounted on this branch's base. The v1 deprecation marker names only group_configurations/ as the successor and says the two flags "are not part of that resource"; the v2 description says the same.
  • Nothing breaks if this PR merges first: v1 is unchanged. But frontend-app-authoring must not switch its Group Configurations page to this endpoint until feat: serve course rerun defaults and settings from authoring v2 #39307 is merged, or it loses the experiments flag and the proctored exam settings link.
  • Checked: settings/ is gated on the permission to view the course, this resource on the permission to view group configurations. A caller holding the second without the first could list configurations but not read content_experiments_enabled. In the pinned openedx-authz 1.34.0 policy, every role that may view group configurations (course auditor, editor, staff, admin) may also view the course, so no shipped role has that combination. A custom policy could still create one.

Changes

Commits:

Commit Endpoint(s) Legacy behaviour kept New behaviour
feat: add the group configurations view permission class — v1's @authz_permission_required decorator unchanged CanViewGroupConfigurations in views/permissions.py: the same user_has_course_permission(user, courses.view_group_configurations, course_key, READ) call v1's decorator makes
feat: serve course group configurations from authoring v2 the v2 operation above (new) v1 route, view and URL name unchanged View, service, serializers, route and tests. The service reuses the legacy helper contentstore.utils.get_group_configurations_context, which is not edited
docs: mark the v1 group configurations endpoint deprecated in favour of authoring v2 the v1 operation Route, view and response unchanged Docstrings name the successor and #39128. (deprecated: true in the schema comes from the exact SUPERSEDED_PATHS entry, with its test, added in the feat commit.)

ADRs applied

  • 0025 serializers — separate output serializers for the configuration, group, usage and validation (GroupConfiguration.v2, GroupConfigurationGroup.v2, GroupConfigurationUsage.v2, GroupConfigurationValidation.v2), a query serializer for view, help_text on every field, explicit .v2 component names; read-only, so no request body serializer
  • 0026 permissions — IsAuthenticated, IsExistingCourse, then CanViewGroupConfigurations (the check v1's decorator makes); the viewset refuses any action it was not written for; deny matrix with policy permissions off and on
  • 0027 schema / docs — @extend_schema with summary, description, parameters, a view=full example and every 4xx as ErrorResponse; tagged openedx-platform-sdk
  • 0028 viewsets — applied except router registration: one ViewSet over a service module, mounted with path(...) like every route in v2/authoring_urls.py, because a router's regex routes cannot use the shared path converters
  • 0029 errors — partial: the envelope on every response this view produces; 405 and 406 are typed internal, and a malformed course key or an address no route matches gets the CMS HTML 404 (library and routing gaps, pinned by tests); the published ErrorResponse component's text comes from edx-drf-extensions
  • 0030 GET idempotence — met for this code (no write, signal or event on GET or HEAD, either view), with one inherited write outside it: openedx-authz inserts its PolicyCacheControl row on the first permission check against a database that lacks it, as on v1
  • 0031 merged endpoints — n/a: nothing merged across endpoints; v1's one endpoint becomes one list, its two arrays concatenated in v1's order
  • 0032 pagination — the list returns the seven-key envelope (page, page_size 10 by default, at most 100) through the shared DocumentedDefaultPagination from feat: serve course custom pages and tabs from authoring v1 #39305; v1 was unbounded
  • 0033 filtering — n/a: no filter or sort parameter; the order is v1's, fixed and documented, and adding filters would be a new capability
  • 0034 authentication — (JwtAuthentication, SessionAuthenticationAllowInactiveUser) declared with its reason, as ADR 0034 requires for an endpoint that used SessionAuthenticationAllowInactiveUser; Bearer dropped
  • 0035 MFE config — n/a: per-course authoring data, not site or front-end configuration
  • 0036 nested JSON — usage (per configuration and per group) only with view=full; the default view=minimal leaves it out of the payload. The minimal view saves payload size only, because the shared Studio helper computes usage either way; a usage-free read is a future-version item
  • 0037 versioning — partial: new v2 modules, v1 frozen (docstring-only markers) and deprecated: true in the schema; the DEPR entry for [DEPR]: Contentstore and enrollment API versions superseded by FC-0118 #39128 is to be posted with this PR, and no removal release is named yet
  • 0038 URL structure — courses/{course_key}/group_configurations/ (a collection of configurations with ids, where a future {id}/ member would take Studio's writes); shared converter, trailing slash, version-free URL name; the v1 name kept
  • OEP-66 queryset scoping — n/a as a mechanism: the view owns no get_queryset(); configurations are user partitions stored with the course in the modulestore; the layering is shown in the matrix
  • OEP-69 conventions — the decisions above as recorded, with the 0028, 0029, 0030 and 0037 departures stated; no governance identifiers in code or schema text

Compliance matrix

Decision Status How it is met, or the concrete reason it is excluded Evidence (test / gate)
0025 met GroupConfigurationSerializer (GroupConfiguration.v2: id, name, scheme, description, parameters, groups, optional nullable usage, version, active, optional read_only, can_manage) gives v1's item plus can_manage. GroupConfigurationGroupSerializer (id, name, optional nullable usage, version), GroupConfigurationUsageSerializer (label, url, optional nullable validation, present only on experiment usage, as on v1), GroupConfigurationValidationSerializer (type, text). GroupConfigurationListQuerySerializer validates view. Explicit .v2 ref_names, because v1's GroupConfigurationGroup and GroupConfigurationUsage components already exist. can_manage's help text says it is course-wide and does not override read_only; id's says the unsaved configuration's id changes per request GroupConfigurationsSchemaTest (test_usage_validation_and_read_only_are_optional_and_usage_is_nullable, test_every_member_is_described); GroupConfigurationsListTest.test_the_body_matches_the_published_schema (both views); hygiene PASS
0026 met Every v1 check has a v2 home (Authorization below). Order 401, 404, 403. CanViewGroupConfigurations reads the parsed course_key and refuses a view without one. The viewset serves only list, OPTIONS and unrouted methods; any other action raises ImproperlyConfigured, answered as a 500 envelope instead of being served unchecked GroupConfigurationsAccessLegacyPermissionsTest, GroupConfigurationsAccessAuthzTest (each access case asserts v1's answer too); GroupConfigurationsPermissionWiringTest (test_the_listing_checks_the_course_then_the_permission_to_view, test_any_other_action_is_refused, test_a_routed_action_the_viewset_does_not_check_is_never_served, test_the_view_permission_asks_for_its_policy_and_studio_read_access, test_the_view_permission_refuses_a_view_without_a_course)
0027 met Operation ID v2_courses_group_configurations_list. course_key is a path parameter with the converter's pattern; view is a query enum (minimal, full; default minimal); the paginator adds page and page_size. Responses 200, 400, 401, 403, 404, every 4xx an ErrorResponse. The ContentGroups example (view=full) is published as a page that validates against the published envelope. Tagged openedx-platform-sdk; the deprecated v1 operation loses the tag through untag_deprecated_operations. The description says the course flags are not part of the resource and the placeholder's id changes GroupConfigurationsSchemaTest (test_the_listing_is_published_for_the_sdk, test_the_parameters, test_the_example_is_a_valid_page, test_the_description_leaves_out_the_course_flags_and_says_the_placeholder_id_changes, test_the_v1_operation_is_deprecated_and_left_out_of_the_sdk)
0028 applied except router registration CourseGroupConfigurationsViewSet (StandardizedErrorMixin first, then IterablePaginationMixin, viewsets.ViewSet) over v2/group_configurations_service.py. A plain ViewSet, because configurations are user partitions stored with the course in the modulestore, not rows this view owns. Deviation: mounted with path(), not a router GroupConfigurationsRoutingTest.test_the_address; GroupConfigurationsCost{LegacyPermissions,Authz}Test
0029 partial StandardizedErrorMixin first. 400 validation for an unknown view (errors.view); 401 authn; 403 authz; 404 not-found (unknown, version-only or library course key; a course the modulestore cannot load; a page out of range, zero or not a number). Gaps pinned by tests that flip when fixed: 405 and 406 are typed internal (edx-drf-extensions 10.9.0 does not catalogue them); a malformed or slash-form key gets the CMS HTML 404 at routing. The published ErrorResponse component's description is the library's own text, which names ADR 0029; changing it is a library change GroupConfigurationsListTest (test_an_unknown_view_is_refused, test_a_page_out_of_range_is_404); GroupConfigurationsResponseDifferencesTest.test_a_course_the_modulestore_returns_nothing_for; GroupConfigurationsRoutingTest (test_other_methods_are_not_allowed, test_unsupported_accept_is_refused_with_406, test_an_unparsable_or_slash_form_key_is_an_html_404)
0030 met for this code; one inherited write GET and HEAD, default, full and paged, on a course listing only the unsaved configuration, on one listing every kind and on one listing a team-set's configuration. With policy permissions off and on, no read writes to SQL or Mongo, stores through the modulestore, or sends a Django signal or Open edX event; the course's stored partitions are unchanged afterwards (the unsaved configuration is not stored). The exception is the first read on a database without openedx-authz's PolicyCacheControl row, which inserts it (one INSERT, its pre_save/post_save). That write belongs to the permission library, happens on v1 too, and is allow-listed by name GroupConfigurationsSideEffects{LegacyPermissions,Authz}Test (test_listings_have_no_side_effects over bare, rich, team_groups; test_the_v1_listing_makes_the_same_first_lookup_write); GroupConfigurationsListTest.test_a_course_without_content_groups_lists_an_unsaved_content_group_configuration
0031 excluded Nothing merged across endpoints. v1's two arrays become one list, in v1's order —
0032 met IterablePaginationMixin with the shared DocumentedDefaultPagination (#39305), which publishes all seven envelope keys. The service builds the full ordered list before paging, so pages are stable (apart from the unsaved configuration's id) GroupConfigurationsListTest (test_the_envelope_has_the_seven_keys, test_page_size_splits_the_pages_in_order, test_the_default_page_size_is_ten, test_a_page_out_of_range_is_404); GroupConfigurationsSchemaTest.test_the_listing_publishes_the_seven_envelope_keys
0033 excluded No filter or sort; v1's fixed, documented order. Unknown query parameters are ignored GroupConfigurationsListTest (test_the_configurations_are_listed_in_studio_order, test_other_query_parameters_are_ignored)
0034 met with declared deviation (JwtAuthentication, SessionAuthenticationAllowInactiveUser), commented at the declaration: an inactive account's session stays authenticated, so the course permission decides; staff status admits it, and its roles count only on courses where policy-based permissions are on. Bearer is dropped. Same as #39303 and #39305 GroupConfigurationsAuthenticationTest (test_a_jwt_caller_may_list, test_a_session_caller_may_list, test_an_inactive_session_is_authenticated, test_a_bearer_token_is_refused); GroupConfigurationsAccessAuthzTest.test_an_inactive_session_with_a_role_is_accepted
0035 excluded Not configuration; per-course authoring data —
0036 met for payload size usage on configurations and groups is the heavy part (a scan of the course's content per configuration). view=minimal, the default, leaves it out of the response; view=full includes it, identical to v1's; both variants are published (usage optional and nullable). The minimal view does not save the work: the frozen helper get_group_configurations_context computes usage for every configuration on either view, and the view drops it at serialization. A usage-free read needs a new helper, a future-version item GroupConfigurationsListTest (test_the_default_view_leaves_out_usage, test_the_minimal_view_is_the_default, test_the_full_view_reports_where_each_configuration_and_group_is_used); GroupConfigurationsResponseDifferencesTest.test_usage_is_returned_only_with_view_full; GroupConfigurationsSchemaTest.test_the_parameters
0037 partial New modules in v2/. v1 frozen apart from docstring-only markers: the syntax tree with docstrings stripped equals the base. deprecated: true through one exact SUPERSEDED_PATHS entry; no runtime header (frozen route). DEPR: an entry for #39128, to be posted with this PR; no removal release is named versions gate; CmsMarkSupersededPathsTest.test_the_group_configurations_operation_is_marked_and_its_successor_is_not; GroupConfigurationsSchemaTest (test_the_v1_operation_is_deprecated_and_left_out_of_the_sdk, test_only_superseded_or_migrated_addresses_are_deprecated)
0038 met group_configurations under the course; shared course_key converter; trailing slash; version-free URL name GroupConfigurationsRoutingTest (test_the_address, test_the_v1_address_is_unchanged, test_the_url_names_are_unique_in_the_namespace, test_the_address_without_its_trailing_slash_redirects_to_it, test_v1_still_takes_a_slash_separated_key_that_v2_does_not); the urls gate itself was skipped
OEP-66 mechanism excluded, layering met (1) endpoint access: the permission classes; (2) record visibility: the configurations Studio's helper displays, as on v1 (enrollment tracks with two tracks or more; content type gating only when Studio may set it; a team-set only with content groups for teams on and a team in it; experiments only when turned on); (3) user selection: view, page and page_size, none of which can widen what is listed access tests above; GroupConfigurationsListTest (test_the_enrollment_track_configuration_is_listed_with_two_tracks_or_more, test_content_type_gating_is_listed_only_when_studio_may_set_it, test_a_team_set_is_listed_only_with_content_groups_for_teams_on_and_a_team_in_it, test_experiment_configurations_are_listed_only_when_experiments_are_on)
OEP-69 met The decisions above, with the departures stated in their rows. No ADR/OEP/PR identifiers or narration in code or schema text hygiene gate PASS (0 FAIL, 0 WARN)

Backward compatibility

  • v1 is unchanged. v1/views/group_configurations.py gains docstring text only; its syntax tree with docstrings stripped equals the base. Its route, view and URL name are untouched, and v1's own tests are not edited.

  • Parity. v2 is compared with v1 through the Django test client, with policy permissions off (course staff) and on (course auditor), on six courses that between them cover every way the listing changes (only the unsaved configuration; experiments on with none defined; experiment configurations with experiments off; one enrollment track; two tracks, Studio-settable gating, content groups and experiments; a team-set with content groups for teams on):

    • with view=full, the items are v1's two arrays in order, identical apart from can_manage;
    • with the default view, they are the same without usage;
    • every item's can_manage is v1's can_manage;
    • 401, 403 and 404 differ only in the error body.

    The only declared body differences, verbatim from the test module:

    INTENTIONAL_DIFFERENCES = [
        ("type", "errors use the standardized envelope"),
        ("title", "errors use the standardized envelope"),
        ("status", "errors use the standardized envelope"),
        ("detail", "errors use the standardized envelope"),
        ("developer_message", "the v1 error body carries developer_message; the envelope does not"),
        ("error_code", "the v1 refusal and not-found bodies carry error_code; the envelope does not"),
    ]

    The comparison fails both on an undeclared difference and on a declared difference that never occurs (_ParityMixin.test_errors_differ_only_by_the_envelope_and_every_declared_difference_occurs); test_the_comparison_sees_a_usage_that_turns_null_or_is_dropped checks that the diff notices a usage that changes.

  • Status and request differences from v1. Each is listed in the test module and observed against v1's own answer; test_every_listed_difference_has_its_test fails on a row without a test:

    RESPONSE_DIFFERENCES = [
        ("any read", "an object holding all_group_configurations and experiment_group_configurations",
         "a page of configurations, the experiment configurations last", "test_the_two_lists_become_one_page"),
        ("any read", "can_manage once, beside the lists", "can_manage on every configuration",
         "test_can_manage_is_on_every_configuration"),
        ("read without view", "usage on every configuration and group", "no usage; view=full includes it",
         "test_usage_is_returned_only_with_view_full"),
        ("any read", "should_show_enrollment_track",
         "not returned; the enrollment_track configuration is listed exactly when it was true",
         "test_should_show_enrollment_track"),
        ("any read", "should_show_experiment_groups", "not returned: it describes the course",
         "test_should_show_experiment_groups"),
        ("any read", "mfe_proctored_exam_settings_url", "not returned: it describes the course",
         "test_mfe_proctored_exam_settings_url"),
        ("read with view=default", "200, the parameter ignored", "400", "test_view_default"),
        ("read with a page past the last, zero or not a number", "200, the parameter ignored", "404",
         "test_an_invalid_page"),
        ("any read for a course the modulestore returns nothing for", "500", "404",
         "test_a_course_the_modulestore_returns_nothing_for"),
        ("OPTIONS by a signed-in caller without access, or for an unknown course", "200", "403 or 404",
         "test_options"),
        ("any read with a Bearer token", "200", "401", "test_a_bearer_token"),
        ("read for a slash-separated course key", "404 JSON", "HTML 404 at routing", "test_a_slash_separated_course_key"),
    ]

    As a table, with the request-shape change that carries no status difference (tests in GroupConfigurationsResponseDifferencesTest unless named):

    Request v1 v2 Test
    list GET v1/group_configurations/{course_id}, one object: two arrays and four flags GET v2/courses/{course_key}/group_configurations/, 7-key envelope, 10 per page by default; experiment configurations last test_the_two_lists_become_one_page
    can_manage once, beside the arrays on every item, same value test_can_manage_is_on_every_configuration
    usage always, on configurations and groups only with view=full test_usage_is_returned_only_with_view_full
    should_show_enrollment_track returned not returned; the enrollment_track item is listed exactly when it was true test_should_show_enrollment_track
    should_show_experiment_groups returned not returned (content_experiments_enabled on feat: serve course rerun defaults and settings from authoring v2 #39307's settings/) test_should_show_experiment_groups
    mfe_proctored_exam_settings_url returned not returned (on feat: serve course rerun defaults and settings from authoring v2 #39307's settings/) test_mfe_proctored_exam_settings_url
    ?view=default (any value but minimal/full) 200, ignored 400 validation test_view_default
    page past the last, 0 or not a number 200, ignored 404 test_an_invalid_page
    course the modulestore cannot load 500 404 test_a_course_the_modulestore_returns_nothing_for
    OPTIONS by an outsider, or for an unknown course 200 403 / 404 test_options
    Bearer token 200 401 test_a_bearer_token
    slash-form course key 404 JSON (course_does_not_exist) 404 HTML (no route) test_a_slash_separated_course_key
    address without the trailing slash served 301 to the slashed address GroupConfigurationsRoutingTest.test_the_address_without_its_trailing_slash_redirects_to_it
  • Schema. One new operation, and deprecated: true on the v1 operation. docs/cms-openapi.yaml is not edited; the generate_openapi_schemas workflow regenerates it.

Known gaps

  • Query counts are relative to v1, not literal. No platform environment was available to measure a baseline. Instead, in the same test, with policy permissions off and on, a listing on either view makes no more SQL queries and no more Mongo reads than v1's endpoint for the same course, and no Mongo writes (on the bare, rich and team_groups courses). From one content group configuration to eight, the SQL count stays the same on both views, and at each size neither view makes more Mongo reads than v1 (GroupConfigurationsCost{LegacyPermissions,Authz}Test).
  • Inherited per-partition usage scans. Studio's helper reads usage once per configuration, each time with a query over the course's content (GroupConfiguration.update_partition_usage_info → get_partitions_usage_info). So Mongo reads grow with the number of configurations, on v1 and v2 alike, on either view. v2 does not add to them; the growth test pins SQL only and holds Mongo to v1's count.
  • The minimal view saves payload, not work. get_group_configurations_context (in contentstore/utils.py, shared and protected here) always computes usage; v2 drops it at serialization. The area plan said the default would skip the scans; that is corrected here. A usage-free helper is a future-version item.
  • Team-set configurations: one half of Studio's check never decides. Studio lists a team-set's configuration when it has at least one team and content groups for teams is on for the course (contentstore/utils.py). With the flag off, the teams partition generator produces no team-set configuration at all (and such a configuration has no groups), so the flag half of that condition is never the one that excludes it; the team count alone decides. v2 inherits the condition unchanged; the tests cover flag on with teams, flag off with teams, and flag on with no team (test_a_team_set_is_listed_only_with_content_groups_for_teams_on_and_a_team_in_it).

In-place edits

File Category Evidence
cms/djangoapps/contentstore/rest_api/v1/views/group_configurations.py docstring-only deprecation marker (class and get) syntax tree with docstrings stripped equals the base; versions gate ALLOWED
cms/djangoapps/contentstore/rest_api/v2/authoring_urls.py strictly opt-in addition: one import, one route no line removed; GroupConfigurationsRoutingTest
cms/djangoapps/contentstore/views/permissions.py (shared, unversioned) one class added (CanViewGroupConfigurations); the openedx_authz.constants.permissions import gains COURSES_VIEW_GROUP_CONFIGURATIONS and is reflowed onto several lines; existing classes unchanged GroupConfigurationsPermissionWiringTest and the access tests
cms/lib/spectacular.py, cms/lib/tests/test_spectacular.py one exact SUPERSEDED_PATHS entry; the exact-set test updated and a test for the group configurations path added CmsMarkSupersededPathsTest

New files inside the pre-existing v2/: group_configurations_service.py, serializers/group_configurations.py, views/group_configurations.py, views/tests/test_group_configurations.py. The production modules import nothing from rest_api/v0/ or rest_api/v1/.

Test-only imports from v1. The test module imports these shared helpers from rest_api/v1/views/tests/helpers.py (added by #39303): FIRST_LOOKUP_SIGNALS, FIRST_LOOKUP_WRITES, UNKNOWN_COURSE_KEY, VERSION_ONLY_KEY, api_client, assert_matches_published_schema, assert_refused, changed_paths, measure, published_schema, record_effects, session_client, undeclared_paths. They are the area's shared test helpers (clients, the side-effect recorder, the parity diff, the cost measure, the schema check); importing them keeps one copy rather than a second one in v2/. ENVELOPE_KEYS comes from rest_api/pagination.py (#39305).

Error-format decision

Versioned. v1 keeps its developer_message/error_code bodies unchanged. v2 uses the envelope. The Group Configurations page in frontend-app-authoring calls the v1 endpoint (src/group-configurations/data/api.ts); the list's pagination, the opt-in usage and the move of the course flags already force a new version, and the error format rides with it.

Deprecations

The v1 operation is superseded by the v2 operation in the Summary. It is deprecated: true in the CMS schema, and the docstrings name the successor. The view is otherwise unchanged: no runtime header. Tracked on the FC-0118 umbrella DEPR ticket, #39128; this resource's entry is posted there with the PR. Window per #39128: removed one named release after the release this ships in.

Migration notes:

  • Configurations: call GET /api/authoring/v2/courses/{course_key}/group_configurations/. It is paginated: read results, and follow next or pass page/page_size (at most 100) to get every configuration. The items are v1's all_group_configurations followed by v1's experiment_group_configurations, in the same order and shape.
  • Usage: pass view=full to get usage on configurations and groups, as v1 returns it.
  • can_manage: read it from any item; it is the same on all of them and does not override an item's read_only.
  • should_show_enrollment_track: true exactly when an item has scheme enrollment_track.
  • should_show_experiment_groups and mfe_proctored_exam_settings_url: read content_experiments_enabled and mfe_proctored_exam_settings_url from GET /api/authoring/v2/courses/{course_key}/settings/ (feat: serve course rerun defaults and settings from authoring v2 #39307). Do not migrate before feat: serve course rerun defaults and settings from authoring v2 #39307 is merged.
  • The unsaved content group configuration is still listed when the course has none, with an id that changes per request until a group is added.
  • Errors use the standard envelope. view accepts only minimal and full; an out-of-range or non-numeric page is a 404.
  • Bearer tokens are refused; use JWT or a session.
  • Every address ends with a slash. Slash-separated (deprecated) course keys are not accepted.

Authorization

Legacy check Where it lives in v2 Deny-path tests
@view_auth_classes(is_authenticated=True) IsAuthenticated test_anonymous_caller_is_refused_with_401 (both permission modes); test_anonymous_caller_of_an_unknown_course_is_refused_with_401
@verify_course_exists() IsExistingCourse (also 404 for version-only and library keys) unknown course, version-only key and library key → 404, before any role check (test_unknown_course_is_404_before_permission_is_checked, test_a_version_only_course_key_is_404, test_a_library_is_not_a_course)
@authz_permission_required(courses.view_group_configurations, READ) CanViewGroupConfigurations (new), same user_has_course_permission call with policy permissions off: no role, limited staff, staff or instructor of another course, a superuser without staff status; with them on: limited staff, data researcher, beta tester, an admin role on another course, Studio's own staff role without a policy role
can_manage = user_has_course_permission(courses.manage_group_configurations, WRITE), non-gating the same call in the service, once per request, copied onto every item test_whether_the_caller_may_manage_is_asked_once_per_request; test_a_role_that_may_only_view_lists_without_can_manage; test_roles_that_may_manage_list_with_can_manage

Order: 401, then 404 for a missing course, then 403. Allowed, with policy permissions off: course staff and instructor, org staff and org instructor, global staff (all with can_manage true). With them on: the course auditor role (can_manage false); the course editor, course staff and course admin roles (can_manage true); global staff and superusers. Each case asserts v1's status and can_manage too.

Superusers. Saving a superuser sets is_staff: the pre_save receiver grant_staff_access_to_superusers (common/djangoapps/student/signals/receivers.py) makes every saved superuser global staff, so such a superuser lists and manages like global staff. A superuser whose staff status was cleared without saving through the model is refused with policy permissions off (Studio read access comes from staff status or a course or org role) and admitted, with can_manage true, with them on (the permission policy admits superusers). v1 decides the same in both cases (test_a_superuser_saved_through_the_model_is_global_staff_and_may_list_and_manage, test_a_superuser_without_staff_status_is_refused, test_staff_and_superuser_may_list_and_manage_without_a_role, test_a_superuser_without_staff_status_may_list_and_manage_without_a_role).

Corrections to the area plan, found in review and checked against the code. The tests follow the platform's behaviour, which v2 keeps:

  • Inactive accounts. With policy permissions off, inactive global staff are admitted (200) and inactive course staff are refused (403): a course role does not count while the account is inactive. With them on, an inactive holder of a policy role is admitted (200), because the policy does not look at whether the account is activated. v1 answers the same. The plan said an inactive session gets 200 (GroupConfigurationsAuthenticationTest.test_an_inactive_session_is_authenticated, GroupConfigurationsAccessAuthzTest.test_an_inactive_session_with_a_role_is_accepted).
  • The default view skips the usage payload, not the usage scans (Known gaps). The plan said the default would omit the per-partition scans.
  • Superuser admission comes from the pre_save receiver, not from the policy engine, contrary to what feat: serve course advanced settings from authoring v1 #39303 and feat: serve course custom pages and tabs from authoring v1 #39305 say.

Gate report

Run on the host against 8721326a74 (the head of #39305, which this branch stacks on). No platform environment was available, so four gates were skipped. The test module has not been run locally, and CI is its first run.

Gate Result
versions PASS: 2 ALLOWED (v1/views/group_configurations.py docstring marker, v2/authoring_urls.py), 4 ADDED in the existing v2 directories, nothing protected touched (23 shared legacy modules protected, including contentstore/utils.py, whose get_group_configurations_context v2 reuses)
hygiene PASS (0 FAIL, 0 WARN) over 9 files
ruff clean; no line over 120 characters
pylint (4.1.3, repo pylintrc) only C1803 (== [] comparisons), the same message #39303 carries with its CI pylint passing
supplementary, not a gate serializer-level parity mutation check, 6/6 mutants caught; static check of the differences-table guard
schema SKIPPED (no platform environment)
urls SKIPPED (no platform environment)
old-tests SKIPPED (no platform environment)
tests SKIPPED (no platform environment)

Follow-ups

Taimoor Ahmed added 13 commits October 25, 2026 15:09
The v0 advanced settings view checks course existence with the
@verify_course_exists decorator and access with an inline
check_course_advanced_settings_access call in each handler. The
conforming views need these as permission classes.

IsExistingCourse answers 404 for a course key that names no course run:
an unknown course, a key that names only a version, or a library key.
CanViewAdvancedSettings and CanManageAdvancedSettings make the same
check_course_advanced_settings_access(read|write) call the v0 view
makes, so the policy-permission and legacy Studio-access branches are
unchanged. Existing classes in the module are untouched.
GET/PATCH /api/contentstore/v0/advanced_settings/{course_id} answers
errors with developer_message bodies and some failures with a 500,
accepts Bearer tokens, returns every settings field by default, and
reports a refused value with the field parser's own text, which can be
Python exception text.

GET/PATCH /api/authoring/v1/courses/{course_key}/advanced_settings/ is
its successor. Errors use the standard envelope; a refused value is
reported under its setting name with one fixed message and the original
text is logged. The default read is the set of settings course authors
edit (v0 with fetch_all=0); view=full returns every field and fields
narrows either view. Authentication keeps the session class that admits
inactive users, as the legacy endpoint did; Bearer is dropped. Access is
IsAuthenticated, IsExistingCourse and a per-action class making v0's
read or write check, and an action without a declared permission is
never served. Parity tests compare v1 with v0 through the test client;
the declared differences are the error envelope and the fixed message.
GET/PATCH /api/contentstore/v0/advanced_settings/{course_id} now has a
conforming successor at /api/authoring/v1/courses/{course_key}/advanced_settings/.
Both v0 operations are marked deprecated in the CMS schema through an
exact SUPERSEDED_PATHS entry, and the view's docstrings name the
successor, how each v0 read maps to a v1 read, and the deprecation
ticket. The view itself is unchanged. The schema hook test that used
this path as its example of an unrelated contentstore path now uses the
heartbeat, which is not deprecated.
Importing cms.djangoapps.contentstore.rest_api.v1.authoring_urls at
module level builds its routes before cms/urls.py registers the
course_key path converter, so collecting the module failed with an
ImproperlyConfigured error. The URL-name test now imports it when it
runs, as the youtube transcript tests do. Also wraps a line pylint
reported as too long.
The first CI run of the module showed three expectations that did not
match the platform:

- The legacy role checks consult the policy engine, which admits
  superusers, so a superuser who is not global staff is served by v0 and
  v1 alike. The test now expects that.
- The proctoring provider field keeps its default unless proctored exams
  are enabled, so the proctoring tests enable them.
- The JWT middleware appends NotJwtRestrictedApplication to the
  permissions of views that accept JWTs, so the wiring test compares only
  the classes the view declares.
The side-effect recorder, the body comparison helpers, the schema
helpers and the client helpers in the advanced settings tests are not
specific to advanced settings. They move unchanged, under public names,
into v1/views/tests/helpers.py so the other authoring v1 endpoint tests
reuse one copy instead of each carrying its own.
edx-drf-extensions' DefaultPagination returns seven envelope keys but
publishes a schema with four, so generated clients drop num_pages,
current_page and start. DocumentedDefaultPagination keeps the library's
pagination and publishes all seven keys, each with an example. It lives
in an unversioned module for every authoring list to reuse, and is to
be deleted once the library documents the envelope itself.
The v0 tab views check courses.manage_pages_and_resources inline with a
legacy write-access fallback before reordering custom pages or changing
a tab. CanManagePagesAndResources makes the same call, beside the
existing CanViewPagesAndResources. Existing classes are unchanged.
The v0 tabs endpoints return the custom pages as a bare array, reorder
them through a POST verb that answers a missing page with 500 and stores
a repeated page twice, change a tab's visibility through a POST verb that
echoes its input in errors, and accept Bearer tokens.

GET /api/authoring/v1/custom_pages/?course_key=... lists a course's
custom pages a page at a time in course order; a custom page is
addressed by its own usage key, so the list is top level with the course
in the query. PUT /api/authoring/v1/courses/{course_key}/custom_page_order/
replaces their order, with pages left out kept after the listed ones.
PATCH /api/authoring/v1/courses/{course_key}/tabs/{tab_id}/ shows or
hides a built-in tab and returns it. Errors use the standard envelope
with fixed messages. Access makes v0's view and manage checks. Parity
tests compare the stored tabs and each refusal with v0.
The three v0 tab operations now have conforming successors under
/api/authoring/v1/. They are marked deprecated in the CMS schema through
exact SUPERSEDED_PATHS entries, and the views' docstrings name the
successors and the deprecation ticket. The views themselves are
unchanged.
CanViewGroupConfigurations makes the same check the v1 group
configurations endpoint applies through its decorator (view group
configurations, falling back to Studio read access), so the authoring v2
endpoint can declare it as a permission class.
Adds GET /api/authoring/v2/courses/{course_key}/group_configurations/,
replacing GET /api/contentstore/v1/group_configurations/{course_id}.

The two legacy lists become one paginated list in the legacy order. Whether
the caller may manage configurations moves onto each item; where each
configuration and group is used is returned only with view=full. The
course-level flags of the legacy response are not part of this resource.
The endpoint uses the standard error envelope, drops Bearer
authentication, answers 404 for an unknown course before checking
permissions, and is tagged for the SDK. The legacy path is marked
deprecated in the published schema.
…of authoring v2

Docstring-only: names the successor address and the deprecation ticket.
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 25, 2026
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @taimoor-ahmed-1!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform-oncall.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Update the status of your PR

Your PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate.


Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Needs Triage

Development

Successfully merging this pull request may close these issues.

2 participants