diff --git a/openedx_tagging/core/tagging/api.py b/openedx_tagging/core/tagging/api.py index c416f6f09..fe175b160 100644 --- a/openedx_tagging/core/tagging/api.py +++ b/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: """ diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index 114c2ea5f..20057d825 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/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: diff --git a/tests/openedx_tagging/core/tagging/test_api.py b/tests/openedx_tagging/core/tagging/test_api.py index 972159235..712e9bf7f 100644 --- a/tests/openedx_tagging/core/tagging/test_api.py +++ b/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,17 +547,17 @@ 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) @@ -565,18 +565,18 @@ def test_tag_object_limit(self) -> None: # 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) diff --git a/tests/openedx_tagging/core/tagging/test_models.py b/tests/openedx_tagging/core/tagging/test_models.py index f1a611183..76bfb3127 100644 --- a/tests/openedx_tagging/core/tagging/test_models.py +++ b/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)] == [ diff --git a/tests/openedx_tagging/core/tagging/test_system_defined_models.py b/tests/openedx_tagging/core/tagging/test_system_defined_models.py index e75754ac1..7d126f4d2 100644 --- a/tests/openedx_tagging/core/tagging/test_system_defined_models.py +++ b/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