Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tag in Collections [FC-0062] #35383

Merged
merged 14 commits into from
Sep 13, 2024
Merged

Tag in Collections [FC-0062] #35383

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2765132
chore: Use opencraft branch of opaque-keys
ChrisChV Aug 27, 2024
b2e9ca7
feat: Allow tag collections
ChrisChV Aug 28, 2024
98ffb23
style: Fix lint
ChrisChV Aug 28, 2024
2849d4b
refactor: Update LibCollectionKey to LibraryCollectionKey
ChrisChV Aug 30, 2024
af1665b
fix: Type check fails
ChrisChV Aug 30, 2024
5eac513
style: Clean code in get_context_key_from_key
ChrisChV Aug 31, 2024
278ab45
test: Check permissions for tag collections
ChrisChV Aug 31, 2024
4bc07a0
refactor: Verify LibraryCollectionKey in generate_csv_rows
ChrisChV Aug 31, 2024
55185d5
Merge branch 'master' into chris/FAL-3785-tags-to-collections
ChrisChV Sep 4, 2024
b6e3ee3
Merge branch 'master' into chris/FAL-3785-tags-to-collections
ChrisChV Sep 4, 2024
c01c51f
chore: Bump version of opaque-keys
ChrisChV Sep 12, 2024
120900d
chore: Bumpb version of opaque-keys
ChrisChV Sep 12, 2024
5554b9b
Merge branch 'master' into chris/FAL-3785-tags-to-collections
ChrisChV Sep 13, 2024
44bb7bb
Merge branch 'master' into chris/FAL-3785-tags-to-collections
ChrisChV Sep 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/content_tagging/helpers/objecttag_export_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from typing import Any, Callable, Iterator, Union

from attrs import define
from opaque_keys.edx.keys import CourseKey, UsageKey
from opaque_keys.edx.keys import CourseKey, UsageKey, LibCollectionKey
from opaque_keys.edx.locator import LibraryLocatorV2
from xblock.core import XBlock

Expand Down Expand Up @@ -135,7 +135,7 @@ def _get_library_block_tagged_object(


def build_object_tree_with_objecttags(
content_key: LibraryLocatorV2 | CourseKey,
content_key: LibraryLocatorV2 | CourseKey | LibCollectionKey,
object_tag_cache: TagValuesByObjectIdDict,
) -> TaggedContent:
"""
Expand Down
19 changes: 18 additions & 1 deletion openedx/core/djangoapps/content_tagging/tests/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
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, LibCollectionKey
from opaque_keys.edx.locator import LibraryLocatorV2
from openedx_tagging.core.tagging.models import ObjectTag
from organizations.models import Organization
Expand Down Expand Up @@ -380,6 +380,23 @@ def test_copy_cross_org_tags(self):
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 = LibCollectionKey.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):
"""
Expand Down
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/content_tagging/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@

from typing import Dict, List, Union

from opaque_keys.edx.keys import CourseKey, UsageKey
from opaque_keys.edx.keys import CourseKey, UsageKey, LibCollectionKey
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, LibCollectionKey]
ContextKey = Union[LibraryLocatorV2, CourseKey]

TagValuesByTaxonomyIdDict = Dict[int, List[str]]
Expand Down
9 changes: 6 additions & 3 deletions openedx/core/djangoapps/content_tagging/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

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, LibCollectionKey
from opaque_keys.edx.locator import LibraryLocatorV2
from openedx_tagging.core.tagging.models import Taxonomy
from organizations.models import Organization
Expand All @@ -26,8 +26,11 @@ 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 LibCollectionKey.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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
raise ValueError("object_id must be a CourseKey, LibraryLocatorV2 or a UsageKey") from usage_key_error
raise ValueError("object_id must be one of the following keys: CourseKey, LibraryLocatorV2, UsageKey or LibCollectionKey") from usage_key_error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated here: 2849d4b



def get_context_key_from_key(content_key: ContentKey) -> ContextKey:
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/base.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ edx-milestones==0.6.0
# via -r requirements/edx/kernel.in
edx-name-affirmation==2.3.7
# via -r requirements/edx/kernel.in
edx-opaque-keys[django]==2.10.0
edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key
# via
# -r requirements/edx/kernel.in
# -r requirements/edx/paver.txt
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/development.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,7 @@ edx-name-affirmation==2.3.7
# via
# -r requirements/edx/doc.txt
# -r requirements/edx/testing.txt
edx-opaque-keys[django]==2.10.0
edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key
# via
# -r requirements/edx/doc.txt
# -r requirements/edx/testing.txt
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/doc.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ edx-milestones==0.6.0
# via -r requirements/edx/base.txt
edx-name-affirmation==2.3.7
# via -r requirements/edx/base.txt
edx-opaque-keys[django]==2.10.0
edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key
# via
# -r requirements/edx/base.txt
# edx-bulk-grades
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/kernel.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ edx-event-bus-kafka>=5.6.0 # Kafka implementation of event bus
edx-event-bus-redis
edx-milestones
edx-name-affirmation
edx-opaque-keys
git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key#egg=edx-opaque-keys
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Bump opaque-keys version

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

edx-organizations
edx-proctoring>=2.0.1
# using hash to support django42
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/testing.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ edx-milestones==0.6.0
# via -r requirements/edx/base.txt
edx-name-affirmation==2.3.7
# via -r requirements/edx/base.txt
edx-opaque-keys[django]==2.10.0
edx-opaque-keys[django] @ git+https://github.com/open-craft/opaque-keys.git@chris/FAL-3785-collections-key
# via
# -r requirements/edx/base.txt
# edx-bulk-grades
Expand Down
Loading