From 85d7ebb2d31505799c17bccc62d8b82d2ff2acbf Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Mon, 31 Mar 2025 11:34:37 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(back)=20check=20title=20unicity=20?= =?UTF-8?q?when=20renaming=20an=20existing=20one?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An item title can be updated. Check title unicity constraint on this case. --- src/backend/core/api/serializers.py | 17 +++++ src/backend/core/models.py | 34 +++++++--- .../core/tests/items/test_api_items_update.py | 62 +++++++++++++++++++ 3 files changed, 103 insertions(+), 10 deletions(-) diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index b549918e..e7465d61 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -255,6 +255,23 @@ class ItemSerializer(ListItemSerializer): def create(self, validated_data): raise NotImplementedError("Create method can not be used.") + def update(self, instance, validated_data): + """Validate that the title is unique in the current path.""" + if ( + instance.title != validated_data.get("title") + and instance.depth > 1 + and instance.is_item_title_existing(validated_data.get("title")) + ): + raise serializers.ValidationError( + { + "title": _( + "An item with this title already exists in the current path." + ) + } + ) + + return super().update(instance, validated_data) + class CreateItemSerializer(ItemSerializer): """Serializer used to create a new item""" diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 28b305bd..7c3804f1 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -417,6 +417,20 @@ class ItemQuerySet(TreeQuerySet): return self.filter(models.Q(link_reach=LinkReachChoices.PUBLIC)) +def _is_item_title_existing(queryset, title): + """Check if the title is unique in the same path.""" + return ( + queryset.filter(title=title) + .filter( + models.Q( + models.Q(deleted_at__isnull=True) + | models.Q(ancestors_deleted_at__isnull=True) + ) + ) + .exists() + ) + + class ItemManager(TreeManager): """Custom manager for Item model overriding create_child method.""" @@ -440,16 +454,9 @@ class ItemManager(TreeManager): if parent.type != ItemTypeChoices.FOLDER: raise ValidationError({"type": _("Only folders can have children.")}) - if ( - self.children(parent.path) - .filter(title=kwargs.get("title")) - .filter( - models.Q( - models.Q(deleted_at__isnull=True) - | models.Q(ancestors_deleted_at__isnull=True) - ) - ) - .exists() + if _is_item_title_existing( + self.children(parent.path), + kwargs.get("title"), ): raise ValidationError( {"title": _("title already exists in this folder.")} @@ -603,6 +610,13 @@ class Item(TreeModel, BaseModel): """Generate a unique cache key for each item.""" return f"item_{self.id!s}_nb_accesses" + def is_item_title_existing(self, title): + """Check if the title is unique in the same path.""" + return _is_item_title_existing( + self.siblings(), + title, + ) + @property def nb_accesses(self): """Calculate the number of accesses.""" diff --git a/src/backend/core/tests/items/test_api_items_update.py b/src/backend/core/tests/items/test_api_items_update.py index 1a7d4db0..60cfaac3 100644 --- a/src/backend/core/tests/items/test_api_items_update.py +++ b/src/backend/core/tests/items/test_api_items_update.py @@ -351,3 +351,65 @@ def test_api_items_update_administrator_or_owner_of_another(via, mock_user_teams other_item.refresh_from_db() other_item_values = serializers.ItemSerializer(instance=other_item).data assert other_item_values == old_item_values + + +def test_api_items_update_title_unique_in_current_path(): + """ + The title of an item should be unique in the current path. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + root = factories.ItemFactory( + title="item1", type=models.ItemTypeChoices.FOLDER, users=[user] + ) + child = factories.ItemFactory( + title="child1", type=models.ItemTypeChoices.FOLDER, parent=root, users=[user] + ) + + factories.ItemFactory( + title="child2", type=models.ItemTypeChoices.FOLDER, parent=root, users=[user] + ) + + # update child1 to rename it to child2 should fails + response = client.put( + f"/api/v1.0/items/{child.id!s}/", + {"title": "child2"}, + format="json", + ) + assert response.status_code == 400 + assert response.json() == { + "title": "An item with this title already exists in the current path." + } + + +def test_api_items_update_title_unique_in_current_path_soft_deleted(): + """ + Reusing a title of a soft-deleted item should be allowed. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + root = factories.ItemFactory( + title="item1", type=models.ItemTypeChoices.FOLDER, users=[user] + ) + child = factories.ItemFactory( + title="child1", type=models.ItemTypeChoices.FOLDER, parent=root, users=[user] + ) + child.soft_delete() + + child2 = factories.ItemFactory( + title="child2", type=models.ItemTypeChoices.FOLDER, parent=root, users=[user] + ) + + # update child 2 title using child 1 title is allowed + response = client.put( + f"/api/v1.0/items/{child2.id!s}/", + {"title": "child1"}, + format="json", + ) + assert response.status_code == 200