You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Arch] Implementation approach for competency data delete/edit guardrails #655
docs/openedx_learning/decisions/0002-competency-criteria-model.rst (Decision 7, "Delete protection boundaries") and 0003-competency-criteria-versioning.rst (Decision 4, "Authoring guardrails must warn on potentially impactful edits") already decide what should happen when a Platform Administrator or Course Author tries to change competency taxonomy data once a related learner mastery status exists:
Hard delete is blocked once any related StudentCompetencyStatus, StudentCompetencyCriteriaGroupStatus, or StudentCompetencyCriteriaStatus row exists, for Taxonomy/CompetencyTaxonomy, Tag, ObjectTag, CompetencyCriteriaGroup, and CompetencyCriteria.
CompetencyRuleProfile is never hard-deletable, with or without learner status.
Edits are allowed even after learner status exists, but must warn the user and require explicit confirmation.
What's not yet decided is how to implement this: how to compute and surface a "can this be deleted" signal on the relevant GET endpoints without introducing N+1 queries, and where the corresponding write-path validation should live so the rule is actually enforced, not just hinted at in the UI.
Ask
Agree on the implementation approach below before opening execution tickets against it. See the proposal in the comment.
Out of scope for this MVP (raised as an open question in the proposal, not silently assumed):
Gradeable subsection edit/delete behavior when a subsection has contributed to a learner's competency status. No existing Open edX precedent blocks or warns on this today; the proposal recommends staying at parity with current behavior for now.
Archiving/restore UI beyond what ADR-0002/0003 already require for delete-protected records.
CompetencyRuleProfile is not a stricter version of the "blocked once status exists" rule the other models follow, it's a different rule: there's no delete action for it at all, ever. This proposal doesn't add a DELETE endpoint for it, and it shouldn't have one.
Open question: should Tag/Taxonomy edits also require a warning?
ADR-0003 Decision 2 exempts Tag, Taxonomy, and CompetencyTaxonomy from the edit warning, on the grounds that these are "non-evaluative display/metadata" for competency purposes: editing a tag's name or a taxonomy's description doesn't change how mastery gets computed, so there's nothing to warn about.
Worth reconsidering for consistency: every other model in scope warns on edit once learner status exists, and a Platform Administrator has no way to know which of these five models is the "safe to edit silently" one without reading the ADR. The tradeoff is added friction on what's likely the highest-frequency, lowest-risk edit in the set (fixing a typo in a tag's display name).
Raising this here rather than deciding it unilaterally: if the answer is "yes, warn here too," that's an amendment to ADR-0003 Decision 2, not just an implementation detail, and this issue is already the place we're getting community agreement on this feature.
Proposed approach, part 1: surface deletability on GET endpoints
Add a deletable field to the relevant list/detail endpoints (CompetencyCriteria, CompetencyCriteriaGroup, Tag, Taxonomy/CompetencyTaxonomy, ObjectTag) so the frontend can disable the delete action without needing a separate round trip per record.
Compute it as a queryset annotation using Django's Exists()/OuterRef(), which asks the database to answer "does at least one matching row exist" as a correlated subquery run once per row, entirely inside the single SQL statement that already fetches the list. That avoids checking each record with a separate query.
CompetencyCriteria.deletable = NOT Exists(StudentCompetencyCriteriaStatus.objects.filter(competency_criteria=OuterRef('pk')))
CompetencyCriteriaGroup.deletable = NOT Exists(StudentCompetencyCriteriaGroupStatus.objects.filter(competency_criteria_group=OuterRef('pk')))
Tag.deletable = NOT Exists(StudentCompetencyStatus.objects.filter(tag=OuterRef('pk')))
ObjectTag.deletable = NOT Exists(StudentCompetencyCriteriaStatus.objects.filter(competency_criteria__object_tag=OuterRef('pk'))) (no direct FK from any status table to ObjectTag; joins through CompetencyCriterion)
Taxonomy/CompetencyTaxonomy.deletable = NOT Exists(StudentCompetencyStatus.objects.filter(tag__taxonomy=OuterRef('pk'))) (joins through Tag, since no status table points at Taxonomy directly; widest fan-out of the group, worth confirming indexes on StudentCompetencyStatus.tag_id and Tag.taxonomy_id before shipping)
CompetencyRuleProfile.deletable: not applicable, there's no delete action to gate (see table above).
This repo doesn't yet use Exists() specifically, but it does use the same underlying technique (OuterRef + Subquery) for the same purpose elsewhere — e.g. child_count in openedx_tagging/models/base.py and last_contributed in openedx_content/applets/publishing/api.py are both computed as correlated subquery annotations to avoid per-row queries. Exists() is the more direct tool for a plain yes/no check than Subquery+Count.
Whether to show a disabled delete action or an enabled one with a warning dialog (mirroring the edit-warning pattern) is a UX call, not an API one; either way the API needs to expose the same deletable signal. Flagging it here so it isn't decided by default: a disabled control with no explanation risks leaving the admin unsure why, so this probably needs a tooltip or similar at minimum. Leaving the actual choice to UX design.
Proposed approach, part 2: enforce it on write endpoints
The GET-endpoint flag above is a UX convenience, not enforcement — it disables a button, but nothing stops a direct API call from deleting a protected record. The actual rule needs to live on the write path too:
DELETE: each DELETE endpoint for the five deletable-with-conditions models re-runs the same existence check server-side (perform_destroy or the model's delete path) and returns 409 Conflict, naming the blocking status record(s), if one is found. CompetencyRuleProfile simply has no DELETE endpoint to guard.
Edit: no backend block needed, ADR-0003 permits the edit and only requires the frontend warning + confirmation step (scope fields on CompetencyRuleProfile/CompetencyCriterion stay immutable after creation per ADR-0002, but that's ordinary field validation, not guardrail logic).
Open question: gradeable subsections
Student competency statuses are ultimately derived from grades, which raises the question of whether editing or deleting a gradeable subsection after a learner has been graded on it needs a similar guardrail.
There's no existing precedent to inherit here. openedx-platform's only grading-immutability rule (grades/docs/decisions/0005-grade-freeze.rst) freezes grade values a fixed time after course end, unrelated to content edits, and Studio's delete handler removes any xblock, including graded subsections, with no grade check at all. There's also no FK from any StudentCompetency*Status table to a subsection or usage key, so deleting one wouldn't corrupt an already-computed status row, it would only affect the ObjectTag linking that subsection to a taxonomy tag, and orphaned ObjectTags already degrade safely (see below).
Proposed answer: out of scope for this MVP. Leave subsection edit/delete at parity with today's behavior, no new restriction, no new warning. Flagging as an explicit open question rather than a silent assumption: if the community wants better protection here, it's a new precedent for openedx-platform, not something this feature should introduce as a side effect.
Out of scope
Any DB-level FK constraint (ON DELETE RESTRICT) in place of application-layer validation. Keeping the check at the serializer/view layer matches how this ADR set already handles similar validation (see ADR-0002's "backend validation must reject" language for empty criteria groups) and keeps the error message controllable.
Archiving workflows beyond the archive-only path ADR-0002/0003 already specify for ObjectTag and CompetencyRuleProfile.
Appendix: how orphaned tags already degrade safely
Referenced above, for context: ObjectTag (the record linking a piece of content to a tag) doesn't only hold a foreign key to Tag/Taxonomy, it also stores its own copy of the tag's display text (_export_id, _value) at the time the content was tagged. The foreign keys are set to NULL automatically if the source Tag/Taxonomy is deleted (on_delete=SET_NULL, not a cascade delete), but the cached copy stays put, so the content's tag display keeps working using the cached text instead of breaking or going blank. See ADR-0006 and openedx_tagging/models/base.py.
Appendix: PUT vs PATCH for the new endpoints
This repo has no single stated convention. TaxonomyView uses DRF's default ModelViewSet, which supports both. ObjectTagView explicitly disables PATCH and only allows PUT, because its update semantics are a full replace. Given ADR-0002/0003 describe edits to CompetencyRuleProfile and CompetencyCriteria as changing specific fields only (e.g. "editing a profile may change rule_type/rule_payload only"), PATCH is the better fit for the new CompetencyCriteria/CompetencyCriteriaGroup/CompetencyRuleProfile endpoints, following the same "restrict when the semantics call for it" reasoning ObjectTagView already sets as precedent, not a repo-wide rule.
Some thoughts about adding this deletable property.
If it's a UX nice-to-have to disable the button, but there's no real critical reason not to just return an error when trying to delete except from aesthetics, I wouldn't think this is needed for the MVP. The backend guardrail already prevents deletion, so as we are talking about an "MVP"-like iteration, I would say this is an effort that we can consider out of scope for the competencies MVP. Very much "nice-to-have"-y.
Reading the criteria will happen a lot, while deleting something will happen only occasionally. I'm not sure adding the additional database JOINS and such to every single read is optimally performance-wise, so we should add caching here with generous time to live.
If instead you make the "deletable" a database field and set it when a related object is written that prevents deletion would be better than calculating at every read, but need some overhead in terms of development to make sure this works correctly.
If we really want to go with the "deletable" property - and I don't see a strong reason not to move this out scope - I would recommend using caching.
About deleting / editing graded subsections that have already been graded by learners: we investigated this a while ago. As far as I'm up to date:
editing or deleting a subsection with existing grades can indeed skew learner data and impose problems.
we generally hold course authors responsible when they edit subsections with learner data on live courses. I think there is a warning already in place. The philosophy so far is to not treat this as our responsibility.
For course authors editing / deleting already graded subsections there are ways for them to fix it, I believe. With our system of append-only writing fulfilled assignments, editing / deleting graded subsections should not impact the mastery, but if it would for some reason, we should also provide some conceivable way to fix it.
I second considering this out of scope while we make sure that deleting / editing subsections don't corrupt skill mastery for a learner; if we do find a reason to worry about this, I recommend just providing some way for course authors to correct it if it happens.
Decided to keep deletable in scope: frontend validation should match backend validation, so the delete action can be disabled/warned on directly rather than only surfacing the block after a failed request. Went with a persisted field rather than caching, given the read-cost concern you raised. It turned into a real design question (how do you keep a persisted signal correct without a general invalidation scheme), so it got its own ADR rather than staying an implementation detail here: openedx-core#661 adds ADR-0005. Short version: the flip is one-directional (deletable only ever goes True to False, never back, since learner status is append-only), set synchronously at learner-status write time, so there's no cache-invalidation problem to solve in the first place.
Agreed, and thanks for confirming the existing precedent there. Treating gradeable-subsection edit/delete as out of scope for this MVP: no new restriction, no new warning, revisit only if it turns out to actually corrupt mastery in practice.
Posting a status update on the "Proposed approach" sections above, since both halves have moved since mid-July.
Part 2 (enforcement) is done for CompetencyCriteria and CompetencyCriteriaGroup.#674 and #675 spec the DELETE behavior directly: check for a referencing StudentCompetencyCriteriaStatus/StudentCompetencyCriteriaGroupStatus row, hard-delete if none exists, archive (archived=True) instead of deleting if one does. That answers the enforcement half of this proposal for those two models.
Part 1 (the GET-side deletable signal) is also settled without needing the standalone ADR (#661), for CompetencyCriteria, CompetencyCriteriaGroup, and ObjectTag.#681 already returns the criteria-group/criteria list for a competency, and ObjectTagView's own GET endpoint only ever returns tags for one object at a time. Both lists are small and already scoped narrowly enough, one competency or one object, that adding a plain per-row Exists() annotation to each is implementation detail on those endpoints rather than a design question.
Tag and Taxonomy/CompetencyTaxonomy are still open, and that's now the entire remaining scope of #661. The open question there is real: those two are read through the existing, much higher-traffic TaxonomyTagsView list rather than a narrowly-scoped one, so the read-cost concern Jesper raised on this issue back in July still applies. I've posted a proposal on #661 to resolve it using the same technique openedx_tagging already uses for per-tag usage_count (a bulk query plus in-Python computation, instead of a per-row database annotation), rather than the persisted database field originally proposed here. Following up there rather than duplicating the technical discussion in two places.
If anyone here has thoughts on the remaining Tag/Taxonomy question, #661 is where that's being worked out.
Implementation proposal: delete/edit guardrails for competency taxonomy data
Updated from the original proposal above to reflect where things stand after #674, #675, #681, and the discussion on #661. Section by section below; anything not mentioned is unchanged from the original.
What's already decided (not up for debate here, except the open question below)
Unchanged from the original proposal:
Model
Delete
Edit warning (once learner status exists / profile is "in use")
CompetencyCriteria
Blocked once learner status exists
Required
CompetencyCriteriaGroup
Blocked once learner status exists
Required
Tag / Taxonomy / CompetencyTaxonomy
Blocked once learner status exists
Not required per ADR-0003 today — see open question below
ObjectTag
Hard delete allowed before learner status exists; archive-only (soft delete) after
Required
CompetencyRuleProfile
No DELETE endpoint at all. Archive (archived field, set via a normal field-level update) is the only removal path, regardless of learner status
Open question: should Tag/Taxonomy edits also require a warning?
Unchanged from the original proposal. Not discussed on #661; still open.
Resolved: gradeable subsections
No longer an open question. jesperhodge confirmed on this issue (comment, 2026-07-13) that editing/deleting a gradeable subsection after learner grading is a known, separately-handled risk in openedx-platform today, that course authors are already held responsible for it, and that there's a way for authors to correct it if it happens. Agreed: out of scope for this MVP, no new restriction, no new warning, revisit only if it turns out to actually corrupt mastery in practice.
Proposed approach, part 1: surface deletability on GET endpoints
This has split into two tracks with different answers, where the original proposal treated all five models the same way.
CompetencyCriteria, CompetencyCriteriaGroup, and ObjectTag: settled, no design question remains. Each of these is read through an endpoint scoped to a single competency's criteria tree or a single object's tags (#681 for the first two; ObjectTagView.get_queryset, which only ever returns tags for one object at a time, for the third). At that scope, a plain per-row Exists() annotation, exactly as the original proposal described, is implementation detail on each endpoint rather than a design decision. No standalone design doc needed for these three.
Tag and Taxonomy/CompetencyTaxonomy: still the open question, being worked out on #661. These are read through TaxonomyTagsView, a much higher-traffic endpoint used throughout Studio's tagging UI, not just this feature. jesperhodge's original cost concern (comment, 2026-07-13) — that adding a per-row JOIN to every read of a high-traffic list is worth avoiding — still applies here, which is why this pair needs a different answer than the other three:
Rejected: a persisted deletable field. This was the original response to Jesper's concern (issue comment, 2026-07-15) and became ADR-0005 / PR #661: a boolean set synchronously at learner-status write time, flipping one-directionally from True to False. kdmccormick and jesperhodge both came out against maintaining a bespoke deletable row (PR docs: add ADR for persisted competency deletability signal #661 comments, 2026-08-13 and 2026-08-18), and mgwozdz has proposed closing PR docs: add ADR for persisted competency deletability signal #661 as a result once this is settled.
Rejected: a dry-run delete-and-rollback using ON DELETE RESTRICT. bradenmacdonald proposed this on PR docs: add ADR for persisted competency deletability signal #661 (2026-07-20): attempt the real delete inside a transaction, roll it back, and let the FK constraint's RestrictedError report what's blocking it. This depends on Tag/Taxonomy's foreign keys carrying a real DB constraint, which was in question at the time because Competency Status Storage ADR #657 (still open) proposed moving learner-status tables onto a separate DB alias with logical-only FKs. That dependency has since resolved: jesperhodge confirmed (PR docs: add ADR for persisted competency deletability signal #661, 2026-07-22/23) that only the ...History table moves to a separate alias, and the active status tables keep normal foreign keys, so the constraint would in fact be there to trial against. Even so, mgwozdz's response (PR docs: add ADR for persisted competency deletability signal #661, 2026-08-16) is that this doesn't serve the same purpose as the GET-side signal: a dry-run only tells you the answer at the moment you attempt to delete, so it can produce a better error message but it cannot let Studio disable the delete control before the user opens that dialog in the first place, which is what this deletable field is for.
Current direction: the same bulk-query-plus-Python-attachment technique the endpoint already uses for usage_count.TaxonomyTagsView already computes something analogous via add_usage_counts() (openedx_tagging/api.py:529), whose own docstring calls a per-row annotation "a very expensive annotation" and instead runs one query for the whole page of tags, then attaches each tag's count in Python before the response goes out. Applying the same technique to deletable (one bulk existence-check query per page, attached to each row in Python) avoids the per-row JOIN Jesper flagged, without needing a persisted field or any write-side bookkeeping. deletable is a plain existence check with no counting or dedup, so it should cost less than usage_count already does on this same endpoint today. As of 2026-08-16, kdmccormick's summary confirmed no objection to this direction and jesperhodge confirmed no objections (2026-08-18).
Whether to show a disabled delete action or an enabled one with a warning dialog is still nominally a UX call, but mgwozdz stated a preference on PR #661 (2026-08-16) for disabling with an explanation over letting someone click through to a failed attempt, given that ADR-0002 blocks the delete outright rather than allowing an override. Not yet a final decision.
Proposed approach, part 2: enforce it on write endpoints
CompetencyCriteria and CompetencyCriteriaGroup: settled and shipped differently than originally proposed.#674 and #675 spec the actual DELETE behavior: check for a referencing StudentCompetencyCriteriaStatus/StudentCompetencyCriteriaGroupStatus row; if none exists, hard-delete; if one exists, set archived=True instead of deleting. This replaces the original proposal's "return 409 Conflict, naming the blocking record" for these two models — the record is archived instead of the request being rejected.
Tag/Taxonomy: recommend enforcing the delete block at the database level rather than with an application-layer check, reversing the original proposal for this pair. A foreign key can be configured to automatically block a delete when other rows still reference it (on_delete=RESTRICT), raising an error the view can catch and return, instead of the view checking first. The original proposal ruled this out because #657 was going to move the learner-status tables to a separate database without a real constraint to enforce; #657 has since resolved to keep those tables on normal foreign keys (only the ...History table moves), so the constraint is available again. We recommend adopting the RestrictedError idea from PR #661 (2026-07-20) for the real DELETE call itself, not just as an on-demand check. This differs from CompetencyCriteria/CompetencyCriteriaGroup, which run their own manual existence check instead because those will be archived instead of deleted in this case per ADR 0002 (#674/#675).
Edit: unchanged from the original proposal, no backend block needed.
Out of scope
Unchanged for CompetencyRuleProfile and the archiving-workflow line. The blanket "no DB-level FK constraint" line no longer applies cleanly to Tag/Taxonomy per the open point above; it still holds for CompetencyCriteria/CompetencyCriteriaGroup, whose enforcement mechanism (#674/#675) is application-layer archive-on-conflict, not a DB constraint.
Appendix: how orphaned tags already degrade safely
@jmakowski1123 I'd like to nail down the precise product requirements for our Competency Guardrails Epic before settling on a specific technical approach here. Could you please review the following questions and options and let me know what you think is best? cc @adolfo@ormsbee@thelmick-unicon
Competency data guardrails: questions for Product
Once a learner has been credited with mastering a competency, we want to make sure an admin can't accidentally destroy the tag, rule, or grouping behind that record. For each record type below there are two questions: what happens to the record itself, and what the admin sees when they try to delete it, from the specific page where that removal happens.
"Scope" marks whether an option was already part of our original ticket list. "Development Effort" is a rough relative sizing (Lowest/Low/Medium/Medium-High/Highest).
Already decided (Context only)
Once a learner has been credited using a competency rule, rule grouping, tag, taxonomy, or tagged association, an admin silently and permanently destroying that record isn't acceptable according to CBE ADR-0002.
That same ADR says retiring any of these five record types (Tag, Taxonomy, Object Tag association, Competency Criterion, Competency Criteria Group) may be archive-only rather than a hard block — it doesn't single any of them out as block-only. Right now, only Competency Criterion ([BE] Delete Competency Criterion #674) and Competency Criteria Group ([BE] Build endpoint for removing a Competency Criteria Group #675) have tickets drafted to actually build that. The questions below ask, case by case, whether the rest should match or stay simpler.
Taxonomy Editing page
Tag
Parity Note: deleting a tag today already shows a confirmation dialog.
Q1. Once a learner has been credited using a tag, what should happen if someone tries to delete it?
Option
Scope
Development Effort
Block it permanently — no way to ever delete it
Simpler, but doesn't match current draft of #674 & #675 described below
Lowest
Archive it instead — hide it going forward, keep it on file, matching Criterion/Group
The ADR explicitly allows this ("may be archive-only"), though doesn't mandate it — no ticket has scoped it yet
Medium-High (needs a new "archived" concept for tags, which doesn't exist today)
Q2. Depending on your Q1 answer, what should the admin see?
If Q1 is...
Admin sees...
Scope
Development Effort
Block
Nothing changes up front; error only if they try and it fails
No
Lowest
Block
Delete option greyed out / disabled in advance
Yes
Medium
Block
Delete option removed entirely
No
Medium
Block
Confirmation dialog; block only shows up after they confirm
No
Low
Archive
No visible difference — disappears the same as a real removal
No
Lowest
Archive
Confirmation dialog before archiving
No
Low
Archive
Distinct "archive" icon and tooltip, clear in advance
Yes
Medium-High
Taxonomy
Parity Note: deleting a taxonomy today already shows a confirmation dialog too, same as deleting a tag.
Q3. Once a learner has been credited using any tag within a taxonomy, what should happen if someone tries to delete the whole taxonomy?
Option
Scope
Development Effort
Block it permanently — no way to ever delete it
Simpler, but doesn't match current draft of #674 & #675 described below
Lowest
Archive it instead — hide it going forward, keep it on file, matching Criterion/Group
The ADR explicitly allows this ("may be archive-only"), though doesn't mandate it — no ticket has scoped it yet
Medium-High (same missing-field gap as tags)
Q4. Depending on your Q3 answer, what should the admin see?
If Q3 is...
Admin sees...
Scope
Development Effort
Block
Nothing changes up front; error only if they try and it fails
No
Lowest
Block
Delete option greyed out / disabled in advance
Yes
Medium
Block
Delete option deleted entirely
No
Medium
Block
Confirmation dialog; block only shows up after they confirm
No
Low
Archive
No visible difference — disappears the same as a real removal
No
Lowest
Archive
Confirmation dialog before archiving
No
Low
Archive
Distinct "archive" icon and tooltip, clear in advance
Q8. Depending on your Q7 answer, what should the admin see?
If Q7 is...
Admin sees...
Scope
Development Effort
Block
Nothing changes up front; error only if they try and it fails
No
Lowest
Block
Delete option greyed out / disabled in advance
Yes
Medium
Block
Delete option deleted entirely
No
Medium
Block
Confirmation dialog; block only shows up after they confirm
No
Low
Archive
No visible difference — disappears the same as a real removal
No
Lowest
Archive
Confirmation dialog before archiving
No
Low
Archive
Distinct "archive" icon and tooltip, clear in advance
Yes
Medium-High
Object Tag associations
(For CBE, this is the link that connects a subsection to a competency tag, but this is also used by plain taxonomy tags to link any content to a plain tag.)
Bigger question first: an association created via the Competency Management page could, in principle, also show up on the Course Outline page or the Libraries page. But those two pages show tags generically today, with no competency context — an admin there has no way to tell a competency tag apart from an ordinary one.
Q9. What should happen on the Course Outline and Libraries pages?
Option
Scope
Development Effort
Don't display competency tags on these pages at all
Not yet scoped
Medium — confirmed no way to tell these tags apart exists today; needs one new field
Display them, but don't allow editing or deleting from these pages
Not yet scoped
Medium-High — needs that same new field, but the on/off switch for "can this tag be edited/deleted" already exists in the UI, which helps
Display them, and allow editing/deleting, following whatever process is chosen below for the Competency Management page
Not yet scoped
Highest — confirmed, and likely higher than shown: this depends on the Competency Management page's own guardrail behavior (Q5-Q8 above), which isn't built yet either
(If you pick the third option, Q10/Q11 below apply on Course Outline and Libraries too, using the same answer. If you pick either of the first two, Q10/Q11 below apply only to the Competency Management page.)
Q10. Once a learner has been credited using a tagged association, what should happen if someone tries to delete it?
Option
Scope
Development Effort
Block it permanently — no way to ever delete it
Simpler, but doesn't match current draft of #674 & #675
Lowest
Archive it instead — hide it going forward, keep it on file, matching Criterion/Group
The ADR explicitly allows this ("may be archive-only"), though doesn't mandate it — no ticket has scoped it yet
Medium-High (same missing-field gap as tags)
Q11. Depending on your Q10 answer, what should the admin see?
If Q10 is...
Admin sees...
Scope
Development Effort
Block
Nothing changes up front; error only if they try and it fails
No
Lowest
Block
Delete option greyed out / disabled in advance
Yes
Medium
Block
Delete option removed entirely
No
Medium
Block
Confirmation dialog; block only shows up after they confirm
No
Low
Archive
No visible difference — disappears the same as a real removal
No
Lowest
Archive
Confirmation dialog before archiving
No
Low
Archive
Distinct "archive" icon and tooltip, clear in advance
Yes
Medium-High
Graded assignments already used in mastery calculations
This one is a proposal of mine that still needs your sign-off, not something engineering has already agreed to. My proposal: leave today's behavior as-is, no new warning when editing or deleting a graded assignment a learner has already been credited against. This matches how the platform already treats graded assignments today (parity), and avoids new engineering work here.
Q12. Do you approve leaving this as-is for the MVP, or do you want a new warning added?
Option
Scope
Development Effort
Approve as-is — no new warning, matches today's platform behavior
If we went with the "never delete it options" here, is there a way to hide it from the end user in the UI? The assumption here is that an admin doesn't want to use (or have their teams use) that tag going forward. So it were hidden from the UI, but saved on the backend for learners already in progress or who have completed it, is that the best outcome?
Hi, Claude here, coming from a conversation with @jesperhodge.
@mgwozdz-unicon@ormsbee: while bringing #613's subtasks in line with its latest revision, I hit a case
where the on_delete values #641 sets and the approved design in this issue disagree. Flagging it
before either gets built. I have left #641 and #642 declaring PROTECT as a fail-closed value and
recorded the question as open in both, rather than resolving it myself.
Explaining it from the ground up, since the interaction is easy to miss.
The mechanic
Every Django foreign key has to declare what happens when the row it points at is deleted. Two values
matter here:
CASCADE means "delete me too."
PROTECT means "refuse the whole deletion and raise ProtectedError."
The three players
A Tag is a competency, say "Writing Poetry." It lives in openedx_tagging, a generic library used
across Open edX that knows nothing about CBE.
A CompetencyCriteriaGroup is a CBE row saying "to demonstrate Writing Poetry, pass assignment 1 OR
assignment 2." It holds a foreign key pointing at that Tag.
A learner status row records that a specific learner actually earned mastery against the competency.
The reference direction is what matters: the CBE row points at the Tag, and the Tag knows nothing
about the CBE row.
What this issue decided
When an admin deletes a competency tag, the outcome depends on whether any learner has earned mastery
against it. The approved table says: archive it if someone has, and hard-delete it, unchanged from
today's behavior, if nobody has.
The load-bearing part is the enforcement mechanism. @ormsbee rejected the original proposal, in which openedx_tagging would ask CBE "is this in use?", in favor of a push model: CBE stamps a deletion_locked flag onto the ObjectTag row when a learner earns something, and at delete time the
tagging code reads only its own flag. As the design puts it, openedx_tagging never calls into CBE.
The hole
Walk the second branch. An author has built a criteria tree for "Writing Poetry," but no learner has
been graded yet. That is a completely ordinary state, not an edge case: #665 creates the root,
course-level and leaf CompetencyCriteriaGroup rows at authoring time, long before any grade exists.
Because nobody has earned anything, deletion_locked is false, so the tagging code proceeds to a plain
hard delete. But a CompetencyCriteriaGroup row is still pointing at that tag. With PROTECT on that
foreign key, Django refuses and raises ProtectedError. The admin gets a failure where this issue
promised a successful delete, and nothing cleans up the CBE row first, because the tagging code is
deliberately forbidden from knowing the CBE row exists.
Three things I checked rather than assumed:
Both delete paths are plain ORM deletes that go through Django's collector: taxonomy.delete() in src/openedx_tagging/rest_api/v1/views.py, and Taxonomy.delete_tags() in src/openedx_tagging/models/base.py.
Tag.taxonomy is already on_delete=CASCADE, so deleting a taxonomy collects every tag beneath it
and each one hits the same wall. That makes taxonomy deletion the likeliest instance rather than an
outlier, since a CompetencyTaxonomy is precisely the taxonomy whose tags carry criteria.
Django's PROTECT raises even when the referencing row is in the same delete batch. Only RESTRICT
exempts that case.
Two foreign keys cross this boundary: CompetencyCriteriaGroup.tag to oel_tagging_tag, and CompetencyCriterion.object_tag to oel_tagging_objecttag. StudentCompetencyStatus.tag is not
affected, because once learner status exists this issue archives the tag rather than deleting it, and
archiving is a field update that never reaches on_delete.
The options as I see them
Option
What it costs
Keep PROTECT and have the tagging-side delete paths resolve referencing CBE rows before hard-deleting
Real work that this issue's design did not scope, and it needs a way to do it without openedx_tagging importing CBE.
Make the two crossing foreign keys CASCADE
Deleting a tag with no learner status then works with no tagging-side work at all, but it silently destroys the author's criteria tree with no archive step. It also needs CompetencyCriterion.group and CompetencyCriteriaGroup.parent to be CASCADE, otherwise the collector walks down and trips their PROTECT instead, so it is closer to a full revert to the earlier database-level design than a two-line change.
Set the lock at authoring time rather than at learner-status write time
Closes the gap by making deletion_locked true whenever criteria exist, but it changes what the flag means and would block deletes this issue says should succeed.
I do not think this reopens the design. The push-based lock is right for the case it was chosen for,
which is protecting earned mastery. What is missing is the unlocked branch, where CBE rows exist and no
mastery has been earned yet.
@jesperhodge@ormsbee
Thanks for digging into this. After walking through the tradeoff with Claude, I think the two crossing foreign keys, CompetencyCriteriaGroup.tag and CompetencyCriterion.object_tag, should become CASCADE rather than staying PROTECT.
The case that matters here is a tag with an authored-but-ungraded criteria tree: no learner has earned anything yet, so the delete should succeed, but it currently fails because the criteria rows still point at the tag. Keeping PROTECT and having the tagging-side delete paths clean those rows up first (your option 1) means openedx_tagging has to call into CBE-specific models to do that cleanup, which breaks the "never calls into CBE" boundary this design was built around. I think adhering to the dependency direction boundary of the design takes priority over addressing what feels like a bunch of edge cases to me, so these should become CASCADE. Here's that tradeoff laid out:
PROTECT (current)
CASCADE
Deleting a tag with no criteria attached
Succeeds normally
Succeeds normally
Deleting a tag with a live criteria tree, no learner status yet
Fails, this is the bug this thread is about
Succeeds, deletes the whole criteria tree along with it
Deleting a tag through a bug, script, admin action, or future plugin that skips the "should this archive?" check
Fails loudly (ProtectedError), nothing is lost
Succeeds silently, wipes out the author's competency definition with no warning
CBE dependency-direction problem (openedx_tagging calling into CBE)
Still needs solving
Goes away
One case isn't in the table because it doesn't actually depend on this choice: a tag with a live criteria tree where a learner has also earned status against it. StudentCompetencyStatus.tag, a separate foreign key from the learner's status row to the tag, already stays PROTECT regardless of what we pick here, and Django aborts the whole deletion if it finds even one PROTECT anywhere in what it collects, so that case is protected either way.
One thing worth flagging before this lands: CompetencyCriterion.group and CompetencyCriteriaGroup.parent are also PROTECT, so the collector would still hit those on its way down and abort partway through a tag deletion. Those would need to become CASCADE too for a tag delete to actually cascade cleanly through the whole tree, and that's a bigger change than just the tag FKs: it's the same backstop #674/#675 relies on to keep a group deletion from bypassing their student-status check, so it removes that protection for direct group deletion too, not only for deletion that starts from the tag. I'm applying the same edge-case reasoning to that as I am to the tag FKs, but flagging it since it's a second place this decision now reaches, not a detail of the first one.
Activity
mgwozdz-unicon commentedon Jul 8, 2026
Implementation proposal: delete/edit guardrails for competency taxonomy data
What's already decided (not up for debate here, except the open question below)
CompetencyCriteriaCompetencyCriteriaGroupTag/Taxonomy/CompetencyTaxonomyObjectTagCompetencyRuleProfilearchivedfield, set via a normal field-level update) is the only removal path, regardless of learner statusSource: ADR-0002 Decision 7, ADR-0003 Decision 4.
CompetencyRuleProfileis not a stricter version of the "blocked once status exists" rule the other models follow, it's a different rule: there's no delete action for it at all, ever. This proposal doesn't add a DELETE endpoint for it, and it shouldn't have one.Open question: should Tag/Taxonomy edits also require a warning?
ADR-0003 Decision 2 exempts
Tag,Taxonomy, andCompetencyTaxonomyfrom the edit warning, on the grounds that these are "non-evaluative display/metadata" for competency purposes: editing a tag's name or a taxonomy's description doesn't change how mastery gets computed, so there's nothing to warn about.Worth reconsidering for consistency: every other model in scope warns on edit once learner status exists, and a Platform Administrator has no way to know which of these five models is the "safe to edit silently" one without reading the ADR. The tradeoff is added friction on what's likely the highest-frequency, lowest-risk edit in the set (fixing a typo in a tag's display name).
Raising this here rather than deciding it unilaterally: if the answer is "yes, warn here too," that's an amendment to ADR-0003 Decision 2, not just an implementation detail, and this issue is already the place we're getting community agreement on this feature.
Proposed approach, part 1: surface deletability on GET endpoints
Add a
deletablefield to the relevant list/detail endpoints (CompetencyCriteria,CompetencyCriteriaGroup,Tag,Taxonomy/CompetencyTaxonomy,ObjectTag) so the frontend can disable the delete action without needing a separate round trip per record.Compute it as a queryset annotation using Django's
Exists()/OuterRef(), which asks the database to answer "does at least one matching row exist" as a correlated subquery run once per row, entirely inside the single SQL statement that already fetches the list. That avoids checking each record with a separate query.CompetencyCriteria.deletable= NOTExists(StudentCompetencyCriteriaStatus.objects.filter(competency_criteria=OuterRef('pk')))CompetencyCriteriaGroup.deletable= NOTExists(StudentCompetencyCriteriaGroupStatus.objects.filter(competency_criteria_group=OuterRef('pk')))Tag.deletable= NOTExists(StudentCompetencyStatus.objects.filter(tag=OuterRef('pk')))ObjectTag.deletable= NOTExists(StudentCompetencyCriteriaStatus.objects.filter(competency_criteria__object_tag=OuterRef('pk')))(no direct FK from any status table toObjectTag; joins throughCompetencyCriterion)Taxonomy/CompetencyTaxonomy.deletable= NOTExists(StudentCompetencyStatus.objects.filter(tag__taxonomy=OuterRef('pk')))(joins throughTag, since no status table points atTaxonomydirectly; widest fan-out of the group, worth confirming indexes onStudentCompetencyStatus.tag_idandTag.taxonomy_idbefore shipping)CompetencyRuleProfile.deletable: not applicable, there's no delete action to gate (see table above).This repo doesn't yet use
Exists()specifically, but it does use the same underlying technique (OuterRef+Subquery) for the same purpose elsewhere — e.g.child_countinopenedx_tagging/models/base.pyandlast_contributedinopenedx_content/applets/publishing/api.pyare both computed as correlated subquery annotations to avoid per-row queries.Exists()is the more direct tool for a plain yes/no check thanSubquery+Count.Whether to show a disabled delete action or an enabled one with a warning dialog (mirroring the edit-warning pattern) is a UX call, not an API one; either way the API needs to expose the same
deletablesignal. Flagging it here so it isn't decided by default: a disabled control with no explanation risks leaving the admin unsure why, so this probably needs a tooltip or similar at minimum. Leaving the actual choice to UX design.Proposed approach, part 2: enforce it on write endpoints
The GET-endpoint flag above is a UX convenience, not enforcement — it disables a button, but nothing stops a direct API call from deleting a protected record. The actual rule needs to live on the write path too:
perform_destroyor the model's delete path) and returns409 Conflict, naming the blocking status record(s), if one is found.CompetencyRuleProfilesimply has no DELETE endpoint to guard.CompetencyRuleProfile/CompetencyCriterionstay immutable after creation per ADR-0002, but that's ordinary field validation, not guardrail logic).Open question: gradeable subsections
Student competency statuses are ultimately derived from grades, which raises the question of whether editing or deleting a gradeable subsection after a learner has been graded on it needs a similar guardrail.
There's no existing precedent to inherit here.
openedx-platform's only grading-immutability rule (grades/docs/decisions/0005-grade-freeze.rst) freezes grade values a fixed time after course end, unrelated to content edits, and Studio's delete handler removes any xblock, including graded subsections, with no grade check at all. There's also no FK from anyStudentCompetency*Statustable to a subsection or usage key, so deleting one wouldn't corrupt an already-computed status row, it would only affect theObjectTaglinking that subsection to a taxonomy tag, and orphanedObjectTags already degrade safely (see below).Proposed answer: out of scope for this MVP. Leave subsection edit/delete at parity with today's behavior, no new restriction, no new warning. Flagging as an explicit open question rather than a silent assumption: if the community wants better protection here, it's a new precedent for
openedx-platform, not something this feature should introduce as a side effect.Out of scope
ON DELETE RESTRICT) in place of application-layer validation. Keeping the check at the serializer/view layer matches how this ADR set already handles similar validation (see ADR-0002's "backend validation must reject" language for empty criteria groups) and keeps the error message controllable.ObjectTagandCompetencyRuleProfile.Appendix: how orphaned tags already degrade safely
Referenced above, for context:
ObjectTag(the record linking a piece of content to a tag) doesn't only hold a foreign key toTag/Taxonomy, it also stores its own copy of the tag's display text (_export_id,_value) at the time the content was tagged. The foreign keys are set toNULLautomatically if the sourceTag/Taxonomyis deleted (on_delete=SET_NULL, not a cascade delete), but the cached copy stays put, so the content's tag display keeps working using the cached text instead of breaking or going blank. See ADR-0006 andopenedx_tagging/models/base.py.Appendix: PUT vs PATCH for the new endpoints
This repo has no single stated convention.
TaxonomyViewuses DRF's defaultModelViewSet, which supports both.ObjectTagViewexplicitly disables PATCH and only allows PUT, because its update semantics are a full replace. Given ADR-0002/0003 describe edits toCompetencyRuleProfileandCompetencyCriteriaas changing specific fields only (e.g. "editing a profile may changerule_type/rule_payloadonly"), PATCH is the better fit for the newCompetencyCriteria/CompetencyCriteriaGroup/CompetencyRuleProfileendpoints, following the same "restrict when the semantics call for it" reasoningObjectTagViewalready sets as precedent, not a repo-wide rule.jesperhodge commentedon Jul 13, 2026
Some thoughts about adding this
deletableproperty.If we really want to go with the "deletable" property - and I don't see a strong reason not to move this out scope - I would recommend using caching.
jesperhodge commentedon Jul 13, 2026
About deleting / editing graded subsections that have already been graded by learners: we investigated this a while ago. As far as I'm up to date:
I second considering this out of scope while we make sure that deleting / editing subsections don't corrupt skill mastery for a learner; if we do find a reason to worry about this, I recommend just providing some way for course authors to correct it if it happens.
mgwozdz-unicon commentedon Jul 15, 2026
@jesperhodge, addressed both:
deletablein scope: frontend validation should match backend validation, so the delete action can be disabled/warned on directly rather than only surfacing the block after a failed request. Went with a persisted field rather than caching, given the read-cost concern you raised. It turned into a real design question (how do you keep a persisted signal correct without a general invalidation scheme), so it got its own ADR rather than staying an implementation detail here: openedx-core#661 adds ADR-0005. Short version: the flip is one-directional (deletableonly ever goesTruetoFalse, never back, since learner status is append-only), set synchronously at learner-status write time, so there's no cache-invalidation problem to solve in the first place.mgwozdz-unicon commentedon Aug 12, 2026
Posting a status update on the "Proposed approach" sections above, since both halves have moved since mid-July.
Part 2 (enforcement) is done for
CompetencyCriteriaandCompetencyCriteriaGroup. #674 and #675 spec the DELETE behavior directly: check for a referencingStudentCompetencyCriteriaStatus/StudentCompetencyCriteriaGroupStatusrow, hard-delete if none exists, archive (archived=True) instead of deleting if one does. That answers the enforcement half of this proposal for those two models.Part 1 (the GET-side
deletablesignal) is also settled without needing the standalone ADR (#661), forCompetencyCriteria,CompetencyCriteriaGroup, andObjectTag. #681 already returns the criteria-group/criteria list for a competency, andObjectTagView's own GET endpoint only ever returns tags for one object at a time. Both lists are small and already scoped narrowly enough, one competency or one object, that adding a plain per-rowExists()annotation to each is implementation detail on those endpoints rather than a design question.TagandTaxonomy/CompetencyTaxonomyare still open, and that's now the entire remaining scope of #661. The open question there is real: those two are read through the existing, much higher-trafficTaxonomyTagsViewlist rather than a narrowly-scoped one, so the read-cost concern Jesper raised on this issue back in July still applies. I've posted a proposal on #661 to resolve it using the same techniqueopenedx_taggingalready uses for per-tagusage_count(a bulk query plus in-Python computation, instead of a per-row database annotation), rather than the persisted database field originally proposed here. Following up there rather than duplicating the technical discussion in two places.If anyone here has thoughts on the remaining
Tag/Taxonomyquestion, #661 is where that's being worked out.mgwozdz-unicon commentedon Aug 20, 2026
Implementation proposal: delete/edit guardrails for competency taxonomy data
Updated from the original proposal above to reflect where things stand after #674, #675, #681, and the discussion on #661. Section by section below; anything not mentioned is unchanged from the original.
What's already decided (not up for debate here, except the open question below)
Unchanged from the original proposal:
CompetencyCriteriaCompetencyCriteriaGroupTag/Taxonomy/CompetencyTaxonomyObjectTagCompetencyRuleProfilearchivedfield, set via a normal field-level update) is the only removal path, regardless of learner statusSource: ADR-0002 Decision 7, ADR-0003 Decision 4.
Open question: should Tag/Taxonomy edits also require a warning?
Unchanged from the original proposal. Not discussed on #661; still open.
Resolved: gradeable subsections
No longer an open question. jesperhodge confirmed on this issue (comment, 2026-07-13) that editing/deleting a gradeable subsection after learner grading is a known, separately-handled risk in
openedx-platformtoday, that course authors are already held responsible for it, and that there's a way for authors to correct it if it happens. Agreed: out of scope for this MVP, no new restriction, no new warning, revisit only if it turns out to actually corrupt mastery in practice.Proposed approach, part 1: surface deletability on GET endpoints
This has split into two tracks with different answers, where the original proposal treated all five models the same way.
CompetencyCriteria,CompetencyCriteriaGroup, andObjectTag: settled, no design question remains. Each of these is read through an endpoint scoped to a single competency's criteria tree or a single object's tags (#681 for the first two;ObjectTagView.get_queryset, which only ever returns tags for one object at a time, for the third). At that scope, a plain per-rowExists()annotation, exactly as the original proposal described, is implementation detail on each endpoint rather than a design decision. No standalone design doc needed for these three.TagandTaxonomy/CompetencyTaxonomy: still the open question, being worked out on #661. These are read throughTaxonomyTagsView, a much higher-traffic endpoint used throughout Studio's tagging UI, not just this feature. jesperhodge's original cost concern (comment, 2026-07-13) — that adding a per-row JOIN to every read of a high-traffic list is worth avoiding — still applies here, which is why this pair needs a different answer than the other three:deletablefield. This was the original response to Jesper's concern (issue comment, 2026-07-15) and became ADR-0005 / PR #661: a boolean set synchronously at learner-status write time, flipping one-directionally fromTruetoFalse. kdmccormick and jesperhodge both came out against maintaining a bespokedeletablerow (PR docs: add ADR for persisted competency deletability signal #661 comments, 2026-08-13 and 2026-08-18), and mgwozdz has proposed closing PR docs: add ADR for persisted competency deletability signal #661 as a result once this is settled.ON DELETE RESTRICT. bradenmacdonald proposed this on PR docs: add ADR for persisted competency deletability signal #661 (2026-07-20): attempt the real delete inside a transaction, roll it back, and let the FK constraint'sRestrictedErrorreport what's blocking it. This depends onTag/Taxonomy's foreign keys carrying a real DB constraint, which was in question at the time because Competency Status Storage ADR #657 (still open) proposed moving learner-status tables onto a separate DB alias with logical-only FKs. That dependency has since resolved: jesperhodge confirmed (PR docs: add ADR for persisted competency deletability signal #661, 2026-07-22/23) that only the...Historytable moves to a separate alias, and the active status tables keep normal foreign keys, so the constraint would in fact be there to trial against. Even so, mgwozdz's response (PR docs: add ADR for persisted competency deletability signal #661, 2026-08-16) is that this doesn't serve the same purpose as the GET-side signal: a dry-run only tells you the answer at the moment you attempt to delete, so it can produce a better error message but it cannot let Studio disable the delete control before the user opens that dialog in the first place, which is what thisdeletablefield is for.usage_count.TaxonomyTagsViewalready computes something analogous viaadd_usage_counts()(openedx_tagging/api.py:529), whose own docstring calls a per-row annotation "a very expensive annotation" and instead runs one query for the whole page of tags, then attaches each tag's count in Python before the response goes out. Applying the same technique todeletable(one bulk existence-check query per page, attached to each row in Python) avoids the per-row JOIN Jesper flagged, without needing a persisted field or any write-side bookkeeping.deletableis a plain existence check with no counting or dedup, so it should cost less thanusage_countalready does on this same endpoint today. As of 2026-08-16, kdmccormick's summary confirmed no objection to this direction and jesperhodge confirmed no objections (2026-08-18).Whether to show a disabled delete action or an enabled one with a warning dialog is still nominally a UX call, but mgwozdz stated a preference on PR #661 (2026-08-16) for disabling with an explanation over letting someone click through to a failed attempt, given that ADR-0002 blocks the delete outright rather than allowing an override. Not yet a final decision.
Proposed approach, part 2: enforce it on write endpoints
CompetencyCriteriaandCompetencyCriteriaGroup: settled and shipped differently than originally proposed. #674 and #675 spec the actual DELETE behavior: check for a referencingStudentCompetencyCriteriaStatus/StudentCompetencyCriteriaGroupStatusrow; if none exists, hard-delete; if one exists, setarchived=Trueinstead of deleting. This replaces the original proposal's "return409 Conflict, naming the blocking record" for these two models — the record is archived instead of the request being rejected.Tag/Taxonomy: recommend enforcing the delete block at the database level rather than with an application-layer check, reversing the original proposal for this pair. A foreign key can be configured to automatically block a delete when other rows still reference it (on_delete=RESTRICT), raising an error the view can catch and return, instead of the view checking first. The original proposal ruled this out because #657 was going to move the learner-status tables to a separate database without a real constraint to enforce; #657 has since resolved to keep those tables on normal foreign keys (only the...Historytable moves), so the constraint is available again. We recommend adopting theRestrictedErroridea from PR #661 (2026-07-20) for the real DELETE call itself, not just as an on-demand check. This differs fromCompetencyCriteria/CompetencyCriteriaGroup, which run their own manual existence check instead because those will be archived instead of deleted in this case per ADR 0002 (#674/#675).Edit: unchanged from the original proposal, no backend block needed.
Out of scope
Unchanged for
CompetencyRuleProfileand the archiving-workflow line. The blanket "no DB-level FK constraint" line no longer applies cleanly toTag/Taxonomyper the open point above; it still holds forCompetencyCriteria/CompetencyCriteriaGroup, whose enforcement mechanism (#674/#675) is application-layer archive-on-conflict, not a DB constraint.Appendix: how orphaned tags already degrade safely
Unchanged from the original proposal.
Appendix: PUT vs PATCH for the new endpoints
Unchanged from the original proposal.
mgwozdz-unicon commentedon Aug 20, 2026
@jmakowski1123 I'd like to nail down the precise product requirements for our Competency Guardrails Epic before settling on a specific technical approach here. Could you please review the following questions and options and let me know what you think is best? cc @adolfo @ormsbee @thelmick-unicon
Competency data guardrails: questions for Product
Once a learner has been credited with mastering a competency, we want to make sure an admin can't accidentally destroy the tag, rule, or grouping behind that record. For each record type below there are two questions: what happens to the record itself, and what the admin sees when they try to delete it, from the specific page where that removal happens.
"Scope" marks whether an option was already part of our original ticket list. "Development Effort" is a rough relative sizing (Lowest/Low/Medium/Medium-High/Highest).
Already decided (Context only)
Taxonomy Editing page
Tag
Parity Note: deleting a tag today already shows a confirmation dialog.
Q1. Once a learner has been credited using a tag, what should happen if someone tries to delete it?
Q2. Depending on your Q1 answer, what should the admin see?
Taxonomy
Parity Note: deleting a taxonomy today already shows a confirmation dialog too, same as deleting a tag.
Q3. Once a learner has been credited using any tag within a taxonomy, what should happen if someone tries to delete the whole taxonomy?
Q4. Depending on your Q3 answer, what should the admin see?
Competency Management page
Competency Criterion (a single mastery rule)
Draft ticket: #674 (not yet built)
Q5. Once a learner has been credited using this rule, what should happen if someone tries to delete it?
Q6. Depending on your Q5 answer, what should the admin see?
Competency Criteria Group (a grouping of rules)
Draft ticket: #675 (not yet built)
Q7. Once a learner has been credited using this grouping, or anything nested inside it, what should happen if someone tries to delete it?
Q8. Depending on your Q7 answer, what should the admin see?
Object Tag associations
(For CBE, this is the link that connects a subsection to a competency tag, but this is also used by plain taxonomy tags to link any content to a plain tag.)
Bigger question first: an association created via the Competency Management page could, in principle, also show up on the Course Outline page or the Libraries page. But those two pages show tags generically today, with no competency context — an admin there has no way to tell a competency tag apart from an ordinary one.
Q9. What should happen on the Course Outline and Libraries pages?
(If you pick the third option, Q10/Q11 below apply on Course Outline and Libraries too, using the same answer. If you pick either of the first two, Q10/Q11 below apply only to the Competency Management page.)
Q10. Once a learner has been credited using a tagged association, what should happen if someone tries to delete it?
Q11. Depending on your Q10 answer, what should the admin see?
Graded assignments already used in mastery calculations
This one is a proposal of mine that still needs your sign-off, not something engineering has already agreed to. My proposal: leave today's behavior as-is, no new warning when editing or deleting a graded assignment a learner has already been credited against. This matches how the platform already treats graded assignments today (parity), and avoids new engineering work here.
Q12. Do you approve leaving this as-is for the MVP, or do you want a new warning added?
jmakowski1123 commentedon Aug 20, 2026
If we went with the "never delete it options" here, is there a way to hide it from the end user in the UI? The assumption here is that an admin doesn't want to use (or have their teams use) that tag going forward. So it were hidden from the UI, but saved on the backend for learners already in progress or who have completed it, is that the best outcome?
28 remaining items
jesperhodge commentedon Sep 2, 2026
Hi, Claude here, coming from a conversation with @jesperhodge.
@mgwozdz-unicon @ormsbee: while bringing #613's subtasks in line with its latest revision, I hit a case
where the
on_deletevalues #641 sets and the approved design in this issue disagree. Flagging itbefore either gets built. I have left #641 and #642 declaring
PROTECTas a fail-closed value andrecorded the question as open in both, rather than resolving it myself.
Explaining it from the ground up, since the interaction is easy to miss.
The mechanic
Every Django foreign key has to declare what happens when the row it points at is deleted. Two values
matter here:
CASCADEmeans "delete me too."PROTECTmeans "refuse the whole deletion and raiseProtectedError."The three players
Tagis a competency, say "Writing Poetry." It lives inopenedx_tagging, a generic library usedacross Open edX that knows nothing about CBE.
CompetencyCriteriaGroupis a CBE row saying "to demonstrate Writing Poetry, pass assignment 1 ORassignment 2." It holds a foreign key pointing at that
Tag.The reference direction is what matters: the CBE row points at the
Tag, and theTagknows nothingabout the CBE row.
What this issue decided
When an admin deletes a competency tag, the outcome depends on whether any learner has earned mastery
against it. The approved table says: archive it if someone has, and hard-delete it, unchanged from
today's behavior, if nobody has.
The load-bearing part is the enforcement mechanism. @ormsbee rejected the original proposal, in which
openedx_taggingwould ask CBE "is this in use?", in favor of a push model: CBE stamps adeletion_lockedflag onto theObjectTagrow when a learner earns something, and at delete time thetagging code reads only its own flag. As the design puts it,
openedx_taggingnever calls into CBE.The hole
Walk the second branch. An author has built a criteria tree for "Writing Poetry," but no learner has
been graded yet. That is a completely ordinary state, not an edge case: #665 creates the root,
course-level and leaf
CompetencyCriteriaGrouprows at authoring time, long before any grade exists.Because nobody has earned anything,
deletion_lockedis false, so the tagging code proceeds to a plainhard delete. But a
CompetencyCriteriaGrouprow is still pointing at that tag. WithPROTECTon thatforeign key, Django refuses and raises
ProtectedError. The admin gets a failure where this issuepromised a successful delete, and nothing cleans up the CBE row first, because the tagging code is
deliberately forbidden from knowing the CBE row exists.
Three things I checked rather than assumed:
taxonomy.delete()insrc/openedx_tagging/rest_api/v1/views.py, andTaxonomy.delete_tags()insrc/openedx_tagging/models/base.py.Tag.taxonomyis alreadyon_delete=CASCADE, so deleting a taxonomy collects every tag beneath itand each one hits the same wall. That makes taxonomy deletion the likeliest instance rather than an
outlier, since a
CompetencyTaxonomyis precisely the taxonomy whose tags carry criteria.PROTECTraises even when the referencing row is in the same delete batch. OnlyRESTRICTexempts that case.
Two foreign keys cross this boundary:
CompetencyCriteriaGroup.tagtooel_tagging_tag, andCompetencyCriterion.object_tagtooel_tagging_objecttag.StudentCompetencyStatus.tagis notaffected, because once learner status exists this issue archives the tag rather than deleting it, and
archiving is a field update that never reaches
on_delete.The options as I see them
PROTECTand have the tagging-side delete paths resolve referencing CBE rows before hard-deletingopenedx_taggingimporting CBE.CASCADECompetencyCriterion.groupandCompetencyCriteriaGroup.parentto beCASCADE, otherwise the collector walks down and trips theirPROTECTinstead, so it is closer to a full revert to the earlier database-level design than a two-line change.deletion_lockedtrue whenever criteria exist, but it changes what the flag means and would block deletes this issue says should succeed.I do not think this reopens the design. The push-based lock is right for the case it was chosen for,
which is protecting earned mastery. What is missing is the unlocked branch, where CBE rows exist and no
mastery has been earned yet.
Two smaller things from the same pass
lines. This issue asked for both to name [BE] Add archived field to Competency Criteria & Group models #716 instead.
such and recorded the work this issue identified that no ticket owns yet, most importantly the
openedx_taggingticket addingarchivedanddeletion_lockedplus the lock functions, which [BE] Add archived field to Competency Criteria & Group models #716explicitly does not cover.
docs: drop superseded openedx#799 references from criteria models
test: cover on_delete behavior for every CBE criteria foreign key
ormsbee commentedon Sep 2, 2026
@jesperhodge: When you paste in a Claude-generated analysis with options, please note which option you support and why.
mgwozdz-unicon commentedon Sep 2, 2026
@jesperhodge @ormsbee
Thanks for digging into this. After walking through the tradeoff with Claude, I think the two crossing foreign keys,
CompetencyCriteriaGroup.tagandCompetencyCriterion.object_tag, should becomeCASCADErather than stayingPROTECT.The case that matters here is a tag with an authored-but-ungraded criteria tree: no learner has earned anything yet, so the delete should succeed, but it currently fails because the criteria rows still point at the tag. Keeping
PROTECTand having the tagging-side delete paths clean those rows up first (your option 1) meansopenedx_tagginghas to call into CBE-specific models to do that cleanup, which breaks the "never calls into CBE" boundary this design was built around. I think adhering to the dependency direction boundary of the design takes priority over addressing what feels like a bunch of edge cases to me, so these should becomeCASCADE. Here's that tradeoff laid out:PROTECT(current)CASCADEProtectedError), nothing is lostopenedx_taggingcalling into CBE)One case isn't in the table because it doesn't actually depend on this choice: a tag with a live criteria tree where a learner has also earned status against it.
StudentCompetencyStatus.tag, a separate foreign key from the learner's status row to the tag, already staysPROTECTregardless of what we pick here, and Django aborts the whole deletion if it finds even onePROTECTanywhere in what it collects, so that case is protected either way.One thing worth flagging before this lands:
CompetencyCriterion.groupandCompetencyCriteriaGroup.parentare alsoPROTECT, so the collector would still hit those on its way down and abort partway through a tag deletion. Those would need to becomeCASCADEtoo for a tag delete to actually cascade cleanly through the whole tree, and that's a bigger change than just the tag FKs: it's the same backstop #674/#675 relies on to keep a group deletion from bypassing their student-status check, so it removes that protection for direct group deletion too, not only for deletion that starts from the tag. I'm applying the same edge-case reasoning to that as I am to the tag FKs, but flagging it since it's a second place this decision now reaches, not a detail of the first one.fix: set the learner foreign key to CASCADE and cover deletes with tests
feat: cascade criteria deletes from tag, group and objecttag
refactor: align criteria models with the 2026-09-03 revision of opene…
feat: add competency criteria models for CBE authoring layer
docs: drop superseded openedx#799 references from criteria models
test: cover on_delete behavior for every CBE criteria foreign key
feat: cascade criteria deletes from tag, group and objecttag
refactor: align criteria models with the 2026-09-03 revision of opene…