Release 0.81.0

RELEASE.rst

@@ -1,6 +1,18 @@
 Release Notes
 =============
 
+Version 0.81.0
+--------------
+
+- Smooth FAQ accordion open/close animation (#3997)
+- feat(website-content): save drafts automatically, drop the draft button (#3978)
+- Posthog checkout_completed event (#3982)
+- fix: scope unsubscribe lookup to the requesting user's own subscriptions (#3984)
+- Sanitize MITPE news and events summary/content like sibling sources (#3977)
+- feat(website-content): unpublish published items from the listing card (#3963)
+- Re-ingest content files of republished runs, fix inconsistent contentfile (de)indexing  (#3976)
+- add back schedule and fix admin criteria text field (#3989)
+
 Version 0.80.18
 ---------------
 

frontends/api/src/hooks/website_content/index.test.ts

@@ -102,6 +102,19 @@ describe("Website Content CRUD", () => {
     expect(queryClient.invalidateQueries).toHaveBeenCalledWith({
       queryKey: websiteContentKeys.detail(article.id),
     })
+    expect(queryClient.invalidateQueries).toHaveBeenCalledWith({
+      queryKey: websiteContentKeys.websiteContentDetailRetrieve(
+        article.slug || String(article.id),
+      ),
+    })
+    /**
+     * The listings render the fields a patch can change -- title, summary and
+     * whether the item is published at all. Without this, a card unpublished
+     * from the listing stays on screen until something else refetches.
+     */
+    expect(queryClient.invalidateQueries).toHaveBeenCalledWith({
+      queryKey: websiteContentKeys.listRoot(),
+    })
   })
 
   test("useWebsiteContentDestroy calls correct API", async () => {

frontends/api/src/hooks/website_content/index.ts

@@ -136,6 +136,14 @@ const useWebsiteContentPartialUpdate = ({ meta }: MutationHookOptions = {}) => {
       client.invalidateQueries({
         queryKey: websiteContentKeys.websiteContentDetailRetrieve(identifier),
       })
+      /**
+       * The listings render the fields a patch can change -- title, summary
+       * and whether the item is published at all -- so they go stale too. The
+       * destroy hook already invalidates them for the same reason.
+       */
+      client.invalidateQueries({
+        queryKey: websiteContentKeys.listRoot(),
+      })
     },
   })
 }

frontends/main/src/app-pages/Articles/ArticleListingPage.test.tsx

@@ -0,0 +1,130 @@
+import React from "react"
+import { screen, waitFor } from "@testing-library/react"
+import userEvent from "@testing-library/user-event"
+import { setMockResponse, factories, urls, makeRequest } from "api/test-utils"
+import { renderWithProviders } from "@/test-utils"
+import { ArticleListingPage } from "./ArticleListingPage"
+
+const setup = ({
+  isArticleEditor = true,
+  isPublished = true,
+}: { isArticleEditor?: boolean; isPublished?: boolean } = {}) => {
+  const user = factories.user.user({
+    is_authenticated: true,
+    is_article_editor: isArticleEditor,
+  })
+  setMockResponse.get(urls.userMe.get(), user)
+
+  const article = factories.websiteContent.websiteContent({
+    id: 701,
+    title: "Breaking the old model of education",
+    content_type: "article",
+    is_published: isPublished,
+  })
+  setMockResponse.get(
+    urls.websiteContent.list({ limit: 10, offset: 0, content_type: "article" }),
+    { count: 1, next: null, previous: null, results: [article] },
+  )
+
+  renderWithProviders(<ArticleListingPage />, { user })
+  return { article }
+}
+
+const menuName = (title: string) => `More options for ${title}`
+
+/**
+ * The page renders its mobile and desktop layouts together and hides one with
+ * CSS, so every card is in the DOM twice. These queries take the first match
+ * rather than asserting a single one.
+ */
+const findMenuButtons = (title: string) =>
+  screen.findAllByRole("button", { name: menuName(title) })
+const queryMenuButtons = (title: string) =>
+  screen.queryAllByRole("button", { name: menuName(title) })
+
+describe("ArticleListingPage article actions", () => {
+  test("an editor can unpublish a published article from the card", async () => {
+    const { article } = setup()
+    setMockResponse.patch(urls.websiteContent.details(article.id), {
+      ...article,
+      is_published: false,
+    })
+
+    await userEvent.click((await findMenuButtons(article.title))[0])
+    await userEvent.click(
+      await screen.findByRole("menuitem", { name: "Unpublish" }),
+    )
+
+    // The menu only asks; the dialog owns the confirmation.
+    await screen.findByRole("heading", { name: "Unpublish article" })
+    await userEvent.click(
+      await screen.findByRole("button", { name: "Yes, Unpublish article" }),
+    )
+
+    await waitFor(() => {
+      expect(makeRequest).toHaveBeenCalledWith(
+        expect.objectContaining({
+          method: "patch",
+          url: urls.websiteContent.details(article.id),
+          body: { is_published: false },
+        }),
+      )
+    })
+  })
+
+  test("cancelling the dialog unpublishes nothing", async () => {
+    const { article } = setup()
+
+    await userEvent.click((await findMenuButtons(article.title))[0])
+    await userEvent.click(
+      await screen.findByRole("menuitem", { name: "Unpublish" }),
+    )
+    await userEvent.click(await screen.findByRole("button", { name: "Cancel" }))
+
+    expect(makeRequest).not.toHaveBeenCalledWith(
+      expect.objectContaining({ method: "patch" }),
+    )
+  })
+
+  test("the menu is hidden from users who cannot edit articles", async () => {
+    const { article } = setup({ isArticleEditor: false })
+
+    await screen.findAllByText(article.title)
+
+    expect(queryMenuButtons(article.title)).toHaveLength(0)
+  })
+
+  test("a draft has no menu, since unpublishing is the only action", async () => {
+    const { article } = setup({ isPublished: false })
+
+    await screen.findAllByText(article.title)
+
+    expect(queryMenuButtons(article.title)).toHaveLength(0)
+  })
+})
+
+describe("ArticleListingPage links", () => {
+  const hrefs = (title: string) =>
+    screen
+      .getAllByRole("link", { name: title })
+      .map((a) => a.getAttribute("href"))
+
+  test("a draft links to the editor, since it has no public page", async () => {
+    const { article } = setup({ isPublished: false })
+
+    await screen.findAllByText(article.title)
+
+    /* Every link on the card, so the title and the image cannot diverge. */
+    const unique = [...new Set(hrefs(article.title))]
+    expect(unique).toEqual([`/website_content/article/${article.id}/edit`])
+  })
+
+  test("a published article links to its public page", async () => {
+    const { article } = setup({ isPublished: true })
+
+    await screen.findAllByText(article.title)
+
+    const unique = [...new Set(hrefs(article.title))]
+    expect(unique).toEqual([`/articles/${article.slug ?? article.id}`])
+  })
+})

frontends/main/src/app-pages/Articles/ArticleListingPage.tsx

@@ -19,13 +19,19 @@ import {
 } from "ol-components"
 import Link from "next/link"
 import { RiArrowLeftLine, RiArrowRightLine } from "@remixicon/react"
-import type { WebsiteContent } from "api/v1"
+import { WebsiteContentContentTypeEnum, type WebsiteContent } from "api/v1"
 import { LocalDate } from "ol-utilities"
 import { useWebsiteContentList } from "api/hooks/website_content"
 import { extractArticleContent } from "@/common/websiteContentUtils"
-import { articleView, websiteContentCreateView } from "@/common/urls"
+import { CONTENT_TYPE_LABELS } from "@/common/website_content"
+import {
+  articleView,
+  websiteContentCreateView,
+  websiteContentEditView,
+} from "@/common/urls"
 import { Permission, useUserHasPermission } from "api/hooks/user"
 import { ButtonLink } from "@mitodl/smoot-design"
+import { WebsiteContentActionsMenu } from "@/page-components/WebsiteContentActionsMenu/WebsiteContentActionsMenu"
 
 const PAGE_SIZE = 10
 const MAX_PAGE = 50
@@ -57,6 +63,7 @@ const RegularStoryTitleWrapper = styled.div`
 const StoryCard = styled.div`
   display: flex;
   flex-direction: row;
+  position: relative;
   gap: 24px;
   background: white;
   border-radius: 8px;
@@ -92,6 +99,25 @@ const StoryCard = styled.div`
   }
 `
 
+/**
+ * Holds the three-dot menu over the card's top-right corner, where the design
+ * puts it: level with the top of the image, inside the card's 16px padding.
+ *
+ * A sibling of the image's link rather than a child of it, so clicking the
+ * menu cannot navigate to the article.
+ */
+const StoryActions = styled.div`
+  position: absolute;
+  top: 16px;
+  right: 16px;
+  z-index: 1;
+
+  ${theme.breakpoints.down("sm")} {
+    top: 16px;
+    right: 0;
+  }
+`
+
 const StoryImage = styled.div`
   width: 280px;
   min-width: 280px;
@@ -372,14 +398,33 @@ const BreadcrumContainer = styled(Container)(({ theme }) => ({
 const RegularStory: React.FC<{ item: WebsiteContent }> = ({ item }) => {
   const articleContent = extractArticleContent(item)
   const [imageError, setImageError] = React.useState(false)
+  /**
+   * An unpublished article has no public page to land on, so the card points
+   * at the editor instead. Only an editor ever sees one here: the listing
+   * endpoint filters unpublished items out for everyone else.
+   *
+   * By id rather than slug, which a draft may not have yet.
+   */
+  const href = item.is_published
+    ? articleView(item.slug ?? String(item.id))
+    : websiteContentEditView(WebsiteContentContentTypeEnum.Article, item.id)
   return (
     <StoryCard>
+      {/* A draft has nothing to unpublish; the menu hides itself from
+          users who cannot edit articles. */}
+      {item.is_published ? (
+        <StoryActions>
+          <WebsiteContentActionsMenu
+            contentId={item.id}
+            contentLabel={CONTENT_TYPE_LABELS.article}
+            title={item.title}
+          />
+        </StoryActions>
+      ) : null}
       <StoryContent>
         <RegularStoryTitleWrapper>
           <StoryTitle>
-            <Link href={articleView(item.slug ?? String(item.id))}>
-              {item.title}
-            </Link>
+            <Link href={href}>{item.title}</Link>
           </StoryTitle>
           {articleContent.paragraph && (
             <StorySummary
@@ -394,10 +439,7 @@ const RegularStory: React.FC<{ item: WebsiteContent }> = ({ item }) => {
         </StoryDate>
       </StoryContent>
       {articleContent?.image?.src && !imageError && (
-        <Link
-          href={articleView(item.slug ?? String(item.id))}
-          style={{ textDecoration: "none", order: 2 }}
-        >
+        <Link href={href} style={{ textDecoration: "none", order: 2 }}>
           <StoryImage>
             <Image
               src={articleContent.image.src}

frontends/main/src/app-pages/DashboardPage/EnrollmentRedirectAlert.test.tsx

@@ -10,16 +10,29 @@ import EnrollmentRedirectAlert from "./EnrollmentRedirectAlert"
 import { DASHBOARD_MY_LEARNING } from "@/common/urls"
 import * as mitxonline from "api/mitxonline-test-utils"
 import { trackCheckoutCompleted } from "@/common/analytics/gtm"
+import { usePostHog } from "posthog-js/react"
+import type { PostHog } from "posthog-js"
+import { PostHogEvents } from "@/common/constants"
 
 jest.mock("@/common/analytics/gtm", () => ({
   trackCheckoutCompleted: jest.fn(),
 }))
+jest.mock("posthog-js/react")
+const mockedPostHogCapture = jest.fn()
+jest.mocked(usePostHog).mockReturnValue({
+  capture: mockedPostHogCapture,
+} as unknown as PostHog)
 
 const escapeRegExp = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")
 
 describe("EnrollmentRedirectAlert", () => {
   beforeEach(() => {
     jest.clearAllMocks()
+    process.env.NEXT_PUBLIC_POSTHOG_API_KEY = "test-key"
+  })
+
+  afterEach(() => {
+    delete process.env.NEXT_PUBLIC_POSTHOG_API_KEY
   })
 
   test("shows invalid-enrollment-code error alert and clears params", async () => {
@@ -256,7 +269,11 @@ describe("EnrollmentRedirectAlert", () => {
 
   test("tracks checkout-completed once with order id, course name, and value from receipt", async () => {
     const receipt = mitxonline.factories.orders.order({
-      lines: [mitxonline.factories.orders.transactionLine()],
+      lines: [
+        mitxonline.factories.orders.transactionLine({
+          content_type: "courserun",
+        }),
+      ],
       total_price_paid: "199.99",
     })
 
@@ -274,6 +291,17 @@ describe("EnrollmentRedirectAlert", () => {
       courseName: receipt.lines[0].content_title,
       value: 199.99,
     })
+    expect(mockedPostHogCapture).toHaveBeenCalledTimes(1)
+    expect(mockedPostHogCapture).toHaveBeenCalledWith(
+      PostHogEvents.CheckoutCompleted,
+      {
+        orderId: 17,
+        courseName: receipt.lines[0].content_title,
+        value: 199.99,
+        readableId: receipt.lines[0].readable_id,
+        contentType: "courserun",
+      },
+    )
   })
 
   test("tracks checkout-completed with a null value when the receipt fails to load", async () => {
@@ -293,6 +321,33 @@ describe("EnrollmentRedirectAlert", () => {
       courseName: undefined,
       value: null,
     })
+    expect(mockedPostHogCapture).toHaveBeenCalledWith(
+      PostHogEvents.CheckoutCompleted,
+      {
+        orderId: 18,
+        courseName: undefined,
+        value: null,
+        readableId: undefined,
+        contentType: undefined,
+      },
+    )
+  })
+
+  test("does not capture PostHog checkout_completed when PostHog is not configured", async () => {
+    delete process.env.NEXT_PUBLIC_POSTHOG_API_KEY
+    setMockResponse.get(
+      mitxonline.urls.orders.receipt(19),
+      mitxonline.factories.orders.order(),
+    )
+
+    renderWithProviders(<EnrollmentRedirectAlert />, {
+      url: "/dashboard?order_status=fulfilled&order_id=19",
+    })
+
+    await screen.findByRole("alert")
+
+    expect(trackCheckoutCompleted).toHaveBeenCalledTimes(1)
+    expect(mockedPostHogCapture).not.toHaveBeenCalled()
   })
 
   test("does not track checkout-completed for non-paid alerts", async () => {
@@ -303,6 +358,7 @@ describe("EnrollmentRedirectAlert", () => {
     await screen.findByRole("alert")
 
     expect(trackCheckoutCompleted).not.toHaveBeenCalled()
+    expect(mockedPostHogCapture).not.toHaveBeenCalled()
   })
 
   test.each([