Fix issues related to referencing content (#2371)

* added regex matching and navmenu references * Revert "populated referencing content field for the website content (#2332)" This reverts commit 9a4292c. * added handling for missing keys in metadata * added emptying the references set when all links have been unlinked from content
mitodl · Jan 8, 2025 · 8de1d5c · 8de1d5c
1 parent 5a206ba
commit 8de1d5c
Show file tree

Hide file tree

Showing 15 changed files with 100 additions and 225 deletions.
diff --git a/static/js/components/SiteContentEditor.tsx b/static/js/components/SiteContentEditor.tsx
@@ -1,4 +1,4 @@
-import React, { useState } from "react"
+import React from "react"
 import { useMutation, useRequest } from "redux-query-react"
 import { useSelector, useStore } from "react-redux"
 import { FormikHelpers } from "formik"
@@ -27,11 +27,9 @@ import {
   EditableConfigItem,
   WebsiteContentModalState,
   WebsiteContent,
-  ReferencedContent,
 } from "../types/websites"
 import { SiteFormValues } from "../types/forms"
 import ErrorBoundary from "./ErrorBoundary"
-import ReferencesContext from "../context/References"
 
 export interface SiteContentEditorProps {
   content?: WebsiteContent | null
@@ -55,25 +53,6 @@ export default function SiteContentEditor(
     setDirty,
   } = props
 
-  const [references, setReferences] = useState<ReferencedContent>({
-    link: [],
-    unlink: [],
-  })
-
-  function addReferences(item: string) {
-    setReferences((prev) => ({
-      ...prev,
-      link: [...prev.link, item],
-    }))
-  }
-
-  function removeReferences(item: string) {
-    setReferences((prev) => ({
-      link: prev.link.filter((value) => value !== item),
-      unlink: prev.link.includes(item) ? prev.unlink : [...prev.unlink, item],
-    }))
-  }
-
   const site = useWebsite()
   const store = useStore()
   const refreshWebsiteStatus = () =>
@@ -140,14 +119,6 @@ export default function SiteContentEditor(
       payload["text_id"] = configItem.name
     }
 
-    const references = values.references as ReferencedContent
-    if (
-      (Array.isArray(references?.link) && references.link.length > 0) ||
-      (Array.isArray(references?.unlink) && references.unlink.length > 0)
-    ) {
-      payload["references"] = values.references
-    }
-
     const response = editorState.editing()
       ? await editWebsiteContent(payload, editorState.wrapped)
       : await addWebsiteContent(payload as NewWebsiteContentPayload)
@@ -188,17 +159,13 @@ export default function SiteContentEditor(
 
   return (
     <ErrorBoundary>
-      <ReferencesContext.Provider
-        value={{ references, addReferences, removeReferences }}
-      >
-        <SiteContentForm
-          onSubmit={onSubmitForm}
-          configItem={configItem}
-          content={content}
-          editorState={editorState}
-          setDirty={setDirty}
-        />
-      </ReferencesContext.Provider>
+      <SiteContentForm
+        onSubmit={onSubmitForm}
+        configItem={configItem}
+        content={content}
+        editorState={editorState}
+        setDirty={setDirty}
+      />
     </ErrorBoundary>
   )
 }
diff --git a/static/js/components/forms/SiteContentForm.test.tsx b/static/js/components/forms/SiteContentForm.test.tsx
@@ -23,7 +23,6 @@ import ObjectField from "../widgets/ObjectField"
 
 import * as Website from "../../context/Website"
 import Label from "../widgets/Label"
-import { useReferences } from "../../context/References"
 const { useWebsite: mockUseWebsite } = Website as jest.Mocked<typeof Website>
 
 // ckeditor is not working properly in tests, but we don't need to test it here so just mock it away
@@ -44,18 +43,6 @@ jest.mock("../widgets/SelectField", () => ({
 jest.mock("../../context/Website")
 jest.mock("../../util/dom")
 
-jest.mock("../../context/References", () => ({
-  ...jest.requireActual("../../context/References"),
-  useReferences: jest.fn(),
-}))
-
-const mockedUseReferences = jest.mocked(useReferences)
-mockedUseReferences.mockReturnValue({
-  references: { link: [], unlink: [] },
-  addReferences: jest.fn(),
-  removeReferences: jest.fn(),
-})
-
 const { contentInitialValues, renameNestedFields } = siteContent as jest.Mocked<
   typeof siteContent
 >
@@ -297,7 +284,6 @@ test("SiteContentField creates new values", () => {
   ]
   const { form } = setup(data)
   expect(form.find(FormFields).prop("values")).toEqual({
-    references: {},
     "test-name": "test-default",
   })
 })
@@ -308,8 +294,7 @@ test("SiteContentField uses existing values when editing", () => {
     ...data,
     editorState: createModalState("editing", "id"),
   })
-  expect(form.find(FormFields).prop("values")).toEqual({
-    references: {},
-    ...contentInitialValues(data.content, data.configItem.fields, data.website),
-  })
+  expect(form.find(FormFields).prop("values")).toEqual(
+    contentInitialValues(data.content, data.configItem.fields, data.website),
+  )
 })
diff --git a/static/js/components/forms/SiteContentForm.tsx b/static/js/components/forms/SiteContentForm.tsx
@@ -9,7 +9,6 @@ import {
   validateYupSchema,
   yupToFormErrors,
   FormikErrors,
-  useFormikContext,
 } from "formik"
 
 import SiteContentField from "./SiteContentField"
@@ -34,7 +33,6 @@ import {
 import { SiteFormValues } from "../../types/forms"
 import { getContentSchema } from "./validation"
 import { useWebsite } from "../../context/Website"
-import { useReferences } from "../../context/References"
 
 export interface FormProps {
   onSubmit: (
@@ -53,16 +51,14 @@ export default function SiteContentForm(props: FormProps): JSX.Element {
   const website = useWebsite()
 
   const initialValues: SiteFormValues = useMemo(
-    () => ({
-      ...(editorState.adding()
+    () =>
+      editorState.adding()
         ? newInitialValues(configItem.fields, website)
         : contentInitialValues(
             content as WebsiteContent,
             configItem.fields,
             website,
-          )),
-      references: {}, // Add this to ensure Formik tracks the `references` field
-    }),
+          ),
     [configItem.fields, editorState, content, website],
   )
 
@@ -118,9 +114,6 @@ export function FormFields(props: InnerFormProps): JSX.Element {
     [configItem],
   )
 
-  const { setFieldValue } = useFormikContext()
-  const { references } = useReferences()
-
   useEffect(() => {
     setDirty(dirty)
   }, [setDirty, dirty])
@@ -175,7 +168,6 @@ export function FormFields(props: InnerFormProps): JSX.Element {
           type="submit"
           disabled={isSubmitting}
           className="px-5 btn cyan-button"
-          onClick={() => setFieldValue("references", references)}
         >
           Save
         </button>

diff --git a/static/js/components/widgets/MarkdownEditor.test.tsx b/static/js/components/widgets/MarkdownEditor.test.tsx
@@ -15,7 +15,6 @@ import {
   ADD_RESOURCE_LINK,
   CKEDITOR_RESOURCE_UTILS,
   MARKDOWN_CONFIG_KEY,
-  REFERENCED_CONTENT,
   RESOURCE_EMBED,
   RESOURCE_LINK,
   RESOURCE_LINK_CONFIG_KEY,
@@ -26,7 +25,6 @@ import { getMockEditor } from "../../test_util"
 import { useWebsite } from "../../context/Website"
 import { makeWebsiteDetail } from "../../util/factories/websites"
 import ResourceLink from "../../lib/ckeditor/plugins/ResourceLink"
-import { useReferences } from "../../context/References"
 
 jest.mock("../../lib/ckeditor/CKEditor", () => {
   const originalModule = jest.requireActual("../../lib/ckeditor/CKEditor")
@@ -52,11 +50,6 @@ jest.mock("@ckeditor/ckeditor5-react", () => ({
   CKEditor: () => <div />,
 }))
 
-jest.mock("react-router", () => ({
-  ...jest.requireActual("react-router"),
-  useParams: jest.fn(),
-}))
-
 const render = (props = {}) => {
   return shallow(
     <MarkdownEditor allowedHtml={[]} link={[]} embed={[]} {...props} />,
@@ -65,24 +58,13 @@ const render = (props = {}) => {
 
 const mocUseWebsite = jest.mocked(useWebsite)
 
-jest.mock("../../context/References", () => ({
-  ...jest.requireActual("../../context/References"),
-  useReferences: jest.fn(),
-}))
-const mockedUseReferences = jest.mocked(useReferences)
-
 describe("MarkdownEditor", () => {
   let sandbox: SinonSandbox
 
   beforeEach(() => {
     sandbox = sinon.createSandbox()
     const website = makeWebsiteDetail()
     mocUseWebsite.mockReturnValue(website)
-    mockedUseReferences.mockReturnValue({
-      references: { link: [], unlink: [] },
-      addReferences: jest.fn(),
-      removeReferences: jest.fn(),
-    })
   })
 
   afterEach(() => {
@@ -123,7 +105,6 @@ describe("MarkdownEditor", () => {
           MARKDOWN_CONFIG_KEY,
           RESOURCE_LINK_CONFIG_KEY,
           WEBSITE_NAME,
-          REFERENCED_CONTENT,
         ],
         ckWrapper.prop("config"),
       )

diff --git a/static/js/components/widgets/MarkdownEditor.tsx b/static/js/components/widgets/MarkdownEditor.tsx
@@ -23,15 +23,13 @@ import {
   MARKDOWN_CONFIG_KEY,
   RESOURCE_LINK_CONFIG_KEY,
   WEBSITE_NAME,
-  REFERENCED_CONTENT,
 } from "../../lib/ckeditor/plugins/constants"
 import ResourcePickerDialog from "./ResourcePickerDialog"
 import useThrowSynchronously from "../../hooks/useAsyncError"
 import { useWebsite } from "../../context/Website"
 import { siteContentRerouteUrl } from "../../lib/urls"
 import { checkFeatureFlag } from "../../lib/util"
 import CustomLink from "../../lib/ckeditor/plugins/CustomLink"
-import { useReferences } from "../../context/References"
 
 export interface Props {
   value?: string
@@ -55,8 +53,8 @@ export default function MarkdownEditor(props: Props): JSX.Element {
   const { link, embed, value, name, onChange, minimal, allowedHtml } = props
   const throwSynchronously = useThrowSynchronously()
   const website = useWebsite()
+
   const [isCustomLinkUIEnabled, setIsCustomLinkUIEnabled] = useState(false)
-  const { addReferences, removeReferences } = useReferences()
 
   useEffect(() => {
     checkFeatureFlag(
@@ -148,9 +146,6 @@ export default function MarkdownEditor(props: Props): JSX.Element {
         }`,
       },
       [WEBSITE_NAME]: website.name,
-
-      // Function cannot be directly passed in config
-      [REFERENCED_CONTENT]: { add: addReferences, remove: removeReferences },
     }
 
     // Create a copy of plugins to avoid mutating the original
@@ -205,8 +200,6 @@ export default function MarkdownEditor(props: Props): JSX.Element {
     allowedHtml,
     website.name,
     isCustomLinkUIEnabled,
-    addReferences,
-    removeReferences,
   ])
 
   const configKey = useMemo(() => JSON.stringify(editorConfig), [editorConfig])

diff --git a/static/js/components/widgets/ResourcePickerDialog.test.tsx b/static/js/components/widgets/ResourcePickerDialog.test.tsx
@@ -24,7 +24,6 @@ import WebsiteContext from "../../context/Website"
 import { Website } from "../../types/websites"
 import origResourcePickerListing from "./ResourcePickerListing"
 import { assertNotNil } from "../../test_util"
-import { useReferences } from "../../context/References"
 
 jest.mock("../../hooks/state")
 const useDebouncedState = jest.mocked(hooksState.useDebouncedState)
@@ -39,12 +38,6 @@ jest.mock("./ResourcePickerListing", () => {
 })
 const ResourcePickerListing = jest.mocked(origResourcePickerListing)
 
-jest.mock("../../context/References", () => ({
-  ...jest.requireActual("../../context/References"),
-  useReferences: jest.fn(),
-}))
-const mockedUseReferences = jest.mocked(useReferences)
-
 const focusResource = (wrapper: ReactWrapper, resource: WebsiteContent) => {
   act(() => {
     wrapper.find(ResourcePickerListing).prop("focusResource")(resource)
@@ -65,12 +58,6 @@ describe("ResourcePickerDialog", () => {
     helper = new IntegrationTestHelper()
     website = makeWebsiteDetail()
 
-    mockedUseReferences.mockReturnValue({
-      references: { link: [], unlink: [] },
-      addReferences: jest.fn(),
-      removeReferences: jest.fn(),
-    })
-
     insertEmbedStub = helper.sandbox.stub()
     closeDialogStub = helper.sandbox.stub()
     resource = makeWebsiteContentDetail()

diff --git a/static/js/components/widgets/ResourcePickerDialog.tsx b/static/js/components/widgets/ResourcePickerDialog.tsx
@@ -20,7 +20,6 @@ import {
 import { WebsiteContent } from "../../types/websites"
 import { useWebsite } from "../../context/Website"
 import _, { keyBy } from "lodash"
-import { useReferences } from "../../context/References"
 
 interface Props {
   mode: ResourceDialogMode
@@ -160,8 +159,6 @@ export default function ResourcePickerDialog(props: Props): JSX.Element {
     null,
   )
 
-  const { addReferences } = useReferences()
-
   const addResource = useCallback(() => {
     if (focusedResource && isOpen) {
       insertEmbed(
@@ -170,9 +167,8 @@ export default function ResourcePickerDialog(props: Props): JSX.Element {
         mode,
       )
       closeDialog()
-      addReferences(focusedResource.text_id)
     }
-  }, [insertEmbed, focusedResource, closeDialog, isOpen, mode, addReferences])
+  }, [insertEmbed, focusedResource, closeDialog, isOpen, mode])
 
   const { acceptText, title } = modeText[mode]
 

diff --git a/static/js/context/References.ts b/static/js/context/References.ts