Repository navigation
[Placeholder] Competency Criteria Deletions #799
Description
Status: superseded, closed
This issue owned nothing by the time #655 resolved. It existed to hold #613's delete-protection
criteria while #655 decided how to implement them, and to replace the provisional on_delete=PROTECT
values that #641 and #642 were told to declare in the meantime. #655 is now closed with an approved
design that answers the question differently, so none of the four criteria below survive. They are
kept for history rather than as work.
Where each piece went:
| Criterion below | Where it lands now |
|---|---|
on_delete=PROTECT on Student*Status foreign keys |
#641 and #642 declare it directly. #641 also records the one question still open about the two foreign keys crossing into openedx_tagging. |
on_delete=CASCADE on definition-to-definition foreign keys |
Dropped. #655's appendix puts enforcement at the application layer rather than in database-level cascade behavior. |
| Transitive delete tests | #641 and #642 each now carry a criterion requiring test coverage for whatever on_delete values they land. The product behavior is covered by #674 and #675 through their own endpoints. |
CompetencyRuleProfile is archive-only, never hard-deleted |
#655 resolves this differently: the model gets no DELETE endpoint at all, and only the single system-default row exists in MVP, so there is no archive-versus-delete case for it. Whoever builds the profile endpoints owns not building a DELETE. |
Work #655 identified that no issue owns yet. Recorded here so it is not lost, but it belongs in
freshly filed tickets rather than in this one, because it changes openedx_tagging rather than CBE:
- An
openedx_taggingticket addingarchivedtoTag,TaxonomyandObjectTag,deletion_locked
toObjectTagonly, and the three lock functions CBE calls. [Arch] Implementation approach for competency data delete/edit guardrails #655 states explicitly that [BE] Add archived field to Competency Criteria & Group models #716 does
not cover this. - The CBE call site that locks an
ObjectTagwhen aStudentCompetencyCriteriaStatusrow is written,
with the read-before-write guard so a sharedObjectTagrow is not write-locked inside every grade
transaction. - The tagging-side delete-path work: the per-tag branch in
Taxonomy.delete_tags(), a
perform_destroyonTaxonomyView, the archive branch intag_object(), and thearchived
exclusions across the read paths. - An
openedx-platformticket extending ADR-0013'staxonomy_typecomputation to the per-object tag
endpoint, plus the frontend gating of the delete and Add Tag controls. - The ADR follow-up PR @mgwozdz-unicon committed to on [Arch] Implementation approach for competency data delete/edit guardrails #655, covering the nine ADR edits raised there.
Parent: #613.
Historical content
Everything below is the issue as it stood before #655 closed. Kept for the record.
Why this is separate
ADR-0002 Decision 7 and ADR-0003 Decision 4 already decide what should happen when someone tries to
delete or edit competency data after a learner has mastery status against it. #655 is deciding how
that gets implemented: how a "can this be deleted" signal is computed and surfaced on the GET endpoints
without N+1 queries, and where the write-path validation lives so the rule is enforced rather than only
hinted at in the UI.
Those answers change what the model layer has to look like, so these criteria were pulled out of #613's
model tickets rather than guessed at. #641 and #642 build the tables with no delete behavior of their
own.
What #641 and #642 leave behind
Django requires an on_delete value on every ForeignKey, so #641 and #642 cannot leave the question
open. Both declare every new foreign key as on_delete=PROTECT with a TODO(#799) comment. That is a
fail-closed placeholder, not a considered per-foreign-key choice: nothing can be hard-deleted until this
issue sets the real values. Neither ticket ships an authoring API, so nothing performs deletes meanwhile.
So the first job here, whatever #655 decides, is to replace every on_delete=PROTECT that carries a
TODO(#799) comment, and to leave none behind.
Acceptance criteria (from #613)
Copied unchanged from #613's "Deletions" section. Expect these to change once #655 is resolved.
- Delete protection is implemented via
on_delete=PROTECTon FKs fromStudent*Statusmodels to definition models; hard deletion is blocked at the ORM layer without requiring instance-leveldelete()overrides, sinceQuerySet.delete()bypasses those. - Every foreign key from one definition table to another uses
on_delete=CASCADE: criterion to group, group to parent group, group to tag, and criterion to objecttag. The one exception is the criterion's foreign key toCompetencyRuleProfile, which usesPROTECT, because a rule profile is never hard-deleted at all. These values are what make ADR-0002 Decision 7's delete protection work. That protection is conditional, not a blanket ban: deleting a tag has to succeed when no learner has a status under it, and fail when one does. ThePROTECTthat produces the failure lives on the foreign keys from the threeStudent*Statustables, and two of those point at rows one and two levels below the tag. Django reaches them only by walking down the tree from the tag, and it only walks down a foreign key markedCASCADE. Change any link in that chain toSET_NULLand deleting the tag succeeds while learner statuses still exist. - The transitive cases are tested, not only the direct ones: deleting an
oel_tagging_tagwith a learner status row anywhere beneath it raisesProtectedError, and deleting one with no status rows beneath it succeeds and cascades. -
CompetencyRuleProfileis never hard-deleted, even before any learner status exists. Retirement is archive-only through itsarchivedcolumn, per ADR-0002 Decision 7's exception, because a profile is shared across many criteria rather than tied to one.
Out of scope
The archived column itself. #641 creates it as one of CompetencyRuleProfile's columns, per ADR-0002
Decision 3. What this issue adds is the rule that retirement goes through it.
The edit guardrails from ADR-0003 Decision 4, meaning the Studio warning and confirmation when someone
edits criteria that learners already have status against. #655 covers edits as well as deletes, but that
warning is API and UI work, not model work.
Depends on
#655 for the approach. #641 and #642 for the tables and the foreign keys to change.
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsDone
Activity
jesperhodge commentedon Sep 1, 2026
Hi, Claude here.
While implementing #642's learner status models I hit a gap in this issue's scope that is
worth recording before anyone starts work here.
This issue's acceptance criteria scope delete protection to "FKs from
Student*Statusmodels to definition models". Each of the three
Student*Statusmodels carries two furtherforeign keys that are not to definition models, and so are covered by no criterion here:
user_id, pointing atsettings.AUTH_USER_MODEL.status_id, pointing at theCompetencyMasteryStatuseslookup table.Both of those are declared
on_delete=PROTECTwith aTODO(#799)comment, because #641 and#642 declare every new foreign key that way as a fail-closed placeholder. This issue's
stated first job is to replace every
PROTECTcarrying aTODO(#799)comment and to leavenone behind, so both of those foreign keys land in scope by that sentence while no
acceptance criterion mentions them.
That matters most for the user foreign key. Taken literally,
PROTECTthere means deleting auser row fails once that user has any competency status. That may well be the answer we want,
since Open edX retires learners rather than deleting rows, but it is currently an accident of
the placeholder rather than a decision. The existing precedent in openedx-core is different
again: every user foreign key in
openedx_contentisSET_NULLwith a nullable column, butthose are
created_byandchanged_byattribution fields, where a null is meaningful. On alearner status row the user is identifying rather than attributive, and
SET_NULLwouldbreak the
(user_id, node_id)uniqueness that the whole in-place-update design rests on. Sothe realistic candidates are
CASCADEandPROTECT, and the choice needs stating.Suggested addition to this issue, whenever #655 unblocks it: one criterion covering the
user_idandstatus_idforeign keys on all threeStudent*Statusmodels, either setting avalue or recording explicitly that they keep
PROTECTand why.jesperhodge commentedon Sep 2, 2026
Hi, Claude here, coming from a conversation with @jesperhodge.
Closing this as superseded. #655 is now resolved with an approved design that answers the
delete-and-edit guardrail question at the application layer, so none of the four criteria this issue
held survive. The body above records where each one went, and lists the work #655 identified that no
ticket owns yet.
Two things worth stating plainly, since this issue's stated first job was to replace every provisional
on_deletevalue:PROTECTdirectly, and noTODO(#799)comment should be attached to any of them. Draft PR feat: add mastery status lookup and the three learner status models #802 currently carries three such comments and
they should come out.
userandstatusforeign keys on the threeStudent*Statusmodels, which I raised on thisissue on 2026-09-01 as covered by no criterion here, are now settled in Mastery status lookup + learner progress models #642:
PROTECTis ruled outfor the user foreign key, the choice is between
CASCADEandDO_NOTHING, and thestatusforeignkey keeps
PROTECTbecause that lookup table is system-owned and never deleted.One question is still open rather than closed, and it is recorded in #641 and raised on #655 instead of
here: with
PROTECTonCompetencyCriteriaGroup.tagandCompetencyCriterion.object_tag, deleting aTagorTaxonomythat an author has built criteria against, but against which no learner holdsmastery, raises
ProtectedErrorwhere #655's approved table says it should hard-delete.docs: drop superseded openedx#799 references from criteria models
fix: set the learner foreign key to CASCADE and cover deletes with tests
feat: add competency criteria models for CBE authoring layer
docs: drop superseded openedx#799 references from criteria models