diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index 8872149d9..38d376f15 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.3.0" +__version__ = "0.3.1" diff --git a/openedx_learning/core/publishing/admin.py b/openedx_learning/core/publishing/admin.py index 56c59d28f..90f72bcc8 100644 --- a/openedx_learning/core/publishing/admin.py +++ b/openedx_learning/core/publishing/admin.py @@ -5,7 +5,7 @@ from django.contrib import admin -from openedx_learning.lib.admin_utils import ReadOnlyModelAdmin +from openedx_learning.lib.admin_utils import ReadOnlyModelAdmin, one_to_one_related_model_html from .models import LearningPackage, PublishableEntity, Published, PublishLog, PublishLogRecord @@ -77,7 +77,7 @@ class PublishableEntityAdmin(ReadOnlyModelAdmin): """ Read-only admin view for Publishable Entities """ - fields = ( + list_display = [ "key", "draft_version", "published_version", @@ -85,23 +85,49 @@ class PublishableEntityAdmin(ReadOnlyModelAdmin): "learning_package", "created", "created_by", - ) - readonly_fields = fields - list_display = fields + ] list_filter = ["learning_package"] search_fields = ["key", "uuid"] + fields = [ + "key", + "draft_version", + "published_version", + "uuid", + "learning_package", + "created", + "created_by", + "see_also", + ] + readonly_fields = [ + "key", + "draft_version", + "published_version", + "uuid", + "learning_package", + "created", + "created_by", + "see_also", + ] + def get_queryset(self, request): queryset = super().get_queryset(request) return queryset.select_related( - "learning_package", "published__version", "draft__version" + "learning_package", "published__version", ) + def see_also(self, entity): + return one_to_one_related_model_html(entity) + def draft_version(self, entity): - return entity.draft.version.version_num + if entity.draft.version: + return entity.draft.version.version_num + return None def published_version(self, entity): - return entity.published.version.version_num + if entity.published.version: + return entity.published.version.version_num + return None @admin.register(Published) diff --git a/openedx_learning/core/publishing/api.py b/openedx_learning/core/publishing/api.py index b4b701a20..ab4099ad2 100644 --- a/openedx_learning/core/publishing/api.py +++ b/openedx_learning/core/publishing/api.py @@ -93,9 +93,9 @@ def create_publishable_entity_version( created=created, created_by_id=created_by, ) - Draft.objects.create( + Draft.objects.update_or_create( entity_id=entity_id, - version=version, + defaults={"version": version}, ) return version @@ -180,6 +180,42 @@ def publish_from_drafts( return publish_log +def get_draft_version(publishable_entity_id: int) -> PublishableEntityVersion | None: + """ + Return current draft PublishableEntityVersion for this PublishableEntity. + + This function will return None if there is no current draft. + """ + try: + draft = Draft.objects.select_related("version").get( + entity_id=publishable_entity_id + ) + except Draft.DoesNotExist: + # No draft was ever created. + return None + + # draft.version could be None if it was set that way by set_draft_version. + # Setting the Draft.version to None is how we show that we've "deleted" the + # content in Studio. + return draft.version + + +def set_draft_version(publishable_entity_id: int, publishable_entity_version_pk: int | None) -> None: + """ + Modify the Draft of a PublishableEntity to be a PublishableEntityVersion. + + 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. We don't actually delete the Draft row + because we'll need that for publishing purposes (i.e. to delete content from + the published branch). + """ + draft = Draft.objects.get(entity_id=publishable_entity_id) + draft.version_id = publishable_entity_version_pk + draft.save() + + def register_content_models( content_model_cls: type[PublishableEntityMixin], content_version_model_cls: type[PublishableEntityVersionMixin], diff --git a/openedx_learning/core/publishing/migrations/0002_alter_fk_on_delete.py b/openedx_learning/core/publishing/migrations/0002_alter_fk_on_delete.py new file mode 100644 index 000000000..c77a20400 --- /dev/null +++ b/openedx_learning/core/publishing/migrations/0002_alter_fk_on_delete.py @@ -0,0 +1,30 @@ +# Generated by Django 3.2.21 on 2023-10-13 14:25 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + """ + Make it so that Draft and Published cascade deletes from PublishableEntity. + + This makes it so that deleting a LearningPackage properly cleans up these + models as well. + """ + + dependencies = [ + ('oel_publishing', '0001_initial'), + ] + + operations = [ + migrations.AlterField( + model_name='draft', + name='entity', + field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, serialize=False, to='oel_publishing.publishableentity'), + ), + migrations.AlterField( + model_name='published', + name='entity', + field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, serialize=False, to='oel_publishing.publishableentity'), + ), + ] diff --git a/openedx_learning/core/publishing/models.py b/openedx_learning/core/publishing/models.py index 7cbeef87c..1eb6803c8 100644 --- a/openedx_learning/core/publishing/models.py +++ b/openedx_learning/core/publishing/models.py @@ -308,10 +308,12 @@ class Draft(models.Model): 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.RESTRICT, + on_delete=models.CASCADE, primary_key=True, ) version = models.OneToOneField( @@ -425,7 +427,7 @@ class Published(models.Model): entity = models.OneToOneField( PublishableEntity, - on_delete=models.RESTRICT, + on_delete=models.CASCADE, primary_key=True, ) version = models.OneToOneField( diff --git a/openedx_learning/lib/admin_utils.py b/openedx_learning/lib/admin_utils.py index f7509726f..ece0e3a45 100644 --- a/openedx_learning/lib/admin_utils.py +++ b/openedx_learning/lib/admin_utils.py @@ -2,6 +2,9 @@ Convenience utilities for the Django Admin. """ from django.contrib import admin +from django.db.models.fields.reverse_related import OneToOneRel +from django.urls import NoReverseMatch, reverse +from django.utils.html import format_html, format_html_join class ReadOnlyModelAdmin(admin.ModelAdmin): @@ -26,3 +29,72 @@ def has_change_permission(self, request, obj=None): def has_delete_permission(self, request, obj=None): return False + + +def one_to_one_related_model_html(model_obj): + """ + HTML for clickable list of a models that are 1:1-related to ``model_obj``. + + Our design pattern encourages people to hang models off of our lower-level + core lib models. For example, Component has a OneToOneField that references + PublishableEntity. It would be really convenient to have PublishableEntity's + admin page display the link to Component, but the ``publishable`` app is + intended to be a lower-level app than ``components`` and isn't supposed to + be aware of it. The same situation occurs for third-party apps that might + want to extend Component. + + So instead of creating a circular dependency by having ``publishing`` + referencing ``components``, we use Django model introspection to iterate + over all models that have a OneToOneField to the passe din``model_obj``. + This allows us to preserve our dependency boundaries within openedx-learning + and accomodate any third party apps that might further extend these models. + + This will output a list with one entry for each related field. + + * If the field's value is None, we output f"{field_name}: -" + * If the field has a value but no "change" admin page, we output the string + representation of the model obj referenced by that field, i.e. + f{"field_name: {related_model_obj}"}. + * If the field has a value and an admin page, we output the same as above, + but we make the related model object's string representation a link to its + "change" admin page. + """ + one_to_one_field_names = [ + field.name + for field in model_obj._meta.related_objects + if isinstance(field, OneToOneRel) + ] + text = [] + for field_name in one_to_one_field_names: + related_model_obj = getattr(model_obj, field_name, None) + + # No instance of the related model was found, so just use "-" + if related_model_obj is None: + text.append(f"{field_name}: -") + continue + + app_label = related_model_obj._meta.app_label + model_name = related_model_obj._meta.model_name + try: + details_url = reverse( + f"admin:{app_label}_{model_name}_change", + args=(related_model_obj.pk,) + ) + except NoReverseMatch: + # No Admin URL available, so just put the str representation of the + # related model instance. + text.append(f"{field_name}: {related_model_obj}") + continue + + # If we go this far, there is a related model instance and it has a + # "change" admin page (even though it's probably read-only via + # permissions). + html = format_html( + '{}: {}', + field_name, + details_url, + related_model_obj, + ) + text.append(html) + + return format_html_join("\n", "
  • {}
  • ", ((t,) for t in text)) diff --git a/tests/openedx_learning/core/publishing/test_api.py b/tests/openedx_learning/core/publishing/test_api.py index 2a4fdced3..9327a1008 100644 --- a/tests/openedx_learning/core/publishing/test_api.py +++ b/tests/openedx_learning/core/publishing/test_api.py @@ -8,7 +8,7 @@ from django.core.exceptions import ValidationError from django.test import TestCase -from openedx_learning.core.publishing.api import create_learning_package +from openedx_learning.core.publishing import api as publishing_api class CreateLearningPackageTestCase(TestCase): @@ -22,7 +22,7 @@ def test_normal(self) -> None: # Note: we must specify '-> None' to opt in to t key = "my_key" title = "My Excellent Title with Emoji 🔥" created = datetime(2023, 4, 2, 15, 9, 0, tzinfo=timezone.utc) - package = create_learning_package(key, title, created) + package = publishing_api.create_learning_package(key, title, created) assert package.key == "my_key" assert package.title == "My Excellent Title with Emoji 🔥" @@ -41,7 +41,7 @@ def test_auto_datetime(self) -> None: """ key = "my_key" title = "My Excellent Title with Emoji 🔥" - package = create_learning_package(key, title) + package = publishing_api.create_learning_package(key, title) assert package.key == "my_key" assert package.title == "My Excellent Title with Emoji 🔥" @@ -62,7 +62,7 @@ def test_non_utc_time(self) -> None: Require UTC timezone for created. """ with pytest.raises(ValidationError) as excinfo: - create_learning_package("my_key", "A Title", datetime(2023, 4, 2)) + publishing_api.create_learning_package("my_key", "A Title", datetime(2023, 4, 2)) message_dict = excinfo.value.message_dict # Both datetime fields should be marked as invalid @@ -73,8 +73,49 @@ def test_already_exists(self) -> None: """ Raises ValidationError for duplicate keys. """ - create_learning_package("my_key", "Original") + publishing_api.create_learning_package("my_key", "Original") with pytest.raises(ValidationError) as excinfo: - create_learning_package("my_key", "Duplicate") + publishing_api.create_learning_package("my_key", "Duplicate") message_dict = excinfo.value.message_dict assert "key" in message_dict + + +class DraftTestCase(TestCase): + """ + Test basic operations with Drafts. + """ + + def test_draft_lifecycle(self): + """ + Test basic lifecycle of a Draft. + """ + created = datetime(2023, 4, 2, 15, 9, 0, tzinfo=timezone.utc) + package = publishing_api.create_learning_package( + "my_package_key", + "Draft Testing LearningPackage 🔥", + created=created, + ) + entity = publishing_api.create_publishable_entity( + package.id, + "my_entity", + created, + created_by=None, + ) + # Drafts are NOT created when a PublishableEntity is created, only when + # its first PublisahbleEntityVersion is. + assert publishing_api.get_draft_version(entity.id) is None + + entity_version = publishing_api.create_publishable_entity_version( + entity_id=entity.id, + version_num=1, + title="An Entity 🌴", + created=created, + created_by=None, + ) + assert entity_version == publishing_api.get_draft_version(entity.id) + + # We never really remove rows from the table holding Drafts. We just + # mark the version as None. + publishing_api.set_draft_version(entity.id, None) + entity_version = publishing_api.get_draft_version(entity.id) + assert entity_version is None