Merge pull request #350 from NYPL/SCC-4305/fix-incorrect-subject-head…

…ing-urls-bug SCC-4305 - Fix incorrect subject heading urls bug
NYPL · Oct 7, 2024 · 010e96a · 010e96a
2 parents 4da5687 + 61ad76c
commit 010e96a
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 60 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -5,17 +5,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [1.3.2] 2024-10-4
+## [1.3.2] 2024-10-7
 
 ### Fixed
 
 - Adjust ItemTable styles according to design (SCC-4299)
 - Remove border and padding from eResources link as part of post-Bib page launch VQA (SCC-4299)
+- Remove base url from finding aid urls (SCC-4306)
+- Fix incorrect subject heading urls in Bib Details (SCC-4305)
 
 ### [1.3.1] Hotfix
 
 - Fix Table column widths on Search Results after DS 3.4.0 update (SCC-4299)
-- Remove base url from finding aid urls
 
 ## [1.3.0] 2024-10-3
 

diff --git a/src/components/BibPage/BibDetail.tsx b/src/components/BibPage/BibDetail.tsx
@@ -7,7 +7,7 @@ import RCLink from "../Links/RCLink/RCLink"
 import ExternalLink from "../Links/ExternalLink/ExternalLink"
 import type {
   BibDetail,
-  Url,
+  BibDetailURL,
   LinkedBibDetail,
   SubjectHeadingDetail,
   AnyBibDetail,
@@ -77,7 +77,7 @@ const PlainTextElement = (field: BibDetail) => {
 
 const CompoundSubjectHeadingElement = (field: SubjectHeadingDetail) => {
   const subjectHeadingLinksPerSubject = field.value.map(
-    (subjectHeadingUrls: Url[]) => {
+    (subjectHeadingUrls: BibDetailURL[]) => {
       return SingleSubjectHeadingElement(subjectHeadingUrls)
     }
   )
@@ -89,27 +89,30 @@ const CompoundSubjectHeadingElement = (field: SubjectHeadingDetail) => {
   return DetailElement(field.label, values)
 }
 
-const SingleSubjectHeadingElement = (subjectHeadingUrls: Url[]) => {
-  return subjectHeadingUrls.reduce((linksPerSubject, url: Url, index) => {
-    const divider = (
-      // this span will render as > in between the divided subject heading links
-      <span data-testid="divider" key={`divider-${index}`}>
-        {" "}
-        &gt;{" "}
-      </span>
-    )
-    const link = LinkElement(url, "internal")
-    linksPerSubject.push(link)
-    if (!isItTheLastElement(index, subjectHeadingUrls)) {
-      linksPerSubject.push(divider)
-    }
-    return linksPerSubject
-  }, [] as ReactElement[])
+const SingleSubjectHeadingElement = (subjectHeadingUrls: BibDetailURL[]) => {
+  return subjectHeadingUrls.reduce(
+    (linksPerSubject, url: BibDetailURL, index) => {
+      const divider = (
+        // this span will render as > in between the divided subject heading links
+        <span data-testid="divider" key={`divider-${index}`}>
+          {" "}
+          &gt;{" "}
+        </span>
+      )
+      const link = LinkElement(url, "internal")
+      linksPerSubject.push(link)
+      if (!isItTheLastElement(index, subjectHeadingUrls)) {
+        linksPerSubject.push(divider)
+      }
+      return linksPerSubject
+    },
+    [] as ReactElement[]
+  )
 }
 
 const LinkedDetailElement = (field: LinkedBibDetail) => {
   const internalOrExternal = field.link
-  const values = field.value.map((urlInfo: Url, i) => {
+  const values = field.value.map((urlInfo: BibDetailURL, i) => {
     return (
       <li key={`${kebabCase(field.label)}-${i}`}>
         {LinkElement(urlInfo, internalOrExternal)}
@@ -119,7 +122,7 @@ const LinkedDetailElement = (field: LinkedBibDetail) => {
   return DetailElement(field.label, values)
 }
 
-const LinkElement = (url: Url, linkType: string) => {
+const LinkElement = (url: BibDetailURL, linkType: string) => {
   let Link: typeof RCLink | typeof ExternalLink
   if (linkType === "internal") Link = RCLink
   else if (linkType === "external") Link = ExternalLink

diff --git a/src/models/BibDetails.ts b/src/models/BibDetails.ts
@@ -4,7 +4,7 @@ import type {
   BibDetail,
   FieldMapping,
   AnnotatedMarcField,
-  Url,
+  BibDetailURL,
   SubjectHeadingDetail,
   AnnotatedMarc,
   AnyBibDetail,
@@ -207,7 +207,10 @@ export default class BibDetails {
     }
   }
 
-  buildExternalLinkedDetail(label: string, values: Url[]): LinkedBibDetail {
+  buildExternalLinkedDetail(
+    label: string,
+    values: BibDetailURL[]
+  ): LinkedBibDetail {
     if (!values.length) return null
     return {
       link: "external",
@@ -365,32 +368,16 @@ export default class BibDetails {
 
   buildSubjectHeadings(): SubjectHeadingDetail {
     if (!this.bib.subjectHeadings) return
-    const subjectHeadingsUrls = this.bib.subjectHeadings.map(
-      ({ label, uuid }) => {
-        if (!label || !uuid) return
-        const subject = label.replace(/\.$/, "")
-        // stackedSubjectHeadings: ["a", "a -- b", "a -- b -- c"]
-        const stackedSubjectHeadings =
-          this.constructSubjectHeadingsArray(subject)
-        const shepUrl = `/subject_headings/${uuid}`
-        // splitSubjectHeadings: ["a", "b", "c"]
-        const splitSubjectHeadings = subject.split(" -- ")
+    const subjectHeadingsUrls = this.bib.subjectHeadings.map((heading) =>
+      this.flattenSubjectHeadingUrls(heading)
+    )
 
-        return splitSubjectHeadings.map((heading, index) => {
-          const urlWithLabelParam = `${shepUrl}?label=${encodeURI(
-            stackedSubjectHeadings[index]
-          )}`
-          return {
-            url: urlWithLabelParam,
-            urlLabel: heading,
-          }
-        })
+    return (
+      subjectHeadingsUrls?.length && {
+        label: "Subject",
+        value: subjectHeadingsUrls,
       }
     )
-    return {
-      label: "Subject",
-      value: subjectHeadingsUrls,
-    }
   }
 
   buildSubjectLiterals(): SubjectHeadingDetail {
@@ -400,7 +387,7 @@ export default class BibDetails {
         subject = subject.replace(/\.$/, "")
         // stackedSubjectHeadings: ["a", "a -- b", "a -- b -- c"]
         const stackedSubjectHeadings =
-          this.constructSubjectHeadingsArray(subject)
+          this.constructSubjectLiteralsArray(subject)
         const filterQueryForSubjectHeading = "/search?filters[subjectLiteral]="
         // splitSubjectHeadings: ["a", "b", "c"]
         const splitSubjectHeadings = subject.split(" -- ")
@@ -421,19 +408,49 @@ export default class BibDetails {
     }
   }
 
-  constructSubjectHeadingsArray(subject: string) {
+  constructSubjectLiteralsArray(subject: string) {
     // subject = "Italian food -- Spaghetti"
-    let stackedSubjectHeading = ""
+    let stackedSubjectLiteral = ""
 
     return subject
       .split(" -- ") // ["Italian food", "spaghetti"]
       .map((urlString, index) => {
         const dashDivided = index !== 0 ? " -- " : ""
         // First iteration "Italian food"
         // Second iteration "Italian food -- spaghetti"
-        stackedSubjectHeading = `${stackedSubjectHeading}${dashDivided}${urlString}`
+        stackedSubjectLiteral = `${stackedSubjectLiteral}${dashDivided}${urlString}`
 
-        return stackedSubjectHeading
+        return stackedSubjectLiteral
       })
   }
+
+  /**
+   * Flatten subject headings into a list of objects with a url and a label
+   */
+  flattenSubjectHeadingUrls(heading): BibDetailURL[] | null {
+    if (!heading.label || !heading.uuid) return null
+    const subjectHeadingsArray = []
+
+    // iterate through each nested subject until there's no parent element
+    let currentHeading = heading
+
+    while (currentHeading.parent) {
+      subjectHeadingsArray.unshift(
+        this.getSubjectHeadingUrl(currentHeading.uuid, currentHeading.label)
+      )
+      currentHeading = currentHeading.parent
+    }
+    // add the top level subject heading
+    subjectHeadingsArray.unshift(
+      this.getSubjectHeadingUrl(currentHeading.uuid, currentHeading.label)
+    )
+    return subjectHeadingsArray
+  }
+
+  getSubjectHeadingUrl(uuid: string, label: string): BibDetailURL {
+    return {
+      url: `/subject_headings/${uuid}?label=${encodeURIComponent(label)}`,
+      urlLabel: label.split(" -- ").pop(),
+    }
+  }
 }
diff --git a/src/models/modelTests/BibDetails.test.ts b/src/models/modelTests/BibDetails.test.ts
@@ -63,25 +63,25 @@ describe("Bib model", () => {
         value: [
           [
             {
-              url: "/subject_headings/74746d11-638b-4cfb-a72a-9a2bd296e6fd?label=Cortanze,%20G%C3%A9rard%20de",
+              url: "/subject_headings/cf347108-e1f2-4c0f-808a-ac4ace2f0765?label=Cortanze%2C%20G%C3%A9rard%20de",
               urlLabel: "Cortanze, Gérard de",
             },
             {
-              url: "/subject_headings/74746d11-638b-4cfb-a72a-9a2bd296e6fd?label=Cortanze,%20G%C3%A9rard%20de%20--%20Childhood%20and%20youth",
+              url: "/subject_headings/74746d11-638b-4cfb-a72a-9a2bd296e6fd?label=Cortanze%2C%20G%C3%A9rard%20de%20--%20Childhood%20and%20youth",
               urlLabel: "Childhood and youth",
             },
           ],
           [
             {
-              url: "/subject_headings/9391bc26-e44c-44ac-98cc-e3800da51926?label=Authors,%20French",
+              url: "/subject_headings/5fd065df-b4e9-48cb-b13c-ea15f36b96b4?label=Authors%2C%20French",
               urlLabel: "Authors, French",
             },
             {
-              url: "/subject_headings/9391bc26-e44c-44ac-98cc-e3800da51926?label=Authors,%20French%20--%2020th%20century",
+              url: "/subject_headings/e43674a7-5f02-44f1-95cd-dbcc776331b7?label=Authors%2C%20French%20--%2020th%20century",
               urlLabel: "20th century",
             },
             {
-              url: "/subject_headings/9391bc26-e44c-44ac-98cc-e3800da51926?label=Authors,%20French%20--%2020th%20century%20--%20Biography",
+              url: "/subject_headings/9391bc26-e44c-44ac-98cc-e3800da51926?label=Authors%2C%20French%20--%2020th%20century%20--%20Biography",
               urlLabel: "Biography",
             },
           ],

diff --git a/src/types/bibDetailsTypes.ts b/src/types/bibDetailsTypes.ts
@@ -1,7 +1,7 @@
 export type AnyBibDetail = BibDetail | LinkedBibDetail | SubjectHeadingDetail
 
 export interface SubjectHeadingDetail {
-  value: Url[][]
+  value: BibDetailURL[][]
   label: string
 }
 
@@ -13,15 +13,15 @@ export interface BibDetail {
 }
 
 export interface LinkedBibDetail {
-  value: Url[]
+  value: BibDetailURL[]
   // label is the formatted name of the field, such as "Author"
   label: string
   // indicates if a linked bib detail is internal, like a link to a creator
   // literal search, or external, like supplementary content
   link?: "internal" | "external"
 }
 
-export interface Url {
+export interface BibDetailURL {
   url: string
   urlLabel: string
 }