refactor: simplify changed version card message logic

folio-org · Nov 29, 2024 · 0bb039f · 0bb039f
1 parent 96b52f3
commit 0bb039f
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 57 deletions.
diff --git a/lib/VersionHistory/VersionCard/VersionCard.js b/lib/VersionHistory/VersionCard/VersionCard.js
@@ -20,7 +20,7 @@ const VersionCard = ({
   id,
   isCurrent,
   isLatest,
-  isSystemChange = false,
+  isOriginal,
   onSelect,
   source,
   title,
@@ -64,22 +64,18 @@ const VersionCard = ({
   ), [id, isCurrent, onSelectVersion, tooltip]);
 
   const message = useMemo(() => {
-    if (isSystemChange) {
+    if (isOriginal) {
       return (
-        <>
-          <FormattedMessage
-            id="stripes-acq-components.versionHistory.card.changed"
-            tagName="b"
-          />
+        <b>
           <FormattedMessage
-            id="stripes-acq-components.versionHistory.card.systemChange"
-            tagName="div"
+            id="stripes-acq-components.versionHistory.card.version.original"
+            tagName="em"
           />
-        </>
+        </b>
       );
     }
 
-    if (changedFields.length) {
+    if (changedFields?.length) {
       return (
         <>
           <FormattedMessage
@@ -97,14 +93,18 @@ const VersionCard = ({
     }
 
     return (
-      <b>
+      <>
+        <FormattedMessage
+          id="stripes-acq-components.versionHistory.card.changed"
+          tagName="b"
+        />
         <FormattedMessage
-          id="stripes-acq-components.versionHistory.card.version.original"
-          tagName="em"
+          id="stripes-acq-components.versionHistory.card.systemChange"
+          tagName="div"
         />
-      </b>
+      </>
     );
-  }, [changedFields, isSystemChange]);
+  }, [changedFields, isOriginal]);
 
   return (
     <Card
@@ -138,7 +138,7 @@ VersionCard.propTypes = {
   id: PropTypes.string.isRequired,
   isCurrent: PropTypes.bool,
   isLatest: PropTypes.bool,
-  isSystemChange: PropTypes.bool,
+  isOriginal: PropTypes.bool,
   onSelect: PropTypes.func.isRequired,
   source: PropTypes.node.isRequired,
   title: PropTypes.node.isRequired,

diff --git a/lib/VersionHistory/VersionCard/VersionCard.test.js b/lib/VersionHistory/VersionCard/VersionCard.test.js
@@ -33,12 +33,6 @@ describe('VersionCard', () => {
     expect(screen.getByText(defaultProps.title)).toBeInTheDocument();
   });
 
-  it('should display version history card with system update', () => {
-    renderVersionCard({ isSystemChange: true });
-
-    expect(screen.getByText(/systemChange/)).toBeInTheDocument();
-  });
-
   it('should call \'onSelect\' when \'View this version\' icon button was clicked', async () => {
     renderVersionCard();
 

diff --git a/lib/VersionHistory/VersionHistoryPane/VersionHistoryPane.js b/lib/VersionHistory/VersionHistoryPane/VersionHistoryPane.js
@@ -59,14 +59,15 @@ const VersionHistoryPane = ({
       userId,
     }, i) => {
       const user = usersMap[userId];
-      const { changedFields, isSystemChange } = getFieldLabels(
+      const changedFields = getFieldLabels(
         intl,
         versionContext?.versionsMap?.[versionId]?.paths,
         labelsMap,
         hiddenFields,
-        systemUpdatedFields,
       );
 
+      const isOriginal = i === versionsToDisplay.length - 1 && !changedFields?.length;
+
       const source = (
         <FormattedMessage
           id="stripes-components.metaSection.source"
@@ -84,11 +85,11 @@ const VersionHistoryPane = ({
           id={versionId}
           isCurrent={versionId === currentVersion}
           isLatest={i === 0}
+          isOriginal={isOriginal}
           onSelect={onSelectVersion}
           title={<FolioFormattedTime dateString={eventDate} />}
           source={source}
           changedFields={changedFields}
-          isSystemChange={isSystemChange}
         />
       );
     })
@@ -99,7 +100,6 @@ const VersionHistoryPane = ({
     hiddenFields,
     labelsMap,
     onSelectVersion,
-    systemUpdatedFields,
     usersMap,
     versionContext?.versionsMap,
     versionsToDisplay,

diff --git a/lib/VersionHistory/getFieldLabels.js b/lib/VersionHistory/getFieldLabels.js
@@ -1,30 +1,22 @@
 import { escapeRegExp, keyBy } from 'lodash';
 
-export const getFieldLabels = (intl, paths, labelsMap = {}, hiddenFields = [], systemUpdatedFields = []) => {
-  if (!paths) return { changedFields: [], systemChanges: [] };
+export const getFieldLabels = (intl, paths, labelsMap = {}, hiddenFields = []) => {
+  if (!paths) return null;
 
   const labelsMapEntries = Object.entries(labelsMap);
   const hiddenFieldsMap = keyBy(hiddenFields);
 
-  const { changedFields, systemChanges } = paths.reduce((acc, path) => {
+  return paths.reduce((acc, path) => {
     const fieldLabel = labelsMapEntries.find(([fieldPath]) => {
-      const regex = new RegExp(`^${escapeRegExp(fieldPath).replaceAll('\\\\d', '\\d')}$`);
+      const regex = new RegExp(`^${escapeRegExp(fieldPath).replace('\\\\d', '\\d')}$`);
 
       return regex.test(path);
     })?.[1] || path;
 
-    if (systemUpdatedFields.includes(path.replace(/\[\d+\]/g, ''))) {
-      acc.systemChanges.push(fieldLabel);
-    } else if (!hiddenFieldsMap[fieldLabel]) {
-      acc.changedFields.push(intl.formatMessage({ id: fieldLabel }));
+    if (!hiddenFieldsMap[fieldLabel]) {
+      acc.push(intl.formatMessage({ id: fieldLabel }));
     }
 
     return acc;
-  }, { changedFields: [], systemChanges: [] });
-
-  return {
-    changedFields: changedFields.sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase())),
-    systemChanges,
-    isSystemChange: !changedFields.length && systemChanges.length,
-  };
+  }, []).sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase()));
 };
diff --git a/lib/VersionHistory/getFieldLabels.test.js b/lib/VersionHistory/getFieldLabels.test.js
@@ -16,12 +16,8 @@ const paths = [
 const intl = { formatMessage: ({ id }) => id };
 
 describe('getFieldLabels', () => {
-  beforeEach(() => {
-    jest.clearAllMocks();
-  });
-
   it('should use keys of labelsMap as regexp to parse labels of paths', () => {
-    expect(getFieldLabels(intl, paths, labelsMap).changedFields).toEqual(expect.arrayContaining([
+    expect(getFieldLabels(intl, paths, labelsMap)).toEqual(expect.arrayContaining([
       ...Object.values(labelsMap),
     ]));
   });
@@ -32,18 +28,9 @@ describe('getFieldLabels', () => {
     };
 
     const pathsWithHiddenFields = [...paths, hiddenFields.donors];
-    const result = getFieldLabels(intl, pathsWithHiddenFields, labelsMap, hiddenFields);
 
-    expect(result.changedFields).toEqual(expect.arrayContaining([
+    expect(getFieldLabels(intl, pathsWithHiddenFields, labelsMap, hiddenFields)).toEqual(expect.arrayContaining([
       ...Object.values(labelsMap),
     ]));
   });
-
-  it('should isSystemChange equal `true` if there are only systemChanges', () => {
-    const systemUpdatedFields = ['fundDistribution'];
-
-    const result = getFieldLabels(intl, ['fundDistribution'], labelsMap, [], systemUpdatedFields);
-
-    expect(result.isSystemChange).toBeTruthy();
-  });
 });