From a21934b0df7c63bc046a958091175a5ec9cb847f Mon Sep 17 00:00:00 2001 From: Ahtesham Quraish Date: Wed, 10 Jun 2026 11:43:22 +0500 Subject: [PATCH 1/4] fix: pick cover image of video if no image is inserted in article content (#3433) * fix: pick cover image of video if no image is inserted in article content --------- Co-authored-by: Ahtesham Quraish --- frontends/api/src/generated/v1/api.ts | 6 + .../api/src/hooks/website_content/index.ts | 7 +- .../test-utils/factories/websiteContent.ts | 1 + .../main/src/common/websiteContentUtils.ts | 37 ++-- .../node/ByLineInfoBar/ByLineInfoBar.tsx | 71 ++++---- news_events/etl/articles_news.py | 109 +----------- news_events/etl/articles_news_test.py | 61 +------ openapi/specs/v1.yaml | 7 + .../commands/backpopulate_cover_images.py | 55 ++++++ .../0005_add_cover_image_to_websitecontent.py | 17 ++ website_content/models.py | 13 +- website_content/models_test.py | 107 ++++++++++++ website_content/serializers.py | 4 + website_content/utils.py | 103 +++++++++++ website_content/utils_test.py | 164 ++++++++++++++++++ 15 files changed, 557 insertions(+), 205 deletions(-) create mode 100644 website_content/management/commands/backpopulate_cover_images.py create mode 100644 website_content/migrations/0005_add_cover_image_to_websitecontent.py create mode 100644 website_content/utils_test.py diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index f50c980fa0..a17ee4dc6a 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -10129,6 +10129,12 @@ export interface WebsiteContent { * @memberof WebsiteContent */ slug?: string + /** + * + * @type {string} + * @memberof WebsiteContent + */ + cover_image: string } /** diff --git a/frontends/api/src/hooks/website_content/index.ts b/frontends/api/src/hooks/website_content/index.ts index adeedce1fb..2ed18c7ee8 100644 --- a/frontends/api/src/hooks/website_content/index.ts +++ b/frontends/api/src/hooks/website_content/index.ts @@ -42,7 +42,12 @@ const useWebsiteContentCreate = () => { mutationFn: ( data: Omit< WebsiteContent, - "id" | "user" | "created_on" | "updated_on" | "publish_date" + | "id" + | "user" + | "created_on" + | "updated_on" + | "publish_date" + | "cover_image" >, ) => websiteContentApi diff --git a/frontends/api/src/test-utils/factories/websiteContent.ts b/frontends/api/src/test-utils/factories/websiteContent.ts index 9e19593357..5a7248b3ec 100644 --- a/frontends/api/src/test-utils/factories/websiteContent.ts +++ b/frontends/api/src/test-utils/factories/websiteContent.ts @@ -6,6 +6,7 @@ import type { WebsiteContent } from "../../generated/v1" const websiteContent: Factory = (overrides = {}) => ({ id: faker.number.int(), title: faker.lorem.sentence(), + cover_image: faker.image.url(), content: { type: "doc", content: [ diff --git a/frontends/main/src/common/websiteContentUtils.ts b/frontends/main/src/common/websiteContentUtils.ts index 6ef96a706a..a46a22a097 100644 --- a/frontends/main/src/common/websiteContentUtils.ts +++ b/frontends/main/src/common/websiteContentUtils.ts @@ -94,19 +94,30 @@ export function extractArticleContent( const banner = topLevel.find((n) => n.type === "banner") const headingNode = banner?.content?.find((n) => n.type === "heading") const paragraphNode = banner?.content?.find((n) => n.type === "paragraph") - - const imageNode = topLevel.find((n) => n.type === "imageWithCaption") - const imageAttrs = imageNode?.attrs - const image = - imageAttrs?.src && typeof imageAttrs.src === "string" - ? { - src: imageAttrs.src, - alt: typeof imageAttrs.alt === "string" ? imageAttrs.alt : null, - caption: - typeof imageAttrs.caption === "string" ? imageAttrs.caption : null, - } - : null - + let image = null + if (article?.cover_image) { + image = { + src: article?.cover_image || "", + alt: "", + caption: "", + } + } + if (image === null) { + const imageNode = topLevel.find((n) => n.type === "imageWithCaption") + const imageAttrs = imageNode?.attrs + + image = + imageAttrs?.src && typeof imageAttrs.src === "string" + ? { + src: imageAttrs.src, + alt: typeof imageAttrs.alt === "string" ? imageAttrs.alt : null, + caption: + typeof imageAttrs.caption === "string" + ? imageAttrs.caption + : null, + } + : null + } return { heading: extractText(headingNode?.content) || null, paragraph: nodesToHtml(paragraphNode?.content) || null, diff --git a/frontends/main/src/page-components/TiptapEditor/extensions/node/ByLineInfoBar/ByLineInfoBar.tsx b/frontends/main/src/page-components/TiptapEditor/extensions/node/ByLineInfoBar/ByLineInfoBar.tsx index fbeb563ea1..b9977ce21b 100644 --- a/frontends/main/src/page-components/TiptapEditor/extensions/node/ByLineInfoBar/ByLineInfoBar.tsx +++ b/frontends/main/src/page-components/TiptapEditor/extensions/node/ByLineInfoBar/ByLineInfoBar.tsx @@ -30,12 +30,11 @@ const StyledWrapper = styled.div(({ theme }) => ({ border: `1px solid ${theme.custom.colors.lightGray2}`, })) -const InnerContainer = styled(Container, { - shouldForwardProp: (prop) => prop !== "noAuthor", -})<{ noAuthor?: boolean }>(({ noAuthor }) => ({ +const InnerContainer = styled(Container)(() => ({ display: "flex", - justifyContent: noAuthor ? "flex-end" : "space-between", + justifyContent: "space-between", alignItems: "center", + gap: "8px", "&&": { maxWidth: "890px", }, @@ -126,38 +125,38 @@ export const ByLineInfoBarContent = ({ onClose={() => setShareOpen(false)} pageUrl={`${NEXT_PUBLIC_ORIGIN}/${article?.content_type === "article" ? "articles" : "news"}/${article?.slug}`} /> - - {(displayAuthorName || isEditable) && ( - - {isEditable ? ( - onAuthorNameChange?.(e.target.value)} - /> - ) : ( - By {displayAuthorName} - )} - {readTime ? {readTime} min read : null} - {readTime && publishedDate ? ( - - ) : null} - - {publishedDate - ? new Date(publishedDate).toLocaleDateString("en-US", { - month: "short", - day: "numeric", - year: "numeric", - }) - : isEditable - ? null - : "Draft"} - - - )} + + + {isEditable ? ( + onAuthorNameChange?.(e.target.value)} + /> + ) : ( + <> + {displayAuthorName && By {displayAuthorName}} + + )} + {readTime ? {readTime} min read : null} + {readTime && publishedDate ? ( + + ) : null} + + {publishedDate + ? new Date(publishedDate).toLocaleDateString("en-US", { + month: "short", + day: "numeric", + year: "numeric", + }) + : isEditable + ? null + : "Draft"} + +
dict: "title": article.title, "slug": article.slug, "content": article.content, + "cover_image": article.cover_image, "user": article.user, "created_on": article.created_on, "updated_on": article.updated_on, @@ -83,6 +84,7 @@ def extract() -> list[dict]: "title": article.title, "slug": article.slug, "content": article.content, + "cover_image": article.cover_image, "user": article.user, "created_on": article.created_on, "updated_on": article.updated_on, @@ -114,10 +116,13 @@ def transform_items(articles_data: list[dict]) -> list[dict]: # Convert JSON content to plain text for full content content_text = extract_text_from_content(content_json) - # Extract first image from content - image_data = extract_image_from_content(content_json) + cover_image_url = article.get("cover_image", "") + image_data = ( + {"url": cover_image_url, "alt": "", "description": ""} + if cover_image_url + else None + ) - # Debug logging log.info( "Article %s: Extracted image: %s", article.get("id"), @@ -268,104 +273,6 @@ def extract_summary_from_banner(content_json: dict) -> str: return _find_first_paragraph(content_array) -def extract_image_from_content(content_json: dict) -> dict | None: # noqa: C901 - """ - Extract the first image from JSON content structure. - - Args: - content_json (dict): The JSON content from Article - - Returns: - dict | None: Image data dict with url, alt, description or None - """ - if not content_json: - return None - - def traverse_for_image(node): # noqa: C901, PLR0911, PLR0912 - """Recursively traverse JSON structure to find first image""" - if not node: - return None - - # Handle dict nodes - if isinstance(node, dict): - # Check if this node is an image node (common patterns) - - # ProseMirror imageWithCaption (your format) - if node.get("type") == "imageWithCaption": - attrs = node.get("attrs", {}) - if attrs.get("src"): - return { - "url": attrs["src"], - "alt": attrs.get("alt", attrs.get("title", "")), - "description": attrs.get( - "caption", attrs.get("alt", attrs.get("title", "")) - ), - } - - # ProseMirror image node - if node.get("type") == "image": - attrs = node.get("attrs", {}) - if attrs.get("src"): - return { - "url": attrs["src"], - "alt": attrs.get("alt", attrs.get("title", "")), - "description": attrs.get("alt", attrs.get("title", "")), - } - # EditorJS image block - data = node.get("data", {}) - if data.get("file", {}).get("url"): - return { - "url": data["file"]["url"], - "alt": data.get("caption", ""), - "description": data.get("caption", ""), - } - - # Direct image node with url - if "image" in node or "img" in node: - img_data = node.get("image") or node.get("img") - if isinstance(img_data, dict) and img_data.get("url"): - return { - "url": img_data["url"], - "alt": img_data.get("alt", ""), - "description": img_data.get( - "description", img_data.get("alt", "") - ), - } - elif isinstance(img_data, str): - return { - "url": img_data, - "alt": "", - "description": "", - } - - # Check for url field that might be an image - if node.get("url") and any( - ext in str(node.get("url", "")).lower() - for ext in [".jpg", ".jpeg", ".png", ".gif", ".webp", ".svg"] - ): - return { - "url": node["url"], - "alt": node.get("alt", ""), - "description": node.get("description", node.get("caption", "")), - } - - # Traverse nested dict values - for value in node.values(): - result = traverse_for_image(value) - if result: - return result - # Handle list nodes - elif isinstance(node, list): - for item in node: - result = traverse_for_image(item) - if result: - return result - - return None - - return traverse_for_image(content_json) - - def extract_text_from_content(content_json: dict) -> str: # noqa: C901 """ Extract plain text from JSON content structure. diff --git a/news_events/etl/articles_news_test.py b/news_events/etl/articles_news_test.py index 617dec88c6..5a52cc2837 100644 --- a/news_events/etl/articles_news_test.py +++ b/news_events/etl/articles_news_test.py @@ -3,6 +3,7 @@ import pytest from news_events.etl import articles_news +from website_content.utils import extract_image_from_content @pytest.fixture @@ -163,7 +164,7 @@ def test_extract_image_from_content(): ], } - result = articles_news.extract_image_from_content(content_json) + result = extract_image_from_content(content_json) assert result is not None assert result["url"] == "http://example.com/image.webp" @@ -187,72 +188,24 @@ def test_extract_image_from_prosemirror_image(): ], } - result = articles_news.extract_image_from_content(content_json) + result = extract_image_from_content(content_json) assert result is not None assert result["url"] == "http://example.com/photo.jpg" assert result["alt"] == "Photo alt text" -def test_extract_image_editorjs_format(): - """Test image extraction from EditorJS format""" - # EditorJS style image - content_json = { - "blocks": [ - {"type": "paragraph", "text": "Some text"}, - { - "type": "image", - "data": { - "file": {"url": "https://example.com/image.jpg"}, - "caption": "Test image caption", - }, - }, - ] - } - - result = articles_news.extract_image_from_content(content_json) - - assert result is not None - assert result["url"] == "https://example.com/image.jpg" - assert result["alt"] == "Test image caption" - assert result["description"] == "Test image caption" - - -def test_extract_image_from_nested_structure(): - """Test image extraction from nested JSON structure""" - content_json = { - "content": { - "blocks": [ - { - "type": "media", - "image": { - "url": "https://example.com/nested.png", - "alt": "Nested image", - "description": "A nested image", - }, - } - ] - } - } - - result = articles_news.extract_image_from_content(content_json) - - assert result is not None - assert result["url"] == "https://example.com/nested.png" - assert result["alt"] == "Nested image" - - def test_extract_image_returns_none_when_no_image(): """Test image extraction returns None when no image found""" content_json = {"blocks": [{"type": "paragraph", "text": "Just text, no images"}]} - result = articles_news.extract_image_from_content(content_json) + result = extract_image_from_content(content_json) assert result is None - result = articles_news.extract_image_from_content({}) + result = extract_image_from_content({}) assert result is None - result = articles_news.extract_image_from_content(None) + result = extract_image_from_content(None) assert result is None @@ -559,6 +512,7 @@ class MockArticle: title = "Test Article" slug = "test-article" content = {"blocks": [{"text": "Test content"}]} + cover_image = "" created_on = datetime(2024, 1, 1, 0, 0, 0, tzinfo=UTC) updated_on = datetime(2024, 1, 1, 0, 0, 0, tzinfo=UTC) publish_date = datetime(2024, 1, 1, 0, 0, 0, tzinfo=UTC) @@ -632,6 +586,7 @@ class MockArticle: {"type": "paragraph", "content": [{"type": "text", "text": "Test"}]} ], } + cover_image = "" is_published = True content_type = "news" created_on = datetime(2024, 1, 1, 0, 0, 0, tzinfo=UTC) diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 3eb33e5169..84e12c516e 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -17402,7 +17402,14 @@ components: type: string maxLength: 60 pattern: ^[-a-zA-Z0-9_]+$ + cover_image: + type: string + format: uri + readOnly: true + default: '' + maxLength: 2083 required: + - cover_image - created_on - id - publish_date diff --git a/website_content/management/commands/backpopulate_cover_images.py b/website_content/management/commands/backpopulate_cover_images.py new file mode 100644 index 0000000000..addd3c1a96 --- /dev/null +++ b/website_content/management/commands/backpopulate_cover_images.py @@ -0,0 +1,55 @@ +"""Management command to backpopulate cover_image on existing WebsiteContent records.""" + +from django.core.management.base import BaseCommand + +from website_content.models import WebsiteContent +from website_content.utils import extract_image_from_content + + +class Command(BaseCommand): + """Derive and store cover_image for all WebsiteContent records that lack one.""" + + help = "Backpopulate cover_image from content JSON for all WebsiteContent records" + + def add_arguments(self, parser): + """Add --all flag to process records that already have a cover_image.""" + parser.add_argument( + "--all", + action="store_true", + dest="process_all", + help="Re-derive cover_image even for records that already have one set", + ) + + def handle(self, *args, **options): # noqa: ARG002 + """Iterate over WebsiteContent records and write derived cover_image values.""" + qs = WebsiteContent.objects.all() + if not options["process_all"]: + qs = qs.filter(cover_image="") + + total = qs.count() + if total == 0: + self.stdout.write("No records to update.") + return + + self.stdout.write(f"Backpopulating cover_image for {total} record(s)...") + + updated = 0 + skipped = 0 + + for obj in qs.only("pk", "content").iterator(): + image_data = extract_image_from_content(obj.content) + url = image_data.get("url", "") if image_data else "" + + WebsiteContent.objects.filter(pk=obj.pk).update(cover_image=url) + + if url: + updated += 1 + else: + skipped += 1 + + self.stdout.write( + self.style.SUCCESS( + f"Done. {updated} record(s) updated with an image URL, " + f"{skipped} record(s) had no extractable image." + ) + ) diff --git a/website_content/migrations/0005_add_cover_image_to_websitecontent.py b/website_content/migrations/0005_add_cover_image_to_websitecontent.py new file mode 100644 index 0000000000..3318fe25c6 --- /dev/null +++ b/website_content/migrations/0005_add_cover_image_to_websitecontent.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.30 on 2026-06-08 09:17 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("website_content", "0004_alter_websitecontent_created_on"), + ] + + operations = [ + migrations.AddField( + model_name="websitecontent", + name="cover_image", + field=models.URLField(blank=True, default="", max_length=2083), + ), + ] diff --git a/website_content/models.py b/website_content/models.py index 2ad3198582..ab1469a973 100644 --- a/website_content/models.py +++ b/website_content/models.py @@ -7,7 +7,10 @@ from main.models import TimestampedModel from website_content.constants import WebsiteContentType -from website_content.utils import website_content_image_upload_uri +from website_content.utils import ( + extract_image_from_content, + website_content_image_upload_uri, +) class WebsiteContent(TimestampedModel): @@ -35,8 +38,10 @@ class WebsiteContent(TimestampedModel): choices=WebsiteContentType.as_tuple(), default=WebsiteContentType.news.name, ) + cover_image = models.URLField(max_length=2083, blank=True, default="") def save(self, *args, **kwargs): + """Auto-populate slug and cover_image before persisting.""" previous = WebsiteContent.objects.get(pk=self.pk) if self.pk else None was_published = getattr(previous, "is_published", None) @@ -58,6 +63,9 @@ def save(self, *args, **kwargs): counter += 1 self.slug = slug + image_data = extract_image_from_content(self.content) + self.cover_image = image_data.get("url", "") if image_data else "" + super().save(*args, **kwargs) def get_url(self): @@ -72,6 +80,8 @@ def get_url(self): class WebsiteContentImageUpload(models.Model): + """Tracks image files uploaded through the website content editor.""" + user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE) image_file = models.ImageField( null=True, @@ -82,4 +92,5 @@ class WebsiteContentImageUpload(models.Model): created_at = models.DateTimeField(auto_now_add=True) def __str__(self): + """Return a string representation.""" return f"WebsiteContentImageUpload({self.user_id})" diff --git a/website_content/models_test.py b/website_content/models_test.py index 7610432994..6f272cec67 100644 --- a/website_content/models_test.py +++ b/website_content/models_test.py @@ -114,3 +114,110 @@ def test_content_type_defaults_to_news( ) assert content.content_type == WebsiteContentType.news.name + + def test_cover_image_set_from_content_on_create( + self, + _mock_queue_purge_delay, # noqa: PT019 + _mock_purge_url, # noqa: PT019 + _mock_queue_list, # noqa: PT019 + ): + """cover_image is derived from the first image node when the record is created.""" + user = User.objects.create_user(username="testuser6", email="test6@example.com") + content = WebsiteContent.objects.create( + title="Has Image", + content={ + "type": "doc", + "content": [ + { + "type": "imageWithCaption", + "attrs": {"src": "https://example.com/first.jpg", "alt": ""}, + } + ], + }, + user=user, + ) + + assert content.cover_image == "https://example.com/first.jpg" + + def test_cover_image_empty_when_no_image_in_content( + self, + _mock_queue_purge_delay, # noqa: PT019 + _mock_purge_url, # noqa: PT019 + _mock_queue_list, # noqa: PT019 + ): + """cover_image is empty string when the content contains no image nodes.""" + user = User.objects.create_user(username="testuser7", email="test7@example.com") + content = WebsiteContent.objects.create( + title="No Image", + content={"type": "doc", "content": []}, + user=user, + ) + + assert content.cover_image == "" + + def test_cover_image_updates_when_content_image_changes( + self, + _mock_queue_purge_delay, # noqa: PT019 + _mock_purge_url, # noqa: PT019 + _mock_queue_list, # noqa: PT019 + ): + """cover_image is re-derived on every save, so swapping the image is reflected.""" + user = User.objects.create_user(username="testuser8", email="test8@example.com") + content = WebsiteContent.objects.create( + title="Changing Image", + content={ + "type": "doc", + "content": [ + { + "type": "imageWithCaption", + "attrs": {"src": "https://example.com/original.jpg", "alt": ""}, + } + ], + }, + user=user, + ) + assert content.cover_image == "https://example.com/original.jpg" + + content.content = { + "type": "doc", + "content": [ + { + "type": "imageWithCaption", + "attrs": {"src": "https://example.com/updated.jpg", "alt": ""}, + } + ], + } + content.save() + + assert content.cover_image == "https://example.com/updated.jpg" + + def test_cover_image_cleared_when_image_removed_from_content( + self, + _mock_queue_purge_delay, # noqa: PT019 + _mock_purge_url, # noqa: PT019 + _mock_queue_list, # noqa: PT019 + ): + """cover_image is cleared to '' when the author removes all images from content.""" + user = User.objects.create_user(username="testuser9", email="test9@example.com") + content = WebsiteContent.objects.create( + title="Image Removed", + content={ + "type": "doc", + "content": [ + { + "type": "imageWithCaption", + "attrs": { + "src": "https://example.com/soon-gone.jpg", + "alt": "", + }, + } + ], + }, + user=user, + ) + assert content.cover_image == "https://example.com/soon-gone.jpg" + + content.content = {"type": "doc", "content": []} + content.save() + + assert content.cover_image == "" diff --git a/website_content/serializers.py b/website_content/serializers.py index be9f4308d9..1a737e8a77 100644 --- a/website_content/serializers.py +++ b/website_content/serializers.py @@ -36,6 +36,9 @@ class WebsiteContentSerializer(serializers.ModelSerializer): content = serializers.JSONField(default=dict) slug = serializers.SlugField(max_length=60, required=False, allow_blank=True) title = serializers.CharField(max_length=255) + cover_image = serializers.URLField( + max_length=2083, allow_blank=True, default="", read_only=True + ) author_name = serializers.CharField(required=False, allow_blank=True, default="") user = UserSerializer(read_only=True) content_type = serializers.ChoiceField( @@ -58,6 +61,7 @@ class Meta: "publish_date", "is_published", "slug", + "cover_image", ] diff --git a/website_content/utils.py b/website_content/utils.py index 190f5c9022..97841ed243 100644 --- a/website_content/utils.py +++ b/website_content/utils.py @@ -1,10 +1,113 @@ """website_content utilities""" +import logging +from typing import Any, NotRequired, TypedDict + from main.utils import generate_filepath +log = logging.getLogger(__name__) + + +class ProseMirrorNode(TypedDict): + """Minimal structural type for a ProseMirror/Tiptap JSON node. + + `attrs` and `content` are both optional: leaf nodes (text, image, mediaEmbed) + typically have attrs but no content; container nodes (doc, paragraph, banner) + have content but may have no attrs. + """ + + type: str + attrs: NotRequired[dict[str, Any]] + content: NotRequired[list["ProseMirrorNode"]] + def website_content_image_upload_uri(_, filename): """ upload_to handler for WebsiteContentImageUpload.image_file """ return generate_filepath(filename, "", "", "website_content") + + +def _traverse_for_image(node: ProseMirrorNode) -> dict | None: + node_type = node.get("type") + attrs = node.get("attrs", {}) + + if node_type == "imageWithCaption" and attrs.get("src"): + return { + "url": attrs["src"], + "alt": attrs.get("alt", attrs.get("title", "")), + "description": attrs.get( + "caption", attrs.get("alt", attrs.get("title", "")) + ), + } + + if node_type == "image" and attrs.get("src"): + return { + "url": attrs["src"], + "alt": attrs.get("alt", attrs.get("title", "")), + "description": attrs.get("alt", attrs.get("title", "")), + } + + for child in node.get("content", []): + result = _traverse_for_image(child) + if result: + return result + + return None + + +def _image_from_media_embed(attrs: dict) -> dict | None: + mit_learn_id = attrs.get("mitLearnVideoId") + if not mit_learn_id: + return None + + from learning_resources.models import Video + + video = Video.objects.filter(learning_resource_id=mit_learn_id).first() + if video is None: + log.warning( + "mediaEmbed references mitLearnVideoId=%s but no Video was found", + mit_learn_id, + ) + return None + + if video.cover_image_url: + return {"url": video.cover_image_url, "alt": "", "description": ""} + + return None + + +def _traverse_for_embed_cover(node: ProseMirrorNode) -> dict | None: + if node.get("type") == "mediaEmbed": + result = _image_from_media_embed(node.get("attrs", {})) + if result: + return result + for child in node.get("content", []): + result = _traverse_for_embed_cover(child) + if result: + return result + return None + + +def extract_image_from_content(content_json: dict) -> dict | None: + """ + Extract the first image from a ProseMirror/Tiptap JSON document. + + Checks inline image nodes first (imageWithCaption, image). If none are + found, falls back to mediaEmbed nodes: for MIT Learn videos, looks up + the resource's cover_image_url via the DB using the stored mitLearnVideoId. + + Args: + content_json: The JSON content from a WebsiteContent record. + + Returns: + dict | None: Image data dict with url, alt, description or None + """ + if not content_json: + return None + + image = _traverse_for_image(content_json) + if image: + return image + + return _traverse_for_embed_cover(content_json) diff --git a/website_content/utils_test.py b/website_content/utils_test.py new file mode 100644 index 0000000000..d3e5bfe7bb --- /dev/null +++ b/website_content/utils_test.py @@ -0,0 +1,164 @@ +"""Tests for website_content utilities""" + +from website_content.utils import extract_image_from_content + +MIT_LEARN_EMBED_URL = "https://rc.learn.mit.edu/video/123/embed" + + +class TestExtractImageFromContentImageNodes: + """Tests for inline image node extraction in extract_image_from_content.""" + + def test_returns_image_with_caption_src(self): + """Extracts src, alt, and caption from an imageWithCaption node.""" + content = { + "type": "doc", + "content": [ + { + "type": "imageWithCaption", + "attrs": { + "src": "https://example.com/photo.jpg", + "alt": "A photo", + "caption": "The caption", + }, + } + ], + } + result = extract_image_from_content(content) + assert result == { + "url": "https://example.com/photo.jpg", + "alt": "A photo", + "description": "The caption", + } + + def test_returns_image_node_src(self): + """Extracts src and alt from a plain ProseMirror image node.""" + content = { + "type": "doc", + "content": [ + { + "type": "image", + "attrs": {"src": "https://example.com/img.png", "alt": "Alt text"}, + } + ], + } + result = extract_image_from_content(content) + assert result == { + "url": "https://example.com/img.png", + "alt": "Alt text", + "description": "Alt text", + } + + def test_image_node_takes_priority_over_media_embed(self, mocker): + """An imageWithCaption earlier in the doc wins; the DB is never queried.""" + mock_get = mocker.patch( + "learning_resources.models.LearningResource.objects.get" + ) + content = { + "type": "doc", + "content": [ + { + "type": "imageWithCaption", + "attrs": {"src": "https://example.com/real.jpg", "alt": "real"}, + }, + { + "type": "mediaEmbed", + "attrs": {"mitLearnVideoId": 123, "src": MIT_LEARN_EMBED_URL}, + }, + ], + } + result = extract_image_from_content(content) + assert result["url"] == "https://example.com/real.jpg" + mock_get.assert_not_called() + + def test_returns_none_for_empty_content(self): + """Returns None for empty dict and None input.""" + assert extract_image_from_content({}) is None + assert extract_image_from_content(None) is None + + def test_returns_none_when_no_image_nodes(self): + """Returns None when the document contains only text, no images.""" + content = { + "type": "doc", + "content": [ + { + "type": "paragraph", + "content": [{"type": "text", "text": "Just text"}], + } + ], + } + assert extract_image_from_content(content) is None + + +class TestExtractImageFromContentMediaEmbed: + """Tests for the mediaEmbed fallback path in extract_image_from_content.""" + + def _doc(self, *nodes): + return {"type": "doc", "content": list(nodes)} + + def _media_embed(self, **attrs): + return {"type": "mediaEmbed", "attrs": attrs} + + def test_returns_cover_image_from_mit_learn_video_id(self, mocker): + """Looks up cover_image_url from the Video record when mitLearnVideoId is set.""" + mock_video = mocker.Mock() + mock_video.cover_image_url = "https://cdn.example.com/thumb.png" + mocker.patch( + "learning_resources.models.Video.objects.filter", + return_value=mocker.Mock(first=mocker.Mock(return_value=mock_video)), + ) + content = self._doc( + self._media_embed(mitLearnVideoId=123, src=MIT_LEARN_EMBED_URL) + ) + + result = extract_image_from_content(content) + + assert result == { + "url": "https://cdn.example.com/thumb.png", + "alt": "", + "description": "", + } + + def test_returns_none_when_video_not_found(self, mocker): + """Returns None gracefully when no Video record exists for the given ID.""" + mocker.patch( + "learning_resources.models.Video.objects.filter", + return_value=mocker.Mock(first=mocker.Mock(return_value=None)), + ) + content = self._doc( + self._media_embed(mitLearnVideoId=999, src=MIT_LEARN_EMBED_URL) + ) + + assert extract_image_from_content(content) is None + + def test_returns_none_when_no_mit_learn_video_id(self): + """Returns None for a mediaEmbed with no mitLearnVideoId (e.g. a YouTube embed).""" + content = self._doc(self._media_embed(src="https://www.youtube.com/embed/abc")) + assert extract_image_from_content(content) is None + + def test_finds_media_embed_nested_inside_content(self, mocker): + """Finds a mediaEmbed node even when it is nested inside another container node.""" + mock_video = mocker.Mock() + mock_video.cover_image_url = "https://cdn.example.com/deep-thumb.png" + mocker.patch( + "learning_resources.models.Video.objects.filter", + return_value=mocker.Mock(first=mocker.Mock(return_value=mock_video)), + ) + content = { + "type": "doc", + "content": [ + { + "type": "section", + "content": [ + self._media_embed(mitLearnVideoId=123, src=MIT_LEARN_EMBED_URL) + ], + } + ], + } + + result = extract_image_from_content(content) + + assert result == { + "url": "https://cdn.example.com/deep-thumb.png", + "alt": "", + "description": "", + } From 29dfb6589f63c58b03eee9729231e01dd30a96e8 Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Date: Wed, 10 Jun 2026 15:15:12 +0500 Subject: [PATCH 2/4] feat: add surrogate-key in the header for Fastly (#3393) * feat: add surrogate-key in the header for fastly * fix: move middlewares to proxy and some improvements * fix: courses/p/ are actually programs --- frontends/main/src/proxy.test.ts | 123 ++++++++++++++++++++++++++++++- frontends/main/src/proxy.ts | 81 +++++++++++++++++++- 2 files changed, 197 insertions(+), 7 deletions(-) diff --git a/frontends/main/src/proxy.test.ts b/frontends/main/src/proxy.test.ts index 1c392bd9b3..598c1b77f7 100644 --- a/frontends/main/src/proxy.test.ts +++ b/frontends/main/src/proxy.test.ts @@ -1,5 +1,5 @@ import { NextRequest } from "next/server" -import { isPageRoute, proxy } from "./proxy" +import { isPageRoute, mitxonlineSurrogateKey, proxy } from "./proxy" describe("isPageRoute", () => { test.each([ @@ -33,19 +33,136 @@ describe("isPageRoute", () => { }) }) +describe("mitxonlineSurrogateKey", () => { + describe("course pages — /courses/:readable_id", () => { + it("returns mitxonline:course key", () => { + expect( + mitxonlineSurrogateKey("/courses/course-v1:MITx+6.00.1x+3T2019"), + ).toBe("mitxonline:course:course-v1:MITx+6.00.1x+3T2019") + }) + + it("handles URL-encoded readable_id", () => { + const encoded = encodeURIComponent("course-v1:MITx+6.00.1x+3T2019") + expect(mitxonlineSurrogateKey(`/courses/${encoded}`)).toBe( + "mitxonline:course:course-v1:MITx+6.00.1x+3T2019", + ) + }) + }) + + describe("program pages — /programs/:readable_id and /courses/p/:readable_id", () => { + it("returns mitxonline:program key for /programs/:readable_id", () => { + expect(mitxonlineSurrogateKey("/programs/program-v1:MITx+SDS")).toBe( + "mitxonline:program:program-v1:MITx+SDS", + ) + }) + + it("returns mitxonline:program key for /courses/p/:readable_id (ProgramAsCoursePage)", () => { + // /courses/p/ renders ProgramAsCoursePage — the readable_id belongs to a + // program, so the surrogate key must use the program namespace so that + // MITxOnline's program-save signal purges this page correctly. + expect(mitxonlineSurrogateKey("/courses/p/program-v1:MITx+SDS")).toBe( + "mitxonline:program:program-v1:MITx+SDS", + ) + }) + }) + + describe("non-product pages", () => { + it.each(["/", "/search", "/about", "/courses", "/programs"])( + "returns null for %s", + (pathname) => { + expect(mitxonlineSurrogateKey(pathname)).toBeNull() + }, + ) + }) + + describe("hostile URL segments — regression: Headers.set() crash on control characters", () => { + it("returns null without throwing on malformed percent-encoding in course path", () => { + // %GG is not valid percent-encoding — decodeURIComponent would throw URIError + expect(() => mitxonlineSurrogateKey("/courses/%GG")).not.toThrow() + expect(mitxonlineSurrogateKey("/courses/%GG")).toBeNull() + }) + + it("returns null without throwing on malformed percent-encoding in program path", () => { + expect(() => mitxonlineSurrogateKey("/programs/%GG")).not.toThrow() + expect(mitxonlineSurrogateKey("/programs/%GG")).toBeNull() + }) + + it("returns null on CRLF in course path (%0D%0A decodes to \\r\\n)", () => { + // Without safeDecodeSegment, passing this to Headers.set() would throw TypeError + expect(() => + mitxonlineSurrogateKey("/courses/foo%0D%0AX-Injected%3A+yes"), + ).not.toThrow() + expect( + mitxonlineSurrogateKey("/courses/foo%0D%0AX-Injected%3A+yes"), + ).toBeNull() + }) + + it("returns null on CRLF in program path", () => { + expect(() => + mitxonlineSurrogateKey("/programs/foo%0D%0AX-Injected%3A+yes"), + ).not.toThrow() + expect( + mitxonlineSurrogateKey("/programs/foo%0D%0AX-Injected%3A+yes"), + ).toBeNull() + }) + + it("returns null on null byte in course path", () => { + expect(() => mitxonlineSurrogateKey("/courses/foo%00bar")).not.toThrow() + expect(mitxonlineSurrogateKey("/courses/foo%00bar")).toBeNull() + }) + + it("returns null on null byte in program path", () => { + expect(() => mitxonlineSurrogateKey("/programs/foo%00bar")).not.toThrow() + expect(mitxonlineSurrogateKey("/programs/foo%00bar")).toBeNull() + }) + }) +}) + describe("proxy", () => { const makeRequest = (pathname: string) => new NextRequest(new URL(pathname, "https://learn.mit.edu")) - test("tags page routes with both Cache-Control and Surrogate-Key", () => { - const response = proxy(makeRequest("/courses/course-v1:MITxT+5.601x")) + test("tags generic page routes with Cache-Control and html-pages Surrogate-Key", () => { + const response = proxy(makeRequest("/about")) expect(response.headers.get("Surrogate-Key")).toBe("html-pages") expect(response.headers.get("Cache-Control")).toContain("s-maxage=") }) + test("appends per-item surrogate key for MITxOnline course pages", () => { + const response = proxy(makeRequest("/courses/course-v1:MITxT+5.601x")) + expect(response.headers.get("Surrogate-Key")).toBe( + "html-pages mitxonline:course:course-v1:MITxT+5.601x", + ) + expect(response.headers.get("Cache-Control")).toContain("s-maxage=") + }) + + test("appends per-item surrogate key for MITxOnline program pages (/programs/)", () => { + const response = proxy(makeRequest("/programs/program-v1:MITxT+18.01x")) + expect(response.headers.get("Surrogate-Key")).toBe( + "html-pages mitxonline:program:program-v1:MITxT+18.01x", + ) + }) + + test("appends mitxonline:program surrogate key for /courses/p/ (ProgramAsCoursePage)", () => { + const response = proxy(makeRequest("/courses/p/program-v1:MITxT+18.01x")) + expect(response.headers.get("Surrogate-Key")).toBe( + "html-pages mitxonline:program:program-v1:MITxT+18.01x", + ) + }) + test("leaves non-page routes untagged", () => { const response = proxy(makeRequest("/healthcheck")) expect(response.headers.get("Surrogate-Key")).toBeNull() expect(response.headers.get("Cache-Control")).toBeNull() }) + + test("does not throw on CRLF in course path and falls back to html-pages only", () => { + // Regression: without safeDecodeSegment, Headers.set() would throw TypeError + // when the decoded segment contains \r\n (%0D%0A). + expect(() => + proxy(makeRequest("/courses/foo%0D%0AX-Injected%3A+yes")), + ).not.toThrow() + const response = proxy(makeRequest("/courses/foo%0D%0AX-Injected%3A+yes")) + expect(response.headers.get("Surrogate-Key")).toBe("html-pages") + }) }) diff --git a/frontends/main/src/proxy.ts b/frontends/main/src/proxy.ts index 7dc95a2212..f03ca6e16e 100644 --- a/frontends/main/src/proxy.ts +++ b/frontends/main/src/proxy.ts @@ -34,6 +34,75 @@ export function isPageRoute(pathname: string): boolean { return true } +// Matches /courses/ — readable_ids contain colons and + but no slashes. +const COURSE_PATTERN = /^\/courses\/([^/?]+)$/ +// Matches /courses/p/ — despite the /courses/ prefix this is a +// *program* page (ProgramAsCoursePage); the readable_id is a program readable_id. +const PROGRAM_AS_COURSE_PATTERN = /^\/courses\/p\/([^/?]+)$/ +// Matches /programs/ +const PROGRAM_PATTERN = /^\/programs\/([^/?]+)$/ + +/** + * Safely decode a URL path segment for use in a response header value. + * + * Returns null instead of throwing when: + * - percent-encoding is malformed (decodeURIComponent would throw URIError) + * - the decoded value contains characters invalid in HTTP header values + * (control characters, \r, \n, \0) which would cause Headers.set() to throw + */ +function safeDecodeSegment(segment: string): string | null { + let decoded: string + try { + decoded = decodeURIComponent(segment) + } catch { + return null + } + // HTTP header values must only contain visible ASCII + SP + HT (RFC 7230 §3.2.6). + // Reject anything with control chars (\x00-\x1F except \x09, or \x7F) to prevent + // header injection and avoid Headers.set() throwing. + // eslint-disable-next-line no-control-regex + if (/[\x00-\x08\x0A-\x1F\x7F]/.test(decoded)) { + return null + } + return decoded +} + +/** + * Derives a per-item MITxOnline Surrogate-Key tag from the request path, or + * returns null if the path is not a MITxOnline course or program page. + * + * Key format (matches what MITxOnline purges): + * mitxonline:course: — /courses/ + * mitxonline:program: — /programs/ + * — /courses/p/ + * (/courses/p/ renders ProgramAsCoursePage; + * the readable_id is a program readable_id) + * + * NOTE: Surrogate-Key headers must be set here rather than in page.tsx because + * Next.js commits response headers before any page/layout code runs (to begin + * streaming). By the time page.tsx executes, headers have already been sent. + */ +export function mitxonlineSurrogateKey(pathname: string): string | null { + const courseMatch = COURSE_PATTERN.exec(pathname) + if (courseMatch) { + const readableId = safeDecodeSegment(courseMatch[1]) + if (readableId !== null) { + return `mitxonline:course:${readableId}` + } + } + + const programMatch = + PROGRAM_AS_COURSE_PATTERN.exec(pathname) ?? PROGRAM_PATTERN.exec(pathname) + if (programMatch) { + const readableId = safeDecodeSegment(programMatch[1]) + if (readableId !== null) { + return `mitxonline:program:${readableId}` + } + } + + return null +} + /** * Next.js proxy (formerly "middleware"): sets the Cache-Control header at * request time so that NEXT_CACHE_S_MAXAGE_SECONDS is read from the Kubernetes @@ -53,10 +122,14 @@ export function proxy(request: NextRequest) { const response = NextResponse.next() response.headers.set("Cache-Control", cacheControl) - // Tag all HTML/page routes so Fastly can purge them on deploy without also - // purging immutable /_next/static/ chunks. Driven by the same isPageRoute() - // test as Cache-Control above, so the tag and the cache policy never diverge. - response.headers.set("Surrogate-Key", "html-pages") + + // All page routes share the html-pages tag so Fastly can purge them on + // deploy. MITxOnline course/program pages additionally carry a per-item tag + // so MITxOnline can invalidate individual product pages on data change. + const itemKey = mitxonlineSurrogateKey(request.nextUrl.pathname) + const surrogateKey = itemKey ? `html-pages ${itemKey}` : "html-pages" + response.headers.set("Surrogate-Key", surrogateKey) + return response } From 2cc81fbb2c9175abf02a397e167bb3e438d66b4d Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 10 Jun 2026 10:04:52 -0400 Subject: [PATCH 3/4] Exclude MicroMasters FIN programs from ETL ingestion (#3458) --- learning_resources/etl/micromasters.py | 2 +- learning_resources/etl/micromasters_test.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/learning_resources/etl/micromasters.py b/learning_resources/etl/micromasters.py index 8f66e05cca..34ef0376e7 100644 --- a/learning_resources/etl/micromasters.py +++ b/learning_resources/etl/micromasters.py @@ -21,7 +21,7 @@ OFFERED_BY = {"code": OfferedBy.mitx.name} READABLE_ID_PREFIX = "micromasters-program-" -IGNORE_URLS = re.compile(r"/(dedp|scm)/") +IGNORE_URLS = re.compile(r"/(dedp|scm|fin)/") log = logging.getLogger(__name__) diff --git a/learning_resources/etl/micromasters_test.py b/learning_resources/etl/micromasters_test.py index 7e01b9b9ce..8bca07dace 100644 --- a/learning_resources/etl/micromasters_test.py +++ b/learning_resources/etl/micromasters_test.py @@ -191,6 +191,8 @@ def test_micromasters_transform(mock_micromasters_data, missing_url): ("http://example.com/program/1/url", False), ("http://example.com/dedp/program/1/url", True), ("http://example.com/scm/program/1/url", True), + ("http://example.com/fin/program/1/url", True), + ("http://example.com/finis/program/1/url", False), ], ) def test_micromasters_transform_ignores_urls( From c41d8ee6d32031e2fef3938a4b456c8bba656787 Mon Sep 17 00:00:00 2001 From: Doof Date: Wed, 10 Jun 2026 14:42:27 +0000 Subject: [PATCH 4/4] Release 0.71.4 --- RELEASE.rst | 7 +++++++ main/settings.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/RELEASE.rst b/RELEASE.rst index c62e14a74b..8503ffa875 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,13 @@ Release Notes ============= +Version 0.71.4 +-------------- + +- Exclude MicroMasters FIN programs from ETL ingestion (#3458) +- feat: add surrogate-key in the header for Fastly (#3393) +- fix: pick cover image of video if no image is inserted in article content (#3433) + Version 0.71.3 (Released June 10, 2026) -------------- diff --git a/main/settings.py b/main/settings.py index fbdea2f6b8..e2eb0ba623 100644 --- a/main/settings.py +++ b/main/settings.py @@ -35,7 +35,7 @@ from main.settings_pluggy import * # noqa: F403 from openapi.settings_spectacular import open_spectacular_settings -VERSION = "0.71.3" +VERSION = "0.71.4" log = logging.getLogger()