From 432e4c22eb18a05eeafb5070a14666680f4c1484 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=B4mulo=20Penido?= <romulo@opencraft.com>
Date: Thu, 21 Sep 2023 09:14:43 -0300
Subject: [PATCH] fix: validate allow_multiple tags

---
 openedx_tagging/core/tagging/models/base.py       | 11 ++++++-----
 tests/openedx_tagging/core/tagging/test_api.py    | 14 +++++++++++++-
 tests/openedx_tagging/core/tagging/test_models.py |  6 +++++-
 tests/openedx_tagging/core/tagging/test_views.py  | 14 ++++++++++++--
 4 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/openedx_tagging/core/tagging/models/base.py b/openedx_tagging/core/tagging/models/base.py
index 876ef439..f102cfce 100644
--- a/openedx_tagging/core/tagging/models/base.py
+++ b/openedx_tagging/core/tagging/models/base.py
@@ -446,25 +446,26 @@ def _find_object_tag_index(tag_ref, object_tags) -> int:
                 -1,
             )
 
-        def _check_current_tag_count() -> None:
+        def _check_new_tag_count(new_tag_count: int) -> None:
             """
-            Checks if the current count of tags for the object is less than 100
+            Checks if the new count of tags for the object is equal or less than 100
             """
             # Exclude self.id to avoid counting the tags that are going to be updated
             current_count = ObjectTag.objects.filter(object_id=object_id).exclude(taxonomy_id=self.id).count()
 
-            if current_count >= 100:
+            if current_count + new_tag_count > 100:
                 raise ValueError(
-                    _(f"Object ({object_id}) already have 100 or more tags.")
+                    _(f"Cannot add more than 100 tags to ({object_id}).")
                 )
 
-        _check_current_tag_count()
 
         if not isinstance(tags, list):
             raise ValueError(_(f"Tags must be a list, not {type(tags).__name__}."))
 
         tags = list(dict.fromkeys(tags))  # Remove duplicates preserving order
 
+        _check_new_tag_count(len(tags))
+
         if not self.allow_multiple and len(tags) > 1:
             raise ValueError(_(f"Taxonomy ({self.id}) only allows one tag per object."))
 
diff --git a/tests/openedx_tagging/core/tagging/test_api.py b/tests/openedx_tagging/core/tagging/test_api.py
index af55af10..3525bc5c 100644
--- a/tests/openedx_tagging/core/tagging/test_api.py
+++ b/tests/openedx_tagging/core/tagging/test_api.py
@@ -606,7 +606,8 @@ def test_tag_object_limit(self) -> None:
                 ["Eubacteria"],
                 "object_1",
             )
-            assert "already have 100 or more tags" in str(exc.exception)
+            assert exc.exception
+            assert "Cannot add more than 100 tags to" in str(exc.exception)
 
         # Updating existing tags should work
         for taxonomy in self.dummy_taxonomies:
@@ -616,6 +617,17 @@ def test_tag_object_limit(self) -> None:
                 "object_1",
             )
 
+        # Updating existing tags adding a new one should fail
+        for taxonomy in self.dummy_taxonomies:
+            with self.assertRaises(ValueError) as exc:
+                tagging_api.tag_object(
+                    taxonomy,
+                    ["New Dummy Tag 1", "New Dummy Tag 2"],
+                    "object_1",
+                )
+                assert exc.exception
+                assert "Cannot add more than 100 tags to" in str(exc.exception)
+
     def test_get_object_tags(self) -> None:
         # Alpha tag has no taxonomy
         alpha = ObjectTag(object_id="abc")
diff --git a/tests/openedx_tagging/core/tagging/test_models.py b/tests/openedx_tagging/core/tagging/test_models.py
index f5ce6520..44905c3a 100644
--- a/tests/openedx_tagging/core/tagging/test_models.py
+++ b/tests/openedx_tagging/core/tagging/test_models.py
@@ -117,7 +117,11 @@ def setUp(self):
 
         self.dummy_taxonomies = []
         for i in range(100):
-            taxonomy = Taxonomy.objects.create(name=f"ZZ Dummy Taxonomy {i:03}", allow_free_text=True)
+            taxonomy = Taxonomy.objects.create(
+                name=f"ZZ Dummy Taxonomy {i:03}",
+                allow_free_text=True,
+                allow_multiple=True
+            )
             ObjectTag.objects.create(
                 object_id="limit_tag_count",
                 taxonomy=taxonomy,
diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py
index 3366b5e9..b7b12cd3 100644
--- a/tests/openedx_tagging/core/tagging/test_views.py
+++ b/tests/openedx_tagging/core/tagging/test_views.py
@@ -469,7 +469,11 @@ def _object_permission(_user, object_id: str) -> bool:
 
         self.dummy_taxonomies = []
         for i in range(100):
-            taxonomy = Taxonomy.objects.create(name=f"Dummy Taxonomy {i}", allow_free_text=True)
+            taxonomy = Taxonomy.objects.create(
+                name=f"Dummy Taxonomy {i}",
+                allow_free_text=True,
+                allow_multiple=True
+            )
             ObjectTag.objects.create(
                 object_id="limit_tag_count",
                 taxonomy=taxonomy,
@@ -771,6 +775,12 @@ def test_tag_object_count_limit(self):
             response = self.client.put(url, {"tags": ["New Tag"]}, format="json")
             assert response.status_code == status.HTTP_200_OK
 
+        # Editing tags adding another one will fail
+        for taxonomy in self.dummy_taxonomies:
+            url = OBJECT_TAGS_UPDATE_URL.format(object_id=object_id, taxonomy_id=taxonomy.pk)
+            response = self.client.put(url, {"tags": ["New Tag 1", "New Tag 2"]}, format="json")
+            assert response.status_code == status.HTTP_400_BAD_REQUEST
+
 
 class TestTaxonomyTagsView(TestTaxonomyViewMixin):
     """
@@ -1085,4 +1095,4 @@ def test_next_children(self):
         )
         assert data.get("count") == self.children_tags_count[0]
         assert data.get("num_pages") == 2
-        assert data.get("current_page") == 2
\ No newline at end of file
+        assert data.get("current_page") == 2