Skip to content

Commit

Permalink
refactor: use "entities" instead of "contents" or "objects"
Browse files Browse the repository at this point in the history
Addresses PR review.

* CollectionObject -> CollectionPublishableEntity,
  with added created_by and created fields
* contents -> entities, contents_qset -> entities_qset
* get_object_collections -> get_entity_collections,
  with added learning_package_id argument
  • Loading branch information
pomegranited committed Aug 29, 2024
1 parent 0e15dd0 commit 00111bc
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 79 deletions.
55 changes: 31 additions & 24 deletions openedx_learning/apps/authoring/collections/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
from django.db.transaction import atomic
from django.utils import timezone

from ..publishing import api as publishing_api
from ..publishing.models import PublishableEntity
from .models import Collection, CollectionObject
from .models import Collection, CollectionPublishableEntity

# The public API that will be re-exported by openedx_learning.apps.authoring.api
# is listed in the __all__ entries below. Internal helper functions that are
Expand All @@ -21,7 +22,7 @@
"get_collection",
"get_collections",
"get_learning_package_collections",
"get_object_collections",
"get_entity_collections",
"remove_from_collections",
"update_collection",
]
Expand All @@ -32,7 +33,7 @@ def create_collection(
title: str,
created_by: int | None,
description: str = "",
contents_qset: QuerySet[PublishableEntity] = PublishableEntity.objects.none(), # default to empty set,
entities_qset: QuerySet[PublishableEntity] = PublishableEntity.objects.none(), # default to empty set,
) -> Collection:
"""
Create a new Collection
Expand All @@ -48,7 +49,8 @@ def create_collection(

added = add_to_collections(
Collection.objects.filter(id=collection.id),
contents_qset,
entities_qset,
created_by,
)
if added:
collection.refresh_from_db() # fetch updated modified date
Expand Down Expand Up @@ -89,33 +91,35 @@ def update_collection(

def add_to_collections(
collections_qset: QuerySet[Collection],
contents_qset: QuerySet[PublishableEntity],
entities_qset: QuerySet[PublishableEntity],
created_by: int | None = None,
) -> int:
"""
Adds a QuerySet of PublishableEntities to a QuerySet of Collections.
Records are created in bulk, and so integrity errors are deliberately ignored: they indicate that the content(s)
Records are created in bulk, and so integrity errors are deliberately ignored: they indicate that the entity(s)
have already been added to the collection(s).
Returns the number of entities added (including any that already exist).
"""
collection_objects = []
object_ids = contents_qset.values_list("pk", flat=True)
collection_entities = []
entity_ids = entities_qset.values_list("pk", flat=True)
collection_ids = collections_qset.values_list("pk", flat=True)

for collection_id in collection_ids:
for object_id in object_ids:
collection_objects.append(
CollectionObject(
for entity_id in entity_ids:
collection_entities.append(
CollectionPublishableEntity(
collection_id=collection_id,
object_id=object_id,
entity_id=entity_id,
created_by_id=created_by,
)
)

created = []
if collection_objects:
created = CollectionObject.objects.bulk_create(
collection_objects,
if collection_entities:
created = CollectionPublishableEntity.objects.bulk_create(
collection_entities,
ignore_conflicts=True,
)

Expand All @@ -127,27 +131,27 @@ def add_to_collections(


def remove_from_collections(
collections_qset: QuerySet,
contents_qset: QuerySet,
collections_qset: QuerySet[Collection],
entities_qset: QuerySet[PublishableEntity],
) -> int:
"""
Removes a QuerySet of PublishableEntities from a QuerySet of Collections.
PublishableEntities are deleted from each Collection, in bulk.
Collections which had objects removed are marked with modified=now().
Collections which had entities removed are marked with modified=now().
Returns the total number of entities deleted.
"""
total_deleted = 0
object_ids = contents_qset.values_list("pk", flat=True)
entity_ids = entities_qset.values_list("pk", flat=True)
collection_ids = collections_qset.values_list("pk", flat=True)
modified_collection_ids = []

for collection_id in collection_ids:
num_deleted, _ = CollectionObject.objects.filter(
num_deleted, _ = CollectionPublishableEntity.objects.filter(
collection_id=collection_id,
object_id__in=object_ids,
entity__in=entity_ids,
).delete()

if num_deleted:
Expand All @@ -161,13 +165,16 @@ def remove_from_collections(
return total_deleted


def get_object_collections(object_key: str) -> QuerySet[Collection]:
def get_entity_collections(learning_package_id: int, entity_key: str) -> QuerySet[Collection]:
"""
Get all collections associated with a given PublishableEntity.key.
Get all collections in the given learning package which contain this entity.
Only enabled collections are returned.
"""
entity = PublishableEntity.objects.get(key=object_key)
entity = publishing_api.get_publishable_entity_by_key(
learning_package_id,
key=entity_key,
)
return entity.collections.filter(enabled=True).order_by("pk")


Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,38 @@
# Generated by Django 4.2.14 on 2024-08-21 07:15
# Generated by Django 4.2.15 on 2024-08-29 00:05

import django.db.models.deletion
from django.conf import settings
from django.db import migrations, models

import openedx_learning.lib.validators


class Migration(migrations.Migration):

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('oel_publishing', '0002_alter_learningpackage_key_and_more'),
('oel_collections', '0002_remove_collection_name_collection_created_by_and_more'),
]

operations = [
migrations.CreateModel(
name='CollectionObject',
name='CollectionPublishableEntity',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('created', models.DateTimeField(auto_now_add=True, validators=[openedx_learning.lib.validators.validate_utc_datetime])),
('collection', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='oel_collections.collection')),
('object', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='oel_publishing.publishableentity')),
('created_by', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)),
('entity', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, to='oel_publishing.publishableentity')),
],
),
migrations.AddField(
model_name='collection',
name='contents',
field=models.ManyToManyField(related_name='collections', through='oel_collections.CollectionObject', to='oel_publishing.publishableentity'),
name='entities',
field=models.ManyToManyField(related_name='collections', through='oel_collections.CollectionPublishableEntity', to='oel_publishing.publishableentity'),
),
migrations.AddConstraint(
model_name='collectionobject',
constraint=models.UniqueConstraint(fields=('collection', 'object'), name='oel_collections_cpe_uniq_col_obj'),
model_name='collectionpublishableentity',
constraint=models.UniqueConstraint(fields=('collection', 'entity'), name='oel_collections_cpe_uniq_col_ent'),
),
]
28 changes: 20 additions & 8 deletions openedx_learning/apps/authoring/collections/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@

__all__ = [
"Collection",
"CollectionObject",
"CollectionPublishableEntity",
]


Expand Down Expand Up @@ -143,9 +143,9 @@ class Collection(models.Model):
],
)

contents: models.ManyToManyField[PublishableEntity, "CollectionObject"] = models.ManyToManyField(
entities: models.ManyToManyField[PublishableEntity, "CollectionPublishableEntity"] = models.ManyToManyField(
PublishableEntity,
through="CollectionObject",
through="CollectionPublishableEntity",
related_name="collections",
)

Expand All @@ -168,17 +168,29 @@ def __str__(self) -> str:
return f"<{self.__class__.__name__}> ({self.id}:{self.title})"


class CollectionObject(models.Model):
class CollectionPublishableEntity(models.Model):
"""
Collection -> PublishableEntity association.
"""
collection = models.ForeignKey(
Collection,
on_delete=models.CASCADE,
)
object = models.ForeignKey(
entity = models.ForeignKey(
PublishableEntity,
on_delete=models.CASCADE,
on_delete=models.RESTRICT,
)
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,
],
)

class Meta:
Expand All @@ -188,8 +200,8 @@ class Meta:
models.UniqueConstraint(
fields=[
"collection",
"object",
"entity",
],
name="oel_collections_cpe_uniq_col_obj",
name="oel_collections_cpe_uniq_col_ent",
)
]
2 changes: 1 addition & 1 deletion openedx_learning/apps/authoring/collections/readme.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ Things to remember:

* Collections may grow very large.
* Collections are not publishable in and of themselves.
* Collections link to PublisableEntity records, **not** PublishableEntityVersion records.
* Collections link to PublishableEntity records, **not** PublishableEntityVersion records.
Loading

0 comments on commit 00111bc

Please sign in to comment.