refactor: tag_object param order

openedx_tagging/core/tagging/api.py

@@ -266,11 +266,10 @@ def delete_object_tags(object_id: str):
     tags.delete()
 
 
-# TODO: a function called "tag_object" should take "object_id" as its first parameter, not taxonomy
 def tag_object(
+    object_id: str,
     taxonomy: Taxonomy,
     tags: list[str],
-    object_id: str,
     object_tag_class: type[ObjectTag] = ObjectTag,
 ) -> None:
     """

openedx_tagging/core/tagging/rest_api/v1/views.py

@@ -549,7 +549,7 @@ def update(self, request, *args, **kwargs) -> Response:
 
         tags = body.data.get("tags", [])
         try:
-            tag_object(taxonomy, tags, object_id)
+            tag_object(object_id, taxonomy, tags)
         except TagDoesNotExist as e:
             raise ValidationError from e
         except ValueError as e:

tests/openedx_tagging/core/tagging/test_api.py

@@ -279,8 +279,8 @@ def test_resync_object_tags(self) -> None:
 
         object_id = "obj1"
         # Create some tags:
-        tagging_api.tag_object(self.taxonomy, [self.archaea.value, self.bacteria.value], object_id)  # Regular tags
-        tagging_api.tag_object(open_taxonomy, ["foo", "bar"], object_id)  # Free text tags
+        tagging_api.tag_object(object_id, self.taxonomy, [self.archaea.value, self.bacteria.value])  # Regular tags
+        tagging_api.tag_object(object_id, open_taxonomy, ["foo", "bar"])  # Free text tags
 
         # At first, none of these will be deleted:
         assert [(t.value, t.is_deleted) for t in tagging_api.get_object_tags(object_id, include_deleted=True)] == [
@@ -344,9 +344,9 @@ def test_tag_object(self):
         # Tag and re-tag the object, checking that the expected tags are returned and deleted
         for tag_list in test_tags:
             tagging_api.tag_object(
+                "biology101",
                 self.taxonomy,
                 [t.value for t in tag_list],
-                "biology101",
             )
             # Ensure the expected number of tags exist in the database
             object_tags = tagging_api.get_object_tags("biology101", taxonomy_id=self.taxonomy.id)
@@ -381,43 +381,43 @@ def test_tag_object_free_text(self) -> None:
 
     def test_tag_object_no_multiple(self):
         with pytest.raises(ValueError) as excinfo:
-            tagging_api.tag_object(self.taxonomy, ["A", "B"], "biology101")
+            tagging_api.tag_object("biology101", self.taxonomy, ["A", "B"])
         assert "only allows one tag per object" in str(excinfo.value)
 
     def test_tag_object_invalid_tag(self):
         with pytest.raises(tagging_api.TagDoesNotExist) as excinfo:
-            tagging_api.tag_object(self.taxonomy, ["Eukaryota Xenomorph"], "biology101")
+            tagging_api.tag_object("biology101", self.taxonomy, ["Eukaryota Xenomorph"])
         assert "Tag matching query does not exist." in str(excinfo.value)
 
     def test_tag_object_string(self) -> None:
         with self.assertRaises(ValueError) as exc:
             tagging_api.tag_object(
+                "biology101",
                 self.taxonomy,
                 'string',  # type: ignore[arg-type]
-                "biology101",
             )
         assert "Tags must be a list, not str." in str(exc.exception)
 
     def test_tag_object_integer(self) -> None:
         with self.assertRaises(ValueError) as exc:
             tagging_api.tag_object(
+                "biology101",
                 self.taxonomy,
                 1,  # type: ignore[arg-type]
-                "biology101",
             )
         assert "Tags must be a list, not int." in str(exc.exception)
 
     def test_tag_object_same_id(self) -> None:
         # Tag the object with the same tag twice
         tagging_api.tag_object(
+            "biology101",
             self.taxonomy,
             [self.eubacteria.value],
-            "biology101",
         )
         tagging_api.tag_object(
+            "biology101",
             self.taxonomy,
             [self.eubacteria.value],
-            "biology101",
         )
         object_tags = tagging_api.get_object_tags("biology101")
         assert len(object_tags) == 1
@@ -426,9 +426,9 @@ def test_tag_object_same_id(self) -> None:
     def test_tag_object_same_value(self) -> None:
         # Tag the object with the same value twice
         tagging_api.tag_object(
+            "biology101",
             self.taxonomy,
             [self.eubacteria.value, self.eubacteria.value],
-            "biology101",
         )
         object_tags = tagging_api.get_object_tags("biology101")
         assert len(object_tags) == 1
@@ -440,9 +440,9 @@ def test_tag_object_same_value_multiple_free(self) -> None:
         self.taxonomy.save()
         # Tag the object with the same value twice
         tagging_api.tag_object(
+            "biology101",
             self.taxonomy,
             ["tag1", "tag1"],
-            "biology101",
         )
         object_tags = tagging_api.get_object_tags("biology101")
         assert len(object_tags) == 1
@@ -452,15 +452,15 @@ def test_tag_object_case_id(self) -> None:
         Test that the case of the object_id is preserved.
         """
         tagging_api.tag_object(
+            "biology101",
             self.taxonomy,
             [self.eubacteria.value],
-            "biology101",
         )
 
         tagging_api.tag_object(
+            "BIOLOGY101",
             self.taxonomy,
             [self.archaea.value],
-            "BIOLOGY101",
         )
 
         object_tags_lower = tagging_api.get_object_tags(
@@ -487,7 +487,7 @@ def test_tag_object_language_taxonomy(self) -> None:
         ]
 
         for tags in tags_list:
