From 352c08c58352c4d31de5ff7d684efbbbc247467f Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Tue, 30 Jul 2024 06:00:12 +0930 Subject: [PATCH 1/6] feat: get_last_publish log for PublishableEntity --- .../apps/authoring/publishing/api.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/openedx_learning/apps/authoring/publishing/api.py b/openedx_learning/apps/authoring/publishing/api.py index 2d5233770..6084239bb 100644 --- a/openedx_learning/apps/authoring/publishing/api.py +++ b/openedx_learning/apps/authoring/publishing/api.py @@ -202,7 +202,21 @@ def get_publishable_entity_by_key(learning_package_id, /, key) -> PublishableEnt ) -def get_last_publish(learning_package_id: int, /) -> PublishLog | None: +def get_last_publish(learning_package_id: int, /, key=None) -> PublishLog | None: + """ + Return the most recent PublishLog for the given LearningPackage + optional PublishableEntity. + + Returns None if no related PublishLog (or PublishLogRecord) is found. + """ + if key: + publish_log_record = PublishLogRecord.objects \ + .filter(publish_log__learning_package_id=learning_package_id) \ + .filter(entity__key=key) \ + .order_by('-publish_log__id') \ + .select_related('publish_log') \ + .first() + return publish_log_record.publish_log if publish_log_record else None + return PublishLog.objects \ .filter(learning_package_id=learning_package_id) \ .order_by('-id') \ From b0da420f0cbc95db40443a2a7851521a8216d4ee Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Tue, 30 Jul 2024 06:40:06 +0930 Subject: [PATCH 2/6] test: last_publish_log for LearningPackage and PublishableEntity Adds freezegun to the test requirements so we can check published_at dates. --- requirements/dev.txt | 3 + requirements/doc.txt | 3 + requirements/quality.txt | 3 + requirements/test.in | 1 + requirements/test.txt | 3 + .../apps/authoring/publishing/test_api.py | 108 ++++++++++++++++++ 6 files changed, 121 insertions(+) diff --git a/requirements/dev.txt b/requirements/dev.txt index db1656411..59ae351a5 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -178,6 +178,8 @@ filelock==3.15.4 # -r requirements/ci.txt # tox # virtualenv +freezegun==1.5.1 + # via -r requirements/quality.txt grimp==3.4.1 # via # -r requirements/ci.txt @@ -391,6 +393,7 @@ python-dateutil==2.9.0.post0 # via # -r requirements/quality.txt # celery + # freezegun python-slugify==8.0.4 # via # -r requirements/quality.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index c2614e169..993591bbd 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -145,6 +145,8 @@ edx-opaque-keys==2.10.0 # via # -r requirements/test.txt # edx-drf-extensions +freezegun==1.5.1 + # via -r requirements/test.txt grimp==3.4.1 # via # -r requirements/test.txt @@ -253,6 +255,7 @@ python-dateutil==2.9.0.post0 # via # -r requirements/test.txt # celery + # freezegun python-slugify==8.0.4 # via # -r requirements/test.txt diff --git a/requirements/quality.txt b/requirements/quality.txt index 4dabc758f..9bad017d7 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -144,6 +144,8 @@ edx-opaque-keys==2.10.0 # via # -r requirements/test.txt # edx-drf-extensions +freezegun==1.5.1 + # via -r requirements/test.txt grimp==3.4.1 # via # -r requirements/test.txt @@ -292,6 +294,7 @@ python-dateutil==2.9.0.post0 # via # -r requirements/test.txt # celery + # freezegun python-slugify==8.0.4 # via # -r requirements/test.txt diff --git a/requirements/test.in b/requirements/test.in index 0d3126aef..164affcf3 100644 --- a/requirements/test.in +++ b/requirements/test.in @@ -18,3 +18,4 @@ mypy # static type checking django-stubs # Typing stubs for Django, so it works with mypy djangorestframework-stubs # Typing stubs for DRF django-debug-toolbar # provides a debug toolbar for Django +freezegun # Allows tests to mock the output of assorted datetime module functions diff --git a/requirements/test.txt b/requirements/test.txt index 66f222d42..89ed3eb8d 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -122,6 +122,8 @@ edx-opaque-keys==2.10.0 # via # -r requirements/base.txt # edx-drf-extensions +freezegun==1.5.1 + # via -r requirements/test.in grimp==3.4.1 # via import-linter idna==3.7 @@ -198,6 +200,7 @@ python-dateutil==2.9.0.post0 # via # -r requirements/base.txt # celery + # freezegun python-slugify==8.0.4 # via code-annotations pyyaml==6.0.1 diff --git a/tests/openedx_learning/apps/authoring/publishing/test_api.py b/tests/openedx_learning/apps/authoring/publishing/test_api.py index 03d9ebf06..985197c24 100644 --- a/tests/openedx_learning/apps/authoring/publishing/test_api.py +++ b/tests/openedx_learning/apps/authoring/publishing/test_api.py @@ -8,6 +8,7 @@ import pytest from django.core.exceptions import ValidationError +from freezegun import freeze_time from openedx_learning.apps.authoring.publishing import api as publishing_api from openedx_learning.apps.authoring.publishing.models import LearningPackage, PublishableEntity @@ -284,6 +285,113 @@ def test_reset_drafts_to_published(self) -> None: pub_version_num ) + def test_last_publish(self): + """ + Test that PublishLog records are created when publishing a LearningPackage and its PublishableEntities. + """ + # This Entity will get a couple of Published versions + entity_with_changes = publishing_api.create_publishable_entity( + self.learning_package.id, + "entity_with_changes", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + entity_with_changes.id, + version_num=1, + title="I'm entity_with_changes v1", + created=self.now, + created_by=None, + ) + + # This Entity will only be Published once. + entity_with_no_changes = publishing_api.create_publishable_entity( + self.learning_package.id, + "entity_with_no_changes", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + entity_with_no_changes.id, + version_num=1, + title="I'm entity_with_no_changes v1", + created=self.now, + created_by=None, + ) + + # Publish first time. + published_first_time = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc) + with freeze_time(published_first_time): + publishing_api.publish_all_drafts(self.learning_package.id) + + # Fetch the most recent PublishLog for the LearningPackage and its PublishableEntities. + # Each call should only require a single query. + with self.assertNumQueries(1): + first_publish_log_for_library = publishing_api.get_last_publish( + self.learning_package.id, + ) + with self.assertNumQueries(1): + first_publish_log_for_entity_with_changes = publishing_api.get_last_publish( + self.learning_package.id, + key=entity_with_changes.key, + ) + with self.assertNumQueries(1): + first_publish_log_for_entity_with_no_changes = publishing_api.get_last_publish( + self.learning_package.id, + key=entity_with_no_changes.key, + ) + + # PublishLog for library + both entities should match each other + assert published_first_time == first_publish_log_for_library.published_at + assert ( + first_publish_log_for_library == + first_publish_log_for_entity_with_changes == + first_publish_log_for_entity_with_no_changes + ) + + # Modify entity_with_changes + publishing_api.create_publishable_entity_version( + entity_with_changes.id, + version_num=2, + title="I'm entity_with_changes v2", + created=self.now, + created_by=None, + ) + + # Publish second time + published_second_time = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc) + with freeze_time(published_second_time): + publishing_api.publish_all_drafts(self.learning_package.id) + + # Re-fetch the most recent PublishLog for the LearningPackage and its PublishableEntities. + # Each call should only require a single query. + with self.assertNumQueries(1): + next_publish_log_for_library = publishing_api.get_last_publish( + self.learning_package.id, + ) + with self.assertNumQueries(1): + next_publish_log_for_entity_with_changes = publishing_api.get_last_publish( + self.learning_package.id, + key=entity_with_changes.key, + ) + with self.assertNumQueries(1): + next_publish_log_for_entity_with_no_changes = publishing_api.get_last_publish( + self.learning_package.id, + key=entity_with_no_changes.key, + ) + + # PublishLog for library and and entity_with_changes should match each other + assert next_publish_log_for_library.published_at == published_second_time + assert ( + next_publish_log_for_library == + next_publish_log_for_entity_with_changes + ) + # But the entity_with_no_changes should still be on the original publish log + assert ( + first_publish_log_for_entity_with_no_changes == + next_publish_log_for_entity_with_no_changes + ) + def _get_published_version_num(self, entity: PublishableEntity) -> int | None: published_version = publishing_api.get_published_version(entity.id) if published_version is not None: From 6dcbc653b0f6efae36e1e6188d11c08c40646f24 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 31 Jul 2024 04:39:14 +0930 Subject: [PATCH 3/6] revert: previous changes --- .../apps/authoring/publishing/api.py | 16 +-- .../apps/authoring/publishing/test_api.py | 108 ------------------ 2 files changed, 1 insertion(+), 123 deletions(-) diff --git a/openedx_learning/apps/authoring/publishing/api.py b/openedx_learning/apps/authoring/publishing/api.py index 6084239bb..2d5233770 100644 --- a/openedx_learning/apps/authoring/publishing/api.py +++ b/openedx_learning/apps/authoring/publishing/api.py @@ -202,21 +202,7 @@ def get_publishable_entity_by_key(learning_package_id, /, key) -> PublishableEnt ) -def get_last_publish(learning_package_id: int, /, key=None) -> PublishLog | None: - """ - Return the most recent PublishLog for the given LearningPackage + optional PublishableEntity. - - Returns None if no related PublishLog (or PublishLogRecord) is found. - """ - if key: - publish_log_record = PublishLogRecord.objects \ - .filter(publish_log__learning_package_id=learning_package_id) \ - .filter(entity__key=key) \ - .order_by('-publish_log__id') \ - .select_related('publish_log') \ - .first() - return publish_log_record.publish_log if publish_log_record else None - +def get_last_publish(learning_package_id: int, /) -> PublishLog | None: return PublishLog.objects \ .filter(learning_package_id=learning_package_id) \ .order_by('-id') \ diff --git a/tests/openedx_learning/apps/authoring/publishing/test_api.py b/tests/openedx_learning/apps/authoring/publishing/test_api.py index 985197c24..03d9ebf06 100644 --- a/tests/openedx_learning/apps/authoring/publishing/test_api.py +++ b/tests/openedx_learning/apps/authoring/publishing/test_api.py @@ -8,7 +8,6 @@ import pytest from django.core.exceptions import ValidationError -from freezegun import freeze_time from openedx_learning.apps.authoring.publishing import api as publishing_api from openedx_learning.apps.authoring.publishing.models import LearningPackage, PublishableEntity @@ -285,113 +284,6 @@ def test_reset_drafts_to_published(self) -> None: pub_version_num ) - def test_last_publish(self): - """ - Test that PublishLog records are created when publishing a LearningPackage and its PublishableEntities. - """ - # This Entity will get a couple of Published versions - entity_with_changes = publishing_api.create_publishable_entity( - self.learning_package.id, - "entity_with_changes", - created=self.now, - created_by=None, - ) - publishing_api.create_publishable_entity_version( - entity_with_changes.id, - version_num=1, - title="I'm entity_with_changes v1", - created=self.now, - created_by=None, - ) - - # This Entity will only be Published once. - entity_with_no_changes = publishing_api.create_publishable_entity( - self.learning_package.id, - "entity_with_no_changes", - created=self.now, - created_by=None, - ) - publishing_api.create_publishable_entity_version( - entity_with_no_changes.id, - version_num=1, - title="I'm entity_with_no_changes v1", - created=self.now, - created_by=None, - ) - - # Publish first time. - published_first_time = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc) - with freeze_time(published_first_time): - publishing_api.publish_all_drafts(self.learning_package.id) - - # Fetch the most recent PublishLog for the LearningPackage and its PublishableEntities. - # Each call should only require a single query. - with self.assertNumQueries(1): - first_publish_log_for_library = publishing_api.get_last_publish( - self.learning_package.id, - ) - with self.assertNumQueries(1): - first_publish_log_for_entity_with_changes = publishing_api.get_last_publish( - self.learning_package.id, - key=entity_with_changes.key, - ) - with self.assertNumQueries(1): - first_publish_log_for_entity_with_no_changes = publishing_api.get_last_publish( - self.learning_package.id, - key=entity_with_no_changes.key, - ) - - # PublishLog for library + both entities should match each other - assert published_first_time == first_publish_log_for_library.published_at - assert ( - first_publish_log_for_library == - first_publish_log_for_entity_with_changes == - first_publish_log_for_entity_with_no_changes - ) - - # Modify entity_with_changes - publishing_api.create_publishable_entity_version( - entity_with_changes.id, - version_num=2, - title="I'm entity_with_changes v2", - created=self.now, - created_by=None, - ) - - # Publish second time - published_second_time = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc) - with freeze_time(published_second_time): - publishing_api.publish_all_drafts(self.learning_package.id) - - # Re-fetch the most recent PublishLog for the LearningPackage and its PublishableEntities. - # Each call should only require a single query. - with self.assertNumQueries(1): - next_publish_log_for_library = publishing_api.get_last_publish( - self.learning_package.id, - ) - with self.assertNumQueries(1): - next_publish_log_for_entity_with_changes = publishing_api.get_last_publish( - self.learning_package.id, - key=entity_with_changes.key, - ) - with self.assertNumQueries(1): - next_publish_log_for_entity_with_no_changes = publishing_api.get_last_publish( - self.learning_package.id, - key=entity_with_no_changes.key, - ) - - # PublishLog for library and and entity_with_changes should match each other - assert next_publish_log_for_library.published_at == published_second_time - assert ( - next_publish_log_for_library == - next_publish_log_for_entity_with_changes - ) - # But the entity_with_no_changes should still be on the original publish log - assert ( - first_publish_log_for_entity_with_no_changes == - next_publish_log_for_entity_with_no_changes - ) - def _get_published_version_num(self, entity: PublishableEntity) -> int | None: published_version = publishing_api.get_published_version(entity.id) if published_version is not None: From f969a138ba4edb10cb7f21cf843de1b742ec0c02 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 31 Jul 2024 04:40:23 +0930 Subject: [PATCH 4/6] feat: adds VersioningHelper.last_publish_log property and test --- .../apps/authoring/publishing/model_mixins.py | 12 +++ .../apps/authoring/components/test_models.py | 86 ++++++++++++++++++- 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/openedx_learning/apps/authoring/publishing/model_mixins.py b/openedx_learning/apps/authoring/publishing/model_mixins.py index ab5672f1a..1f0210daf 100644 --- a/openedx_learning/apps/authoring/publishing/model_mixins.py +++ b/openedx_learning/apps/authoring/publishing/model_mixins.py @@ -243,6 +243,18 @@ def has_unpublished_changes(self): return draft_version_id != published_version_id + @property + def last_publish_log(self): + """ + Return the most recent PublishLog for this component. + + Return None if the component is not published. + """ + pub_entity = self.content_obj.publishable_entity + if hasattr(pub_entity, 'published'): + return pub_entity.published.publish_log_record.publish_log + return None + @property def versions(self): """ diff --git a/tests/openedx_learning/apps/authoring/components/test_models.py b/tests/openedx_learning/apps/authoring/components/test_models.py index f087d4f3a..99b43dafd 100644 --- a/tests/openedx_learning/apps/authoring/components/test_models.py +++ b/tests/openedx_learning/apps/authoring/components/test_models.py @@ -3,13 +3,20 @@ """ from datetime import datetime, timezone +from freezegun import freeze_time + from openedx_learning.apps.authoring.components.api import ( create_component_and_version, get_component, get_or_create_component_type, ) from openedx_learning.apps.authoring.components.models import ComponentType -from openedx_learning.apps.authoring.publishing.api import LearningPackage, create_learning_package, publish_all_drafts +from openedx_learning.apps.authoring.publishing.api import ( + LearningPackage, + create_learning_package, + create_publishable_entity_version, + publish_all_drafts, +) from openedx_learning.lib.test_utils import TestCase @@ -53,3 +60,80 @@ def test_latest_version(self) -> None: # Grabbing the list of versions for this component assert list(component.versioning.versions) == [component_version] + + def test_last_publish_log(self): + """ + Test last_publish_log versioning property for published Components. + """ + # This Component will get a couple of Published versions + component_with_changes, _ = create_component_and_version( + self.learning_package.id, + component_type=self.problem_type, + local_key="with_changes", + title="Component with changes v1", + created=self.now, + created_by=None, + ) + + # This Component will only be Published once. + component_with_no_changes, _ = create_component_and_version( + self.learning_package.id, + component_type=self.problem_type, + local_key="with_no_changes", + title="Component with no changes v1", + created=self.now, + created_by=None, + ) + + # Publish first time. + published_first_time = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc) + with freeze_time(published_first_time): + publish_all_drafts(self.learning_package.id) + + # Refetch the entities to get latest versions + component_with_changes = get_component(component_with_changes.pk) + component_with_no_changes = get_component(component_with_no_changes.pk) + + # Fetch the most recent PublishLog for these components + first_publish_log_for_component_with_changes = component_with_changes.versioning.last_publish_log + first_publish_log_for_component_with_no_changes = component_with_no_changes.versioning.last_publish_log + + # PublishLog for library + both entities should match each other + assert ( + published_first_time == + first_publish_log_for_component_with_changes.published_at == + first_publish_log_for_component_with_no_changes.published_at + ) + + # Modify component_with_changes + create_publishable_entity_version( + component_with_changes.publishable_entity.id, + version_num=2, + title="Component with changes v2", + created=self.now, + created_by=None, + ) + + # Publish second time + published_second_time = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc) + with freeze_time(published_second_time): + publish_all_drafts(self.learning_package.id) + + # Refetch the entities to get latest versions + component_with_changes = get_component(component_with_changes.pk) + component_with_no_changes = get_component(component_with_no_changes.pk) + + # Re-fetch the most recent PublishLog for these components + next_publish_log_for_component_with_changes = component_with_changes.versioning.last_publish_log + next_publish_log_for_component_with_no_changes = component_with_no_changes.versioning.last_publish_log + + # PublishLog for component_with_changes should have been updated + assert ( + published_second_time == + next_publish_log_for_component_with_changes.published_at + ) + # But the component_with_no_changes should still be on the original publish log + assert ( + first_publish_log_for_component_with_no_changes == + next_publish_log_for_component_with_no_changes + ) From e7dddadc4155135193748e5d75584eae74648b4f Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 31 Jul 2024 04:41:05 +0930 Subject: [PATCH 5/6] perf: prefetch fields for VersioningHelper.last_publish_log and add test. --- openedx_learning/apps/authoring/components/models.py | 2 ++ tests/openedx_learning/apps/authoring/components/test_api.py | 1 + 2 files changed, 3 insertions(+) diff --git a/openedx_learning/apps/authoring/components/models.py b/openedx_learning/apps/authoring/components/models.py index 1e6f52159..df17a9993 100644 --- a/openedx_learning/apps/authoring/components/models.py +++ b/openedx_learning/apps/authoring/components/models.py @@ -134,6 +134,8 @@ class Component(PublishableEntityMixin): # type: ignore[django-manager-missing] 'publishable_entity__draft__version__componentversion', 'publishable_entity__published__version', 'publishable_entity__published__version__componentversion', + 'publishable_entity__published__publish_log_record', + 'publishable_entity__published__publish_log_record__publish_log', ) # This foreign key is technically redundant because we're already locked to diff --git a/tests/openedx_learning/apps/authoring/components/test_api.py b/tests/openedx_learning/apps/authoring/components/test_api.py index a6fca7d83..eb5a09787 100644 --- a/tests/openedx_learning/apps/authoring/components/test_api.py +++ b/tests/openedx_learning/apps/authoring/components/test_api.py @@ -73,6 +73,7 @@ def test_component_num_queries(self) -> None: draft = component.versioning.draft published = component.versioning.published assert draft.title == published.title + assert component.versioning.last_publish_log.published_at == self.now class GetComponentsTestCase(ComponentTestCase): From 1650893752d8232fb97d2c7fa0b85e5c56293bba Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 1 Aug 2024 02:18:59 +0930 Subject: [PATCH 6/6] chore: bumps version to 0.10.1 --- openedx_learning/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index bcc19e544..7262add03 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.10.0" +__version__ = "0.10.1"