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.

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

feat: REST API for create Library Collection [FC-0062] #35447

Closed
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
7 changes: 4 additions & 3 deletions openedx/core/djangoapps/content/search/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from meilisearch.models.task import TaskInfo
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import LibraryLocatorV2
from openedx_learning.api import authoring as authoring_api
from openedx_learning.api.authoring_models import Collection
from common.djangoapps.student.roles import GlobalStaff
from rest_framework.request import Request
from common.djangoapps.student.role_helpers import get_course_roles
Expand Down Expand Up @@ -298,7 +298,8 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:

# Get the list of collections
status_cb("Counting collections...")
num_collections = authoring_api.get_collections().count()
collections = Collection.objects.filter(enabled=True).select_related("learning_package").order_by('pk')
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: This is also being solved here (#35321) with a different approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rpenido Do you think I should revert this and let #35321 sort it out?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes! I think you don't need to worry about it here. cc @pomegranited

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: 3180f76

num_collections = collections.count()

# Some counters so we can track our progress as indexing progresses:
num_contexts = num_courses + num_libraries + num_collections
Expand Down Expand Up @@ -459,7 +460,7 @@ def index_collection_batch(batch, num_contexts_done) -> int:
return num_contexts_done

# To reduce memory usage on large instances, split up the Collections into pages of 100 collections:
paginator = Paginator(authoring_api.get_collections(enabled=True), 100)
paginator = Paginator(collections, 100)
for p in paginator.page_range:
num_contexts_done = index_collection_batch(paginator.page(p).object_list, num_contexts_done)

Expand Down
2 changes: 2 additions & 0 deletions openedx/core/djangoapps/content_libraries/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ class ContentLibraryMetadata:
key = attr.ib(type=LibraryLocatorV2)
title = attr.ib("")
description = attr.ib("")
learning_package_id = attr.ib(default=None, type=int)
num_blocks = attr.ib(0)
version = attr.ib(0)
type = attr.ib(default=COMPLEX)
Expand Down Expand Up @@ -392,6 +393,7 @@ def get_library(library_key):
return ContentLibraryMetadata(
key=library_key,
title=learning_package.title,
learning_package_id=learning_package.id,
type=ref.type,
description=ref.learning_package.description,
num_blocks=num_blocks,
Expand Down
22 changes: 22 additions & 0 deletions openedx/core/djangoapps/content_libraries/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,3 +245,25 @@ class ContentLibraryBlockImportTaskCreateSerializer(serializers.Serializer):
"""

course_key = CourseKeyField()


class LibraryCollectionCreationSerializer(serializers.Serializer):
"""
Serializer to create a new library collection.
"""

title = serializers.CharField()
description = serializers.CharField(allow_blank=True)


class LibraryCollectionMetadataSerializer(serializers.Serializer):
"""
Serializer for Library Collection Metadata.
"""

# TODO Set this "id" with the LibraryCollectionKey
id = serializers.CharField(read_only=True)
# Rename collection.key to "slug" because "key" is a reserved prop name in React
slug = serializers.CharField(read_only=True)
Copy link
Contributor

@rpenido rpenido Sep 9, 2024

Choose a reason for hiding this comment

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

Do we plan to change it in the model?
Also, this is not being returned from the rest call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That wasn't my plan. I've already updated the code to return the slug in the response: 817e3b4

title = serializers.CharField()
description = serializers.CharField(allow_blank=True)
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from openedx_events.tests.utils import OpenEdxEventsTestMixin
from openedx.core.djangoapps.content_libraries.tests.base import (
ContentLibrariesRestApiTest,
URL_LIB_DETAIL,
URL_BLOCK_METADATA_URL,
URL_BLOCK_RENDER_VIEW,
URL_BLOCK_GET_HANDLER_URL,
Expand Down Expand Up @@ -1063,3 +1064,27 @@ def test_not_found_fails_correctly(self):
self.assertEqual(response.json(), {
'detail': f"XBlock {valid_not_found_key} does not exist, or you don't have permission to view it.",
})


class LibraryCollectionsTestCase(ContentLibrariesRestApiTest):

def test_create_collection(self):
"""
Test collection creation

Tests with some non-ASCII chars in title, description.
"""
url = URL_LIB_DETAIL + 'collections/'
lib = self._create_library(
slug="téstlꜟط", title="A Tést Lꜟطrary", description="Just Téstꜟng", license_type=CC_4_BY,
)
collection = self._api("post", url.format(lib_key=lib["id"]), {
'title': 'A Tést Lꜟطrary Collection',
'description': 'Just Téstꜟng',
}, 200)
expected_data = {
'id': '1',
'title': 'A Tést Lꜟطrary Collection',
'description': 'Just Téstꜟng'
}
self.assertDictContainsEntries(collection, expected_data)
2 changes: 2 additions & 0 deletions openedx/core/djangoapps/content_libraries/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
path('import_blocks/', include(import_blocks_router.urls)),
# Paste contents of clipboard into library
path('paste_clipboard/', views.LibraryPasteClipboardView.as_view()),
# list of library collections / create a library collection
path('collections/', views.LibraryCollectionsRootView.as_view()),
])),
path('blocks/<str:usage_key_str>/', include([
# Get metadata about a specific XBlock in this library, or delete the block:
Expand Down
49 changes: 49 additions & 0 deletions openedx/core/djangoapps/content_libraries/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
from django.contrib.auth import authenticate, get_user_model, login
from django.contrib.auth.models import Group
from django.db.transaction import atomic, non_atomic_requests
from django.db.utils import IntegrityError
from django.http import Http404, HttpResponseBadRequest, JsonResponse
from django.shortcuts import get_object_or_404
from django.urls import reverse
Expand All @@ -80,6 +81,7 @@
from django.views.decorators.clickjacking import xframe_options_exempt
from django.views.decorators.csrf import csrf_exempt
from django.views.generic.base import TemplateResponseMixin, View
from django.utils.text import slugify
from pylti1p3.contrib.django import DjangoCacheDataStorage, DjangoDbToolConf, DjangoMessageLaunch, DjangoOIDCLogin
from pylti1p3.exception import LtiException, OIDCException

Expand All @@ -88,6 +90,7 @@
from organizations.api import ensure_organization
from organizations.exceptions import InvalidOrganizationException
from organizations.models import Organization
from openedx_learning.api import authoring as authoring_api
from rest_framework import status
from rest_framework.exceptions import NotFound, PermissionDenied, ValidationError
from rest_framework.generics import GenericAPIView
Expand All @@ -113,6 +116,8 @@
LibraryXBlockStaticFilesSerializer,
ContentLibraryAddPermissionByEmailSerializer,
LibraryPasteClipboardSerializer,
LibraryCollectionCreationSerializer,
LibraryCollectionMetadataSerializer,
)
import openedx.core.djangoapps.site_configuration.helpers as configuration_helpers
from openedx.core.lib.api.view_utils import view_auth_classes
Expand Down Expand Up @@ -154,6 +159,10 @@ def wrapped_fn(*args, **kwargs):
return wrapped_fn


# Library Views
# =============


class LibraryApiPaginationDocs:
"""
API docs for query params related to paginating ContentLibraryMetadata objects.
Expand Down Expand Up @@ -829,6 +838,46 @@ def retrieve(self, request, lib_key_str, pk=None):
return Response(ContentLibraryBlockImportTaskSerializer(import_task).data)


# Library Collections Views
# =============

@method_decorator(non_atomic_requests, name="dispatch")
@view_auth_classes()
class LibraryCollectionsRootView(GenericAPIView):
"""
Views to list and create library collections.
"""

# TODO Implement list collections

def post(self, request, lib_key_str):
"""
Create a new collection library.
"""
library_key = LibraryLocatorV2.from_string(lib_key_str)
api.require_permission_for_library_key(library_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
serializer = LibraryCollectionCreationSerializer(data=request.data)
serializer.is_valid(raise_exception=True)

library = api.get_library(library_key)
title = serializer.validated_data['title']

key = slugify(title)

try:
result = authoring_api.create_collection(
learning_package_id=library.learning_package_id,
key=key,
title=title,
description=serializer.validated_data['description'],
created_by=request.user.id,
)
except IntegrityError:
return Response(status=status.HTTP_409_CONFLICT)
Copy link
Contributor

Choose a reason for hiding this comment

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

As the user can't directly edit the slug, do you think it is worth an approach like this?

Suggested change
try:
result = authoring_api.create_collection(
learning_package_id=library.learning_package_id,
key=key,
title=title,
description=serializer.validated_data['description'],
created_by=request.user.id,
)
except IntegrityError:
return Response(status=status.HTTP_409_CONFLICT)
attempt = 0
result = None
while not result:
modified_key = key if attempt = 0 else key + str(attempt)
try:
result = authoring_api.create_collection(
learning_package_id=library.learning_package_id,
key=modified_key,
title=title,
description=serializer.validated_data['description'],
created_by=request.user.id,
)
except IntegrityError:
attempt += 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rpenido Thanks! Updated here: 44144c4


return Response(LibraryCollectionMetadataSerializer(result).data)


# LTI 1.3 Views
# =============

Expand Down
2 changes: 1 addition & 1 deletion requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ libsass==0.10.0
click==8.1.6

# pinning this version to avoid updates while the library is being developed
openedx-learning==0.11.2
openedx-learning==0.11.4

# Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise.
openai<=0.28.1
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,7 @@ openedx-filters==1.9.0
# -r requirements/edx/kernel.in
# lti-consumer-xblock
# ora2
openedx-learning==0.11.2
openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/collection-key
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/kernel.in
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/development.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1373,7 +1373,7 @@ openedx-filters==1.9.0
# -r requirements/edx/testing.txt
# lti-consumer-xblock
# ora2
openedx-learning==0.11.2
openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/collection-key
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/doc.txt
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/doc.txt
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,7 @@ openedx-filters==1.9.0
# -r requirements/edx/base.txt
# lti-consumer-xblock
# ora2
openedx-learning==0.11.2
openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/collection-key
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/base.txt
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/kernel.in
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ openedx-calc # Library supporting mathematical calculatio
openedx-django-require
openedx-events # Open edX Events from Hooks Extension Framework (OEP-50)
openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50)
openedx-learning # Open edX Learning core (experimental)
git+https://github.com/open-craft/openedx-learning.git@jill/collection-key#egg=openedx-learning # Open edX Learning core (experimental)
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 version

openedx-mongodbproxy
openedx-django-wiki
path
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/testing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,7 @@ openedx-filters==1.9.0
# -r requirements/edx/base.txt
# lti-consumer-xblock
# ora2
openedx-learning==0.11.2
openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/collection-key
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/base.txt
Expand Down
Loading