Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
16 changes: 15 additions & 1 deletion src/openedx_content/applets/backup_restore/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,24 @@ class CollectionSerializer(serializers.Serializer): # pylint: disable=abstract-
Serializer for collections.
"""
title = serializers.CharField(required=True)
key = serializers.CharField(required=True)
# 'collection_code' is the current field name; 'key' is the old name kept for
# back-compat with archives written before the rename. At least one must be present.
collection_code = serializers.CharField(required=False)
key = serializers.CharField(required=False)
description = serializers.CharField(required=True, allow_blank=True)
entities = serializers.ListField(
child=serializers.CharField(),
required=True,
allow_empty=True,
)

def validate(self, attrs):
# Prefer 'collection_code'; fall back to legacy 'key'. Always remove
# both so only the normalised 'collection_code' key reaches the caller.
code = attrs.pop("collection_code", None)
legacy_key = attrs.pop("key", None)
code = code or legacy_key
if not code:
raise serializers.ValidationError("Either 'collection_code' or 'key' is required.")
attrs["collection_code"] = code
return attrs
5 changes: 4 additions & 1 deletion src/openedx_content/applets/backup_restore/toml.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,10 @@ def toml_collection(collection: Collection, entity_keys: list[str]) -> str:

collection_table = tomlkit.table()
collection_table.add("title", collection.title)
collection_table.add("key", collection.key)
# Write both names so that older software (which reads 'key') stays compatible
# with archives produced after the Collection.key -> Collection.collection_code rename.
collection_table.add("key", collection.collection_code)
collection_table.add("collection_code", collection.collection_code)
collection_table.add("description", collection.description)
collection_table.add("created", collection.created)
collection_table.add("entities", entities_array)
Expand Down
4 changes: 2 additions & 2 deletions src/openedx_content/applets/backup_restore/zipper.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ def create_zip(self, path: str) -> None:
collections = self.get_collections()

for collection in collections:
collection_hash_slug = self.get_entity_toml_filename(collection.key)
collection_hash_slug = self.get_entity_toml_filename(collection.collection_code)
collection_toml_file_path = collections_folder / f"{collection_hash_slug}.toml"
entity_keys_related = collection.entities.order_by("key").values_list("key", flat=True)
self.add_file_to_zip(
Expand Down Expand Up @@ -779,7 +779,7 @@ def _save_collections(self, learning_package, collections):
)
collection = collections_api.add_to_collection(
learning_package_id=learning_package.id,
key=collection.key,
collection_code=collection.collection_code,
entities_qset=publishing_api.get_publishable_entities(learning_package.id).filter(key__in=entities)
)

Expand Down
6 changes: 3 additions & 3 deletions src/openedx_content/applets/collections/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ class CollectionAdmin(admin.ModelAdmin):

Allows users to easily disable/enable (aka soft delete and restore) or bulk delete Collections.
"""
readonly_fields = ["key", "learning_package"]
readonly_fields = ["collection_code", "learning_package"]
list_filter = ["enabled"]
list_display = ["key", "title", "enabled", "modified"]
list_display = ["collection_code", "title", "enabled", "modified"]
fieldsets = [
(
"",
{
"fields": ["key", "learning_package"],
"fields": ["collection_code", "learning_package"],
}
),
(
Expand Down
42 changes: 22 additions & 20 deletions src/openedx_content/applets/collections/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@

def create_collection(
learning_package_id: int,
key: str,
collection_code: str,
*,
title: str,
created_by: int | None,
Expand All @@ -44,35 +44,37 @@ def create_collection(
"""
Create a new Collection
"""
collection = Collection.objects.create(
collection = Collection(
learning_package_id=learning_package_id,
key=key,
collection_code=collection_code,
title=title,
created_by_id=created_by,
description=description,
enabled=enabled,
)
collection.full_clean()
collection.save()
return collection


def get_collection(learning_package_id: int, collection_key: str) -> Collection:
def get_collection(learning_package_id: int, collection_code: str) -> Collection:
"""
Get a Collection by ID
"""
return Collection.objects.get_by_key(learning_package_id, collection_key)
return Collection.objects.get_by_code(learning_package_id, collection_code)


def update_collection(
learning_package_id: int,
key: str,
collection_code: str,
*,
title: str | None = None,
description: str | None = None,
) -> Collection:
"""
Update a Collection identified by the learning_package_id + key.
Update a Collection identified by the learning_package_id + collection_code.
"""
collection = get_collection(learning_package_id, key)
collection = get_collection(learning_package_id, collection_code)

# If no changes were requested, there's nothing to update, so just return
# the Collection as-is
Expand All @@ -90,17 +92,17 @@ def update_collection(

def delete_collection(
learning_package_id: int,
key: str,
collection_code: str,
*,
hard_delete=False,
) -> Collection:
"""
Disables or deletes a collection identified by the given learning_package + key.
Disables or deletes a collection identified by the given learning_package + collection_code.

By default (hard_delete=False), the collection is "soft deleted", i.e disabled.
Soft-deleted collections can be re-enabled using restore_collection.
"""
collection = get_collection(learning_package_id, key)
collection = get_collection(learning_package_id, collection_code)

if hard_delete:
collection.delete()
Expand All @@ -112,12 +114,12 @@ def delete_collection(

def restore_collection(
learning_package_id: int,
key: str,
collection_code: str,
) -> Collection:
"""
Undo a "soft delete" by re-enabling a Collection.
"""
collection = get_collection(learning_package_id, key)
collection = get_collection(learning_package_id, collection_code)

collection.enabled = True
collection.save()
Expand All @@ -126,7 +128,7 @@ def restore_collection(

def add_to_collection(
learning_package_id: int,
key: str,
collection_code: str,
entities_qset: QuerySet[PublishableEntity],
created_by: int | None = None,
) -> Collection:
Expand All @@ -146,10 +148,10 @@ def add_to_collection(
if invalid_entity:
raise ValidationError(
f"Cannot add entity {invalid_entity.pk} in learning package {invalid_entity.learning_package_id} "
f"to collection {key} in learning package {learning_package_id}."
f"to collection {collection_code} in learning package {learning_package_id}."
)

collection = get_collection(learning_package_id, key)
collection = get_collection(learning_package_id, collection_code)
collection.entities.add(
*entities_qset.all(),
through_defaults={"created_by_id": created_by},
Expand All @@ -162,7 +164,7 @@ def add_to_collection(

def remove_from_collection(
learning_package_id: int,
key: str,
collection_code: str,
entities_qset: QuerySet[PublishableEntity],
) -> Collection:
"""
Expand All @@ -174,7 +176,7 @@ def remove_from_collection(

Returns the updated Collection.
"""
collection = get_collection(learning_package_id, key)
collection = get_collection(learning_package_id, collection_code)

collection.entities.remove(*entities_qset.all())
collection.modified = datetime.now(tz=timezone.utc)
Expand All @@ -196,15 +198,15 @@ def get_entity_collections(learning_package_id: int, entity_key: str) -> QuerySe
return entity.collections.filter(enabled=True).order_by("pk")


def get_collection_entities(learning_package_id: int, collection_key: str) -> QuerySet[PublishableEntity]:
def get_collection_entities(learning_package_id: int, collection_code: str) -> QuerySet[PublishableEntity]:
"""
Returns a QuerySet of PublishableEntities in a Collection.

This is the same as `collection.entities.all()`
"""
return PublishableEntity.objects.filter(
learning_package_id=learning_package_id,
collections__key=collection_key,
collections__collection_code=collection_code,
).order_by("pk")


Expand Down
21 changes: 11 additions & 10 deletions src/openedx_content/applets/collections/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
from django.db import models
from django.utils.translation import gettext_lazy as _

from openedx_django_lib.fields import MultiCollationTextField, case_insensitive_char_field, key_field
from openedx_django_lib.fields import MultiCollationTextField, case_insensitive_char_field, code_field
from openedx_django_lib.validators import validate_utc_datetime

from ..publishing.models import LearningPackage, PublishableEntity
Expand All @@ -85,12 +85,12 @@ class CollectionManager(models.Manager):
"""
Custom manager for Collection class.
"""
def get_by_key(self, learning_package_id: int, key: str):
def get_by_code(self, learning_package_id: int, collection_code: str):
"""
Get the Collection for the given Learning Package + key.
Get the Collection for the given Learning Package + collection code.
"""
return self.select_related('learning_package') \
.get(learning_package_id=learning_package_id, key=key)
.get(learning_package_id=learning_package_id, collection_code=collection_code)


class Collection(models.Model):
Expand All @@ -105,10 +105,11 @@ class Collection(models.Model):
learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE)

# Every collection is uniquely and permanently identified within its learning package
# by a 'key' that is set during creation. Both will appear in the
# by a 'code' that is set during creation. Both will appear in the
# collection's opaque key:
# e.g. "lib-collection:lib:key" is the opaque key for a library collection.
key = key_field(db_column='_key')
# e.g. "lib-collection:{org_code}:{library_code}:{collection_code}"
# is the opaque key for a library collection.
collection_code = code_field()

title = case_insensitive_char_field(
null=False,
Expand Down Expand Up @@ -170,11 +171,11 @@ class Collection(models.Model):
class Meta:
verbose_name_plural = "Collections"
constraints = [
# Keys are unique within a given LearningPackage.
# Collection codes are unique within a given LearningPackage.
models.UniqueConstraint(
fields=[
"learning_package",
"key",
"collection_code",
],
name="oel_coll_uniq_lp_key",
),
Expand All @@ -196,7 +197,7 @@ def __str__(self) -> str:
"""
User-facing string representation of a Collection.
"""
return f"<{self.__class__.__name__}> (lp:{self.learning_package_id} {self.key}:{self.title})"
return f"<{self.__class__.__name__}> (lp:{self.learning_package_id} {self.collection_code}:{self.title})"


class CollectionPublishableEntity(models.Model):
Expand Down
4 changes: 2 additions & 2 deletions src/openedx_content/applets/components/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ def get_components( # pylint: disable=too-many-positional-arguments

def get_collection_components(
learning_package_id: int,
collection_key: str,
collection_code: str,
) -> QuerySet[Component]:
"""
Returns a QuerySet of Components relating to the PublishableEntities in a Collection.
Expand All @@ -445,7 +445,7 @@ def get_collection_components(
"""
return Component.objects.filter(
learning_package_id=learning_package_id,
publishable_entity__collections__key=collection_key,
publishable_entity__collections__collection_code=collection_code,
).order_by('pk')


Expand Down
14 changes: 7 additions & 7 deletions src/openedx_content/applets/components/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
The model hierarchy is Component -> ComponentVersion -> Media.
The model hierarchy is Component -> ComponentVersion -> Content.

A Component is an entity like a Problem or Video. It has enough information to
identify the Component and determine what the handler should be (e.g. XBlock
Expand All @@ -10,7 +10,7 @@
publishing app. Component maps 1:1 to PublishableEntity and ComponentVersion
maps 1:1 to PublishableEntityVersion.

Multiple pieces of Media may be associated with a ComponentVersion, through
Multiple pieces of Content may be associated with a ComponentVersion, through
the ComponentVersionMedia model. ComponentVersionMedia allows to specify a
ComponentVersion-local identifier. We're using this like a file path by
convention, but it's possible we might want to have special identifiers later.
Expand Down Expand Up @@ -91,7 +91,7 @@ class Component(PublishableEntityMixin):
Problem), but little beyond that.

A Component will have many ComponentVersions over time, and most metadata is
associated with the ComponentVersion model and the Media that
associated with the ComponentVersion model and the Content that
ComponentVersions are associated with.

A Component belongs to exactly one LearningPackage.
Expand Down Expand Up @@ -199,7 +199,7 @@ class ComponentVersion(PublishableEntityVersionMixin):
"""
A particular version of a Component.

This holds the media using a M:M relationship with Media via
This holds the media using a M:M relationship with Content via
ComponentVersionMedia.
"""

Expand All @@ -225,18 +225,18 @@ class Meta:

class ComponentVersionMedia(models.Model):
"""
Determines the Media for a given ComponentVersion.
Determines the Content for a given ComponentVersion.

An ComponentVersion may be associated with multiple pieces of binary data.
For instance, a Video ComponentVersion might be associated with multiple
transcripts in different languages.

When Media is associated with an ComponentVersion, it has some local
When Content is associated with an ComponentVersion, it has some local
key that is unique within the the context of that ComponentVersion. This
allows the ComponentVersion to do things like store an image file and
reference it by a "path" key.

Media is immutable and sharable across multiple ComponentVersions.
Content is immutable and sharable across multiple ComponentVersions.
"""

component_version = models.ForeignKey(ComponentVersion, on_delete=models.CASCADE)
Expand Down
4 changes: 2 additions & 2 deletions src/openedx_content/applets/components/readme.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Components App
==============

The ``components`` app holds the versioned data models for the lowest-level pieces of content that can be stored in Open edX: Components (e.g. XBlocks), as well as the individual pieces of raw data Media that they reference.
The ``components`` app holds the versioned data models for the lowest-level pieces of content that can be stored in Open edX: Components (e.g. XBlocks), as well as the individual pieces of raw data content that they reference.

Motivation
----------
Expand All @@ -18,6 +18,6 @@ Architecture Guidelines

* We're keeping nearly unlimited history, so per-version metadata (i.e. the space/time cost of making a new version) must be kept low.
* Do not assume that all Components will be XBlocks.
* Encourage other apps to make models that join to (and add their own metadata to) Component, ComponentVersion, Media, etc. But it should be done in such a way that this app is not aware of them.
* Encourage other apps to make models that join to (and add their own metadata to) Component, ComponentVersion, Content, etc. But it should be done in such a way that this app is not aware of them.
* Always preserve the most raw version of the data possible, e.g. OLX, even if XBlocks then extend that with more sophisticated data models. At some point those XBlocks will get deprecated/removed, and we will still want to be able to export the raw data.
* Exports should be fast and *not* require the invocation of plugin code.
1 change: 1 addition & 0 deletions src/openedx_content/applets/units/models.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Models that implement units
"""
from __future__ import annotations

from typing import override

Expand Down
Loading