diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index ae0f62e3316e..ba767df78dc9 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -29,7 +29,7 @@ from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import load_services_for_studio from openedx.core.lib.xblock_utils import get_aside_from_xblock, is_xblock_aside from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration -from openedx.core.djangoapps.content_tagging.api import get_content_tags +from openedx.core.djangoapps.content_tagging.api import get_object_tags from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -533,7 +533,7 @@ def get_unit_tags(usage_key): which already provides this grouping + sorting logic. """ # Get content tags from content tagging API - content_tags = get_content_tags(usage_key) + content_tags = get_object_tags(str(usage_key)) # Group content tags by taxonomy taxonomy_dict = {} diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 0df9e470035a..0a9d2b1886e1 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -4,12 +4,11 @@ from __future__ import annotations import openedx_tagging.core.tagging.api as oel_tagging -from django.db.models import Q, QuerySet, Exists, OuterRef +from django.db.models import QuerySet, Exists, OuterRef from openedx_tagging.core.tagging.models import Taxonomy from organizations.models import Organization -from .models import ContentObjectTag, TaxonomyOrg -from .types import ContentKey +from .models import TaxonomyOrg def create_taxonomy( @@ -127,56 +126,6 @@ def get_unassigned_taxonomies(enabled=True) -> QuerySet: ) -def get_content_tags( - object_key: ContentKey, - taxonomy_id: int | None = None, -) -> QuerySet: - """ - Generates a list of content tags for a given object. - - Pass taxonomy to limit the returned object_tags to a specific taxonomy. - """ - return oel_tagging.get_object_tags( - object_id=str(object_key), - taxonomy_id=taxonomy_id, - object_tag_class=ContentObjectTag, - ) - - -def tag_content_object( - object_key: ContentKey, - taxonomy: Taxonomy, - tags: list, -) -> QuerySet: - """ - This is the main API to use when you want to add/update/delete tags from a content object (e.g. an XBlock or - course). - - It works one "Taxonomy" at a time, i.e. one field at a time, so you can set call it with taxonomy=Keywords, - tags=["gravity", "newton"] to replace any "Keywords" [Taxonomy] tags on the given content object with "gravity" and - "newton". Doing so to change the "Keywords" Taxonomy won't affect other Taxonomy's tags (other fields) on the - object, such as "Language: [en]" or "Difficulty: [hard]". - - If it's a free-text taxonomy, then the list should be a list of tag values. - Otherwise, it should be a list of existing Tag IDs. - - Raises ValueError if the proposed tags are invalid for this taxonomy. - Preserves existing (valid) tags, adds new (valid) tags, and removes omitted (or invalid) tags. - """ - if not taxonomy.system_defined: - # We require that this taxonomy is linked to the content object's "org" or linked to "all orgs" (None): - org_short_name = object_key.org # type: ignore - if not taxonomy.taxonomyorg_set.filter(Q(org__short_name=org_short_name) | Q(org=None)).exists(): - raise ValueError(f"The specified Taxonomy is not enabled for the content object's org ({org_short_name})") - oel_tagging.tag_object( - taxonomy=taxonomy, - tags=tags, - object_id=str(object_key), - object_tag_class=ContentObjectTag, - ) - return get_content_tags(object_key, taxonomy_id=taxonomy.id) - - # Expose the oel_tagging APIs get_taxonomy = oel_tagging.get_taxonomy @@ -185,3 +134,5 @@ def tag_content_object( get_object_tag_counts = oel_tagging.get_object_tag_counts delete_object_tags = oel_tagging.delete_object_tags resync_object_tags = oel_tagging.resync_object_tags +get_object_tags = oel_tagging.get_object_tags +tag_object = oel_tagging.tag_object diff --git a/openedx/core/djangoapps/content_tagging/migrations/0008_remove_content_object_tag.py b/openedx/core/djangoapps/content_tagging/migrations/0008_remove_content_object_tag.py new file mode 100644 index 000000000000..ecaeec7f8c8b --- /dev/null +++ b/openedx/core/djangoapps/content_tagging/migrations/0008_remove_content_object_tag.py @@ -0,0 +1,16 @@ +# Generated by Django 3.2.23 on 2024-01-30 21:15 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('content_tagging', '0007_system_defined_org_2'), + ] + + operations = [ + migrations.DeleteModel( + name='ContentObjectTag', + ), + ] diff --git a/openedx/core/djangoapps/content_tagging/models/__init__.py b/openedx/core/djangoapps/content_tagging/models/__init__.py index f330b8ae819b..4a25224b4bae 100644 --- a/openedx/core/djangoapps/content_tagging/models/__init__.py +++ b/openedx/core/djangoapps/content_tagging/models/__init__.py @@ -3,5 +3,4 @@ """ from .base import ( TaxonomyOrg, - ContentObjectTag, ) diff --git a/openedx/core/djangoapps/content_tagging/models/base.py b/openedx/core/djangoapps/content_tagging/models/base.py index 77061e46118a..3ce125d99af3 100644 --- a/openedx/core/djangoapps/content_tagging/models/base.py +++ b/openedx/core/djangoapps/content_tagging/models/base.py @@ -3,13 +3,10 @@ """ from __future__ import annotations -from django.core.exceptions import ValidationError from django.db import models from django.db.models import Q, QuerySet from django.utils.translation import gettext as _ -from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import LearningContextKey, UsageKey -from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy +from openedx_tagging.core.tagging.models import Taxonomy from organizations.models import Organization @@ -80,33 +77,3 @@ def get_organizations( if rels.filter(org=None).exists(): return list(Organization.objects.all()) return [rel.org for rel in rels] - - -class ContentObjectTag(ObjectTag): - """ - ObjectTag that requires an LearningContextKey or BlockUsageLocator as the object ID. - """ - - class Meta: - proxy = True - - @property - def object_key(self) -> UsageKey | LearningContextKey: - """ - Returns the object ID parsed as a UsageKey or LearningContextKey. - Raises InvalidKeyError object_id cannot be parse into one of those key types. - - Returns None if there's no object_id. - """ - try: - return LearningContextKey.from_string(self.object_id) - except InvalidKeyError: - return UsageKey.from_string(self.object_id) - - def clean(self): - super().clean() - # Make sure that object_id is a valid key - try: - self.object_key - except InvalidKeyError as err: - raise ValidationError("object_id is not a valid opaque key string.") from err diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index f65be2cd8c16..a9307fc07dbf 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -110,6 +110,7 @@ def _setUp_library(self): title="Library Org A", description="This is a library from Org A", ) + self.libraryA = str(self.content_libraryA.key) def _setUp_users(self): """ @@ -127,10 +128,16 @@ def _setUp_users(self): self.staffA = User.objects.create( username="staffA", - email="userA@example.com", + email="staffA@example.com", ) update_org_role(self.staff, OrgStaffRole, self.staffA, [self.orgA.short_name]) + self.staffB = User.objects.create( + username="staffB", + email="staffB@example.com", + ) + update_org_role(self.staff, OrgStaffRole, self.staffB, [self.orgB.short_name]) + self.content_creatorA = User.objects.create( username="content_creatorA", email="content_creatorA@example.com", @@ -1380,7 +1387,7 @@ class TestObjectTagViewSet(TestObjectTagMixin, APITestCase): """ @ddt.data( - # userA and userS are staff in courseA and can tag using enabled taxonomies + # staffA and staff are staff in courseA and can tag using enabled taxonomies ("user", "tA1", ["Tag 1"], status.HTTP_403_FORBIDDEN), ("staffA", "tA1", ["Tag 1"], status.HTTP_200_OK), ("staff", "tA1", ["Tag 1"], status.HTTP_200_OK), @@ -1465,7 +1472,7 @@ def test_tag_course_invalid(self, user_attr, taxonomy_attr): assert response.status_code == status.HTTP_400_BAD_REQUEST @ddt.data( - # userA and userS are staff in courseA (owner of xblockA) and can tag using any taxonomies + # staffA and staff are staff in courseA (owner of xblockA) and can tag using any taxonomies ("user", "tA1", ["Tag 1"], status.HTTP_403_FORBIDDEN), ("staffA", "tA1", ["Tag 1"], status.HTTP_200_OK), ("staff", "tA1", ["Tag 1"], status.HTTP_200_OK), @@ -1548,6 +1555,129 @@ def test_tag_xblock_invalid(self, user_attr, taxonomy_attr): response = self.client.put(url, {"tags": ["invalid"]}, format="json") assert response.status_code == status.HTTP_400_BAD_REQUEST + @ddt.data( + # staffA and staff are staff in libraryA and can tag using enabled taxonomies + ("user", "tA1", ["Tag 1"], status.HTTP_403_FORBIDDEN), + ("staffA", "tA1", ["Tag 1"], status.HTTP_200_OK), + ("staff", "tA1", ["Tag 1"], status.HTTP_200_OK), + ("user", "tA1", [], status.HTTP_403_FORBIDDEN), + ("staffA", "tA1", [], status.HTTP_200_OK), + ("staff", "tA1", [], status.HTTP_200_OK), + ("user", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_403_FORBIDDEN), + ("staffA", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), + ("staff", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), + ("user", "open_taxonomy", ["tag1"], status.HTTP_403_FORBIDDEN), + ("staffA", "open_taxonomy", ["tag1"], status.HTTP_200_OK), + ("staff", "open_taxonomy", ["tag1"], status.HTTP_200_OK), + ) + @ddt.unpack + def test_tag_library(self, user_attr, taxonomy_attr, tag_values, expected_status): + """ + Tests that only staff and org level users can tag libraries + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + taxonomy = getattr(self, taxonomy_attr) + + url = OBJECT_TAG_UPDATE_URL.format(object_id=self.libraryA, taxonomy_id=taxonomy.pk) + + response = self.client.put(url, {"tags": tag_values}, format="json") + + assert response.status_code == expected_status + if status.is_success(expected_status): + tags_by_taxonomy = response.data[str(self.libraryA)]["taxonomies"] + if tag_values: + response_taxonomy = tags_by_taxonomy[0] + assert response_taxonomy["name"] == taxonomy.name + response_tags = response_taxonomy["tags"] + assert [t["value"] for t in response_tags] == tag_values + else: + assert tags_by_taxonomy == [] # No tags are set from any taxonomy + + # Check that re-fetching the tags returns what we set + new_response = self.client.get(url, format="json") + assert status.is_success(new_response.status_code) + assert new_response.data == response.data + + @ddt.data( + "staffA", + "staff", + ) + def test_tag_library_disabled_taxonomy(self, user_attr): + """ + Nobody can use disabled taxonomies to tag objects + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + disabled_taxonomy = self.tA2 + assert disabled_taxonomy.enabled is False + + url = OBJECT_TAG_UPDATE_URL.format(object_id=self.libraryA, taxonomy_id=disabled_taxonomy.pk) + response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") + + assert response.status_code == status.HTTP_403_FORBIDDEN + + @ddt.data( + ("staffA", "tA1"), + ("staff", "tA1"), + ("staffA", "multiple_taxonomy"), + ("staff", "multiple_taxonomy"), + ) + @ddt.unpack + def test_tag_library_invalid(self, user_attr, taxonomy_attr): + """ + Tests that nobody can add invalid tags to a library using a closed taxonomy + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + taxonomy = getattr(self, taxonomy_attr) + + url = OBJECT_TAG_UPDATE_URL.format(object_id=self.libraryA, taxonomy_id=taxonomy.pk) + + response = self.client.put(url, {"tags": ["invalid"]}, format="json") + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @ddt.data( + ("staff", status.HTTP_200_OK), + ("staffA", status.HTTP_403_FORBIDDEN), + ("staffB", status.HTTP_403_FORBIDDEN), + ) + @ddt.unpack + def test_tag_cross_org(self, user_attr, expected_status): + """ + Tests that only global admins can add a taxonomy from orgA to an object from orgB + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + url = OBJECT_TAG_UPDATE_URL.format(object_id=self.courseB, taxonomy_id=self.tA1.pk) + + response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") + + assert response.status_code == expected_status + + @ddt.data( + ("staff", status.HTTP_200_OK), + ("staffA", status.HTTP_403_FORBIDDEN), + ("staffB", status.HTTP_403_FORBIDDEN), + ) + @ddt.unpack + def test_tag_no_org(self, user_attr, expected_status): + """ + Tests that only global admins can add a no-org taxonomy to an object + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + url = OBJECT_TAG_UPDATE_URL.format(object_id=self.courseA, taxonomy_id=self.ot1.pk) + + response = self.client.put(url, {"tags": []}, format="json") + + assert response.status_code == expected_status + @ddt.data( "courseB", "xblockB", @@ -1581,7 +1711,21 @@ def test_tag_unauthorized(self, objectid_attr): assert response.status_code == status.HTTP_401_UNAUTHORIZED + def test_tag_invalid_object(self): + """ + Test that we cannot tag an object that is not a CouseKey, LibraryLocatorV2 or UsageKey + """ + url = OBJECT_TAG_UPDATE_URL.format(object_id='invalid_key', taxonomy_id=self.tA1.pk) + self.client.force_authenticate(user=self.staff) + + response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") + + assert response.status_code == status.HTTP_403_FORBIDDEN + def test_get_tags(self): + """ + Test that we can get tags for an object + """ self.client.force_authenticate(user=self.staffA) taxonomy = self.multiple_taxonomy tag_values = ["Tag 1", "Tag 2"] @@ -1621,7 +1765,7 @@ def test_object_tags_query_count(self): """ object_key = self.courseA object_id = str(object_key) - tagging_api.tag_content_object(object_key=object_key, taxonomy=self.t1, tags=["anvil", "android"]) + tagging_api.tag_object(object_id=object_id, taxonomy=self.t1, tags=["anvil", "android"]) expected_tags = [ {"value": "android", "lineage": ["ALPHABET", "android"], "can_delete_objecttag": True}, {"value": "anvil", "lineage": ["ALPHABET", "anvil"], "can_delete_objecttag": True}, diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index af6bdbeb9435..fef71eeaf5de 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -6,10 +6,9 @@ import django.contrib.auth.models import openedx_tagging.core.tagging.rules as oel_tagging -from organizations.models import Organization import rules -from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey, UsageKey +from django.db.models import Q +from organizations.models import Organization from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from common.djangoapps.student.models import CourseAccessRole @@ -24,6 +23,7 @@ from openedx.core.djangoapps.content_libraries.api import get_libraries_for_user from .models import TaxonomyOrg +from .utils import get_context_key_from_key_string UserType = Union[django.contrib.auth.models.User, django.contrib.auth.models.AnonymousUser] @@ -220,15 +220,13 @@ def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool: """ if not object_id: return True + try: - usage_key = UsageKey.from_string(object_id) - if not usage_key.course_key.is_course: - raise ValueError("object_id must be from a block or a course") - course_key = usage_key.course_key - except InvalidKeyError: - course_key = CourseKey.from_string(object_id) + context_key = get_context_key_from_key_string(object_id) + except ValueError: + return False - return has_studio_write_access(user, course_key) + return has_studio_write_access(user, context_key) @rules.predicate @@ -251,15 +249,46 @@ def can_view_object_tag_objectid(user: UserType, object_id: str) -> bool: """ if not object_id: raise ValueError("object_id must be provided") + try: - usage_key = UsageKey.from_string(object_id) - if not usage_key.course_key.is_course: - raise ValueError("object_id must be from a block or a course") - course_key = usage_key.course_key - except InvalidKeyError: - course_key = CourseKey.from_string(object_id) + context_key = get_context_key_from_key_string(object_id) + except ValueError: + return False + + return has_studio_read_access(user, context_key) + + +@rules.predicate +def can_change_object_tag( + user: UserType, perm_obj: oel_tagging.ObjectTagPermissionItem | None = None +) -> bool: + """ + Checks if the user has permissions to create or modify tags on the given taxonomy and object_id. + """ + if not oel_tagging.can_change_object_tag(user, perm_obj): + return False + + # The following code allows METHOD permission (PUT) in the viewset for everyone + if perm_obj is None: + return True + + # TaxonomySerializer use this rule passing object_id = "" to check if the user + # can use the taxonomy + if perm_obj.object_id == "": + return True + + # Also skip taxonomy check if the taxonomy is not set + if not perm_obj.taxonomy: + return True + + # Taxonomy admins can tag any object using any taxonomy + if oel_tagging.is_taxonomy_admin(user): + return True + + context_key = get_context_key_from_key_string(perm_obj.object_id) - return has_studio_read_access(user, course_key) + org_short_name = context_key.org + return perm_obj.taxonomy.taxonomyorg_set.filter(Q(org__short_name=org_short_name) | Q(org=None)).exists() @rules.predicate @@ -292,10 +321,11 @@ def can_change_taxonomy_tag(user: UserType, tag: oel_tagging.Tag | None = None) rules.set_perm("oel_tagging.view_tag", rules.always_allow) # ObjectTag -rules.set_perm("oel_tagging.add_object_tag", oel_tagging.can_change_object_tag) -rules.set_perm("oel_tagging.change_objecttag", oel_tagging.can_change_object_tag) -rules.set_perm("oel_tagging.delete_objecttag", oel_tagging.can_change_object_tag) +rules.set_perm("oel_tagging.add_objecttag", can_change_object_tag) +rules.set_perm("oel_tagging.change_objecttag", can_change_object_tag) +rules.set_perm("oel_tagging.delete_objecttag", can_change_object_tag) rules.set_perm("oel_tagging.view_objecttag", oel_tagging.can_view_object_tag) +rules.set_perm("oel_tagging.can_tag_object", can_change_object_tag) # This perms are used in the tagging rest api from openedx_tagging that is exposed in the CMS. They are overridden here # to include Organization and objects permissions. diff --git a/openedx/core/djangoapps/content_tagging/tasks.py b/openedx/core/djangoapps/content_tagging/tasks.py index c566a020cbcb..bd90aad6eaba 100644 --- a/openedx/core/djangoapps/content_tagging/tasks.py +++ b/openedx/core/djangoapps/content_tagging/tasks.py @@ -10,7 +10,7 @@ from django.conf import settings from django.contrib.auth import get_user_model from edx_django_utils.monitoring import set_code_owner_attribute -from opaque_keys.edx.keys import LearningContextKey, UsageKey +from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import LibraryUsageLocatorV2 from openedx_tagging.core.tagging.models import Taxonomy @@ -36,7 +36,7 @@ def _set_initial_language_tag(content_key: ContentKey, lang_code: str) -> None: """ lang_taxonomy = Taxonomy.objects.get(pk=LANGUAGE_TAXONOMY_ID).cast() - if lang_code and not api.get_content_tags(object_key=content_key, taxonomy_id=lang_taxonomy.id): + if lang_code and not api.get_object_tags(object_id=str(content_key), taxonomy_id=lang_taxonomy.id): try: lang_tag = lang_taxonomy.tag_for_external_id(lang_code) except api.oel_tagging.TagDoesNotExist: @@ -47,7 +47,11 @@ def _set_initial_language_tag(content_key: ContentKey, lang_code: str) -> None: default_lang_code, ) lang_tag = lang_taxonomy.tag_for_external_id(default_lang_code) - api.tag_content_object(content_key, lang_taxonomy, [lang_tag.value]) + api.tag_object( + object_id=str(content_key), + taxonomy=lang_taxonomy, + tags=[lang_tag.value], + ) def _delete_tags(content_object: ContentKey) -> None: @@ -65,7 +69,7 @@ def update_course_tags(course_key_str: str) -> bool: course_key_str (str): identifier of the Course """ try: - course_key = LearningContextKey.from_string(course_key_str) + course_key = CourseKey.from_string(course_key_str) log.info("Updating tags for Course with id: %s", course_key) @@ -90,7 +94,7 @@ def delete_course_tags(course_key_str: str) -> bool: course_key_str (str): identifier of the Course """ try: - course_key = LearningContextKey.from_string(course_key_str) + course_key = CourseKey.from_string(course_key_str) log.info("Deleting tags for Course with id: %s", course_key) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 9a297be968b1..807b7d8e1dc6 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -1,7 +1,6 @@ """Tests for the Tagging models""" import ddt from django.test.testcases import TestCase -from opaque_keys.edx.keys import CourseKey, UsageKey from openedx_tagging.core.tagging.models import Tag from organizations.models import Organization @@ -59,41 +58,53 @@ def setUp(self): value="learning", ) # ObjectTags - self.all_orgs_course_tag = api.tag_content_object( - object_key=CourseKey.from_string("course-v1:OeX+DemoX+Demo_Course"), + api.tag_object( + object_id="course-v1:OeX+DemoX+Demo_Course", taxonomy=self.taxonomy_all_orgs, tags=[self.tag_all_orgs.value], + ) + self.all_orgs_course_tag = api.get_object_tags( + object_id="course-v1:OeX+DemoX+Demo_Course", )[0] - self.all_orgs_block_tag = api.tag_content_object( - object_key=UsageKey.from_string( - "block-v1:Ax+DemoX+Demo_Course+type@vertical+block@abcde" - ), + api.tag_object( + object_id="block-v1:Ax+DemoX+Demo_Course+type@vertical+block@abcde", taxonomy=self.taxonomy_all_orgs, tags=[self.tag_all_orgs.value], + ) + self.all_orgs_block_tag = api.get_object_tags( + object_id="block-v1:Ax+DemoX+Demo_Course+type@vertical+block@abcde", )[0] - self.both_orgs_course_tag = api.tag_content_object( - object_key=CourseKey.from_string("course-v1:Ax+DemoX+Demo_Course"), + api.tag_object( + object_id="course-v1:Ax+DemoX+Demo_Course", taxonomy=self.taxonomy_both_orgs, tags=[self.tag_both_orgs.value], + ) + self.both_orgs_course_tag = api.get_object_tags( + object_id="course-v1:Ax+DemoX+Demo_Course", )[0] - self.both_orgs_block_tag = api.tag_content_object( - object_key=UsageKey.from_string( - "block-v1:OeX+DemoX+Demo_Course+type@video+block@abcde" - ), + api.tag_object( + object_id="block-v1:OeX+DemoX+Demo_Course+type@video+block@abcde", taxonomy=self.taxonomy_both_orgs, tags=[self.tag_both_orgs.value], + ) + self.both_orgs_block_tag = api.get_object_tags( + object_id="block-v1:OeX+DemoX+Demo_Course+type@video+block@abcde", )[0] - self.one_org_block_tag = api.tag_content_object( - object_key=UsageKey.from_string( - "block-v1:OeX+DemoX+Demo_Course+type@html+block@abcde" - ), + api.tag_object( + object_id="block-v1:OeX+DemoX+Demo_Course+type@html+block@abcde", taxonomy=self.taxonomy_one_org, tags=[self.tag_one_org.value], + ) + self.one_org_block_tag = api.get_object_tags( + object_id="block-v1:OeX+DemoX+Demo_Course+type@html+block@abcde", )[0] - self.disabled_course_tag = api.tag_content_object( - object_key=CourseKey.from_string("course-v1:Ax+DemoX+Demo_Course"), + api.tag_object( + object_id="course-v1:Ax+DemoX+Demo_Course", taxonomy=self.taxonomy_disabled, tags=[self.tag_disabled.value], + ) + self.disabled_course_tag = api.get_object_tags( + object_id="course-v1:Ax+DemoX+Demo_Course", )[0] self.taxonomy_disabled.enabled = False self.taxonomy_disabled.save() @@ -180,8 +191,8 @@ def test_get_content_tags_valid_for_org( object_tag = getattr(self, object_tag_attr) with self.assertNumQueries(1): valid_tags = list( - api.get_content_tags( - object_key=object_tag.object_key, + api.get_object_tags( + object_id=object_tag.object_id, taxonomy_id=taxonomy_id, ) ) @@ -205,8 +216,8 @@ def test_get_content_tags( object_tag = getattr(self, object_tag_attr) with self.assertNumQueries(1): valid_tags = list( - api.get_content_tags( - object_key=object_tag.object_key, + api.get_object_tags( + object_id=object_tag.object_id, taxonomy_id=taxonomy_id, ) ) @@ -220,31 +231,3 @@ def test_get_tags(self): assert result[0]["_id"] == self.tag_all_orgs.id assert result[0]["parent_value"] is None assert result[0]["depth"] == 0 - - def test_cannot_tag_across_orgs(self): - """ - Ensure that I cannot apply tags from a taxonomy that's linked to another - org. - """ - # This taxonomy is only linked to the "OpenedX org", so it can't be used for "Axim" content. - taxonomy = self.taxonomy_one_org - tags = [self.tag_one_org.value] - with self.assertRaises(ValueError) as exc: - api.tag_content_object( - object_key=CourseKey.from_string("course-v1:Ax+DemoX+Demo_Course"), - taxonomy=taxonomy, - tags=tags, - ) - assert "The specified Taxonomy is not enabled for the content object's org (Ax)" in str(exc.exception) - # But this will work fine: - api.tag_content_object( - object_key=CourseKey.from_string("course-v1:OeX+DemoX+Demo_Course"), - taxonomy=taxonomy, - tags=tags, - ) - # As will this: - api.tag_content_object( - object_key=CourseKey.from_string("course-v1:Ax+DemoX+Demo_Course"), - taxonomy=self.taxonomy_both_orgs, - tags=[self.tag_both_orgs.value], - ) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_rules.py b/openedx/core/djangoapps/content_tagging/tests/test_rules.py index d0aba1f91fb3..8dd8db125843 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_rules.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_rules.py @@ -534,7 +534,7 @@ def test_object_tag_disabled_taxonomy(self, perm, tag_attr): ) @ddt.unpack def test_object_tag_no_orgs(self, perm, tag_attr): - """Only staff & superusers can create/edit an ObjectTag with a no-org Taxonomy""" + """Only superusers can create/edit an ObjectTag with a no-org Taxonomy""" object_tag = getattr(self, tag_attr) assert self.superuser.has_perm(perm, object_tag) assert self.staff.has_perm(perm, object_tag) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py index 7c8527e3b5e8..52a6fcb5b883 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py @@ -73,7 +73,7 @@ def _check_tag(self, object_key: ContentKey, taxonomy_id: int, value: str | None If value is None, check if the ObjectTag does not exists """ - object_tags = list(api.get_content_tags(object_key, taxonomy_id=taxonomy_id)) + object_tags = list(api.get_object_tags(str(object_key), taxonomy_id=taxonomy_id)) object_tag = object_tags[0] if len(object_tags) == 1 else None if len(object_tags) > 1: raise ValueError("Found too many object tags") @@ -166,7 +166,11 @@ def test_update_course(self): # Simulates user manually changing a tag lang_taxonomy = Taxonomy.objects.get(pk=LANGUAGE_TAXONOMY_ID) - api.tag_content_object(course.id, lang_taxonomy, ["Español (España)"]) + api.tag_object( + object_id=str(course.id), + taxonomy=lang_taxonomy, + tags=["Español (España)"] + ) # Update course language course.language = "en" diff --git a/openedx/core/djangoapps/content_tagging/types.py b/openedx/core/djangoapps/content_tagging/types.py index 3a9f6ec54935..44b3fd3e8a59 100644 --- a/openedx/core/djangoapps/content_tagging/types.py +++ b/openedx/core/djangoapps/content_tagging/types.py @@ -3,6 +3,7 @@ """ from typing import Union -from opaque_keys.edx.keys import LearningContextKey, UsageKey +from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.locator import LibraryLocatorV2 -ContentKey = Union[LearningContextKey, UsageKey] +ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey] diff --git a/openedx/core/djangoapps/content_tagging/utils.py b/openedx/core/djangoapps/content_tagging/utils.py new file mode 100644 index 000000000000..7e8efa9a8933 --- /dev/null +++ b/openedx/core/djangoapps/content_tagging/utils.py @@ -0,0 +1,44 @@ +""" +Utils functions for tagging +""" +from __future__ import annotations + +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.locator import LibraryLocatorV2 + +from .types import ContentKey + + +def get_content_key_from_string(key_str: str) -> ContentKey: + """ + Get content key from string + """ + try: + return CourseKey.from_string(key_str) + except InvalidKeyError: + try: + return LibraryLocatorV2.from_string(key_str) + except InvalidKeyError: + try: + return UsageKey.from_string(key_str) + except InvalidKeyError as usage_key_error: + raise ValueError("object_id must be a CourseKey, LibraryLocatorV2 or a UsageKey") from usage_key_error + + +def get_context_key_from_key_string(key_str: str) -> CourseKey | LibraryLocatorV2: + """ + Get context key from an key string + """ + content_key = get_content_key_from_string(key_str) + # If the content key is a CourseKey or a LibraryLocatorV2, return it + if isinstance(content_key, (CourseKey, LibraryLocatorV2)): + return content_key + + # If the content key is a UsageKey, return the context key + context_key = content_key.context_key + + if isinstance(context_key, (CourseKey, LibraryLocatorV2)): + return context_key + + raise ValueError("context must be a CourseKey or a LibraryLocatorV2")