feat: add detail and editing page for article feature #2679

ahtesham-quraish · 2025-11-06T11:09:58Z

What are the relevant tickets?

In reference to #91

Description (What does it do?)

Fetch the article from existing API endpoint (e.g., /api/articles/1)
Display each article with:
- Title
- "Edit" button / link to navigate to article edit page
Show loader while fetching data
Handle error state (API failure, network error, etc.)

Screenshots (if appropriate):

Screen.Recording.2025-11-06.at.3.50.27.PM.mp4

How can this be tested?

run yarn to install the dependencies
after installing the dependencies you need to up the frontend next app
then hit the following url /articles/new for accessing the page

Additional Context

github-actions · 2025-11-06T11:10:12Z

OpenAPI Changes

Show/hide 3 changes: 0 error, 0 warning, 3 info

3 changes: 0 error, 0 warning, 3 info
info	[request-parameter-property-type-generalized] at head/openapi/specs/v1.yaml	
	in API GET /api/v1/contentfiles/
		for the 'query' request parameter 'resource_id', the type/format of property '/items/' was generalized from 'integer'/'' to 'number'/''

info	[request-parameter-property-type-generalized] at head/openapi/specs/v1.yaml	
	in API GET /api/v1/courses/{learning_resource_id}/contentfiles/
		for the 'query' request parameter 'resource_id', the type/format of property '/items/' was generalized from 'integer'/'' to 'number'/''

info	[request-parameter-property-type-generalized] at head/openapi/specs/v1.yaml	
	in API GET /api/v1/learning_resources/{learning_resource_id}/contentfiles/
		for the 'query' request parameter 'resource_id', the type/format of property '/items/' was generalized from 'integer'/'' to 'number'/''

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

ChristopherChudzicki

Edit: Please see https://github.com/mitodl/hq/issues/9198 before working more on this.

Original post

The new page + editing + viewing functionality is working well 👍

I left some feedback about more standard ways to do things within learn, using the component library, and testing stuff.

Overall Request:

Don't spend any more time on CKEditor changes... As mentioned in slack, we're planning to switch to tiptap. We can leave the CKEditor code in this PR as-is.

The changes I've mentioned here are not CKEditor related, so are still worth making.

IMO, simplest to close #2671 and focus on this PR. (This one is based off that, anyway, right?)

ChristopherChudzicki · 2025-11-06T17:03:57Z

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

+    return notFound()
+  }
+  return (
+    <Container>


Suggestion: Here and on the other pages, I'd do something like

const Page = styled(Container)({ marginTop: "40px", marginBottom: "40px", })

(note: don't want to override the horizontal margins on container) and use that as the root element for some top/bottom margins.

I used the way you suggested

ChristopherChudzicki · 2025-11-06T17:10:26Z

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

+const ArticleTitle = styled.h1({
+  fontSize: "24px",
+  marginBottom: "12px",
+})


In general, avoid specifying any font size, etc, directly. Here we'd do

const ArticleTitle = styled.h1(({theme})=>({ ...theme.custom.typography.h3, // I'm making up h3 since we don't have designs. marginBottom: "12px", }))

though I'd probably do <Typography variant="h3" component="h1" sx={{marginBottom: "12px"}}> see https://mui.com/system/getting-started/the-sx-prop/

I have used Typography for our use

ChristopherChudzicki · 2025-11-06T17:48:08Z

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

+        <EditButton>
+          <EditButtonLink
+            href={`/articles/${data.id}/edit`}
+            className="btn btn-edit"
+            color="red"
+          >
+            Edit
+          </EditButtonLink>
+        </EditButton>


Instead of custom styles, this should be replaceable by <ButtonLink href={/articles/${data.id}/edit} variant="primary">. See https://mitodl.github.io/smoot-design/?path=/docs/smoot-design-button--docs#links for more.

Re className="btn btn-edit": Here and elsewhere, let's not add unneessary classes. Styling in mit-learn is generally done through styled(...).

I have used the ButtonLink

ChristopherChudzicki · 2025-11-06T18:01:04Z

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

+
+  const [title, setTitle] = React.useState<string>("")
+  const [editorContent, setEditorContent] = React.useState<string>("")
+  const [editorKey] = React.useState(0)


Do we need this?

I have removed this

ChristopherChudzicki · 2025-11-06T18:03:36Z

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

+  const [alertText, setAlertText] = React.useState("")
+  const [severity, setSeverity] = React.useState<"success" | "error">("success")


Let's not use state for this... check articleUpdate.isError. (I don't think clearing the error upon title change, body change, is necessary. It's an edge case anyway and will be cleared on a successful submission. If you want to clear it, you can do articleUpdate.reset().

const articleUpdate = useArticlePartialUpdate()

I have removed the severity in our case and error remains there if user does not remove it and will be gone upon successful submission

ChristopherChudzicki · 2025-11-06T18:10:03Z

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

+      <TestErrorBoundary>
+        <NewArticlePage />
+      </TestErrorBoundary>,


Instead of a custom error boundary, there is a TestingErrorBoundary in @/test-utils (aka frontends/main/src/test-utils/index.tsx) that you could use like

const onError = jest.fn() renderWithProviders(<TestingErrorBoundary onError={onError}>...<TestingErrorBoundary/>) await waitFor(() => expect(onError).toHaveBeenCalled() )

This should definitely be easier to test, though :/

ChristopherChudzicki · 2025-11-06T18:24:26Z

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

+  useArticleDetail: (id: number) => ({
+    data: {
+      id,
+      title: "Existing Title",
+      html: "<p>Existing content</p>",
+    },
+    isLoading: false,
+  }),
+  useArticlePartialUpdate: () => ({
+    mutate: mockUpdateMutate,
+    isPending: false,
+  }),
+}))


(Here and elsewhere in the pr)

Request: Don't mock these hooks—it can be fragile. For example:

useArticleDetail removing the custom hooks and using queries directly would artificially break this (code would work, just tests fail)

For the mutation, switching the code to something along the lines of below would break the updating tests.
onSubmit = {() => { await articleUpdate.mutateAsync(newData) redirect(article.id) })

Instead, mock the API call:

const makeArticle = factories.articles.article const article = makeArticle() setMockResponse.get(urls.articles.details(article.id), article) setMockResponse.post(urls.articles.details(article.id), WHATEVER_RESPONSE_BODY) // if you need an error response: setMockResponse.post(urls.articles.details(article.id), WHATEVER_RESPONSE_BODY, { code: 500 } )

ChristopherChudzicki · 2025-11-06T18:31:38Z

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

+    })
+
+    // Click save
+    fireEvent.click(screen.getByText(/save article/i))


