fix: improve collections models and api [FC-0059] (#208)

* fix: improve collections models and api
* feat: export models
* fix: index field
* fix: add created_by parameter to create API
* test: small changes to improve test coverage for collections
* chore: bumps version to 0.11.1

openedx_learning/__init__.py

@@ -1,4 +1,4 @@
 """
 Open edX Learning ("Learning Core").
 """
-__version__ = "0.11.0"
+__version__ = "0.11.1"

openedx_learning/api/authoring_models.py

@@ -7,6 +7,7 @@
 """
 # These wildcard imports are okay because these modules declare __all__.
 # pylint: disable=wildcard-import
+from ..apps.authoring.collections.models import *
 from ..apps.authoring.components.models import *
 from ..apps.authoring.contents.models import *
 from ..apps.authoring.publishing.model_mixins import *

openedx_learning/apps/authoring/collections/api.py

@@ -22,15 +22,17 @@
 
 def create_collection(
     learning_package_id: int,
-    name: str,
+    title: str,
+    created_by: int | None,
     description: str = "",
 ) -> Collection:
     """
     Create a new Collection
     """
     collection = Collection.objects.create(
         learning_package_id=learning_package_id,
-        name=name,
+        title=title,
+        created_by_id=created_by,
         description=description,
     )
     return collection
@@ -45,26 +47,26 @@ def get_collection(collection_id: int) -> Collection:
 
 def update_collection(
     collection_id: int,
-    name: str | None = None,
+    title: str | None = None,
     description: str | None = None,
 ) -> Collection:
     """
     Update a Collection
     """
-    lp = Collection.objects.get(id=collection_id)
+    collection = Collection.objects.get(id=collection_id)
 
     # If no changes were requested, there's nothing to update, so just return
     # the Collection as-is
-    if all(field is None for field in [name, description]):
-        return lp
+    if all(field is None for field in [title, description]):
+        return collection
 
-    if name is not None:
-        lp.name = name
+    if title is not None:
+        collection.title = title
     if description is not None:
-        lp.description = description
+        collection.description = description
 
-    lp.save()
-    return lp
+    collection.save()
+    return collection
 
 
 def get_learning_package_collections(learning_package_id: int) -> QuerySet[Collection]:
@@ -73,8 +75,6 @@ def get_learning_package_collections(learning_package_id: int) -> QuerySet[Colle
 
     Only enabled collections are returned
     """
-    return (
-        Collection.objects
-        .filter(learning_package_id=learning_package_id, enabled=True)
-        .select_related("learning_package")
-    )
+    return Collection.objects \
+                     .filter(learning_package_id=learning_package_id, enabled=True) \
+                     .select_related("learning_package")

openedx_learning/apps/authoring/collections/migrations/0002_remove_collection_name_collection_created_by_and_more.py

@@ -0,0 +1,53 @@
+# Generated by Django 4.2.14 on 2024-08-14 14:20
+
+import django.db.models.deletion
+from django.conf import settings
+from django.db import migrations, models
+
+import openedx_learning.lib.fields
+import openedx_learning.lib.validators
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('oel_collections', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='collection',
+            name='name',
+        ),
+        migrations.AddField(
+            model_name='collection',
+            name='created_by',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AddField(
+            model_name='collection',
+            name='title',
+            field=openedx_learning.lib.fields.MultiCollationCharField(db_collations={'mysql': 'utf8mb4_unicode_ci', 'sqlite': 'NOCASE'}, default='Collection', help_text='The title of the collection.', max_length=500),
+            preserve_default=False,
+        ),
+        migrations.AlterField(
+            model_name='collection',
+            name='created',
+            field=models.DateTimeField(auto_now_add=True, validators=[openedx_learning.lib.validators.validate_utc_datetime]),
+        ),
+        migrations.AlterField(
+            model_name='collection',
+            name='description',
+            field=openedx_learning.lib.fields.MultiCollationTextField(blank=True, db_collations={'mysql': 'utf8mb4_unicode_ci', 'sqlite': 'NOCASE'}, default='', help_text='Provides extra information for the user about this collection.', max_length=10000),
+        ),
+        migrations.AlterField(
+            model_name='collection',
+            name='modified',
+            field=models.DateTimeField(auto_now=True, validators=[openedx_learning.lib.validators.validate_utc_datetime]),
+        ),
+        migrations.AddIndex(
+            model_name='collection',
+            index=models.Index(fields=['learning_package', 'title'], name='oel_collect_learnin_dfaf89_idx'),
+        ),
+    ]

openedx_learning/apps/authoring/collections/models.py

@@ -3,12 +3,18 @@
 """
 from __future__ import annotations
 
+from django.conf import settings
 from django.db import models
 from django.utils.translation import gettext_lazy as _
 
-from ....lib.fields import case_insensitive_char_field
+from ....lib.fields import MultiCollationTextField, case_insensitive_char_field
+from ....lib.validators import validate_utc_datetime
 from ..publishing.models import LearningPackage
 
+__all__ = [
+    "Collection",
+]
+
 
 class Collection(models.Model):
     """
@@ -20,22 +26,29 @@ class Collection(models.Model):
     # Each collection belongs to a learning package
     learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE)
 
