diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index c15af56f2..d2a9779dd 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -2,4 +2,4 @@ Open edX Learning ("Learning Core"). """ -__version__ = "0.22.0" +__version__ = "0.23.0" diff --git a/openedx_learning/apps/authoring/publishing/admin.py b/openedx_learning/apps/authoring/publishing/admin.py index c36262b3e..a710536c8 100644 --- a/openedx_learning/apps/authoring/publishing/admin.py +++ b/openedx_learning/apps/authoring/publishing/admin.py @@ -4,10 +4,19 @@ from __future__ import annotations from django.contrib import admin +from django.db.models import Count from openedx_learning.lib.admin_utils import ReadOnlyModelAdmin, one_to_one_related_model_html -from .models import LearningPackage, PublishableEntity, Published, PublishLog, PublishLogRecord +from .models import ( + DraftChangeLog, + DraftChangeLogRecord, + LearningPackage, + PublishableEntity, + PublishLog, + PublishLogRecord, +) +from .models.publish_log import Published @admin.register(LearningPackage) @@ -171,3 +180,69 @@ def published_at(self, published_obj): def message(self, published_obj): return published_obj.publish_log_record.publish_log.message + + +class DraftChangeLogRecordTabularInline(admin.TabularInline): + """ + Tabular inline for a single Draft change. + """ + model = DraftChangeLogRecord + + fields = ( + "entity", + "title", + "old_version_num", + "new_version_num", + ) + readonly_fields = fields + + def get_queryset(self, request): + queryset = super().get_queryset(request) + return queryset.select_related("entity", "old_version", "new_version") \ + .order_by("entity__key") + + def old_version_num(self, draft_change: DraftChangeLogRecord): + if draft_change.old_version is None: + return "-" + return draft_change.old_version.version_num + + def new_version_num(self, draft_change: DraftChangeLogRecord): + if draft_change.new_version is None: + return "-" + return draft_change.new_version.version_num + + def title(self, draft_change: DraftChangeLogRecord): + """ + Get the title to display for the DraftChange + """ + if draft_change.new_version: + return draft_change.new_version.title + if draft_change.old_version: + return draft_change.old_version.title + return "" + + +@admin.register(DraftChangeLog) +class DraftChangeSetAdmin(ReadOnlyModelAdmin): + """ + Read-only admin to view Draft changes (via inline tables) + """ + inlines = [DraftChangeLogRecordTabularInline] + fields = ( + "uuid", + "learning_package", + "num_changes", + "changed_at", + "changed_by", + ) + readonly_fields = fields + list_display = fields + list_filter = ["learning_package"] + + def num_changes(self, draft_change_set): + return draft_change_set.num_changes + + def get_queryset(self, request): + queryset = super().get_queryset(request) + return queryset.select_related("learning_package", "changed_by") \ + .annotate(num_changes=Count("records")) diff --git a/openedx_learning/apps/authoring/publishing/api.py b/openedx_learning/apps/authoring/publishing/api.py index 52f666ac3..4e1af6414 100644 --- a/openedx_learning/apps/authoring/publishing/api.py +++ b/openedx_learning/apps/authoring/publishing/api.py @@ -6,19 +6,24 @@ """ from __future__ import annotations +from contextlib import nullcontext from dataclasses import dataclass from datetime import datetime, timezone from enum import Enum -from typing import TypeVar +from typing import ContextManager, TypeVar from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db.models import F, Q, QuerySet from django.db.transaction import atomic +from .contextmanagers import DraftChangeLogContext from .models import ( Container, ContainerVersion, Draft, + DraftChangeLog, + DraftChangeLogRecord, + DraftSideEffect, EntityList, EntityListRow, LearningPackage, @@ -27,10 +32,10 @@ PublishableEntityMixin, PublishableEntityVersion, PublishableEntityVersionMixin, - Published, PublishLog, PublishLogRecord, ) +from .models.publish_log import Published # 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 @@ -82,6 +87,7 @@ "contains_unpublished_changes", "get_containers_with_entity", "get_container_children_count", + "bulk_draft_changes_for", ] @@ -220,10 +226,13 @@ def create_publishable_entity_version( created=created, created_by_id=created_by, ) - Draft.objects.update_or_create( - entity_id=entity_id, - defaults={"version": version}, + set_draft_version( + entity_id, + version.id, + set_at=created, + set_by=created_by, ) + return version @@ -447,25 +456,247 @@ def get_published_version(publishable_entity_id: int, /) -> PublishableEntityVer def set_draft_version( - publishable_entity_id: int, + draft_or_id: Draft | int, publishable_entity_version_pk: int | None, /, + set_at: datetime | None = None, + set_by: int | None = None, # User.id ) -> None: """ Modify the Draft of a PublishableEntity to be a PublishableEntityVersion. + The ``draft`` argument can be either a Draft model object, or the primary + key of a Draft/PublishableEntity (Draft is defined so these will be the same + value). + This would most commonly be used to set the Draft to point to a newly created PublishableEntityVersion that was created in Studio (because someone edited some content). Setting a Draft's version to None is like deleting it from Studio's editing point of view (see ``soft_delete_draft`` for more details). + + Calling this function attaches a new DraftChangeLogRecordand attaches it to a + DraftChangeLog. + + This function will create DraftSideEffect entries and properly add any + containers that may have been affected by this draft update, UNLESS it is + called from within a bulk_draft_changes_for block. If it is called from + inside a bulk_draft_changes_for block, it will not add side-effects for + containers, as bulk_draft_changes_for will automatically do that when the + block exits. + """ + if set_at is None: + set_at = datetime.now(tz=timezone.utc) + + with atomic(savepoint=False): + if isinstance(draft_or_id, Draft): + draft = draft_or_id + elif isinstance(draft_or_id, int): + draft, _created = Draft.objects.select_related("entity") \ + .get_or_create(entity_id=draft_or_id) + else: + class_name = draft_or_id.__class__.__name__ + raise TypeError( + f"draft_or_id must be a Draft or int, not ({class_name})" + ) + + # If the Draft is already pointing at this version, there's nothing to do. + old_version_id = draft.version_id + if old_version_id == publishable_entity_version_pk: + return + + # The actual update of the Draft model is here. Everything after this + # block is bookkeeping in our DraftChangeLog. + draft.version_id = publishable_entity_version_pk + draft.save() + + # Check to see if we're inside a context manager for an active + # DraftChangeLog (i.e. what happens if the caller is using the public + # bulk_draft_changes_for() API call), or if we have to make our own. + learning_package_id = draft.entity.learning_package_id + active_change_log = DraftChangeLogContext.get_active_draft_change_log( + learning_package_id + ) + if active_change_log: + change = _add_to_existing_draft_change_log( + active_change_log, + draft.entity_id, + old_version_id=old_version_id, + new_version_id=publishable_entity_version_pk, + ) + # We explicitly *don't* create container side effects here because + # there may be many changes in this DraftChangeLog, some of which + # haven't been made yet. It wouldn't make sense to create a side + # effect that says, "this Unit changed because this Component in it + # changed" if we were changing that same Unit later on in the same + # DraftChangeLog, because that new Unit version might not even + # include the child Component. So we'll let DraftChangeLogContext + # do that work when it exits its context. + else: + # This means there is no active DraftChangeLog, so we create our own + # and add our DraftChangeLogRecord to it. This has the minor + # optimization that we don't have to check for an existing + # DraftChangeLogRecord, because we know it can't exist yet. + change_log = DraftChangeLog.objects.create( + learning_package_id=learning_package_id, + changed_at=set_at, + changed_by_id=set_by, + ) + change = DraftChangeLogRecord.objects.create( + draft_change_log=change_log, + entity_id=draft.entity_id, + old_version_id=old_version_id, + new_version_id=publishable_entity_version_pk, + ) + _create_container_side_effects_for_draft_change(change) + + +def _add_to_existing_draft_change_log( + active_change_log: DraftChangeLog, + entity_id: int, + old_version_id: int | None, + new_version_id: int | None, +) -> DraftChangeLogRecord: + """ + Create or update a DraftChangeLogRecord for the active_change_log passed in. + + The an active_change_log may have many DraftChangeLogRecords already + associated with it. A DraftChangeLog can only have one DraftChangeLogRecord + per PublishableEntity, e.g. the same Component can't go from v1 to v2 and v2 + to v3 in the same DraftChangeLog. The DraftChangeLogRecord is meant to + capture the before and after states of the Draft version for that entity, + so we always keep the first value for old_version, while updating to the + most recent value for new_version. + + So for example, if we called this function with the same active_change_log + and the same entity_id but with versions: (None, v1), (v1, v2), (v2, v3); + we would collapse them into one DraftChangeLogrecord with old_version = None + and new_version = v3. + """ + try: + # Check to see if this PublishableEntity has already been changed in + # this DraftChangeLog. If so, we update that record instead of creating + # a new one. + change = DraftChangeLogRecord.objects.get( + draft_change_log=active_change_log, + entity_id=entity_id, + ) + if change.old_version_id == new_version_id: + # Special case: This change undoes the previous change(s). The value + # in change.old_version_id represents the Draft version before the + # DraftChangeLog was started, regardless of how many times we've + # changed it since we entered the bulk_draft_changes_for() context. + # If we get here in the code, it means that we're now setting the + # Draft version of this entity to be exactly what it was at the + # start, and we should remove it entirely from the DraftChangeLog. + # + # It's important that we remove these cases, because we use the + # old_version == new_version convention to record entities that have + # changed purely due to side-effects. + change.delete() + else: + # Normal case: We update the new_version, but leave the old_version + # as is. The old_version represents what the Draft was pointing to + # when the bulk_draft_changes_for() context started, so it persists + # if we change the same entity multiple times in the DraftChangeLog. + change.new_version_id = new_version_id + change.save() + except DraftChangeLogRecord.DoesNotExist: + # If we're here, this is the first DraftChangeLogRecord we're making for + # this PublishableEntity in the active DraftChangeLog. + change = DraftChangeLogRecord.objects.create( + draft_change_log=active_change_log, + entity_id=entity_id, + old_version_id=old_version_id, + new_version_id=new_version_id, + ) + + return change + + +def _create_container_side_effects_for_draft_change_log(change_log: DraftChangeLog): """ - draft = Draft.objects.get(entity_id=publishable_entity_id) - draft.version_id = publishable_entity_version_pk - draft.save() + Iterate through the whole DraftChangeLog and process side-effects. + """ + processed_entity_ids: set[int] = set() + for change in change_log.records.all(): + _create_container_side_effects_for_draft_change( + change, + processed_entity_ids=processed_entity_ids, + ) + + +def _create_container_side_effects_for_draft_change( + original_change: DraftChangeLogRecord, + processed_entity_ids: set | None = None +): + """ + Given a draft change, add side effects for all affected containers. + + This should only be run after the DraftChangeLogRecord has been otherwise + fully written out. We want to avoid the scenario where we create a + side-effect that a Component change affects a Unit if the Unit version is + also changed (maybe even deleted) in the same DraftChangeLog. + + The `processed_entity_ids` set holds the entity IDs that we've already + calculated side-effects for. This is to save us from recalculating side- + effects for the same ancestor relationships over and over again. So if we're + calling this function in a loop for all the Components in a Unit, we won't + be recalculating the Unit's side-effect on its Subsection, and its + Subsection's side-effect on its Section. + + TODO: This could get very expensive with the get_containers_with_entity + calls. We should measure the impact of this. + """ + if processed_entity_ids is None: + # An optimization, but also a guard against infinite side-effect loops. + processed_entity_ids = set() + + changes_and_containers = [ + (original_change, container) + for container + in get_containers_with_entity(original_change.entity_id, ignore_pinned=True) + ] + while changes_and_containers: + change, container = changes_and_containers.pop() + + # If the container is not already in the DraftChangeLog, we need to + # add it. Since it's being caused as a DraftSideEffect, we're going + # add it with the old_version == new_version convention. + container_draft_version_pk = container.versioning.draft.pk + container_change, _created = DraftChangeLogRecord.objects.get_or_create( + draft_change_log=change.draft_change_log, + entity_id=container.pk, + defaults={ + 'old_version_id': container_draft_version_pk, + 'new_version_id': container_draft_version_pk + } + ) + + # Mark that change in the current loop has the side effect of changing + # the parent container. We'll do this regardless of whether the + # container version itself also changed. If a Unit has a Component and + # both the Unit and Component have their versions incremented, then the + # Unit has changed in both ways (the Unit's internal metadata as well as + # the new version of the child component). + DraftSideEffect.objects.get_or_create(cause=change, effect=container_change) + processed_entity_ids.add(change.entity_id) + + # Now we find the next layer up of containers. So if the originally + # passed in publishable_entity_id was for a Component, then the + # ``container`` we've been creating the side effect for in this loop + # is the Unit, and ``parents_of_container`` would be any Sequences + # that contain the Unit. + parents_of_container = get_containers_with_entity(container.pk, ignore_pinned=True) + + changes_and_containers.extend( + (container_change, container_parent) + for container_parent in parents_of_container + if container_parent.pk not in processed_entity_ids + ) -def soft_delete_draft(publishable_entity_id: int, /) -> None: +def soft_delete_draft(publishable_entity_id: int, /, deleted_by: int | None = None) -> None: """ Sets the Draft version to None. @@ -475,10 +706,15 @@ def soft_delete_draft(publishable_entity_id: int, /) -> None: of pointing the Draft back to the most recent ``PublishableEntityVersion`` for a given ``PublishableEntity``. """ - return set_draft_version(publishable_entity_id, None) + return set_draft_version(publishable_entity_id, None, set_by=deleted_by) -def reset_drafts_to_published(learning_package_id: int, /) -> None: +def reset_drafts_to_published( + learning_package_id: int, + /, + reset_at: datetime | None = None, + reset_by: int | None = None, # User.id +) -> None: """ Reset all Drafts to point to the most recently Published versions. @@ -502,27 +738,55 @@ def reset_drafts_to_published(learning_package_id: int, /) -> None: it's important that the code creating the "next" version_num looks at the latest version created for a PublishableEntity (its ``latest`` attribute), rather than basing it off of the version that Draft points to. - - Also, there is no current immutable record for when a reset happens. It's - not like a publish that leaves an entry in the ``PublishLog``. """ + if reset_at is None: + reset_at = datetime.now(tz=timezone.utc) + # These are all the drafts that are different from the published versions. draft_qset = Draft.objects \ .select_related("entity__published") \ .filter(entity__learning_package_id=learning_package_id) \ - .exclude(entity__published__version_id=F("version_id")) + .exclude(entity__published__version_id=F("version_id")) \ + .exclude( + # NULL != NULL in SQL, so we want to exclude entries + # where both the published version and draft version + # are None. This edge case happens when we create + # something and then delete it without publishing, and + # then reset Drafts to their published state. + Q(entity__published__version__isnull=True) & + Q(version__isnull=True) + ) + # If there's nothing to reset because there are no changes from the + # published version, just return early rather than making an empty + # DraftChangeLog. + if not draft_qset: + return + + active_change_log = DraftChangeLogContext.get_active_draft_change_log(learning_package_id) + + # If there's an active DraftChangeLog, we're already in a transaction, so + # there's no need to open a new one. + tx_context: ContextManager + if active_change_log: + tx_context = nullcontext() + else: + tx_context = bulk_draft_changes_for( + learning_package_id, changed_at=reset_at, changed_by=reset_by + ) - # Note: We can't do an .update with a F() on a joined field in the ORM, so - # we have to loop through the drafts individually to reset them. We can - # rework this into a bulk update or custom SQL if it becomes a performance - # issue. - with atomic(): + with tx_context: + # Note: We can't do an .update with a F() on a joined field in the ORM, + # so we have to loop through the drafts individually to reset them + # anyhow. We can rework this into a bulk update or custom SQL if it + # becomes a performance issue, as long as we also port over the + # bookkeeping code in set_draft_version. for draft in draft_qset: if hasattr(draft.entity, 'published'): - draft.version_id = draft.entity.published.version_id + published_version_id = draft.entity.published.version_id else: - draft.version = None - draft.save() + published_version_id = None + + set_draft_version(draft, published_version_id) def register_content_models( @@ -1103,6 +1367,7 @@ def get_containers_with_entity( ignore_pinned: if true, ignore any pinned references to the entity. """ if ignore_pinned: + # TODO: Do we need to run distinct() on this? Will fix in https://github.com/openedx/openedx-learning/issues/303 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 @@ -1145,3 +1410,47 @@ def get_container_children_count( else: filter_deleted = {"entity__draft__version__isnull": False} return container_version.entity_list.entitylistrow_set.filter(**filter_deleted).count() + + +def bulk_draft_changes_for( + learning_package_id: int, + changed_by: int | None = None, + changed_at: datetime | None = None +) -> DraftChangeLogContext: + """ + Context manager to do a single batch of Draft changes in. + + Each publishable entity that is edited in this context will be tied to a + single DraftChangeLogRecord, representing the cumulative changes made to + that entity. Upon closing of the context, side effects of these changes will + be calcuated, which may result in more DraftChangeLogRecords being created + or updated. The resulting DraftChangeLogRecords and DraftChangeSideEffects + will be tied together into a single DraftChangeLog, representing the + collective changes to the learning package that happened in this context. + All changes will be committed in a single atomic transaction. + + Example:: + + with bulk_draft_changes_for(learning_package.id): + for section in course: + update_section_drafts(learning_package.id, section) + + If you make a change to an entity *without* using this context manager, then + the individual change (and its side effects) will be automatically wrapped + in a one-off change context. For example, this:: + + update_one_component(component.learning_package, component) + + is identical to this:: + + with bulk_draft_changes_for(component.learning_package.id): + update_one_component(component.learning_package.id, component) + """ + return DraftChangeLogContext( + learning_package_id, + changed_at=changed_at, + changed_by=changed_by, + exit_callbacks=[ + _create_container_side_effects_for_draft_change_log, + ] + ) diff --git a/openedx_learning/apps/authoring/publishing/contextmanagers.py b/openedx_learning/apps/authoring/publishing/contextmanagers.py new file mode 100644 index 000000000..64ca9fb1f --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/contextmanagers.py @@ -0,0 +1,157 @@ +""" +Context Managers for internal use in the publishing app. + +Do not use this directly outside the publishing app. Use the public API's +bulk_draft_changes_for instead (which will invoke this internally). +""" +from __future__ import annotations + +from contextvars import ContextVar +from datetime import datetime, timezone +from typing import Callable + +from django.db.transaction import Atomic + +from .models import DraftChangeLog + + +class DraftChangeLogContext(Atomic): + """ + Context manager for batching draft changes into DraftChangeChangeLogs. + + 🛑 This is a PRIVATE implementation. Outside of the publishing app, clients + should use the bulk_draft_changes_for() API call instead of using this + manager directly, since this is a bit experimental and the implementation + may shift around a bit. + + The main idea is that we want to create a context manager that will keep + track of what the "active" DraftChangeChangeLogs is for a given + LearningPackage. The problem is that declaring the context can happen many + layers higher in the stack than doing the draft changes. For instance, + imagine:: + + with bulk_draft_changes_for(learning_package.id): + for section in course: + update_section_drafts(learning_package_id, section) + + In this hypothetical code block, update_section_drafts might call a function + to update sequences, which calls something to update units, etc. Only at the + very bottom layer of the stack will those code paths actually alter the + drafts themselves. It would make the API too cumbersome to explicitly pass + the active DraftChangeChangeLog through every layer. So instead, we use this + class to essentially keep the active DraftChangeChangeLog in a global + (ContextVar) so that the publishing API draft-related code can access it + later. + + Since it is possible to nest context managers, we keep a list of the + DraftChangeChangeLogs that have been created and treat it like a stack that + gets pushed to whenever we __enter__ and popped off whenever we __exit__. + + DraftChangeLogContext also subclasses Django's Atomic context manager, since + any operation on multiple Drafts as part of a DraftChangeLog will want to be + an atomic operation. + """ + _draft_change_logs: ContextVar[list | None] = ContextVar('_draft_change_logs', default=None) + + def __init__( + self, + learning_package_id: int, + changed_at: datetime | None = None, + changed_by: int | None = None, + exit_callbacks: list[Callable[[DraftChangeLog], None]] | None = None + ) -> None: + super().__init__(using=None, savepoint=False, durable=False) + + self.learning_package_id = learning_package_id + self.changed_by = changed_by + self.changed_at = changed_at or datetime.now(tz=timezone.utc) + + # This will get properly initialized on __enter__() + self.draft_change_log = None + + # We currently use self.exit_callbacks as a way to run parent/child + # side-effect creation. DraftChangeLogContext itself is a lower-level + # part of the code that doesn't understand what containers are. + self.exit_callbacks = exit_callbacks or [] + + @classmethod + def get_active_draft_change_log(cls, learning_package_id: int) -> DraftChangeLog | None: + """ + Get the DraftChangeLogContext for new DraftChangeLogRecords. + + This is expected to be called internally by the publishing API when it + modifies Drafts. If there is no active DraftChangeLog, this method will + return None, and the caller should create their own DraftChangeLog. + """ + draft_change_logs = cls._draft_change_logs.get() + + # If we've never used this manager... + if draft_change_logs is None: + return None + + # Otherwise, find the most recently created DraftChangeLog *that matches + # the learning_package_id*. This is for two reasons: + # + # 1. We might nest operations so that we're working across multiple + # LearningPackages at once, e.g. copying content from different + # libraries and importing them into another library. + # 2. It's a guard in case we somehow got the global state management + # wrong. We want the failure mode to be "we didn't find the + # DraftChangeLog you were looking for, so make another one and suffer + # a performance penalty", as opposed to, "we accidentally gave you a + # DraftChangeLog for an entirely different LearningPackage, and now + # your Draft data is corrupted." + for draft_change_log in reversed(draft_change_logs): + if draft_change_log.learning_package_id == learning_package_id: + return draft_change_log + + # If we got here, then either the list was empty (the manager was used + # at some point but exited), or none of the DraftChangeLogs are for the + # correct LearningPackage. + return None + + def __enter__(self): + """ + Enter our context. + + This starts the transaction and sets up our active DraftChangeLog. + """ + super().__enter__() + + self.draft_change_log = DraftChangeLog.objects.create( + learning_package_id=self.learning_package_id, + changed_by_id=self.changed_by, + changed_at=self.changed_at, + ) + draft_change_sets = self._draft_change_logs.get() + if not draft_change_sets: + draft_change_sets = [] + draft_change_sets.append(self.draft_change_log) + self._draft_change_logs.set(draft_change_sets) + + return self.draft_change_log + + def __exit__(self, exc_type, exc_value, traceback): + """ + Exit our context. + + Pops the active DraftChangeLog off of our stack and run any + post-processing callbacks needed. This callback mechanism is how child- + parent side-effects are calculated. + """ + draft_change_logs = self._draft_change_logs.get() + if draft_change_logs: + draft_change_log = draft_change_logs.pop() + for exit_callback in self.exit_callbacks: + exit_callback(draft_change_log) + + # Edge case: the draft changes that accumulated during our context + # cancel each other out, and there are no actual + # DraftChangeLogRecords at the end. In this case, we might as well + # delete the entire DraftChangeLog. + if not draft_change_log.records.exists(): + draft_change_log.delete() + + self._draft_change_logs.set(draft_change_logs) + + return super().__exit__(exc_type, exc_value, traceback) diff --git a/openedx_learning/apps/authoring/publishing/migrations/0006_draftchangelog.py b/openedx_learning/apps/authoring/publishing/migrations/0006_draftchangelog.py new file mode 100644 index 000000000..b72e16843 --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/migrations/0006_draftchangelog.py @@ -0,0 +1,68 @@ +# Generated by Django 4.2.18 on 2025-03-28 02:21 + +import uuid + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + +import openedx_learning.lib.validators + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('oel_publishing', '0005_alter_entitylistrow_options'), + ] + + operations = [ + migrations.CreateModel( + name='DraftChangeLog', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('uuid', models.UUIDField(default=uuid.uuid4, editable=False, unique=True, verbose_name='UUID')), + ('changed_at', models.DateTimeField(validators=[openedx_learning.lib.validators.validate_utc_datetime])), + ('changed_by', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)), + ('learning_package', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='oel_publishing.learningpackage')), + ], + options={ + 'verbose_name': 'Draft Change Log', + 'verbose_name_plural': 'Draft Change Logs', + }, + ), + migrations.CreateModel( + name='DraftChangeLogRecord', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('draft_change_log', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='changes', to='oel_publishing.draftchangelog')), + ('entity', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, to='oel_publishing.publishableentity')), + ('new_version', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, to='oel_publishing.publishableentityversion')), + ('old_version', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, related_name='+', to='oel_publishing.publishableentityversion')), + ], + options={ + 'verbose_name': 'Draft Log', + 'verbose_name_plural': 'Draft Log', + }, + ), + migrations.CreateModel( + name='DraftSideEffect', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('cause', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, related_name='causes', to='oel_publishing.draftchangelogrecord')), + ('effect', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, related_name='caused_by', to='oel_publishing.draftchangelogrecord')), + ], + ), + migrations.AddConstraint( + model_name='draftsideeffect', + constraint=models.UniqueConstraint(fields=('cause', 'effect'), name='oel_pub_dse_uniq_c_e'), + ), + migrations.AddIndex( + model_name='draftchangelogrecord', + index=models.Index(fields=['entity', '-draft_change_log'], name='oel_dlr_idx_entity_rdcl'), + ), + migrations.AddConstraint( + model_name='draftchangelogrecord', + constraint=models.UniqueConstraint(fields=('draft_change_log', 'entity'), name='oel_dlr_uniq_dcl'), + ), + ] diff --git a/openedx_learning/apps/authoring/publishing/migrations/0007_bootstrap_draftchangelog.py b/openedx_learning/apps/authoring/publishing/migrations/0007_bootstrap_draftchangelog.py new file mode 100644 index 000000000..5c8ab3735 --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/migrations/0007_bootstrap_draftchangelog.py @@ -0,0 +1,94 @@ +""" +Bootstrap DraftChangeLogs + +DraftChangeLog and DraftChangeLogRecord are being introduced after Drafts, so +we're going to retroactively make entries for all the changes that were in our +Learning Packages. + +This migration will try to reconstruct create, edit, reset-to-published, and +delete operations, but it won't be fully accurate because we only have the +create dates of the versions and the current state of active Drafts to go on. +This means we won't accurately capture when things were deleted and then reset, +or when reset and then later edited. We're also missing the user for a number of +these operations, so we'll add those with null created_by entries. Addressing +these gaps is a big part of why we created DraftChangeLogs in the first place. +""" +# Generated by Django 4.2.18 on 2025-03-13 10:29 +import logging +from datetime import datetime, timezone + +from django.db import migrations + +logger = logging.getLogger(__name__) + + +def bootstrap_draft_change_logs(apps, schema_editor): + """ + Create a fake DraftChangeSet that encompasses the state of current Drafts. + """ + LearningPackage = apps.get_model("oel_publishing", "LearningPackage") + PublishableEntityVersion = apps.get_model("oel_publishing", "PublishableEntityVersion") + + Draft = apps.get_model("oel_publishing", "Draft") + DraftChangeLogRecord = apps.get_model("oel_publishing", "DraftChangeLogRecord") + DraftChangeLog = apps.get_model("oel_publishing", "DraftChangeLog") + now = datetime.now(tz=timezone.utc) + + for learning_package in LearningPackage.objects.all().order_by("key"): + logger.info(f"Creating bootstrap DraftChangeLogs for {learning_package.key}") + pub_ent_versions = PublishableEntityVersion.objects.filter( + entity__learning_package=learning_package + ).select_related("entity") + + # First cycle though all the simple create/edit operations... + last_version_seen = {} # PublishableEntity.id -> PublishableEntityVersion.id + for pub_ent_version in pub_ent_versions.order_by("pk"): + draft_change_log = DraftChangeLog.objects.create( + learning_package=learning_package, + changed_at=pub_ent_version.created, + changed_by=pub_ent_version.created_by, + ) + DraftChangeLogRecord.objects.create( + draft_change_log=draft_change_log, + entity=pub_ent_version.entity, + old_version_id=last_version_seen.get(pub_ent_version.entity.id), + new_version_id=pub_ent_version.id, + ) + last_version_seen[pub_ent_version.entity.id] = pub_ent_version.id + + # Now that we've created change sets for create/edit operations, we look + # at the latest state of the Draft model in order to determine whether + # we need to apply deletes or resets. + for draft in Draft.objects.filter(entity__learning_package=learning_package).order_by("entity_id"): + last_version_id = last_version_seen.get(draft.entity_id) + if draft.version_id == last_version_id: + continue + # We don't really know who did this or when, so we use None and now. + draft_change_log = DraftChangeLog.objects.create( + learning_package=learning_package, + changed_at=now, + changed_by=None, + ) + DraftChangeLogRecord.objects.create( + draft_change_log=draft_change_log, + entity_id=draft.entity_id, + old_version_id=last_version_id, + new_version_id=draft.version_id, + ) + + +def delete_draft_change_logs(apps, schema_editor): + logger.info(f"Deleting all DraftChangeLogs (reverse migration)") + DraftChangeLog = apps.get_model("oel_publishing", "DraftChangeLog") + DraftChangeLog.objects.all().delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('oel_publishing', '0006_draftchangelog'), + ] + + operations = [ + migrations.RunPython(bootstrap_draft_change_logs, reverse_code=delete_draft_change_logs) + ] diff --git a/openedx_learning/apps/authoring/publishing/models/__init__.py b/openedx_learning/apps/authoring/publishing/models/__init__.py index 27329076d..7e13c426a 100644 --- a/openedx_learning/apps/authoring/publishing/models/__init__.py +++ b/openedx_learning/apps/authoring/publishing/models/__init__.py @@ -14,10 +14,10 @@ """ from .container import Container, ContainerVersion -from .draft_published import Draft, Published +from .draft_log import Draft, DraftChangeLog, DraftChangeLogRecord, DraftSideEffect from .entity_list import EntityList, EntityListRow from .learning_package import LearningPackage -from .publish_log import PublishLog, PublishLogRecord +from .publish_log import Published, PublishLog, PublishLogRecord from .publishable_entity import ( PublishableContentModelRegistry, PublishableEntity, diff --git a/openedx_learning/apps/authoring/publishing/models/draft_log.py b/openedx_learning/apps/authoring/publishing/models/draft_log.py new file mode 100644 index 000000000..6e85a6ec8 --- /dev/null +++ b/openedx_learning/apps/authoring/publishing/models/draft_log.py @@ -0,0 +1,315 @@ +""" +Models relating to the creation and management of Draft information. +""" +from django.conf import settings +from django.db import models +from django.utils.translation import gettext_lazy as _ + +from openedx_learning.lib.fields import immutable_uuid_field, manual_date_time_field + +from .learning_package import LearningPackage +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. + + .. no_pii + """ + # 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 DraftChangeLog(models.Model): + """ + There is one row in this table for every time Drafts are created/modified. + + There are some operations that affect many Drafts at once, such as + discarding changes (i.e. reset to the published versions) or doing an + import. These would be represented by one DraftChangeLog with many + DraftChangeLogRecords in it--one DraftChangeLogRecord for every + PublishableEntity that was modified. + + Even if we're only directly changing the draft version of one + PublishableEntity, we will get multiple DraftChangeLogRecords if changing + that entity causes side-effects. See the docstrings for DraftChangeLogRecord + and DraftSideEffect for more details. + + .. no_pii: + """ + uuid = immutable_uuid_field() + learning_package = models.ForeignKey(LearningPackage, on_delete=models.CASCADE) + changed_at = manual_date_time_field() + changed_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.SET_NULL, + null=True, + blank=True, + ) + + class Meta: + verbose_name = _("Draft Change Log") + verbose_name_plural = _("Draft Change Logs") + + +class DraftChangeLogRecord(models.Model): + """ + A single change in the PublishableEntity that Draft points to. + + Within a single DraftChangeLog, there can be only one DraftChangeLogRecord + per PublishableEntity. If a PublishableEntity goes from v1 -> v2 and then v2 + -> v3 within the same DraftChangeLog, the expectation is that these will be + collapsed into one DraftChangeLogRecord that goes from v1 -> v3. A single + PublishableEntity may have many DraftChangeLogRecords that describe its full + draft edit history, but each DraftChangeLogRecord will be a part of a + different DraftChangeLog. + + New PublishableEntityVersions are created with a monotonically increasing + version_num for their PublishableEntity. However, knowing that is not enough + to accurately reconstruct how the Draft changes over time because the Draft + does not always point to the most recently created PublishableEntityVersion. + We also have the concept of side-effects, where we consider a + PublishableEntity to have changed in some way, even if no new version is + explicitly created. + + The following scenarios may occur: + + Scenario 1: old_version is None, new_version.version_num = 1 + + This is the common case when we're creating the first version for editing. + + Scenario 2: old_version.version_num + 1 == new_version.version_num + + This is the common case when we've made an edit to something, which + creates the next version of an entity, which we then point the Draft at. + + Scenario 3: old_version.version_num >=1, new_version is None + + This is a soft-deletion. We never actually delete a row from the + PublishableEntity model, but set its current Draft version to be None + instead. + + Scenario 4: old_version.version_num > new_version.version_num + + This can happen if we "discard changes", meaning that we call + reset_drafts_to_published(). The versions we created before resetting + don't get deleted, but the Draft model's pointer to the current version + has been reset to match the Published model. + + Scenario 5: old_version.version_num + 1 < new_version.version_num + + Sometimes we'll have a gap between the two version numbers that is > 1. + This can happen if we make edits (new versions) after we called + reset_drafts_to_published. PublishableEntityVersions are created with a + monotonically incrementing version_num which will continue to go up with + the next edit, regardless of whether Draft is pointing to the most + recently created version or not. In terms of (old_version, new version) + changes, it could look like this: + + - (None, v1): Initial creation + - # Publish happens here, so v1 of this PublishableEntity is published. + - (v1, v2): Normal edit in draft + - (v2, v3): Normal edit in draft + - (v3, v1): Reset to published happened here. + - (v1, v4): Normal edit in draft + + This could also technically happen if we change the same entity more than + once in the the same bulk_draft_changes_for() context, thereby putting + them into the same DraftChangeLog, which forces us to squash the changes + together into one DraftChangeLogRecord. + + Scenario 6: old_version is None, new_version > 1 + + This edge case can happen if we soft-deleted a published entity, and then + called reset_drafts_to_published before we published that soft-deletion. + It would effectively undo our soft-delete because the published version + was not yet marked as deleted. + + Scenario 7: old_version == new_version + + This means that the data associated with the Draft version of an entity + has changed purely as a side-effect of some other entity changing. + + The main example we have of this are containers. Imagine that we have a + Unit that is at v1, and has unpinned references to various Components that + are its children. The Unit's version does not get incremented when the + Components are edited, because the Unit container is defined to always get + the most recent version of those Components. We would only make a new + version of the Unit if we changed the metadata of the Unit itself (e.g. + the title), or if we added, removed, or reordered the children. + + Yet updating a Component intuitively changes what we think of as the + content of the Unit. Users who are working on Units also expect that a + change to a Component will be reflected when looking at a Unit's + "last updated" info. The old_version == new_version convention lets us + represent that in a useful way because that Unit *is* a part of the change + set represented by a DraftChangeLog, even if its own versioned data hasn't + changed. + + .. no_pii: + """ + draft_change_log = models.ForeignKey( + DraftChangeLog, + 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 PublishableEntity can have only one DraftLogRecord per DraftLog. + # You can't simultaneously change the same thing in two different + # ways, e.g. set the Draft to version 1 and version 2 at the same + # time; or delete a Draft and set it to version 2 at the same time. + models.UniqueConstraint( + fields=[ + "draft_change_log", + "entity", + ], + name="oel_dlr_uniq_dcl", + ) + ] + indexes = [ + # Entity (reverse) DraftLog Index: + # * Find the history of draft changes for a given entity, starting + # with the most recent (since IDs are ascending ints). + models.Index( + fields=["entity", "-draft_change_log"], + name="oel_dlr_idx_entity_rdcl", + ), + ] + verbose_name = _("Draft Change Log Record") + verbose_name_plural = _("Draft Change Log Records") + + +class DraftSideEffect(models.Model): + """ + Model to track when a change in one Draft affects other Drafts. + + Our first use case for this is that changes involving child components are + thought to affect parent Units, even if the parent's version doesn't change. + + Side-effects are recorded in a collapsed form that only captures one level. + So if Components C1 and C2 are both changed and they are part of Unit U1, + which is in turn a part of Subsection SS1, then the DraftSideEffect entries + are:: + + (C1, U1) + (C2, U1) + (U1, SS1) + + We do not keep entries for (C1, SS1) or (C2, SS1). This is to make the model + simpler, so we don't have to differentiate between direct side-effects and + transitive side-effects in the model. + + We will record side-effects on a parent container whenever a child changes, + even if the parent container is also changing in the same DraftChangeLog. + The child change is still affecting the parent container, whether the + container happens to be changing for other reasons as well. Whether a parent + -child relationship exists or not depends on the draft state of the + container at the *end* of a bulk_draft_changes_for context. To give concrete + examples: + + Setup: A Unit version U1.v1 has defined C1 to be a child. The current draft + version of C1 is C1.v1. + + Scenario 1: In the a bulk_draft_changes_for context, we edit C1 so that the + draft version of C1 is now C1.v2. Result: + - a DraftChangeLogRecord is created for C1.v1 -> C1.v2 + - a DraftChangeLogRecord is created for U1.v1 -> U1.v1 + - a DraftSideEffect is created with cause (C1.v1 -> C1.v2) and effect + (U1.v1 -> U1.v1). The Unit draft version has not been incremented because + the metadata a Unit defines for itself hasn't been altered, but the Unit + has *changed* in some way because of the side effect of its child being + edited. + + Scenario 2: In a bulk_draft_changes_for context, we edit C1 so that the + draft version of C1 is now C1.v2. In the same context, we edit U1's metadata + so that the draft version of U1 is now U1.v2. U1.v2 still lists C1 as a + child entity. Result: + - a DraftChangeLogRecord is created for C1.v1 -> C1.v2 + - a DraftChangeLogRecord is created for U1.v1 -> U1.v2 + - a DraftSideEffect is created with cause (C1.v1 -> C1.v2) and effect + (U1.v1 -> U1.v2) + + Scenario 3: In a bulk_draft_changes_for context, we edit C1 so that the + draft version of C1 is now C1.v2. In the same context, we edit U1's list of + children so that C1 is no longer a child of U1.v2. Result: + - a DraftChangeLogRecord is created for C1.v1 -> C1.v2 + - a DraftChangeLogRecord is created for U1.v1 -> U1.v2 + - no SideEffect is created, since changing C1 does not have an impact on the + current draft of U1 (U1.v2). A DraftChangeLog is considered a single + atomic operation, so there was never a point at which C1.v1 -> C1.v2 + affected the draft state of U1. + + .. no_pii: + """ + cause = models.ForeignKey( + DraftChangeLogRecord, + on_delete=models.RESTRICT, + related_name='causes', + ) + effect = models.ForeignKey( + DraftChangeLogRecord, + on_delete=models.RESTRICT, + related_name='affected_by', + ) + + class Meta: + constraints = [ + # Duplicate entries for cause & effect are just redundant. This is + # here to guard against weird bugs that might introduce this state. + models.UniqueConstraint( + fields=["cause", "effect"], + name="oel_pub_dse_uniq_c_e", + ) + ] + verbose_name = _("Draft Side Effect") + verbose_name_plural = _("Draft Side Effects") diff --git a/openedx_learning/apps/authoring/publishing/models/draft_published.py b/openedx_learning/apps/authoring/publishing/models/draft_published.py deleted file mode 100644 index c945d807d..000000000 --- a/openedx_learning/apps/authoring/publishing/models/draft_published.py +++ /dev/null @@ -1,95 +0,0 @@ -""" -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/publish_log.py b/openedx_learning/apps/authoring/publishing/models/publish_log.py index e71f08bee..037f1b8b1 100644 --- a/openedx_learning/apps/authoring/publishing/models/publish_log.py +++ b/openedx_learning/apps/authoring/publishing/models/publish_log.py @@ -104,3 +104,47 @@ class Meta: ] 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/tests/openedx_learning/apps/authoring/publishing/test_api.py b/tests/openedx_learning/apps/authoring/publishing/test_api.py index 119a55484..b9e45ec91 100644 --- a/tests/openedx_learning/apps/authoring/publishing/test_api.py +++ b/tests/openedx_learning/apps/authoring/publishing/test_api.py @@ -10,7 +10,16 @@ from django.core.exceptions import ValidationError from openedx_learning.apps.authoring.publishing import api as publishing_api -from openedx_learning.apps.authoring.publishing.models import LearningPackage, PublishableEntity +from openedx_learning.apps.authoring.publishing.models import ( + Container, + ContainerVersion, + Draft, + DraftChangeLog, + DraftChangeLogRecord, + DraftSideEffect, + LearningPackage, + PublishableEntity, +) from openedx_learning.lib.test_utils import TestCase @@ -112,14 +121,20 @@ class DraftTestCase(TestCase): Test basic operations with Drafts. """ now: datetime - learning_package: LearningPackage + learning_package_1: LearningPackage + learning_package_2: LearningPackage @classmethod def setUpTestData(cls) -> None: cls.now = datetime(2024, 1, 28, 16, 45, 30, tzinfo=timezone.utc) - cls.learning_package = publishing_api.create_learning_package( - "my_package_key", - "Draft Testing LearningPackage 🔥", + cls.learning_package_1 = publishing_api.create_learning_package( + "my_package_key_1", + "Draft Testing LearningPackage 🔥 1", + created=cls.now, + ) + cls.learning_package_2 = publishing_api.create_learning_package( + "my_package_key_2", + "Draft Testing LearningPackage 🔥 2", created=cls.now, ) @@ -128,7 +143,7 @@ def test_draft_lifecycle(self) -> None: Test basic lifecycle of a Draft. """ entity = publishing_api.create_publishable_entity( - self.learning_package.id, + self.learning_package_1.id, "my_entity", created=self.now, created_by=None, @@ -152,10 +167,40 @@ def test_draft_lifecycle(self) -> None: deleted_entity_version = publishing_api.get_draft_version(entity.id) assert deleted_entity_version is None + def test_set_draft_args(self) -> None: + """Make sure it works with Draft and int, and raises exception otherwise""" + entity = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "my_set_draft_args_entity", + created=self.now, + created_by=None, + ) + entity_version = publishing_api.create_publishable_entity_version( + entity.id, + version_num=1, + title="An Entity 🌴", + created=self.now, + created_by=None, + ) + + # Int calling version + publishing_api.soft_delete_draft(entity.id) + publishing_api.set_draft_version(entity.draft.pk, entity_version.pk) + assert Draft.objects.get(entity=entity).version == entity_version + + # Draft calling version + publishing_api.soft_delete_draft(entity.id) + publishing_api.set_draft_version(entity.draft, entity_version.pk) + assert Draft.objects.get(entity=entity).version == entity_version + + # Unrecognized type + with pytest.raises(TypeError): + publishing_api.set_draft_version(1.0, entity_version.pk) # type: ignore[arg-type] + def test_soft_deletes(self) -> None: """Test the publishing behavior of soft deletes.""" entity = publishing_api.create_publishable_entity( - self.learning_package.id, + self.learning_package_1.id, "my_entity", created=self.now, created_by=None, @@ -169,7 +214,7 @@ def test_soft_deletes(self) -> None: ) # Initial publish - publish_log = publishing_api.publish_all_drafts(self.learning_package.id) + publish_log = publishing_api.publish_all_drafts(self.learning_package_1.id) log_records = list(publish_log.records.all()) assert len(log_records) == 1 record = log_records[0] @@ -179,7 +224,7 @@ def test_soft_deletes(self) -> None: # Publishing the soft-delete publishing_api.soft_delete_draft(entity.id) - publish_log = publishing_api.publish_all_drafts(self.learning_package.id) + publish_log = publishing_api.publish_all_drafts(self.learning_package_1.id) log_records = list(publish_log.records.all()) assert len(log_records) == 1 record = log_records[0] @@ -192,9 +237,45 @@ def test_soft_deletes(self) -> None: # all the Drafts that have different versions than their Published # counterparts" would mistakenly pull in records that were NULL in both # places. - publish_log = publishing_api.publish_all_drafts(self.learning_package.id) + publish_log = publishing_api.publish_all_drafts(self.learning_package_1.id) assert publish_log.records.count() == 0 + def test_soft_delete_and_reset(self) -> None: + """ + Test edge case where we create, soft-delete, and then reset. + + This is an edge case because creating and then soft-deleting an item + sequence of actions will make both the Draft and Published version NULL. + In this situation reset_drafts_to_published should NOT create a + DraftChangeLog (because they already match, so there's nothing to do). + But we had a bug that redundantly set the Draft version to NULL again + because NULL != NULL in SQL and we were doing the Draft vs. Published + comparison naively without taking that into account. + """ + entity = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "my_entity", + created=self.now, + created_by=None, + ) + # Draft Change #1: create the new version + publishing_api.create_publishable_entity_version( + entity.id, + version_num=1, + title="An Entity 🌴", + created=self.now, + created_by=None, + ) + assert DraftChangeLog.objects.count() == 1 + + # Change #1: delete the draft (set the draft version to None) + publishing_api.soft_delete_draft(entity.id) + assert DraftChangeLog.objects.count() == 2 + + # This should NOT create a change: + publishing_api.reset_drafts_to_published(self.learning_package_1.id) + assert DraftChangeLog.objects.count() == 2 + def test_reset_drafts_to_published(self) -> None: """ Test throwing out Draft data and resetting to the Published versions. @@ -218,7 +299,7 @@ def test_reset_drafts_to_published(self) -> None: # This is the Entity that's going to get a couple of new versions # between Draft and Published. entity_with_changes = publishing_api.create_publishable_entity( - self.learning_package.id, + self.learning_package_1.id, "entity_with_changes", created=self.now, created_by=None, @@ -233,7 +314,7 @@ def test_reset_drafts_to_published(self) -> None: # This Entity is going to remain unchanged between Draft and Published. entity_with_no_changes = publishing_api.create_publishable_entity( - self.learning_package.id, + self.learning_package_1.id, "entity_with_no_changes", created=self.now, created_by=None, @@ -248,7 +329,7 @@ def test_reset_drafts_to_published(self) -> None: # This Entity will be Published, but will then be soft-deleted. entity_with_pending_delete = publishing_api.create_publishable_entity( - self.learning_package.id, + self.learning_package_1.id, "entity_with_pending_delete", created=self.now, created_by=None, @@ -262,7 +343,7 @@ def test_reset_drafts_to_published(self) -> None: ) # Publish! - publishing_api.publish_all_drafts(self.learning_package.id) + publishing_api.publish_all_drafts(self.learning_package_1.id) # Create versions 2, 3, 4 of entity_with_changes. After this, the # Published version is 1 and the Draft version is 4. @@ -282,7 +363,7 @@ def test_reset_drafts_to_published(self) -> None: # Create a new entity that only exists in Draft form (no Published # version). new_entity = publishing_api.create_publishable_entity( - self.learning_package.id, + self.learning_package_1.id, "new_entity", created=self.now, created_by=None, @@ -310,7 +391,7 @@ def test_reset_drafts_to_published(self) -> None: assert draft_version_num == self._get_draft_version_num(entity) # Now reset to draft here! - publishing_api.reset_drafts_to_published(self.learning_package.id) + publishing_api.reset_drafts_to_published(self.learning_package_1.id) # Versions we expect after reset in (entity, published version num) # tuples. The only entity that is not version 1 is the new one. @@ -327,10 +408,16 @@ def test_reset_drafts_to_published(self) -> None: pub_version_num ) + def test_reset_drafts_to_published_bulk(self) -> None: + """bulk_draft_changes_for creates only one DraftChangeLog.""" + with publishing_api.bulk_draft_changes_for(self.learning_package_1.id): + self.test_reset_drafts_to_published() + assert DraftChangeLog.objects.count() == 1 + def test_get_entities_with_unpublished_changes(self) -> None: """Test fetching entities with unpublished changes after soft deletes.""" entity = publishing_api.create_publishable_entity( - self.learning_package.id, + self.learning_package_1.id, "my_entity", created=self.now, created_by=None, @@ -344,26 +431,26 @@ def test_get_entities_with_unpublished_changes(self) -> None: ) # Fetch unpublished entities - entities = publishing_api.get_entities_with_unpublished_changes(self.learning_package.id) + entities = publishing_api.get_entities_with_unpublished_changes(self.learning_package_1.id) records = list(entities.all()) assert len(records) == 1 record = records[0] assert record.id == entity.id # Initial publish - publishing_api.publish_all_drafts(self.learning_package.id) + publishing_api.publish_all_drafts(self.learning_package_1.id) # soft-delete entity publishing_api.soft_delete_draft(entity.id) - entities = publishing_api.get_entities_with_unpublished_changes(self.learning_package.id) + entities = publishing_api.get_entities_with_unpublished_changes(self.learning_package_1.id) assert len(entities) == 0 - entities = publishing_api.get_entities_with_unpublished_changes(self.learning_package.id, + entities = publishing_api.get_entities_with_unpublished_changes(self.learning_package_1.id, include_deleted_drafts=True) assert len(entities) == 1 # publish soft-delete - publishing_api.publish_all_drafts(self.learning_package.id) - entities = publishing_api.get_entities_with_unpublished_changes(self.learning_package.id, + publishing_api.publish_all_drafts(self.learning_package_1.id) + entities = publishing_api.get_entities_with_unpublished_changes(self.learning_package_1.id, include_deleted_drafts=True) # should not return published soft-deleted entities. assert len(entities) == 0 @@ -376,7 +463,7 @@ def test_filter_publishable_entities(self) -> None: for index in range(count_published): # Create entities to publish entity = publishing_api.create_publishable_entity( - self.learning_package.id, + self.learning_package_1.id, f"entity_published_{index}", created=self.now, created_by=None, @@ -390,12 +477,12 @@ def test_filter_publishable_entities(self) -> None: created_by=None, ) - publishing_api.publish_all_drafts(self.learning_package.id) + publishing_api.publish_all_drafts(self.learning_package_1.id) for index in range(count_drafts): # Create entities with drafts entity = publishing_api.create_publishable_entity( - self.learning_package.id, + self.learning_package_1.id, f"entity_draft_{index}", created=self.now, created_by=None, @@ -412,7 +499,7 @@ def test_filter_publishable_entities(self) -> None: for index in range(count_no_drafts): # Create entities without drafts entity = publishing_api.create_publishable_entity( - self.learning_package.id, + self.learning_package_1.id, f"entity_no_draft_{index}", created=self.now, created_by=None, @@ -453,3 +540,538 @@ def _get_draft_version_num(self, entity: PublishableEntity) -> int | None: if draft_version is not None: return draft_version.version_num return None + + +class DraftChangeLogTestCase(TestCase): + """ + Test basic operations with DraftChangeLogs and bulk draft operations. + """ + now: datetime + learning_package_1: LearningPackage + learning_package_2: LearningPackage + + @classmethod + def setUpTestData(cls) -> None: + cls.now = datetime(2024, 1, 28, 16, 45, 30, tzinfo=timezone.utc) + cls.learning_package_1 = publishing_api.create_learning_package( + "my_package_key_1", + "Draft Testing LearningPackage 🔥 1", + created=cls.now, + ) + cls.learning_package_2 = publishing_api.create_learning_package( + "my_package_key_2", + "Draft Testing LearningPackage 🔥 2", + created=cls.now, + ) + + def test_simple_draft_change_log(self) -> None: + """ + Simplest test that multiple writes make it into one DraftChangeLog. + """ + with publishing_api.bulk_draft_changes_for(self.learning_package_1.id): + entity = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "my_entity", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + entity.id, + version_num=1, + title="An Entity 🌴", + created=self.now, + created_by=None, + ) + entity2 = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "my_entity2", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + entity2.id, + version_num=1, + title="An Entity 🌴 2", + created=self.now, + created_by=None, + ) + draft_sets = list(DraftChangeLog.objects.all()) + assert len(draft_sets) == 1 + assert len(draft_sets[0].records.all()) == 2 + + # Now that we're outside of the context manager, check that we're making + # a new DraftChangeLog... + entity3 = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "my_entity3", + created=self.now, + created_by=None, + ) + e3_v1 = publishing_api.create_publishable_entity_version( + entity3.id, + version_num=1, + title="An Entity 🌴 3", + created=self.now, + created_by=None, + ) + draft_sets = list(DraftChangeLog.objects.all().order_by('id')) + assert len(draft_sets) == 2 + assert len(draft_sets[1].records.all()) == 1 + + # Now make one entirely redundant change, and make sure it didn't create + # anything (setting a draft to the same version it already was should be + # a no-op). + publishing_api.set_draft_version(entity3.id, e3_v1.pk) + draft_sets = list(DraftChangeLog.objects.all().order_by('id')) + assert len(draft_sets) == 2 + assert len(draft_sets[1].records.all()) == 1 + + def test_nested_draft_changesets(self) -> None: + """ + We should look up the stack to find the right one for our Learning Package. + """ + with publishing_api.bulk_draft_changes_for(self.learning_package_1.id) as dcl_1: + lp1_e1 = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "lp1_e1", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + lp1_e1.id, + version_num=1, + title="LP1 E1 v1", + created=self.now, + created_by=None, + ) + with publishing_api.bulk_draft_changes_for(self.learning_package_2.id) as dcl_2: + # This should make its way into the *outer* context, because + # we're creating the new publishable entity version for + # learning_package_1, not learning_package_2 + lp1_e1_v2 = publishing_api.create_publishable_entity_version( + lp1_e1.id, + version_num=2, + title="LP1 E1 v1", + created=self.now, + created_by=None, + ) + + # Make sure our change above made it to the outer context and + # didn't make a new one (or go to the inner context). + assert DraftChangeLog.objects.all().count() == 2 + assert DraftChangeLogRecord.objects.all().count() == 1 + lp1_e1_record = DraftChangeLogRecord.objects.first() + assert lp1_e1_record is not None + assert lp1_e1_record.old_version is None + assert lp1_e1_record.new_version == lp1_e1_v2 + assert lp1_e1_record.draft_change_log.learning_package == self.learning_package_1 + + # This will go to the inner context: + lp2_e1 = publishing_api.create_publishable_entity( + self.learning_package_2.id, + "lp2_e1", + created=self.now, + created_by=None, + ) + lp2_e1_v1 = publishing_api.create_publishable_entity_version( + lp2_e1.id, + version_num=1, + title="LP2 E1 v1", + created=self.now, + created_by=None, + ) + # This doesn't error, but it creates a new DraftChangeLog instead of + # re-using dcl_2 + lp2_e1_v2 = publishing_api.create_publishable_entity_version( + lp2_e1.id, + version_num=2, + title="LP2 E1 v2", + created=self.now, + created_by=None, + ) + + # Sanity check that the first/outer DraftChangeLog hasn't changed. + assert dcl_1.records.count() == 1 + + # Check the state of the second/inner DraftChangeLog + assert dcl_2.records.count() == 1 + lp2_e1_record = dcl_2.records.get(entity=lp2_e1) + assert lp2_e1_record.old_version is None + assert lp2_e1_record.new_version == lp2_e1_v1 + + # We should have 3 DraftChangeLogs, because the last change to lp2_e1 + # was done outside of any context for Learning Package 2. Instead of + # using Learning Package 1's context, it should create its own + # DraftChangeLog: + assert DraftChangeLog.objects.count() == 3 + implicit_dcl = DraftChangeLog.objects.order_by('id').last() + assert implicit_dcl is not None + assert implicit_dcl.records.count() == 1 + implicit_lp2_e1_record = implicit_dcl.records.get(entity=lp2_e1) + assert implicit_lp2_e1_record.old_version == lp2_e1_v1 + assert implicit_lp2_e1_record.new_version == lp2_e1_v2 + + def test_multiple_draft_changes(self) -> None: + """ + Test that multiple changes to the same entity are collapsed. + """ + with publishing_api.bulk_draft_changes_for(self.learning_package_1.id): + entity = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "my_entity", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + entity.id, + version_num=1, + title="An Entity 🌴 v1", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + entity.id, + version_num=2, + title="An Entity 🌴 v2", + created=self.now, + created_by=None, + ) + draft_sets = list(DraftChangeLog.objects.all().order_by('id')) + assert len(draft_sets) == 1 + changes = list(draft_sets[0].records.all()) + assert len(changes) == 1 + change = changes[0] + assert change.old_version is None + assert change.new_version is not None + assert change.new_version.version_num == 2 + + def test_some_draft_changes_cancel_out(self) -> None: + """Test that re remove redundant changes from our DraftChangeLog.""" + with publishing_api.bulk_draft_changes_for(self.learning_package_1.id): + # This change will get cancelled out (because we create a draft and + # then delete it), so changes related to entity_1 will be removed + # after the context ends. + entity_1 = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "Entity-1", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + entity_1.id, + version_num=1, + title="An Entity 🌴 v1", + created=self.now, + created_by=None, + ) + publishing_api.soft_delete_draft(entity_1.id) + + # The change to entity_2 will persist + entity_2 = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "Entity-2", + created=self.now, + created_by=None, + ) + e2_v1 = publishing_api.create_publishable_entity_version( + entity_2.id, + version_num=1, + title="E2 title", + created=self.now, + created_by=None, + ) + assert DraftChangeLog.objects.all().count() == 1 + change_log = DraftChangeLog.objects.first() + assert change_log is not None + assert change_log.records.count() == 1 + change = change_log.records.get(entity_id=entity_2.id) + assert change.old_version is None + assert change.new_version == e2_v1 + + def test_multiple_draft_changes_all_cancel_out(self) -> None: + """ + If all changes made cancel out, the entire DraftRecord gets deleted. + """ + # Make sure a version change from (None -> None) gets removed. + with publishing_api.bulk_draft_changes_for(self.learning_package_1.id): + entity = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "my_entity", + created=self.now, + created_by=None, + ) + v1 = publishing_api.create_publishable_entity_version( + entity.id, + version_num=1, + title="An Entity 🌴 v1", + created=self.now, + created_by=None, + ) + publishing_api.soft_delete_draft(entity.id) + + assert not DraftChangeLog.objects.all().exists() + + # This next call implicitly makes a DraftChangeLog + publishing_api.set_draft_version(entity.id, v1.pk) + assert DraftChangeLog.objects.all().count() == 1 + + # Make sure a change from v1 -> v2 -> v1 gets removed. + with publishing_api.bulk_draft_changes_for(self.learning_package_1.id): + for i in range(2, 5): + # Make a few new versions + publishing_api.create_publishable_entity_version( + entity.id, + version_num=i, + title=f"An Entity v{i}", + created=self.now, + created_by=None, + ) + # Reset to version 1 + publishing_api.set_draft_version(entity.id, v1.pk) + + assert DraftChangeLog.objects.all().count() == 1 + + +class ContainerTestCase(TestCase): + """ + Test basic operations with Drafts. + """ + now: datetime + learning_package: LearningPackage + + @classmethod + def setUpTestData(cls) -> None: + cls.now = datetime(2024, 1, 28, 16, 45, 30, tzinfo=timezone.utc) + cls.learning_package = publishing_api.create_learning_package( + "containers_package_key", + "Container Testing LearningPackage 🔥 1", + created=cls.now, + ) + + def test_parent_child_side_effects(self) -> None: + """Test that modifying a child has side-effects on its parent.""" + child_1 = publishing_api.create_publishable_entity( + self.learning_package.id, + "child_1", + created=self.now, + created_by=None, + ) + child_1_v1 = publishing_api.create_publishable_entity_version( + child_1.id, + version_num=1, + title="Child 1 🌴", + created=self.now, + created_by=None, + ) + child_2 = publishing_api.create_publishable_entity( + self.learning_package.id, + "child_2", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + child_2.id, + version_num=1, + title="Child 2 🌴", + created=self.now, + created_by=None, + ) + container: Container = publishing_api.create_container( + self.learning_package.id, + "my_container", + created=self.now, + created_by=None, + ) + container_v1: ContainerVersion = publishing_api.create_container_version( + container.pk, + 1, + title="My Container", + entity_rows=[ + publishing_api.ContainerEntityRow(entity_pk=child_1.pk), + publishing_api.ContainerEntityRow(entity_pk=child_2.pk), + ], + created=self.now, + created_by=None, + ) + + # All this was just set up. Now that we have our container-child + # relationships, altering a child should add the parent container to + # the DraftChangeLog. + child_1_v2 = publishing_api.create_publishable_entity_version( + child_1.id, + version_num=2, + title="Child 1 v2", + created=self.now, + created_by=None, + ) + last_change_log = DraftChangeLog.objects.order_by('-id').first() + assert last_change_log is not None + assert last_change_log.records.count() == 2 + child_1_change = last_change_log.records.get(entity=child_1) + assert child_1_change.old_version == child_1_v1 + assert child_1_change.new_version == child_1_v2 + + # The container should be here, but the versions should be the same for + # before and after: + container_change = last_change_log.records.get( + entity=container.publishable_entity + ) + assert container_change.old_version == container_v1.publishable_entity_version + assert container_change.new_version == container_v1.publishable_entity_version + + # Exactly one side-effect should have been created because we changed + # child_1 after it was part of a container. + side_effects = DraftSideEffect.objects.all() + assert side_effects.count() == 1 + side_effect = side_effects.first() + assert side_effect is not None + assert side_effect.cause == child_1_change + assert side_effect.effect == container_change + + def test_bulk_parent_child_side_effects(self) -> None: + """Test that modifying a child has side-effects on its parent. (bulk version)""" + with publishing_api.bulk_draft_changes_for(self.learning_package.id): + child_1 = publishing_api.create_publishable_entity( + self.learning_package.id, + "child_1", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + child_1.id, + version_num=1, + title="Child 1 🌴", + created=self.now, + created_by=None, + ) + child_2 = publishing_api.create_publishable_entity( + self.learning_package.id, + "child_2", + created=self.now, + created_by=None, + ) + child_2_v1 = publishing_api.create_publishable_entity_version( + child_2.id, + version_num=1, + title="Child 2 🌴", + created=self.now, + created_by=None, + ) + container: Container = publishing_api.create_container( + self.learning_package.id, + "my_container", + created=self.now, + created_by=None, + ) + container_v1: ContainerVersion = publishing_api.create_container_version( + container.pk, + 1, + title="My Container", + entity_rows=[ + publishing_api.ContainerEntityRow(entity_pk=child_1.pk), + publishing_api.ContainerEntityRow(entity_pk=child_2.pk), + ], + created=self.now, + created_by=None, + ) + + # All this was just set up. Now that we have our container-child + # relationships, altering a child should add the parent container to + # the DraftChangeLog. + child_1_v2 = publishing_api.create_publishable_entity_version( + child_1.id, + version_num=2, + title="Child 1 v2", + created=self.now, + created_by=None, + ) + + # Because we're doing it in bulk, there's only one DraftChangeLog entry. + assert DraftChangeLog.objects.count() == 1 + last_change_log = DraftChangeLog.objects.first() + assert last_change_log is not None + # There's only ever one change entry per publishable entity + assert last_change_log.records.count() == 3 + + child_1_change = last_change_log.records.get(entity=child_1) + assert child_1_change.old_version is None + assert child_1_change.new_version == child_1_v2 + + child_2_change = last_change_log.records.get(entity=child_2) + assert child_2_change.old_version is None + assert child_2_change.new_version == child_2_v1 + + container_change = last_change_log.records.get( + entity=container.publishable_entity + ) + assert container_change.old_version is None + assert container_change.new_version == container_v1.publishable_entity_version + + # There are two side effects here, because we grouped our draft edits + # together using bulk_draft_changes_for, so changes to both children + # count towards side-effects on the container. + side_effects = DraftSideEffect.objects.all() + assert side_effects.count() == 2 + caused_by_child_1 = side_effects.get(cause=child_1_change) + caused_by_child_2 = side_effects.get(cause=child_2_change) + assert caused_by_child_1.effect == container_change + assert caused_by_child_2.effect == container_change + + def test_multiple_layers_of_containers(self): + """Test stacking containers three layers deep.""" + # Note that these aren't real "components" and "units". Everything being + # tested is confined to the publishing app, so those concepts shouldn't + # be imported here. They're just named this way to make it more obvious + # what the intended hierarchy is for testing container nesting. + component = publishing_api.create_publishable_entity( + self.learning_package.id, "component_1", created=self.now, created_by=None, + ) + publishing_api.create_publishable_entity_version( + component.id, version_num=1, title="Component 1 🌴", created=self.now, created_by=None, + ) + unit = publishing_api.create_container( + self.learning_package.id, "unit_1", created=self.now, created_by=None, + ) + publishing_api.create_container_version( + unit.pk, + 1, + title="My Unit", + entity_rows=[ + publishing_api.ContainerEntityRow(entity_pk=component.pk), + ], + created=self.now, + created_by=None, + ) + subsection = publishing_api.create_container( + self.learning_package.id, "subsection_1", created=self.now, created_by=None, + ) + publishing_api.create_container_version( + subsection.pk, + 1, + title="My Subsection", + entity_rows=[ + publishing_api.ContainerEntityRow(entity_pk=unit.pk), + ], + created=self.now, + created_by=None, + ) + + # At this point, no side-effects exist yet because we built it from the + # bottom-up using different DraftChangeLogs + assert not DraftSideEffect.objects.all().exists() + + with publishing_api.bulk_draft_changes_for(self.learning_package.id) as change_log: + publishing_api.create_publishable_entity_version( + component.id, version_num=2, title="Component 1v2🌴", created=self.now, created_by=None, + ) + + assert DraftSideEffect.objects.count() == 2 + component_change = change_log.records.get(entity=component) + unit_change = change_log.records.get(entity=unit.publishable_entity) + subsection_change = change_log.records.get(entity=subsection.publishable_entity) + + assert not component_change.affected_by.exists() + assert unit_change.affected_by.count() == 1 + assert unit_change.affected_by.first().cause == component_change + assert subsection_change.affected_by.count() == 1 + assert subsection_change.affected_by.first().cause == unit_change diff --git a/tests/openedx_learning/apps/authoring/units/test_api.py b/tests/openedx_learning/apps/authoring/units/test_api.py index c482f9375..1e7c0ea14 100644 --- a/tests/openedx_learning/apps/authoring/units/test_api.py +++ b/tests/openedx_learning/apps/authoring/units/test_api.py @@ -171,9 +171,9 @@ 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): + with self.assertNumQueries(25): _empty_unit = self.create_unit_with_components([]) - with self.assertNumQueries(26): + with self.assertNumQueries(29): # And try with a non-empty unit: self.create_unit_with_components([self.component_1, self.component_2_v1], key="u2")