From fb8d16112fa4705fda8f8ac44cd3401cfb08619e Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 20 Dec 2023 17:08:15 -0500 Subject: [PATCH 1/8] fix: Issue when delete all tags on import --- .../core/tagging/import_export/actions.py | 48 +++++++++++++++---- .../core/tagging/import_export/import_plan.py | 2 +- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/openedx_tagging/core/tagging/import_export/actions.py b/openedx_tagging/core/tagging/import_export/actions.py index 679f36ae2..524c6589a 100644 --- a/openedx_tagging/core/tagging/import_export/actions.py +++ b/openedx_tagging/core/tagging/import_export/actions.py @@ -72,7 +72,9 @@ 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: + return self.taxonomy.tag_set.get(external_id=self.tag.id) + return self.taxonomy.tag_set.get(value=self.tag.value) def _search_action( self, @@ -256,16 +258,35 @@ 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) + if taxonomy_tag.parent.external_id: + from_str = _("from parent (external_id={external_id})").format( + external_id=taxonomy_tag.parent.external_id + ) + else: + from_str = _("from parent (value={value})").format( + value=taxonomy_tag.parent.value + ) + from_str = "" + + if taxonomy_tag.external_id: + prefix_str = _("Update the parent of tag (external_id={external_id})").format( + external_id=taxonomy_tag.external_id + ) + else: + prefix_str = "" + prefix_str = _("Update the parent of tag (value={value})").format( + value=taxonomy_tag.value + ) return str( _( - "Update the parent of tag (external_id={external_id}) " + "{prefix_str} " "{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) + ).format(prefix_str=prefix_str, from_str=from_str, parent_id=self.tag.parent_id) ) @classmethod @@ -323,11 +344,18 @@ class RenameTag(ImportAction): def __str__(self) -> str: taxonomy_tag = self._get_tag() + if taxonomy_tag.external_id: + prefix_str = _("Rename tag value of tag (external_id={external_id})").format( + external_id=taxonomy_tag.external_id + ) + else: + prefix_str = _("Rename tag value of tag (id={id})").format(id=taxonomy_tag.id) + return str( _( - "Rename tag value of tag (external_id={external_id}) " + "{prefix_str} " "from '{from_value}' to '{to_value}'" - ).format(external_id=taxonomy_tag.external_id, from_value=taxonomy_tag.value, to_value=self.tag.value) + ).format(prefix_str=prefix_str, from_value=taxonomy_tag.value, to_value=self.tag.value) ) @classmethod @@ -373,7 +401,9 @@ class DeleteTag(ImportAction): """ def __str__(self) -> str: - return str(_("Delete tag (external_id={external_id})").format(external_id=self.tag.id)) + if self.tag.id: + return str(_("Delete tag (external_id={external_id})").format(external_id=self.tag.id)) + return str(_("Delete tag (value={value})").format(value=self.tag.value)) name = "delete" @@ -413,7 +443,9 @@ 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)) + if self.tag.id: + return str(_("No changes needed for tag (external_id={external_id})").format(external_id=self.tag.id)) + return str(_("No changes needed for tag (value={value})").format(value=self.tag.value)) @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 b5ce41305..af71964e5 100644 --- a/openedx_tagging/core/tagging/import_export/import_plan.py +++ b/openedx_tagging/core/tagging/import_export/import_plan.py @@ -136,7 +136,7 @@ def generate_actions( if replace: tags_for_delete = { - tag.external_id: tag for tag in self.taxonomy.tag_set.all() + tag.external_id or tag.id: tag for tag in self.taxonomy.tag_set.all() } for tag in tags: From b1c4cc902182b48bf902241f69148693d81422d2 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 21 Dec 2023 14:30:05 -0500 Subject: [PATCH 2/8] fix: issuo on delete all taxonomy on import --- .../core/tagging/import_export/actions.py | 21 +++--- .../core/tagging/import_export/import_plan.py | 20 ++++-- .../tagging/import_export/test_actions.py | 28 ++++++++ .../core/tagging/import_export/test_api.py | 72 ++++++++++++++++++- 4 files changed, 125 insertions(+), 16 deletions(-) diff --git a/openedx_tagging/core/tagging/import_export/actions.py b/openedx_tagging/core/tagging/import_export/actions.py index 524c6589a..cc191d161 100644 --- a/openedx_tagging/core/tagging/import_export/actions.py +++ b/openedx_tagging/core/tagging/import_export/actions.py @@ -259,6 +259,16 @@ class UpdateParentTag(ImportAction): def __str__(self) -> str: taxonomy_tag = self._get_tag() + if taxonomy_tag.external_id: + prefix_str = _("Update the parent of tag (external_id={external_id})").format( + external_id=taxonomy_tag.external_id + ) + else: + prefix_str = "" + prefix_str = _("Update the parent of tag (value={value})").format( + value=taxonomy_tag.value + ) + if not taxonomy_tag.parent: from_str = _("from empty parent") else: @@ -270,17 +280,6 @@ def __str__(self) -> str: from_str = _("from parent (value={value})").format( value=taxonomy_tag.parent.value ) - from_str = "" - - if taxonomy_tag.external_id: - prefix_str = _("Update the parent of tag (external_id={external_id})").format( - external_id=taxonomy_tag.external_id - ) - else: - prefix_str = "" - prefix_str = _("Update the parent of tag (value={value})").format( - value=taxonomy_tag.value - ) return str( _( diff --git a/openedx_tagging/core/tagging/import_export/import_plan.py b/openedx_tagging/core/tagging/import_export/import_plan.py index af71964e5..e120a1bdc 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 @@ -84,6 +84,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 +103,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 +148,7 @@ def generate_actions( if replace: tags_for_delete = { - tag.external_id or tag.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/tests/openedx_tagging/core/tagging/import_export/test_actions.py b/tests/openedx_tagging/core/tagging/import_export/test_actions.py index 22c85b2a7..f7fc3e70a 100644 --- a/tests/openedx_tagging/core/tagging/import_export/test_actions.py +++ b/tests/openedx_tagging/core/tagging/import_export/test_actions.py @@ -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 (value=Tag 6) from parent (value=Tag 5)" + " to parent (external_id=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 a02a939c2..a7e5eba6b 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,9 +278,48 @@ 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 From e23f5420547e83ee596a4ce5bf7f66872016f01b Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 21 Dec 2023 14:58:32 -0500 Subject: [PATCH 3/8] chore: bump version --- openedx_learning/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index b2647f381..e77368e2c 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.4.1" +__version__ = "0.4.2" From 6181f7c06f6cbcbcdcb3649f96e7ebc0a93cfba3 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 8 Jan 2024 10:39:37 -0500 Subject: [PATCH 4/8] refactor: Updates on representation of tags and actions --- .../core/tagging/import_export/actions.py | 61 ++++++------------- .../core/tagging/import_export/import_plan.py | 8 +++ openedx_tagging/core/tagging/models/base.py | 4 +- .../tagging/import_export/test_actions.py | 12 ++-- .../tagging/import_export/test_import_plan.py | 29 +++++---- .../core/tagging/test_models.py | 2 +- 6 files changed, 49 insertions(+), 67 deletions(-) diff --git a/openedx_tagging/core/tagging/import_export/actions.py b/openedx_tagging/core/tagging/import_export/actions.py index cc191d161..a6e70f416 100644 --- a/openedx_tagging/core/tagging/import_export/actions.py +++ b/openedx_tagging/core/tagging/import_export/actions.py @@ -73,8 +73,11 @@ def _get_tag(self) -> Tag: Returns the respective tag of this actions """ if self.tag.id: - return self.taxonomy.tag_set.get(external_id=self.tag.id) - return self.taxonomy.tag_set.get(value=self.tag.value) + 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, @@ -259,35 +262,14 @@ class UpdateParentTag(ImportAction): def __str__(self) -> str: taxonomy_tag = self._get_tag() - if taxonomy_tag.external_id: - prefix_str = _("Update the parent of tag (external_id={external_id})").format( - external_id=taxonomy_tag.external_id - ) - else: - prefix_str = "" - prefix_str = _("Update the parent of tag (value={value})").format( - value=taxonomy_tag.value - ) - - if not taxonomy_tag.parent: - from_str = _("from empty parent") - else: - if taxonomy_tag.parent.external_id: - from_str = _("from parent (external_id={external_id})").format( - external_id=taxonomy_tag.parent.external_id - ) - else: - from_str = _("from parent (value={value})").format( - value=taxonomy_tag.parent.value - ) - - return str( - _( - "{prefix_str} " - "{from_str} to parent (external_id={parent_id})." - ).format(prefix_str=prefix_str, 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, + old_parent=taxonomy_tag.parent, + new_parent=self.tag.parent_id, ) + return str(description_str) + @classmethod def applies_for(cls, taxonomy: Taxonomy, tag) -> bool: """ @@ -343,20 +325,13 @@ class RenameTag(ImportAction): def __str__(self) -> str: taxonomy_tag = self._get_tag() - if taxonomy_tag.external_id: - prefix_str = _("Rename tag value of tag (external_id={external_id})").format( - external_id=taxonomy_tag.external_id - ) - else: - prefix_str = _("Rename tag value of tag (id={id})").format(id=taxonomy_tag.id) - - return str( - _( - "{prefix_str} " - "from '{from_value}' to '{to_value}'" - ).format(prefix_str=prefix_str, from_value=taxonomy_tag.value, to_value=self.tag.value) + description_str = _("Rename tag value of {tag} to '{new_value}'").format( + tag=taxonomy_tag, + new_value=self.tag.value, ) + return str(description_str) + @classmethod def applies_for(cls, taxonomy: Taxonomy, tag) -> bool: """ @@ -442,9 +417,7 @@ class WithoutChanges(ImportAction): name = "without_changes" def __str__(self) -> str: - if self.tag.id: - return str(_("No changes needed for tag (external_id={external_id})").format(external_id=self.tag.id)) - return str(_("No changes needed for tag (value={value})").format(value=self.tag.value)) + 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 e120a1bdc..fb8aaffd5 100644 --- a/openedx_tagging/core/tagging/import_export/import_plan.py +++ b/openedx_tagging/core/tagging/import_export/import_plan.py @@ -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: """ diff --git a/openedx_tagging/core/tagging/models/base.py b/openedx_tagging/core/tagging/models/base.py index 981cffc36..ff448d7c2 100644 --- a/openedx_tagging/core/tagging/models/base.py +++ b/openedx_tagging/core/tagging/models/base.py @@ -94,7 +94,9 @@ def __str__(self): """ User-facing string representation of a Tag. """ - return f"<{self.__class__.__name__}> ({self.id}) {self.value}" + if self.external_id: + return f"<{self.__class__.__name__}> ({self.id} / {self.external_id} / {self.value})" + return f"<{self.__class__.__name__}> ({self.id} / {self.value})" def get_lineage(self) -> Lineage: """ 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 f7fc3e70a..641445cc3 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 (29 / tag_4 / Tag 4) " + "from parent (28 / 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 (28 / tag_3 / Tag 3) " + "from parent None to tag_2" ) ), ) @@ -333,8 +333,8 @@ def test_str_no_external_id(self): index=100, ) expected = ( - "Update the parent of tag (value=Tag 6) from parent (value=Tag 5)" - " to parent (external_id=tag_3)." + "Update the parent of (31 / Tag 6) " + "from parent (30 / Tag 5) to tag_3" ) assert str(action) == expected 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 c8dbaa0c1..31a3c485b 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 (26 / tag_1 / Tag 1) to 'Tag 2'\n" + "#4: Update the parent of (29 / tag_4 / Tag 4) from parent " + " (28 / 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 (27 / tag_2 / Tag 2) from parent " + " (26 / tag_1 / Tag 1) to None\n" + "#7: Rename tag value of (27 / 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 (27 / tag_2 / Tag 2) to 'Tag 2 v2'\n" + "#4: Update the parent of (29 / tag_4 / Tag 4) from parent " + " (28 / tag_3 / Tag 3) to tag_1\n" + "#5: Rename tag value of (29 / tag_4 / Tag 4) to 'Tag 4 v2'\n" + "#6: No changes needed for (tag_1 / Tag 1)\n" ), # Testing deletes (replace=True) ( @@ -317,11 +317,11 @@ def test_generate_actions(self, tags, replace, expected_errors, expected_actions True, "Import plan for Import Taxonomy Test\n" "--------------------------------\n" - "#1: No changes needed for tag (external_id=tag_4)\n" + "#1: No changes needed for (tag_4 / Tag 4)\n" "#2: Delete tag (external_id=tag_1)\n" "#3: Delete tag (external_id=tag_2)\n" - "#4: Update the parent of tag (external_id=tag_4) from parent (external_id=tag_3) " - "to parent (external_id=None).\n" + "#4: Update the parent of (29 / tag_4 / Tag 4) from parent " + " (28 / tag_3 / Tag 3) to None\n" "#5: Delete tag (external_id=tag_3)\n" ), ) @@ -336,7 +336,6 @@ def test_plan(self, tags, replace, expected): tags = [TagItem(**tag) for tag in tags] self.import_plan.generate_actions(tags=tags, replace=replace) plan = self.import_plan.plan() - print(plan) assert plan == expected @ddt.data( diff --git a/tests/openedx_tagging/core/tagging/test_models.py b/tests/openedx_tagging/core/tagging/test_models.py index c730b16bc..4b224e9a8 100644 --- a/tests/openedx_tagging/core/tagging/test_models.py +++ b/tests/openedx_tagging/core/tagging/test_models.py @@ -178,7 +178,7 @@ def test_representations(self): == repr(self.language_taxonomy) == " (-1) Languages" ) - assert str(self.bacteria) == repr(self.bacteria) == " (1) Bacteria" + assert str(self.bacteria) == repr(self.bacteria) == " (1 / Bacteria)" def test_taxonomy_cast(self): for subclass in ( From 0b0987219c361dcce38f98795973c96ad9a80f47 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 8 Jan 2024 10:51:58 -0500 Subject: [PATCH 5/8] fix: Bug on test --- .../openedx_tagging/core/tagging/import_export/test_actions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 641445cc3..dcaf5884c 100644 --- a/tests/openedx_tagging/core/tagging/import_export/test_actions.py +++ b/tests/openedx_tagging/core/tagging/import_export/test_actions.py @@ -334,7 +334,7 @@ def test_str_no_external_id(self): ) expected = ( "Update the parent of (31 / Tag 6) " - "from parent (30 / Tag 5) to tag_3" + f"from parent ({tag_1.id} / Tag 5) to tag_3" ) assert str(action) == expected From f2df68b35fc89afd62bea31b219ebab0a66998ef Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 8 Jan 2024 11:01:04 -0500 Subject: [PATCH 6/8] fix: Bug on test --- .../openedx_tagging/core/tagging/import_export/test_actions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 dcaf5884c..02028cc72 100644 --- a/tests/openedx_tagging/core/tagging/import_export/test_actions.py +++ b/tests/openedx_tagging/core/tagging/import_export/test_actions.py @@ -333,7 +333,7 @@ def test_str_no_external_id(self): index=100, ) expected = ( - "Update the parent of (31 / Tag 6) " + f"Update the parent of ({tag_2.id} / Tag 6) " f"from parent ({tag_1.id} / Tag 5) to tag_3" ) assert str(action) == expected From 9030b1e8d56fddb7d997dd4b14c4fec4975528d9 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 11 Jan 2024 10:56:47 -0500 Subject: [PATCH 7/8] chore: Update actions logs --- .../core/tagging/import_export/actions.py | 10 +++--- openedx_tagging/core/tagging/models/base.py | 10 ++++-- .../tagging/import_export/test_actions.py | 10 +++--- .../tagging/import_export/test_import_plan.py | 35 +++++++++---------- .../core/tagging/test_models.py | 2 +- .../core/tagging/test_views.py | 5 +-- 6 files changed, 37 insertions(+), 35 deletions(-) diff --git a/openedx_tagging/core/tagging/import_export/actions.py b/openedx_tagging/core/tagging/import_export/actions.py index 9e2ba866b..70061a1be 100644 --- a/openedx_tagging/core/tagging/import_export/actions.py +++ b/openedx_tagging/core/tagging/import_export/actions.py @@ -273,8 +273,8 @@ def __str__(self) -> str: taxonomy_tag = self._get_tag() description_str = _("Update the parent of {tag} from parent {old_parent} to {new_parent}").format( - tag=taxonomy_tag, - old_parent=taxonomy_tag.parent, + tag=taxonomy_tag.display_str(), + old_parent=taxonomy_tag.parent.display_str() if taxonomy_tag.parent else None, new_parent=self.tag.parent_id, ) @@ -336,7 +336,7 @@ class RenameTag(ImportAction): def __str__(self) -> str: taxonomy_tag = self._get_tag() description_str = _("Rename tag value of {tag} to '{new_value}'").format( - tag=taxonomy_tag, + tag=taxonomy_tag.display_str(), new_value=self.tag.value, ) @@ -385,9 +385,7 @@ class DeleteTag(ImportAction): """ def __str__(self) -> str: - if self.tag.id: - return str(_("Delete tag (external_id={external_id})").format(external_id=self.tag.id)) - return str(_("Delete tag (value={value})").format(value=self.tag.value)) + return str(_("Delete tag {tag}").format(tag=self.tag)) name = "delete" diff --git a/openedx_tagging/core/tagging/models/base.py b/openedx_tagging/core/tagging/models/base.py index ff448d7c2..13beeb0a2 100644 --- a/openedx_tagging/core/tagging/models/base.py +++ b/openedx_tagging/core/tagging/models/base.py @@ -94,9 +94,15 @@ def __str__(self): """ User-facing string representation of a Tag. """ + 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.id} / {self.external_id} / {self.value})" - return f"<{self.__class__.__name__}> ({self.id} / {self.value})" + return f"<{self.__class__.__name__}> ({self.external_id} / {self.value})" + return f"<{self.__class__.__name__}> ({self.value})" def get_lineage(self) -> Lineage: """ 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 02028cc72..cd3aec60c 100644 --- a/tests/openedx_tagging/core/tagging/import_export/test_actions.py +++ b/tests/openedx_tagging/core/tagging/import_export/test_actions.py @@ -283,15 +283,15 @@ class TestUpdateParentTag(TestImportActionMixin, TestCase): "tag_4", "tag_3", ( - "Update the parent of (29 / tag_4 / Tag 4) " - "from parent (28 / tag_3 / Tag 3) to 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 (28 / tag_3 / Tag 3) " + "Update the parent of (tag_3 / Tag 3) " "from parent None to tag_2" ) ), @@ -333,8 +333,8 @@ def test_str_no_external_id(self): index=100, ) expected = ( - f"Update the parent of ({tag_2.id} / Tag 6) " - f"from parent ({tag_1.id} / Tag 5) to tag_3" + "Update the parent of (Tag 6) " + "from parent (Tag 5) to tag_3" ) assert str(action) == expected 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 8cf4c7bcb..553d6bc3c 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 (26 / tag_1 / Tag 1) to 'Tag 2'\n" - "#4: Update the parent of (29 / tag_4 / Tag 4) from parent " - " (28 / tag_3 / Tag 3) to 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 (27 / tag_2 / Tag 2) from parent " - " (26 / tag_1 / Tag 1) to None\n" - "#7: Rename tag value of (27 / tag_2 / 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,10 +299,10 @@ 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 (27 / tag_2 / Tag 2) to 'Tag 2 v2'\n" - "#4: Update the parent of (29 / tag_4 / Tag 4) from parent " - " (28 / tag_3 / Tag 3) to tag_1\n" - "#5: Rename tag value of (29 / tag_4 / Tag 4) to 'Tag 4 v2'\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 (29 / tag_4 / Tag 4) from parent " - " (28 / tag_3 / Tag 3) to 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" + "#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" - # "#5: No changes needed for (tag_4 / Tag 4)\n" ), ) @ddt.unpack @@ -340,6 +336,7 @@ def test_plan(self, tags, replace, expected): tags = [TagItem(**tag) for tag in tags] self.import_plan.generate_actions(tags=tags, replace=replace) plan = self.import_plan.plan() + print(plan) assert plan == expected @ddt.data( diff --git a/tests/openedx_tagging/core/tagging/test_models.py b/tests/openedx_tagging/core/tagging/test_models.py index 4b224e9a8..c730b16bc 100644 --- a/tests/openedx_tagging/core/tagging/test_models.py +++ b/tests/openedx_tagging/core/tagging/test_models.py @@ -178,7 +178,7 @@ def test_representations(self): == repr(self.language_taxonomy) == " (-1) Languages" ) - assert str(self.bacteria) == repr(self.bacteria) == " (1 / Bacteria)" + assert str(self.bacteria) == repr(self.bacteria) == " (1) Bacteria" def test_taxonomy_cast(self): for subclass in ( diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index bad9d8f23..1a553525a 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -2457,12 +2457,13 @@ 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" \ + "#6: Create a new tag with values (external_id=tag_4, value=Tag 4, parent_id=None).\n" + print(response.data["plan"]) assert response.data["plan"] == expected_plan self._check_taxonomy_not_changed() From fa42366a09a3d244bcb24403aa916fe52ac75971 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 12 Jan 2024 09:22:20 -0500 Subject: [PATCH 8/8] style: Delete prints --- .../core/tagging/import_export/test_import_plan.py | 1 - tests/openedx_tagging/core/tagging/test_views.py | 1 - 2 files changed, 2 deletions(-) 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 553d6bc3c..2e158748a 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 @@ -336,7 +336,6 @@ def test_plan(self, tags, replace, expected): tags = [TagItem(**tag) for tag in tags] self.import_plan.generate_actions(tags=tags, replace=replace) plan = self.import_plan.plan() - print(plan) assert plan == expected @ddt.data( diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 1a553525a..72352bccb 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -2463,7 +2463,6 @@ def test_import_plan(self, file_format: str) -> None: + "#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" \ + "#6: Create a new tag with values (external_id=tag_4, value=Tag 4, parent_id=None).\n" - print(response.data["plan"]) assert response.data["plan"] == expected_plan self._check_taxonomy_not_changed()