From 09f903529da52218bf03eb5c5d688c31e9cac8ca Mon Sep 17 00:00:00 2001 From: tbain Date: Thu, 8 Oct 2026 13:23:55 -0700 Subject: [PATCH 1/4] feat: Adding archived and deletion-locked flags to tagging models --- .../0023_archived_and_deletion_locked.py | 33 ++++++++++++ src/openedx_tagging/models/base.py | 24 +++++++++ tests/openedx_tagging/test_migrations.py | 49 +++++++++++++++++- tests/openedx_tagging/test_models.py | 50 +++++++++++++++++++ 4 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 src/openedx_tagging/migrations/0023_archived_and_deletion_locked.py diff --git a/src/openedx_tagging/migrations/0023_archived_and_deletion_locked.py b/src/openedx_tagging/migrations/0023_archived_and_deletion_locked.py new file mode 100644 index 000000000..a17445050 --- /dev/null +++ b/src/openedx_tagging/migrations/0023_archived_and_deletion_locked.py @@ -0,0 +1,33 @@ +# Generated by Django 5.2.17 on 2026-10-07 23:12 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('oel_tagging', '0022_tag_external_id_not_null'), + ] + + operations = [ + migrations.AddField( + model_name='objecttag', + name='archived', + field=models.BooleanField(default=False, help_text='Hides this association from active use by default, without removing it.'), + ), + migrations.AddField( + model_name='objecttag', + name='deletion_locked', + field=models.BooleanField(default=False, help_text='Set when something outside the tagging app depends on this association; a delete archives it instead of removing it.'), + ), + migrations.AddField( + model_name='tag', + name='archived', + field=models.BooleanField(default=False, help_text='Hides this tag from active use by default, without removing it.'), + ), + migrations.AddField( + model_name='taxonomy', + name='archived', + field=models.BooleanField(default=False, help_text="Hides this taxonomy from active use by default, without removing it. Unlike disabling it with 'enabled', archiving marks the taxonomy as retired."), + ), + ] diff --git a/src/openedx_tagging/models/base.py b/src/openedx_tagging/models/base.py index f725ec778..e019d4b3c 100644 --- a/src/openedx_tagging/models/base.py +++ b/src/openedx_tagging/models/base.py @@ -84,6 +84,10 @@ class Tag(models.Model): " Set automatically by save(); do not set manually." ), ) + archived = models.BooleanField( + default=False, + help_text=_("Hides this tag from active use by default, without removing it."), + ) class Meta: indexes = [ @@ -311,6 +315,13 @@ class Taxonomy(models.Model): " taxonomy admins will not be permitted to add, edit, or delete its tags." ), ) + archived = models.BooleanField( + default=False, + help_text=_( + "Hides this taxonomy from active use by default, without removing it." + " Unlike disabling it with 'enabled', archiving marks the taxonomy as retired." + ), + ) class Meta: verbose_name_plural = "Taxonomies" @@ -724,6 +735,17 @@ class ObjectTag(models.Model): " If the tag field is set, then tag.value takes precedence over this field." ), ) + archived = models.BooleanField( + default=False, + help_text=_("Hides this association from active use by default, without removing it."), + ) + deletion_locked = models.BooleanField( + default=False, + help_text=_( + "Set when something outside the tagging app depends on this association;" + " a delete archives it instead of removing it." + ), + ) class Meta: indexes = [ @@ -896,6 +918,8 @@ def copy(self, object_tag: ObjectTag) -> Self: self.taxonomy = object_tag.taxonomy self.object_id = object_tag.object_id self.is_copied = object_tag.is_copied + self.archived = object_tag.archived + self.deletion_locked = object_tag.deletion_locked self._value = object_tag._value # pylint: disable=protected-access self._export_id = object_tag._export_id # pylint: disable=protected-access return self diff --git a/tests/openedx_tagging/test_migrations.py b/tests/openedx_tagging/test_migrations.py index ca1c5c906..776fad7f2 100644 --- a/tests/openedx_tagging/test_migrations.py +++ b/tests/openedx_tagging/test_migrations.py @@ -1,5 +1,5 @@ """ -Tests for the 0022_tag_external_id_not_null migration's backfill logic. +Tests for openedx_tagging migrations, run against existing data. Uses the `migrator` pytest fixture from django_test_migrations to run the actual migration against historical (frozen) model states, the same way it will run against @@ -12,6 +12,9 @@ MIGRATE_FROM = ("oel_tagging", "0021_remove_system_defined_add_read_only") MIGRATE_TO = ("oel_tagging", "0022_tag_external_id_not_null") +MIGRATE_0023_FROM = ("oel_tagging", "0022_tag_external_id_not_null") +MIGRATE_0023_TO = ("oel_tagging", "0023_archived_and_deletion_locked") + @pytest.mark.django_db def test_backfill_generates_missing_external_ids(migrator) -> None: @@ -98,3 +101,47 @@ def test_reverse_migration_is_a_noop_that_keeps_data(migrator) -> None: reverted_tag = RevertedTag.objects.get(pk=tag.pk) assert reverted_tag.external_id == "Rollback Tag" + + +@pytest.mark.django_db +def test_archived_and_deletion_locked_default_to_false_for_existing_rows(migrator) -> None: + """ + Rows that exist before 0023 come out with archived and deletion_locked False, and their + other fields are untouched. + """ + old_state = migrator.apply_initial_migration(MIGRATE_0023_FROM) + Taxonomy = old_state.apps.get_model("oel_tagging", "Taxonomy") + Tag = old_state.apps.get_model("oel_tagging", "Tag") + ObjectTag = old_state.apps.get_model("oel_tagging", "ObjectTag") + + taxonomy = Taxonomy.objects.create(name="Flags Test", export_id="flags_test") + tag = Tag.objects.create( + taxonomy=taxonomy, value="Flags Tag", external_id="flags-tag", depth=0, lineage="Flags Tag\t", + ) + # Historical models don't run ObjectTag.__init__, so the cached fields are set explicitly. + object_tag = ObjectTag.objects.create( + object_id="object:id:1", taxonomy=taxonomy, tag=tag, _export_id="flags_test", _value="Flags Tag", + ) + + new_state = migrator.apply_tested_migration(MIGRATE_0023_TO) + NewTaxonomy = new_state.apps.get_model("oel_tagging", "Taxonomy") + NewTag = new_state.apps.get_model("oel_tagging", "Tag") + NewObjectTag = new_state.apps.get_model("oel_tagging", "ObjectTag") + + new_taxonomy = NewTaxonomy.objects.get(pk=taxonomy.pk) + new_tag = NewTag.objects.get(pk=tag.pk) + new_object_tag = NewObjectTag.objects.get(pk=object_tag.pk) + + assert new_taxonomy.archived is False + assert new_tag.archived is False + assert new_object_tag.archived is False + assert new_object_tag.deletion_locked is False + + assert new_taxonomy.export_id == "flags_test" + assert new_tag.value == "Flags Tag" + assert new_tag.external_id == "flags-tag" + assert new_object_tag.object_id == "object:id:1" + assert new_object_tag.taxonomy_id == taxonomy.pk + assert new_object_tag.tag_id == tag.pk + assert new_object_tag._export_id == "flags_test" # pylint: disable=protected-access + assert new_object_tag._value == "Flags Tag" # pylint: disable=protected-access diff --git a/tests/openedx_tagging/test_models.py b/tests/openedx_tagging/test_models.py index abcdc9728..a60b19f50 100644 --- a/tests/openedx_tagging/test_models.py +++ b/tests/openedx_tagging/test_models.py @@ -155,6 +155,15 @@ def test_read_only(self): assert not self.taxonomy.read_only assert self.read_only_taxonomy.read_only + def test_archived_defaults_to_false(self) -> None: + """ + Newly created taxonomies and tags are not archived. + """ + taxonomy = Taxonomy.objects.create(name="Fresh Taxonomy", export_id="fresh_taxonomy") + tag = Tag.objects.create(taxonomy=taxonomy, value="Fresh Tag") + assert taxonomy.archived is False + assert tag.archived is False + def test_read_only_taxonomy_tags_immutable(self): """ The tags of a read-only taxonomy cannot be added, edited, or deleted. @@ -584,6 +593,7 @@ def test_get_filtered_tags_with_search(self) -> None: assert result1 == result2 +@ddt.ddt class TestObjectTag(TestTagTaxonomyMixin, TestCase): """ Test the ObjectTag model and the related Taxonomy methods and fields. @@ -598,6 +608,36 @@ def setUp(self): tag=self.tag, ) + def test_archived_and_deletion_locked_default_to_false(self) -> None: + """ + Newly created object tags are neither archived nor deletion-locked. + """ + assert self.object_tag.archived is False + assert self.object_tag.deletion_locked is False + + @ddt.data( + ("archived", "deletion_locked"), + ("deletion_locked", "archived"), + ) + @ddt.unpack + def test_setting_one_flag_leaves_the_other_and_identity_unchanged(self, flag: str, other_flag: str) -> None: + """ + Setting archived or deletion_locked on its own does not change the other flag or the tag's identity. + """ + expected = { + field: getattr(self.object_tag, field) + for field in ("object_id", "taxonomy_id", "tag_id", "_value", "_export_id", "is_copied") + } + + setattr(self.object_tag, flag, True) + self.object_tag.save() + self.object_tag.refresh_from_db() + + assert getattr(self.object_tag, flag) is True + assert getattr(self.object_tag, other_flag) is False + for field, value in expected.items(): + assert getattr(self.object_tag, field) == value + def test_representations(self): assert ( str(self.object_tag) @@ -613,6 +653,16 @@ def test_cast(self): == " object:id:1: Life on Earth=Bacteria" ) + def test_cast_copies_archived_and_deletion_locked(self) -> None: + """ + Casting an object tag carries over its archived and deletion_locked flags. + """ + self.object_tag.archived = True + self.object_tag.deletion_locked = True + copy_tag = ObjectTagTestSubclass.cast(self.object_tag) + assert copy_tag.archived is True + assert copy_tag.deletion_locked is True + def test_object_tag_export_id(self): # ObjectTag's export_id defaults to its taxonomy's export_id assert self.object_tag.export_id == self.taxonomy.export_id From 94aef8d038eac4a153f5ef256508c0d5c1147345 Mon Sep 17 00:00:00 2001 From: tbain Date: Fri, 9 Oct 2026 14:48:22 -0700 Subject: [PATCH 2/4] feat: Addressing code review comments --- tests/openedx_tagging/test_migrations.py | 19 ++--------- tests/openedx_tagging/test_models.py | 43 ------------------------ 2 files changed, 3 insertions(+), 59 deletions(-) diff --git a/tests/openedx_tagging/test_migrations.py b/tests/openedx_tagging/test_migrations.py index 776fad7f2..7dd59e2c9 100644 --- a/tests/openedx_tagging/test_migrations.py +++ b/tests/openedx_tagging/test_migrations.py @@ -1,5 +1,5 @@ """ -Tests for openedx_tagging migrations, run against existing data. +Tests that run openedx_tagging migrations over rows created on the earlier schema. Uses the `migrator` pytest fixture from django_test_migrations to run the actual migration against historical (frozen) model states, the same way it will run against @@ -106,8 +106,7 @@ def test_reverse_migration_is_a_noop_that_keeps_data(migrator) -> None: @pytest.mark.django_db def test_archived_and_deletion_locked_default_to_false_for_existing_rows(migrator) -> None: """ - Rows that exist before 0023 come out with archived and deletion_locked False, and their - other fields are untouched. + Rows that exist before 0023 come out unarchived and unlocked. """ old_state = migrator.apply_initial_migration(MIGRATE_0023_FROM) Taxonomy = old_state.apps.get_model("oel_tagging", "Taxonomy") @@ -118,10 +117,7 @@ def test_archived_and_deletion_locked_default_to_false_for_existing_rows(migrato tag = Tag.objects.create( taxonomy=taxonomy, value="Flags Tag", external_id="flags-tag", depth=0, lineage="Flags Tag\t", ) - # Historical models don't run ObjectTag.__init__, so the cached fields are set explicitly. - object_tag = ObjectTag.objects.create( - object_id="object:id:1", taxonomy=taxonomy, tag=tag, _export_id="flags_test", _value="Flags Tag", - ) + object_tag = ObjectTag.objects.create(object_id="object:id:1", taxonomy=taxonomy, tag=tag) new_state = migrator.apply_tested_migration(MIGRATE_0023_TO) NewTaxonomy = new_state.apps.get_model("oel_tagging", "Taxonomy") @@ -136,12 +132,3 @@ def test_archived_and_deletion_locked_default_to_false_for_existing_rows(migrato assert new_tag.archived is False assert new_object_tag.archived is False assert new_object_tag.deletion_locked is False - - assert new_taxonomy.export_id == "flags_test" - assert new_tag.value == "Flags Tag" - assert new_tag.external_id == "flags-tag" - assert new_object_tag.object_id == "object:id:1" - assert new_object_tag.taxonomy_id == taxonomy.pk - assert new_object_tag.tag_id == tag.pk - assert new_object_tag._export_id == "flags_test" # pylint: disable=protected-access - assert new_object_tag._value == "Flags Tag" # pylint: disable=protected-access diff --git a/tests/openedx_tagging/test_models.py b/tests/openedx_tagging/test_models.py index a60b19f50..93ab100c5 100644 --- a/tests/openedx_tagging/test_models.py +++ b/tests/openedx_tagging/test_models.py @@ -155,15 +155,6 @@ def test_read_only(self): assert not self.taxonomy.read_only assert self.read_only_taxonomy.read_only - def test_archived_defaults_to_false(self) -> None: - """ - Newly created taxonomies and tags are not archived. - """ - taxonomy = Taxonomy.objects.create(name="Fresh Taxonomy", export_id="fresh_taxonomy") - tag = Tag.objects.create(taxonomy=taxonomy, value="Fresh Tag") - assert taxonomy.archived is False - assert tag.archived is False - def test_read_only_taxonomy_tags_immutable(self): """ The tags of a read-only taxonomy cannot be added, edited, or deleted. @@ -593,7 +584,6 @@ def test_get_filtered_tags_with_search(self) -> None: assert result1 == result2 -@ddt.ddt class TestObjectTag(TestTagTaxonomyMixin, TestCase): """ Test the ObjectTag model and the related Taxonomy methods and fields. @@ -608,36 +598,6 @@ def setUp(self): tag=self.tag, ) - def test_archived_and_deletion_locked_default_to_false(self) -> None: - """ - Newly created object tags are neither archived nor deletion-locked. - """ - assert self.object_tag.archived is False - assert self.object_tag.deletion_locked is False - - @ddt.data( - ("archived", "deletion_locked"), - ("deletion_locked", "archived"), - ) - @ddt.unpack - def test_setting_one_flag_leaves_the_other_and_identity_unchanged(self, flag: str, other_flag: str) -> None: - """ - Setting archived or deletion_locked on its own does not change the other flag or the tag's identity. - """ - expected = { - field: getattr(self.object_tag, field) - for field in ("object_id", "taxonomy_id", "tag_id", "_value", "_export_id", "is_copied") - } - - setattr(self.object_tag, flag, True) - self.object_tag.save() - self.object_tag.refresh_from_db() - - assert getattr(self.object_tag, flag) is True - assert getattr(self.object_tag, other_flag) is False - for field, value in expected.items(): - assert getattr(self.object_tag, field) == value - def test_representations(self): assert ( str(self.object_tag) @@ -654,9 +614,6 @@ def test_cast(self): ) def test_cast_copies_archived_and_deletion_locked(self) -> None: - """ - Casting an object tag carries over its archived and deletion_locked flags. - """ self.object_tag.archived = True self.object_tag.deletion_locked = True copy_tag = ObjectTagTestSubclass.cast(self.object_tag) From 87d0f17753ffacfff801dee348819a6e8c2301d4 Mon Sep 17 00:00:00 2001 From: tbain Date: Mon, 12 Oct 2026 10:15:13 -0700 Subject: [PATCH 3/4] chore: re-trigger CI From 2bab7c2bd7b21f35942dc1738e104fbe6a85ce80 Mon Sep 17 00:00:00 2001 From: tbain Date: Tue, 20 Oct 2026 08:48:16 -0700 Subject: [PATCH 4/4] feat: updating test name per code review request --- tests/openedx_tagging/test_models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/openedx_tagging/test_models.py b/tests/openedx_tagging/test_models.py index 93ab100c5..e95c61edc 100644 --- a/tests/openedx_tagging/test_models.py +++ b/tests/openedx_tagging/test_models.py @@ -613,7 +613,7 @@ def test_cast(self): == " object:id:1: Life on Earth=Bacteria" ) - def test_cast_copies_archived_and_deletion_locked(self) -> None: + def test_copying_a_tag_via_casting_copies_field_values(self) -> None: self.object_tag.archived = True self.object_tag.deletion_locked = True copy_tag = ObjectTagTestSubclass.cast(self.object_tag)