Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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,

@rpenido rpenido Sep 6, 2024 •

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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,38 @@
# Generated by Django 4.2.15 on 2024-09-03 06:25

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

import openedx_learning.lib.fields


def generate_key():
"""
Generates a random string to initialize a SlugField.

Used here to initialize Collection.key, a required field that was added after the model was created.
"""
return get_random_string(length=50)


class Migration(migrations.Migration):

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

operations = [
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,
default=generate_key,
Comment thread
pomegranited marked this conversation as resolved.
Resolved
),
preserve_default=False,
),
migrations.AddIndex(
model_name='collection',
index=models.Index(fields=['learning_package', 'key'], name='oel_collect_learnin_f23741_idx'),
),
]
27 changes: 24 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 @@ -153,6 +173,7 @@ class Meta:
verbose_name_plural = "Collections"
indexes = [
models.Index(fields=["learning_package", "title"]),
models.Index(fields=["learning_package", "key"]),
]
Comment thread
pomegranited marked this conversation as resolved.
Resolved

def __repr__(self) -> str:
Expand All @@ -165,7 +186,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__}> ({self.key}:{self.title})"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This would not be unique. Do you think we should add the library key here?

@pomegranited pomegranited Sep 4, 2024 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, see bdcb52b



class CollectionPublishableEntity(models.Model):
Expand Down
Loading