Skip to content

Commit 55c3071

Browse files
committed
feat!: Collection.key -> Collection.collection_code (WIP)
Also, standardize internal usage of collection_key to collection_code. This helps clarify that Collection.key is *not* an OpaqueKey, but is rather a local slug, which can be combined with other identifiers to form a fully- qualified LibraryCollectionKey instance. BREAKING CHANGE: Collection.key has been renamed to Collection.collection_code. BREAKING CHANGE: Collection.collection_code now validates that its contents matches '[A-Za-z0-9\-\_\.]+'. This was already effectively true, because LibraryCollectionKey can only be built with slug-like parts, but we now we explicitly raise ValiationError from create_collection. Backup now writes both 'key' and 'collection_code' to collection TOML files, so older software (which only knows 'key') can still read new archives. Restore accepts either field, preferring 'collection_code' and falling back to the legacy 'key'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Part of: #322
1 parent e28fab7 commit 55c3071

File tree

19 files changed

+308
-96
lines changed

19 files changed

+308
-96
lines changed

src/openedx_content/applets/backup_restore/serializers.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,24 @@ class CollectionSerializer(serializers.Serializer): # pylint: disable=abstract-
156156
Serializer for collections.
157157
"""
158158
title = serializers.CharField(required=True)
159-
key = serializers.CharField(required=True)
159+
# 'collection_code' is the current field name; 'key' is the old name kept for
160+
# back-compat with archives written before the rename. At least one must be present.
161+
collection_code = serializers.CharField(required=False)
162+
key = serializers.CharField(required=False)
160163
description = serializers.CharField(required=True, allow_blank=True)
161164
entities = serializers.ListField(
162165
child=serializers.CharField(),
163166
required=True,
164167
allow_empty=True,
165168
)
169+
170+
def validate(self, attrs):
171+
# Prefer 'collection_code'; fall back to legacy 'key'. Always remove
172+
# both so only the normalised 'collection_code' key reaches the caller.
173+
code = attrs.pop("collection_code", None)
174+
legacy_key = attrs.pop("key", None)
175+
code = code or legacy_key
176+
if not code:
177+
raise serializers.ValidationError("Either 'collection_code' or 'key' is required.")
178+
attrs["collection_code"] = code
179+
return attrs

src/openedx_content/applets/backup_restore/toml.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,10 @@ def toml_collection(collection: Collection, entity_keys: list[str]) -> str:
220220

221221
collection_table = tomlkit.table()
222222
collection_table.add("title", collection.title)
223-
collection_table.add("key", collection.key)
223+
# Write both names so that older software (which reads 'key') stays compatible
224+
# with archives produced after the Collection.key -> Collection.collection_code rename.
225+
collection_table.add("key", collection.collection_code)
226+
collection_table.add("collection_code", collection.collection_code)
224227
collection_table.add("description", collection.description)
225228
collection_table.add("created", collection.created)
226229
collection_table.add("entities", entities_array)

src/openedx_content/applets/backup_restore/zipper.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ def create_zip(self, path: str) -> None:
401401
collections = self.get_collections()
402402

403403
for collection in collections:
404-
collection_hash_slug = self.get_entity_toml_filename(collection.key)
404+
collection_hash_slug = self.get_entity_toml_filename(collection.collection_code)
405405
collection_toml_file_path = collections_folder / f"{collection_hash_slug}.toml"
406406
entity_keys_related = collection.entities.order_by("key").values_list("key", flat=True)
407407
self.add_file_to_zip(
@@ -779,7 +779,7 @@ def _save_collections(self, learning_package, collections):
779779
)
780780
collection = collections_api.add_to_collection(
781781
learning_package_id=learning_package.id,
782-
key=collection.key,
782+
collection_code=collection.collection_code,
783783
entities_qset=publishing_api.get_publishable_entities(learning_package.id).filter(key__in=entities)
784784
)
785785

src/openedx_content/applets/collections/admin.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,14 @@ class CollectionAdmin(admin.ModelAdmin):
1313
1414
Allows users to easily disable/enable (aka soft delete and restore) or bulk delete Collections.
1515
"""
16-
readonly_fields = ["key", "learning_package"]
16+
readonly_fields = ["collection_code", "learning_package"]
1717
list_filter = ["enabled"]
18-
list_display = ["key", "title", "enabled", "modified"]
18+
list_display = ["collection_code", "title", "enabled", "modified"]
1919
fieldsets = [
2020
(
2121
"",
2222
{
23-
"fields": ["key", "learning_package"],
23+
"fields": ["collection_code", "learning_package"],
2424
}
2525
),
2626
(

src/openedx_content/applets/collections/api.py

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434

3535
def create_collection(
3636
learning_package_id: int,
37-
key: str,
37+
collection_code: str,
3838
*,
3939
title: str,
4040
created_by: int | None,
@@ -44,35 +44,37 @@ def create_collection(
4444
"""
4545
Create a new Collection
4646
"""
47-
collection = Collection.objects.create(
47+
collection = Collection(
4848
learning_package_id=learning_package_id,
49-
key=key,
49+
collection_code=collection_code,
5050
title=title,
5151
created_by_id=created_by,
5252
description=description,
5353
enabled=enabled,
5454
)
55+
collection.full_clean()
56+
collection.save()
5557
return collection
5658

5759

58-
def get_collection(learning_package_id: int, collection_key: str) -> Collection:
60+
def get_collection(learning_package_id: int, collection_code: str) -> Collection:
5961
"""
6062
Get a Collection by ID
6163
"""
62-
return Collection.objects.get_by_key(learning_package_id, collection_key)
64+
return Collection.objects.get_by_code(learning_package_id, collection_code)
6365

6466

6567
def update_collection(
6668
learning_package_id: int,
67-
key: str,
69+
collection_code: str,
6870
*,
6971
title: str | None = None,
7072
description: str | None = None,
7173
) -> Collection:
7274
"""
73-
Update a Collection identified by the learning_package_id + key.
75+
Update a Collection identified by the learning_package_id + collection_code.
7476
"""
75-
collection = get_collection(learning_package_id, key)
77+
collection = get_collection(learning_package_id, collection_code)
7678

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

9193
def delete_collection(
9294
learning_package_id: int,
93-
key: str,
95+
collection_code: str,
9496
*,
9597
hard_delete=False,
9698
) -> Collection:
9799
"""
98-
Disables or deletes a collection identified by the given learning_package + key.
100+
Disables or deletes a collection identified by the given learning_package + collection_code.
99101
100102
By default (hard_delete=False), the collection is "soft deleted", i.e disabled.
101103
Soft-deleted collections can be re-enabled using restore_collection.
102104
"""
103-
collection = get_collection(learning_package_id, key)
105+
collection = get_collection(learning_package_id, collection_code)
104106

105107
if hard_delete:
106108
collection.delete()
@@ -112,12 +114,12 @@ def delete_collection(
112114

113115
def restore_collection(
114116
learning_package_id: int,
115-
key: str,
117+
collection_code: str,
116118
) -> Collection:
117119
"""
118120
Undo a "soft delete" by re-enabling a Collection.
119121
"""
120-
collection = get_collection(learning_package_id, key)
122+
collection = get_collection(learning_package_id, collection_code)
121123

122124
collection.enabled = True
123125
collection.save()
@@ -126,7 +128,7 @@ def restore_collection(
126128

127129
def add_to_collection(
128130
learning_package_id: int,
129-
key: str,
131+
collection_code: str,
130132
entities_qset: QuerySet[PublishableEntity],
131133
created_by: int | None = None,
132134
) -> Collection:
@@ -146,10 +148,10 @@ def add_to_collection(
146148
if invalid_entity:
147149
raise ValidationError(
148150
f"Cannot add entity {invalid_entity.pk} in learning package {invalid_entity.learning_package_id} "
149-
f"to collection {key} in learning package {learning_package_id}."
151+
f"to collection {collection_code} in learning package {learning_package_id}."
150152
)
151153

152-
collection = get_collection(learning_package_id, key)
154+
collection = get_collection(learning_package_id, collection_code)
153155
collection.entities.add(
154156
*entities_qset.all(),
155157
through_defaults={"created_by_id": created_by},
@@ -162,7 +164,7 @@ def add_to_collection(
162164

163165
def remove_from_collection(
164166
learning_package_id: int,
165-
key: str,
167+
collection_code: str,
166168
entities_qset: QuerySet[PublishableEntity],
167169
) -> Collection:
168170
"""
@@ -174,7 +176,7 @@ def remove_from_collection(
174176
175177
Returns the updated Collection.
176178
"""
177-
collection = get_collection(learning_package_id, key)
179+
collection = get_collection(learning_package_id, collection_code)
178180

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

198200

199-
def get_collection_entities(learning_package_id: int, collection_key: str) -> QuerySet[PublishableEntity]:
201+
def get_collection_entities(learning_package_id: int, collection_code: str) -> QuerySet[PublishableEntity]:
200202
"""
201203
Returns a QuerySet of PublishableEntities in a Collection.
202204
203205
This is the same as `collection.entities.all()`
204206
"""
205207
return PublishableEntity.objects.filter(
206208
learning_package_id=learning_package_id,
207-
collections__key=collection_key,
209+
collections__collection_code=collection_code,
208210
).order_by("pk")
209211

210212

src/openedx_content/applets/collections/models.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@
7070
from django.db import models
7171
from django.utils.translation import gettext_lazy as _
7272

73-
from openedx_django_lib.fields import MultiCollationTextField, case_insensitive_char_field, key_field
73+
from openedx_django_lib.fields import MultiCollationTextField, case_insensitive_char_field, code_field
7474
from openedx_django_lib.validators import validate_utc_datetime
7575

7676
from ..publishing.models import LearningPackage, PublishableEntity
@@ -85,12 +85,12 @@ class CollectionManager(models.Manager):
8585
"""
8686
Custom manager for Collection class.
8787
"""
88-
def get_by_key(self, learning_package_id: int, key: str):
88+
def get_by_code(self, learning_package_id: int, collection_code: str):
8989
"""
90-
Get the Collection for the given Learning Package + key.
90+
Get the Collection for the given Learning Package + collection code.
9191
"""
9292
return self.select_related('learning_package') \
93-
.get(learning_package_id=learning_package_id, key=key)
93+
.get(learning_package_id=learning_package_id, collection_code=collection_code)
9494

9595

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

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

113114
title = case_insensitive_char_field(
114115
null=False,
@@ -170,11 +171,11 @@ class Collection(models.Model):
170171
class Meta:
171172
verbose_name_plural = "Collections"
172173
constraints = [
173-
# Keys are unique within a given LearningPackage.
174+
# Collection codes are unique within a given LearningPackage.
174175
models.UniqueConstraint(
175176
fields=[
176177
"learning_package",
177-
"key",
178+
"collection_code",
178179
],
179180
name="oel_coll_uniq_lp_key",
180181
),
@@ -196,7 +197,7 @@ def __str__(self) -> str:
196197
"""
197198
User-facing string representation of a Collection.
198199
"""
199-
return f"<{self.__class__.__name__}> (lp:{self.learning_package_id} {self.key}:{self.title})"
200+
return f"<{self.__class__.__name__}> (lp:{self.learning_package_id} {self.collection_code}:{self.title})"
200201

201202

202203
class CollectionPublishableEntity(models.Model):

src/openedx_content/applets/components/api.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ def get_components( # pylint: disable=too-many-positional-arguments
436436

437437
def get_collection_components(
438438
learning_package_id: int,
439-
collection_key: str,
439+
collection_code: str,
440440
) -> QuerySet[Component]:
441441
"""
442442
Returns a QuerySet of Components relating to the PublishableEntities in a Collection.
@@ -445,7 +445,7 @@ def get_collection_components(
445445
"""
446446
return Component.objects.filter(
447447
learning_package_id=learning_package_id,
448-
publishable_entity__collections__key=collection_key,
448+
publishable_entity__collections__collection_code=collection_code,
449449
).order_by('pk')
450450

451451

src/openedx_content/applets/components/models.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
"""
2-
The model hierarchy is Component -> ComponentVersion -> Media.
2+
The model hierarchy is Component -> ComponentVersion -> Content.
33
44
A Component is an entity like a Problem or Video. It has enough information to
55
identify the Component and determine what the handler should be (e.g. XBlock
@@ -10,7 +10,7 @@
1010
publishing app. Component maps 1:1 to PublishableEntity and ComponentVersion
1111
maps 1:1 to PublishableEntityVersion.
1212
13-
Multiple pieces of Media may be associated with a ComponentVersion, through
13+
Multiple pieces of Content may be associated with a ComponentVersion, through
1414
the ComponentVersionMedia model. ComponentVersionMedia allows to specify a
1515
ComponentVersion-local identifier. We're using this like a file path by
1616
convention, but it's possible we might want to have special identifiers later.
@@ -91,7 +91,7 @@ class Component(PublishableEntityMixin):
9191
Problem), but little beyond that.
9292
9393
A Component will have many ComponentVersions over time, and most metadata is
94-
associated with the ComponentVersion model and the Media that
94+
associated with the ComponentVersion model and the Content that
9595
ComponentVersions are associated with.
9696
9797
A Component belongs to exactly one LearningPackage.
@@ -199,7 +199,7 @@ class ComponentVersion(PublishableEntityVersionMixin):
199199
"""
200200
A particular version of a Component.
201201
202-
This holds the media using a M:M relationship with Media via
202+
This holds the media using a M:M relationship with Content via
203203
ComponentVersionMedia.
204204
"""
205205

@@ -225,18 +225,18 @@ class Meta:
225225

226226
class ComponentVersionMedia(models.Model):
227227
"""
228-
Determines the Media for a given ComponentVersion.
228+
Determines the Content for a given ComponentVersion.
229229
230230
An ComponentVersion may be associated with multiple pieces of binary data.
231231
For instance, a Video ComponentVersion might be associated with multiple
232232
transcripts in different languages.
233233
234-
When Media is associated with an ComponentVersion, it has some local
234+
When Content is associated with an ComponentVersion, it has some local
235235
key that is unique within the the context of that ComponentVersion. This
236236
allows the ComponentVersion to do things like store an image file and
237237
reference it by a "path" key.
238238
239-
Media is immutable and sharable across multiple ComponentVersions.
239+
Content is immutable and sharable across multiple ComponentVersions.
240240
"""
241241

242242
component_version = models.ForeignKey(ComponentVersion, on_delete=models.CASCADE)

src/openedx_content/applets/components/readme.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Components App
22
==============
33

4-
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.
4+
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.
55

66
Motivation
77
----------
@@ -18,6 +18,6 @@ Architecture Guidelines
1818

1919
* 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.
2020
* Do not assume that all Components will be XBlocks.
21-
* 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.
21+
* 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.
2222
* 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.
2323
* Exports should be fast and *not* require the invocation of plugin code.

src/openedx_content/applets/units/models.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
Models that implement units
33
"""
4+
from __future__ import annotations
45

56
from typing import override
67

0 commit comments

Comments
 (0)