Skip to content

Commit d3750e8

Browse files
kdmccormickclaude
andauthored
fix: Handle Taxonomy CASCADE deletes in _is_explicit_tag_delete (openedx#575)
When a Taxonomy is deleted, Django fires pre_delete for each related Tag with origin set to the Taxonomy instance. The previous code raised a TypeError for any non-Tag, non-QuerySet origin, breaking any consumer that deletes a Taxonomy. For a non-Tag-queryset origin (e.g. taxonomy.delete() or Taxonomy.objects.filter(...).delete()), we now emit only for root-level tags; their handler covers the whole subtree via lineage__startswith. Bumps to v1.0.1. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 6d074b9 commit d3750e8

3 files changed

Lines changed: 41 additions & 15 deletions

File tree

src/openedx_core/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
"""
77

88
# The version for the entire repository
9-
__version__ = "1.0.0"
9+
__version__ = "1.0.1"

src/openedx_tagging/signal_handlers.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,30 +16,33 @@
1616

1717
def _is_explicit_tag_delete(
1818
instance: Tag,
19-
origin: Tag | QuerySet[Tag] | None,
19+
origin: object,
2020
using: str | None,
2121
) -> bool:
2222
"""
2323
Return True only for tags explicitly targeted by the delete operation.
2424
2525
Descendants deleted via CASCADE are skipped here because the explicit root
26-
tag's handler emits updates for the whole subtree.
26+
tag's handler emits updates for the whole subtree via lineage__startswith.
2727
2828
Args:
2929
instance: The Tag being deleted.
30-
origin: The source of the delete operation - either a Tag instance (for instance.delete())
31-
or a QuerySet[Tag] (for queryset.delete()), or None for other origins.
30+
origin: The source of the delete operation — a Tag instance (instance.delete()),
31+
a QuerySet[Tag] (queryset.delete()), or any other value when the delete
32+
was triggered by CASCADE from a parent model (e.g. taxonomy.delete()).
3233
using: The database alias to use for queries, passed from the Django signal.
3334
"""
3435
if isinstance(origin, Tag):
3536
return origin.pk == instance.pk
3637

37-
# Fail fast if origin has an unexpected type so callsites don't silently
38-
# skip event emission logic.
3938
if not isinstance(origin, QuerySet):
40-
raise TypeError(f"Expected origin to be Tag, QuerySet[Tag], or None; got {type(origin).__name__}")
39+
# CASCADE from a non-queryset origin (e.g., taxonomy.delete(), or None for unknown callers).
40+
# Only emit for root-level tags; the root handler covers the whole subtree via lineage__startswith.
41+
return len(instance.get_lineage()) == 1
4142
if origin.model is not Tag:
42-
raise TypeError(f"Expected origin queryset model Tag; got {origin.model.__name__}")
43+
# CASCADE from a queryset of a non-Tag model (e.g., Taxonomy.objects.filter(...).delete()).
44+
# Only emit for root-level tags; the root handler covers the whole subtree via lineage__startswith.
45+
return len(instance.get_lineage()) == 1
4346

4447
# Check if this instance is in the set of explicitly-targeted tags. If not, it's being deleted
4548
# as a CASCADE side-effect, so it's not explicit.

tests/openedx_tagging/test_models.py

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -676,12 +676,16 @@ def test_object_tag_export_id(self):
676676
self.object_tag.refresh_from_db()
677677
assert self.object_tag.export_id == "another-taxonomy"
678678

679-
def test_is_explicit_tag_delete_raises_for_unexpected_origin_type(self):
680-
with pytest.raises(
681-
TypeError,
682-
match=r"Expected origin to be Tag, QuerySet\[Tag\], or None; got Taxonomy",
683-
):
684-
_is_explicit_tag_delete(instance=self.tag, origin=cast(Any, self.taxonomy), using="default")
679+
def test_is_explicit_tag_delete_taxonomy_cascade(self):
680+
# Root-level tag is treated as explicit when cascaded from a Taxonomy deletion,
681+
# so its handler covers the whole subtree via lineage__startswith.
682+
assert _is_explicit_tag_delete(
683+
instance=self.tag, origin=cast(Any, self.taxonomy), using="default"
684+
)
685+
# Non-root tag returns False — its ancestor's handler already covers it.
686+
assert not _is_explicit_tag_delete(
687+
instance=self.eubacteria, origin=cast(Any, self.taxonomy), using="default"
688+
)
685689

686690
def test_object_tag_value(self):
687691
# ObjectTag's value defaults to its tag's value
@@ -1185,6 +1189,25 @@ def test_delete_with_descendants_updates_search_index(self, mock_task_delay) ->
11851189
delta_object_id,
11861190
}
11871191

1192+
@patch("openedx_tagging.signal_handlers.emit_content_object_associations_changed_for_object_ids_task.delay")
1193+
def test_taxonomy_delete_updates_search_index(self, mock_task_delay) -> None:
1194+
"""
1195+
Deleting a Taxonomy should enqueue updates for any tagged objects,
1196+
including ones tagged with deep descendants of root tags.
1197+
"""
1198+
object_id = "content-v1:org+course+run+type@unit+block@128"
1199+
api.tag_object(
1200+
object_id=object_id,
1201+
taxonomy=self.foxtrot.taxonomy,
1202+
tags=[self.foxtrot.value],
1203+
)
1204+
1205+
with self.captureOnCommitCallbacks(execute=True):
1206+
self.foxtrot.taxonomy.delete()
1207+
1208+
assert mock_task_delay.call_count == 1
1209+
assert mock_task_delay.call_args.kwargs["object_ids"] == [object_id]
1210+
11881211
@patch("openedx_tagging.tasks.CONTENT_OBJECT_ASSOCIATIONS_CHANGED", new_callable=MagicMock)
11891212
def test_emit_content_object_associations_changed_for_object_ids_task(self, mock_signal) -> None:
11901213
"""Task emits one CONTENT_OBJECT_ASSOCIATIONS_CHANGED event per distinct object."""

0 commit comments

Comments
 (0)