diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index e77368e2..aeffa12d 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.4.2" +__version__ = "0.4.3" diff --git a/openedx_tagging/core/tagging/import_export/actions.py b/openedx_tagging/core/tagging/import_export/actions.py index b0c7cb1e..70061a1b 100644 --- a/openedx_tagging/core/tagging/import_export/actions.py +++ b/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: diff --git a/openedx_tagging/core/tagging/import_export/import_plan.py b/openedx_tagging/core/tagging/import_export/import_plan.py index 5187abba..7d0a1d10 100644 --- a/openedx_tagging/core/tagging/import_export/import_plan.py +++ b/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,6 +92,18 @@ 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` @@ -91,9 +111,9 @@ def _build_delete_actions(self, tags: dict): 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: diff --git a/openedx_tagging/core/tagging/models/base.py b/openedx_tagging/core/tagging/models/base.py index 981cffc3..13beeb0a 100644 --- a/openedx_tagging/core/tagging/models/base.py +++ b/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. diff --git a/tests/openedx_tagging/core/tagging/import_export/test_actions.py b/tests/openedx_tagging/core/tagging/import_export/test_actions.py index 22c85b2a..cd3aec60 100644 --- a/tests/openedx_tagging/core/tagging/import_export/test_actions.py +++ b/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_4 / Tag 4) " + "from parent (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_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 6) " + "from parent (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 diff --git a/tests/openedx_tagging/core/tagging/import_export/test_api.py b/tests/openedx_tagging/core/tagging/import_export/test_api.py index b2453077..d23eb6dd 100644 --- a/tests/openedx_tagging/core/tagging/import_export/test_api.py +++ b/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 diff --git a/tests/openedx_tagging/core/tagging/import_export/test_import_plan.py b/tests/openedx_tagging/core/tagging/import_export/test_import_plan.py index 6dfbcca8..2e158748 100644 --- a/tests/openedx_tagging/core/tagging/import_export/test_import_plan.py +++ b/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_1 / Tag 1) to 'Tag 2'\n" + "#4: Update the parent of (tag_4 / Tag 4) from parent " + " (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_2 / Tag 2) from parent " + " (tag_1 / Tag 1) to None\n" + "#7: Rename tag value of (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_2 / Tag 2) to 'Tag 2 v2'\n" + "#4: Update the parent of (tag_4 / Tag 4) from parent " + " (tag_3 / Tag 3) to tag_1\n" + "#5: Rename tag value of (tag_4 / Tag 4) to 'Tag 4 v2'\n" + "#6: No changes needed for (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 (29 / tag_4 / Tag 4) from parent " - # " (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 (tag_4 / Tag 4)\n" + "#1: Delete tag (tag_1 / Tag 1)\n" + "#2: Delete tag (tag_2 / Tag 2)\n" + "#3: Update the parent of (tag_4 / Tag 4) from parent " + " (tag_3 / Tag 3) to None\n" + "#4: Delete tag (tag_3 / Tag 3)\n" + "#5: No changes needed for (tag_4 / Tag 4)\n" ), ) @ddt.unpack diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index bad9d8f2..72352bcc 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/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 (old_tag_1 / Old tag 1)\n" \ + + "#2: Delete tag (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" \