Article editor refactor for reuse and layout updates (#2699)

* Initial Tiptap Editor

* Update lockfile

* Ignore stylelint errors in vendor sheets

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Run fmt-fix

* Eslint ignore vendor code

* Revert main package.json

* Remove aliases from Tiptap template code imports

* Prettier fixes

* CodeQL fix

* React in scope fixes

* Format fix

* Transform react-hotkeys-hook to resolve esm error

* feat: adding tiptap integration with article crud operation

* feat: add upload image control in toolbar

* made wrapper component for editor

* Refactor ArticleEditor. Layour changed (wip)

* Refactor and layout fixes

* Remove initial editor route

* Reinstate edit page redirect. Update tests

* Remove unused value prop

* Lint

* Invalidate article detail cache

* Detail page layout. Fix race condition on content display after save

* Remove EditorContainer

* Update test

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Ahtesham Quraish <ahtesham.quraish@A006-01455.local>
Co-authored-by: Ahtesham Quraish <ahtesham.quraish@192.168.10.30>
Co-authored-by: Ahtesham Quraish <ahtesham.quraish@192.168.10.8>

Author: 5 people

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

@@ -90,7 +90,7 @@ describe("Article CRUD", () => {
     const { id, ...patchData } = article
     expect(makeRequest).toHaveBeenCalledWith("patch", url, patchData)
     expect(queryClient.invalidateQueries).toHaveBeenCalledWith({
-      queryKey: articleKeys.root,
+      queryKey: articleKeys.detail(article.id),
     })
   })
 

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

@@ -18,7 +18,7 @@ const useArticleList = (
 }
 
 /**
- * Query is diabled if id is undefined.
+ * Query is disabled if id is undefined.
  */
 const useArticleDetail = (id: number | undefined) => {
   return useQuery({
@@ -58,8 +58,8 @@ const useArticlePartialUpdate = () => {
           PatchedRichTextArticleRequest: data,
         })
         .then((response) => response.data),
-    onSuccess: (_data) => {
-      client.invalidateQueries({ queryKey: articleKeys.root })
+    onSuccess: (article: Article) => {
+      client.invalidateQueries({ queryKey: articleKeys.detail(article.id) })
     },
   })
 }

frontends/main/src/app-pages/ArticlePage/NewArticlePage.tsx

@@ -2,66 +2,27 @@
 
 import React from "react"
 import { useArticleDetail } from "api/hooks/articles"
-import {
-  Container,
-  LoadingSpinner,
-  styled,
-  Typography,
-  TiptapEditorContainer,
-} from "ol-components"
-import { ButtonLink } from "@mitodl/smoot-design"
+import { LoadingSpinner, ArticleEditor } from "ol-components"
 import { notFound } from "next/navigation"
 import { Permission } from "api/hooks/user"
 import RestrictedRoute from "@/components/RestrictedRoute/RestrictedRoute"
-import { articlesEditView } from "@/common/urls"
-
-const Page = styled(Container)({
-  marginTop: "40px",
-  marginBottom: "40px",
-})
-
-const ControlsContainer = styled.div({
-  display: "flex",
-  justifyContent: "flex-end",
-  margin: "10px",
-})
-const WrapperContainer = styled.div({
-  borderBottom: "1px solid rgb(222, 208, 208)",
-  paddingBottom: "10px",
-})
 
 export const ArticleDetailPage = ({ articleId }: { articleId: number }) => {
-  const id = Number(articleId)
-  const { data, isLoading } = useArticleDetail(id)
+  const {
+    data: article,
+    isLoading,
+    isFetching,
+  } = useArticleDetail(Number(articleId))
 
-  const editUrl = articlesEditView(id)
-
-  if (isLoading) {
-    return <LoadingSpinner color="inherit" loading={isLoading} size={32} />
+  if (isLoading || isFetching) {
+    return <LoadingSpinner color="inherit" loading size={32} />
   }
-  if (!data) {
+  if (!article) {
     return notFound()
   }
   return (
     <RestrictedRoute requires={Permission.ArticleEditor}>
-      <Page>
-        <WrapperContainer>
-          <Typography variant="h3" component="h1">
-            {data?.title}
-          </Typography>
-
-          <ControlsContainer>
-            <ButtonLink href={editUrl} variant="primary">
-              Edit
-            </ButtonLink>
-          </ControlsContainer>
-        </WrapperContainer>
-        <TiptapEditorContainer
-          data-testid="editor"
-          value={data.content}
-          readOnly
-        />
-      </Page>
+      <ArticleEditor article={article} readOnly />
     </RestrictedRoute>
   )
 }

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

@@ -2,14 +2,14 @@ import React from "react"
 import { screen, renderWithProviders, setMockResponse } from "@/test-utils"
 import { waitFor, fireEvent } from "@testing-library/react"
 import userEvent from "@testing-library/user-event"
-import { factories, urls } from "api/test-utils"
+import { factories, urls, makeRequest } from "api/test-utils"
 import { ArticleEditPage } from "./ArticleEditPage"
 
-const pushMock = jest.fn()
+const mockPush = jest.fn()
 
 jest.mock("next-nprogress-bar", () => ({
   useRouter: () => ({
-    push: pushMock,
+    push: mockPush,
   }),
 }))
 
@@ -38,12 +38,12 @@ describe("ArticleEditPage", () => {
 
     renderWithProviders(<ArticleEditPage articleId={"42"} />)
 
-    expect(await screen.findByText("Edit Article")).toBeInTheDocument()
-    expect(screen.getByTestId("editor")).toBeInTheDocument()
+    await screen.findByTestId("editor")
+
     expect(screen.getByDisplayValue("Existing Title")).toBeInTheDocument()
   })
 
-  test("submits article successfully and redirects", async () => {
+  test("submits article successfully", async () => {
     const user = factories.user.user({
       is_authenticated: true,
       is_article_editor: true,
@@ -65,22 +65,30 @@ describe("ArticleEditPage", () => {
     })
     setMockResponse.get(urls.articles.details(article.id), article)
 
-    // ✅ Mock successful update response
     const updated = { ...article, title: "Updated Title" }
     setMockResponse.patch(urls.articles.details(article.id), updated)
 
     renderWithProviders(<ArticleEditPage articleId={"123"} />)
 
-    const titleInput = await screen.findByPlaceholderText("Enter article title")
+    await screen.findByTestId("editor")
+
+    const titleInput = await screen.findByPlaceholderText("Article title")
 
     fireEvent.change(titleInput, { target: { value: "Updated Title" } })
 
     await waitFor(() => expect(titleInput).toHaveValue("Updated Title"))
 
-    await userEvent.click(screen.getByText(/save article/i))
+    await userEvent.click(screen.getByRole("button", { name: "Save" }))
 
-    // ✅ Wait for redirect after update success
-    await waitFor(() => expect(pushMock).toHaveBeenCalledWith("/articles/123"))
+    await waitFor(() =>
+      expect(makeRequest).toHaveBeenCalledWith(
+        "patch",
+        urls.articles.details(article.id),
+        expect.objectContaining({ title: "Updated Title" }),
+      ),
+    )
+
+    await waitFor(() => expect(mockPush).toHaveBeenCalledWith("/articles/123"))
   })
 
   test("shows error alert on failure", async () => {
@@ -105,7 +113,6 @@ describe("ArticleEditPage", () => {
     })
     setMockResponse.get(urls.articles.details(article.id), article)
 
-    // ✅ Mock failed update (500)
     setMockResponse.patch(
       urls.articles.details(article.id),
       { detail: "Server Error" },
@@ -114,10 +121,12 @@ describe("ArticleEditPage", () => {
 
     renderWithProviders(<ArticleEditPage articleId={"7"} />)
 
-    const titleInput = await screen.findByPlaceholderText("Enter article title")
+    await screen.findByTestId("editor")
+
+    const titleInput = await screen.findByPlaceholderText("Article title")
     fireEvent.change(titleInput, { target: { value: "Bad Article" } })
 
-    await userEvent.click(screen.getByText(/save article/i))
+    await userEvent.click(screen.getByRole("button", { name: "Save" }))
 
     expect(await screen.findByText(/Mock Error/i)).toBeInTheDocument()
   })

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

@@ -1,125 +1,50 @@
 "use client"
-import React, { useEffect, useState } from "react"
-import { Permission } from "api/hooks/user"
+
+import React from "react"
 import { useRouter } from "next-nprogress-bar"
-import { useArticleDetail, useArticlePartialUpdate } from "api/hooks/articles"
-import { Button, Alert } from "@mitodl/smoot-design"
+import { notFound } from "next/navigation"
+import { Permission } from "api/hooks/user"
+import { useArticleDetail } from "api/hooks/articles"
 import RestrictedRoute from "@/components/RestrictedRoute/RestrictedRoute"
 import {
-  Container,
-  Typography,
   styled,
   LoadingSpinner,
-  TiptapEditorContainer,
-  JSONContent,
+  ArticleEditor,
+  HEADER_HEIGHT,
 } from "ol-components"
-
-import { notFound } from "next/navigation"
 import { articlesView } from "@/common/urls"
 
-const SaveButton = styled.div({
-  textAlign: "right",
-  margin: "10px",
-})
-
-const ClientContainer = styled.div({
-  width: "100%",
-  margin: "10px 0",
-})
+const PageContainer = styled.div(({ theme }) => ({
+  color: theme.custom.colors.darkGray2,
+  display: "flex",
+  height: `calc(100vh - ${HEADER_HEIGHT}px - 132px)`,
+}))
 
 const ArticleEditPage = ({ articleId }: { articleId: string }) => {
+  const {
+    data: article,
+    isLoading,
+    isFetching,
+  } = useArticleDetail(Number(articleId))
   const router = useRouter()
 
-  const id = Number(articleId)
-  const { data: article, isLoading } = useArticleDetail(id)
-
-  const [title, setTitle] = useState<string>("")
-  const [json, setJson] = useState<JSONContent>({
-    type: "doc",
-    content: [{ type: "paragraph", content: [] }],
-  })
-  const [alertText, setAlertText] = useState("")
-
-  const { mutate: updateArticle, isPending } = useArticlePartialUpdate()
-
-  const handleSave = () => {
-    const payload = {
-      id: id,
-      title: title.trim(),
-      content: json,
-    }
-
-    updateArticle(payload, {
-      onSuccess: (article) => {
-        router.push(articlesView(article.id))
-      },
-      onError: (error) => {
-        setAlertText(`❌ ${error.message}`)
-      },
-    })
-  }
-
-  useEffect(() => {
-    if (article && !title) {
-      setTitle(article.title)
-      setJson(article.content)
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [article])
-
-  if (isLoading) {
+  if (isLoading || isFetching) {
     return <LoadingSpinner color="inherit" loading={isLoading} size={32} />
   }
   if (!article) {
     return notFound()
   }
 
-  const handleChange = (json: object) => {
-    setJson(json)
-  }
-
   return (
     <RestrictedRoute requires={Permission.ArticleEditor}>
-      <Container>
-        <Typography variant="h3" component="h1">
-          Edit Article
-        </Typography>
-        {alertText && (
-          <Alert
-            key={alertText}
-            severity="error"
-            className="info-alert"
-            closable
-          >
-            <Typography variant="body2" color="textPrimary">
-              {alertText}
-            </Typography>
-          </Alert>
-        )}
-
-        <ClientContainer>
-          <TiptapEditorContainer
-            data-testid="editor"
-            value={json}
-            onChange={handleChange}
-            title={title}
-            setTitle={(e) => {
-              setTitle(e.target.value)
-              setAlertText("")
-            }}
-          />
-        </ClientContainer>
-
-        <SaveButton>
-          <Button
-            variant="primary"
-            disabled={isPending || !title.trim()}
-            onClick={handleSave}
-          >
-            {isPending ? "Saving..." : "Save Article"}
-          </Button>
-        </SaveButton>
-      </Container>
+      <PageContainer>
+        <ArticleEditor
+          article={article}
+          onSave={(article) => {
+            router.push(articlesView(article.id))
+          }}
+        />
+      </PageContainer>
     </RestrictedRoute>
   )
 }