[Arch] Implementation approach for competency data delete/edit guardrails

[mgwozdz-unicon]

Context

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.

Scope

In scope:

• CompetencyCriteria, CompetencyCriteriaGroup, CompetencyRuleProfile, Tag, Taxonomy/CompetencyTaxonomy, ObjectTag

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.

[mgwozdz-unicon]

Implementation proposal: delete/edit guardrails for competency taxonomy data

What's already decided (not up for debate here, except the open question below)

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	Required when the profile is "in use"

Source: ADR-0002 Decision 7, ADR-0003 Decision 4.

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.

[jesperhodge]

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.

[jesperhodge]

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.

[mgwozdz-unicon]

@jesperhodge, addressed both:

1. 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.
2. 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.