Improve Latest Changes section on Stop Details Page

Improve the change listing to better align with what is to come with
the the Stop Area Latest Changes implementation.

- Improved general utilities.
- Dropped separate query and replaced with the normal history query:
  This ensures we can always access the proper previous version, even
  if multiple copies of the stop have been created and/or edited recently.
- Fixed data fetch error retry button to actually refetch the data.
- Added test ids for the data fetch loading and error states.

Author: Huulivoide

cypress/e2e/stop-registry/stopDetails.cy.ts

@@ -3087,7 +3087,7 @@ describe('Stop details', { tags: [Tag.StopRegistry] }, () => {
         .shouldBeVisible()
         .should('contain.text', 'Muutoshistoria');
 
-      StopDetailsPage.latestChangeHistory.getItems().should('have.length', 1);
+      StopDetailsPage.latestChangeHistory.getItems().should('have.length', 3);
 
       cy.section('Make multiple changes to create history', () => {
         // Change 1

cypress/pageObjects/stop-registry/StopDetailsPage.ts

@@ -101,8 +101,9 @@ export class StopDetailsPage {
     title: () => cy.getByTestId('LatestStopChangeHistoryTable::Title'),
     showAllLink: () =>
       cy.getByTestId('LatestStopChangeHistoryTable::ShowAllLink'),
-    getItems: () => cy.getByTestId('LatestStopChangeHistoryTable::Item'),
+    getItems: () =>
+      cy.get('[data-testid^="LatestStopChangeHistoryTable::Item"]'),
     getNthItem: (index: number) =>
-      cy.getByTestId('LatestStopChangeHistoryTable::Item').eq(index),
+      cy.get('[data-testid^="LatestStopChangeHistoryTable::Item"]').eq(index),
   };
 }

ui/src/components/common/ChangeHistory/latest/ErrorLoadingState.tsx

@@ -3,8 +3,8 @@ import { useTranslation } from 'react-i18next';
 import { SimpleButton } from '../../../../uiComponents';
 
 type ErrorLoadingStateProps = {
-  readonly onRetry?: () => void;
-  readonly testIdPrefix?: string;
+  readonly onRetry: () => void;
+  readonly testIdPrefix: string;
 };
 
 export const ErrorLoadingState: FC<ErrorLoadingStateProps> = ({

ui/src/components/common/ChangeHistory/latest/LatestChangeHistoryItem.tsx

@@ -1,70 +1,116 @@
-import { ReactNode, isValidElement } from 'react';
+import { FC, isValidElement } from 'react';
 import { FaPlay } from 'react-icons/fa';
 import { Link, To } from 'react-router';
 import { useGetUserNames } from '../../../../hooks';
-import { mapUTCToDateTime } from '../../../../time';
+import { mapToShortDateTime } from '../../../../time';
 import { EmptyCell } from '../EmptyCell';
-import { ChangedValue } from '../types';
+import { BaseChangeHistoryItemDetails, ChangedValue } from '../types';
+
+function changeKey(change: ChangedValue): string {
+  return change.key ?? change.field;
+}
+
+const testIds = {
+  change: (change: ChangedValue) =>
+    `LatestChangeHistoryItemChange::${changeKey(change)}`,
+  fieldName: 'LatestChangeHistoryItemChange::FieldName',
+  oldValue: 'LatestChangeHistoryItemChange::OldValue',
+  newValue: 'LatestChangeHistoryItemChange::NewValue',
+};
 
 type ChangeSection = {
   readonly title: string;
   readonly changes: ReadonlyArray<ChangedValue>;
 };
 
+function isEmptyCell(value: unknown): boolean {
+  return isValidElement(value) && value.type === EmptyCell;
+}
+
+type ChangeProps = {
+  readonly changedValue: ChangedValue;
+};
+const LatestChangeHistoryItemChange: FC<ChangeProps> = ({ changedValue }) => {
+  const { field, newValue, oldValue } = changedValue;
+  const hasOldValue = oldValue && !isEmptyCell(oldValue);
+
+  return (
+    <div data-testid={testIds.change(changedValue)}>
+      <span data-testid={testIds.fieldName}>{field}</span>
+      <span>{': '}</span>
+
+      {hasOldValue ? (
+        <>
+          <span data-testid={testIds.oldValue}>{oldValue}</span>{' '}
+          <FaPlay className="mx-1 inline text-[8px]" />{' '}
+        </>
+      ) : (
+        <span data-testid={testIds.oldValue} />
+      )}
+
+      <span data-testid={testIds.newValue}>{newValue}</span>
+    </div>
+  );
+};
+
+type SectionProps = ChangeSection & {
+  readonly historyItem: BaseChangeHistoryItemDetails;
+  readonly link: To;
+};
+const LatestChangeHistoryItemSection: FC<SectionProps> = ({
+  changes,
+  historyItem,
+  link,
+  title,
+}) => {
+  return (
+    <div>
+      <Link to={link} className="font-semibold text-brand hover:underline">
+        {/* eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing */}
+        {historyItem.versionComment || title}
+      </Link>
+      {changes.map((changedValue) => (
+        <LatestChangeHistoryItemChange
+          key={changeKey(changedValue)}
+          changedValue={changedValue}
+        />
+      ))}
+    </div>
+  );
+};
+
 type LatestChangeHistoryItemProps = {
-  readonly historyItem: {
-    readonly changed?: string | null;
-    readonly changedBy?: string | null;
-    readonly versionComment?: string | null;
-  };
+  readonly historyItem: BaseChangeHistoryItemDetails;
   readonly sections: ReadonlyArray<ChangeSection>;
   readonly link: To;
   readonly testId: string;
 };
 
-function isEmptyCell(value: unknown): boolean {
-  return isValidElement(value) && value.type === EmptyCell;
-}
-
-export const LatestChangeHistoryItem = ({
+export const LatestChangeHistoryItem: FC<LatestChangeHistoryItemProps> = ({
   historyItem,
   sections,
   link,
   testId,
-}: LatestChangeHistoryItemProps): ReactNode => {
+}) => {
   const { getUserNameById } = useGetUserNames();
 
   const changedBy = getUserNameById(historyItem.changedBy) ?? 'HSL';
-  const changedAt = mapUTCToDateTime(historyItem.changed);
+  const changedAt = mapToShortDateTime(historyItem.changed);
 
   if (sections.length === 0) {
     return null;
   }
 
   return (
     <div className="mb-3 text-sm" data-testid={testId}>
-      {sections.map((section) => (
-        <div key={section.title}>
-          <Link to={link} className="font-semibold text-brand hover:underline">
-            {/* eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing */}
-            {historyItem.versionComment || section.title}
-          </Link>
-          {section.changes.map((c) => {
-            const hasOldValue = c.oldValue && !isEmptyCell(c.oldValue);
-            return (
-              <span className="block" key={c.key ?? c.field}>
-                {c.field && `${c.field}: `}
-                {hasOldValue && (
-                  <>
-                    {c.oldValue}{' '}
-                    <FaPlay className="mx-1 inline text-[8px]" />{' '}
-                  </>
-                )}
-                {c.newValue}
-              </span>
-            );
-          })}
-        </div>
+      {sections.map(({ title, changes }) => (
+        <LatestChangeHistoryItemSection
+          key={title}
+          changes={changes}
+          historyItem={historyItem}
+          link={link}
+          title={title}
+        />
       ))}
       <div>
         {changedAt} | {changedBy}

ui/src/components/common/ChangeHistory/latest/LoadingState.tsx

@@ -2,7 +2,7 @@ import { FC } from 'react';
 import { useTranslation } from 'react-i18next';
 
 type LoadingStateProps = {
-  readonly testIdPrefix?: string;
+  readonly testIdPrefix: string;
 };
 
 export const LoadingState: FC<LoadingStateProps> = ({

ui/src/components/common/ChangeHistory/types/BaseChangeHistoryItem.ts

@@ -5,4 +5,5 @@ export type BaseChangeHistoryItemDetails = {
   readonly changedBy?: string | null;
   readonly validityEnd?: DateLike | null;
   readonly validityStart?: DateLike | null;
+  readonly versionComment?: string | null;
 };

ui/src/components/stop-registry/stops/change-history/components/LatestStopChangeDataDiff.tsx

@@ -0,0 +1,76 @@
+import { FC } from 'react';
+import { useTranslation } from 'react-i18next';
+import { QuayChangeHistoryItem } from '../../../../../generated/graphql';
+import { Path, routeDetails } from '../../../../../router/routeDetails';
+import { Priority } from '../../../../../types/enums';
+import {
+  ErrorLoadingState,
+  LatestChangeHistoryItem,
+  LoadingState,
+} from '../../../../common/ChangeHistory/latest';
+import { latestStopChangeSections } from '../utils/latestStopChangeSections';
+import {
+  useHistoricalStopVersion,
+  useRefetchFailedHistoricalStopVersions,
+} from './HistoricalStopDataProvider';
+
+const testIds = {
+  prefix: 'LatestStopChangeHistoryTable::Item:',
+  diff: 'LatestStopChangeHistoryTable::Item::Diffs',
+};
+
+type LatestStopChangeDataDiffProps = {
+  readonly historyItem: QuayChangeHistoryItem;
+  readonly previousHistoryItem: QuayChangeHistoryItem;
+  readonly publicCode: string;
+  readonly priority: Priority;
+};
+
+export const LatestStopChangeDataDiff: FC<LatestStopChangeDataDiffProps> = ({
+  historyItem,
+  previousHistoryItem,
+  publicCode,
+  priority,
+}) => {
+  const { t } = useTranslation();
+
+  const currentCached = useHistoricalStopVersion(historyItem);
+  const previousCached = useHistoricalStopVersion(previousHistoryItem);
+  const refetchFailed = useRefetchFailedHistoricalStopVersions();
+
+  if (
+    currentCached.status === 'fetching' ||
+    previousCached.status === 'fetching'
+  ) {
+    return <LoadingState testIdPrefix={testIds.prefix} />;
+  }
+
+  if (
+    currentCached.status !== 'fetched' ||
+    previousCached.status !== 'fetched'
+  ) {
+    return (
+      <ErrorLoadingState
+        onRetry={refetchFailed}
+        testIdPrefix={testIds.prefix}
+      />
+    );
+  }
+
+  const sections = latestStopChangeSections(
+    t,
+    previousCached.value,
+    currentCached.value,
+  );
+
+  return (
+    <LatestChangeHistoryItem
+      historyItem={historyItem}
+      sections={sections}
+      link={routeDetails[Path.stopChangeHistory].getLink(publicCode, {
+        priority: priority === Priority.Standard ? undefined : priority,
+      })}
+      testId={testIds.diff}
+    />
+  );
+};

ui/src/components/stop-registry/stops/change-history/components/LatestStopChangeHistoryItem.tsx

@@ -4,25 +4,22 @@ import { Link } from 'react-router';
 import { QuayChangeHistoryItem } from '../../../../../generated/graphql';
 import { Path, routeDetails } from '../../../../../router/routeDetails';
 import { Priority } from '../../../../../types/enums';
-import {
-  ErrorLoadingState,
-  LatestChangeHistoryItem,
-  LoadingState,
-} from '../../../../common/ChangeHistory/latest';
-import { latestStopChangeSections } from '../utils/latestStopChangeSections';
-import { useHistoricalStopVersion } from './HistoricalStopDataProvider';
+import { NoEarlierVersionExists } from '../../../../common/ChangeHistory';
+import { LatestStopChangeDataDiff } from './LatestStopChangeDataDiff';
 import {
   determineType,
   getHeadingText,
 } from './NoPreviousChangeVersionSection';
 
 const testIds = {
-  item: 'LatestStopChangeHistoryTable::Item',
+  newItem: 'LatestStopChangeHistoryTable::Item::NewItem',
 };
 
 type LatestStopChangeHistoryItemProps = {
   readonly historyItem: QuayChangeHistoryItem;
-  readonly previousHistoryItem: QuayChangeHistoryItem | null;
+  readonly previousHistoryItem:
+    | QuayChangeHistoryItem
+    | typeof NoEarlierVersionExists;
   readonly publicCode: string;
   readonly priority: Priority;
 };
@@ -32,53 +29,28 @@ export const LatestStopChangeHistoryItem: FC<
 > = ({ historyItem, previousHistoryItem, publicCode, priority }) => {
   const { t } = useTranslation();
 
-  const currentCached = useHistoricalStopVersion(historyItem);
-  const previousCached = useHistoricalStopVersion(
-    previousHistoryItem ?? historyItem,
-  );
-
   const link = routeDetails[Path.stopChangeHistory].getLink(publicCode, {
     priority: priority === Priority.Standard ? undefined : priority,
   });
 
-  if (!previousHistoryItem) {
+  if (previousHistoryItem === NoEarlierVersionExists) {
     const type = determineType(historyItem);
     const versionText = getHeadingText(t, type);
     return (
-      <div className="mb-3 text-sm font-semibold" data-testid={testIds.item}>
+      <div className="mb-3 text-sm font-semibold" data-testid={testIds.newItem}>
         <Link to={link} className="text-brand hover:underline">
           {versionText}
         </Link>
       </div>
     );
   }
 
-  if (
-    currentCached?.status === 'fetching' ||
-    previousCached?.status === 'fetching'
-  ) {
-    return <LoadingState />;
-  }
-
-  if (
-    currentCached?.status !== 'fetched' ||
-    previousCached?.status !== 'fetched'
-  ) {
-    return <ErrorLoadingState />;
-  }
-
-  const sections = latestStopChangeSections(
-    t,
-    previousCached.value,
-    currentCached.value,
-  );
-
   return (
-    <LatestChangeHistoryItem
+    <LatestStopChangeDataDiff
       historyItem={historyItem}
-      sections={sections}
-      link={link}
-      testId={testIds.item}
+      previousHistoryItem={previousHistoryItem}
+      priority={priority}
+      publicCode={publicCode}
     />
   );
 };