Jesperhodge/cbe 641 01 adr - #10
Open
jesperhodge wants to merge 4 commits into
Open
jesperhodge wants to merge 4 commits into
jesperhodge wants to merge 4 commits into
Conversation
Decision 3 described scope_code as a never-null, database-generated column. The implementation needs a plain column that goes null while a profile is archived, because nulling it is what frees that scope for a replacement: SQL never treats two NULLs as equal, so any number of archived rows may share a scope while exactly one live row holds it. The alternative, a conditional unique index over the three nullable scope columns, is what Rejected Alternative 6 already ruled out, because MySQL does not support partial indexes and Django silently skips creating one there. Says explicitly that the three scope columns are never cleared, so archiving loses no information and an archived profile stays restorable. Nulling scope_code releases its claim on the unique slot, not the record of the scope. Refs openedx#641 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Decision 7 described the archive-versus-hard-delete rule and left every foreign key's on_delete to be inferred from it. State them instead, so the models can be written from the ADR rather than the ADR from the models. The criteria tree cascades, which is what carries a delete down to the learner status tables where the guarantee is enforced. CompetencyCriterion.rule_profile is RESTRICT rather than PROTECT: both refuse a direct profile delete, but only RESTRICT lets a scope owner's deletion carry the profile away, because it ignores referencing rows that the same operation is already deleting. That removes the taxonomy-delete failure PROTECT would have caused. A narrower course-scoped case survives, since a criterion's profile assignment is independent of its tree's course scope, and it is recorded rather than fixed. The learner status tables protect the node they track, cascade from the learner so this library cannot veto User.delete() platform-wide, and protect the seeded status lookup. Foreign keys into tables this decision does not own are listed as inherited, so the whole delete path is legible in one place. Also note that CompetencyMasteryStatuses has no delete constraint of its own: the referencing PROTECTs cover a status in use, and nothing covers an unused one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Jesper Hodge <19345795+jesperhodge@users.noreply.github.com>
jesperhodge
added this pull request to stack #11
September 11, 2026 13:17
jesperhodge
removed this pull request from stack #11
September 11, 2026 14:29
jesperhodge
added this pull request to stack #12
September 11, 2026 14:33
jesperhodge
removed this pull request from stack #12
September 11, 2026 14:34
jesperhodge
added this pull request to stack #13
September 11, 2026 14:34
mgwozdz-unicon
approved these changes
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements openedx#641 .
This is a stacked PR.
Replaces openedx#809.
Reviews: from bottom to top of the stack.
Merge: The plan is to review and merge each piece of the stack into the base branch in this fork, then open a PR to merge the base branch into the openedx upstream main.