address the feedback

Author: Ahtesham Quraish

frontends/api/src/generated/v1/api.ts

@@ -4,13 +4,13 @@
 
 from news_events.constants import FeedType
 from news_events.etl import loaders
-from website_content.constants import CONTENT_TYPE_NEWS
+from website_content.constants import WebsiteContentType
 from website_content.models import WebsiteContent
 
 log = logging.getLogger(__name__)
 
 
-def extract_single_article(article: WebsiteContent) -> dict:
+def extract_single_website_content(article: WebsiteContent) -> dict:
     """
     Extract a single published news content item from the database.
     Returns a dict in the same format as extract().
@@ -36,16 +36,16 @@ def transform_single_article(article_data: dict) -> dict:
     return items[0] if items else None
 
 
-def sync_single_article_to_news(article: WebsiteContent):
+def sync_single_website_content_news_to_news(article: WebsiteContent):
     """
     Sync a single published news content item to the news feed.
     Only syncs content items with content_type='news'.
     """
     if not article.is_published:
         return
-    if article.content_type != CONTENT_TYPE_NEWS:
+    if article.content_type != WebsiteContentType.news.name:
         return
-    article_data = extract_single_article(article)
+    article_data = extract_single_website_content(article)
     item_data = transform_single_article(article_data)
     if not item_data:
         return
@@ -72,7 +72,7 @@ def extract() -> list[dict]:
     """
     # Get only published news-type content items
     articles = WebsiteContent.objects.filter(
-        is_published=True, content_type=CONTENT_TYPE_NEWS
+        is_published=True, content_type=WebsiteContentType.news.name
     ).select_related("user")
 
     return [

news_events/etl/articles_news.py

@@ -564,7 +564,7 @@ class MockArticle:
         publish_date = datetime(2024, 1, 1, 0, 0, 0, tzinfo=UTC)
         user = User()
 
-    result = articles_news.extract_single_article(MockArticle())
+    result = articles_news.extract_single_website_content(MockArticle())
 
     assert result["id"] == 1
     assert result["title"] == "Test Article"
@@ -640,7 +640,7 @@ class MockArticle:
         user = User()
 
     # Sync the article
-    articles_news.sync_single_article_to_news(MockArticle())
+    articles_news.sync_single_website_content_news_to_news(MockArticle())
 
     # Verify FeedSource was created/retrieved
     mock_get_or_create.assert_called_once()
@@ -661,7 +661,20 @@ class MockArticle:
         is_published = False
 
     # Sync the article
-    articles_news.sync_single_article_to_news(MockArticle())
+    articles_news.sync_single_website_content_news_to_news(MockArticle())
 
     # Verify load_feed_item was NOT called
     mock_load_feed_item.assert_not_called()
+
+
+def test_sync_single_article_wrong_content_type(mocker):
+    """Test that articles with a content_type other than 'news' are not synced"""
+    mock_load_feed_item = mocker.patch("news_events.etl.loaders.load_feed_item")
+
+    class MockArticle:
+        is_published = True
+        content_type = "article"  # not WebsiteContentType.news
+
+    articles_news.sync_single_website_content_news_to_news(MockArticle())
+
+    mock_load_feed_item.assert_not_called()

news_events/etl/articles_news_test.py

@@ -21,9 +21,9 @@ def website_content_published(self, content):
         Args:
             content (WebsiteContent): The content item that was published or updated
         """
-        from website_content.constants import CONTENT_TYPE_NEWS
+        from website_content.constants import WebsiteContentType
 
-        if content.content_type != CONTENT_TYPE_NEWS:
+        if content.content_type != WebsiteContentType.news.name:
             log.info(
                 "WebsiteContentNewsPlugin: Skipping non-news content: id=%s, type=%s",
                 content.id,

news_events/plugins.py

@@ -76,14 +76,14 @@ def sync_website_content_to_news(self, content_id: int):
     """
     import logging
 
-    from news_events.etl.articles_news import sync_single_article_to_news
+    from news_events.etl.articles_news import sync_single_website_content_news_to_news
     from website_content.models import WebsiteContent
 
     logger = logging.getLogger(__name__)
 
     try:
         content = WebsiteContent.objects.get(id=content_id, is_published=True)
-        sync_single_article_to_news(content)
+        sync_single_website_content_news_to_news(content)
         clear_views_cache()
         logger.info(
             "Successfully synced content %s to news feed",

news_events/tasks.py

@@ -66,7 +66,8 @@ def test_sync_article_to_news_success(mocker, user):
     )
 
     mock_sync = mocker.patch(
-        "news_events.etl.articles_news.sync_single_article_to_news", autospec=True
+        "news_events.etl.articles_news.sync_single_website_content_news_to_news",
+        autospec=True,
     )
     mock_clear_cache = mocker.patch(
         "news_events.tasks.clear_views_cache", autospec=True
@@ -82,7 +83,8 @@ def test_sync_article_to_news_success(mocker, user):
 def test_sync_article_to_news_article_not_found(mocker, caplog):
     """Task should log warning if content item doesn't exist"""
     mock_sync = mocker.patch(
-        "news_events.etl.articles_news.sync_single_article_to_news", autospec=True
+        "news_events.etl.articles_news.sync_single_website_content_news_to_news",
+        autospec=True,
     )
     mock_clear_cache = mocker.patch(
         "news_events.tasks.clear_views_cache", autospec=True
@@ -110,7 +112,8 @@ def test_sync_article_to_news_unpublished_article(mocker, user, caplog):
     )
 
     mock_sync = mocker.patch(
-        "news_events.etl.articles_news.sync_single_article_to_news", autospec=True
+        "news_events.etl.articles_news.sync_single_website_content_news_to_news",
+        autospec=True,
     )
     mock_clear_cache = mocker.patch(
         "news_events.tasks.clear_views_cache", autospec=True
@@ -138,7 +141,7 @@ def test_sync_article_to_news_sync_failure(mocker, user):
     )
 
     mock_sync = mocker.patch(
-        "news_events.etl.articles_news.sync_single_article_to_news",
+        "news_events.etl.articles_news.sync_single_website_content_news_to_news",
         autospec=True,
         side_effect=Exception("Sync failed"),
     )

news_events/tasks_test.py

@@ -14361,8 +14361,7 @@ components:
         author_name:
           type: string
           default: ''
-        content:
-          default: {}
+        content: {}
         content_type:
           allOf:
           - $ref: '#/components/schemas/WebsiteContentContentTypeEnum'
@@ -17437,8 +17436,7 @@ components:
         author_name:
           type: string
           default: ''
-        content:
-          default: {}
+        content: {}
         content_type:
           allOf:
           - $ref: '#/components/schemas/WebsiteContentContentTypeEnum'
@@ -17478,11 +17476,11 @@ components:
       - article
       type: string
       description: |-
-        * `news` - news
-        * `article` - article
+        * `news` - News
+        * `article` - Article
       x-enum-descriptions:
-      - news
-      - article
+      - News
+      - Article
     WebsiteContentImageUploadRequest:
       type: object
       properties:
@@ -17502,8 +17500,7 @@ components:
         author_name:
           type: string
           default: ''
-        content:
-          default: {}
+        content: {}
         content_type:
           allOf:
           - $ref: '#/components/schemas/WebsiteContentContentTypeEnum'

openapi/specs/v1.yaml

@@ -118,6 +118,9 @@ def profile_image_upload_uri(instance, filename):
 def article_image_upload_uri(_, filename):
     """
     upload_to handler for ArticleImageUpload.image_file
+
+    Deprecated: use website_content.utils.website_content_image_upload_uri instead.
+    Kept here so existing migrations that reference this dotted path remain replayable.
     """
     return generate_filepath(filename, "", "", "article")
 

profiles/utils.py

@@ -2,6 +2,7 @@
 
 import logging
 
+from website_content.constants import WebsiteContentType
 from website_content.hooks import get_plugin_manager
 from website_content.tasks import (
     PURGE_TIMEOUT_SECONDS,
@@ -11,6 +12,11 @@
 
 log = logging.getLogger(__name__)
 
+_CONTENT_TYPE_LISTING_URL = {
+    WebsiteContentType.news.name: "/news",
+    WebsiteContentType.article.name: "/articles",
+}
+
 
 def purge_content_on_save(content):
     """
@@ -44,7 +50,9 @@ def purge_content_on_save(content):
             fastly_purge_relative_url.delay(content_url)
             log.exception("Content purge request failed, enqueued for retry.")
 
-        fastly_purge_website_content_list.delay()
+        fastly_purge_website_content_list.delay(
+            _CONTENT_TYPE_LISTING_URL.get(content.content_type, "/news")
+        )
     else:
         log.debug(
             "WebsiteContent %s is not published or has no slug, skipping CDN purge.",

website_content/api.py

@@ -2,6 +2,81 @@
 
 import pytest
 
+from website_content.api import purge_content_on_save
+from website_content.constants import WebsiteContentType
+from website_content.factories import WebsiteContentFactory
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    ("slug", "is_published", "expect_purge"),
+    [
+        ("test-article", True, True),
+        ("test-article", False, False),
+        ("", True, False),
+        ("", False, False),
+    ],
+)
+def test_purge_content_on_save(mocker, slug, is_published, expect_purge):
+    """
+    CDN purge should only fire when the content is published AND has a slug.
+    Asserts on call_fastly_purge_api (the lowest plumbing before the HTTP call)
+    so the test is independent of FASTLY_API_KEY being set in the test env.
+    @single_task uses a Redis lock so fastly_purge_website_content_list.delay
+    is mocked to avoid that dependency in unit tests.
+    """
+    mock_purge_api = mocker.patch("website_content.tasks.call_fastly_purge_api")
+    mock_purge_api.return_value = {"status": "ok", "id": "abc"}
+    mocker.patch("website_content.tasks.fastly_purge_website_content_list.delay")
+
+    content = WebsiteContentFactory.build(
+        is_published=is_published,
+        slug=slug or None,
+        content_type=WebsiteContentType.news.name,
+    )
+
+    purge_content_on_save(content)
+
+    if expect_purge:
+        assert mock_purge_api.called
+    else:
+        mock_purge_api.assert_not_called()
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    ("content_type", "expected_listing"),
+    [
+        (WebsiteContentType.news.name, "/news"),
+        (WebsiteContentType.article.name, "/articles"),
+    ],
+)
+def test_purge_content_on_save_listing_url(mocker, content_type, expected_listing):
+    """
+    The listing-page purge task should be enqueued with the URL matching
+    the content's content_type.
+    """
+    mocker.patch("website_content.tasks.call_fastly_purge_api").return_value = {
+        "status": "ok",
+        "id": "abc",
+    }
+    mock_purge_list = mocker.patch(
+        "website_content.tasks.fastly_purge_website_content_list.delay"
+    )
+
+    content = WebsiteContentFactory.build(
+        is_published=True,
+        slug="some-slug",
+        content_type=content_type,
+    )
+    mocker.patch.object(
+        type(content), "get_url", return_value=f"/{content_type}/some-slug"
+    )
+
+    purge_content_on_save(content)
+
+    mock_purge_list.assert_called_once_with(expected_listing)
+
 
 @pytest.mark.django_db
 def test_content_published_actions_triggers_hook(mocker, user):