diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index edaf1ca459..5c26b4fd94 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -53,7 +53,7 @@ repos: pass_filenames: false always_run: true - repo: https://github.com/scop/pre-commit-shfmt - rev: v3.13.1-1 + rev: v3.14.1-1 hooks: - id: shfmt - repo: https://github.com/adrienverge/yamllint.git @@ -90,7 +90,7 @@ repos: - "config/keycloak/realms/ol-local-realm.json" additional_dependencies: ["gibberish-detector"] - repo: https://github.com/astral-sh/ruff-pre-commit - rev: "v0.15.21" + rev: "v0.16.8" hooks: - id: ruff-format - id: ruff @@ -118,12 +118,12 @@ repos: exclude: node_modules/ require_serial: false - repo: https://github.com/shellcheck-py/shellcheck-py - rev: v0.11.0.1 + rev: v0.11.0.1-1 hooks: - id: shellcheck args: ["--severity=warning"] - repo: https://github.com/zizmorcore/zizmor-pre-commit - rev: v1.29.0 + rev: v1.30.1 hooks: - id: zizmor args: [--no-progress, --min-severity=medium, --min-confidence=medium] diff --git a/RELEASE.rst b/RELEASE.rst index 852334a008..820f5acc3a 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,18 @@ Release Notes ============= +Version 0.81.2 +-------------- + +- Fix OLL archive content ingestion (#3973) +- Read the warehouse catalog and schema from settings (#4005) +- Fall back to the original image before the default image (#4007) +- feat(website-content): require SEO fields to publish, and align the published control bar (#4009) +- Do not follow redirects when fetching OVS transcripts (#4003) +- fix(learning_resources): gate summary/flashcards like content on ContentFileViewSet (#3999) +- fix(widgets): remove the unused RSS Feed widget type (#4000) +- Update pre-commit hooks and adapt to ruff 0.16 (#3993) + Version 0.81.1 -------------- diff --git a/docs/articles-cdn-purge.md b/docs/articles-cdn-purge.md index 985d344722..41c134ef5a 100644 --- a/docs/articles-cdn-purge.md +++ b/docs/articles-cdn-purge.md @@ -150,7 +150,7 @@ article = Article.objects.create( title="New Article", content={"type": "doc", "content": []}, is_published=True, - user=some_user + user=some_user, ) # CDN purge is automatically queued! @@ -168,7 +168,7 @@ You can manually trigger CDN purges: from articles.tasks import ( fastly_purge_relative_url, fastly_purge_articles_list, - fastly_full_purge + fastly_full_purge, ) # Purge a specific URL immediately (blocking) @@ -192,7 +192,7 @@ For backwards compatibility, the following aliases are available but deprecated: # Old names (still work but discouraged) from articles.tasks import ( queue_fastly_purge_articles_list, # Use fastly_purge_articles_list - queue_fastly_full_purge, # Use fastly_full_purge + queue_fastly_full_purge, # Use fastly_full_purge ) ``` @@ -260,6 +260,7 @@ All CDN purge operations are logged using Python's standard logging: ```python import logging + logger = logging.getLogger("fastly_purge") ``` diff --git a/docs/how-to/articles-to-news.md b/docs/how-to/articles-to-news.md index 0ab3bfe08b..22fb05507b 100644 --- a/docs/how-to/articles-to-news.md +++ b/docs/how-to/articles-to-news.md @@ -144,23 +144,27 @@ Article.objects.filter(is_published=True) ### 2. Transform ```python -[{ - 'title': 'MIT Learn Articles', - 'url': '/articles', - 'feed_type': 'news', - 'items': [{ - 'guid': 'article-1', - 'title': 'My Article', - 'url': '/articles/my-article', - 'summary': 'First 500 chars...', - 'content': 'Full text...', - 'detail': { - 'authors': ['John Doe'], - 'topics': [], - 'publish_date': '2024-01-01T00:00:00Z', - } - }] -}] +[ + { + "title": "MIT Learn Articles", + "url": "/articles", + "feed_type": "news", + "items": [ + { + "guid": "article-1", + "title": "My Article", + "url": "/articles/my-article", + "summary": "First 500 chars...", + "content": "Full text...", + "detail": { + "authors": ["John Doe"], + "topics": [], + "publish_date": "2024-01-01T00:00:00Z", + }, + } + ], + } +] ``` ### 3. Load @@ -190,21 +194,20 @@ The `extract_text_from_content()` function needs customization based on your JSO ```python def extract_text_from_content(content_json: dict) -> str: # For Draft.js - blocks = content_json.get('blocks', []) - return ' '.join([block.get('text', '') for block in blocks]) + blocks = content_json.get("blocks", []) + return " ".join([block.get("text", "") for block in blocks]) + # For ProseMirror def walk_nodes(node): - if node.get('type') == 'text': - return node.get('text', '') - children = node.get('content', []) - return ' '.join(walk_nodes(child) for child in children) + if node.get("type") == "text": + return node.get("text", "") + children = node.get("content", []) + return " ".join(walk_nodes(child) for child in children) + return walk_nodes(content_json) # For EditorJS - blocks = content_json.get('blocks', []) - return ' '.join([ - block.get('data', {}).get('text', '') - for block in blocks - ]) + blocks = content_json.get("blocks", []) + return " ".join([block.get("data", {}).get("text", "") for block in blocks]) ``` ### 2. Add Image Support @@ -235,7 +238,8 @@ If you add topics to your Article model: ```python class Article(TimestampedModel): # ... existing fields ... - topics = models.ManyToManyField('Topic') + topics = models.ManyToManyField("Topic") + # In transform_items: entry = { @@ -294,7 +298,7 @@ article = Article.objects.create( title="Test Article", content={"blocks": [{"text": "Test content"}]}, user=user, - is_published=True + is_published=True, ) ``` @@ -317,6 +321,7 @@ result = pipelines.articles_news_etl() # Check results from news_events.models import FeedSource + source = FeedSource.objects.get(title="MIT Learn Articles") print(f"Found {source.feed_items.count()} articles in news feed") ``` @@ -340,6 +345,7 @@ print(f"Found {source.feed_items.count()} articles in news feed") 3. **Check for errors:** ```python from news_events.tasks import get_articles_news + get_articles_news() # Run synchronously to see errors ``` @@ -350,6 +356,7 @@ print(f"Found {source.feed_items.count()} articles in news feed") - Add debug logging: ```python import logging + log = logging.getLogger(__name__) log.info(f"Content structure: {content_json}") ``` diff --git a/frontends/api/src/generated/v0/api.ts b/frontends/api/src/generated/v0/api.ts index bef5eb1592..6c81b6a9cc 100644 --- a/frontends/api/src/generated/v0/api.ts +++ b/frontends/api/src/generated/v0/api.ts @@ -2597,13 +2597,12 @@ export interface WidgetListRequest { widgets?: Array | null } /** - * * `Markdown` - Markdown * `URL` - URL * `RSS Feed` - RSS Feed * `People` - People + * * `Markdown` - Markdown * `URL` - URL * `People` - People */ export const WidgetTypeEnumDescriptions = { Markdown: "Markdown", URL: "URL", - "RSS Feed": "RSS Feed", People: "People", } as const @@ -2616,10 +2615,6 @@ export const WidgetTypeEnum = { * URL */ Url: "URL", - /** - * RSS Feed - */ - RssFeed: "RSS Feed", /** * People */ diff --git a/frontends/main/src/app-pages/ProductPages/MitxOnlineResourceCard.test.tsx b/frontends/main/src/app-pages/ProductPages/MitxOnlineResourceCard.test.tsx index 0c6e5413d9..23d6bcd1b9 100644 --- a/frontends/main/src/app-pages/ProductPages/MitxOnlineResourceCard.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/MitxOnlineResourceCard.test.tsx @@ -5,7 +5,7 @@ import { factories } from "api/mitxonline-test-utils" import { DisplayModeEnum } from "@mitodl/mitxonline-api-axios/v2" import { renderWithProviders } from "@/test-utils" import { DEFAULT_RESOURCE_IMG } from "ol-utilities" -import { getByImageSrc } from "ol-test-utilities" +import { getByImageSrc, queryByImageSrc } from "ol-test-utilities" import type { MitxOnlineResourceCardProps } from "./MitxOnlineResourceCard" const renderCard = (props: MitxOnlineResourceCardProps) => @@ -163,7 +163,7 @@ describe("MitxOnlineResourceCard", () => { }) describe("image error fallback", () => { - test("falls back to DEFAULT_RESOURCE_IMG when course image returns 404", () => { + test("falls back to the original, then DEFAULT_RESOURCE_IMG, when course image fails", () => { const course = factories.courses.course({ page: { feature_image_src: "https://example.com/course.jpg", @@ -175,13 +175,25 @@ describe("MitxOnlineResourceCard", () => { resourceType: "course", href: "/test", }) + const raw = { nextJsOriginalSrc: false } + expect( + queryByImageSrc(view.container, "https://example.com/course.jpg", raw), + ).toBeNull() + // Optimized image fails: retry the original, loaded directly fireEvent.error( getByImageSrc(view.container, "https://example.com/course.jpg"), ) + const original = getByImageSrc( + view.container, + "https://example.com/course.jpg", + raw, + ) + // Original fails too: use the default + fireEvent.error(original) getByImageSrc(view.container, DEFAULT_RESOURCE_IMG) }) - test("falls back to DEFAULT_RESOURCE_IMG when program image returns 404", () => { + test("falls back to the original, then DEFAULT_RESOURCE_IMG, when program image fails", () => { const program = factories.programs.program({ page: { feature_image_src: "https://example.com/program.jpg", @@ -193,9 +205,21 @@ describe("MitxOnlineResourceCard", () => { resourceType: "program", href: "/test", }) + const raw = { nextJsOriginalSrc: false } + expect( + queryByImageSrc(view.container, "https://example.com/program.jpg", raw), + ).toBeNull() + // Optimized image fails: retry the original, loaded directly fireEvent.error( getByImageSrc(view.container, "https://example.com/program.jpg"), ) + const original = getByImageSrc( + view.container, + "https://example.com/program.jpg", + raw, + ) + // Original fails too: use the default + fireEvent.error(original) getByImageSrc(view.container, DEFAULT_RESOURCE_IMG) }) }) diff --git a/frontends/main/src/app-pages/ProductPages/MitxOnlineResourceCard.tsx b/frontends/main/src/app-pages/ProductPages/MitxOnlineResourceCard.tsx index 106b63f1a1..bb5a18fdcb 100644 --- a/frontends/main/src/app-pages/ProductPages/MitxOnlineResourceCard.tsx +++ b/frontends/main/src/app-pages/ProductPages/MitxOnlineResourceCard.tsx @@ -151,7 +151,11 @@ const MitxOnlineResourceCard: React.FC = ( label, } = props - const { src: imageSrc, onError: onImageError } = useImageWithFallback( + const { + src: imageSrc, + unoptimized: imageUnoptimized, + onError: onImageError, + } = useImageWithFallback( props.resource?.page?.feature_image_src, DEFAULT_RESOURCE_IMG, ) @@ -180,6 +184,7 @@ const MitxOnlineResourceCard: React.FC = ( imageSrc={imageSrc} imageAlt="" onImageError={onImageError} + imageUnoptimized={imageUnoptimized} title={data.title} resourceType={data.displayType} resourcePrice={data.resourcePrice} diff --git a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx index 78bc07ca5a..e4bd457db7 100644 --- a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx @@ -12,7 +12,7 @@ import { PostHogEvents } from "@/common/constants" import type { ResourceInfo } from "./ProductPageTemplate" import { PlatformEnum } from "api" import { DEFAULT_RESOURCE_IMG } from "ol-utilities" -import { getAllByImageSrc } from "ol-test-utilities" +import { getAllByImageSrc, queryAllByImageSrc } from "ol-test-utilities" jest.mock("posthog-js/react", () => ({ ...jest.requireActual("posthog-js/react"), @@ -74,7 +74,7 @@ const renderProductPageTemplate = ( } describe("ProductPageTemplate image error fallback", () => { - it("falls back to DEFAULT_RESOURCE_IMG when imageSrc returns 404", () => { + it("falls back to the original image, then DEFAULT_RESOURCE_IMG, when imageSrc fails", () => { setMockResponse.get(urls.userMe.get(), { is_authenticated: false }) const { view } = renderWithProviders( { , ) - getAllByImageSrc(view.container, "https://example.com/image.jpg").forEach( - (img) => fireEvent.error(img), - ) + const src = "https://example.com/image.jpg" + const raw = { nextJsOriginalSrc: false } + expect(queryAllByImageSrc(view.container, src, raw)).toHaveLength(0) + // Optimized image fails: retry the original, loaded directly + getAllByImageSrc(view.container, src).forEach((img) => fireEvent.error(img)) + const originals = getAllByImageSrc(view.container, src, raw) + // Original fails too: use the default + originals.forEach((img) => fireEvent.error(img)) expect( getAllByImageSrc(view.container, DEFAULT_RESOURCE_IMG).length, ).toBeGreaterThan(0) diff --git a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx index 9d87d82b23..ae01e8d681 100644 --- a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx @@ -248,10 +248,11 @@ const SidebarMedia: React.FC<{ title: string priority?: boolean }> = ({ videoUrl, imageSrc, title, priority }) => { - const { src: resolvedSrc, onError } = useImageWithFallback( - imageSrc, - DEFAULT_RESOURCE_IMG, - ) + const { + src: resolvedSrc, + unoptimized, + onError, + } = useImageWithFallback(imageSrc, DEFAULT_RESOURCE_IMG) if (videoUrl) { const embedUrl = convertToEmbedUrl(videoUrl) @@ -268,6 +269,7 @@ const SidebarMedia: React.FC<{ height={306} src={resolvedSrc} alt="" + unoptimized={unoptimized} onError={onError} /> ) diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoCard.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoCard.tsx index c401c38e4c..6191d06ac6 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoCard.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoCard.tsx @@ -1,4 +1,4 @@ -import React, { useState } from "react" +import React from "react" import Image from "next/image" import Link from "next/link" import { @@ -8,7 +8,7 @@ import { Skeleton, type TypographyProps, } from "ol-components" -import { formatDurationClockTime } from "ol-utilities" +import { formatDurationClockTime, useImageWithFallback } from "ol-utilities" import { stripAnchorTags } from "@/common/utils" import type { VideoResource } from "api/v1" import { @@ -126,10 +126,11 @@ type VideoCardProps = { } const VideoCard: React.FC = ({ resource, href }) => { - const [imgError, setImgError] = useState(false) - const imageUrl = !imgError - ? (resource?.image?.url ?? PLACEHOLDER_IMG) - : PLACEHOLDER_IMG + const { + src: imageUrl, + unoptimized, + onError, + } = useImageWithFallback(resource?.image?.url, PLACEHOLDER_IMG) const description = resource.description ?? "" const duration = resource.video?.duration ? formatDurationClockTime(resource.video.duration) @@ -143,7 +144,8 @@ const VideoCard: React.FC = ({ resource, href }) => { alt={resource.title} fill sizes="160px" - onError={() => setImgError(true)} + unoptimized={unoptimized} + onError={onError} /> {duration && {duration}} diff --git a/frontends/main/src/app-pages/WebsiteContent/WebsiteContentEditPage.happydom.test.tsx b/frontends/main/src/app-pages/WebsiteContent/WebsiteContentEditPage.happydom.test.tsx index ee2d814956..ab2dab81ba 100644 --- a/frontends/main/src/app-pages/WebsiteContent/WebsiteContentEditPage.happydom.test.tsx +++ b/frontends/main/src/app-pages/WebsiteContent/WebsiteContentEditPage.happydom.test.tsx @@ -101,6 +101,9 @@ const setup = async (id: number, autosaveDelayMs = AUTOSAVE_OFF) => { content, content_type: "article", is_published: false, + /* Required to save the drawer, and not what these tests are about. */ + seo_title: "A title for search", + seo_description: "A description for search results.", }) setMockResponse.get(detailUrl(id), article) diff --git a/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.test.tsx b/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.test.tsx index e5132b36ae..1935e68c5b 100644 --- a/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.test.tsx +++ b/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.test.tsx @@ -187,7 +187,7 @@ describe("ArticleSettingsDrawer SEO fields", () => { mockTopics() const { onSave } = renderDrawer() - const field = await screen.findByLabelText("SEO Title") + const field = await screen.findByLabelText(/^SEO Title/) expect(field).toHaveAttribute("maxLength", "255") await userEvent.type(field, "x".repeat(260)) @@ -238,7 +238,7 @@ describe("ArticleSettingsDrawer SEO fields", () => { expect(under).toHaveAttribute("data-over-budget", "false") await userEvent.type( - await screen.findByLabelText("SEO Title"), + await screen.findByLabelText(/^SEO Title/), "x".repeat(50), ) diff --git a/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.tsx b/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.tsx index f38743cd91..f48999651a 100644 --- a/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.tsx +++ b/frontends/main/src/page-components/ArticleSettings/ArticleSettingsDrawer.tsx @@ -199,17 +199,21 @@ const FooterCta = styled.div({ /** * What the topics section says about itself, which depends on which rule is - * speaking: one that refuses to save without a topic, one that only wants them - * before publishing, or neither. + * speaking: content already public that cannot be left without them, content + * that only needs them before it goes public, or neither. + * + * Keyed on `published` rather than on whether the save is refused: the two no + * longer coincide -- a draft's save is refused too while a publish is waiting + * on the drawer -- and telling a draft it is published would be simply wrong. */ const topicsMessage = ( contentLabel: string, empty: boolean, required: boolean, - mayNotBeEmptied: boolean, + published: boolean, ) => { const noun = contentLabel.toLowerCase() - if (empty && mayNotBeEmptied) { + if (empty && published) { return `A published ${noun} needs at least one topic` } if (empty && required) { @@ -218,6 +222,31 @@ const topicsMessage = ( return `Select one or more topics for your ${noun}` } +/** + * What the SEO section says about itself, on the same three rules as topics. + * + * Named for what is missing rather than "these fields are required": the + * drawer opens on its own when a publish is held back, and the first thing + * the author needs to know is why it did. + */ +const seoMessage = ( + contentLabel: string, + missing: boolean, + required: boolean, + published: boolean, +) => { + const noun = contentLabel.toLowerCase() + const always = + "Both should be unique to this page, and they are the first thing someone reads in search results." + if (missing && published) { + return `A published ${noun} needs an SEO title and description. ${always}` + } + if (missing && required) { + return `Add an SEO title and description to publish your ${noun}. ${always}` + } + return `Add an SEO title and description to help search engines understand and display your ${noun}. ${always}` +} + /** Settings the drawer collects. Mirrors the fields in the design. */ export interface ArticleSettingsValues { /** @@ -261,21 +290,30 @@ export interface ArticleSettingsDrawerProps { */ showTopics?: boolean /** - * Whether the content needs a topic before it can go public. The section - * says so while none is picked, which is what tells an editor why the - * drawer opened on them when they pressed Publish. + * Whether the content needs at least one topic. + * + * The section says so while none is picked -- which is what tells an editor + * why the drawer opened on them when they pressed Publish -- and the save is + * refused until one is. Refused rather than merely announced because this + * drawer is the one place a selection can be taken away, and because a + * caller holding a publish back is waiting on this save: letting it through + * incomplete would close the drawer and forget the press. */ topicsRequired?: boolean /** - * Whether an empty selection may not be saved at all. + * Whether the content needs an SEO title and description, on exactly the + * same terms as `topicsRequired`. * - * This drawer is the one place a selection can be taken away, so a caller - * that gates only its own save buttons would still lose the topics through - * here. Separate from `topicsRequired` because the two do not coincide: - * content that is not public yet can be left without topics -- it is - * stopped at publishing -- while content already public cannot. + * Unlike topics this is not an article-only rule: a search result and a link + * preview are the editor's to write on news just as much. */ - topicsMayNotBeEmptied?: boolean + seoRequired?: boolean + /** + * Whether the content is already public, which is only a matter of wording: + * which sentence a section shows when something it needs is missing. What is + * required, and what the save refuses, does not depend on it. + */ + contentIsPublished?: boolean /** Values to open with. Re-read each time the drawer opens. */ initialValues?: Partial /** @@ -294,7 +332,8 @@ const ArticleSettingsDrawer = ({ contentLabel = "Article", showTopics = true, topicsRequired = false, - topicsMayNotBeEmptied = false, + seoRequired = false, + contentIsPublished = false, initialValues, onSave, }: ArticleSettingsDrawerProps) => { @@ -313,6 +352,13 @@ const ArticleSettingsDrawer = ({ * Read here rather than at module scope, where NEXT_PUBLIC_* values are not * set yet. Missing, there is no suffix to reserve for. */ + /** + * Whitespace does not count as provided: a space would satisfy a bare + * emptiness check and reach the page head as a blank title, which is worse + * than the fallback it displaced. + */ + const seoMissing = !seoTitle.trim() || !seoDescription.trim() + const siteName = env("NEXT_PUBLIC_SITE_NAME") const titleSuffix = siteName ? ` | ${siteName}` : "" const seoTitleBudget = SEO_TITLE_TAG_BUDGET - titleSuffix.length @@ -514,7 +560,7 @@ const ArticleSettingsDrawer = ({ contentLabel, selectedIds.length === 0, topicsRequired, - topicsMayNotBeEmptied, + contentIsPublished, )} @@ -600,10 +646,12 @@ const ArticleSettingsDrawer = ({ SEO Settings - Add an SEO title and description to help search engines - understand and display your {contentLabel.toLowerCase()}. Both - should be unique to this page, and they are the first thing - someone reads in search results. + {seoMessage( + contentLabel, + seoMissing, + seoRequired, + contentIsPublished, + )}
@@ -611,6 +659,7 @@ const ArticleSettingsDrawer = ({ name="seo_title" label="SEO Title" fullWidth + required={seoRequired} placeholder="Enter a title for search results" /* The budget belongs in the description, not only in the counter: otherwise it is discoverable only by being run @@ -635,6 +684,7 @@ const ArticleSettingsDrawer = ({ name="seo_description" label="SEO Description" fullWidth + required={seoRequired} multiline /* Sized to the budget rather than to the space: nine rows read as an invitation to write far more than will ever show. */ @@ -662,10 +712,16 @@ const ArticleSettingsDrawer = ({