-            tagging_api.tag_object(self.language_taxonomy, tags, "biology101")
+            tagging_api.tag_object("biology101", self.language_taxonomy, tags)
 
             # Ensure the expected number of tags exist in the database
             object_tags = tagging_api.get_object_tags("biology101")
@@ -505,9 +505,9 @@ def test_tag_object_language_taxonomy(self) -> None:
     def test_tag_object_language_taxonomy_invalid(self) -> None:
         with self.assertRaises(tagging_api.TagDoesNotExist):
             tagging_api.tag_object(
+                "biology101",
                 self.language_taxonomy,
                 ["Spanish"],
-                "biology101",
             )
 
     def test_tag_object_model_system_taxonomy(self) -> None:
@@ -518,7 +518,7 @@ def test_tag_object_model_system_taxonomy(self) -> None:
 
         for user in users:
             tags = [user.username]
-            tagging_api.tag_object(self.user_taxonomy, tags, "biology101")
+            tagging_api.tag_object("biology101", self.user_taxonomy, tags)
 
             # Ensure the expected number of tags exist in the database
             object_tags = tagging_api.get_object_tags("biology101")
@@ -537,7 +537,7 @@ def test_tag_object_model_system_taxonomy(self) -> None:
     def test_tag_object_model_system_taxonomy_invalid(self) -> None:
         tags = ["Invalid id"]
         with self.assertRaises(tagging_api.TagDoesNotExist):
-            tagging_api.tag_object(self.user_taxonomy, tags, "biology101")
+            tagging_api.tag_object("biology101", self.user_taxonomy, tags)
 
     def test_tag_object_limit(self) -> None:
         """
@@ -547,36 +547,36 @@ def test_tag_object_limit(self) -> None:
         # The user can add up to 100 tags to a object
         for taxonomy in dummy_taxonomies:
             tagging_api.tag_object(
+                "object_1",
                 taxonomy,
                 ["Dummy Tag"],
-                "object_1",
             )
 
         # Adding a new tag should fail
         with self.assertRaises(ValueError) as exc:
             tagging_api.tag_object(
+                "object_1",
                 self.taxonomy,
                 ["Eubacteria"],
-                "object_1",
             )
         assert exc.exception
         assert "Cannot add more than 100 tags to" in str(exc.exception)
 
         # Updating existing tags should work
         for taxonomy in dummy_taxonomies:
             tagging_api.tag_object(
+                "object_1",
                 taxonomy,
                 ["New Dummy Tag"],
-                "object_1",
             )
 
         # Updating existing tags adding a new one should fail
         for taxonomy in dummy_taxonomies:
             with self.assertRaises(ValueError) as exc:
                 tagging_api.tag_object(
+                    "object_1",
                     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)

tests/openedx_tagging/core/tagging/test_models.py

@@ -816,10 +816,10 @@ def test_tag_case(self) -> None:
         Test that the object_id is case sensitive.
         """
         # Tag with object_id with lower case
-        api.tag_object(self.taxonomy, [self.chordata.value], object_id="case:id:2")
+        api.tag_object("case:id:2", self.taxonomy, [self.chordata.value])
 
         # Tag with object_id with upper case should not trigger IntegrityError
-        api.tag_object(self.taxonomy, [self.chordata.value], object_id="CASE:id:2")
+        api.tag_object("CASE:id:2", self.taxonomy, [self.chordata.value])
 
         # Create another ObjectTag with lower case object_id should trigger IntegrityError
         with transaction.atomic():
@@ -856,8 +856,8 @@ def test_is_deleted(self):
 
         object_id = "obj1"
         # Create some tags:
