Publish topic channels based on resource count (#1349)

* add channel published field

* perform deletion during resource_delete_actions

* make channel_url null if not published

* fix bad rebase :(

* add a data migration

* return 404 for unpublished channels

* add another test

* fix typo (haha pun)

* rename migration function

* bump migration number

* bump migration

Author: ChristopherChudzicki

channels/factories.py

@@ -26,6 +26,7 @@ class ChannelFactory(DjangoModelFactory):
 
     name = factory.fuzzy.FuzzyText(length=21)
     title = factory.Faker("text", max_nb_chars=50)
+    published = True
     public_description = factory.Faker("text", max_nb_chars=50)
     channel_type = factory.fuzzy.FuzzyChoice(ChannelType.names())
 
@@ -116,6 +117,9 @@ class ChannelTopicDetailFactory(DjangoModelFactory):
     class Meta:
         model = ChannelTopicDetail
 
+    class Params:
+        is_unpublished = factory.Trait(channel__published=False)
+
 
 class ChannelDepartmentDetailFactory(DjangoModelFactory):
     """Factory for a channels.models.ChannelDepartmentDetail object"""
@@ -128,6 +132,9 @@ class ChannelDepartmentDetailFactory(DjangoModelFactory):
     class Meta:
         model = ChannelDepartmentDetail
 
+    class Params:
+        is_unpublished = factory.Trait(channel__published=False)
+
 
 class ChannelUnitDetailFactory(DjangoModelFactory):
     """Factory for a channels.models.ChannelUnitDetail object"""
@@ -138,6 +145,9 @@ class ChannelUnitDetailFactory(DjangoModelFactory):
     class Meta:
         model = ChannelUnitDetail
 
+    class Params:
+        is_unpublished = factory.Trait(channel__published=False)
+
 
 class ChannelPathwayDetailFactory(DjangoModelFactory):
     """Factory for a channels.models.ChannelPathwayDetail object"""

channels/migrations/0015_channel_published.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.14 on 2024-08-06 14:39
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("channels", "0014_dept_detail_related_name"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="channel",
+            name="published",
+            field=models.BooleanField(db_index=True, default=True),
+        ),
+    ]

channels/models.py

@@ -5,7 +5,7 @@
 from django.contrib.auth.models import Group
 from django.core.validators import RegexValidator
 from django.db import models
-from django.db.models import JSONField, deletion
+from django.db.models import Case, JSONField, When, deletion
 from django.db.models.functions import Concat
 from imagekit.models import ImageSpecField, ProcessedImageField
 from imagekit.processors import ResizeToFit
@@ -32,12 +32,18 @@ class ChannelQuerySet(TimestampedModelQuerySet):
     def annotate_channel_url(self):
         """Annotate the channel for serialization"""
         return self.annotate(
-            channel_url=Concat(
-                models.Value(frontend_absolute_url("/c/")),
-                "channel_type",
-                models.Value("/"),
-                "name",
-                models.Value("/"),
+            channel_url=Case(
+                When(
+                    published=True,
+                    then=Concat(
+                        models.Value(frontend_absolute_url("/c/")),
+                        "channel_type",
+                        models.Value("/"),
+                        "name",
+                        models.Value("/"),
+                    ),
+                ),
+                default=None,
             )
         )
 
@@ -95,6 +101,7 @@ class Channel(TimestampedModel):
     configuration = models.JSONField(null=True, default=dict, blank=True)
     search_filter = models.CharField(max_length=2048, blank=True, default="")
     public_description = models.TextField(blank=True, default="")
+    published = models.BooleanField(default=True, db_index=True)
 
     featured_list = models.ForeignKey(
         LearningResource, null=True, blank=True, on_delete=deletion.SET_NULL
@@ -115,9 +122,11 @@ def __str__(self):
         return self.title
 
     @cached_property
-    def channel_url(self) -> str:
+    def channel_url(self) -> str | None:
         """Return the channel url"""
-        return frontend_absolute_url(f"/c/{self.channel_type}/{self.name}/")
+        if self.published:
+            return frontend_absolute_url(f"/c/{self.channel_type}/{self.name}/")
+        return None
 
     class Meta:
         unique_together = ("name", "channel_type")

channels/models_test.py

@@ -0,0 +1,39 @@
+from urllib.parse import urlparse
+
+import pytest
+
+from channels.constants import ChannelType
+from channels.factories import (
+    ChannelDepartmentDetailFactory,
+    ChannelTopicDetailFactory,
+    ChannelUnitDetailFactory,
+)
+
+pytestmark = [pytest.mark.django_db]
+
+
+@pytest.mark.parametrize("published", [True, False])
+@pytest.mark.parametrize(
+    (
+        "channel_type",
+        "detail_factory",
+    ),
+    [
+        (ChannelType.department, ChannelDepartmentDetailFactory),
+        (ChannelType.topic, ChannelTopicDetailFactory),
+        (ChannelType.unit, ChannelUnitDetailFactory),
+    ],
+)
+def test_channel_url_for_departments(published, channel_type, detail_factory):
+    """Test that the channel URL is correct for department channels"""
+    channel = detail_factory.create(
+        channel__published=published,
+    ).channel
+
+    if published:
+        assert (
+            urlparse(channel.channel_url).path
+            == f"/c/{channel_type.name}/{channel.name}/"
+        )
+    else:
+        assert channel.channel_url is None

channels/plugins.py

@@ -12,6 +12,29 @@
     ChannelTopicDetail,
     ChannelUnitDetail,
 )
+from learning_resources.models import LearningResource
+
+
+def unpublish_topics_for_resource(resource):
+    """
+    Unpublish channels for topics that are used exclusively by the resource
+
+    Args:
+        resource(LearningResource): The resource that was unpublished
+    """
+    other_published = LearningResource.objects.filter(
+        published=True,
+    ).exclude(id=resource.id)
+
+    channels = Channel.objects.filter(
+        topic_detail__topic__in=resource.topics.all(),
+        channel_type=ChannelType.topic.name,  # Redundant, but left for clarity
+        published=True,
+    ).exclude(topic_detail__topic__learningresource__in=other_published)
+
+    for channel in channels:
+        channel.published = False
+        channel.save()
 
 
 class ChannelPlugin:
@@ -140,3 +163,29 @@ def offeror_delete(self, offeror):
         """
         Channel.objects.filter(unit_detail__unit=offeror).delete()
         offeror.delete()
+
+    @hookimpl
+    def resource_upserted(self, resource, percolate):  # noqa: ARG002
+        """
+        Publish channels for the resource's topics
+        """
+        channels = Channel.objects.filter(
+            topic_detail__topic__in=resource.topics.all(), published=False
+        )
+        for channel in channels:
+            channel.published = True
+            channel.save()
+
+    @hookimpl
+    def resource_before_delete(self, resource):
+        """
+        Unpublish channels for the resource's topics
+        """
+        unpublish_topics_for_resource(resource)
+
+    @hookimpl
+    def resource_unpublished(self, resource):
+        """
+        Unpublish channels for the resource's topics
+        """
+        unpublish_topics_for_resource(resource)

channels/plugins_test.py

@@ -3,11 +3,16 @@
 import pytest
 
 from channels.constants import ChannelType
-from channels.factories import ChannelDepartmentDetailFactory, ChannelFactory
+from channels.factories import (
+    ChannelDepartmentDetailFactory,
+    ChannelFactory,
+    ChannelTopicDetailFactory,
+)
 from channels.models import Channel
 from channels.plugins import ChannelPlugin
 from learning_resources.factories import (
     LearningResourceDepartmentFactory,
+    LearningResourceFactory,
     LearningResourceOfferorFactory,
     LearningResourceSchoolFactory,
     LearningResourceTopicFactory,
@@ -140,3 +145,83 @@ def test_search_index_plugin_offeror_delete():
     ChannelPlugin().offeror_delete(offeror)
     assert Channel.objects.filter(id=channel.id).exists() is False
     assert LearningResourceOfferor.objects.filter(code=offeror.code).exists() is False
+
+
+@pytest.mark.parametrize("action", ["delete", "unpublish"])
+@pytest.mark.parametrize(
+    ("published_resources", "to_remove", "expect_channel_published"),
+    [
+        (2, 0, True),  # 2 published resources remain
+        (2, 1, True),  # 1 published resources remain
+        (2, 2, False),  # 0 published resource remains
+    ],
+)
+@pytest.mark.django_db()
+def test_resource_before_delete_and_resource_unpublish(
+    action, published_resources, to_remove, expect_channel_published
+):
+    """
+    Test that topic channels are unpublished when they no longer have any resources
+    remaining.
+    """
+    topic1 = LearningResourceTopicFactory.create()  # for to-be-deleted resources
+    topic2 = LearningResourceTopicFactory.create()  # for to-be-deleted & others
+    topic3 = LearningResourceTopicFactory.create()  # for to-be-deleted resources
+    detail1 = ChannelTopicDetailFactory.create(topic=topic1)
+    detail2 = ChannelTopicDetailFactory.create(topic=topic2)
+    detail3 = ChannelTopicDetailFactory.create(topic=topic3)
+    channel1, channel2, channel3 = detail1.channel, detail2.channel, detail3.channel
+
+    resources_in_play = LearningResourceFactory.create_batch(
+        published_resources,
+        topics=[topic1, topic2, topic3],
+    )
+
+    # Create extra published + unpublished resources to ensure topic2 sticks around
+    LearningResourceFactory.create(topics=[topic2])  # extra resources
+
+    assert channel1.published
+    assert channel2.published
+    assert channel3.published
+
+    for resource in resources_in_play[:to_remove]:
+        if action == "delete":
+            ChannelPlugin().resource_before_delete(resource)
+            resource.delete()
+        elif action == "unpublish":
+            resource.published = False
+            resource.save()
+            ChannelPlugin().resource_unpublished(resource)
+        else:
+            msg = ValueError(f"Invalid action {action}")
+            raise msg
+
+    channel1.refresh_from_db()
+    channel2.refresh_from_db()
+    channel3.refresh_from_db()
+    assert channel1.published is expect_channel_published
+    assert channel2.published is True
+    assert channel3.published is expect_channel_published
+
+
+@pytest.mark.django_db()
+def test_resource_upserted():
+    """
+    Test that channels are published when a resource is created or updated
+    """
+    channel1 = ChannelFactory.create(is_topic=True, published=False)
+    channel2 = ChannelFactory.create(is_topic=True, published=False)
+    channel3 = ChannelFactory.create(is_topic=True, published=False)
+
+    resource = LearningResourceFactory.create(
+        topics=[channel1.topic_detail.topic, channel2.topic_detail.topic]
+    )
+    ChannelPlugin().resource_upserted(resource, None)
+
+    channel1.refresh_from_db()
+    channel2.refresh_from_db()
+    channel3.refresh_from_db()
+
+    assert channel1.published is True
+    assert channel2.published is True
+    assert channel3.published is False

channels/serializers.py

@@ -157,7 +157,7 @@ def get_channel_url(self, instance) -> str:
 
     class Meta:
         model = Channel
-        exclude = []
+        exclude = ["published"]
 
 
 class ChannelTopicDetailSerializer(serializers.ModelSerializer):

channels/views.py

@@ -88,7 +88,8 @@ def get_serializer_context(self):
     def get_queryset(self):
         """Return a queryset"""
         return (
-            Channel.objects.prefetch_related(
+            Channel.objects.filter(published=True)
+            .prefetch_related(
                 Prefetch(
                     "lists",
                     queryset=ChannelList.objects.prefetch_related(
@@ -144,6 +145,7 @@ def get_object(self):
             Channel,
             channel_type=self.kwargs["channel_type"],
             name=self.kwargs["name"],
+            published=True,
         )
 
 

channels/views_test.py

@@ -28,11 +28,14 @@
 
 def test_list_channels(user_client):
     """Test that all channels are returned"""
-    channels = sorted(ChannelFactory.create_batch(10), key=lambda f: f.id)
+    ChannelFactory.create_batch(2, published=False)  # should be filtered out
+    channels = sorted(ChannelFactory.create_batch(3), key=lambda f: f.id)
+    ChannelFactory.create_batch(2, published=False)  # should be filtered out
+
     url = reverse("channels:v0:channels_api-list")
     channel_list = sorted(user_client.get(url).json()["results"], key=lambda f: f["id"])
-    assert len(channel_list) == 10
-    for idx, channel in enumerate(channels[:10]):
+    assert len(channel_list) == 3
+    for idx, channel in enumerate(channels[:3]):
         assert channel_list[idx] == ChannelSerializer(instance=channel).data
 
 
@@ -206,20 +209,26 @@ def test_patch_channel_image(client, channel, attribute):
             assert len(size_image.read()) > 0
 
 
-def test_channel_by_type_name_detail(user_client):
+@pytest.mark.parametrize(
+    ("published", "requested_type", "response_status"),
+    [
+        (True, ChannelType.topic, 200),
+        (False, ChannelType.topic, 404),
+        (True, ChannelType.department, 404),
+        (False, ChannelType.department, 404),
+    ],
+)
+def test_channel_by_type_name_detail(
+    user_client, published, requested_type, response_status
+):
     """ChannelByTypeNameDetailView should return expected result"""
-    channel = ChannelFactory.create(is_topic=True)
+    channel = ChannelFactory.create(is_topic=True, published=published)
     url = reverse(
         "channels:v0:channel_by_type_name_api-detail",
-        kwargs={"channel_type": ChannelType.topic.name, "name": channel.name},
-    )
-    response = user_client.get(url)
-    assert response.json() == ChannelSerializer(instance=channel).data
-    Channel.objects.filter(id=channel.id).update(
-        channel_type=ChannelType.department.name
+        kwargs={"channel_type": requested_type.name, "name": channel.name},
     )
     response = user_client.get(url)
-    assert response.status_code == 404
+    assert response.status_code == response_status
 
 
 def test_update_channel_forbidden(channel, user_client):