Skip to content

Commit

Permalink
fix: allow importing a tag with the same value as one that will be de…
Browse files Browse the repository at this point in the history
…leted
  • Loading branch information
rpenido committed Jan 9, 2024
1 parent 7072032 commit af48902
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 48 deletions.
60 changes: 36 additions & 24 deletions openedx_tagging/core/tagging/import_export/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,38 +117,50 @@ def _validate_value(self, indexed_actions) -> ImportActionError | None:
actions
"""
try:
# Validates if exists a tag with the same value on the Taxonomy
taxonomy_tag = self.taxonomy.tag_set.get(value=self.tag.value)
return ImportActionError(
action=self,
message=_(
"Duplicated tag value with tag in database (external_id={external_id})."
).format(external_id=taxonomy_tag.external_id)
)
is_deleted_tag_value = any(
[
self.tag.value == action.tag.value
for action in indexed_actions["delete"]
]
) if "delete" in indexed_actions else False

# If the tag will be deleted, skip the Database validation
if not is_deleted_tag_value:
# Validates if exists a tag with the same value on the Taxonomy
taxonomy_tag = self.taxonomy.tag_set.get(value=self.tag.value)
return ImportActionError(
action=self,
message=_(
"Duplicated tag value with tag in database (external_id={external_id})."
).format(external_id=taxonomy_tag.external_id)
)
except Tag.DoesNotExist:
# Validates value duplication on create actions
pass

# Validates value duplication on create actions
action = self._search_action(
indexed_actions,
CreateTag.name,
"value",
self.tag.value,
)

if not action:
# Validates value duplication on rename actions
action = self._search_action(
indexed_actions,
CreateTag.name,
RenameTag.name,
"value",
self.tag.value,
)

if not action:
# Validates value duplication on rename actions
action = self._search_action(
indexed_actions,
RenameTag.name,
"value",
self.tag.value,
)
if action:
return ImportActionConflict(
action=self,
conflict_action_index=action.index,
message=_("Duplicated tag value."),
)

if action:
return ImportActionConflict(
action=self,
conflict_action_index=action.index,
message=_("Duplicated tag value."),
)
return None


Expand Down
16 changes: 8 additions & 8 deletions openedx_tagging/core/tagging/import_export/import_plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def _init_indexed_actions(self):
for action in available_actions:
self.indexed_actions[action.name] = []

def _build_action(self, action_cls, tag: TagItem):
def _build_action(self, action_cls: type[ImportAction], tag: TagItem):
"""
Build an action with `tag`.
Expand Down Expand Up @@ -139,6 +139,13 @@ def generate_actions(
tag.external_id: tag for tag in self.taxonomy.tag_set.all()
}

for tag in tags:
if tag.id in tags_for_delete:
tags_for_delete.pop(tag.id)

# Delete all not readed tags
self._build_delete_actions(tags_for_delete)

for tag in tags:
has_action = False

Expand All @@ -152,13 +159,6 @@ def generate_actions(
# If it doesn't find an action, a "without changes" is added
self._build_action(WithoutChanges, tag)

if replace and tag.id in tags_for_delete:
tags_for_delete.pop(tag.id)

if replace:
# Delete all not readed tags
self._build_delete_actions(tags_for_delete)

def plan(self) -> str:
"""
Returns an string with the plan and errors
Expand Down
22 changes: 22 additions & 0 deletions tests/openedx_tagging/core/tagging/import_export/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,25 @@ def test_import_removing_with_childs(self) -> None:
ParserFormat.JSON,
replace=True,
)

def test_import_same_value_without_external_id(self) -> None:
new_taxonomy = Taxonomy(name="New taxonomy")
new_taxonomy.save()

# Tag with no external_id
Tag.objects.create(
value="same_value",
taxonomy=new_taxonomy,
)

# Import with one tag with the same value
importFile = BytesIO(json.dumps({"tags": [{"id": "imported_tag", "value": "same_value"}]}).encode())

result, _tasks, _plan = import_export_api.import_tags(
new_taxonomy,
importFile,
ParserFormat.JSON,
replace=True,
)

assert result
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,6 @@ def test_build_delete_actions(self) -> None:
True,
0,
[
{
'name': 'without_changes',
'id': 'tag_4',
},
{
'name': 'delete',
'id': 'tag_1',
Expand All @@ -201,6 +197,10 @@ def test_build_delete_actions(self) -> None:
'name': 'delete',
'id': 'tag_3',
},
{
'name': 'without_changes',
'id': 'tag_4',
},
]
)
)
Expand Down Expand Up @@ -317,12 +317,12 @@ 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"
"#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"
"#5: Delete tag (external_id=tag_3)\n"
"#1: Delete tag (external_id=tag_1)\n"
"#2: Delete tag (external_id=tag_2)\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 <TagItem> (tag_4 / Tag 4)\n"
),
)
@ddt.unpack
Expand Down
12 changes: 6 additions & 6 deletions tests/openedx_tagging/core/tagging/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2457,12 +2457,12 @@ 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: Create a new tag with values (external_id=tag_1, value=Tag 1, parent_id=None).\n" \
+ "#2: Create a new tag with values (external_id=tag_2, value=Tag 2, parent_id=None).\n" \
+ "#3: Create a new tag with values (external_id=tag_3, value=Tag 3, parent_id=None).\n" \
+ "#4: Create a new tag with values (external_id=tag_4, value=Tag 4, parent_id=None).\n" \
+ "#5: Delete tag (external_id=old_tag_1)\n" \
+ "#6: Delete tag (external_id=old_tag_2)\n"
+ "#1: Delete tag (external_id=old_tag_1)\n" \
+ "#2: Delete tag (external_id=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"
assert response.data["plan"] == expected_plan

self._check_taxonomy_not_changed()
Expand Down

0 comments on commit af48902

Please sign in to comment.