diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index 9f57f2a2..f2c501ee 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -2,4 +2,4 @@ Open edX Learning ("Learning Core"). """ -__version__ = "0.18.3" +__version__ = "0.19.0" diff --git a/openedx_learning/api/authoring.py b/openedx_learning/api/authoring.py index 1da667af..c0610487 100644 --- a/openedx_learning/api/authoring.py +++ b/openedx_learning/api/authoring.py @@ -13,6 +13,7 @@ from ..apps.authoring.components.api import * from ..apps.authoring.contents.api import * from ..apps.authoring.publishing.api import * +from ..apps.authoring.units.api import * # This was renamed after the authoring API refactoring pushed this and other # app APIs into the openedx_learning.api.authoring module. Here I'm aliasing to diff --git a/openedx_learning/api/authoring_models.py b/openedx_learning/api/authoring_models.py index fb0ab669..2413fc2b 100644 --- a/openedx_learning/api/authoring_models.py +++ b/openedx_learning/api/authoring_models.py @@ -10,5 +10,5 @@ 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 * from ..apps.authoring.publishing.models import * +from ..apps.authoring.units.models import * diff --git a/openedx_learning/apps/authoring/components/api.py b/openedx_learning/apps/authoring/components/api.py index b1bfe63c..1c6d1693 100644 --- a/openedx_learning/apps/authoring/components/api.py +++ b/openedx_learning/apps/authoring/components/api.py @@ -231,7 +231,7 @@ def create_next_component_version( return component_version -def create_component_and_version( +def create_component_and_version( # pylint: disable=too-many-positional-arguments learning_package_id: int, /, component_type: ComponentType, @@ -326,7 +326,7 @@ def component_exists_by_key( return False -def get_components( +def get_components( # pylint: disable=too-many-positional-arguments learning_package_id: int, /, draft: bool | None = None, diff --git a/openedx_learning/apps/authoring/components/models.py b/openedx_learning/apps/authoring/components/models.py index 08c4fb72..85af54d9 100644 --- a/openedx_learning/apps/authoring/components/models.py +++ b/openedx_learning/apps/authoring/components/models.py @@ -17,13 +17,14 @@ """ from __future__ import annotations +from typing import ClassVar + from django.db import models from ....lib.fields import case_sensitive_char_field, immutable_uuid_field, key_field from ....lib.managers import WithRelationsManager from ..contents.models import Content -from ..publishing.model_mixins import PublishableEntityMixin, PublishableEntityVersionMixin -from ..publishing.models import LearningPackage +from ..publishing.models import LearningPackage, PublishableEntityMixin, PublishableEntityVersionMixin __all__ = [ "ComponentType", @@ -76,7 +77,7 @@ def __str__(self) -> str: return f"{self.namespace}:{self.name}" -class Component(PublishableEntityMixin): # type: ignore[django-manager-missing] +class Component(PublishableEntityMixin): """ This represents any Component that has ever existed in a LearningPackage. @@ -120,14 +121,12 @@ class Component(PublishableEntityMixin): # type: ignore[django-manager-missing] Make a foreign key to the Component model when you need a stable reference that will exist for as long as the LearningPackage itself exists. """ - # Tell mypy what type our objects manager has. - # It's actually PublishableEntityMixinManager, but that has the exact same - # interface as the base manager class. - objects: models.Manager[Component] = WithRelationsManager( + # Set up our custom manager. It has the same API as the default one, but selects related objects by default. + objects: ClassVar[WithRelationsManager[Component]] = WithRelationsManager( # type: ignore[assignment] 'component_type' ) - with_publishing_relations: models.Manager[Component] = WithRelationsManager( + with_publishing_relations = WithRelationsManager( 'component_type', 'publishable_entity', 'publishable_entity__draft__version', @@ -201,10 +200,6 @@ class ComponentVersion(PublishableEntityVersionMixin): This holds the content using a M:M relationship with Content via ComponentVersionContent. """ - # Tell mypy what type our objects manager has. - # It's actually PublishableEntityVersionMixinManager, but that has the exact - # same interface as the base manager class. - objects: models.Manager[ComponentVersion] # This is technically redundant, since we can get this through # publishable_entity_version.publishable.component, but this is more diff --git a/openedx_learning/apps/authoring/publishing/api.py b/openedx_learning/apps/authoring/publishing/api.py index 3facc891..231e42c4 100644 --- a/openedx_learning/apps/authoring/publishing/api.py +++ b/openedx_learning/apps/authoring/publishing/api.py @@ -6,23 +6,37 @@ """ from __future__ import annotations +from dataclasses import dataclass from datetime import datetime, timezone +from typing import TypeVar -from django.core.exceptions import ObjectDoesNotExist +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db.models import F, Q, QuerySet from django.db.transaction import atomic -from .model_mixins import PublishableContentModelRegistry, PublishableEntityMixin, PublishableEntityVersionMixin from .models import ( + Container, + ContainerVersion, Draft, + EntityList, + EntityListRow, LearningPackage, + PublishableContentModelRegistry, PublishableEntity, + PublishableEntityMixin, PublishableEntityVersion, + PublishableEntityVersionMixin, Published, PublishLog, PublishLogRecord, ) +# A few of the APIs in this file are generic and can be used for Containers in +# general, or e.g. Units (subclass of Container) in particular. These type +# variables are used to provide correct typing for those generic API methods. +ContainerModel = TypeVar('ContainerModel', bound=Container) +ContainerVersionModel = TypeVar('ContainerVersionModel', bound=ContainerVersion) + # 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 # private to this module should start with an underscore. If a function does not @@ -51,6 +65,16 @@ "reset_drafts_to_published", "register_content_models", "filter_publishable_entities", + # πŸ›‘ UNSTABLE: All APIs related to containers are unstable until we've figured + # out our approach to dynamic content (randomized, A/B tests, etc.) + "create_container", + "create_container_version", + "create_next_container_version", + "get_container", + "ContainerEntityListEntry", + "get_entities_in_container", + "contains_unpublished_changes", + "get_containers_with_entity", ] @@ -310,6 +334,30 @@ def publish_from_drafts( published_at = datetime.now(tz=timezone.utc) with atomic(): + # If the drafts include any containers, we need to auto-publish their descendants: + # TODO: this only handles one level deep and would need to be updated to support sections > subsections > units + + # Get the IDs of the ContainerVersion for any Containers whose drafts are slated to be published. + container_version_ids = ( + Container.objects.filter(publishable_entity__draft__in=draft_qset) + .values_list("publishable_entity__draft__version__containerversion__pk", flat=True) + ) + if container_version_ids: + # We are publishing at least one container. Check if it has any child components that aren't already slated + # to be published. + unpublished_draft_children = EntityListRow.objects.filter( + entity_list__container_versions__pk__in=container_version_ids, + entity_version=None, # Unpinned entities only + ).exclude( + entity__draft__version=F("entity__published__version") # Exclude already published things + ).values_list("entity__draft__pk", flat=True) + if unpublished_draft_children: + # Force these additional child components to be published at the same time by adding them to the qset: + draft_qset = Draft.objects.filter( + Q(pk__in=draft_qset.values_list("pk", flat=True)) | + Q(pk__in=unpublished_draft_children) + ) + # One PublishLog for this entire publish operation. publish_log = PublishLog( learning_package_id=learning_package_id, @@ -477,7 +525,7 @@ def register_content_models( This is so that we can provide convenience links between content models and content version models *through* the publishing apps, so that you can do things like finding the draft version of a content model more easily. See - the model_mixins.py module for more details. + the publishable_entity.py module for more details. This should only be imported and run from the your app's AppConfig.ready() method. For example, in the components app, this looks like: @@ -513,3 +561,411 @@ def filter_publishable_entities( entities = entities.filter(published__version__isnull=not has_published) return entities + + +def get_published_version_as_of(entity_id: int, publish_log_id: int) -> PublishableEntityVersion | None: + """ + Get the published version of the given entity, at a specific snapshot in the + history of this Learning Package, given by the PublishLog ID. + + This is a semi-private function, only available to other apps in the + authoring package. + """ + record = PublishLogRecord.objects.filter( + entity_id=entity_id, + publish_log_id__lte=publish_log_id, + ).order_by('-publish_log_id').first() + return record.new_version if record else None + + +def create_container( + learning_package_id: int, + key: str, + created: datetime, + created_by: int | None, + # The types on the following line are correct, but mypy will complain - https://github.com/python/mypy/issues/3737 + container_cls: type[ContainerModel] = Container, # type: ignore[assignment] +) -> ContainerModel: + """ + [ πŸ›‘ UNSTABLE ] + Create a new container. + + Args: + learning_package_id: The ID of the learning package that contains the container. + key: The key of the container. + created: The date and time the container was created. + created_by: The ID of the user who created the container + container_cls: The subclass of Container to use, if applicable + + Returns: + The newly created container. + """ + assert issubclass(container_cls, Container) + with atomic(): + publishable_entity = create_publishable_entity( + learning_package_id, key, created, created_by + ) + container = container_cls.objects.create( + publishable_entity=publishable_entity, + ) + return container + + +def create_entity_list() -> EntityList: + """ + [ πŸ›‘ UNSTABLE ] + Create a new entity list. This is an structure that holds a list of entities + that will be referenced by the container. + + Returns: + The newly created entity list. + """ + return EntityList.objects.create() + + +def create_entity_list_with_rows( + entity_pks: list[int], + entity_version_pks: list[int | None], + *, + learning_package_id: int | None, +) -> EntityList: + """ + [ πŸ›‘ UNSTABLE ] + Create new entity list rows for an entity list. + + Args: + entity_pks: The IDs of the publishable entities that the entity list rows reference. + entity_version_pks: The IDs of the versions of the entities + (PublishableEntityVersion) that the entity list rows reference, or + Nones for "unpinned" (default). + learning_package_id: Optional. Verify that all the entities are from + the specified learning package. + + Returns: + The newly created entity list. + """ + # Do a quick check that the given entities are in the right learning package: + if learning_package_id: + if PublishableEntity.objects.filter( + pk__in=entity_pks, + ).exclude( + learning_package_id=learning_package_id, + ).exists(): + raise ValidationError("Container entities must be from the same learning package.") + + order_nums = range(len(entity_pks)) + with atomic(savepoint=False): + + entity_list = create_entity_list() + EntityListRow.objects.bulk_create( + [ + EntityListRow( + entity_list=entity_list, + entity_id=entity_pk, + order_num=order_num, + entity_version_id=entity_version_pk, + ) + for order_num, entity_pk, entity_version_pk in zip( + order_nums, entity_pks, entity_version_pks + ) + ] + ) + return entity_list + + +def _create_container_version( + container: Container, + version_num: int, + *, + title: str, + entity_list: EntityList, + created: datetime, + created_by: int | None, + container_version_cls: type[ContainerVersionModel] = ContainerVersion, # type: ignore[assignment] +) -> ContainerVersionModel: + """ + Private internal method for logic shared by create_container_version() and + create_next_container_version(). + """ + assert issubclass(container_version_cls, ContainerVersion) + with atomic(savepoint=False): # Make sure this will happen atomically but we don't need to create a new savepoint. + publishable_entity_version = create_publishable_entity_version( + container.publishable_entity_id, + version_num=version_num, + title=title, + created=created, + created_by=created_by, + ) + container_version = container_version_cls.objects.create( + publishable_entity_version=publishable_entity_version, + container_id=container.pk, + entity_list=entity_list, + ) + + return container_version + + +def create_container_version( + container_id: int, + version_num: int, + *, + title: str, + publishable_entities_pks: list[int], + entity_version_pks: list[int | None] | None, + created: datetime, + created_by: int | None, + container_version_cls: type[ContainerVersionModel] = ContainerVersion, # type: ignore[assignment] +) -> ContainerVersionModel: + """ + [ πŸ›‘ UNSTABLE ] + Create a new container version. + + Args: + container_id: The ID of the container that the version belongs to. + version_num: The version number of the container. + title: The title of the container. + publishable_entities_pks: The IDs of the members of the container. + entity_version_pks: The IDs of the versions to pin to, if pinning is desired. + created: The date and time the container version was created. + created_by: The ID of the user who created the container version. + container_version_cls: The subclass of ContainerVersion to use, if applicable. + + Returns: + The newly created container version. + """ + assert title is not None + assert publishable_entities_pks is not None + + with atomic(savepoint=False): + container = Container.objects.select_related("publishable_entity").get(pk=container_id) + entity = container.publishable_entity + if entity_version_pks is None: + entity_version_pks = [None] * len(publishable_entities_pks) + entity_list = create_entity_list_with_rows( + entity_pks=publishable_entities_pks, + entity_version_pks=entity_version_pks, + learning_package_id=entity.learning_package_id, + ) + container_version = _create_container_version( + container, + version_num, + title=title, + entity_list=entity_list, + created=created, + created_by=created_by, + container_version_cls=container_version_cls, + ) + + return container_version + + +def create_next_container_version( + container_pk: int, + *, + title: str | None, + publishable_entities_pks: list[int] | None, + entity_version_pks: list[int | None] | None, + created: datetime, + created_by: int | None, + container_version_cls: type[ContainerVersionModel] = ContainerVersion, # type: ignore[assignment] +) -> ContainerVersionModel: + """ + [ πŸ›‘ UNSTABLE ] + Create the next version of a container. A new version of the container is created + only when its metadata changes: + + * Something was added to the Container. + * We re-ordered the rows in the container. + * Something was removed from the container. + * The Container's metadata changed, e.g. the title. + * We pin to different versions of the Container. + + Args: + container_pk: The ID of the container to create the next version of. + title: The title of the container. None to keep the current title. + publishable_entities_pks: The IDs of the members current members of the container. Or None for no change. + entity_version_pks: The IDs of the versions to pin to, if pinning is desired. + created: The date and time the container version was created. + created_by: The ID of the user who created the container version. + container_version_cls: The subclass of ContainerVersion to use, if applicable. + + Returns: + The newly created container version. + """ + assert issubclass(container_version_cls, ContainerVersion) + with atomic(): + container = Container.objects.select_related("publishable_entity").get(pk=container_pk) + entity = container.publishable_entity + last_version = container.versioning.latest + assert last_version is not None + next_version_num = last_version.version_num + 1 + if publishable_entities_pks is None: + # We're only changing metadata. Keep the same entity list. + next_entity_list = last_version.entity_list + else: + if entity_version_pks is None: + entity_version_pks = [None] * len(publishable_entities_pks) + next_entity_list = create_entity_list_with_rows( + entity_pks=publishable_entities_pks, + entity_version_pks=entity_version_pks, + learning_package_id=entity.learning_package_id, + ) + next_container_version = _create_container_version( + container, + next_version_num, + title=title if title is not None else last_version.title, + entity_list=next_entity_list, + created=created, + created_by=created_by, + container_version_cls=container_version_cls, + ) + + return next_container_version + + +def get_container(pk: int) -> Container: + """ + [ πŸ›‘ UNSTABLE ] + Get a container by its primary key. + + Args: + pk: The primary key of the container. + + Returns: + The container with the given primary key. + """ + return Container.objects.get(pk=pk) + + +@dataclass(frozen=True) +class ContainerEntityListEntry: + """ + [ πŸ›‘ UNSTABLE ] + Data about a single entity in a container, e.g. a component in a unit. + """ + entity_version: PublishableEntityVersion + pinned: bool + + @property + def entity(self): + return self.entity_version.entity + + +def get_entities_in_container( + container: Container, + *, + published: bool, +) -> list[ContainerEntityListEntry]: + """ + [ πŸ›‘ UNSTABLE ] + Get the list of entities and their versions in the current draft or + published version of the given container. + + Args: + container: The Container, e.g. returned by `get_container()` + published: `True` if we want the published version of the container, or + `False` for the draft version. + """ + assert isinstance(container, Container) + container_version = container.versioning.published if published else container.versioning.draft + if container_version is None: + raise ContainerVersion.DoesNotExist # This container has not been published yet, or has been deleted. + assert isinstance(container_version, ContainerVersion) + entity_list = [] + for row in container_version.entity_list.entitylistrow_set.order_by("order_num"): + entity_version = row.entity_version # This will be set if pinned + if not entity_version: # If this entity is "unpinned", use the latest published/draft version: + entity_version = row.entity.published.version if published else row.entity.draft.version + if entity_version is not None: # As long as this hasn't been soft-deleted: + entity_list.append(ContainerEntityListEntry( + entity_version=entity_version, + pinned=row.entity_version is not None, + )) + # else we could indicate somehow a deleted item was here, e.g. by returning a ContainerEntityListEntry with + # deleted=True, but we don't have a use case for that yet. + return entity_list + + +def contains_unpublished_changes(container_id: int) -> bool: + """ + [ πŸ›‘ UNSTABLE ] + Check recursively if a container has any unpublished changes. + + Note: unlike this method, the similar-sounding + `container.versioning.has_unpublished_changes` property only reports + if the container itself has unpublished changes, not + if its contents do. So if you change a title or add a new child component, + `has_unpublished_changes` will be `True`, but if you merely edit a component + that's in the container, it will be `False`. This method will return `True` + in either case. + """ + # This is similar to 'get_container(container.container_id)' but pre-loads more data. + container = Container.objects.select_related( + "publishable_entity__draft__version__containerversion__entity_list", + ).get(pk=container_id) + + if container.versioning.has_unpublished_changes: + return True + + # We only care about children that are un-pinned, since published changes to pinned children don't matter + entity_list = container.versioning.draft.entity_list + + # This is a naive and inefficient implementation but should be correct. + # TODO: Once we have expanded the containers system to support multiple levels (not just Units and Components but + # also subsections and sections) and we have an expanded test suite for correctness, then we can optimize. + # We will likely change to a tracking-based approach rather than a "scan for changes" based approach. + for row in entity_list.entitylistrow_set.filter(entity_version=None).select_related( + "entity__container", + "entity__draft__version", + "entity__published__version", + ): + try: + child_container = row.entity.container + except Container.DoesNotExist: + child_container = None + if child_container: + # This is itself a container - check recursively: + if contains_unpublished_changes(child_container.pk): + return True + else: + # This is not a container: + draft_pk = row.entity.draft.version_id if row.entity.draft else None + published_pk = row.entity.published.version_id if hasattr(row.entity, "published") else None + if draft_pk != published_pk: + return True + return False + + +def get_containers_with_entity( + publishable_entity_pk: int, + *, + ignore_pinned=False, +) -> QuerySet[Container]: + """ + [ πŸ›‘ UNSTABLE ] + Find all draft containers that directly contain the given entity. + + They will always be from the same learning package; cross-package containers + are not allowed. + + Args: + publishable_entity_pk: The ID of the PublishableEntity to search for. + ignore_pinned: if true, ignore any pinned references to the entity. + """ + if ignore_pinned: + qs = Container.objects.filter( + # Note: these two conditions must be in the same filter() call, or the query won't be correct. + publishable_entity__draft__version__containerversion__entity_list__entitylistrow__entity_id=publishable_entity_pk, # pylint: disable=line-too-long # noqa: E501 + publishable_entity__draft__version__containerversion__entity_list__entitylistrow__entity_version_id=None, # pylint: disable=line-too-long # noqa: E501 + ).order_by("pk") # Ordering is mostly for consistent test cases. + else: + qs = Container.objects.filter( + publishable_entity__draft__version__containerversion__entity_list__entitylistrow__entity_id=publishable_entity_pk, # pylint: disable=line-too-long # noqa: E501 + ).order_by("pk") # Ordering is mostly for consistent test cases. + # Could alternately do this query in two steps. Not sure which is more efficient; depends on how the DB plans it. + # # Find all the EntityLists that contain the given entity: + # lists = EntityList.objects.filter(entitylistrow__entity_id=publishable_entity_pk).values_list('pk', flat=True) + # qs = Container.objects.filter( + # publishable_entity__draft__version__containerversion__entity_list__in=lists + # ) + return qs diff --git a/openedx_learning/apps/authoring/publishing/apps.py b/openedx_learning/apps/authoring/publishing/apps.py index bfa1bbe6..f5a8b1f9 100644 --- a/openedx_learning/apps/authoring/publishing/apps.py +++ b/openedx_learning/apps/authoring/publishing/apps.py @@ -14,3 +14,12 @@ class PublishingConfig(AppConfig): verbose_name = "Learning Core > Authoring > Publishing" default_auto_field = "django.db.models.BigAutoField" label = "oel_publishing" + + def ready(self): + """ + Register Container and ContainerVersion. + """ + from .api import register_content_models # pylint: disable=import-outside-toplevel + from .models import Container, ContainerVersion # pylint: disable=import-outside-toplevel + + register_content_models(Container, ContainerVersion) diff --git a/openedx_learning/apps/authoring/publishing/migrations/0003_containers.py b/openedx_learning/apps/authoring/publishing/migrations/0003_containers.py new file mode 100644 index 00000000..55e96777 --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/migrations/0003_containers.py @@ -0,0 +1,54 @@ +# Generated by Django 4.2.19 on 2025-03-11 04:10 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('oel_publishing', '0002_alter_learningpackage_key_and_more'), + ] + + operations = [ + migrations.CreateModel( + name='Container', + fields=[ + ('publishable_entity', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, serialize=False, to='oel_publishing.publishableentity')), + ], + options={ + 'abstract': False, + }, + ), + migrations.CreateModel( + name='EntityList', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ], + ), + migrations.CreateModel( + name='EntityListRow', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('order_num', models.PositiveIntegerField()), + ('entity', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, to='oel_publishing.publishableentity')), + ('entity_list', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='oel_publishing.entitylist')), + ('entity_version', models.ForeignKey(null=True, on_delete=django.db.models.deletion.RESTRICT, related_name='+', to='oel_publishing.publishableentityversion')), + ], + ), + migrations.CreateModel( + name='ContainerVersion', + fields=[ + ('publishable_entity_version', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, serialize=False, to='oel_publishing.publishableentityversion')), + ('container', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='versions', to='oel_publishing.container')), + ('entity_list', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, related_name='container_versions', to='oel_publishing.entitylist')), + ], + options={ + 'abstract': False, + }, + ), + migrations.AddConstraint( + model_name='entitylistrow', + constraint=models.UniqueConstraint(fields=('entity_list', 'order_num'), name='oel_publishing_elist_row_order'), + ), + ] diff --git a/openedx_learning/apps/authoring/publishing/models.py b/openedx_learning/apps/authoring/publishing/models.py deleted file mode 100644 index c8b1c34f..00000000 --- a/openedx_learning/apps/authoring/publishing/models.py +++ /dev/null @@ -1,517 +0,0 @@ -""" -The data models here are intended to be used by other apps to publish different -types of content, such as Components, Units, Sections, etc. These models should -support the logic for the management of the publishing process: - -* The relationship between publishable entities and their many versions. -* The management of drafts. -* Publishing specific versions of publishable entities. -* Finding the currently published versions. -* The act of publishing, and doing so atomically. -* Managing reverts. -* Storing and querying publish history. -""" -from django.conf import settings -from django.core.validators import MinValueValidator -from django.db import models - -from openedx_learning.lib.fields import ( - MultiCollationTextField, - case_insensitive_char_field, - immutable_uuid_field, - key_field, - manual_date_time_field, -) - -__all__ = [ - "LearningPackage", - "PublishableEntity", - "PublishableEntityVersion", - "Draft", - "PublishLog", - "PublishLogRecord", - "Published", -] - - -class LearningPackage(models.Model): - """ - Top level container for a grouping of authored content. - - Each PublishableEntity belongs to exactly one LearningPackage. - """ - # Explictly declare a 4-byte ID instead of using the app-default 8-byte ID. - # We do not expect to have more than 2 billion LearningPackages on a given - # site. Furthermore, many, many things have foreign keys to this model and - # uniqueness indexes on those foreign keys + their own fields, so the 4 - # bytes saved will add up over time. - id = models.AutoField(primary_key=True) - - uuid = immutable_uuid_field() - - # "key" is a reserved word for MySQL, so we're temporarily using the column - # name of "_key" to avoid breaking downstream tooling. There's an open - # question as to whether this field needs to exist at all, or whether the - # top level library key it's currently used for should be entirely in the - # LibraryContent model. - key = key_field(db_column="_key") - - title = case_insensitive_char_field(max_length=500, blank=False) - - # TODO: We should probably defer this field, since many things pull back - # LearningPackage as select_related. Usually those relations only care about - # the UUID and key, so maybe it makes sense to separate the model at some - # point. - description = MultiCollationTextField( - blank=True, - null=False, - default="", - max_length=10_000, - # We don't really expect to ever sort by the text column, but we may - # want to do case-insensitive searches, so it's useful to have a case - # and accent insensitive collation. - db_collations={ - "sqlite": "NOCASE", - "mysql": "utf8mb4_unicode_ci", - } - ) - - created = manual_date_time_field() - updated = manual_date_time_field() - - def __str__(self): - return f"{self.key}" - - class Meta: - constraints = [ - # LearningPackage keys must be globally unique. This is something - # that might be relaxed in the future if this system were to be - # extensible to something like multi-tenancy, in which case we'd tie - # it to something like a Site or Org. - models.UniqueConstraint( - fields=["key"], - name="oel_publishing_lp_uniq_key", - ) - ] - verbose_name = "Learning Package" - verbose_name_plural = "Learning Packages" - - -class PublishableEntity(models.Model): - """ - This represents any publishable thing that has ever existed in a - LearningPackage. It serves as a stable model that will not go away even if - these things are later unpublished or deleted. - - A PublishableEntity belongs to exactly one LearningPackage. - - Examples of Publishable Entities - -------------------------------- - - Components (e.g. VideoBlock, ProblemBlock), Units, and Sections/Subsections - would all be considered Publishable Entites. But anything that can be - imported, exported, published, and reverted in a course or library could be - modeled as a PublishableEntity, including things like Grading Policy or - possibly Taxonomies (?). - - How to use this model - --------------------- - - The publishing app understands that publishable entities exist, along with - their drafts and published versions. It has some basic metadata, such as - identifiers, who created it, and when it was created. It's meant to - encapsulate the draft and publishing related aspects of your content, but - the ``publishing`` app doesn't know anything about the actual content being - referenced. - - You have to provide actual meaning to PublishableEntity by creating your own - models that will represent your particular content and associating them to - PublishableEntity via a OneToOneField with primary_key=True. The easiest way - to do this is to have your model inherit from PublishableEntityMixin. - - Identifiers - ----------- - The UUID is globally unique and should be treated as immutable. - - The key field *is* mutable, but changing it will affect all - PublishedEntityVersions. They are locally unique within the LearningPackage. - - If you are referencing this model from within the same process, use a - foreign key to the id. If you are referencing this PublishedEntity from an - external system/service, use the UUID. The key is the part that is most - likely to be human-readable, and may be exported/copied, but try not to rely - on it, since this value may change. - - Note: When we actually implement the ability to change identifiers, we - should make a history table and a modified attribute on this model. - - Why are Identifiers in this Model? - ---------------------------------- - - A PublishableEntity never stands alone–it's always intended to be used with - a 1:1 model like Component or Unit. So why have all the identifiers in this - model instead of storing them in those other models? Two reasons: - - * Published things need to have the right identifiers so they can be used - throughout the system, and the UUID is serving the role of ISBN in physical - book publishing. - * We want to be able to enforce the idea that "key" is locally unique across - all PublishableEntities within a given LearningPackage. Component and Unit - can't do that without a shared model. - - That being said, models that build on PublishableEntity are free to add - their own identifiers if it's useful to do so. - - Why not Inherit from this Model? - -------------------------------- - - Django supports multi-table inheritance: - - https://docs.djangoproject.com/en/4.2/topics/db/models/#multi-table-inheritance - - We don't use that, primarily because we want to more clearly decouple - publishing concerns from the rest of the logic around Components, Units, - etc. If you made a Component and ComponentVersion models that subclassed - PublishableEntity and PublishableEntityVersion, and then accessed - ``component.versions``, you might expect ComponentVersions to come back and - be surprised when you get EntityVersions instead. - - In general, we want freedom to add new Publishing models, fields, and - methods without having to worry about the downstream name collisions with - other apps (many of which might live in other repositories). The helper - mixins will provide a little syntactic sugar to make common access patterns - more convenient, like file access. - """ - - uuid = immutable_uuid_field() - learning_package = models.ForeignKey( - LearningPackage, - on_delete=models.CASCADE, - related_name="publishable_entities", - ) - - # "key" is a reserved word for MySQL, so we're temporarily using the column - # name of "_key" to avoid breaking downstream tooling. Consider renaming - # this later. - key = key_field(db_column="_key") - - created = manual_date_time_field() - created_by = models.ForeignKey( - settings.AUTH_USER_MODEL, - on_delete=models.SET_NULL, - null=True, - blank=True, - ) - - class Meta: - constraints = [ - # Keys are unique within a given LearningPackage. - models.UniqueConstraint( - fields=[ - "learning_package", - "key", - ], - name="oel_pub_ent_uniq_lp_key", - ) - ] - indexes = [ - # Global Key Index: - # * Search by key across all PublishableEntities on the site. This - # would be a support-oriented tool from Django Admin. - models.Index( - fields=["key"], - name="oel_pub_ent_idx_key", - ), - # LearningPackage (reverse) Created Index: - # * Search for most recently *created* PublishableEntities for a - # given LearningPackage, since they're the most likely to be - # actively worked on. - models.Index( - fields=["learning_package", "-created"], - name="oel_pub_ent_idx_lp_rcreated", - ), - ] - # These are for the Django Admin UI. - verbose_name = "Publishable Entity" - verbose_name_plural = "Publishable Entities" - - def __str__(self): - return f"{self.key}" - - -class PublishableEntityVersion(models.Model): - """ - A particular version of a PublishableEntity. - - This model has its own ``uuid`` so that it can be referenced directly. The - ``uuid`` should be treated as immutable. - - PublishableEntityVersions are created once and never updated. So for - instance, the ``title`` should never be modified. - - Like PublishableEntity, the data in this model is only enough to cover the - parts that are most important for the actual process of managing drafts and - publishes. You will want to create your own models to represent the actual - content data that's associated with this PublishableEntityVersion, and - connect them using a OneToOneField with primary_key=True. The easiest way to - do this is to inherit from PublishableEntityVersionMixin. Be sure to treat - these versioned models in your app as immutable as well. - """ - - uuid = immutable_uuid_field() - entity = models.ForeignKey( - PublishableEntity, on_delete=models.CASCADE, related_name="versions" - ) - - # Most publishable things will have some sort of title, but blanks are - # allowed for those that don't require one. - title = case_insensitive_char_field(max_length=500, blank=True, default="") - - # The version_num starts at 1 and increments by 1 with each new version for - # a given PublishableEntity. Doing it this way makes it more convenient for - # users to refer to than a hash or UUID value. It also helps us catch race - # conditions on save, by setting a unique constraint on the entity and - # version_num. - version_num = models.PositiveIntegerField( - null=False, - validators=[MinValueValidator(1)], - ) - - # All PublishableEntityVersions created as part of the same publish should - # have the exact same created datetime (not off by a handful of - # microseconds). - created = manual_date_time_field() - - # User who created the PublishableEntityVersion. This can be null if the - # user is later removed. Open edX in general doesn't let you remove users, - # but we should try to model it so that this is possible eventually. - created_by = models.ForeignKey( - settings.AUTH_USER_MODEL, - on_delete=models.SET_NULL, - null=True, - blank=True, - ) - - class Meta: - constraints = [ - # Prevent the situation where we have multiple - # PublishableEntityVersions claiming to be the same version_num for - # a given PublishableEntity. This can happen if there's a race - # condition between concurrent editors in different browsers, - # working on the same Publishable. With this constraint, one of - # those processes will raise an IntegrityError. - models.UniqueConstraint( - fields=[ - "entity", - "version_num", - ], - name="oel_pv_uniq_entity_version_num", - ) - ] - indexes = [ - # LearningPackage (reverse) Created Index: - # * Make it cheap to find the most recently created - # PublishableEntityVersions for a given LearningPackage. This - # represents the most recently saved work for a LearningPackage - # and would be the most likely areas to get worked on next. - models.Index( - fields=["entity", "-created"], - name="oel_pv_idx_entity_rcreated", - ), - # Title Index: - # * Search by title. - models.Index( - fields=[ - "title", - ], - name="oel_pv_idx_title", - ), - ] - - # These are for the Django Admin UI. - verbose_name = "Publishable Entity Version" - verbose_name_plural = "Publishable Entity Versions" - - -class Draft(models.Model): - """ - Find the active draft version of an entity (usually most recently created). - - This model mostly only exists to allow us to join against a bunch of - PublishableEntity objects at once and get all their latest drafts. You might - use this together with Published in order to see which Drafts haven't been - published yet. - - A Draft entry should be created whenever a new PublishableEntityVersion is - created. This means there are three possible states: - - 1. No Draft entry for a PublishableEntity: This means a PublishableEntity - was created, but no PublishableEntityVersion was ever made for it, so - there was never a Draft version. - 2. A Draft entry exists and points to a PublishableEntityVersion: This is - the most common state. - 3. A Draft entry exists and points to a null version: This means a version - used to be the draft, but it's been functionally "deleted". The versions - still exist in our history, but we're done using it. - - It would have saved a little space to add this data to the Published model - (and possibly call the combined model something else). Split Modulestore did - this with its active_versions table. I keep it separate here to get a better - separation of lifecycle events: i.e. this table *only* changes when drafts - are updated, not when publishing happens. The Published model only changes - when something is published. - """ - # If we're removing a PublishableEntity entirely, also remove the Draft - # entry for it. This isn't a normal operation, but can happen if you're - # deleting an entire LearningPackage. - entity = models.OneToOneField( - PublishableEntity, - on_delete=models.CASCADE, - primary_key=True, - ) - version = models.OneToOneField( - PublishableEntityVersion, - on_delete=models.RESTRICT, - null=True, - blank=True, - ) - - -class PublishLog(models.Model): - """ - There is one row in this table for every time content is published. - - Each PublishLog has 0 or more PublishLogRecords describing exactly which - PublishableEntites were published and what the version changes are. A - PublishLog is like a git commit in that sense, with individual - PublishLogRecords representing the files changed. - - Open question: Empty publishes are allowed at this time, and might be useful - for "fake" publishes that are necessary to invoke other post-publish - actions. It's not clear at this point how useful this will actually be. - - The absence of a ``version_num`` field in this model is intentional, because - having one would potentially cause write contention/locking issues when - there are many people working on different entities in a very large library. - We already see some contention issues occuring in ModuleStore for courses, - and we want to support Libraries that are far larger. - - If you need a LearningPackage-wide indicator for version and the only thing - you care about is "has *something* changed?", you can make a foreign key to - the most recent PublishLog, or use the most recent PublishLog's primary key. - This should be monotonically increasing, though there will be large gaps in - values, e.g. (5, 190, 1291, etc.). Be warned that this value will not port - across sites. If you need site-portability, the UUIDs for this model are a - safer bet, though there's a lot about import/export that we haven't fully - mapped out yet. - """ - - uuid = immutable_uuid_field() - learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE) - message = case_insensitive_char_field(max_length=500, blank=True, default="") - published_at = manual_date_time_field() - published_by = models.ForeignKey( - settings.AUTH_USER_MODEL, - on_delete=models.SET_NULL, - null=True, - blank=True, - ) - - class Meta: - verbose_name = "Publish Log" - verbose_name_plural = "Publish Logs" - - -class PublishLogRecord(models.Model): - """ - A record for each publishable entity version changed, for each publish. - - To revert a publish, we would make a new publish that swaps ``old_version`` - and ``new_version`` field values. - """ - - publish_log = models.ForeignKey( - PublishLog, - on_delete=models.CASCADE, - related_name="records", - ) - entity = models.ForeignKey(PublishableEntity, on_delete=models.RESTRICT) - old_version = models.ForeignKey( - PublishableEntityVersion, - on_delete=models.RESTRICT, - null=True, - blank=True, - related_name="+", - ) - new_version = models.ForeignKey( - PublishableEntityVersion, on_delete=models.RESTRICT, null=True, blank=True - ) - - class Meta: - constraints = [ - # A Publishable can have only one PublishLogRecord per PublishLog. - # You can't simultaneously publish two different versions of the - # same publishable. - models.UniqueConstraint( - fields=[ - "publish_log", - "entity", - ], - name="oel_plr_uniq_pl_publishable", - ) - ] - indexes = [ - # Publishable (reverse) Publish Log Index: - # * Find the history of publishes for a given Publishable, - # starting with the most recent (since IDs are ascending ints). - models.Index( - fields=["entity", "-publish_log"], - name="oel_plr_idx_entity_rplr", - ), - ] - verbose_name = "Publish Log Record" - verbose_name_plural = "Publish Log Records" - - -class Published(models.Model): - """ - Find the currently published version of an entity. - - Notes: - - * There is only ever one published PublishableEntityVersion per - PublishableEntity at any given time. - * It may be possible for a PublishableEntity to exist only as a Draft (and thus - not show up in this table). - * If a row exists for a PublishableEntity, but the ``version`` field is - None, it means that the entity was published at some point, but is no - longer published now–i.e. it's functionally "deleted", even though all - the version history is preserved behind the scenes. - - TODO: Do we need to create a (redundant) title field in this model so that - we can more efficiently search across titles within a LearningPackage? - Probably not an immediate concern because the number of rows currently - shouldn't be > 10,000 in the more extreme cases. - - TODO: Do we need to make a "most_recently" published version when an entry - is unpublished/deleted? - """ - - entity = models.OneToOneField( - PublishableEntity, - on_delete=models.CASCADE, - primary_key=True, - ) - version = models.OneToOneField( - PublishableEntityVersion, - on_delete=models.RESTRICT, - null=True, - ) - publish_log_record = models.ForeignKey( - PublishLogRecord, - on_delete=models.RESTRICT, - ) - - class Meta: - verbose_name = "Published Entity" - verbose_name_plural = "Published Entities" diff --git a/openedx_learning/apps/authoring/publishing/models/__init__.py b/openedx_learning/apps/authoring/publishing/models/__init__.py new file mode 100644 index 00000000..27329076 --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/models/__init__.py @@ -0,0 +1,27 @@ +""" +The data models here are intended to be used by other apps to publish different +types of content, such as Components, Units, Sections, etc. These models should +support the logic for the management of the publishing process: + +* The relationship between publishable entities and their many versions. +* Hierarchical relationships between "container" entities and their children +* The management of drafts. +* Publishing specific versions of publishable entities. +* Finding the currently published versions. +* The act of publishing, and doing so atomically. +* Managing reverts. +* Storing and querying publish history. +""" + +from .container import Container, ContainerVersion +from .draft_published import Draft, Published +from .entity_list import EntityList, EntityListRow +from .learning_package import LearningPackage +from .publish_log import PublishLog, PublishLogRecord +from .publishable_entity import ( + PublishableContentModelRegistry, + PublishableEntity, + PublishableEntityMixin, + PublishableEntityVersion, + PublishableEntityVersionMixin, +) diff --git a/openedx_learning/apps/authoring/publishing/models/container.py b/openedx_learning/apps/authoring/publishing/models/container.py new file mode 100644 index 00000000..e34bb6a7 --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/models/container.py @@ -0,0 +1,70 @@ +""" +Container and ContainerVersion models +""" +from django.core.exceptions import ValidationError +from django.db import models + +from .entity_list import EntityList +from .publishable_entity import PublishableEntityMixin, PublishableEntityVersionMixin + + +class Container(PublishableEntityMixin): + """ + A Container is a type of PublishableEntity that holds other + PublishableEntities. For example, a "Unit" Container might hold several + Components. + + For now, all containers have a static "entity list" that defines which + containers/components/enities they hold. As we complete the Containers API, + we will also add support for dynamic containers which may contain different + entities for different learners or at different times. + + NOTE: We're going to want to eventually have some association between the + PublishLog and Containers that were affected in a publish because their + child elements were published. + """ + + +class ContainerVersion(PublishableEntityVersionMixin): + """ + A version of a Container. + + By convention, we would only want to create new versions when the Container + itself changes, and not when the Container's child elements change. For + example: + + * Something was added to the Container. + * We re-ordered the rows in the container. + * Something was removed to the container. + * The Container's metadata changed, e.g. the title. + * We pin to different versions of the Container. + + The last looks a bit odd, but it's because *how we've defined the Unit* has + changed if we decide to explicitly pin a set of versions for the children, + and then later change our minds and move to a different set. It also just + makes things easier to reason about if we say that entity_list never + changes for a given ContainerVersion. + """ + + container = models.ForeignKey( + Container, + on_delete=models.CASCADE, + related_name="versions", + ) + + # The list of entities (frozen and/or unfrozen) in this container + entity_list = models.ForeignKey( + EntityList, + on_delete=models.RESTRICT, + null=False, + related_name="container_versions", + ) + + def clean(self): + """ + Validate this model before saving. Not called normally, but will be + called if anything is edited via a ModelForm like the Django admin. + """ + super().clean() + if self.container_id != self.publishable_entity_version.entity.container.pk: # pylint: disable=no-member + raise ValidationError("Inconsistent foreign keys to Container") diff --git a/openedx_learning/apps/authoring/publishing/models/draft_published.py b/openedx_learning/apps/authoring/publishing/models/draft_published.py new file mode 100644 index 00000000..c945d807 --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/models/draft_published.py @@ -0,0 +1,95 @@ +""" +Draft and Published models +""" +from django.db import models + +from .publish_log import PublishLogRecord +from .publishable_entity import PublishableEntity, PublishableEntityVersion + + +class Draft(models.Model): + """ + Find the active draft version of an entity (usually most recently created). + + This model mostly only exists to allow us to join against a bunch of + PublishableEntity objects at once and get all their latest drafts. You might + use this together with Published in order to see which Drafts haven't been + published yet. + + A Draft entry should be created whenever a new PublishableEntityVersion is + created. This means there are three possible states: + + 1. No Draft entry for a PublishableEntity: This means a PublishableEntity + was created, but no PublishableEntityVersion was ever made for it, so + there was never a Draft version. + 2. A Draft entry exists and points to a PublishableEntityVersion: This is + the most common state. + 3. A Draft entry exists and points to a null version: This means a version + used to be the draft, but it's been functionally "deleted". The versions + still exist in our history, but we're done using it. + + It would have saved a little space to add this data to the Published model + (and possibly call the combined model something else). Split Modulestore did + this with its active_versions table. I keep it separate here to get a better + separation of lifecycle events: i.e. this table *only* changes when drafts + are updated, not when publishing happens. The Published model only changes + when something is published. + """ + # If we're removing a PublishableEntity entirely, also remove the Draft + # entry for it. This isn't a normal operation, but can happen if you're + # deleting an entire LearningPackage. + entity = models.OneToOneField( + PublishableEntity, + on_delete=models.CASCADE, + primary_key=True, + ) + version = models.OneToOneField( + PublishableEntityVersion, + on_delete=models.RESTRICT, + null=True, + blank=True, + ) + + +class Published(models.Model): + """ + Find the currently published version of an entity. + + Notes: + + * There is only ever one published PublishableEntityVersion per + PublishableEntity at any given time. + * It may be possible for a PublishableEntity to exist only as a Draft (and thus + not show up in this table). + * If a row exists for a PublishableEntity, but the ``version`` field is + None, it means that the entity was published at some point, but is no + longer published now–i.e. it's functionally "deleted", even though all + the version history is preserved behind the scenes. + + TODO: Do we need to create a (redundant) title field in this model so that + we can more efficiently search across titles within a LearningPackage? + Probably not an immediate concern because the number of rows currently + shouldn't be > 10,000 in the more extreme cases. + + TODO: Do we need to make a "most_recently" published version when an entry + is unpublished/deleted? + """ + + entity = models.OneToOneField( + PublishableEntity, + on_delete=models.CASCADE, + primary_key=True, + ) + version = models.OneToOneField( + PublishableEntityVersion, + on_delete=models.RESTRICT, + null=True, + ) + publish_log_record = models.ForeignKey( + PublishLogRecord, + on_delete=models.RESTRICT, + ) + + class Meta: + verbose_name = "Published Entity" + verbose_name_plural = "Published Entities" diff --git a/openedx_learning/apps/authoring/publishing/models/entity_list.py b/openedx_learning/apps/authoring/publishing/models/entity_list.py new file mode 100644 index 00000000..af7d0eca --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/models/entity_list.py @@ -0,0 +1,69 @@ +""" +Entity List models +""" +from django.db import models + +from .publishable_entity import PublishableEntity, PublishableEntityVersion + + +class EntityList(models.Model): + """ + EntityLists are a common structure to hold parent-child relations. + + EntityLists are not PublishableEntities in and of themselves. That's because + sometimes we'll want the same kind of data structure for things that we + dynamically generate for individual students (e.g. Variants). EntityLists are + anonymous in a sense–they're pointed to by ContainerVersions and + other models, rather than being looked up by their own identifiers. + """ + + +class EntityListRow(models.Model): + """ + Each EntityListRow points to a PublishableEntity, optionally at a specific + version. + + There is a row in this table for each member of an EntityList. The order_num + field is used to determine the order of the members in the list. + """ + + entity_list = models.ForeignKey(EntityList, on_delete=models.CASCADE) + + # This ordering should be treated as immutable–if the ordering needs to + # change, we create a new EntityList and copy things over. + order_num = models.PositiveIntegerField() + + # Simple case would use these fields with our convention that null versions + # means "get the latest draft or published as appropriate". These entities + # could be Selectors, in which case we'd need to do more work to find the right + # variant. The publishing app itself doesn't know anything about Selectors + # however, and just treats it as another PublishableEntity. + entity = models.ForeignKey(PublishableEntity, on_delete=models.RESTRICT) + + # The version references point to the specific PublishableEntityVersion that + # this EntityList has for this PublishableEntity for both the draft and + # published states. However, we don't want to have to create new EntityList + # every time that a member is updated, because that would waste a lot of + # space and make it difficult to figure out when the metadata of something + # like a Unit *actually* changed, vs. when its child members were being + # updated. Doing so could also potentially lead to race conditions when + # updating multiple layers of containers. + # + # So our approach to this is to use a value of None (null) to represent an + # unpinned reference to a PublishableEntity. It's shorthand for "just use + # the latest draft or published version of this, as appropriate". + entity_version = models.ForeignKey( + PublishableEntityVersion, + on_delete=models.RESTRICT, + null=True, + related_name="+", # Do we need the reverse relation? + ) + + class Meta: + constraints = [ + # If (entity_list, order_num) is not unique, it likely indicates a race condition - so force uniqueness. + models.UniqueConstraint( + fields=["entity_list", "order_num"], + name="oel_publishing_elist_row_order", + ), + ] diff --git a/openedx_learning/apps/authoring/publishing/models/learning_package.py b/openedx_learning/apps/authoring/publishing/models/learning_package.py new file mode 100644 index 00000000..3ff5bb4b --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/models/learning_package.py @@ -0,0 +1,75 @@ +""" +LearningPackage model +""" +from django.db import models + +from openedx_learning.lib.fields import ( + MultiCollationTextField, + case_insensitive_char_field, + immutable_uuid_field, + key_field, + manual_date_time_field, +) + + +class LearningPackage(models.Model): + """ + Top level container for a grouping of authored content. + + Each PublishableEntity belongs to exactly one LearningPackage. + """ + # Explictly declare a 4-byte ID instead of using the app-default 8-byte ID. + # We do not expect to have more than 2 billion LearningPackages on a given + # site. Furthermore, many, many things have foreign keys to this model and + # uniqueness indexes on those foreign keys + their own fields, so the 4 + # bytes saved will add up over time. + id = models.AutoField(primary_key=True) + + uuid = immutable_uuid_field() + + # "key" is a reserved word for MySQL, so we're temporarily using the column + # name of "_key" to avoid breaking downstream tooling. There's an open + # question as to whether this field needs to exist at all, or whether the + # top level library key it's currently used for should be entirely in the + # LibraryContent model. + key = key_field(db_column="_key") + + title = case_insensitive_char_field(max_length=500, blank=False) + + # TODO: We should probably defer this field, since many things pull back + # LearningPackage as select_related. Usually those relations only care about + # the UUID and key, so maybe it makes sense to separate the model at some + # point. + description = MultiCollationTextField( + blank=True, + null=False, + default="", + max_length=10_000, + # We don't really expect to ever sort by the text column, but we may + # want to do case-insensitive searches, so it's useful to have a case + # and accent insensitive collation. + db_collations={ + "sqlite": "NOCASE", + "mysql": "utf8mb4_unicode_ci", + } + ) + + created = manual_date_time_field() + updated = manual_date_time_field() + + def __str__(self): + return f"{self.key}" + + class Meta: + constraints = [ + # LearningPackage keys must be globally unique. This is something + # that might be relaxed in the future if this system were to be + # extensible to something like multi-tenancy, in which case we'd tie + # it to something like a Site or Org. + models.UniqueConstraint( + fields=["key"], + name="oel_publishing_lp_uniq_key", + ) + ] + verbose_name = "Learning Package" + verbose_name_plural = "Learning Packages" diff --git a/openedx_learning/apps/authoring/publishing/models/publish_log.py b/openedx_learning/apps/authoring/publishing/models/publish_log.py new file mode 100644 index 00000000..e71f08be --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/models/publish_log.py @@ -0,0 +1,106 @@ +""" +PublishLog and PublishLogRecord models +""" +from django.conf import settings +from django.db import models + +from openedx_learning.lib.fields import case_insensitive_char_field, immutable_uuid_field, manual_date_time_field + +from .learning_package import LearningPackage +from .publishable_entity import PublishableEntity, PublishableEntityVersion + + +class PublishLog(models.Model): + """ + There is one row in this table for every time content is published. + + Each PublishLog has 0 or more PublishLogRecords describing exactly which + PublishableEntites were published and what the version changes are. A + PublishLog is like a git commit in that sense, with individual + PublishLogRecords representing the files changed. + + Open question: Empty publishes are allowed at this time, and might be useful + for "fake" publishes that are necessary to invoke other post-publish + actions. It's not clear at this point how useful this will actually be. + + The absence of a ``version_num`` field in this model is intentional, because + having one would potentially cause write contention/locking issues when + there are many people working on different entities in a very large library. + We already see some contention issues occuring in ModuleStore for courses, + and we want to support Libraries that are far larger. + + If you need a LearningPackage-wide indicator for version and the only thing + you care about is "has *something* changed?", you can make a foreign key to + the most recent PublishLog, or use the most recent PublishLog's primary key. + This should be monotonically increasing, though there will be large gaps in + values, e.g. (5, 190, 1291, etc.). Be warned that this value will not port + across sites. If you need site-portability, the UUIDs for this model are a + safer bet, though there's a lot about import/export that we haven't fully + mapped out yet. + """ + + uuid = immutable_uuid_field() + learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE) + message = case_insensitive_char_field(max_length=500, blank=True, default="") + published_at = manual_date_time_field() + published_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.SET_NULL, + null=True, + blank=True, + ) + + class Meta: + verbose_name = "Publish Log" + verbose_name_plural = "Publish Logs" + + +class PublishLogRecord(models.Model): + """ + A record for each publishable entity version changed, for each publish. + + To revert a publish, we would make a new publish that swaps ``old_version`` + and ``new_version`` field values. + """ + + publish_log = models.ForeignKey( + PublishLog, + on_delete=models.CASCADE, + related_name="records", + ) + entity = models.ForeignKey(PublishableEntity, on_delete=models.RESTRICT) + old_version = models.ForeignKey( + PublishableEntityVersion, + on_delete=models.RESTRICT, + null=True, + blank=True, + related_name="+", + ) + new_version = models.ForeignKey( + PublishableEntityVersion, on_delete=models.RESTRICT, null=True, blank=True + ) + + class Meta: + constraints = [ + # A Publishable can have only one PublishLogRecord per PublishLog. + # You can't simultaneously publish two different versions of the + # same publishable. + models.UniqueConstraint( + fields=[ + "publish_log", + "entity", + ], + name="oel_plr_uniq_pl_publishable", + ) + ] + indexes = [ + # Publishable (reverse) Publish Log Index: + # * Find the history of publishes for a given Publishable, + # starting with the most recent (since IDs are ascending ints). + models.Index( + fields=["entity", "-publish_log"], + name="oel_plr_idx_entity_rplr", + ), + ] + verbose_name = "Publish Log Record" + verbose_name_plural = "Publish Log Records" diff --git a/openedx_learning/apps/authoring/publishing/model_mixins.py b/openedx_learning/apps/authoring/publishing/models/publishable_entity.py similarity index 52% rename from openedx_learning/apps/authoring/publishing/model_mixins.py rename to openedx_learning/apps/authoring/publishing/models/publishable_entity.py index f94aa1b6..e05172bf 100644 --- a/openedx_learning/apps/authoring/publishing/model_mixins.py +++ b/openedx_learning/apps/authoring/publishing/models/publishable_entity.py @@ -1,22 +1,260 @@ """ -Helper mixin classes for content apps that want to use the publishing app. +PublishableEntity model and PublishableEntityVersion + mixins """ -from __future__ import annotations - from datetime import datetime from functools import cached_property +from typing import ClassVar, Self +from django.conf import settings from django.core.exceptions import ImproperlyConfigured +from django.core.validators import MinValueValidator from django.db import models -from django.db.models.query import QuerySet -from .models import PublishableEntity, PublishableEntityVersion +from openedx_learning.lib.fields import ( + case_insensitive_char_field, + immutable_uuid_field, + key_field, + manual_date_time_field, +) +from openedx_learning.lib.managers import WithRelationsManager + +from .learning_package import LearningPackage + + +class PublishableEntity(models.Model): + """ + This represents any publishable thing that has ever existed in a + LearningPackage. It serves as a stable model that will not go away even if + these things are later unpublished or deleted. + + A PublishableEntity belongs to exactly one LearningPackage. + + Examples of Publishable Entities + -------------------------------- + + Components (e.g. VideoBlock, ProblemBlock), Units, and Sections/Subsections + would all be considered Publishable Entites. But anything that can be + imported, exported, published, and reverted in a course or library could be + modeled as a PublishableEntity, including things like Grading Policy or + possibly Taxonomies (?). + + How to use this model + --------------------- + + The publishing app understands that publishable entities exist, along with + their drafts and published versions. It has some basic metadata, such as + identifiers, who created it, and when it was created. It's meant to + encapsulate the draft and publishing related aspects of your content, but + the ``publishing`` app doesn't know anything about the actual content being + referenced. + + You have to provide actual meaning to PublishableEntity by creating your own + models that will represent your particular content and associating them to + PublishableEntity via a OneToOneField with primary_key=True. The easiest way + to do this is to have your model inherit from PublishableEntityMixin. + + Identifiers + ----------- + The UUID is globally unique and should be treated as immutable. + + The key field *is* mutable, but changing it will affect all + PublishedEntityVersions. They are locally unique within the LearningPackage. + + If you are referencing this model from within the same process, use a + foreign key to the id. If you are referencing this PublishedEntity from an + external system/service, use the UUID. The key is the part that is most + likely to be human-readable, and may be exported/copied, but try not to rely + on it, since this value may change. + + Note: When we actually implement the ability to change identifiers, we + should make a history table and a modified attribute on this model. + + Why are Identifiers in this Model? + ---------------------------------- + + A PublishableEntity never stands alone–it's always intended to be used with + a 1:1 model like Component or Unit. So why have all the identifiers in this + model instead of storing them in those other models? Two reasons: + + * Published things need to have the right identifiers so they can be used + throughout the system, and the UUID is serving the role of ISBN in physical + book publishing. + * We want to be able to enforce the idea that "key" is locally unique across + all PublishableEntities within a given LearningPackage. Component and Unit + can't do that without a shared model. + + That being said, models that build on PublishableEntity are free to add + their own identifiers if it's useful to do so. + + Why not Inherit from this Model? + -------------------------------- + + Django supports multi-table inheritance: + + https://docs.djangoproject.com/en/4.2/topics/db/models/#multi-table-inheritance + + We don't use that, primarily because we want to more clearly decouple + publishing concerns from the rest of the logic around Components, Units, + etc. If you made a Component and ComponentVersion models that subclassed + PublishableEntity and PublishableEntityVersion, and then accessed + ``component.versions``, you might expect ComponentVersions to come back and + be surprised when you get EntityVersions instead. + + In general, we want freedom to add new Publishing models, fields, and + methods without having to worry about the downstream name collisions with + other apps (many of which might live in other repositories). The helper + mixins will provide a little syntactic sugar to make common access patterns + more convenient, like file access. + """ + + uuid = immutable_uuid_field() + learning_package = models.ForeignKey( + LearningPackage, + on_delete=models.CASCADE, + related_name="publishable_entities", + ) + + # "key" is a reserved word for MySQL, so we're temporarily using the column + # name of "_key" to avoid breaking downstream tooling. Consider renaming + # this later. + key = key_field(db_column="_key") + + created = manual_date_time_field() + created_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.SET_NULL, + null=True, + blank=True, + ) + + class Meta: + constraints = [ + # Keys are unique within a given LearningPackage. + models.UniqueConstraint( + fields=[ + "learning_package", + "key", + ], + name="oel_pub_ent_uniq_lp_key", + ) + ] + indexes = [ + # Global Key Index: + # * Search by key across all PublishableEntities on the site. This + # would be a support-oriented tool from Django Admin. + models.Index( + fields=["key"], + name="oel_pub_ent_idx_key", + ), + # LearningPackage (reverse) Created Index: + # * Search for most recently *created* PublishableEntities for a + # given LearningPackage, since they're the most likely to be + # actively worked on. + models.Index( + fields=["learning_package", "-created"], + name="oel_pub_ent_idx_lp_rcreated", + ), + ] + # These are for the Django Admin UI. + verbose_name = "Publishable Entity" + verbose_name_plural = "Publishable Entities" + + def __str__(self): + return f"{self.key}" + + +class PublishableEntityVersion(models.Model): + """ + A particular version of a PublishableEntity. + + This model has its own ``uuid`` so that it can be referenced directly. The + ``uuid`` should be treated as immutable. + + PublishableEntityVersions are created once and never updated. So for + instance, the ``title`` should never be modified. + + Like PublishableEntity, the data in this model is only enough to cover the + parts that are most important for the actual process of managing drafts and + publishes. You will want to create your own models to represent the actual + content data that's associated with this PublishableEntityVersion, and + connect them using a OneToOneField with primary_key=True. The easiest way to + do this is to inherit from PublishableEntityVersionMixin. Be sure to treat + these versioned models in your app as immutable as well. + """ + + uuid = immutable_uuid_field() + entity = models.ForeignKey( + PublishableEntity, on_delete=models.CASCADE, related_name="versions" + ) + + # Most publishable things will have some sort of title, but blanks are + # allowed for those that don't require one. + title = case_insensitive_char_field(max_length=500, blank=True, default="") + + # The version_num starts at 1 and increments by 1 with each new version for + # a given PublishableEntity. Doing it this way makes it more convenient for + # users to refer to than a hash or UUID value. It also helps us catch race + # conditions on save, by setting a unique constraint on the entity and + # version_num. + version_num = models.PositiveIntegerField( + null=False, + validators=[MinValueValidator(1)], + ) -__all__ = [ - "PublishableEntityMixin", - "PublishableEntityVersionMixin", - "PublishableContentModelRegistry", -] + # All PublishableEntityVersions created as part of the same publish should + # have the exact same created datetime (not off by a handful of + # microseconds). + created = manual_date_time_field() + + # User who created the PublishableEntityVersion. This can be null if the + # user is later removed. Open edX in general doesn't let you remove users, + # but we should try to model it so that this is possible eventually. + created_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.SET_NULL, + null=True, + blank=True, + ) + + class Meta: + constraints = [ + # Prevent the situation where we have multiple + # PublishableEntityVersions claiming to be the same version_num for + # a given PublishableEntity. This can happen if there's a race + # condition between concurrent editors in different browsers, + # working on the same Publishable. With this constraint, one of + # those processes will raise an IntegrityError. + models.UniqueConstraint( + fields=[ + "entity", + "version_num", + ], + name="oel_pv_uniq_entity_version_num", + ) + ] + indexes = [ + # LearningPackage (reverse) Created Index: + # * Make it cheap to find the most recently created + # PublishableEntityVersions for a given LearningPackage. This + # represents the most recently saved work for a LearningPackage + # and would be the most likely areas to get worked on next. + models.Index( + fields=["entity", "-created"], + name="oel_pv_idx_entity_rcreated", + ), + # Title Index: + # * Search by title. + models.Index( + fields=[ + "title", + ], + name="oel_pv_idx_title", + ), + ] + + # These are for the Django Admin UI. + verbose_name = "Publishable Entity Version" + verbose_name_plural = "Publishable Entity Versions" class PublishableEntityMixin(models.Model): @@ -29,17 +267,12 @@ class PublishableEntityMixin(models.Model): the publishing app's api.register_content_models (see its docstring for details). """ - - class PublishableEntityMixinManager(models.Manager): - def get_queryset(self) -> QuerySet: - return super().get_queryset() \ - .select_related( - "publishable_entity", - "publishable_entity__published", - "publishable_entity__draft", - ) - - objects: models.Manager[PublishableEntityMixin] = PublishableEntityMixinManager() + # select these related entities by default for all queries + objects: ClassVar[WithRelationsManager[Self]] = WithRelationsManager( + "publishable_entity", + "publishable_entity__published", + "publishable_entity__draft", + ) publishable_entity = models.OneToOneField( PublishableEntity, on_delete=models.CASCADE, primary_key=True @@ -138,11 +371,24 @@ def __init__(self, content_obj): field_to_pev = self.content_version_model_cls._meta.get_field( "publishable_entity_version" ) - # Now that we know the field that leads to PublishableEntityVersion, # get the reverse related field name so that we can use that later. self.related_name = field_to_pev.related_query_name() + if field_to_pev.model != self.content_version_model_cls: + # In the case of multi-table inheritance and mixins, this can get tricky. + # Example: + # content_version_model_cls is UnitVersion, which is a subclass of ContainerVersion + # This versioning helper can be accessed via unit_version.versioning (should return UnitVersion) or + # via container_version.versioning (should return ContainerVersion) + intermediate_model = field_to_pev.model # example: ContainerVersion + # This is the field on the subclass (e.g. UnitVersion) that gets + # the intermediate (e.g. ContainerVersion). Example: "UnitVersion.container_version" (1:1 foreign key) + field_to_intermediate = self.content_version_model_cls._meta.get_ancestor_link(intermediate_model) + if field_to_intermediate: + # Example: self.related_name = "containerversion.unitversion" + self.related_name = self.related_name + "." + field_to_intermediate.related_query_name() + def _content_obj_version(self, pub_ent_version: PublishableEntityVersion | None): """ PublishableEntityVersion -> Content object version @@ -152,7 +398,10 @@ def _content_obj_version(self, pub_ent_version: PublishableEntityVersion | None) """ if pub_ent_version is None: return None - return getattr(pub_ent_version, self.related_name) + obj = pub_ent_version + for field_name in self.related_name.split("."): + obj = getattr(obj, field_name) + return obj @property def draft(self): @@ -295,17 +544,10 @@ class PublishableEntityVersionMixin(models.Model): details). """ - class PublishableEntityVersionMixinManager(models.Manager): - def get_queryset(self) -> QuerySet: - return ( - super() - .get_queryset() - .select_related( - "publishable_entity_version", - ) - ) - - objects: models.Manager[PublishableEntityVersionMixin] = PublishableEntityVersionMixinManager() + # select these related entities by default for all queries + objects: ClassVar[WithRelationsManager[Self]] = WithRelationsManager( + "publishable_entity_version", + ) publishable_entity_version = models.OneToOneField( PublishableEntityVersion, on_delete=models.CASCADE, primary_key=True diff --git a/openedx_learning/apps/authoring/units/__init__.py b/openedx_learning/apps/authoring/units/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/openedx_learning/apps/authoring/units/api.py b/openedx_learning/apps/authoring/units/api.py new file mode 100644 index 00000000..93058dd3 --- /dev/null +++ b/openedx_learning/apps/authoring/units/api.py @@ -0,0 +1,290 @@ +"""Units API. + +This module provides functions to manage units. +""" +from dataclasses import dataclass +from datetime import datetime + +from django.db.transaction import atomic + +from openedx_learning.apps.authoring.components.models import Component, ComponentVersion + +from ..publishing import api as publishing_api +from .models import Unit, UnitVersion + +# πŸ›‘ UNSTABLE: All APIs related to containers are unstable until we've figured +# out our approach to dynamic content (randomized, A/B tests, etc.) +__all__ = [ + "create_unit", + "create_unit_version", + "create_next_unit_version", + "create_unit_and_version", + "get_unit", + "get_unit_version", + "get_latest_unit_version", + "UnitListEntry", + "get_components_in_unit", + "get_components_in_unit", + "get_components_in_published_unit_as_of", +] + + +def create_unit( + learning_package_id: int, key: str, created: datetime, created_by: int | None +) -> Unit: + """ + [ πŸ›‘ UNSTABLE ] Create a new unit. + + Args: + learning_package_id: The learning package ID. + key: The key. + created: The creation date. + created_by: The user who created the unit. + """ + return publishing_api.create_container( + learning_package_id, + key, + created, + created_by, + container_cls=Unit, + ) + + +def create_unit_version( + unit: Unit, + version_num: int, + *, + title: str, + publishable_entities_pks: list[int], + entity_version_pks: list[int | None], + created: datetime, + created_by: int | None = None, +) -> UnitVersion: + """ + [ πŸ›‘ UNSTABLE ] Create a new unit version. + + This is a very low-level API, likely only needed for import/export. In + general, you will use `create_unit_and_version()` and + `create_next_unit_version()` instead. + + Args: + unit_pk: The unit ID. + version_num: The version number. + title: The title. + publishable_entities_pk: The publishable entities. + entity: The entity. + created: The creation date. + created_by: The user who created the unit. + """ + return publishing_api.create_container_version( + unit.pk, + version_num, + title=title, + publishable_entities_pks=publishable_entities_pks, + entity_version_pks=entity_version_pks, + created=created, + created_by=created_by, + container_version_cls=UnitVersion, + ) + + +def _pub_entities_for_components( + components: list[Component | ComponentVersion] | None, +) -> tuple[list[int], list[int | None]] | tuple[None, None]: + """ + Helper method: given a list of Component | ComponentVersion, return the + lists of publishable_entities_pks and entity_version_pks needed for the + base container APIs. + + ComponentVersion is passed when we want to pin a specific version, otherwise + Component is used for unpinned. + """ + if components is None: + # When these are None, that means don't change the entities in the list. + return None, None + for c in components: + if not isinstance(c, (Component, ComponentVersion)): + raise TypeError("Unit components must be either Component or ComponentVersion.") + publishable_entities_pks = [ + (c.publishable_entity_id if isinstance(c, Component) else c.component.publishable_entity_id) + for c in components + ] + entity_version_pks = [ + (cv.pk if isinstance(cv, ComponentVersion) else None) + for cv in components + ] + return publishable_entities_pks, entity_version_pks + + +def create_next_unit_version( + unit: Unit, + *, + title: str | None = None, + components: list[Component | ComponentVersion] | None = None, + created: datetime, + created_by: int | None = None, +) -> UnitVersion: + """ + [ πŸ›‘ UNSTABLE ] Create the next unit version. + + Args: + unit_pk: The unit ID. + title: The title. Leave as None to keep the current title. + components: The components, as a list of Components (unpinned) and/or ComponentVersions (pinned). Passing None + will leave the existing components unchanged. + created: The creation date. + created_by: The user who created the unit. + """ + publishable_entities_pks, entity_version_pks = _pub_entities_for_components(components) + unit_version = publishing_api.create_next_container_version( + unit.pk, + title=title, + publishable_entities_pks=publishable_entities_pks, + entity_version_pks=entity_version_pks, + created=created, + created_by=created_by, + container_version_cls=UnitVersion, + ) + return unit_version + + +def create_unit_and_version( + learning_package_id: int, + key: str, + *, + title: str, + components: list[Component | ComponentVersion] | None = None, + created: datetime, + created_by: int | None = None, +) -> tuple[Unit, UnitVersion]: + """ + [ πŸ›‘ UNSTABLE ] Create a new unit and its version. + + Args: + learning_package_id: The learning package ID. + key: The key. + created: The creation date. + created_by: The user who created the unit. + """ + publishable_entities_pks, entity_version_pks = _pub_entities_for_components(components) + with atomic(): + unit = create_unit(learning_package_id, key, created, created_by) + unit_version = create_unit_version( + unit, + 1, + title=title, + publishable_entities_pks=publishable_entities_pks or [], + entity_version_pks=entity_version_pks or [], + created=created, + created_by=created_by, + ) + return unit, unit_version + + +def get_unit(unit_pk: int) -> Unit: + """ + [ πŸ›‘ UNSTABLE ] Get a unit. + + Args: + unit_pk: The unit ID. + """ + return Unit.objects.get(pk=unit_pk) + + +def get_unit_version(unit_version_pk: int) -> UnitVersion: + """ + [ πŸ›‘ UNSTABLE ] Get a unit version. + + Args: + unit_version_pk: The unit version ID. + """ + return UnitVersion.objects.get(pk=unit_version_pk) + + +def get_latest_unit_version(unit_pk: int) -> UnitVersion: + """ + [ πŸ›‘ UNSTABLE ] Get the latest unit version. + + Args: + unit_pk: The unit ID. + """ + return Unit.objects.get(pk=unit_pk).versioning.latest + + +@dataclass(frozen=True) +class UnitListEntry: + """ + [ πŸ›‘ UNSTABLE ] + Data about a single entity in a container, e.g. a component in a unit. + """ + component_version: ComponentVersion + pinned: bool = False + + @property + def component(self): + return self.component_version.component + + +def get_components_in_unit( + unit: Unit, + *, + published: bool, +) -> list[UnitListEntry]: + """ + [ πŸ›‘ UNSTABLE ] + Get the list of entities and their versions in the draft or published + version of the given Unit. + + Args: + unit: The Unit, e.g. returned by `get_unit()` + published: `True` if we want the published version of the unit, or + `False` for the draft version. + """ + assert isinstance(unit, Unit) + components = [] + for entry in publishing_api.get_entities_in_container(unit, published=published): + # Convert from generic PublishableEntityVersion to ComponentVersion: + component_version = entry.entity_version.componentversion + assert isinstance(component_version, ComponentVersion) + components.append(UnitListEntry(component_version=component_version, pinned=entry.pinned)) + return components + + +def get_components_in_published_unit_as_of( + unit: Unit, + publish_log_id: int, +) -> list[UnitListEntry] | None: + """ + [ πŸ›‘ UNSTABLE ] + Get the list of entities and their versions in the published version of the + given container as of the given PublishLog version (which is essentially a + version for the entire learning package). + + TODO: This API should be updated to also return the UnitVersion so we can + see the unit title and any other metadata from that point in time. + TODO: accept a publish log UUID, not just int ID? + TODO: move the implementation to be a generic 'containers' implementation + that this units function merely wraps. + TODO: optimize, perhaps by having the publishlog store a record of all + ancestors of every modified PublishableEntity in the publish. + """ + assert isinstance(unit, Unit) + unit_pub_entity_version = publishing_api.get_published_version_as_of(unit.publishable_entity_id, publish_log_id) + if unit_pub_entity_version is None: + return None # This unit was not published as of the given PublishLog ID. + container_version = unit_pub_entity_version.containerversion + + entity_list = [] + rows = container_version.entity_list.entitylistrow_set.order_by("order_num") + for row in rows: + if row.entity_version is not None: + component_version = row.entity_version.componentversion + assert isinstance(component_version, ComponentVersion) + entity_list.append(UnitListEntry(component_version=component_version, pinned=True)) + else: + # Unpinned component - figure out what its latest published version was. + # This is not optimized. It could be done in one query per unit rather than one query per component. + pub_entity_version = publishing_api.get_published_version_as_of(row.entity_id, publish_log_id) + if pub_entity_version: + entity_list.append(UnitListEntry(component_version=pub_entity_version.componentversion, pinned=False)) + return entity_list diff --git a/openedx_learning/apps/authoring/units/apps.py b/openedx_learning/apps/authoring/units/apps.py new file mode 100644 index 00000000..f0beecf3 --- /dev/null +++ b/openedx_learning/apps/authoring/units/apps.py @@ -0,0 +1,25 @@ +""" +Unit Django application initialization. +""" + +from django.apps import AppConfig + + +class UnitsConfig(AppConfig): + """ + Configuration for the units Django application. + """ + + name = "openedx_learning.apps.authoring.units" + verbose_name = "Learning Core > Authoring > Units" + default_auto_field = "django.db.models.BigAutoField" + label = "oel_units" + + def ready(self): + """ + Register Unit and UnitVersion. + """ + from ..publishing.api import register_content_models # pylint: disable=import-outside-toplevel + from .models import Unit, UnitVersion # pylint: disable=import-outside-toplevel + + register_content_models(Unit, UnitVersion) diff --git a/openedx_learning/apps/authoring/units/migrations/0001_initial.py b/openedx_learning/apps/authoring/units/migrations/0001_initial.py new file mode 100644 index 00000000..52a5b4fb --- /dev/null +++ b/openedx_learning/apps/authoring/units/migrations/0001_initial.py @@ -0,0 +1,36 @@ +# Generated by Django 4.2.19 on 2025-03-11 04:31 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('oel_publishing', '0003_containers'), + ] + + operations = [ + migrations.CreateModel( + name='Unit', + fields=[ + ('container', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='oel_publishing.container')), + ], + options={ + 'abstract': False, + }, + bases=('oel_publishing.container',), + ), + migrations.CreateModel( + name='UnitVersion', + fields=[ + ('container_version', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='oel_publishing.containerversion')), + ], + options={ + 'abstract': False, + }, + bases=('oel_publishing.containerversion',), + ), + ] diff --git a/openedx_learning/apps/authoring/units/migrations/__init__.py b/openedx_learning/apps/authoring/units/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/openedx_learning/apps/authoring/units/models.py b/openedx_learning/apps/authoring/units/models.py new file mode 100644 index 00000000..0c525584 --- /dev/null +++ b/openedx_learning/apps/authoring/units/models.py @@ -0,0 +1,50 @@ +""" +Models that implement units +""" +from django.db import models + +from ..publishing.models import Container, ContainerVersion + +__all__ = [ + "Unit", + "UnitVersion", +] + + +class Unit(Container): + """ + A Unit is type of Container that holds Components. + + Via Container and its PublishableEntityMixin, Units are also publishable + entities and can be added to other containers. + """ + container = models.OneToOneField( + Container, + on_delete=models.CASCADE, + parent_link=True, + primary_key=True, + ) + + +class UnitVersion(ContainerVersion): + """ + A UnitVersion is a specific version of a Unit. + + Via ContainerVersion and its EntityList, it defines the list of Components + in this version of the Unit. + """ + container_version = models.OneToOneField( + ContainerVersion, + on_delete=models.CASCADE, + parent_link=True, + primary_key=True, + ) + + @property + def unit(self): + """ Convenience accessor to the Unit this version is associated with """ + return self.container_version.container.unit # pylint: disable=no-member + + # Note: the 'publishable_entity_version' field is inherited and will appear on this model, but does not exist + # in the underlying database table. It only exists in the ContainerVersion table. + # You can verify this by running 'python manage.py sqlmigrate oel_units 0001_initial' diff --git a/openedx_learning/lib/managers.py b/openedx_learning/lib/managers.py index b21f0a6b..0298c87b 100644 --- a/openedx_learning/lib/managers.py +++ b/openedx_learning/lib/managers.py @@ -1,11 +1,17 @@ """ Custom Django ORM Managers. """ +from __future__ import annotations + +from typing import TypeVar + from django.db import models from django.db.models.query import QuerySet +M = TypeVar('M', bound=models.Model) + -class WithRelationsManager(models.Manager): +class WithRelationsManager(models.Manager[M]): """ Custom Manager that adds select_related to the default queryset. diff --git a/openedx_tagging/core/tagging/api.py b/openedx_tagging/core/tagging/api.py index a3e1af69..767915e7 100644 --- a/openedx_tagging/core/tagging/api.py +++ b/openedx_tagging/core/tagging/api.py @@ -28,7 +28,7 @@ TagDoesNotExist = Tag.DoesNotExist -def create_taxonomy( +def create_taxonomy( # pylint: disable=too-many-positional-arguments name: str, description: str | None = None, enabled=True, @@ -321,7 +321,7 @@ def _get_current_tags( return current_tags -def tag_object( +def tag_object( # pylint: disable=too-many-positional-arguments object_id: str, taxonomy: Taxonomy | None, tags: list[str], diff --git a/openedx_tagging/core/tagging/models/base.py b/openedx_tagging/core/tagging/models/base.py index 6b2cccf9..aa2ef407 100644 --- a/openedx_tagging/core/tagging/models/base.py +++ b/openedx_tagging/core/tagging/models/base.py @@ -382,7 +382,7 @@ def copy(self, taxonomy: Taxonomy) -> Taxonomy: return self - def get_filtered_tags( + def get_filtered_tags( # pylint: disable=too-many-positional-arguments self, depth: int | None = TAXONOMY_MAX_DEPTH, parent_tag_value: str | None = None, diff --git a/projects/dev.py b/projects/dev.py index ada76e5e..85d01f4a 100644 --- a/projects/dev.py +++ b/projects/dev.py @@ -35,6 +35,7 @@ "openedx_learning.apps.authoring.components.apps.ComponentsConfig", "openedx_learning.apps.authoring.contents.apps.ContentsConfig", "openedx_learning.apps.authoring.publishing.apps.PublishingConfig", + "openedx_learning.apps.authoring.units.apps.UnitsConfig", # Learning Contrib Apps "openedx_learning.contrib.media_server.apps.MediaServerConfig", # Apps that don't belong in this repo in the long term, but are here to make diff --git a/pylintrc b/pylintrc index b56bdeec..42cd7f9d 100644 --- a/pylintrc +++ b/pylintrc @@ -290,7 +290,6 @@ disable = invalid-name, django-not-configured, consider-using-with, - too-many-positional-arguments, [REPORTS] output-format = text diff --git a/pylintrc_tweaks b/pylintrc_tweaks index 8eb879bd..f999b169 100644 --- a/pylintrc_tweaks +++ b/pylintrc_tweaks @@ -8,4 +8,3 @@ disable+= invalid-name, django-not-configured, consider-using-with, - too-many-positional-arguments, diff --git a/test_settings.py b/test_settings.py index f5b154f5..71a71c33 100644 --- a/test_settings.py +++ b/test_settings.py @@ -45,6 +45,7 @@ def root(*args): "openedx_learning.apps.authoring.contents.apps.ContentsConfig", "openedx_learning.apps.authoring.publishing.apps.PublishingConfig", "openedx_tagging.core.tagging.apps.TaggingConfig", + "openedx_learning.apps.authoring.units.apps.UnitsConfig", ] AUTHENTICATION_BACKENDS = [ diff --git a/tests/openedx_learning/apps/authoring/components/test_api.py b/tests/openedx_learning/apps/authoring/components/test_api.py index 46ad540c..aba186c3 100644 --- a/tests/openedx_learning/apps/authoring/components/test_api.py +++ b/tests/openedx_learning/apps/authoring/components/test_api.py @@ -44,6 +44,17 @@ def setUpTestData(cls) -> None: cls.problem_type = components_api.get_or_create_component_type("xblock.v1", "problem") cls.video_type = components_api.get_or_create_component_type("xblock.v1", "video") + def publish_component(self, component: Component): + """ + Helper method to publish a single component. + """ + publishing_api.publish_from_drafts( + self.learning_package.pk, + draft_qset=publishing_api.get_all_drafts(self.learning_package.pk).filter( + entity=component.publishable_entity, + ), + ) + class PerformanceTestCase(ComponentTestCase): """ diff --git a/tests/openedx_learning/apps/authoring/units/__init__.py b/tests/openedx_learning/apps/authoring/units/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/openedx_learning/apps/authoring/units/test_api.py b/tests/openedx_learning/apps/authoring/units/test_api.py new file mode 100644 index 00000000..1556a34b --- /dev/null +++ b/tests/openedx_learning/apps/authoring/units/test_api.py @@ -0,0 +1,923 @@ +""" +Basic tests for the units API. +""" +import ddt # type: ignore[import] +import pytest +from django.core.exceptions import ValidationError +from django.db import IntegrityError + +from openedx_learning.api import authoring as authoring_api +from openedx_learning.api import authoring_models + +from ..components.test_api import ComponentTestCase + +Entry = authoring_api.UnitListEntry + + +@ddt.ddt +class UnitTestCase(ComponentTestCase): + """ Test cases for Units (containers of components) """ + + def setUp(self) -> None: + super().setUp() + self.component_1, self.component_1_v1 = self.create_component( + key="Query Counting", + title="Querying Counting Problem", + ) + self.component_2, self.component_2_v1 = self.create_component( + key="Query Counting (2)", + title="Querying Counting Problem (2)", + ) + + def create_component(self, *, title: str = "Test Component", key: str = "component:1") -> tuple[ + authoring_models.Component, authoring_models.ComponentVersion + ]: + """ Helper method to quickly create a component """ + return authoring_api.create_component_and_version( + self.learning_package.id, + component_type=self.problem_type, + local_key=key, + title=title, + created=self.now, + created_by=None, + ) + + def create_unit_with_components( + self, + components: list[authoring_models.Component | authoring_models.ComponentVersion], + *, + title="Unit", + key="unit:key", + ) -> authoring_models.Unit: + """ Helper method to quickly create a unit with some components """ + unit, _unit_v1 = authoring_api.create_unit_and_version( + learning_package_id=self.learning_package.id, + key=key, + title=title, + components=components, + created=self.now, + created_by=None, + ) + return unit + + def modify_component( + self, + component: authoring_models.Component, + *, + title="Modified Component", + timestamp=None, + ) -> authoring_models.ComponentVersion: + """ + Helper method to modify a component for the purposes of testing units/drafts/pinning/publishing/etc. + """ + return authoring_api.create_next_component_version( + component.pk, + content_to_replace={}, + title=title, + created=timestamp or self.now, + created_by=None, + ) + + def test_get_unit(self): + """ + Test get_unit() + """ + unit = self.create_unit_with_components([self.component_1, self.component_2]) + with self.assertNumQueries(1): + result = authoring_api.get_unit(unit.pk) + assert result == unit + # Versioning data should be pre-loaded via select_related() + with self.assertNumQueries(0): + assert result.versioning.has_unpublished_changes + + def test_get_container(self): + """ + Test get_container() + """ + unit = self.create_unit_with_components([self.component_1, self.component_2]) + with self.assertNumQueries(1): + result = authoring_api.get_container(unit.pk) + assert result == unit.container + # Versioning data should be pre-loaded via select_related() + with self.assertNumQueries(0): + assert result.versioning.has_unpublished_changes + + def test_unit_container_versioning(self): + """ + Test that the .versioning helper of a Unit returns a UnitVersion, and + same for the generic Container equivalent. + """ + unit = self.create_unit_with_components([self.component_1, self.component_2]) + container = unit.container + container_version = container.versioning.draft + assert isinstance(container_version, authoring_models.ContainerVersion) + unit_version = unit.versioning.draft + assert isinstance(unit_version, authoring_models.UnitVersion) + assert unit_version.container_version == container_version + assert unit_version.container_version.container == container + assert unit_version.unit == unit + + def test_create_unit_queries(self): + """ + Test how many database queries are required to create a unit + """ + # The exact numbers here aren't too important - this is just to alert us if anything significant changes. + with self.assertNumQueries(22): + _empty_unit = self.create_unit_with_components([]) + with self.assertNumQueries(25): + # And try with a non-empty unit: + self.create_unit_with_components([self.component_1, self.component_2_v1], key="u2") + + def test_create_unit_with_invalid_children(self): + """ + Verify that only components can be added to units, and a specific + exception is raised. + """ + # Create two units: + unit, unit_version = authoring_api.create_unit_and_version( + learning_package_id=self.learning_package.id, + key="unit:key", + title="Unit", + created=self.now, + created_by=None, + ) + assert unit.versioning.draft == unit_version + unit2, _u2v1 = authoring_api.create_unit_and_version( + learning_package_id=self.learning_package.id, + key="unit:key2", + title="Unit 2", + created=self.now, + created_by=None, + ) + # Try adding a Unit to a Unit + with pytest.raises(TypeError, match="Unit components must be either Component or ComponentVersion."): + authoring_api.create_next_unit_version( + unit=unit, + title="Unit Containing a Unit", + components=[unit2], + created=self.now, + created_by=None, + ) + # Check that a new version was not created: + unit.refresh_from_db() + assert authoring_api.get_unit(unit.pk).versioning.draft == unit_version + assert unit.versioning.draft == unit_version + + def test_adding_external_components(self): + """ + Test that components from another learning package cannot be added to a + unit. + """ + learning_package2 = authoring_api.create_learning_package(key="other-package", title="Other Package") + unit, _unit_version = authoring_api.create_unit_and_version( + learning_package_id=learning_package2.pk, + key="unit:key", + title="Unit", + created=self.now, + created_by=None, + ) + assert self.component_1.learning_package != learning_package2 + # Try adding a a component from LP 1 (self.learning_package) to a unit from LP 2 + with pytest.raises(ValidationError, match="Container entities must be from the same learning package."): + authoring_api.create_next_unit_version( + unit=unit, + title="Unit Containing an External Component", + components=[self.component_1], + created=self.now, + created_by=None, + ) + + @ddt.data(True, False) + def test_cannot_add_invalid_ids(self, pin_version): + """ + Test that non-existent components cannot be added to units + """ + self.component_1.delete() + if pin_version: + components = [self.component_1_v1] + else: + components = [self.component_1] + with pytest.raises((IntegrityError, authoring_models.Component.DoesNotExist)): + self.create_unit_with_components(components) + + def test_create_empty_unit_and_version(self): + """Test creating a unit with no components. + + Expected results: + 1. A unit and unit version are created. + 2. The unit version number is 1. + 3. The unit is a draft with unpublished changes. + 4. There is no published version of the unit. + """ + unit, unit_version = authoring_api.create_unit_and_version( + learning_package_id=self.learning_package.id, + key="unit:key", + title="Unit", + created=self.now, + created_by=None, + ) + assert unit, unit_version + assert unit_version.version_num == 1 + assert unit_version in unit.versioning.versions.all() + assert unit.versioning.has_unpublished_changes + assert unit.versioning.draft == unit_version + assert unit.versioning.published is None + + def test_create_next_unit_version_with_two_unpinned_components(self): + """Test creating a unit version with two unpinned components. + + Expected results: + 1. A new unit version is created. + 2. The unit version number is 2. + 3. The unit version is in the unit's versions. + 4. The components are in the draft unit version's component list and are unpinned. + """ + unit, _unit_version = authoring_api.create_unit_and_version( + learning_package_id=self.learning_package.id, + key="unit:key", + title="Unit", + created=self.now, + created_by=None, + ) + unit_version_v2 = authoring_api.create_next_unit_version( + unit=unit, + title="Unit", + components=[self.component_1, self.component_2], + created=self.now, + created_by=None, + ) + assert unit_version_v2.version_num == 2 + assert unit_version_v2 in unit.versioning.versions.all() + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(self.component_1.versioning.draft), + Entry(self.component_2.versioning.draft), + ] + with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): + # There is no published version of the unit: + authoring_api.get_components_in_unit(unit, published=True) + + def test_create_next_unit_version_with_unpinned_and_pinned_components(self): + """ + Test creating a unit version with one unpinned and one pinned πŸ“Œ component. + """ + unit, _unit_version = authoring_api.create_unit_and_version( + learning_package_id=self.learning_package.id, + key="unit:key", + title="Unit", + created=self.now, + created_by=None, + ) + unit_version_v2 = authoring_api.create_next_unit_version( + unit=unit, + title="Unit", + components=[self.component_1, self.component_2_v1], # Note the "v1" pinning πŸ“Œ the second one to version 1 + created=self.now, + created_by=None, + ) + assert unit_version_v2.version_num == 2 + assert unit_version_v2 in unit.versioning.versions.all() + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(self.component_1_v1), + Entry(self.component_2_v1, pinned=True), # Pinned πŸ“Œ to v1 + ] + with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): + # There is no published version of the unit: + authoring_api.get_components_in_unit(unit, published=True) + + def test_auto_publish_children(self): + """ + Test that publishing a unit publishes its child components automatically. + """ + # Create a draft unit with two draft components + unit = self.create_unit_with_components([self.component_1, self.component_2]) + # Also create another component that's not in the unit at all: + other_component, _oc_v1 = self.create_component(title="A draft component not in the unit", key="component:3") + + assert authoring_api.contains_unpublished_changes(unit.pk) + assert self.component_1.versioning.published is None + assert self.component_2.versioning.published is None + + # Publish ONLY the unit. This should however also auto-publish components 1 & 2 since they're children + authoring_api.publish_from_drafts( + self.learning_package.pk, + draft_qset=authoring_api.get_all_drafts(self.learning_package.pk).filter(entity=unit.publishable_entity), + ) + # Now all changes to the unit and to component 1 are published: + unit.refresh_from_db() + self.component_1.refresh_from_db() + assert unit.versioning.has_unpublished_changes is False # Shallow check + assert self.component_1.versioning.has_unpublished_changes is False + assert authoring_api.contains_unpublished_changes(unit.pk) is False # Deep check + assert self.component_1.versioning.published == self.component_1_v1 # v1 is now the published version. + + # But our other component that's outside the unit is not affected: + other_component.refresh_from_db() + assert other_component.versioning.has_unpublished_changes + assert other_component.versioning.published is None + + def test_no_publish_parent(self): + """ + Test that publishing a component does NOT publish changes to its parent unit + """ + # Create a draft unit with two draft components + unit = self.create_unit_with_components([self.component_1, self.component_2]) + assert unit.versioning.has_unpublished_changes + # Publish ONLY one of its child components + self.publish_component(self.component_1) + self.component_1.refresh_from_db() # Clear cache on '.versioning' + assert self.component_1.versioning.has_unpublished_changes is False + + # The unit that contains that component should still be unpublished: + unit.refresh_from_db() # Clear cache on '.versioning' + assert unit.versioning.has_unpublished_changes + assert unit.versioning.published is None + with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): + # There is no published version of the unit: + authoring_api.get_components_in_unit(unit, published=True) + + def test_add_component_after_publish(self): + """ + Adding a component to a published unit will create a new version and + show that the unit has unpublished changes. + """ + unit, unit_version = authoring_api.create_unit_and_version( + learning_package_id=self.learning_package.id, + key="unit:key", + title="Unit", + created=self.now, + created_by=None, + ) + assert unit.versioning.draft == unit_version + assert unit.versioning.published is None + assert unit.versioning.has_unpublished_changes + # Publish the empty unit: + authoring_api.publish_all_drafts(self.learning_package.id) + unit.refresh_from_db() # Reloading the unit is necessary + assert unit.versioning.has_unpublished_changes is False # Shallow check for just the unit itself, not children + assert authoring_api.contains_unpublished_changes(unit.pk) is False # Deeper check + + # Add a published component (unpinned): + assert self.component_1.versioning.has_unpublished_changes is False + unit_version_v2 = authoring_api.create_next_unit_version( + unit=unit, + title=unit_version.title, + components=[self.component_1], + created=self.now, + created_by=None, + ) + # Now the unit should have unpublished changes: + unit.refresh_from_db() # Reloading the unit is necessary + assert unit.versioning.has_unpublished_changes # Shallow check - adding a child is a change to the unit + assert authoring_api.contains_unpublished_changes(unit.pk) # Deeper check + assert unit.versioning.draft == unit_version_v2 + assert unit.versioning.published == unit_version + + def test_modify_unpinned_component_after_publish(self): + """ + Modifying an unpinned component in a published unit will NOT create a + new version nor show that the unit has unpublished changes (but it will + "contain" unpublished changes). The modifications will appear in the + published version of the unit only after the component is published. + """ + # Create a unit with one unpinned draft component: + assert self.component_1.versioning.has_unpublished_changes + unit = self.create_unit_with_components([self.component_1]) + assert unit.versioning.has_unpublished_changes + + # Publish the unit and the component: + authoring_api.publish_all_drafts(self.learning_package.id) + unit.refresh_from_db() # Reloading the unit is necessary if we accessed 'versioning' before publish + self.component_1.refresh_from_db() + assert unit.versioning.has_unpublished_changes is False # Shallow check + assert authoring_api.contains_unpublished_changes(unit.pk) is False # Deeper check + assert self.component_1.versioning.has_unpublished_changes is False + + # Now modify the component by changing its title (it remains a draft): + component_1_v2 = self.modify_component(self.component_1, title="Modified Counting Problem with new title") + + # The component now has unpublished changes; the unit doesn't directly but does contain + unit.refresh_from_db() # Reloading the unit is necessary, or 'unit.versioning' will be outdated + self.component_1.refresh_from_db() + assert unit.versioning.has_unpublished_changes is False # Shallow check should be false - unit is unchanged + assert authoring_api.contains_unpublished_changes(unit.pk) # But unit DOES contain changes + assert self.component_1.versioning.has_unpublished_changes + + # Since the component changes haven't been published, they should only appear in the draft unit + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(component_1_v2), # new version + ] + assert authoring_api.get_components_in_unit(unit, published=True) == [ + Entry(self.component_1_v1), # old version + ] + + # But if we publish the component, the changes will appear in the published version of the unit. + self.publish_component(self.component_1) + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(component_1_v2), # new version + ] + assert authoring_api.get_components_in_unit(unit, published=True) == [ + Entry(component_1_v2), # new version + ] + assert authoring_api.contains_unpublished_changes(unit.pk) is False # No longer contains unpublished changes + + def test_modify_pinned_component(self): + """ + When a pinned πŸ“Œ component in unit is modified and/or published, it will + have no effect on either the draft nor published version of the unit, + which will continue to use the pinned version. + """ + # Create a unit with one component (pinned πŸ“Œ to v1): + unit = self.create_unit_with_components([self.component_1_v1]) + + # Publish the unit and the component: + authoring_api.publish_all_drafts(self.learning_package.id) + expected_unit_contents = [ + Entry(self.component_1_v1, pinned=True), # pinned πŸ“Œ to v1 + ] + assert authoring_api.get_components_in_unit(unit, published=True) == expected_unit_contents + + # Now modify the component by changing its title (it remains a draft): + self.modify_component(self.component_1, title="Modified Counting Problem with new title") + + # The component now has unpublished changes; the unit is entirely unaffected + unit.refresh_from_db() # Reloading the unit is necessary, or 'unit.versioning' will be outdated + self.component_1.refresh_from_db() + assert unit.versioning.has_unpublished_changes is False # Shallow check + assert authoring_api.contains_unpublished_changes(unit.pk) is False # Deep check + assert self.component_1.versioning.has_unpublished_changes is True + + # Neither the draft nor the published version of the unit is affected + assert authoring_api.get_components_in_unit(unit, published=False) == expected_unit_contents + assert authoring_api.get_components_in_unit(unit, published=True) == expected_unit_contents + # Even if we publish the component, the unit stays pinned to the specified version: + self.publish_component(self.component_1) + assert authoring_api.get_components_in_unit(unit, published=False) == expected_unit_contents + assert authoring_api.get_components_in_unit(unit, published=True) == expected_unit_contents + + def test_create_two_units_with_same_components(self): + """ + Test creating two units with different combinations of the same two + components in each unit. + """ + # Create a unit with component 2 unpinned, component 2 pinned πŸ“Œ, and component 1: + unit1 = self.create_unit_with_components([self.component_2, self.component_2_v1, self.component_1], key="u1") + # Create a second unit with component 1 pinned πŸ“Œ, component 2, and component 1 unpinned: + unit2 = self.create_unit_with_components([self.component_1_v1, self.component_2, self.component_1], key="u2") + + # Check that the contents are as expected: + assert [row.component_version for row in authoring_api.get_components_in_unit(unit1, published=False)] == [ + self.component_2_v1, self.component_2_v1, self.component_1_v1, + ] + assert [row.component_version for row in authoring_api.get_components_in_unit(unit2, published=False)] == [ + self.component_1_v1, self.component_2_v1, self.component_1_v1, + ] + + # Modify component 1 + component_1_v2 = self.modify_component(self.component_1, title="component 1 v2") + # Publish changes + authoring_api.publish_all_drafts(self.learning_package.id) + # Modify component 2 - only in the draft + component_2_v2 = self.modify_component(self.component_2, title="component 2 DRAFT") + + # Check that the draft contents are as expected: + assert authoring_api.get_components_in_unit(unit1, published=False) == [ + Entry(component_2_v2), # v2 in the draft version + Entry(self.component_2_v1, pinned=True), # pinned πŸ“Œ to v1 + Entry(component_1_v2), # v2 + ] + assert authoring_api.get_components_in_unit(unit2, published=False) == [ + Entry(self.component_1_v1, pinned=True), # pinned πŸ“Œ to v1 + Entry(component_2_v2), # v2 in the draft version + Entry(component_1_v2), # v2 + ] + + # Check that the published contents are as expected: + assert authoring_api.get_components_in_unit(unit1, published=True) == [ + Entry(self.component_2_v1), # v1 in the published version + Entry(self.component_2_v1, pinned=True), # pinned πŸ“Œ to v1 + Entry(component_1_v2), # v2 + ] + assert authoring_api.get_components_in_unit(unit2, published=True) == [ + Entry(self.component_1_v1, pinned=True), # pinned πŸ“Œ to v1 + Entry(self.component_2_v1), # v1 in the published version + Entry(component_1_v2), # v2 + ] + + def test_publishing_shared_component(self): + """ + A complex test case involving two units with a shared component and + other non-shared components. + + Unit 1: components C1, C2, C3 + Unit 2: components C2, C4, C5 + Everything is "unpinned". + """ + # 1️⃣ Create the units and publish them: + (c1, c1_v1), (c2, _c2_v1), (c3, c3_v1), (c4, c4_v1), (c5, c5_v1) = [ + self.create_component(key=f"C{i}", title=f"Component {i}") for i in range(1, 6) + ] + unit1 = self.create_unit_with_components([c1, c2, c3], title="Unit 1", key="unit:1") + unit2 = self.create_unit_with_components([c2, c4, c5], title="Unit 2", key="unit:2") + authoring_api.publish_all_drafts(self.learning_package.id) + assert authoring_api.contains_unpublished_changes(unit1.pk) is False + assert authoring_api.contains_unpublished_changes(unit2.pk) is False + + # 2️⃣ Then the author edits C2 inside of Unit 1 making C2v2. + c2_v2 = self.modify_component(c2, title="C2 version 2") + # This makes U1 and U2 both show up as Units that CONTAIN unpublished changes, because they share the component. + assert authoring_api.contains_unpublished_changes(unit1.pk) + assert authoring_api.contains_unpublished_changes(unit2.pk) + # (But the units themselves are unchanged:) + unit1.refresh_from_db() + unit2.refresh_from_db() + assert unit1.versioning.has_unpublished_changes is False + assert unit2.versioning.has_unpublished_changes is False + + # 3️⃣ In addition to this, the author also modifies another component in Unit 2 (C5) + c5_v2 = self.modify_component(c5, title="C5 version 2") + + # 4️⃣ The author then publishes Unit 1, and therefore everything in it. + authoring_api.publish_from_drafts( + self.learning_package.pk, + draft_qset=authoring_api.get_all_drafts(self.learning_package.pk).filter( + # Note: we only publish the unit; the publishing API should auto-publish its components too. + entity_id=unit1.publishable_entity.id, + ), + ) + + # Result: Unit 1 will show the newly published version of C2: + assert authoring_api.get_components_in_unit(unit1, published=True) == [ + Entry(c1_v1), + Entry(c2_v2), # new published version of C2 + Entry(c3_v1), + ] + + # Result: someone looking at Unit 2 should see the newly published component 2, because publishing it anywhere + # publishes it everywhere. But publishing C2 and Unit 1 does not affect the other components in Unit 2. + # (Publish propagates downward, not upward) + assert authoring_api.get_components_in_unit(unit2, published=True) == [ + Entry(c2_v2), # new published version of C2 + Entry(c4_v1), # still original version of C4 (it was never modified) + Entry(c5_v1), # still original version of C5 (it hasn't been published) + ] + + # Result: Unit 2 CONTAINS unpublished changes because of the modified C5. Unit 1 doesn't contain unpub changes. + assert authoring_api.contains_unpublished_changes(unit1.pk) is False + assert authoring_api.contains_unpublished_changes(unit2.pk) + + # 5️⃣ Publish component C5, which should be the only thing unpublished in the learning package + self.publish_component(c5) + # Result: Unit 2 shows the new version of C5 and no longer contains unpublished changes: + assert authoring_api.get_components_in_unit(unit2, published=True) == [ + Entry(c2_v2), # new published version of C2 + Entry(c4_v1), # still original version of C4 (it was never modified) + Entry(c5_v2), # new published version of C5 + ] + assert authoring_api.contains_unpublished_changes(unit2.pk) is False + + def test_query_count_of_contains_unpublished_changes(self): + """ + Checking for unpublished changes in a unit should require a fixed number + of queries, not get more expensive as the unit gets larger. + """ + # Add 100 components (unpinned) + component_count = 100 + components = [] + for i in range(component_count): + component, _version = self.create_component( + key=f"Query Counting {i}", + title=f"Querying Counting Problem {i}", + ) + components.append(component) + unit = self.create_unit_with_components(components) + authoring_api.publish_all_drafts(self.learning_package.id) + unit.refresh_from_db() + with self.assertNumQueries(2): + assert authoring_api.contains_unpublished_changes(unit.pk) is False + + # Modify the most recently created component: + self.modify_component(component, title="Modified Component") + with self.assertNumQueries(2): + assert authoring_api.contains_unpublished_changes(unit.pk) is True + + def test_metadata_change_doesnt_create_entity_list(self): + """ + Test that changing a container's metadata like title will create a new + version, but can re-use the same EntityList. API consumers generally + shouldn't depend on this behavior; it's an optimization. + """ + unit = self.create_unit_with_components([self.component_1, self.component_2_v1]) + + orig_version_num = unit.versioning.draft.version_num + orig_entity_list_id = unit.versioning.draft.entity_list.pk + + authoring_api.create_next_unit_version(unit, title="New Title", created=self.now) + + unit.refresh_from_db() + new_version_num = unit.versioning.draft.version_num + new_entity_list_id = unit.versioning.draft.entity_list.pk + + assert new_version_num > orig_version_num + assert new_entity_list_id == orig_entity_list_id + + @ddt.data(True, False) + @pytest.mark.skip(reason="FIXME: we don't yet prevent adding soft-deleted components to units") + def test_cannot_add_soft_deleted_component(self, publish_first): + """ + Test that a soft-deleted component cannot be added to a unit. + + Although it's valid for units to contain soft-deleted components (by + deleting the component after adding it), it is likely a mistake if + you're trying to add one to the unit. + """ + component, _cv = self.create_component(title="Deleted component") + if publish_first: + # Publish the component: + authoring_api.publish_all_drafts(self.learning_package.id) + # Now delete it. The draft version is now deleted: + authoring_api.soft_delete_draft(component.pk) + # Now try adding that component to a unit: + with pytest.raises(ValidationError, match="component is deleted"): + self.create_unit_with_components([component]) + + def test_removing_component(self): + """ Test removing a component from a unit (but not deleting it) """ + unit = self.create_unit_with_components([self.component_1, self.component_2]) + authoring_api.publish_all_drafts(self.learning_package.id) + + # Now remove component 2 + authoring_api.create_next_unit_version( + unit=unit, + title="Revised with component 2 deleted", + components=[self.component_1], # component 2 is gone + created=self.now, + ) + + # Now it should not be listed in the unit: + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(self.component_1_v1), + ] + unit.refresh_from_db() + assert unit.versioning.has_unpublished_changes # The unit itself and its component list have change + assert authoring_api.contains_unpublished_changes(unit.pk) + # The published version of the unit is not yet affected: + assert authoring_api.get_components_in_unit(unit, published=True) == [ + Entry(self.component_1_v1), + Entry(self.component_2_v1), + ] + + # But when we publish the new unit version with the removal, the published version is affected: + authoring_api.publish_all_drafts(self.learning_package.id) + # FIXME: Refreshing the unit is necessary here because get_entities_in_published_container() accesses + # container.versioning.published, and .versioning is cached with the old version. But this seems like + # a footgun? We could avoid this if get_entities_in_published_container() took only an ID instead of an object, + # but that would involve additional database lookup(s). + unit.refresh_from_db() + assert authoring_api.contains_unpublished_changes(unit.pk) is False + assert authoring_api.get_components_in_unit(unit, published=True) == [ + Entry(self.component_1_v1), + ] + + def test_soft_deleting_component(self): + """ Test soft deleting a component that's in a unit (but not removing it) """ + unit = self.create_unit_with_components([self.component_1, self.component_2]) + authoring_api.publish_all_drafts(self.learning_package.id) + + # Now soft delete component 2 + authoring_api.soft_delete_draft(self.component_2.pk) + + # Now it should not be listed in the unit: + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(self.component_1_v1), + # component 2 is soft deleted from the draft. + # TODO: should we return some kind of placeholder here, to indicate that a component is still listed in the + # unit's component list but has been soft deleted, and will be fully deleted when published, or restored if + # reverted? + ] + assert unit.versioning.has_unpublished_changes is False # The unit itself and its component list is not changed + assert authoring_api.contains_unpublished_changes(unit.pk) # But it CONTAINS an unpublished change (a deletion) + # The published version of the unit is not yet affected: + assert authoring_api.get_components_in_unit(unit, published=True) == [ + Entry(self.component_1_v1), + Entry(self.component_2_v1), + ] + + # But when we publish the deletion, the published version is affected: + authoring_api.publish_all_drafts(self.learning_package.id) + assert authoring_api.contains_unpublished_changes(unit.pk) is False + assert authoring_api.get_components_in_unit(unit, published=True) == [ + Entry(self.component_1_v1), + ] + + def test_soft_deleting_and_removing_component(self): + """ Test soft deleting a component that's in a unit AND removing it """ + unit = self.create_unit_with_components([self.component_1, self.component_2]) + authoring_api.publish_all_drafts(self.learning_package.id) + + # Now soft delete component 2 + authoring_api.soft_delete_draft(self.component_2.pk) + # And remove it from the unit: + authoring_api.create_next_unit_version( + unit=unit, + title="Revised with component 2 deleted", + components=[self.component_1], + created=self.now, + ) + + # Now it should not be listed in the unit: + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(self.component_1_v1), + ] + assert unit.versioning.has_unpublished_changes is True + assert authoring_api.contains_unpublished_changes(unit.pk) + # The published version of the unit is not yet affected: + assert authoring_api.get_components_in_unit(unit, published=True) == [ + Entry(self.component_1_v1), + Entry(self.component_2_v1), + ] + + # But when we publish the deletion, the published version is affected: + authoring_api.publish_all_drafts(self.learning_package.id) + assert authoring_api.contains_unpublished_changes(unit.pk) is False + assert authoring_api.get_components_in_unit(unit, published=True) == [ + Entry(self.component_1_v1), + ] + + def test_soft_deleting_pinned_component(self): + """ Test soft deleting a pinned πŸ“Œ component that's in a unit """ + unit = self.create_unit_with_components([self.component_1_v1, self.component_2_v1]) + authoring_api.publish_all_drafts(self.learning_package.id) + + # Now soft delete component 2 + authoring_api.soft_delete_draft(self.component_2.pk) + + # Now it should still be listed in the unit: + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(self.component_1_v1, pinned=True), + Entry(self.component_2_v1, pinned=True), + ] + assert unit.versioning.has_unpublished_changes is False # The unit itself and its component list is not changed + assert authoring_api.contains_unpublished_changes(unit.pk) is False # nor does it contain changes + # The published version of the unit is also not affected: + assert authoring_api.get_components_in_unit(unit, published=True) == [ + Entry(self.component_1_v1, pinned=True), + Entry(self.component_2_v1, pinned=True), + ] + + def test_soft_delete_unit(self): + """ + I can delete a unit without deleting the components it contains. + + See https://github.com/openedx/frontend-app-authoring/issues/1693 + """ + # Create two units, one of which we will soon delete: + unit_to_delete = self.create_unit_with_components([self.component_1, self.component_2]) + other_unit = self.create_unit_with_components([self.component_1], key="other") + + # Publish everything: + authoring_api.publish_all_drafts(self.learning_package.id) + # Delete the unit: + authoring_api.soft_delete_draft(unit_to_delete.publishable_entity_id) + unit_to_delete.refresh_from_db() + # Now the draft unit is [soft] deleted, but the components, published unit, and other unit is unaffected: + assert unit_to_delete.versioning.draft is None # Unit is soft deleted. + assert unit_to_delete.versioning.published is not None + self.component_1.refresh_from_db() + assert self.component_1.versioning.draft is not None + assert authoring_api.get_components_in_unit(other_unit, published=False) == [Entry(self.component_1_v1)] + + # Publish everything: + authoring_api.publish_all_drafts(self.learning_package.id) + # Now the unit's published version is also deleted, but nothing else is affected. + unit_to_delete.refresh_from_db() + assert unit_to_delete.versioning.draft is None # Unit is soft deleted. + assert unit_to_delete.versioning.published is None + self.component_1.refresh_from_db() + assert self.component_1.versioning.draft is not None + assert self.component_1.versioning.published is not None + assert authoring_api.get_components_in_unit(other_unit, published=False) == [Entry(self.component_1_v1)] + assert authoring_api.get_components_in_unit(other_unit, published=True) == [Entry(self.component_1_v1)] + + def test_snapshots_of_published_unit(self): + """ + Test that we can access snapshots of the historic published version of + units and their contents. + """ + # At first the unit has one component (unpinned): + unit = self.create_unit_with_components([self.component_1]) + self.modify_component(self.component_1, title="Component 1 as of checkpoint 1") + + # Publish everything, creating Checkpoint 1 + checkpoint_1 = authoring_api.publish_all_drafts(self.learning_package.id, message="checkpoint 1") + + ######################################################################## + + # Now we update the title of the component. + self.modify_component(self.component_1, title="Component 1 as of checkpoint 2") + # Publish everything, creating Checkpoint 2 + checkpoint_2 = authoring_api.publish_all_drafts(self.learning_package.id, message="checkpoint 2") + ######################################################################## + + # Now add a second component to the unit: + self.modify_component(self.component_1, title="Component 1 as of checkpoint 3") + self.modify_component(self.component_2, title="Component 2 as of checkpoint 3") + authoring_api.create_next_unit_version( + unit=unit, + title="Unit title in checkpoint 3", + components=[self.component_1, self.component_2], + created=self.now, + ) + # Publish everything, creating Checkpoint 3 + checkpoint_3 = authoring_api.publish_all_drafts(self.learning_package.id, message="checkpoint 3") + ######################################################################## + + # Now add a third component to the unit, a pinned πŸ“Œ version of component 1. + # This will test pinned versions and also test adding at the beginning rather than the end of the unit. + authoring_api.create_next_unit_version( + unit=unit, + title="Unit title in checkpoint 4", + components=[self.component_1_v1, self.component_1, self.component_2], + created=self.now, + ) + # Publish everything, creating Checkpoint 4 + checkpoint_4 = authoring_api.publish_all_drafts(self.learning_package.id, message="checkpoint 4") + ######################################################################## + + # Modify the drafts, but don't publish: + self.modify_component(self.component_1, title="Component 1 draft") + self.modify_component(self.component_2, title="Component 2 draft") + + # Now fetch the snapshots: + as_of_checkpoint_1 = authoring_api.get_components_in_published_unit_as_of(unit, checkpoint_1.pk) + assert [cv.component_version.title for cv in as_of_checkpoint_1] == [ + "Component 1 as of checkpoint 1", + ] + as_of_checkpoint_2 = authoring_api.get_components_in_published_unit_as_of(unit, checkpoint_2.pk) + assert [cv.component_version.title for cv in as_of_checkpoint_2] == [ + "Component 1 as of checkpoint 2", + ] + as_of_checkpoint_3 = authoring_api.get_components_in_published_unit_as_of(unit, checkpoint_3.pk) + assert [cv.component_version.title for cv in as_of_checkpoint_3] == [ + "Component 1 as of checkpoint 3", + "Component 2 as of checkpoint 3", + ] + as_of_checkpoint_4 = authoring_api.get_components_in_published_unit_as_of(unit, checkpoint_4.pk) + assert [cv.component_version.title for cv in as_of_checkpoint_4] == [ + "Querying Counting Problem", # Pinned. This title is self.component_1_v1.title (original v1 title) + "Component 1 as of checkpoint 3", # we didn't modify these components so they're same as in snapshot 3 + "Component 2 as of checkpoint 3", # we didn't modify these components so they're same as in snapshot 3 + ] + + def test_units_containing(self): + """ + Test that we can efficiently get a list of all the [draft] units + containing a given component. + """ + component_1_v2 = self.modify_component(self.component_1, title="modified component 1") + + # Create a few units, some of which contain component 1 and others which don't: + # Note: it is important that some of these units contain other components, to ensure the complex JOINs required + # for this query are working correctly, especially in the case of ignore_pinned=True. + # Unit 1 βœ… has component 1, pinned πŸ“Œ to V1 + unit1_1pinned = self.create_unit_with_components([self.component_1_v1, self.component_2], key="u1") + # Unit 2 βœ… has component 1, pinned πŸ“Œ to V2 + unit2_1pinned_v2 = self.create_unit_with_components([component_1_v2, self.component_2_v1], key="u2") + # Unit 3 doesn't contain it + _unit3_no = self.create_unit_with_components([self.component_2], key="u3") + # Unit 4 βœ… has component 1, unpinned + unit4_unpinned = self.create_unit_with_components([ + self.component_1, self.component_2, self.component_2_v1, + ], key="u4") + # Units 5/6 don't contain it + _unit5_no = self.create_unit_with_components([self.component_2_v1, self.component_2], key="u5") + _unit6_no = self.create_unit_with_components([], key="u6") + + # No need to publish anything as the get_containers_with_entity() API only considers drafts (for now). + + with self.assertNumQueries(1): + result = [ + c.unit for c in + authoring_api.get_containers_with_entity(self.component_1.pk).select_related("unit") + ] + assert result == [ + unit1_1pinned, + unit2_1pinned_v2, + unit4_unpinned, + ] + + # Test retrieving only "unpinned", for cases like potential deletion of a component, where we wouldn't care + # about pinned uses anyways (they would be unaffected by a delete). + + with self.assertNumQueries(1): + result2 = [ + c.unit for c in + authoring_api.get_containers_with_entity(self.component_1.pk, ignore_pinned=True).select_related("unit") + ] + assert result2 == [unit4_unpinned] + + # Tests TODO: + # Test that I can get a [PublishLog] history of a given unit and all its children, including children that aren't + # currently in the unit and excluding children that are only in other units. + # Test that I can get a [PublishLog] history of a given unit and its children, that includes changes made to the + # child components while they were part of the unit but excludes changes made to those children while they were + # not part of the unit. 🫣 diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index feda50d0..08ca9398 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -44,8 +44,9 @@ def check_taxonomy( data, - taxonomy_id, - name, + *, + taxonomy_id: int, + name: str, description="", enabled=True, allow_multiple=True, @@ -332,7 +333,7 @@ def test_detail_taxonomy( expected_data["can_change_taxonomy"] = is_admin expected_data["can_delete_taxonomy"] = is_admin expected_data["can_tag_object"] = False - check_taxonomy(response.data, taxonomy.pk, **expected_data) + check_taxonomy(response.data, taxonomy_id=taxonomy.pk, **expected_data) # type: ignore[arg-type] def test_detail_system_taxonomy(self): url = TAXONOMY_DETAIL_URL.format(pk=LANGUAGE_TAXONOMY_ID) @@ -384,11 +385,11 @@ def test_create_taxonomy(self, user_attr: str | None, expected_status: int): create_data["can_change_taxonomy"] = True create_data["can_delete_taxonomy"] = True create_data["can_tag_object"] = False - check_taxonomy(response.data, response.data["id"], **create_data) + check_taxonomy(response.data, taxonomy_id=response.data["id"], **create_data) # type: ignore[arg-type] url = TAXONOMY_DETAIL_URL.format(pk=response.data["id"]) response = self.client.get(url) - check_taxonomy(response.data, response.data["id"], **create_data) + check_taxonomy(response.data, taxonomy_id=response.data["id"], **create_data) # type: ignore[arg-type] def test_create_without_export_id(self): url = TAXONOMY_LIST_URL @@ -408,7 +409,7 @@ def test_create_without_export_id(self): create_data["can_tag_object"] = False check_taxonomy( response.data, - response.data["id"], + taxonomy_id=response.data["id"], export_id="2-taxonomy-data-3", **create_data, ) @@ -464,7 +465,7 @@ def test_update_taxonomy(self, user_attr, expected_status): response = self.client.get(url) check_taxonomy( response.data, - response.data["id"], + taxonomy_id=response.data["id"], **{ "name": "new name", "description": "taxonomy description", @@ -525,7 +526,7 @@ def test_patch_taxonomy(self, user_attr, expected_status): response = self.client.get(url) check_taxonomy( response.data, - response.data["id"], + taxonomy_id=response.data["id"], **{ "name": "new name", "enabled": True, @@ -1040,7 +1041,15 @@ def test_object_tags_remaining_http_methods( ("staff", "taxonomy", {}, ["Invalid"], status.HTTP_400_BAD_REQUEST, "abc.xyz"), ) @ddt.unpack - def test_tag_object(self, user_attr, taxonomy_attr, taxonomy_flags, tag_values, expected_status, object_id): + def test_tag_object( # pylint: disable=too-many-positional-arguments + self, + user_attr, + taxonomy_attr, + taxonomy_flags, + tag_values, + expected_status, + object_id, + ): if user_attr: user = getattr(self, user_attr) self.client.force_authenticate(user=user)