[tbain]

Description

Adds archived = models.BooleanField(default=False) to Taxonomy, Tag, and ObjectTag, and
deletion_locked = models.BooleanField(default=False) to ObjectTag only, in one schema-only
migration (oel_tagging 0023_archived_and_deletion_locked).

Today these three models can only be hard-deleted, which would destroy the learner mastery history
that points at them. archived records that a row has been retired: hidden from active use, but
kept. deletion_locked records that something outside the tagging app depends on an association,
so a future delete must archive it instead of removing it. Tag and Taxonomy have no lock field
of their own; #777 derives whether one is locked from the associations beneath it.

ObjectTag.copy(), which ObjectTag.cast() uses, also copies both new fields, so saving a cast
instance does not reset them to False on the original row.

Impacted roles: Developer only.

Supporting information

• Closes [BE] Add an archived field to Tag, Taxonomy, and ObjectTag, and a deletion_locked field to ObjectTag #776
• Design: the archive-versus-delete approach approved on [Arch] Implementation approach for competency data delete/edit guardrails #655, and
  docs/openedx_learning/decisions/0003-competency-criteria-versioning.rst, Decision 3
• Mirrors [BE] Add archived field to Competency Criteria & Group models #716, which adds archived to CompetencyCriteriaGroup and CompetencyCriterion
• Consumed by [BE] Add deletion-lock functions to openedx_tagging #777, [BE] Exclude archived tagging records from read paths and block edits to them #778, [BE] Branch tag association deletes between archiving and hard delete #779, [BE] Branch tag and taxonomy deletes between archiving and hard delete #780, and [BE] Fan out the content-tag-changed event when a taxonomy is archived #781

Testing instructions

1. Apply the migration: python manage.py migrate oel_tagging
2. Confirm no migration is missing: python manage.py makemigrations --check --dry-run
3. Manual check in a Django shell: create a Taxonomy, a Tag, and an ObjectTag, and confirm
   archived and deletion_locked are False. Set deletion_locked=True on the ObjectTag,
   save it, reload it, and confirm archived is still False.

Other information

• PII: all three models are already listed as no_pii in .annotation_safe_list.yml, so the new
  fields need no new annotations.
• The migration is reversible: it contains only four AddField operations.
• AI usage: this PR was drafted with Claude (Anthropic) in Cowork. The developer reviewed the
  change and ran all tests and checks locally.

Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added support for archiving tags, taxonomies, and their associations.
  • Associations can now be locked against deletion; locked associations are retained rather than deleted.
  • Existing records remain unarchived and unlocked by default.

[openedx-webhooks]

Thanks for the pull request, @tbain!

This repository is currently maintained by @axim-engineering.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[mgwozdz-unicon]

Claude and I collaborated on this code review. Claude wrote this comment:

Thanks for this, the model and migration changes look good. The ObjectTag.copy() fix is a good catch, and I confirmed that test_cast_copies_archived_and_deletion_locked fails when those two copy() lines are reverted, so that test is doing real work. I also ran the tagging model and migration tests, makemigrations --check, pylint, pycodestyle, isort, mypy, and lint-imports against the PR head, and everything passes.

My requests below are all about the tests. Most of the new tests check things that the schema already guarantees, so they can't fail.

1. Make the test_migrations.py module docstring clearer (line 2). "Run against existing data" is vague. What the tests actually do is create rows on the schema from before a migration, run the real migration, and then check those rows. Something like "Tests that run openedx_tagging migrations over rows created on the earlier schema." would say that directly.

2. In test_archived_and_deletion_locked_default_to_false_for_existing_rows, remove lines 140-147, and with them the line 121 comment and the explicit _export_id/_value arguments on line 123. Lines 140-147 check that the migration left every other field unchanged. Migration 0023 only adds four columns with a default, and adding a column can't change the values in other columns, so these assertions have no way to fail. Lines 146-147 in particular only check the test's own setup. I tried removing the _export_id/_value arguments from line 123: the only failure was assert '' == 'flags_test' on line 146, which comes from the test's setup, not from the migration. With lines 140-147, the comment, and those two arguments all removed, the test still passes. It still checks what matters, lines 135-138: pre-existing rows come out unarchived and unlocked.

3. Remove test_archived_defaults_to_false (test_models.py, line 158) and test_archived_and_deletion_locked_default_to_false (test_models.py, line 611). Both tests check that a newly created row has the default=False declared on the field, so they test the schema itself rather than any behavior. The defaults for pre-existing rows, which are the case #776 actually asks about, are already covered by the migration test.

