Skip to content

Commit 568f1fe

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. Part of: #322
1 parent 7e5dc4e commit 568f1fe

17 files changed

Lines changed: 208 additions & 85 deletions

File tree

src/openedx_content/applets/backup_restore/serializers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ 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+
key = serializers.CharField(required=True, source="collection_code")
160160
description = serializers.CharField(required=True, allow_blank=True)
161161
entities = serializers.ListField(
162162
child=serializers.CharField(),

src/openedx_content/applets/backup_restore/toml.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ 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+
collection_table.add("key", collection.collection_code)
224224
collection_table.add("description", collection.description)
225225
collection_table.add("created", collection.created)
226226
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
@@ -400,7 +400,7 @@ def create_zip(self, path: str) -> None:
400400
collections = self.get_collections()
401401

402402
for collection in collections:
403-
collection_hash_slug = self.get_entity_toml_filename(collection.key)
403+
collection_hash_slug = self.get_entity_toml_filename(collection.collection_code)
404404
collection_toml_file_path = collections_folder / f"{collection_hash_slug}.toml"
405405
entity_keys_related = collection.entities.order_by("key").values_list("key", flat=True)
406406
self.add_file_to_zip(
@@ -778,7 +778,7 @@ def _save_collections(self, learning_package, collections):
778778
)
779779
collection = collections_api.add_to_collection(
780780
learning_package_id=learning_package.id,
781-
key=collection.key,
781+
collection_code=collection.collection_code,
782782
entities_qset=publishing_api.get_publishable_entities(learning_package.id).filter(key__in=entities)
783783
)
784784

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: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333

3434
def create_collection(
3535
learning_package_id: int,
36-
key: str,
36+
collection_code: str,
3737
*,
3838
title: str,
3939
created_by: int | None,
@@ -43,35 +43,37 @@ def create_collection(
4343
"""
4444
Create a new Collection
4545
"""
46-
collection = Collection.objects.create(
46+
collection = Collection(
4747
learning_package_id=learning_package_id,
48-
key=key,
48+
collection_code=collection_code,
4949
title=title,
5050
created_by_id=created_by,
5151
description=description,
5252
enabled=enabled,
5353
)
54+
collection.full_clean()
55+
collection.save()
5456
return collection
5557

5658

57-
def get_collection(learning_package_id: int, collection_key: str) -> Collection:
59+
def get_collection(learning_package_id: int, collection_code: str) -> Collection:
5860
"""
5961
Get a Collection by ID
6062
"""
61-
return Collection.objects.get_by_key(learning_package_id, collection_key)
63+
return Collection.objects.get_by_code(learning_package_id, collection_code)
6264

6365

6466
def update_collection(
6567
learning_package_id: int,
66-
key: str,
68+
collection_code: str,
6769
*,
6870
title: str | None = None,
6971
description: str | None = None,
7072
) -> Collection:
7173
"""
72-
Update a Collection identified by the learning_package_id + key.
74+
Update a Collection identified by the learning_package_id + collection_code.
7375
"""
74-
collection = get_collection(learning_package_id, key)
76+
collection = get_collection(learning_package_id, collection_code)
7577

7678
# If no changes were requested, there's nothing to update, so just return
7779
# the Collection as-is
@@ -89,17 +91,17 @@ def update_collection(
8991

9092
def delete_collection(
9193
learning_package_id: int,
92-
key: str,
94+
collection_code: str,
9395
*,
9496
hard_delete=False,
9597
) -> Collection:
9698
"""
97-
Disables or deletes a collection identified by the given learning_package + key.
99+
Disables or deletes a collection identified by the given learning_package + collection_code.
98100
99101
By default (hard_delete=False), the collection is "soft deleted", i.e disabled.
100102
Soft-deleted collections can be re-enabled using restore_collection.
101103
"""
102-
collection = get_collection(learning_package_id, key)
104+
collection = get_collection(learning_package_id, collection_code)
103105

104106
if hard_delete:
105107
collection.delete()
@@ -111,12 +113,12 @@ def delete_collection(
111113

112114
def restore_collection(
113115
learning_package_id: int,
114-
key: str,
116+
collection_code: str,
115117
) -> Collection:
116118
"""
117119
Undo a "soft delete" by re-enabling a Collection.
118120
"""
119-
collection = get_collection(learning_package_id, key)
121+
collection = get_collection(learning_package_id, collection_code)
120122

121123
collection.enabled = True
122124
collection.save()
@@ -125,7 +127,7 @@ def restore_collection(
125127

126128
def add_to_collection(
127129
learning_package_id: int,
128-
key: str,
130+
collection_code: str,
129131
entities_qset: QuerySet[PublishableEntity],
130132
created_by: int | None = None,
131133
) -> Collection:
@@ -145,10 +147,10 @@ def add_to_collection(
145147
if invalid_entity:
146148
raise ValidationError(
147149
f"Cannot add entity {invalid_entity.pk} in learning package {invalid_entity.learning_package_id} "
148-
f"to collection {key} in learning package {learning_package_id}."
150+
f"to collection {collection_code} in learning package {learning_package_id}."
149151
)
150152

151-
collection = get_collection(learning_package_id, key)
153+
collection = get_collection(learning_package_id, collection_code)
152154
collection.entities.add(
153155
*entities_qset.all(),
154156
through_defaults={"created_by_id": created_by},
@@ -161,7 +163,7 @@ def add_to_collection(
161163

162164
def remove_from_collection(
163165
learning_package_id: int,
164-
key: str,
166+
collection_code: str,
165167
entities_qset: QuerySet[PublishableEntity],
166168
) -> Collection:
167169
"""
@@ -173,7 +175,7 @@ def remove_from_collection(
173175
174176
Returns the updated Collection.
175177
"""
176-
collection = get_collection(learning_package_id, key)
178+
collection = get_collection(learning_package_id, collection_code)
177179

178180
collection.entities.remove(*entities_qset.all())
179181
collection.modified = datetime.now(tz=timezone.utc)

src/openedx_content/applets/collections/models.py

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@
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 (
74+
MultiCollationTextField, case_insensitive_char_field, code_field
75+
)
7476
from openedx_django_lib.validators import validate_utc_datetime
7577

7678
from ..publishing.models import LearningPackage, PublishableEntity
@@ -85,12 +87,12 @@ class CollectionManager(models.Manager):
8587
"""
8688
Custom manager for Collection class.
8789
"""
88-
def get_by_key(self, learning_package_id: int, key: str):
90+
def get_by_code(self, learning_package_id: int, collection_code: str):
8991
"""
90-
Get the Collection for the given Learning Package + key.
92+
Get the Collection for the given Learning Package + collection code.
9193
"""
9294
return self.select_related('learning_package') \
93-
.get(learning_package_id=learning_package_id, key=key)
95+
.get(learning_package_id=learning_package_id, collection_code=collection_code)
9496

9597

9698
class Collection(models.Model):
@@ -105,10 +107,11 @@ class Collection(models.Model):
105107
learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE)
106108

107109
# 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
110+
# by a 'code' that is set during creation. Both will appear in the
109111
# 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')
112+
# e.g. "lib-collection:{org_code}:{library_code}:{collection_code}"
113+
# is the opaque key for a library collection.
114+
collection_code = code_field()
112115

113116
title = case_insensitive_char_field(
114117
null=False,
@@ -170,11 +173,11 @@ class Collection(models.Model):
170173
class Meta:
171174
verbose_name_plural = "Collections"
172175
constraints = [
173-
# Keys are unique within a given LearningPackage.
176+
# Collection codes are unique within a given LearningPackage.
174177
models.UniqueConstraint(
175178
fields=[
176179
"learning_package",
177-
"key",
180+
"collection_code",
178181
],
179182
name="oel_coll_uniq_lp_key",
180183
),
@@ -196,7 +199,7 @@ def __str__(self) -> str:
196199
"""
197200
User-facing string representation of a Collection.
198201
"""
199-
return f"<{self.__class__.__name__}> (lp:{self.learning_package_id} {self.key}:{self.title})"
202+
return f"<{self.__class__.__name__}> (lp:{self.learning_package_id} {self.collection_code}:{self.title})"
200203

201204

202205
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/publishing/api.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1745,7 +1745,7 @@ def get_containers(
17451745

17461746
def get_collection_containers(
17471747
learning_package_id: int,
1748-
collection_key: str,
1748+
collection_code: str,
17491749
) -> QuerySet[Container]:
17501750
"""
17511751
Returns a QuerySet of Containers relating to the PublishableEntities in a Collection.
@@ -1754,7 +1754,7 @@ def get_collection_containers(
17541754
"""
17551755
return Container.objects.filter(
17561756
publishable_entity__learning_package_id=learning_package_id,
1757-
publishable_entity__collections__key=collection_key,
1757+
publishable_entity__collections__collection_code=collection_code,
17581758
).order_by('pk')
17591759

17601760

src/openedx_content/applets/units/models.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
"""
22
Models that implement units
33
"""
4+
from __future__ import annotations
5+
46
from django.db import models
57

68
from ..publishing.models import Container, ContainerVersion
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# Generated by Django 5.2.12 on 2026-03-24 20:33
2+
3+
from django.conf import settings
4+
from django.db import migrations, models
5+
6+
7+
class Migration(migrations.Migration):
8+
9+
dependencies = [
10+
('openedx_content', '0003_rename_content_to_media'),
11+
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
12+
]
13+
14+
operations = [
15+
migrations.RemoveConstraint(
16+
model_name='collection',
17+
name='oel_coll_uniq_lp_key',
18+
),
19+
migrations.RenameField(
20+
model_name='collection',
21+
old_name='key',
22+
new_name='collection_code',
23+
),
24+
migrations.AddConstraint(
25+
model_name='collection',
26+
constraint=models.UniqueConstraint(fields=('learning_package', 'collection_code'), name='oel_coll_uniq_lp_key'),
27+
),
28+
]

0 commit comments

Comments
 (0)