[ufedaseyeuconsultant]

Description

Adds course-run scoping to the criteria-groups read endpoint from #681:

GET /api/cbe/v1/competencies/<int:tag_id>/criteria-groups/?course_keys=<comma-separated keys>

A competency used in many course runs makes the unscoped #681 response unbounded: an author editing one course run's criteria has to load every other run the competency has ever been used in. course_keys lets the caller name the course runs it wants. Deciding which course runs are in view (title search, date filtering, paging) is the job of #669 and #670; this endpoint only accepts the keys it is given.

A course run's groups are its course-level group (the group whose course is that run) and every group nested below it, at any depth. The groups above any course are the ones not nested in any course-level group, such as the root. The parameter has three states, depending on whether it is present at all:

• Omitted: every reachable group, the same as [BE] GET Competency Criteria & Groups #681.
• Present and empty: only the groups above any course.
• Present with keys: the groups above any course, then the groups of each named course run in the order requested. A malformed key returns 400. A key that matches no course run is dropped, and a repeated key counts once, at its first position. More than 100 keys returns 400 with a message that names the limit. Supplying the parameter more than once returns 400.

Each named course run's groups come back as a complete subtree, never truncated, which is the retrieval scope ADR 0002 Decision 2 requires.

How scoping works. A leaf group does not carry its course: per ADR 0002 Decision 2, only the course-level group has course_id, and the groups below it belong to the course through that ancestor. So get_competency_criteria_tree() loads the competency's non-archived groups and walks the tree from the root, assigning each reachable group the course of its course-level ancestor. It then keeps the groups above any course and the groups of the requested runs. Whether to record the course on every group so the groups query can filter by course directly is tracked in #.

Criteria and counts. The criteria query is limited to the ids of the returned groups. The response adds two counts. criteria_count is the number of criteria returned. total_criteria_count is the number of non-archived criteria under every reachable group before scoping, so a caller scoped down to zero criteria can still tell that the competency is not empty. A scoped read gets the total from a separate COUNT and takes four queries in all. An unscoped read takes two queries and uses the length of the criteria list as the total. A test pins both query counts.

Permission and existence checks run before course_keys is parsed, so a malformed query string does not reveal anything ahead of a 403 or 404.