4. Remove test_setting_one_flag_leaves_the_other_and_identity_unchanged (test_models.py, line 623). archived and deletion_locked are separate columns, so saving one can't change the other, and neither can change object_id, taxonomy_id, tag_id, or the cached fields. I know #776's acceptance criteria list "can be set independently ... with no other field affected", but not every acceptance criterion needs a unit test, and the schema already guarantees this one.

5. Nit, not required: the docstrings on the new tests in test_models.py could be dropped. Pylint doesn't require docstrings on tests here, and each one mostly restates the test name. Keeping them is fine too, since the file has both styles today. This applies to whichever of those tests remain after items 3 and 4, which would be only test_cast_copies_archived_and_deletion_locked.

[tbain]

Updated to address all the code review comments, handled with Claude 5.5 Opus. I also updated Step 3 of the testing instructions since we changed/removed some of the tests.

[mgwozdz-unicon]

Thank you! It seems like the CI tests are failing for reasons unrelated to this PR. I'm not sure how to re-trigger them other than possibly pushing a no-op commit unless someone else with access to re-trigger kicks it off.

[jesperhodge]

Hi @tbain , great work! There's only one nit suggestion (non blocking) for changing a test name.

I'd like to make sure that this PR is all set for community review. To this end, I went over the description and checked whether it is accurate and understandable.

• I found some points that are not relevant to the PR reviewer, so I'd recommend cutting them so that the reviewer can focus on what's important and not lose time.
• The "Choices that differ" section is a section that AI likes to add and that sometimes is very helpful, but in this case the points were either outdated/inaccurate like the "help_text_wording", or unnecessary / not real deviations from the ticket. That's pretty misleading. I would only keep the "copy()" mention and since that's not a deviation from the ticket but an edge case move it to a better section.

I came up with a cleaned up version of the PR description, which I recommend to use:

"

Description

Adds archived = models.BooleanField(default=False) to Taxonomy, Tag, and ObjectTag, and
deletion_locked = models.BooleanField(default=False) to ObjectTag only, in one schema-only
migration (oel_tagging 0023_archived_and_deletion_locked).

Today these three models can only be hard-deleted, which would destroy the learner mastery history
that points at them. archived records that a row has been retired: hidden from active use, but
kept. deletion_locked records that something outside the tagging app depends on an association,
so a future delete must archive it instead of removing it. Tag and Taxonomy have no lock field
of their own; #777 derives whether one is locked from the associations beneath it.

ObjectTag.copy(), which ObjectTag.cast() uses, also copies both new fields, so saving a cast
instance does not reset them to False on the original row.

Impacted roles: Developer only.

Supporting information

• Closes #776
• Design: the archive-versus-delete approach approved on #655, and
  docs/openedx_learning/decisions/0003-competency-criteria-versioning.rst, Decision 3
• Mirrors #716, which adds archived to CompetencyCriteriaGroup and CompetencyCriterion
• Consumed by #777, #778, #779, #780, and #781

Testing instructions

1. Apply the migration: python manage.py migrate oel_tagging
2. Confirm no migration is missing: python manage.py makemigrations --check --dry-run
3. Manual check in a Django shell: create a Taxonomy, a Tag, and an ObjectTag, and confirm
   archived and deletion_locked are False. Set deletion_locked=True on the ObjectTag,
   save it, reload it, and confirm archived is still False.

Other information

• PII: all three models are already listed as no_pii in .annotation_safe_list.yml, so the new
  fields need no new annotations.
• The migration is reversible: it contains only four AddField operations.
• AI usage: this PR was drafted with Claude (Anthropic) in Cowork. The developer reviewed the
  change and ran all tests and checks locally.

Generated with Claude Code
"

[jesperhodge]

Suggested change

	def test_cast_copies_archived_and_deletion_locked(self) -> None:
	def test_copying_a_tag_via_casting_copies_field_values(self) -> None:
	"""When copying a tag via `.cast()` / `.copy()`, fields are not reset to their defaults."""

[tbain]

@jesperhodge Updated the test name with your suggestion, as well as the PR description - thank you!

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The change adds default-false archive fields to Tag, Taxonomy, and ObjectTag, plus a default-false deletion-lock field to ObjectTag. A migration adds the fields, and ObjectTag.copy() preserves the two new association values.

Changes: Tagging model fields

