Skip to content

Commit 12e7dc6

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. The underlying database field is still named _key. 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 enforce it at the db level. Part of: #322 TODO: Fix backup/restore, with backcompat for `key` TODO: Should migrations be updated or left alone? TODO: Should the column stay as _key or change?
1 parent 62c937e commit 12e7dc6

18 files changed

Lines changed: 158 additions & 98 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+
collection_code = serializers.CharField(required=True, source="key")
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: 17 additions & 17 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,
@@ -45,7 +45,7 @@ def create_collection(
4545
"""
4646
collection = Collection.objects.create(
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,
@@ -54,24 +54,24 @@ def create_collection(
5454
return collection
5555

5656

57-
def get_collection(learning_package_id: int, collection_key: str) -> Collection:
57+
def get_collection(learning_package_id: int, collection_code: str) -> Collection:
5858
"""
5959
Get a Collection by ID
6060
"""
61-
return Collection.objects.get_by_key(learning_package_id, collection_key)
61+
return Collection.objects.get_by_code(learning_package_id, collection_code)
6262

6363

6464
def update_collection(
6565
learning_package_id: int,
66-
key: str,
66+
collection_code: str,
6767
*,
6868
title: str | None = None,
6969
description: str | None = None,
7070
) -> Collection:
7171
"""
72-
Update a Collection identified by the learning_package_id + key.
72+
Update a Collection identified by the learning_package_id + collection_code.
7373
"""
74-
collection = get_collection(learning_package_id, key)
74+
collection = get_collection(learning_package_id, collection_code)
7575

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

9090
def delete_collection(
9191
learning_package_id: int,
92-
key: str,
92+
collection_code: str,
9393
*,
9494
hard_delete=False,
9595
) -> Collection:
9696
"""
97-
Disables or deletes a collection identified by the given learning_package + key.
97+
Disables or deletes a collection identified by the given learning_package + collection_code.
9898
9999
By default (hard_delete=False), the collection is "soft deleted", i.e disabled.
100100
Soft-deleted collections can be re-enabled using restore_collection.
101101
"""
102-
collection = get_collection(learning_package_id, key)
102+
collection = get_collection(learning_package_id, collection_code)
103103

104104
if hard_delete:
105105
collection.delete()
@@ -111,12 +111,12 @@ def delete_collection(
111111

112112
def restore_collection(
113113
learning_package_id: int,
114-
key: str,
114+
collection_code: str,
115115
) -> Collection:
116116
"""
117117
Undo a "soft delete" by re-enabling a Collection.
118118
"""
119-
collection = get_collection(learning_package_id, key)
119+
collection = get_collection(learning_package_id, collection_code)
120120

121121
collection.enabled = True
122122
collection.save()
@@ -125,7 +125,7 @@ def restore_collection(
125125

126126
def add_to_collection(
127127
learning_package_id: int,
128-
key: str,
128+
collection_code: str,
129129
entities_qset: QuerySet[PublishableEntity],
130130
created_by: int | None = None,
131131
) -> Collection:
@@ -145,10 +145,10 @@ def add_to_collection(
145145
if invalid_entity:
146146
raise ValidationError(
147147
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}."
148+
f"to collection {collection_code} in learning package {learning_package_id}."
149149
)
150150

151-
collection = get_collection(learning_package_id, key)
151+
collection = get_collection(learning_package_id, collection_code)
152152
collection.entities.add(
153153
*entities_qset.all(),
154154
through_defaults={"created_by_id": created_by},
@@ -161,7 +161,7 @@ def add_to_collection(
161161

162162
def remove_from_collection(
163163
learning_package_id: int,
164-
key: str,
164+
collection_code: str,
165165
entities_qset: QuerySet[PublishableEntity],
166166
) -> Collection:
167167
"""
@@ -173,7 +173,7 @@ def remove_from_collection(
173173
174174
Returns the updated Collection.
175175
"""
176-
collection = get_collection(learning_package_id, key)
176+
collection = get_collection(learning_package_id, collection_code)
177177

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

src/openedx_content/applets/collections/models.py

Lines changed: 14 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,12 @@ 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+
# Formerly known as the "key", hence the column name.
115+
collection_code = code_field(db_column='_key')
112116

113117
title = case_insensitive_char_field(
114118
null=False,
@@ -170,11 +174,11 @@ class Collection(models.Model):
170174
class Meta:
171175
verbose_name_plural = "Collections"
172176
constraints = [
173-
# Keys are unique within a given LearningPackage.
177+
# Collection codes are unique within a given LearningPackage.
174178
models.UniqueConstraint(
175179
fields=[
176180
"learning_package",
177-
"key",
181+
"collection_code",
178182
],
179183
name="oel_coll_uniq_lp_key",
180184
),
@@ -196,7 +200,7 @@ def __str__(self) -> str:
196200
"""
197201
User-facing string representation of a Collection.
198202
"""
199-
return f"<{self.__class__.__name__}> (lp:{self.learning_package_id} {self.key}:{self.title})"
203+
return f"<{self.__class__.__name__}> (lp:{self.learning_package_id} {self.collection_code}:{self.title})"
200204

201205

202206
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

src/openedx_content/backcompat/collections/migrations/0004_collection_key.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,19 @@
66
import openedx_django_lib.fields
77

88

9-
def generate_keys(apps, schema_editor):
9+
def generate_codes(apps, schema_editor):
1010
"""
11-
Generates a random strings to initialize the key field where NULL.
11+
Generates a random strings to initialize the collection_code field where NULL.
1212
"""
1313
length = 50
1414
Collection = apps.get_model("oel_collections", "Collection")
15-
for collection in Collection.objects.filter(key=None):
15+
for collection in Collection.objects.filter(collection_code=None):
1616
# Keep generating keys until we get a unique one
17-
key = get_random_string(length)
18-
while Collection.objects.filter(key=key).exists():
19-
key = get_random_string(length)
17+
collection_code = get_random_string(length)
18+
while Collection.objects.filter(collection_code=collection_code).exists():
19+
collection_code = get_random_string(length)
2020

21-
collection.key = key
21+
collection.collection_code = collection_code
2222
collection.save()
2323

2424

@@ -32,18 +32,18 @@ class Migration(migrations.Migration):
3232
# 1. Temporarily add this field with null=True, blank=True
3333
migrations.AddField(
3434
model_name='collection',
35-
name='key',
35+
name='collection_code',
3636
field=openedx_django_lib.fields.MultiCollationCharField(
3737
db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'},
3838
db_column='_key', max_length=500, null=True, blank=True),
3939
preserve_default=False,
4040
),
4141
# 2. Populate the null keys
42-
migrations.RunPython(generate_keys),
42+
migrations.RunPython(generate_codes),
4343
# 3. Add null=False, blank=False to disallow NULL values
4444
migrations.AlterField(
4545
model_name='collection',
46-
name='key',
46+
name='collection_code',
4747
field=openedx_django_lib.fields.MultiCollationCharField(
4848
db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'},
4949
db_column='_key', max_length=500, null=False, blank=False),
@@ -52,6 +52,6 @@ class Migration(migrations.Migration):
5252
# 4. Enforce unique constraint
5353
migrations.AddConstraint(
5454
model_name='collection',
55-
constraint=models.UniqueConstraint(fields=('learning_package', 'key'), name='oel_coll_uniq_lp_key'),
55+
constraint=models.UniqueConstraint(fields=('learning_package', 'collection_code'), name='oel_coll_uniq_lp_key'),
5656
),
5757
]

0 commit comments

Comments
 (0)