diff --git a/openedx_learning/apps/authoring/publishing/api.py b/openedx_learning/apps/authoring/publishing/api.py index 895f091f..d73dfbff 100644 --- a/openedx_learning/apps/authoring/publishing/api.py +++ b/openedx_learning/apps/authoring/publishing/api.py @@ -8,17 +8,13 @@ from dataclasses import dataclass from datetime import datetime, timezone +from typing import TypeVar 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 ( - ContainerMixin, - PublishableContentModelRegistry, - PublishableEntityMixin, - PublishableEntityVersionMixin, -) +from .model_mixins import PublishableContentModelRegistry, PublishableEntityMixin, PublishableEntityVersionMixin from .models import ( Container, ContainerVersion, @@ -33,6 +29,9 @@ PublishLogRecord, ) +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 @@ -66,7 +65,6 @@ "create_container", "create_container_version", "create_next_container_version", - "create_container_and_version", "get_container", "ContainerEntityListEntry", "get_entities_in_draft_container", @@ -581,7 +579,9 @@ def create_container( key: str, created: datetime, created_by: int | None, -) -> Container: + # The types on the following line are correct, but mypy will complain - https://github.com/python/mypy/issues/3737 + container_model: type[ContainerModel] = Container, # type: ignore[assignment] +) -> ContainerModel: """ [ 🛑 UNSTABLE ] Create a new container. @@ -591,15 +591,17 @@ def create_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_model: The subclass of Container to use, if applicable Returns: The newly created container. """ + assert issubclass(container_model, Container) with atomic(): publishable_entity = create_publishable_entity( learning_package_id, key, created, created_by ) - container = Container.objects.create( + container = container_model.objects.create( publishable_entity=publishable_entity, ) return container @@ -635,7 +637,7 @@ def create_entity_list_with_rows( The newly created entity list. """ order_nums = range(len(entity_pks)) - with atomic(): + with atomic(savepoint=False): entity_list = create_entity_list() EntityListRow.objects.bulk_create( [ @@ -662,7 +664,8 @@ def create_container_version( entity_version_pks: list[int | None] | None, created: datetime, created_by: int | None, -) -> ContainerVersion: + container_version_model: type[ContainerVersionModel] = ContainerVersion, # type: ignore[assignment] +) -> ContainerVersionModel: """ [ 🛑 UNSTABLE ] Create a new container version. @@ -675,11 +678,13 @@ def create_container_version( 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_model: The subclass of ContainerVersion to use, if applicable. Returns: The newly created container version. """ - with atomic(): + assert issubclass(container_version_model, ContainerVersion) + with atomic(savepoint=False): container = Container.objects.select_related("publishable_entity").get(pk=container_pk) entity = container.publishable_entity @@ -706,7 +711,7 @@ def create_container_version( created=created, created_by=created_by, ) - container_version = ContainerVersion.objects.create( + container_version = container_version_model.objects.create( publishable_entity_version=publishable_entity_version, container_id=container_pk, entity_list=entity_list, @@ -723,7 +728,8 @@ def create_next_container_version( entity_version_pks: list[int | None] | None, created: datetime, created_by: int | None, -) -> ContainerVersion: + container_version_model: type[ContainerVersionModel] = ContainerVersion, # type: ignore[assignment] +) -> ContainerVersionModel: """ [ 🛑 UNSTABLE ] Create the next version of a container. A new version of the container is created @@ -742,10 +748,12 @@ def create_next_container_version( 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_model: The subclass of ContainerVersion to use, if applicable. Returns: The newly created container version. """ + assert issubclass(container_version_model, ContainerVersion) with atomic(): container = Container.objects.select_related("publishable_entity").get(pk=container_pk) entity = container.publishable_entity @@ -776,7 +784,7 @@ def create_next_container_version( created=created, created_by=created_by, ) - next_container_version = ContainerVersion.objects.create( + next_container_version = container_version_model.objects.create( publishable_entity_version=publishable_entity_version, container_id=container_pk, entity_list=next_entity_list, @@ -785,46 +793,6 @@ def create_next_container_version( return next_container_version -def create_container_and_version( - learning_package_id: int, - key: str, - *, - created: datetime, - created_by: int | None, - title: str, - publishable_entities_pks: list[int], - entity_version_pks: list[int | None], -) -> tuple[Container, ContainerVersion]: - """ - [ 🛑 UNSTABLE ] - Create a new container and its first version. - - 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. - version_num: The version number of the container. - title: The title of the container. - members_pk: The IDs of the members of the container. - - Returns: - The newly created container version. - """ - with atomic(): - container = create_container(learning_package_id, key, created, created_by) - container_version = create_container_version( - container.publishable_entity.pk, - 1, - title=title, - publishable_entities_pks=publishable_entities_pks, - entity_version_pks=entity_version_pks, - created=created, - created_by=created_by, - ) - return (container, container_version) - - def get_container(pk: int) -> Container: """ [ 🛑 UNSTABLE ] @@ -854,15 +822,13 @@ def entity(self): def get_entities_in_draft_container( - container: Container | ContainerMixin, + container: Container, ) -> list[ContainerEntityListEntry]: """ [ 🛑 UNSTABLE ] Get the list of entities and their versions in the draft version of the given container. """ - if isinstance(container, ContainerMixin): - container = container.container assert isinstance(container, Container) entity_list = [] for row in container.versioning.draft.entity_list.entitylistrow_set.order_by("order_num"): @@ -877,19 +843,15 @@ def get_entities_in_draft_container( def get_entities_in_published_container( - container: Container | ContainerMixin, + container: Container, ) -> list[ContainerEntityListEntry] | None: """ [ 🛑 UNSTABLE ] Get the list of entities and their versions in the published version of the given container. """ - if isinstance(container, ContainerMixin): - cv = container.container.versioning.published - elif isinstance(container, Container): - cv = container.versioning.published - else: - raise TypeError(f"Expected Container or ContainerMixin; got {type(container)}") + assert isinstance(container, Container) + cv = container.versioning.published if cv is None: return None # There is no published version of this container. Should this be an exception? assert isinstance(cv, ContainerVersion) @@ -905,9 +867,7 @@ def get_entities_in_published_container( return entity_list -def contains_unpublished_changes( - container: Container | ContainerMixin, -) -> bool: +def contains_unpublished_changes(container_id: int) -> bool: """ [ 🛑 UNSTABLE ] Check recursively if a container has any unpublished changes. @@ -920,14 +880,10 @@ def contains_unpublished_changes( that's in the container, it will be `False`. This method will return `True` in either case. """ - if isinstance(container, ContainerMixin): - # 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.container_id) - else: - pass # TODO: select_related if we're given a raw Container rather than a ContainerMixin like Unit? - assert isinstance(container, Container) + # 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 @@ -949,7 +905,7 @@ def contains_unpublished_changes( child_container = None if child_container: # This is itself a container - check recursively: - if contains_unpublished_changes(child_container): + if contains_unpublished_changes(child_container.pk): return True else: # This is not a container: diff --git a/openedx_learning/apps/authoring/publishing/migrations/0003_containers.py b/openedx_learning/apps/authoring/publishing/migrations/0003_containers.py index 7260e2a0..1f759144 100644 --- a/openedx_learning/apps/authoring/publishing/migrations/0003_containers.py +++ b/openedx_learning/apps/authoring/publishing/migrations/0003_containers.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.19 on 2025-03-07 23:09 +# Generated by Django 4.2.19 on 2025-03-11 04:10 import django.db.models.deletion from django.db import migrations, models diff --git a/openedx_learning/apps/authoring/publishing/model_mixins/__init__.py b/openedx_learning/apps/authoring/publishing/model_mixins/__init__.py index 47f90297..14ea9315 100644 --- a/openedx_learning/apps/authoring/publishing/model_mixins/__init__.py +++ b/openedx_learning/apps/authoring/publishing/model_mixins/__init__.py @@ -1,5 +1,4 @@ """ Mixins provided by the publishing app """ -from .container import * from .publishable_entity import * diff --git a/openedx_learning/apps/authoring/publishing/model_mixins/container.py b/openedx_learning/apps/authoring/publishing/model_mixins/container.py deleted file mode 100644 index 944d49ac..00000000 --- a/openedx_learning/apps/authoring/publishing/model_mixins/container.py +++ /dev/null @@ -1,98 +0,0 @@ -""" -ContainerMixin and ContainerVersionMixin -""" -from __future__ import annotations - -from datetime import datetime -from typing import TYPE_CHECKING, ClassVar, Self - -from django.db import models - -from openedx_learning.lib.managers import WithRelationsManager - -from .publishable_entity import PublishableEntityMixin, PublishableEntityVersionMixin - -if TYPE_CHECKING: - from ..models.container import Container, ContainerVersion -else: - # To avoid circular imports, we need to reference these models using strings only - Container = "oel_publishing.Container" - ContainerVersion = "oel_publishing.ContainerVersion" - -__all__ = [ - "ContainerMixin", - "ContainerVersionMixin", -] - - -class ContainerMixin(PublishableEntityMixin): - """ - Convenience mixin to link your models against Container. - - Please see docstring for Container for more details. - - If you use this class, you *MUST* also use ContainerVersionMixin - """ - - # select these related entities by default for all queries - objects: ClassVar[WithRelationsManager[Self]] = WithRelationsManager( # type: ignore[assignment] - "container", - "publishable_entity", - "publishable_entity__published", - "publishable_entity__draft", - ) - - container = models.OneToOneField( - Container, - on_delete=models.CASCADE, - ) - - @property - def uuid(self) -> str: - return self.container.uuid - - @property - def created(self) -> datetime: - return self.container.created - - class Meta: - abstract = True - - -class ContainerVersionMixin(PublishableEntityVersionMixin): - """ - Convenience mixin to link your models against ContainerVersion. - - Please see docstring for ContainerVersion for more details. - - If you use this class, you *MUST* also use ContainerMixin - """ - - # select these related entities by default for all queries - objects: ClassVar[WithRelationsManager[Self]] = WithRelationsManager( # type: ignore[assignment] - "container_version", - ) - - container_version = models.OneToOneField( - ContainerVersion, - on_delete=models.CASCADE, - ) - - @property - def uuid(self) -> str: - return self.container_version.uuid - - @property - def title(self) -> str: - return self.container_version.title - - @property - def created(self) -> datetime: - return self.container_version.created - - @property - def version_num(self): - return self.container_version.version_num - - class Meta: - abstract = True diff --git a/openedx_learning/apps/authoring/publishing/model_mixins/publishable_entity.py b/openedx_learning/apps/authoring/publishing/model_mixins/publishable_entity.py index 49aaac93..84360310 100644 --- a/openedx_learning/apps/authoring/publishing/model_mixins/publishable_entity.py +++ b/openedx_learning/apps/authoring/publishing/model_mixins/publishable_entity.py @@ -135,11 +135,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 @@ -149,7 +162,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): diff --git a/openedx_learning/apps/authoring/publishing/models/container.py b/openedx_learning/apps/authoring/publishing/models/container.py index 4a0df08a..04152f71 100644 --- a/openedx_learning/apps/authoring/publishing/models/container.py +++ b/openedx_learning/apps/authoring/publishing/models/container.py @@ -1,6 +1,7 @@ """ Container and ContainerVersion models """ +from django.core.exceptions import ValidationError from django.db import models from ..model_mixins.publishable_entity import PublishableEntityMixin, PublishableEntityVersionMixin @@ -58,3 +59,12 @@ class ContainerVersion(PublishableEntityVersionMixin): 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/units/api.py b/openedx_learning/apps/authoring/units/api.py index e18e6b50..d007a57f 100644 --- a/openedx_learning/apps/authoring/units/api.py +++ b/openedx_learning/apps/authoring/units/api.py @@ -41,15 +41,13 @@ def create_unit( created: The creation date. created_by: The user who created the unit. """ - with atomic(): - container = publishing_api.create_container( - learning_package_id, key, created, created_by - ) - unit = Unit.objects.create( - container=container, - publishable_entity=container.publishable_entity, - ) - return unit + return publishing_api.create_container( + learning_package_id, + key, + created, + created_by, + container_model=Unit, + ) def create_unit_version( @@ -78,22 +76,16 @@ def create_unit_version( created: The creation date. created_by: The user who created the unit. """ - with atomic(): - container_version = publishing_api.create_container_version( - unit.container.pk, - version_num, - title=title, - publishable_entities_pks=publishable_entities_pks, - entity_version_pks=entity_version_pks, - created=created, - created_by=created_by, - ) - unit_version = UnitVersion.objects.create( - unit=unit, - container_version=container_version, - publishable_entity_version=container_version.publishable_entity_version, - ) - return unit_version + 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_model=UnitVersion, + ) def _pub_entities_for_components( @@ -144,20 +136,15 @@ def create_next_unit_version( created_by: The user who created the unit. """ publishable_entities_pks, entity_version_pks = _pub_entities_for_components(components) - with atomic(): - container_version = publishing_api.create_next_container_version( - unit.container.pk, - title=title, - publishable_entities_pks=publishable_entities_pks, - entity_version_pks=entity_version_pks, - created=created, - created_by=created_by, - ) - unit_version = UnitVersion.objects.create( - unit=unit, - container_version=container_version, - publishable_entity_version=container_version.publishable_entity_version, - ) + 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_model=UnitVersion, + ) return unit_version @@ -301,10 +288,10 @@ def get_components_in_published_unit_as_of( 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. - unit_version = unit_pub_entity_version.unitversion # type: ignore[attr-defined] + container_version = unit_pub_entity_version.containerversion entity_list = [] - rows = unit_version.container_version.entity_list.entitylistrow_set.order_by("order_num") + 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 diff --git a/openedx_learning/apps/authoring/units/migrations/0001_initial.py b/openedx_learning/apps/authoring/units/migrations/0001_initial.py index 3bfc8a40..52a5b4fb 100644 --- a/openedx_learning/apps/authoring/units/migrations/0001_initial.py +++ b/openedx_learning/apps/authoring/units/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.19 on 2025-03-07 23:10 +# Generated by Django 4.2.19 on 2025-03-11 04:31 import django.db.models.deletion from django.db import migrations, models @@ -16,22 +16,21 @@ class Migration(migrations.Migration): migrations.CreateModel( name='Unit', fields=[ - ('publishable_entity', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, serialize=False, to='oel_publishing.publishableentity')), - ('container', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='oel_publishing.container')), + ('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=[ - ('publishable_entity_version', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, serialize=False, to='oel_publishing.publishableentityversion')), - ('container_version', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='oel_publishing.containerversion')), - ('unit', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='versions', to='oel_units.unit')), + ('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/models.py b/openedx_learning/apps/authoring/units/models.py index 3a12e035..0c525584 100644 --- a/openedx_learning/apps/authoring/units/models.py +++ b/openedx_learning/apps/authoring/units/models.py @@ -3,7 +3,7 @@ """ from django.db import models -from ..publishing.model_mixins import ContainerMixin, ContainerVersionMixin +from ..publishing.models import Container, ContainerVersion __all__ = [ "Unit", @@ -11,21 +11,40 @@ ] -class Unit(ContainerMixin): +class Unit(Container): """ - A Unit is Container, which is a PublishableEntity. + 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(ContainerVersionMixin): - """ - A UnitVersion is a ContainerVersion, which is a PublishableEntityVersion. +class UnitVersion(ContainerVersion): """ + A UnitVersion is a specific version of a Unit. - # Not sure what other metadata goes here, but we want to try to separate things - # like scheduling information and such into different models. - unit = models.ForeignKey( - 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, - related_name="versions", + 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/tests/openedx_learning/apps/authoring/units/test_api.py b/tests/openedx_learning/apps/authoring/units/test_api.py index c063876d..52830206 100644 --- a/tests/openedx_learning/apps/authoring/units/test_api.py +++ b/tests/openedx_learning/apps/authoring/units/test_api.py @@ -83,34 +83,48 @@ def test_get_unit(self): Test get_unit() """ unit = self.create_unit_with_components([self.component_1, self.component_2]) - result = authoring_api.get_unit(unit.pk) + 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 - # TODO: (maybe) This currently has extra queries and is not preloaded even though it's the same: - # with self.assertNumQueries(0): - # assert result.container.versioning.has_unpublished_changes def test_get_container(self): """ Test get_container() """ unit = self.create_unit_with_components([self.component_1, self.component_2]) - result = authoring_api.get_container(unit.container_id) + 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(28): + with self.assertNumQueries(22): _empty_unit = self.create_unit_with_components([]) - with self.assertNumQueries(31): + 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") @@ -127,6 +141,7 @@ def test_create_unit_with_invalid_children(self): 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", @@ -144,6 +159,8 @@ def test_create_unit_with_invalid_children(self): 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): @@ -272,7 +289,7 @@ def test_auto_publish_children(self): # 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) + assert authoring_api.contains_unpublished_changes(unit.pk) assert self.component_1.versioning.published is None assert self.component_2.versioning.published is None @@ -286,7 +303,7 @@ def test_auto_publish_children(self): 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) is False # Deep check + 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: @@ -331,7 +348,7 @@ def test_add_component_after_publish(self): 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) is False # Deeper check + 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 @@ -345,7 +362,7 @@ def test_add_component_after_publish(self): # 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) # Deeper check + assert authoring_api.contains_unpublished_changes(unit.pk) # Deeper check assert unit.versioning.draft == unit_version_v2 assert unit.versioning.published == unit_version @@ -366,7 +383,7 @@ def test_modify_unpinned_component_after_publish(self): 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) is False # Deeper 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): @@ -376,7 +393,7 @@ def test_modify_unpinned_component_after_publish(self): 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) # But unit DOES contain changes + 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 @@ -395,7 +412,7 @@ def test_modify_unpinned_component_after_publish(self): assert authoring_api.get_components_in_published_unit(unit) == [ Entry(component_1_v2), # new version ] - assert authoring_api.contains_unpublished_changes(unit) is False # No longer contains unpublished changes + assert authoring_api.contains_unpublished_changes(unit.pk) is False # No longer contains unpublished changes def test_modify_pinned_component(self): """ @@ -420,7 +437,7 @@ def test_modify_pinned_component(self): 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) is False # Deep 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 @@ -496,14 +513,14 @@ def test_publishing_shared_component(self): 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) is False - assert authoring_api.contains_unpublished_changes(unit2) is False + 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) - assert authoring_api.contains_unpublished_changes(unit2) + 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() @@ -539,8 +556,8 @@ def test_publishing_shared_component(self): ] # 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) is False - assert authoring_api.contains_unpublished_changes(unit2) + 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) @@ -550,7 +567,7 @@ def test_publishing_shared_component(self): 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) is False + assert authoring_api.contains_unpublished_changes(unit2.pk) is False def test_query_count_of_contains_unpublished_changes(self): """ @@ -570,12 +587,12 @@ def test_query_count_of_contains_unpublished_changes(self): authoring_api.publish_all_drafts(self.learning_package.id) unit.refresh_from_db() with self.assertNumQueries(2): - assert authoring_api.contains_unpublished_changes(unit) is False + 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) is True + assert authoring_api.contains_unpublished_changes(unit.pk) is True def test_metadata_change_doesnt_create_entity_list(self): """ @@ -585,14 +602,14 @@ def test_metadata_change_doesnt_create_entity_list(self): """ unit = self.create_unit_with_components([self.component_1, self.component_2_v1]) - orig_version_num = unit.container.versioning.draft.version_num - orig_entity_list_id = unit.container.versioning.draft.entity_list.pk + 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.container.versioning.draft.version_num - new_entity_list_id = unit.container.versioning.draft.entity_list.pk + 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 @@ -636,7 +653,7 @@ def test_removing_component(self): ] 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) + assert authoring_api.contains_unpublished_changes(unit.pk) # The published version of the unit is not yet affected: assert authoring_api.get_components_in_published_unit(unit) == [ Entry(self.component_1_v1), @@ -650,7 +667,7 @@ def test_removing_component(self): # 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) is False + assert authoring_api.contains_unpublished_changes(unit.pk) is False assert authoring_api.get_components_in_published_unit(unit) == [ Entry(self.component_1_v1), ] @@ -672,7 +689,7 @@ def test_soft_deleting_component(self): # 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) # But it CONTAINS an unpublished change (a deletion) + 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_published_unit(unit) == [ Entry(self.component_1_v1), @@ -681,7 +698,7 @@ def test_soft_deleting_component(self): # 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) is False + assert authoring_api.contains_unpublished_changes(unit.pk) is False assert authoring_api.get_components_in_published_unit(unit) == [ Entry(self.component_1_v1), ] @@ -706,7 +723,7 @@ def test_soft_deleting_and_removing_component(self): Entry(self.component_1_v1), ] assert unit.versioning.has_unpublished_changes is True - assert authoring_api.contains_unpublished_changes(unit) + assert authoring_api.contains_unpublished_changes(unit.pk) # The published version of the unit is not yet affected: assert authoring_api.get_components_in_published_unit(unit) == [ Entry(self.component_1_v1), @@ -715,7 +732,7 @@ def test_soft_deleting_and_removing_component(self): # 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) is False + assert authoring_api.contains_unpublished_changes(unit.pk) is False assert authoring_api.get_components_in_published_unit(unit) == [ Entry(self.component_1_v1), ] @@ -734,7 +751,7 @@ def test_soft_deleting_pinned_component(self): 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) is False # nor does it contain changes + 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_published_unit(unit) == [ Entry(self.component_1_v1, pinned=True),