[javoconsultant]

Closes #760.

What this adds

This PR adds PATCH /api/cbe/v1/competencies/<tag_id>/criteria-groups/<group_id>/ and update_competency_criteria_group() to the CBE applet's public API. Together they let an author change how one CompetencyCriteriaGroup combines the criteria and groups beneath it, by setting its logic_operator to AND or OR. Nothing else about the group or its subtree changes, and no StudentCompetency*Status row is read or written.

The view is a thin adapter. It resolves the competency from the URL and first checks that the caller can read the competency's criteria tree, using the same oel_tagging.view_taxonomy check as the criteria-tree endpoint (#681). Only then does it look up the group and check the request body against the stored group, because both steps reveal what is stored. Finally it calls update_competency_criteria_group(). That function makes every other refusal, including the edit permission check, so an in-process caller such as Studio is refused in the same cases as an HTTP caller.

Request body

logic_operator is required, and it is the only field that can change:

{"logic_operator": "OR"}

The body may also restate any other field that the criteria-tree endpoint (#681) returns for a group: id, parent_id, tag_id, course_key, name, and ordering. A client can therefore send back the object it read with only the operator changed. Each restated field must exactly equal the stored value; for example, a name is compared without trimming whitespace. A restated field that differs is rejected and named in the error. So is any key that the group representation does not include.

The response is the group as stored, in the same shape the criteria-tree endpoint returns.

Responses

Code	When
200	The operator changed, or the request asked for the operator the group already has.
400	logic_operator is missing, null, or not AND/OR. Also returned when the body has an unrecognized key or changes a fixed field, when the group is a root, or when no group at or above the target has a course to check permission against.
401	The caller is not authenticated.
403	The caller cannot view the competency's taxonomy (oel_tagging.view_taxonomy). That check runs before the group is looked up or the body is compared, so such a caller gets 403 whatever group ID or body it sends. Also returned when the caller fails oel_tagging.can_tag_object for the competency's taxonomy and the group's course.
404	The tag is not a competency tag, the group does not exist, or the group belongs to a different competency than the one in the URL.
405	The request uses any method other than PATCH.
409	The group is archived. An archived group is refused even when the request would change nothing.

Behavior worth reviewing

• History. The update is a single save(update_fields=["logic_operator"]) inside transaction.atomic(), with _history_user set from the caller. As a result, django-simple-history records exactly one row per real change, attributed to that user (ADR 0003 Decision 1). A request for the operator the group already has returns 200 without saving, so it adds no history row.
• Course used for the permission check. A course-level group carries its own course. Any group below a course-level group uses the course of its nearest ancestor that has one, however deeply it is nested.
• Order of checks in update_competency_criteria_group(). The function checks for a root group, resolves the course, checks permission, validates the operator value, refuses an archived group, and then skips the save if nothing changed. The archived check runs before the no-op check so that an archived group is never reported as successfully edited.
• New public exception. CompetencyCriteriaGroupArchivedError is raised for an archived group. The view maps it to 409 through the existing CompetencyCriteriaConflict.

Deviations from #760

• The ticket lists course_id, oel_tagging_tag_id, and archived among the fields a client may restate. Instead, the endpoint accepts the fields that the criteria-tree endpoint returns, under their REST names (course_key, tag_id), plus id. archived is not part of that representation, so a body that includes it is rejected as an unrecognized key.
• The ticket resolves the permission course from either a course-level group or a leaf. The endpoint walks up to the nearest ancestor with a course, so a group nested below a leaf can also be edited. A non-root group with no course anywhere above it is rejected with 400 before the permission check.
• The ticket orders the permission check before request validation. Here, the permission check is split in two. The view first checks that the caller can read the competency's criteria tree, so a caller without read access is refused with 403 before anything stored is revealed. It then checks the body's shape: a missing or null operator, unrecognized keys, and changed fixed fields. That check stays in the view because only a REST body carries the fixed fields. The edit permission check (oel_tagging.can_tag_object) runs after it, inside the API function, followed by the check that the operator is AND or OR. As a result, a caller who can read the tree but cannot edit the group's course gets 400, not 403, for a malformed body.

Related work

• [BE] Build endpoint for removing a Competency Criteria Group #675 removes a group and its subtree at the same path. Its branch currently registers a separate CompetencyCriteriaGroupDeleteView there. Django dispatches a path to the first pattern that matches it, so whichever PR lands second should add its method to the other PR's view class instead of registering a second route. If this PR lands first, [BE] Build endpoint for removing a Competency Criteria Group #675 would add delete() to CompetencyCriteriaGroupDetailView and add "delete" to its http_method_names.
• [EPIC-CC] Competency Guardrails #723 (Epic 10) owns the learner-status warning that will sit in front of this endpoint.

Testing

• tests/openedx_learning/applets/cbe/test_api.py covers each acceptance-criteria scenario at the API level. It also covers these cases:
  • One history row per real edit, attributed to the caller, and none for a no-op.
  • An archived group is refused even when the request would change nothing.
  • A root group is rejected before the permission check.
  • The permission course is resolved correctly for course-level, leaf, and nested groups.
  • No learner status table is queried on any path.
• tests/openedx_learning/applets/cbe/test_views.py covers every status code above. That includes each fixed field being changed, a restated name being compared exactly, and a client sending back the representation it read from the criteria-tree endpoint.

On this branch, all 449 tests in tests/openedx_learning/applets/cbe pass, and pylint, mypy, pycodestyle, isort, and lint-imports are clean.

🤖 Generated with Claude Code

[openedx-webhooks]

Thanks for the pull request, @javoconsultant!

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#3 (review)

[ormsbee]

Two requests before merging, both small:

1. Please keep Http404 out of api.py, same as on #847 and #874 (inline).
2. Please check that the caller can read the competency before the body gets compared against the stored group. Right now a caller who can't read the tree can still learn a group's name, course_key, and parent_id from whether a guess gets a 400 or a 403 (inline on the view, with Claude's probe).

There are also a few smaller, non-blocking things in the inline comments. Thank you.

---

Claude drafted the first pass of this review and ran the CBE suite (449 tests), the quality checks (pylint, mypy, pycodestyle, pydocstyle, isort, import-linter), and makemigrations --check on 58290ed for me, all clean, plus the permission and query-count probes described inline.

[javoconsultant]

@ormsbee thanks! Both blocking items and the nits are addressed in a9f130c.

[ormsbee]

Thank you for the update. Everything from the last round is addressed.

Two last things, and then I'll merge:

1. Please update the PR description for the new permission order. The paragraph on the view, the 403 row, and the third "Deviations" bullet still say that the body is compared before any permission check.
2. Please squash to one commit with the description's context in the commit body, since the first commit doesn't have one.

Thank you.

---

Claude drafted this review and re-ran the CBE suite (454 tests), the quality checks (pylint, mypy, pycodestyle, pydocstyle, isort, import-linter), and makemigrations --check on a9f130c for me, all clean. CI is green, and the branch merges cleanly with main.

[javoconsultant]

Thank you for the update. Everything from the last round is addressed.

Two last things, and then I'll merge:

1. Please update the PR description for the new permission order. The paragraph on the view, the 403 row, and the third "Deviations" bullet still say that the body is compared before any permission check.
2. Please squash to one commit with the description's context in the commit body, since the first commit doesn't have one.

Thank you.

Claude drafted this review and re-ran the CBE suite (454 tests), the quality checks (pylint, mypy, pycodestyle, pydocstyle, isort, import-linter), and makemigrations --check on a9f130c for me, all clean. CI is green, and the branch merges cleanly with main.

@ormsbee all done