-        api.tag_object(self.taxonomy, [self.archaea.value, self.bacteria.value], object_id)  # Regular tags
-        api.tag_object(self.free_text_taxonomy, ["foo", "bar", "tribble"], object_id)  # Free text tags
+        api.tag_object(object_id, self.taxonomy, [self.archaea.value, self.bacteria.value])  # Regular tags
+        api.tag_object(object_id, self.free_text_taxonomy, ["foo", "bar", "tribble"])  # Free text tags
 
         # At first, none of these will be deleted:
         assert [(t.value, t.is_deleted) for t in api.get_object_tags(object_id, include_deleted=True)] == [

tests/openedx_tagging/core/tagging/test_system_defined_models.py

@@ -150,8 +150,8 @@ def test_simple_tag_object(self):
         Test applying tags to an object.
         """
         object1_id, object2_id = "obj1", "obj2"
-        api.tag_object(self.lp_taxonomy, ["p1"], object1_id)
-        api.tag_object(self.lp_taxonomy, ["p1", "p2"], object2_id)
+        api.tag_object(object1_id, self.lp_taxonomy, ["p1"])
+        api.tag_object(object2_id, self.lp_taxonomy, ["p1", "p2"])
         assert [t.value for t in api.get_object_tags(object1_id)] == ["p1"]
         assert [t.value for t in api.get_object_tags(object2_id)] == ["p1", "p2"]
 
@@ -160,7 +160,7 @@ def test_invalid_tag(self):
         Trying to apply an invalid tag raises TagDoesNotExist
         """
         with pytest.raises(api.TagDoesNotExist):
-            api.tag_object(self.lp_taxonomy, ["nonexistent"], "obj1")
+            api.tag_object("obj1", self.lp_taxonomy, ["nonexistent"])
 
     def test_case_insensitive_values(self):
         """
@@ -173,8 +173,8 @@ def test_case_insensitive_values(self):
             allow_multiple=True,
             export_id="learning_package_title_taxonomy",
         )
-        api.tag_object(taxonomy, ["LEARNING PACKAGE 1"], object1_id)
-        api.tag_object(taxonomy, ["Learning Package 1", "LEARNING PACKAGE 2"], object2_id)
+        api.tag_object(object1_id, taxonomy, ["LEARNING PACKAGE 1"])
+        api.tag_object(object2_id, taxonomy, ["Learning Package 1", "LEARNING PACKAGE 2"])
         # But they always get normalized to the case used on the actual model:
         assert [t.value for t in api.get_object_tags(object1_id)] == ["Learning Package 1"]
         assert [t.value for t in api.get_object_tags(object2_id)] == ["Learning Package 1", "Learning Package 2"]
@@ -192,12 +192,12 @@ def test_multiple_taxonomies(self):
         pr_1_id, pr_2_id = "pull_request_1", "pull_request_2"
 
         # Tag PR 1 as having "Author: user1, user2; Reviewer: user2"
-        api.tag_object(self.author_taxonomy, [self.user_1.username, self.user_2.username], pr_1_id)
-        api.tag_object(reviewer_taxonomy, [self.user_2.username], pr_1_id)
+        api.tag_object(pr_1_id, self.author_taxonomy, [self.user_1.username, self.user_2.username])
+        api.tag_object(pr_1_id, reviewer_taxonomy, [self.user_2.username])
 
         # Tag PR 2 as having "Author: user2, reviewer: user1"
-        api.tag_object(self.author_taxonomy, [self.user_2.username], pr_2_id)
-        api.tag_object(reviewer_taxonomy, [self.user_1.username], pr_2_id)
+        api.tag_object(pr_2_id, self.author_taxonomy, [self.user_2.username])
+        api.tag_object(pr_2_id, reviewer_taxonomy, [self.user_1.username])
 
         # Check the results:
         assert [f"{t.taxonomy.name}:{t.value}" for t in api.get_object_tags(pr_1_id)] == [
@@ -217,8 +217,8 @@ def test_tag_object_resync(self):
         """
         # Tag two objects with "Author: user_1"
         object1_id, object2_id, other_obj_id = "obj1", "obj2", "other"
-        api.tag_object(self.author_taxonomy, [self.user_1.username], object1_id)
-        api.tag_object(self.author_taxonomy, [self.user_1.username], object2_id)
+        api.tag_object(object1_id, self.author_taxonomy, [self.user_1.username])
+        api.tag_object(object2_id, self.author_taxonomy, [self.user_1.username])
         initial_object_tags = api.get_object_tags(object1_id)
         assert [t.value for t in initial_object_tags] == [self.user_1.username]
         assert not list(api.get_object_tags(other_obj_id))
@@ -227,7 +227,7 @@ def test_tag_object_resync(self):
         self.user_1.username = new_username
         self.user_1.save()
         # Now we update the tags on just one of the objects:
-        api.tag_object(self.author_taxonomy, [new_username], object1_id)
+        api.tag_object(object1_id, self.author_taxonomy, [new_username])
         assert [t.value for t in api.get_object_tags(object1_id)] == [new_username]
         # But because this will have updated the shared Tag instance, object2 will also be updated as a side effect.
         # This is good - all the objects throughout the system with this tag now show the new value.
@@ -241,12 +241,12 @@ def test_tag_object_delete_user(self):
         """
         # Tag an object with "Author: user_1"
         object_id = "obj123"
-        api.tag_object(self.author_taxonomy, [self.user_1.username], object_id)
+        api.tag_object(object_id, self.author_taxonomy, [self.user_1.username])
         assert [t.value for t in api.get_object_tags(object_id)] == [self.user_1.username]
         # Test after delete user
         self.user_1.delete()
         with self.assertRaises(api.TagDoesNotExist):
-            api.tag_object(self.author_taxonomy, [self.user_1.username], object_id)
+            api.tag_object(object_id, self.author_taxonomy, [self.user_1.username])
 
 
 @ddt.ddt