Skip to content

Commit

Permalink
feat!: remove all references to content library types (#35726)
Browse files Browse the repository at this point in the history
At one point, we envisioned having different kinds of libraries, e.g.
a "Video" library would be distinct from a "Problem" library. Later on,
we decided on a more generalized form of Libraries, where any given
library can hold any combination of content–which would then be
organized using collections and tagging.

Due to this shift in perspective, these values haven't actually been
used for a long time. This is just getting rid of them altogether.
  • Loading branch information
Ian2012 authored Oct 25, 2024
1 parent ab6d8cb commit e8cdb06
Show file tree
Hide file tree
Showing 13 changed files with 35 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,6 @@ def setUp(self):
self.store = modulestore()

self.library = library_api.create_library(
library_type=library_api.COMPLEX,
org=Organization.objects.create(name="Test Org", short_name="CL-TEST"),
slug="lib",
title="Library",
Expand Down
1 change: 0 additions & 1 deletion openedx/core/djangoapps/content/search/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ def setUp(self):

# Create a content library:
self.library = library_api.create_library(
library_type=library_api.COMPLEX,
org=OrganizationFactory.create(short_name="org1"),
slug="lib",
title="Library",
Expand Down
33 changes: 4 additions & 29 deletions openedx/core/djangoapps/content_libraries/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@
from xmodule.modulestore.django import modulestore

from . import permissions, tasks
from .constants import ALL_RIGHTS_RESERVED, COMPLEX
from .constants import ALL_RIGHTS_RESERVED
from .models import ContentLibrary, ContentLibraryPermission, ContentLibraryBlockImportTask

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -176,7 +176,6 @@ class ContentLibraryMetadata:
description = attr.ib("")
num_blocks = attr.ib(0)
version = attr.ib(0)
type = attr.ib(default=COMPLEX)
last_published = attr.ib(default=None, type=datetime)
last_draft_created = attr.ib(default=None, type=datetime)
last_draft_created_by = attr.ib(default=None, type=datetime)
Expand Down Expand Up @@ -306,15 +305,13 @@ class LibraryXBlockType:
# ============


def get_libraries_for_user(user, org=None, library_type=None, text_search=None, order=None):
def get_libraries_for_user(user, org=None, text_search=None, order=None):
"""
Return content libraries that the user has permission to view.
"""
filter_kwargs = {}
if org:
filter_kwargs['org__short_name'] = org
if library_type:
filter_kwargs['type'] = library_type
qs = ContentLibrary.objects.filter(**filter_kwargs) \
.select_related('learning_package', 'org') \
.order_by('org__short_name', 'slug')
Expand Down Expand Up @@ -361,7 +358,6 @@ def get_metadata(queryset, text_search=None):
ContentLibraryMetadata(
key=lib.library_key,
title=lib.learning_package.title if lib.learning_package else "",
type=lib.type,
description="",
version=0,
allow_public_learning=lib.allow_public_learning,
Expand Down Expand Up @@ -446,7 +442,6 @@ def get_library(library_key):
return ContentLibraryMetadata(
key=library_key,
title=learning_package.title,
type=ref.type,
description=ref.learning_package.description,
num_blocks=num_blocks,
version=version,
Expand Down Expand Up @@ -474,7 +469,6 @@ def create_library(
allow_public_learning=False,
allow_public_read=False,
library_license=ALL_RIGHTS_RESERVED,
library_type=COMPLEX,
):
"""
Create a new content library.
Expand All @@ -491,8 +485,6 @@ def create_library(
allow_public_read: Allow anyone to view blocks (including source) in Studio?
library_type: Deprecated parameter, not really used. Set to COMPLEX.
Returns a ContentLibraryMetadata instance.
"""
assert isinstance(org, Organization)
Expand All @@ -502,7 +494,6 @@ def create_library(
ref = ContentLibrary.objects.create(
org=org,
slug=slug,
type=library_type,
allow_public_learning=allow_public_learning,
allow_public_read=allow_public_read,
license=library_license,
Expand All @@ -526,7 +517,6 @@ def create_library(
return ContentLibraryMetadata(
key=ref.library_key,
title=title,
type=library_type,
description=description,
num_blocks=0,
version=0,
Expand Down Expand Up @@ -611,7 +601,6 @@ def update_library(
description=None,
allow_public_learning=None,
allow_public_read=None,
library_type=None,
library_license=None,
):
"""
Expand All @@ -621,7 +610,7 @@ def update_library(
A value of None means "don't change".
"""
lib_obj_fields = [
allow_public_learning, allow_public_read, library_type, library_license
allow_public_learning, allow_public_read, library_license
]
lib_obj_changed = any(field is not None for field in lib_obj_fields)
learning_pkg_changed = any(field is not None for field in [title, description])
Expand All @@ -640,10 +629,6 @@ def update_library(
content_lib.allow_public_learning = allow_public_learning
if allow_public_read is not None:
content_lib.allow_public_read = allow_public_read
if library_type is not None:
# TODO: Get rid of this field entirely, and remove library_type
# from any functions that take it as an argument.
content_lib.library_type = library_type
if library_license is not None:
content_lib.library_license = library_license
content_lib.save()
Expand Down Expand Up @@ -856,13 +841,6 @@ def validate_can_add_block_to_library(
"""
assert isinstance(library_key, LibraryLocatorV2)
content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined]
if content_library.type != COMPLEX:
if block_type != content_library.type:
raise IncompatibleTypesError(
_('Block type "{block_type}" is not compatible with library type "{library_type}".').format(
block_type=block_type, library_type=content_library.type,
)
)

# If adding a component would take us over our max, return an error.
component_count = authoring_api.get_all_drafts(content_library.learning_package.id).count()
Expand Down Expand Up @@ -1288,10 +1266,7 @@ def get_allowed_block_types(library_key): # pylint: disable=unused-argument
# TODO: return support status and template options
# See cms/djangoapps/contentstore/views/component.py
block_types = sorted(name for name, class_ in XBlock.load_classes())
lib = get_library(library_key)
if lib.type != COMPLEX:
# Problem and Video libraries only permit XBlocks of the same name.
block_types = (name for name in block_types if name == lib.type)

info = []
for block_type in block_types:
# TODO: unify the contentstore helper with the xblock.api version of
Expand Down
10 changes: 0 additions & 10 deletions openedx/core/djangoapps/content_libraries/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,6 @@
from django.utils.translation import gettext_lazy as _


VIDEO = 'video'
COMPLEX = 'complex'
PROBLEM = 'problem'

LIBRARY_TYPES = (
(VIDEO, _('Video')),
(COMPLEX, _('Complex')),
(PROBLEM, _('Problem')),
)

# These are all the licenses we support so far.
ALL_RIGHTS_RESERVED = ''
CC_4_BY = 'CC:4.0:BY'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Generated by Django 4.2.16 on 2024-10-24 20:21

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('content_libraries', '0010_contentlibrary_learning_package_and_more'),
]

operations = [
migrations.RemoveField(
model_name='contentlibrary',
name='bundle_uuid',
),
migrations.RemoveField(
model_name='contentlibrary',
name='type',
),
]
15 changes: 1 addition & 14 deletions openedx/core/djangoapps/content_libraries/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@

from opaque_keys.edx.django.models import UsageKeyField
from openedx.core.djangoapps.content_libraries.constants import (
LIBRARY_TYPES, COMPLEX, LICENSE_OPTIONS,
ALL_RIGHTS_RESERVED,
LICENSE_OPTIONS, ALL_RIGHTS_RESERVED,
)
from openedx_learning.api.authoring_models import LearningPackage
from organizations.models import Organization # lint-amnesty, pylint: disable=wrong-import-order
Expand Down Expand Up @@ -101,18 +100,6 @@ class ContentLibrary(models.Model):
org = models.ForeignKey(Organization, on_delete=models.PROTECT, null=False)
slug = models.SlugField(allow_unicode=True)

# We no longer use the ``bundle_uuid`` and ``type`` fields, but we'll leave
# them in the model until after the Redwood release, just in case someone
# out there was using v2 libraries. We don't expect this, since it wasn't in
# a usable state, but there's always a chance someone managed to do it and
# is still using it. By keeping the schema backwards compatible, the thought
# is that they would update to the latest version, notice their libraries
# aren't working correctly, and still have the ability to recover their data
# if the code was rolled back.
# TODO: Remove these fields after the Redwood release is cut.
bundle_uuid = models.UUIDField(unique=True, null=True, default=None)
type = models.CharField(max_length=25, default=COMPLEX, choices=LIBRARY_TYPES)

license = models.CharField(max_length=25, default=ALL_RIGHTS_RESERVED, choices=LICENSE_OPTIONS)
learning_package = models.OneToOneField(
LearningPackage,
Expand Down
14 changes: 1 addition & 13 deletions openedx/core/djangoapps/content_libraries/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

from openedx_learning.api.authoring_models import Collection
from openedx.core.djangoapps.content_libraries.constants import (
LIBRARY_TYPES,
COMPLEX,
ALL_RIGHTS_RESERVED,
LICENSE_OPTIONS,
)
Expand All @@ -37,10 +35,8 @@ class ContentLibraryMetadataSerializer(serializers.Serializer):
# begins with 'lib:'. (The numeric ID of the ContentLibrary object in MySQL
# is not exposed via this API.)
id = serializers.CharField(source="key", read_only=True)
type = serializers.ChoiceField(choices=LIBRARY_TYPES, default=COMPLEX)
org = serializers.SlugField(source="key.org")
slug = serializers.CharField(source="key.slug", validators=(validate_unicode_slug, ))
bundle_uuid = serializers.UUIDField(format='hex_verbose', read_only=True)
title = serializers.CharField()
description = serializers.CharField(allow_blank=True)
num_blocks = serializers.IntegerField(read_only=True)
Expand Down Expand Up @@ -86,7 +82,6 @@ class ContentLibraryUpdateSerializer(serializers.Serializer):
description = serializers.CharField()
allow_public_learning = serializers.BooleanField()
allow_public_read = serializers.BooleanField()
type = serializers.ChoiceField(choices=LIBRARY_TYPES)
license = serializers.ChoiceField(choices=LICENSE_OPTIONS)


Expand Down Expand Up @@ -118,7 +113,7 @@ class ContentLibraryPermissionSerializer(ContentLibraryPermissionLevelSerializer
group_name = serializers.CharField(source="group.name", allow_null=True, allow_blank=False, default=None)


class BaseFilterSerializer(serializers.Serializer):
class ContentLibraryFilterSerializer(serializers.Serializer):
"""
Base serializer for filtering listings on the content library APIs.
"""
Expand All @@ -127,13 +122,6 @@ class BaseFilterSerializer(serializers.Serializer):
order = serializers.CharField(default=None, required=False)


class ContentLibraryFilterSerializer(BaseFilterSerializer):
"""
Serializer for filtering library listings.
"""
type = serializers.ChoiceField(choices=LIBRARY_TYPES, default=None, required=False)


class CollectionMetadataSerializer(serializers.Serializer):
"""
Serializer for CollectionMetadata
Expand Down
5 changes: 2 additions & 3 deletions openedx/core/djangoapps/content_libraries/tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from rest_framework.test import APITransactionTestCase, APIClient

from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.content_libraries.constants import COMPLEX, ALL_RIGHTS_RESERVED
from openedx.core.djangoapps.content_libraries.constants import ALL_RIGHTS_RESERVED
from openedx.core.djangolib.testing.utils import skip_unless_cms

# Define the URLs here - don't use reverse() because we want to detect
Expand Down Expand Up @@ -124,7 +124,7 @@ def as_user(self, user):
self.client = old_client # pylint: disable=attribute-defined-outside-init

def _create_library(
self, slug, title, description="", org=None, library_type=COMPLEX,
self, slug, title, description="", org=None,
license_type=ALL_RIGHTS_RESERVED, expect_response=200,
):
""" Create a library """
Expand All @@ -135,7 +135,6 @@ def _create_library(
"slug": slug,
"title": title,
"description": description,
"type": library_type,
"license": license_type,
}, expect_response)

Expand Down
Loading

0 comments on commit e8cdb06

Please sign in to comment.