Various fixes/enhancements to support v2 libraries import

openedx_learning/__init__.py

@@ -1,4 +1,4 @@
 """
 Open edX Learning ("Learning Core").
 """
-__version__ = "0.3.0"
+__version__ = "0.3.1"

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,31 +77,57 @@ class PublishableEntityAdmin(ReadOnlyModelAdmin):
     """
     Read-only admin view for Publishable Entities
     """
-    fields = (
+    list_display = [
         "key",
         "draft_version",
         "published_version",
         "uuid",
         "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)

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],

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'),
+        ),
+    ]

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(

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(
+            '{}: <a href="{}">{}</a>',
+            field_name,
+            details_url,
+            related_model_obj,
+        )
+        text.append(html)
+
+    return format_html_join("\n", "<li>{}</li>", ((t,) for t in text))

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