Layer / File(s)	Summary
Add tagging fields and migrate existing rows src/openedx_tagging/models/base.py, src/openedx_tagging/migrations/0023_archived_and_deletion_locked.py, tests/openedx_tagging/test_migrations.py	Tag, Taxonomy, and ObjectTag gain default-false archived fields. ObjectTag also gains a default-false deletion_locked field. The migration test checks that existing rows have these fields set to False.
Preserve fields when copying ObjectTag src/openedx_tagging/models/base.py, tests/openedx_tagging/test_models.py	ObjectTag.copy() copies archived and deletion_locked. A test checks that casting an ObjectTag preserves both values.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: ufedaseyeuconsultant

Merge Risk: Minimal · up to 2bab7

This change adds default-false archive and deletion-lock fields to the tagging models and preserves them when an association is copied. Existing rows get safe defaults and nothing reads the fields yet, so no behavior change is expected. No merge-blocking risk is evident.

Security architecture risk: Minimal · up to 2bab7

The change adds metadata without activating new deletion rules or weakening existing access controls. Copies preserve the new values. Archival and deletion protection are not enforced by these fields alone and remain follow-up work.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The persistent change covers tagging records and object-tag associations. The migration adds metadata without changing primary keys, foreign keys, or existing association constraints, and copy preserves the same row identity.

Trust Boundaries and Controls

• inferred — Taxonomy and Tag use admin registrations without explicit field restrictions, so their archived field becomes available through existing admin forms. ObjectTag's explicit admin field list excludes both new flags. This extends metadata editing within the existing administrative surface rather than adding a public flag-update endpoint.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "feat: Adding archived and deletion-locked flags to tagging models" matches the main change. The PR adds archived fields to Taxonomy, Tag, and ObjectTag, adds deletion_locked to ObjectTag, a…
Linked Issues check		Directly linked issue #776 has coding requirements only. Verified at head: src/openedx_tagging/models/base.py declares archived = models.BooleanField(default=False) on Tag and Taxonomy, with h…
Out of Scope Changes check		The whole-PR changes (merge base 7e150e7 to head 2bab7c2) consist of the four new model fields and the ObjectTag.copy() update in models/base.py, the single schema migration 0023, and tests in `…

Full details: Title check

Explanation

The title "feat: Adding archived and deletion-locked flags to tagging models" matches the main change. The PR adds archived fields to Taxonomy, Tag, and ObjectTag, adds deletion_locked to ObjectTag, and includes the migration 0023 and the copy() update. The title is specific and concise, and it names the primary change for a reviewer scanning history.

Full details: Linked Issues check

Explanation

Directly linked issue #776 has coding requirements only. Verified at head: src/openedx_tagging/models/base.py declares archived = models.BooleanField(default=False) on Tag and Taxonomy, with help text. Migration 0023_archived_and_deletion_locked.py (depends on 0022_tag_external_id_not_null) contains exactly four AddField operations, all BooleanField(default=False): objecttag.archived, objecttag.deletion_locked, tag.archived, taxonomy.archived. No data migration, backfill, archived_at, or locked_at appears. deletion_locked is on ObjectTag only. The migration test in tests/openedx_tagging/test_migrations.py covers existing rows reading back as unarchived and unlocked. The model test in test_models.py covers copy()/cast() preserving both flags. Remaining items are not fully verified: the ObjectTag field declarations and copy() body in base.py were truncated at the 16000-byte read limit, and the base.py diff could not be read. The ObjectTag declarations are confirmed through the migration only. Help text match with #716 and make pii_check result could not be checked, because #716 code and the PII check output are not in the evidence. The index guidance in #776 is advisory ('likely warranted') and is not an acceptance criterion; no index was added, which is consistent with the criteria.

Full details: Out of Scope Changes check

Explanation

The whole-PR changes (merge base 7e150e7 to head 2bab7c2) consist of the four new model fields and the ObjectTag.copy() update in models/base.py, the single schema migration 0023, and tests in test_migrations.py and test_models.py. All of these implement #776 or support it. The copy() change keeps the new flags from resetting during cast(), which the PR description states. The tests validate the migration and the new defaults. No read path, write path, filter, delete branch, lock API, or REST change was identified in the reviewed evidence, which matches #776's exclusions. Unverified: the base.py diff and the ObjectTag/copy() source were not fully readable, so the absence of other edits in base.py rests on the PR summary.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: a5e81215-0b6f-4610-affd-dd7946ad34df

Commits

Reviewing files that changed from the base of the PR and between 7e150e7 and 2bab7c2.

Files selected for processing: 4

• src/openedx_tagging/migrations/0023_archived_and_deletion_locked.py
• src/openedx_tagging/models/base.py
• tests/openedx_tagging/test_migrations.py
• tests/openedx_tagging/test_models.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.