fix: Issue when delete all tags on import (#135)

openedx_learning/__init__.py

@@ -1,4 +1,4 @@
 """
 Open edX Learning ("Learning Core").
 """
-__version__ = "0.4.2"
+__version__ = "0.4.3"

openedx_tagging/core/tagging/import_export/actions.py

@@ -72,7 +72,12 @@ def _get_tag(self) -> Tag:
         """
         Returns the respective tag of this actions
         """
-        return self.taxonomy.tag_set.get(external_id=self.tag.id)
+        if self.tag.id:
+            try:
+                return self.taxonomy.tag_set.get(external_id=self.tag.id)
+            except Tag.DoesNotExist:
+                pass
+        return self.taxonomy.tag_set.get(value=self.tag.value, external_id=None)
 
     def _search_action(
         self,
@@ -266,18 +271,15 @@ class UpdateParentTag(ImportAction):
 
     def __str__(self) -> str:
         taxonomy_tag = self._get_tag()
-        if not taxonomy_tag.parent:
-            from_str = _("from empty parent")
-        else:
-            from_str = _("from parent (external_id={external_id})").format(external_id=taxonomy_tag.parent.external_id)
 
-        return str(
-            _(
-                "Update the parent of tag (external_id={external_id}) "
-                "{from_str} to parent (external_id={parent_id})."
-            ).format(external_id=taxonomy_tag.external_id, from_str=from_str, parent_id=self.tag.parent_id)
+        description_str = _("Update the parent of {tag} from parent {old_parent} to {new_parent}").format(
+            tag=taxonomy_tag.display_str(),
+            old_parent=taxonomy_tag.parent.display_str() if taxonomy_tag.parent else None,
+            new_parent=self.tag.parent_id,
         )
 
+        return str(description_str)
+
     @classmethod
     def applies_for(cls, taxonomy: Taxonomy, tag) -> bool:
         """
@@ -333,13 +335,13 @@ class RenameTag(ImportAction):
 
     def __str__(self) -> str:
         taxonomy_tag = self._get_tag()
-        return str(
-            _(
-                "Rename tag value of tag (external_id={external_id}) "
-                "from '{from_value}' to '{to_value}'"
-            ).format(external_id=taxonomy_tag.external_id, from_value=taxonomy_tag.value, to_value=self.tag.value)
+        description_str = _("Rename tag value of {tag} to '{new_value}'").format(
+            tag=taxonomy_tag.display_str(),
+            new_value=self.tag.value,
         )
 
+        return str(description_str)
+
     @classmethod
     def applies_for(cls, taxonomy: Taxonomy, tag) -> bool:
         """
@@ -383,7 +385,7 @@ class DeleteTag(ImportAction):
     """
 
     def __str__(self) -> str:
-        return str(_("Delete tag (external_id={external_id})").format(external_id=self.tag.id))
+        return str(_("Delete tag {tag}").format(tag=self.tag))
 
     name = "delete"
 
@@ -423,7 +425,7 @@ class WithoutChanges(ImportAction):
     name = "without_changes"
 
     def __str__(self) -> str:
-        return str(_("No changes needed for tag (external_id={external_id})").format(external_id=self.tag.id))
+        return str(_("No changes needed for {tag}").format(tag=self.tag))
 
     @classmethod
     def applies_for(cls, taxonomy: Taxonomy, tag) -> bool:

openedx_tagging/core/tagging/import_export/import_plan.py

@@ -6,7 +6,7 @@
 from attrs import define
 from django.db import transaction
 
-from ..models import TagImportTask, Taxonomy
+from ..models import Tag, TagImportTask, Taxonomy
 from .actions import DeleteTag, ImportAction, UpdateParentTag, WithoutChanges, available_actions
 from .exceptions import ImportActionError
 
@@ -22,6 +22,14 @@ class TagItem:
     index: int | None = 0
     parent_id: str | None = None
 
+    def __str__(self):
+        """
+        User-facing string representation of a Tag.
+        """
+        if self.id:
+            return f"<{self.__class__.__name__}> ({self.id} / {self.value})"
+        return f"<{self.__class__.__name__}> ({self.value})"
+
 
 class TagImportPlan:
     """
@@ -84,16 +92,28 @@ def _search_parent_update(
 
         return False
 
+    def _get_tag_id(self, tag: Tag) -> str:
+        """
+        Get the id used on the Tag model.
+
+        By default, the external_id is used for import and export,
+        but there are cases where taxonomies are created without external_id.
+        In those cases the tag id is used
+        """
+        if tag.external_id:
+            return tag.external_id
+        return str(tag.id)
+
     def _build_delete_actions(self, tags: dict):
         """
         Adds delete actions for `tags`
         """
         for tag in tags.values():
             for child in tag.children.all():
                 # Verify if there is not a parent update before
-                if not self._search_parent_update(child.external_id, tag.external_id):
+                if not self._search_parent_update(self._get_tag_id(child), self._get_tag_id(tag)):
                     # Change parent to avoid delete childs
-                    if child.external_id not in tags:
+                    if self._get_tag_id(child) not in tags:
                         # Only update parent if the child is not going to be deleted
                         self._build_action(
                             UpdateParentTag,
@@ -136,7 +156,7 @@ def generate_actions(
 
         if replace:
             tags_for_delete = {
-                tag.external_id: tag for tag in self.taxonomy.tag_set.all()
+                self._get_tag_id(tag): tag for tag in self.taxonomy.tag_set.all()
             }
 
             for tag in tags:

openedx_tagging/core/tagging/models/base.py

@@ -96,6 +96,14 @@ def __str__(self):
         """
         return f"<{self.__class__.__name__}> ({self.id}) {self.value}"
 
+    def display_str(self):
+        """
+        String representation of a Tag used on user logs.
+        """
+        if self.external_id:
+            return f"<{self.__class__.__name__}> ({self.external_id} / {self.value})"
+        return f"<{self.__class__.__name__}> ({self.value})"
+
     def get_lineage(self) -> Lineage:
         """
         Queries and returns the lineage of the current tag as a list of Tag.value strings.

tests/openedx_tagging/core/tagging/import_export/test_actions.py

@@ -283,16 +283,16 @@ class TestUpdateParentTag(TestImportActionMixin, TestCase):
             "tag_4",
             "tag_3",
             (
-                "Update the parent of tag (external_id=tag_4) from parent "
-                "(external_id=tag_3) to parent (external_id=tag_3)."
+                "Update the parent of <Tag> (tag_4 / Tag 4) "
+                "from parent <Tag> (tag_3 / Tag 3) to tag_3"
             )
         ),
         (
             "tag_3",
             "tag_2",
             (
-                "Update the parent of tag (external_id=tag_3) from empty parent "
-                "to parent (external_id=tag_2)."
+                "Update the parent of <Tag> (tag_3 / Tag 3) "
+                "from parent None to tag_2"
             )
         ),
     )
@@ -310,6 +310,34 @@ def test_str(self, tag_id: str, parent_id: str, expected: str):
         )
         assert str(action) == expected
 
+    def test_str_no_external_id(self):
+        tag_1 = Tag(
+            value="Tag 5",
+            taxonomy=self.taxonomy,
+        )
+        tag_1.save()
+        tag_2 = Tag(
+            value="Tag 6",
+            taxonomy=self.taxonomy,
+            parent=tag_1,
+        )
+        tag_2.save()
+        tag_item = TagItem(
+            id=None,
+            value='Tag 6',
+            parent_id='tag_3',
+        )
+        action = UpdateParentTag(
+            taxonomy=self.taxonomy,
+            tag=tag_item,
+            index=100,
+        )
+        expected = (
+            "Update the parent of <Tag> (Tag 6) "
+            "from parent <Tag> (Tag 5) to tag_3"
+        )
+        assert str(action) == expected
+
     @ddt.data(
         ('tag_100', None, False),  # Tag doesn't exist on database
         ('tag_2', 'tag_1', False),  # Parent don't change

tests/openedx_tagging/core/tagging/import_export/test_api.py

@@ -215,6 +215,37 @@ def test_import_with_export_output(self) -> None:
                     assert new_tag.parent
                     assert tag.parent.external_id == new_tag.parent.external_id
 
+    def test_import_removing_no_external_id(self) -> None:
+        new_taxonomy = Taxonomy(name="New taxonomy")
+        new_taxonomy.save()
+        tag1 = Tag.objects.create(
+            id=1000,
+            value="Tag 1",
+            taxonomy=new_taxonomy,
+        )
+        tag2 = Tag.objects.create(
+            id=1001,
+            value="Tag 2",
+            taxonomy=new_taxonomy,
+        )
+        tag3 = Tag.objects.create(
+            id=1002,
+            value="Tag 3",
+            taxonomy=new_taxonomy,
+        )
+        tag1.save()
+        tag2.save()
+        tag3.save()
+        # Import with empty tags, to remove all tags
+        importFile = BytesIO(json.dumps({"tags": []}).encode())
+        result, _tasks, _plan = import_export_api.import_tags(
+            new_taxonomy,
+            importFile,
+            ParserFormat.JSON,
+            replace=True,
+        )
+        assert result
+
     def test_import_removing_with_childs(self) -> None:
         """
         Test import need to remove childs with parents that will also be removed
@@ -247,12 +278,52 @@ def test_import_removing_with_childs(self) -> None:
 
         # Import with empty tags, to remove all tags
         importFile = BytesIO(json.dumps({"tags": []}).encode())
-        assert import_export_api.import_tags(
+        result, _tasks, _plan = import_export_api.import_tags(
             new_taxonomy,
             importFile,
             ParserFormat.JSON,
             replace=True,
         )
+        assert result
+
+    def test_import_removing_with_childs_no_external_id(self) -> None:
+        """
+        Test import need to remove childs with parents that will also be removed,
+        using tags without external_id
+        """
+        new_taxonomy = Taxonomy(name="New taxonomy")
+        new_taxonomy.save()
+        level2 = Tag.objects.create(
+            id=1000,
+            value="Tag 2",
+            taxonomy=new_taxonomy,
+        )
+        level1 = Tag.objects.create(
+            id=1001,
+            value="Tag 1",
+            taxonomy=new_taxonomy,
+        )
+        level3 = Tag.objects.create(
+            id=1002,
+            value="Tag 3",
+            taxonomy=new_taxonomy,
+        )
+        level2.parent = level1
+        level2.save()
+
+        level3.parent = level3
+        level3.save()
+
+        # Import with empty tags, to remove all tags
+        importFile = BytesIO(json.dumps({"tags": []}).encode())
+
+        result, _tasks, _plan = import_export_api.import_tags(
+            new_taxonomy,
+            importFile,
+            ParserFormat.JSON,
+            replace=True,
+        )
+        assert result
 
     def test_import_same_value_without_external_id(self) -> None:
         new_taxonomy = Taxonomy(name="New taxonomy")
@@ -273,5 +344,4 @@ def test_import_same_value_without_external_id(self) -> None:
             ParserFormat.JSON,
             replace=True,
         )
-
         assert result

tests/openedx_tagging/core/tagging/import_export/test_import_plan.py

@@ -251,13 +251,13 @@ def test_generate_actions(self, tags, replace, expected_errors, expected_actions
             "--------------------------------\n"
             "#1: Create a new tag with values (external_id=tag_31, value=Tag 31, parent_id=None).\n"
             "#2: Create a new tag with values (external_id=tag_31, value=Tag 32, parent_id=None).\n"
-            "#3: Rename tag value of tag (external_id=tag_1) from 'Tag 1' to 'Tag 2'\n"
-            "#4: Update the parent of tag (external_id=tag_4) from parent (external_id=tag_3) "
-            "to parent (external_id=tag_100).\n"
+            "#3: Rename tag value of <Tag> (tag_1 / Tag 1) to 'Tag 2'\n"
+            "#4: Update the parent of <Tag> (tag_4 / Tag 4) from parent "
+            "<Tag> (tag_3 / Tag 3) to tag_100\n"
             "#5: Create a new tag with values (external_id=tag_33, value=Tag 32, parent_id=None).\n"
-            "#6: Update the parent of tag (external_id=tag_2) from parent (external_id=tag_1) "
-            "to parent (external_id=None).\n"
-            "#7: Rename tag value of tag (external_id=tag_2) from 'Tag 2' to 'Tag 31'\n"
+            "#6: Update the parent of <Tag> (tag_2 / Tag 2) from parent "
+            "<Tag> (tag_1 / Tag 1) to None\n"
+            "#7: Rename tag value of <Tag> (tag_2 / Tag 2) to 'Tag 31'\n"
             "\nOutput errors\n"
             "--------------------------------\n"
             "Conflict with 'create' (#2) and action #1: Duplicated external_id tag.\n"
@@ -299,11 +299,11 @@ def test_generate_actions(self, tags, replace, expected_errors, expected_actions
             "--------------------------------\n"
             "#1: Create a new tag with values (external_id=tag_31, value=Tag 31, parent_id=None).\n"
             "#2: Create a new tag with values (external_id=tag_32, value=Tag 32, parent_id=tag_1).\n"
-            "#3: Rename tag value of tag (external_id=tag_2) from 'Tag 2' to 'Tag 2 v2'\n"
-            "#4: Update the parent of tag (external_id=tag_4) from parent (external_id=tag_3) "
-            "to parent (external_id=tag_1).\n"
-            "#5: Rename tag value of tag (external_id=tag_4) from 'Tag 4' to 'Tag 4 v2'\n"
-            "#6: No changes needed for tag (external_id=tag_1)\n"
+            "#3: Rename tag value of <Tag> (tag_2 / Tag 2) to 'Tag 2 v2'\n"
+            "#4: Update the parent of <Tag> (tag_4 / Tag 4) from parent "
+            "<Tag> (tag_3 / Tag 3) to tag_1\n"
+            "#5: Rename tag value of <Tag> (tag_4 / Tag 4) to 'Tag 4 v2'\n"
+            "#6: No changes needed for <TagItem> (tag_1 / Tag 1)\n"
         ),
         # Testing deletes (replace=True)
         (
@@ -315,18 +315,14 @@ def test_generate_actions(self, tags, replace, expected_errors, expected_actions
                 },
             ],
             True,
-            # ToDo: Change the lines below when https://github.com/openedx/openedx-learning/pull/135 is merged
             "Import plan for Import Taxonomy Test\n"
             "--------------------------------\n"
-            "#1: Delete tag (external_id=tag_1)\n"
-            "#2: Delete tag (external_id=tag_2)\n"
-            "#3: Update the parent of tag (external_id=tag_4) from parent (external_id=tag_3) "
-            "to parent (external_id=None).\n"
-            # "#3: Update the parent of <Tag> (29 / tag_4 / Tag 4) from parent "
-            # "<Tag> (28 / tag_3 / Tag 3) to None\n"
-            "#4: Delete tag (external_id=tag_3)\n"
-            "#5: No changes needed for tag (external_id=tag_4)\n"
-            # "#5: No changes needed for <TagItem> (tag_4 / Tag 4)\n"
+            "#1: Delete tag <TagItem> (tag_1 / Tag 1)\n"
+            "#2: Delete tag <TagItem> (tag_2 / Tag 2)\n"
+            "#3: Update the parent of <Tag> (tag_4 / Tag 4) from parent "
+            "<Tag> (tag_3 / Tag 3) to None\n"
+            "#4: Delete tag <TagItem> (tag_3 / Tag 3)\n"
+            "#5: No changes needed for <TagItem> (tag_4 / Tag 4)\n"
         ),
     )
     @ddt.unpack

tests/openedx_tagging/core/tagging/test_views.py

@@ -2457,8 +2457,8 @@ def test_import_plan(self, file_format: str) -> None:
         assert response.data["task"]["status"] == "success"
         expected_plan = "Import plan for Test import taxonomy\n" \
             + "--------------------------------\n" \
-            + "#1: Delete tag (external_id=old_tag_1)\n" \
-            + "#2: Delete tag (external_id=old_tag_2)\n" \
+            + "#1: Delete tag <TagItem> (old_tag_1 / Old tag 1)\n" \
+            + "#2: Delete tag <TagItem> (old_tag_2 / Old tag 2)\n" \
             + "#3: Create a new tag with values (external_id=tag_1, value=Tag 1, parent_id=None).\n" \
             + "#4: Create a new tag with values (external_id=tag_2, value=Tag 2, parent_id=None).\n" \
             + "#5: Create a new tag with values (external_id=tag_3, value=Tag 3, parent_id=None).\n" \