Tag import: reject a parent_id that names a tag the full replace import deletes

This section is background and a suggested approach, not the source of truth. The User Story and Acceptance Criteria are what must be true when the work is done.

In short

Where the check runs. The parent validation that each row's action runs cannot catch every case. A kept tag whose row names its current parent counts as "no change" for the parent-update action, so no action that validates its parent is built at all, and the delete phase quietly promotes the tag to the root. The check therefore runs once at the plan level, over the file's rows, and not inside the per-row parent validation. It looks only at each row's parent_id, so it treats a moved tag, a kept tag, a new tag and a renamed tag the same way.

What counts as naming a deleted tag. By the time the check runs, the plan has built the delete steps for the tags that have no row in the file, and it has computed the tag tree as it will be after the import. A row is rejected when its parent_id, compared without regard to case, names a tag the import deletes and no tag of the final tree holds that id. The second condition keeps the valid case working: when the file renames another tag onto the deleted tag's id, the renamed tag holds the id in the final tree, so a child that names the id is accepted and ends up under the renamed tag, as it does today. The error names the row and the parent_id as the row spells it. Every offending row gets its own error in the same pass.

What stays the same. A kept tag whose row has no parent_id is still promoted to the root by the delete phase, keeping its database row and its content associations. Running the import doesn't change, because a plan with errors never runs. The one changed outcome is the kept-current-parent file that the Description covers. Three existing tests import exactly such a file and change with it.

Implementation specifics

• New plan-level error DeletedParentError(row, parent_id) in src/openedx_tagging/import_export/exceptions.py: a TagImportError subclass like ParentCycleError, using the "Row N:" form from #892. Suggested text: Row 2: Parent tag tag_3 is deleted by this import, because the file has no row for it. Add a row for tag_3 or change the parent_id. The final wording follows the Acceptance Criteria.
• New method TagImportPlan._validate_no_parent_is_deleted(tags, parent_of) in src/openedx_tagging/import_export/import_plan.py, called in generate_actions right after _validate_hierarchy. It builds deleted from self.indexed_actions["delete"], casefolded the way _end_state builds it, and appends one error for each row whose parent_id.casefold() is in deleted and is not a key of parent_of.
• Row numbers: a row's number is its 1-based position in tags, as in _validate_no_duplicate_final_ids and ImportAction.row.
• Leave _validate_parent and UpdateParentTag.applies_for alone: the row that keeps its current parent never reaches _validate_parent. Turning that row into a parent change only so it can be rejected would add a meaningless "update the parent from tag_3 to tag_3" step to the plan.
• No database queries: the check reads only the delete steps and parent_of, which the plan has already built.
• Comments to update:
  • _validate_hierarchy's docstring says that a parent_id naming a deleted tag, with no row reclaiming the id, counts as a root because the delete phase promotes the children. Make it say the new check rejects such a parent_id.
  • The comment in _build_detach_actions says such a parent_id is "left to _validate_parent". Make it say the plan rejects it.
• Contract: import_tags(..., replace=True) keeps its signature and return value, and nothing in it is marked UNSTABLE. The PR description and the fix: commit body must call out the changed outcome for the kept-current-parent file. Imports with replace=False delete nothing, so they are unaffected. No migration.
• Three replace-mode data sets change (tests/openedx_tagging/import_export/test_import_plan.py): the replace-mode data sets of test_generate_actions, test_plan and test_execute each import one row, tag_4 with parent_id tag_3, and no row for tag_3, which this ticket rejects. Drop parent_id from that row, so the data sets keep covering a replace import that deletes tags.
• Their new expected outputs: with no parent_id, the row moves tag_4 to the root, because a row without a parent_id for a tag that has a parent is a parent change.
  • The last expected action in test_generate_actions becomes update_parent for tag_4 instead of without_changes.
  • The last line of test_plan becomes #5: Update the parent of <Tag> (tag_4 / Tag 4) from parent <Tag> (tag_3 / Tag 3) to None.
  • In test_execute, remove the if not replace guard so the replace-mode case checks parents too; that guard is what hid this behavior.
• New unit tests in test_import_plan.py: a moved tag under the deleted tag; a kept tag naming its current, deleted parent, spelled tag_3 and TAG_3; a new tag and a renamed tag under the deleted tag; two offending rows reported in one pass.
• New and extended acceptance tests in tests/openedx_tagging/import_export/test_api.py:
  • Add the rejection scenarios to TestImportAcceptanceScenarios, using _assert_rejected across _orders.
  • Extend test_child_of_deleted_holder_moves_under_tag_renamed_onto_its_id with a child that names the id in another case.
  • Add a content-association check to the case where a kept child becomes a root.
• Out of scope:
  • The double move to the root of a kept child whose row has no parent_id. The delete phase and the row each build one; this already happens today and is noted in the generate_actions TODO.
  • The error-text changes in #892.

Modified files

• Builds on the #858 pull request, whose final-tree computation (TagImportPlan._end_state) this check reads; start after it merges.
• Land this ticket after #892, whose "Row N:" form the new error follows.
• The Studio import endpoints in src/openedx_tagging/rest_api/v1/views.py always call import_tags(..., replace=True).
• Prior art: _validate_no_duplicate_final_ids and _end_state in src/openedx_tagging/import_export/import_plan.py; ParentCycleError in src/openedx_tagging/import_export/exceptions.py.
• test_child_of_deleted_holder_moves_under_tag_renamed_onto_its_id in tests/openedx_tagging/import_export/test_api.py is the valid case that must keep passing.
• #673 (previous_id); epic #720.