Tag in Collections [FC-0062] (#35383)
* feat: Allow tag in collections * chore: Bump version of opaque-keys
This commit is contained in:
@@ -12,7 +12,7 @@ from opaque_keys.edx.keys import UsageKey
|
||||
import openedx_tagging.core.tagging.api as oel_tagging
|
||||
from django.db.models import Exists, OuterRef, Q, QuerySet
|
||||
from django.utils.timezone import now
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from opaque_keys.edx.keys import CourseKey, LibraryCollectionKey
|
||||
from opaque_keys.edx.locator import LibraryLocatorV2
|
||||
from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy
|
||||
from openedx_tagging.core.tagging.models.utils import TAGS_CSV_SEPARATOR
|
||||
@@ -230,7 +230,7 @@ def generate_csv_rows(object_id, buffer) -> Iterator[str]:
|
||||
"""
|
||||
content_key = get_content_key_from_string(object_id)
|
||||
|
||||
if isinstance(content_key, UsageKey):
|
||||
if isinstance(content_key, (UsageKey, LibraryCollectionKey)):
|
||||
raise ValueError("The object_id must be a CourseKey or a LibraryLocatorV2.")
|
||||
|
||||
all_object_tags, taxonomies = get_all_object_tags(content_key)
|
||||
|
||||
@@ -13,7 +13,7 @@ from urllib.parse import parse_qs, urlparse
|
||||
import ddt
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.core.files.uploadedfile import SimpleUploadedFile
|
||||
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
|
||||
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator, LibraryCollectionLocator
|
||||
from openedx_tagging.core.tagging.models import Tag, Taxonomy
|
||||
from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy
|
||||
from openedx_tagging.core.tagging.rest_api.v1.serializers import TaxonomySerializer
|
||||
@@ -111,6 +111,9 @@ class TestTaxonomyObjectsMixin:
|
||||
)
|
||||
self.libraryA = str(self.content_libraryA.key)
|
||||
|
||||
def _setUp_collection(self):
|
||||
self.collection_key = str(LibraryCollectionLocator(self.content_libraryA.key, 'test-collection'))
|
||||
|
||||
def _setUp_users(self):
|
||||
"""
|
||||
Create users for testing
|
||||
@@ -284,6 +287,7 @@ class TestTaxonomyObjectsMixin:
|
||||
self._setUp_library()
|
||||
self._setUp_users()
|
||||
self._setUp_taxonomies()
|
||||
self._setUp_collection()
|
||||
|
||||
# Clear the rules cache in between test runs to keep query counts consistent.
|
||||
rules_cache.clear()
|
||||
@@ -1653,6 +1657,87 @@ class TestObjectTagViewSet(TestObjectTagMixin, APITestCase):
|
||||
response = self._call_put_request(self.libraryA, taxonomy.pk, ["invalid"])
|
||||
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
||||
|
||||
@ddt.data(
|
||||
# staffA and staff are staff in collection 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_collection(self, user_attr, taxonomy_attr, tag_values, expected_status):
|
||||
"""
|
||||
Tests that only staff and org level users can tag collections
|
||||
"""
|
||||
user = getattr(self, user_attr)
|
||||
self.client.force_authenticate(user=user)
|
||||
|
||||
taxonomy = getattr(self, taxonomy_attr)
|
||||
|
||||
response = self._call_put_request(self.collection_key, taxonomy.pk, tag_values)
|
||||
|
||||
assert response.status_code == expected_status
|
||||
if status.is_success(expected_status):
|
||||
tags_by_taxonomy = response.data[str(self.collection_key)]["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
|
||||
url = OBJECT_TAG_UPDATE_URL.format(object_id=self.collection_key)
|
||||
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_collection_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
|
||||
|
||||
response = self._call_put_request(self.collection_key, disabled_taxonomy.pk, ["Tag 1"])
|
||||
|
||||
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_collection_invalid(self, user_attr, taxonomy_attr):
|
||||
"""
|
||||
Tests that nobody can add invalid tags to a collection using a closed taxonomy
|
||||
"""
|
||||
user = getattr(self, user_attr)
|
||||
self.client.force_authenticate(user=user)
|
||||
|
||||
taxonomy = getattr(self, taxonomy_attr)
|
||||
|
||||
response = self._call_put_request(self.collection_key, taxonomy.pk, ["invalid"])
|
||||
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
||||
|
||||
@ddt.data(
|
||||
("superuser", status.HTTP_200_OK),
|
||||
("staff", status.HTTP_403_FORBIDDEN),
|
||||
@@ -1768,10 +1853,14 @@ class TestObjectTagViewSet(TestObjectTagMixin, APITestCase):
|
||||
@ddt.data(
|
||||
('staff', 'courseA', 8),
|
||||
('staff', 'libraryA', 8),
|
||||
('staff', 'collection_key', 8),
|
||||
("content_creatorA", 'courseA', 11, False),
|
||||
("content_creatorA", 'libraryA', 11, False),
|
||||
("content_creatorA", 'collection_key', 11, False),
|
||||
("library_staffA", 'libraryA', 11, False), # Library users can only view objecttags, not change them?
|
||||
("library_staffA", 'collection_key', 11, False),
|
||||
("library_userA", 'libraryA', 11, False),
|
||||
("library_userA", 'collection_key', 11, False),
|
||||
("instructorA", 'courseA', 11),
|
||||
("course_instructorA", 'courseA', 11),
|
||||
("course_staffA", 'courseA', 11),
|
||||
|
||||
@@ -5,7 +5,7 @@ import tempfile
|
||||
import ddt
|
||||
from django.test.testcases import TestCase
|
||||
from fs.osfs import OSFS
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey
|
||||
from opaque_keys.edx.locator import LibraryLocatorV2
|
||||
from openedx_tagging.core.tagging.models import ObjectTag
|
||||
from organizations.models import Organization
|
||||
@@ -380,6 +380,23 @@ class TestAPIObjectTags(TestGetAllObjectTagsMixin, TestCase):
|
||||
with self.assertNumQueries(31): # TODO why so high?
|
||||
self._test_copy_object_tags(src_key, dst_key, expected_tags)
|
||||
|
||||
def test_tag_collection(self):
|
||||
collection_key = LibraryCollectionKey.from_string("lib-collection:orgA:libX:1")
|
||||
|
||||
api.tag_object(
|
||||
object_id=str(collection_key),
|
||||
taxonomy=self.taxonomy_3,
|
||||
tags=["Tag 3.1"],
|
||||
)
|
||||
|
||||
with self.assertNumQueries(1):
|
||||
object_tags, taxonomies = api.get_all_object_tags(collection_key)
|
||||
|
||||
assert object_tags == {'lib-collection:orgA:libX:1': {3: ['Tag 3.1']}}
|
||||
assert taxonomies == {
|
||||
self.taxonomy_3.id: self.taxonomy_3,
|
||||
}
|
||||
|
||||
|
||||
class TestExportImportTags(TaggedCourseMixin):
|
||||
"""
|
||||
|
||||
@@ -5,11 +5,11 @@ from __future__ import annotations
|
||||
|
||||
from typing import Dict, List, Union
|
||||
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey
|
||||
from opaque_keys.edx.locator import LibraryLocatorV2
|
||||
from openedx_tagging.core.tagging.models import Taxonomy
|
||||
|
||||
ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey]
|
||||
ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey, LibraryCollectionKey]
|
||||
ContextKey = Union[LibraryLocatorV2, CourseKey]
|
||||
|
||||
TagValuesByTaxonomyIdDict = Dict[int, List[str]]
|
||||
|
||||
@@ -5,7 +5,7 @@ from __future__ import annotations
|
||||
|
||||
from edx_django_utils.cache import RequestCache
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey
|
||||
from opaque_keys.edx.locator import LibraryLocatorV2
|
||||
from openedx_tagging.core.tagging.models import Taxonomy
|
||||
from organizations.models import Organization
|
||||
@@ -26,8 +26,14 @@ def get_content_key_from_string(key_str: str) -> ContentKey:
|
||||
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
|
||||
except InvalidKeyError:
|
||||
try:
|
||||
return LibraryCollectionKey.from_string(key_str)
|
||||
except InvalidKeyError as usage_key_error:
|
||||
raise ValueError(
|
||||
"object_id must be one of the following "
|
||||
"keys: CourseKey, LibraryLocatorV2, UsageKey or LibCollectionKey"
|
||||
) from usage_key_error
|
||||
|
||||
|
||||
def get_context_key_from_key(content_key: ContentKey) -> ContextKey:
|
||||
@@ -38,6 +44,10 @@ def get_context_key_from_key(content_key: ContentKey) -> ContextKey:
|
||||
if isinstance(content_key, (CourseKey, LibraryLocatorV2)):
|
||||
return content_key
|
||||
|
||||
# If the content key is a LibraryCollectionKey, return the LibraryLocatorV2
|
||||
if isinstance(content_key, LibraryCollectionKey):
|
||||
return content_key.library_key
|
||||
|
||||
# If the content key is a UsageKey, return the context key
|
||||
context_key = content_key.context_key
|
||||
|
||||
|
||||
@@ -484,7 +484,7 @@ edx-milestones==0.6.0
|
||||
# via -r requirements/edx/kernel.in
|
||||
edx-name-affirmation==2.4.0
|
||||
# via -r requirements/edx/kernel.in
|
||||
edx-opaque-keys[django]==2.10.0
|
||||
edx-opaque-keys[django]==2.11.0
|
||||
# via
|
||||
# -r requirements/edx/kernel.in
|
||||
# -r requirements/edx/paver.txt
|
||||
|
||||
@@ -770,7 +770,7 @@ edx-name-affirmation==2.4.0
|
||||
# via
|
||||
# -r requirements/edx/doc.txt
|
||||
# -r requirements/edx/testing.txt
|
||||
edx-opaque-keys[django]==2.10.0
|
||||
edx-opaque-keys[django]==2.11.0
|
||||
# via
|
||||
# -r requirements/edx/doc.txt
|
||||
# -r requirements/edx/testing.txt
|
||||
|
||||
@@ -564,7 +564,7 @@ edx-milestones==0.6.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-name-affirmation==2.4.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-opaque-keys[django]==2.10.0
|
||||
edx-opaque-keys[django]==2.11.0
|
||||
# via
|
||||
# -r requirements/edx/base.txt
|
||||
# edx-bulk-grades
|
||||
|
||||
@@ -12,7 +12,7 @@ charset-normalizer==2.0.12
|
||||
# requests
|
||||
dnspython==2.6.1
|
||||
# via pymongo
|
||||
edx-opaque-keys==2.10.0
|
||||
edx-opaque-keys==2.11.0
|
||||
# via -r requirements/edx/paver.in
|
||||
idna==3.7
|
||||
# via requests
|
||||
|
||||
@@ -590,7 +590,7 @@ edx-milestones==0.6.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-name-affirmation==2.4.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-opaque-keys[django]==2.10.0
|
||||
edx-opaque-keys[django]==2.11.0
|
||||
# via
|
||||
# -r requirements/edx/base.txt
|
||||
# edx-bulk-grades
|
||||
|
||||
Reference in New Issue
Block a user