-    name = case_insensitive_char_field(
+    title = case_insensitive_char_field(
         null=False,
-        max_length=255,
-        db_index=True,
+        blank=False,
+        max_length=500,
         help_text=_(
-            "The name of the collection."
+            "The title of the collection."
         ),
     )
-    description = case_insensitive_char_field(
-        null=False,
+
+    description = MultiCollationTextField(
         blank=True,
+        null=False,
+        default="",
         max_length=10_000,
         help_text=_(
             "Provides extra information for the user about this collection."
         ),
+        db_collations={
+            "sqlite": "NOCASE",
+            "mysql": "utf8mb4_unicode_ci",
+        }
     )
+
     # We don't have api functions to handle the enabled field. This is a placeholder for future use and
     # a way to "soft delete" collections.
     enabled = models.BooleanField(
@@ -44,11 +57,33 @@ class Collection(models.Model):
             "Whether the collection is enabled or not."
         ),
     )
-    created = models.DateTimeField(auto_now_add=True)
-    modified = models.DateTimeField(auto_now=True)
+
+    created_by = models.ForeignKey(
+        settings.AUTH_USER_MODEL,
+        on_delete=models.SET_NULL,
+        null=True,
+        blank=True,
+    )
+
+    created = models.DateTimeField(
+        auto_now_add=True,
+        validators=[
+            validate_utc_datetime,
+        ],
+    )
+
+    modified = models.DateTimeField(
+        auto_now=True,
+        validators=[
+            validate_utc_datetime,
+        ],
+    )
 
     class Meta:
         verbose_name_plural = "Collections"
+        indexes = [
+            models.Index(fields=["learning_package", "title"]),
+        ]
 
     def __repr__(self) -> str:
         """
@@ -60,4 +95,4 @@ def __str__(self) -> str:
         """
         User-facing string representation of a Collection.
         """
-        return f"<{self.__class__.__name__}> ({self.id}:{self.name})"
+        return f"<{self.__class__.__name__}> ({self.id}:{self.title})"

tests/openedx_learning/apps/authoring/collections/test_api.py

@@ -3,6 +3,7 @@
 """
 from datetime import datetime, timezone
 
+from django.contrib.auth import get_user_model
 from django.core.exceptions import ObjectDoesNotExist
 from freezegun import freeze_time
 
@@ -12,6 +13,8 @@
 from openedx_learning.apps.authoring.publishing.models import LearningPackage
 from openedx_learning.lib.test_utils import TestCase
 
+User = get_user_model()
+
 
 class CollectionTestCase(TestCase):
     """
@@ -45,17 +48,20 @@ def setUpTestData(cls) -> None:
         super().setUpTestData()
         cls.collection1 = collection_api.create_collection(
             cls.learning_package.id,
-            name="Collection 1",
+            created_by=None,
+            title="Collection 1",
             description="Description of Collection 1",
         )
         cls.collection2 = collection_api.create_collection(
             cls.learning_package.id,
-            name="Collection 2",
+            created_by=None,
+            title="Collection 2",
             description="Description of Collection 2",
         )
         cls.disabled_collection = collection_api.create_collection(
             cls.learning_package.id,
-            name="Disabled Collection",
+            created_by=None,
+            title="Disabled Collection",
             description="Description of Disabled Collection",
         )
         cls.disabled_collection.enabled = False
@@ -102,29 +108,36 @@ def test_create_collection(self):
         """
         Test creating a collection.
         """
+        user = User.objects.create(
+            username="user",
+            email="[email protected]",
+        )
         created_time = datetime(2024, 8, 8, tzinfo=timezone.utc)
         with freeze_time(created_time):
             collection = collection_api.create_collection(
                 self.learning_package.id,
-                name="My Collection",
+                title="My Collection",
+                created_by=user.id,
                 description="This is my collection",
             )
 
-        assert collection.name == "My Collection"
+        assert collection.title == "My Collection"
         assert collection.description == "This is my collection"
         assert collection.enabled
         assert collection.created == created_time
         assert collection.modified == created_time
+        assert collection.created_by == user
 
     def test_create_collection_without_description(self):
         """
         Test creating a collection without a description.
         """
         collection = collection_api.create_collection(
             self.learning_package.id,
-            name="My Collection",
+            created_by=None,
+            title="My Collection",
         )
-        assert collection.name == "My Collection"
+        assert collection.title == "My Collection"
         assert collection.description == ""
         assert collection.enabled
 
@@ -142,38 +155,48 @@ def setUp(self) -> None:
         super().setUp()
         self.collection = collection_api.create_collection(
             self.learning_package.id,
-            name="Collection",
+            title="Collection",
+            created_by=None,
             description="Description of Collection",
         )
 
     def test_update_collection(self):
         """
-        Test updating a collection's name and description.
+        Test updating a collection's title and description.
         """
         modified_time = datetime(2024, 8, 8, tzinfo=timezone.utc)
         with freeze_time(modified_time):
             collection = collection_api.update_collection(
                 self.collection.pk,
-                name="New Name",
+                title="New Title",
                 description="",
             )
 
-        assert collection.name == "New Name"
+        assert collection.title == "New Title"
         assert collection.description == ""
         assert collection.modified == modified_time
         assert collection.created == self.collection.created  # unchanged
 
     def test_update_collection_partial(self):
         """
-        Test updating a collection's name.
+        Test updating a collection's title.
         """
         collection = collection_api.update_collection(
             self.collection.pk,
-            name="New Name",
+            title="New Title",
         )
 
-        assert collection.name == "New Name"
+        assert collection.title == "New Title"
         assert collection.description == self.collection.description  # unchanged
+        assert f"{collection}" == f"<Collection> ({self.collection.pk}:New Title)"
+
+        collection = collection_api.update_collection(
+            self.collection.pk,
+            description="New description",
+        )
+
+        assert collection.title == "New Title"  # unchanged
+        assert collection.description == "New description"
 
     def test_update_collection_empty(self):
         """
@@ -185,7 +208,7 @@ def test_update_collection_empty(self):
                 self.collection.pk,
             )
 
-        assert collection.name == self.collection.name  # unchanged
+        assert collection.title == self.collection.title  # unchanged
         assert collection.description == self.collection.description  # unchanged
         assert collection.modified == self.collection.modified  # unchanged
 
@@ -194,4 +217,4 @@ def test_update_collection_not_found(self):
         Test updating a collection that doesn't exist.
         """
         with self.assertRaises(ObjectDoesNotExist):
-            collection_api.update_collection(12345, name="New Name")
+            collection_api.update_collection(12345, title="New Title")