Which roles this affects: Course Author and Platform Administrator, once the authoring frontend uses this parameter (#683). Developer, as an additive change to the #681 response.

Supporting information

• Implements [BE] Add filtering to GET Competency Criteria & Groups #682.
• Builds on the [BE] GET Competency Criteria & Groups #681 read endpoint, which is on main.
• Adds get_course_run_ids() to openedx_catalog.api, which resolves course keys to CourseRun ids in one query, following [BE] Create CompetencyCriteria #665's get_course_run.
• ADR: docs/openedx_learning/decisions/0002-competency-criteria-model.rst, Decision 2 for course_id and the retrieval scope. This PR does not change the ADR.

Testing instructions

1. Set up a competency with criteria under at least two course runs (see [BE] GET Competency Criteria & Groups #681's testing instructions for how to attach a criterion).
2. GET .../criteria-groups/ with no course_keys: the same groups and criteria as [BE] GET Competency Criteria & Groups #681, and criteria_count equals total_criteria_count.
3. GET .../criteria-groups/?course_keys= (present, empty): only the groups above any course come back, criteria_count is 0, and total_criteria_count is the competency's full count.
4. GET .../criteria-groups/?course_keys=<course A key>: only the groups above any course and course A's subtree come back, with course A's criteria. Nothing from course B appears in either array.
5. Request <course B key>,<course A key> and then <course A key>,<course B key>: the groups and criteria arrays reorder to match each request.
6. Request the same course key twice: its subtree appears once.
7. Request a well-formed key for a course run that does not exist: no error, and that key is ignored.
8. Request a malformed key: 400.
9. Request 101 course keys: 400, and the error names the limit of 100.
10. Supply course_keys twice in the query string: 400.
11. Request any course_keys value as a user without view access: 403, and the response body contains neither array nor either count.
12. Automated coverage: pytest tests/openedx_learning/applets/cbe/ tests/openedx_catalog/.

Other information

• Backward compatible: course_keys is optional, and omitting it returns the [BE] GET Competency Criteria & Groups #681 response plus the two new count fields.
• No migration. The PR changes query logic and adds one catalog API function.
• Out of scope, tracked separately: deciding which course runs are in view for a caller ([BE] Add start-date range filter and outline delivery for accessible courses (Competency Criteria Associations) #669, [FE] Competency Management Course Search/View #670), using this parameter in the authoring frontend ([FE] Implement the plan to handle large quantities of competency criteria associations (pagination, filtering, cues to user, other?) on the frontend #683), and whether every group should record its course ([Follow-up BE] Scope the criteria-groups query by course instead of loading every group #919).
• Earlier review by @tbain: feat: add course-run scoping to the criteria-groups endpoint ufedaseyeuconsultant/openedx-core#1

🤖 Generated with Claude Code

[openedx-webhooks]

Thanks for the pull request, @ufedaseyeuconsultant!

This repository is currently maintained by @axim-engineering.

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.

Details

---

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:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

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.

[tbain]

This was previously approved at the Unicon level under an older version of this PR:
ufedaseyeuconsultant#1 (review)

[ormsbee]

Two requests before merging, plus a higher-level question about the groups query that I'd like answered in the ADR before this goes in, even if the code for it lands separately.

1. Please filter the criteria query to the groups being returned and get total_criteria_count from a COUNT, rather than loading every criterion for the competency and trimming in Python (inline on api.py).
2. Please make ADR 0002 say one thing about which groups carry course_id. Decision 2 still says every group in a course-scoped subtree does; the sentence this PR adds to the retrieval-scope bullet says only the first group on the path does (inline on the ADR).

At a higher level, the groups query has the same shape of problem as (1): _create_leaf_group() makes a new leaf per criterion, so a competency's group rows grow with its criteria, and a request scoped to one course run still loads all of them. That's partly on me, since I asked for the walk from the roots in #878, and the walk needs every group in memory as long as a leaf doesn't record which course it belongs to. If every group in a course subtree carried course_id, as Decision 2 already says, the groups query could be a predicate and the walk would still work on just those rows. The details and what else it would touch are inline on the groups query. I lean that way since none of this has shipped yet, but if there's a reason the leaf was left without a course that I'm not seeing, please let me know.

There are also a couple of smaller, non-blocking things in the inline comments.

Thank you.

---

Claude drafted the first pass of this review and ran the CBE and catalog suites (592 tests), makemigrations --check, the quality checks (pylint, mypy, pycodestyle, isort, import-linter), and the docs build for me, all clean, plus the row-count probe described in the api.py comment. The checks were re-run on 06fa06c after the rebase onto #760; the PR's own diff didn't change.

[ormsbee]

This loads every non-archived criterion under the competency, with its object_tag, and only then drops the ones outside the scope, so a scoped request does the same database work as an unscoped one and only the JSON gets smaller. Claude ran a probe on this branch with 20 course runs and 10 criteria each: a request scoped to one run returned 10 criteria, but this query listed all 221 group ids in its IN clause and fetched all 200 criteria rows (which is what lets total_criteria_count be len(all_criteria)), and course_keys=[] fetched all 200 rows to return none of them. That's the case #682 exists to avoid.

Please filter this on the scoped group ids and get the total from a COUNT, which is what the ticket's technical details sketched. Something like:

    position_by_group_id = {group.id: position for position, group in enumerate(scoped_groups)}
    criteria = list(
        CompetencyCriterion.objects.filter(group_id__in=position_by_group_id.keys(), archived=False)
        .select_related("object_tag")
        .order_by("group_id", "id")
    )
    criteria.sort(key=lambda criterion: position_by_group_id[criterion.group_id])
    if course_keys is None:
        total_criteria_count = len(criteria)
    else:
        total_criteria_count = CompetencyCriterion.objects.filter(
            group_id__in=branch_course_ids.keys(), archived=False,
        ).count()

A scoped read becomes four queries instead of three, so test_get_competency_criteria_tree_costs_a_bounded_number_of_queries changes, which I think is the right trade. I only sketched this to show the shape and haven't run it.

[ormsbee]

_create_leaf_group() creates a new leaf for every criterion, so this query returns roughly criteria + course runs + 1 rows for the competency, and a request scoped to one run loads all of them, walks them, and keeps the handful it asked for. The walk is there because I asked for it in #878, and it needs every group in memory because a leaf has no course of its own: the only way to learn which course a leaf belongs to is to find its parent.

If every group in a course-scoped subtree carried course_id, which is what ADR 0002 Decision 2 says today ("Within a course-scoped subtree, every group carries the same course_id, not only its topmost group"), this could be:

    groups_query = CompetencyCriteriaGroup.objects.filter(tag=tag, archived=False)
    if course_keys is not None:
        run_ids_by_key = get_course_run_ids(course_keys)
        groups_query = groups_query.filter(Q(course__isnull=True) | Q(course_id__in=run_ids_by_key.values()))

and the walk from the roots still works on just those rows, since every ancestor of a loaded group is either instance-wide or in the same course, so it's either loaded too or archived, in which case the child is unreachable, which is what we want. It's also what the (tag, course) index from Decision 5 was added for. The total count would then be a plain COUNT on (group__tag, group__archived, archived) if we're willing to lean on #675 archiving subtrees whole, or a cheap values_list("id", "parent_id") pass over the non-archived groups if we aren't.

The cost is that resolve_supplied_leaf_group() uses course_id is None as its leaf test, get_courses_with_relative_criteria() joins through parent__course, and #760's update_competency_criteria_group() walks up to the nearest ancestor with a course for its permission check, so this isn't confined to one function (though that last one gets simpler, since the course would just be group.course). Given that, I'm okay with the code part being a follow-up PR if you'd rather land the criteria fix first, as long as the ADR says which way we're going and this docstring stops presenting the "first course on the path" rule as the model.

[ormsbee]

This sentence and Decision 2's course_id bullet (line 86) now contradict each other: that one says "Within a course-scoped subtree, every group carries the same course_id, not only its topmost group", and this one says a group's course is "the first course on its path from the root, at any depth". I understand why this was written to match the code, since _create_leaf_group() has left course null on the leaf since #847 and #760's update_competency_criteria_group() now uses the same nearest-ancestor rule for its permission check, but the ADR has to pick one. See my comment on the groups query in api.py for which way I lean. Whichever we pick, please update both places, and if it's this one, the changelog entry should say it supersedes the 2026-09-04 entry rather than "documented".

[ormsbee]

Nit: Is this a state we want to give meaning to? Nothing in the applet creates a group under course X's subtree that carries course Y, and Decision 2 says every group in a subtree carries the same course, so this test pins the read path's answer for a row that should never exist. I'd rather drop the test (and the "first course on the path" wording that goes with it) than document what the reader does with invalid data. If we care about that row at all, the place to care is at write time, by rejecting it.

[ufedaseyeuconsultant]

@ormsbee

1. Criteria query. Done as you sketched. The criteria query is filtered to the group ids being returned, and on a scoped read total_criteria_count comes from a COUNT over every reachable group. A scoped read is now four queries, and test_get_competency_criteria_tree_costs_a_bounded_number_of_queries pins that number. A new test, test_get_competency_criteria_tree_scoped_fetches_only_the_scoped_criteria_and_counts_the_rest, checks two things. First, the criteria SELECT names none of the other course's groups. Second, the total comes from a separate COUNT. An unscoped read is still two queries, and its total is len(criteria).
2. ADR 0002 and course_id. We're going with Decision 2 as it stands: every group in a course-scoped subtree carries that subtree's course_id. This PR no longer touches the ADR. I removed the sentence I had added to the retrieval-scope bullet and the 2026-09-30 changelog entry, so the ADR says one thing again. The get_competency_criteria_tree docstring and the view docstring now describe scoping by course-level group: "a course run's groups are its course-level group and every group nested below it". They no longer present a "first course on the path" rule.

On the groups query: I agree it should become a predicate, and I'd rather land that as a follow-up PR. There is one concrete reason the leaf was left without a course. The oel_cbe_criteria_group_one_course_group_per_tag_course unique constraint is on (tag, course, parent), with no condition, because MySQL has no partial indexes. Every leaf for a course hangs off that course's one course-level group, so a second leaf with course=X would have the same (tag, X, parent) as the first and violate it. To give leaves a course, the follow-up has to replace that backstop first, for example with an application-computed column that is set only on course-level groups and is unique, the same pattern as scope_code on CompetencyRuleProfile. It also needs a migration that backfills course on existing leaves and on the groups below them. The follow-up then covers the call sites you listed: resolve_supplied_leaf_group(), get_courses_with_relative_criteria(), update_competency_criteria_group(), and the groups query itself.

[ormsbee]

Thank you for walking me through that. 😄 Just a few small requests then:

1. Please file the follow-up issue for the groups query and link it here, with the constraint reasoning from your comment and the ADR's current wording in it, so the decision gets made in one place. I defer to @mgwozdz-unicon on the prioritization of actually implementing it.
2. Please update the PR description, which still describes the earlier version: the parent__course_id__in predicate, the fallback query for an archived parent, total_criteria_count as "always the same query regardless of scope", and #681's branch as the base.
3. Please rebase + squash with an updated commit message, and I'll merge.

Thank you.

---

Claude assistance

Claude drafted the first pass of this review and re-ran the CBE and catalog suites (592 tests), `makemigrations --check`, the quality checks (pylint, mypy, pycodestyle, pydocstyle, isort, import-linter), and the docs build on b8f991c for me, all clean, plus the row-count probe from the first round: scoped to one of 20 runs, the criteria query now names 12 group ids and fetches the 10 rows it returns, and the total comes from a `COUNT`. Claude also did the trial merge onto `main` (`d6e6810`) in a scratch worktree: one conflict, in the `api.py` comment, and after resolving it the same suites passed 607 tests and `makemigrations --check` reported no changes.

[ufedaseyeuconsultant]

@ormsbee

1. Filed [Follow-up BE] Scope the criteria-groups query by course instead of loading every group #919. It quotes ADR 0002 Decision 2's current course_id wording, which the [BE] Enforce one root Competency Criteria Group per competency in the database #872 amendment on main changed on 2026-10-22 to "only the course-level group carries it". So the code and the ADR now agree, and the open question is whether to record the course on every group so the groups query can filter by it. The issue carries the (tag, course, parent) constraint reasoning and three options: keep the current model, put course_id on every group and replace the constraint, or add a separate non-unique subtree-course column. Prioritization is left to @mgwozdz-unicon.
2. Rewrote the PR description to match the code: scoping by course-level group during the tree walk, criteria fetched only for the returned groups, total_criteria_count from a separate COUNT on a scoped read, and main as the base.
3. Rebased onto main and squashed into one feat: commit. The one conflict was the tree-walk comment in api.py, resolved by taking main's wording. The CBE and catalog suites pass (607 passed, 1 skipped), as do pylint, mypy, isort, import-linter and makemigrations --check.