(Here and elsewhere in the PR)

Request: Instead of fireEvent.click, use

import userEvent from "@testing-library/user-event" // then await userEvent.click(screen.getByText(/save article/i))

fireEvent basically just fires a click event, period. If you corresponding HTML button happens to be disabled, it will still fire the event (even though a user couldn't actually trigger that in a real browser).

userEvent.click is a higher-level API. It performs a number of other things:

fires pointer down + pointer up events

performs some sanity checks, like "is the button disabled? if yes, throw an error".

See https://testing-library.com/docs/user-event/intro/#differences-from-fireevent for more.

ChristopherChudzicki · 2025-11-06T18:44:53Z

frontends/main/src/app/articles/[id]/page.tsx

+import { ArticleDetailPage } from "@/app-pages/Articles/ArticleDetailPage"
+
+export const metadata: Metadata = standardizeMetadata({
+  title: "Article Detail",


I'd add noindex, nofollow here, too.

ChristopherChudzicki · 2025-11-06T18:48:15Z

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

+  color: "#FFFFFF",
+})
+
+export const ArticleDetailPage = ({ articleId }: { articleId: number }) => {


Request: This should be a restrictedroute, too. We're at an early stage so none of these pages should be public yet.

Ahtesham Quraish and others added 2 commits November 4, 2025 18:46

feat: add article create page

69e04c1

address the PR commit

92ea580

ahtesham-quraish force-pushed the ahtesham/#9109-article-detail branch 4 times, most recently from 84043af to dc89ff1 Compare November 6, 2025 11:34

ahtesham-quraish changed the title ~~Ahtesham/#9109 article detail~~ feat: add detail and editing page for article feature Nov 6, 2025

ahtesham-quraish force-pushed the ahtesham/#9109-article-detail branch 3 times, most recently from b5b2859 to 6a71023 Compare November 6, 2025 11:51

feat: add article detail and edit page

a8f9f22

ahtesham-quraish force-pushed the ahtesham/#9109-article-detail branch from 6a71023 to a8f9f22 Compare November 6, 2025 12:02

ChristopherChudzicki requested changes Nov 6, 2025

View reviewed changes

		const [alertText, setAlertText] = React.useState("")
		const [severity, setSeverity] = React.useState<"success" \| "error">("success")

feat: add detail and editing page for article feature #2679

Are you sure you want to change the base?

feat: add detail and editing page for article feature #2679

Uh oh!

Conversation

ahtesham-quraish commented Nov 6, 2025

What are the relevant tickets?

Description (What does it do?)

Screenshots (if appropriate):

How can this be tested?

Additional Context

Uh oh!

github-actions bot commented Nov 6, 2025

OpenAPI Changes

Uh oh!

ChristopherChudzicki left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ChristopherChudzicki left a comment •

edited

Loading