Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adds Collection.key #223

Merged
merged 5 commits into from
Sep 10, 2024
Merged
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
2 changes: 1 addition & 1 deletion openedx_learning/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
Open edX Learning ("Learning Core").
"""
__version__ = "0.11.3"
__version__ = "0.11.4"
29 changes: 17 additions & 12 deletions openedx_learning/apps/authoring/collections/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@

def create_collection(
learning_package_id: int,
key: str,
*,
title: str,
created_by: int | None,
description: str = "",
Expand All @@ -40,6 +42,7 @@ def create_collection(
"""
collection = Collection.objects.create(
learning_package_id=learning_package_id,
key=key,
title=title,
created_by_id=created_by,
description=description,
Expand All @@ -48,22 +51,24 @@ def create_collection(
return collection


def get_collection(collection_id: int) -> Collection:
def get_collection(learning_package_id: int, collection_key: str) -> Collection:
"""
Get a Collection by ID
"""
return Collection.objects.get(id=collection_id)
return Collection.objects.get_by_key(learning_package_id, collection_key)


def update_collection(
collection_id: int,
learning_package_id: int,
key: str,
*,
title: str | None = None,
description: str | None = None,
Copy link
Contributor

@rpenido rpenido Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for not catching this before. 😞
I think we should be able to update the key here.

Suggested change
description: str | None = None,
description: str | None = None,
new_key: str,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. if we allow that, it'll change the Collection's opaque key, which will disconnect it from any applied tags.. so I think it should be immutable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right! My bad!

) -> Collection:
"""
Update a Collection
Update a Collection identified by the learning_package_id + key.
"""
collection = Collection.objects.get(id=collection_id)
collection = get_collection(learning_package_id, key)

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


def add_to_collection(
collection_id: int,
learning_package_id: int,
key: str,
entities_qset: QuerySet[PublishableEntity],
created_by: int | None = None,
) -> Collection:
Expand All @@ -95,17 +101,15 @@ def add_to_collection(

Returns the updated Collection object.
"""
collection = get_collection(collection_id)
learning_package_id = collection.learning_package_id

# Disallow adding entities outside the collection's learning package
invalid_entity = entities_qset.exclude(learning_package_id=learning_package_id).first()
if invalid_entity:
raise ValidationError(
f"Cannot add entity {invalid_entity.pk} in learning package {invalid_entity.learning_package_id} "
f"to collection {collection_id} in learning package {learning_package_id}."
f"to collection {key} in learning package {learning_package_id}."
)

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


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

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

collection.entities.remove(*entities_qset.all())
collection.modified = datetime.now(tz=timezone.utc)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Generated by Django 4.2.15 on 2024-09-04 23:15

from django.db import migrations, models
from django.utils.crypto import get_random_string

import openedx_learning.lib.fields


def generate_keys(apps, schema_editor):
"""
Generates a random strings to initialize the key field where NULL.
"""
length = 50
Collection = apps.get_model("oel_collections", "Collection")
for collection in Collection.objects.filter(key=None):
# Keep generating keys until we get a unique one
key = get_random_string(length)
while Collection.objects.filter(key=key).exists():
key = get_random_string(length)

collection.key = key
collection.save()


class Migration(migrations.Migration):

dependencies = [
('oel_collections', '0003_collection_entities'),
]

operations = [
# 1. Temporarily add this field with null=True, blank=True
migrations.AddField(
model_name='collection',
name='key',
field=openedx_learning.lib.fields.MultiCollationCharField(
db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'},
db_column='_key', max_length=500, null=True, blank=True),
preserve_default=False,
),
# 2. Populate the null keys
migrations.RunPython(generate_keys),
# 3. Add null=False, blank=False to disallow NULL values
migrations.AlterField(
model_name='collection',
name='key',
field=openedx_learning.lib.fields.MultiCollationCharField(
db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'},
db_column='_key', max_length=500, null=False, blank=False),
preserve_default=False,
),
# 4. Enforce unique constraint
migrations.AddConstraint(
model_name='collection',
constraint=models.UniqueConstraint(fields=('learning_package', 'key'), name='oel_coll_uniq_lp_key'),
),
]
36 changes: 33 additions & 3 deletions openedx_learning/apps/authoring/collections/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@
from django.db import models
from django.utils.translation import gettext_lazy as _

from ....lib.fields import MultiCollationTextField, case_insensitive_char_field
from ....lib.validators import validate_utc_datetime
from openedx_learning.lib.fields import MultiCollationTextField, case_insensitive_char_field, key_field
from openedx_learning.lib.validators import validate_utc_datetime

from ..publishing.models import LearningPackage, PublishableEntity

__all__ = [
Expand All @@ -80,16 +81,35 @@
]


class CollectionManager(models.Manager):
"""
Custom manager for Collection class.
"""
def get_by_key(self, learning_package_id: int, key: str):
"""
Get the Collection for the given Learning Package + key.
"""
return self.select_related('learning_package') \
.get(learning_package_id=learning_package_id, key=key)


class Collection(models.Model):
"""
Represents a collection of library components
"""
objects: CollectionManager[Collection] = CollectionManager()

id = models.AutoField(primary_key=True)

# Each collection belongs to a learning package
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
# collection's opaque key:
# e.g. "lib-collection:lib:key" is the opaque key for a library collection.
key = key_field(db_column='_key')

title = case_insensitive_char_field(
null=False,
blank=False,
Expand Down Expand Up @@ -151,6 +171,16 @@ class Collection(models.Model):

class Meta:
verbose_name_plural = "Collections"
constraints = [
# Keys are unique within a given LearningPackage.
models.UniqueConstraint(
fields=[
"learning_package",
"key",
],
name="oel_coll_uniq_lp_key",
),
]
indexes = [
models.Index(fields=["learning_package", "title"]),
]
Expand All @@ -165,7 +195,7 @@ def __str__(self) -> str:
"""
User-facing string representation of a Collection.
"""
return f"<{self.__class__.__name__}> ({self.id}:{self.title})"
return f"<{self.__class__.__name__}> (lp:{self.learning_package_id} {self.key}:{self.title})"


class CollectionPublishableEntity(models.Model):
Expand Down
Loading