Bugfix/12323/fix-wrong-status-for-history-groups (#845)

* change calculations for hasMissingEvents

* lint fix

* Make time select in date filter editable (#838)

The datepicker used allows users to select time range. We populate the time select with time interval of 15 minutes. Adding more granular time options hampers the performance of the component. However, the users often need to specify time intervals that does not align with the options present. This change adds the `creatable` flag to the time select to enable this.

Please note that the `creatable` flag in the  underlying BaseWeb component is not quite intuitive.

* fix hasMissingEvents for timeouts

* Create useThrottledState

* use defaultThrottleSettings

* fix failing test

* Show loading indicator and load missing events

* fix lint

* fix types

* fix lint

* revert status changes

* rename helper

* rename method

* remove statusReady from WorkflowHistoryTimelineGroup

---------

Co-authored-by: Pandey Anshuman Kishore <pandeyanshuman@users.noreply.github.com>
Co-authored-by: Adhitya Mamallan <adhitya.mamallan@uber.com>

Author: 3 people

src/config/dynamic/resolvers/clusters.ts

@@ -27,7 +27,7 @@ export default function clusters() {
     ['cadence-frontend']
   );
   if (
-    clusterNames.length !== peers.length &&
+    clusterNames.length !== peers.length ||
     clusterNames.length !== serviceNames.length
   ) {
     throw new Error(

src/views/workflow-history/helpers/__tests__/get-visible-groups-has-missing-events.test.ts

@@ -0,0 +1,68 @@
+import getVisibleGroupsHasMissingEvents from '../get-visible-groups-has-missing-events';
+
+describe('getVisibleGroupsHasMissingEvents', () => {
+  const groupEntries: [string, { hasMissingEvents: boolean }][] = [
+    ['group1', { hasMissingEvents: false }],
+    ['group2', { hasMissingEvents: true }],
+    ['group3', { hasMissingEvents: false }],
+    ['group4', { hasMissingEvents: true }],
+  ];
+
+  it('should return true if any event in the main visible range has missing events', () => {
+    const visibleRanges = {
+      startIndex: 0,
+      endIndex: 2,
+      compactStartIndex: 2,
+      compactEndIndex: 3,
+    };
+    expect(getVisibleGroupsHasMissingEvents(groupEntries, visibleRanges)).toBe(
+      true
+    );
+  });
+
+  it('should return true if any event in the compact visible range has missing events', () => {
+    const visibleRanges = {
+      startIndex: 0,
+      endIndex: 1,
+      compactStartIndex: 2,
+      compactEndIndex: 3,
+    };
+    expect(getVisibleGroupsHasMissingEvents(groupEntries, visibleRanges)).toBe(
+      true
+    );
+  });
+
+  it('should return false if no events in the visible range have missing events', () => {
+    const visibleRanges = {
+      startIndex: 0,
+      endIndex: 0,
+      compactStartIndex: 2,
+      compactEndIndex: 2,
+    };
+    expect(getVisibleGroupsHasMissingEvents(groupEntries, visibleRanges)).toBe(
+      false
+    );
+  });
+
+  it('should handle an empty groupEntries array and return false', () => {
+    const visibleRanges = {
+      startIndex: 0,
+      endIndex: 0,
+      compactStartIndex: 0,
+      compactEndIndex: 0,
+    };
+    expect(getVisibleGroupsHasMissingEvents([], visibleRanges)).toBe(false);
+  });
+
+  it('should handle out of range numbers and return false', () => {
+    const visibleRanges = {
+      startIndex: -1,
+      endIndex: -1,
+      compactStartIndex: 100,
+      compactEndIndex: 200,
+    };
+    expect(getVisibleGroupsHasMissingEvents(groupEntries, visibleRanges)).toBe(
+      false
+    );
+  });
+});

src/views/workflow-history/helpers/get-visible-groups-has-missing-events.ts

@@ -0,0 +1,23 @@
+import type {
+  HistoryEventsGroup,
+  VisibleHistoryGroupRanges,
+} from '../workflow-history.types';
+
+export default function getVisibleGroupsHasMissingEvents(
+  groupEntries: Array<[string, Pick<HistoryEventsGroup, 'hasMissingEvents'>]>,
+  visibleRanges: VisibleHistoryGroupRanges
+): boolean {
+  const { startIndex, endIndex, compactStartIndex, compactEndIndex } =
+    visibleRanges;
+  const visibleHasMissing = groupEntries
+    .slice(startIndex, endIndex + 1)
+    .some(([_, { hasMissingEvents }]) => hasMissingEvents);
+
+  if (visibleHasMissing) return true;
+  const compactVisibleHasMissing = groupEntries
+    .slice(compactStartIndex, compactEndIndex + 1)
+    .some(([_, { hasMissingEvents }]) => hasMissingEvents);
+
+  if (compactVisibleHasMissing) return true;
+  return false;
+}

src/views/workflow-history/hooks/__tests__/use-keep-loading-events.test.ts

@@ -40,6 +40,28 @@ describe('useKeepLoadingEvents', () => {
     expect(fetchNextPageMock).not.toHaveBeenCalled();
   });
 
+  it('should not call fetchNextPage after error when continueLoadingAfterError is false', () => {
+    const { fetchNextPageMock, rerender } = setup({
+      isFetchNextPageError: true,
+      continueLoadingAfterError: false,
+    });
+
+    rerender({ isFetchNextPageError: false });
+
+    expect(fetchNextPageMock).not.toHaveBeenCalled();
+  });
+
+  it('should call fetchNextPage after error when continueLoadingAfterError is true', () => {
+    const { fetchNextPageMock, rerender } = setup({
+      isFetchNextPageError: true,
+      continueLoadingAfterError: true,
+    });
+
+    rerender({ isFetchNextPageError: false });
+
+    expect(fetchNextPageMock).toHaveBeenCalled();
+  });
+
   it('should set stoppedDueToError to true when isFetchNextPageError is true', () => {
     const { result, rerender } = setup({
       isFetchNextPageError: false,

src/views/workflow-history/hooks/use-keep-loading-events.ts

@@ -10,24 +10,29 @@ export default function useKeepLoadingEvents({
   isFetchingNextPage,
   stopAfterEndReached,
   isFetchNextPageError,
+  continueLoadingAfterError,
 }: UseKeepLoadingEventsParams) {
   const reachedAvailableHistoryEnd = useRef(false);
 
-  const stoppedDueToError = useRef(isFetchNextPageError);
-
+  const hadErrorOnce = useRef(isFetchNextPageError);
   // update reachedAvailableHistoryEnd
-  const reached = !hasNextPage || (hasNextPage && isLastPageEmpty);
+  const reached =
+    !hasNextPage || (hasNextPage && isLastPageEmpty && !isFetchNextPageError);
   if (reached && !reachedAvailableHistoryEnd.current)
     reachedAvailableHistoryEnd.current = true;
 
-  // update stoppedDueToError
-  if (isFetchNextPageError && !stoppedDueToError.current)
-    stoppedDueToError.current = true;
+  // update hadErrorOnce
+  if (isFetchNextPageError && !hadErrorOnce.current)
+    hadErrorOnce.current = true;
+
+  const stopDueToError =
+    isFetchNextPageError ||
+    (hadErrorOnce.current && !continueLoadingAfterError);
 
   const canLoadMore =
     shouldKeepLoading &&
     !(stopAfterEndReached && reachedAvailableHistoryEnd.current) &&
-    !stoppedDueToError.current &&
+    !stopDueToError &&
     hasNextPage;
 
   useEffect(() => {
@@ -36,7 +41,7 @@ export default function useKeepLoadingEvents({
 
   return {
     reachedAvailableHistoryEnd: reachedAvailableHistoryEnd.current,
-    stoppedDueToError: stoppedDueToError.current,
+    stoppedDueToError: stopDueToError,
     isLoadingMore: canLoadMore,
   };
 }

src/views/workflow-history/hooks/use-keep-loading-events.types.ts

@@ -6,4 +6,5 @@ export type UseKeepLoadingEventsParams = {
   isFetchingNextPage: boolean;
   isFetchNextPageError: boolean;
   stopAfterEndReached?: boolean;
+  continueLoadingAfterError?: boolean;
 };

src/views/workflow-history/workflow-history-compact-event-card/__tests__/workflow-history-compact-event-card.test.tsx

@@ -67,6 +67,7 @@ function setup(props: Partial<Props>) {
   return {
     ...render(
       <WorkflowHistoryCompactEventCard
+        statusReady
         status="COMPLETED"
         label="test label"
         secondaryLabel="test secondaryLabel"

src/views/workflow-history/workflow-history-compact-event-card/workflow-history-compact-event-card.tsx

@@ -17,6 +17,7 @@ import { type Props } from './workflow-history-compact-event-card.types';
 
 export default function WorkflowHistoryCompactEventCard({
   status,
+  statusReady,
   label,
   badges,
   showLabelPlaceholder,
@@ -37,7 +38,11 @@ export default function WorkflowHistoryCompactEventCard({
       disabled={disabled}
       onClick={onClick}
     >
-      <WorkflowHistoryEventStatusBadge status={status} size="small" />
+      <WorkflowHistoryEventStatusBadge
+        status={status}
+        statusReady={statusReady}
+        size="small"
+      />
       <div className={cls.textContainer}>
         {label && !showLabelPlaceholder && (
           <div className={cls.label}>

src/views/workflow-history/workflow-history-compact-event-card/workflow-history-compact-event-card.types.ts

@@ -5,6 +5,7 @@ import { type HistoryGroupBadge } from '../workflow-history.types';
 
 export type Props = {
   status: WorkflowEventStatus;
+  statusReady: boolean;
   label: string;
   secondaryLabel: string;
   showLabelPlaceholder?: boolean;

src/views/workflow-history/workflow-history-event-status-badge/__tests__/workflow-history-tab-event-status-badge.test.tsx

@@ -1,27 +1,44 @@
 import React from 'react';
 
-import { render } from '@/test-utils/rtl';
+import { render, screen } from '@/test-utils/rtl';
 
 import WorkflowHistoryEventStatusBadge from '../workflow-history-event-status-badge';
 import {
   WORKFLOW_EVENT_STATUS,
   WORKFLOW_EVENT_STATUS_BADGE_SIZES,
 } from '../workflow-history-event-status-badge.constants';
 
+jest.mock('baseui/skeleton', () => ({
+  Skeleton: jest.fn(() => <div>Loading Skeleton</div>),
+}));
+
 describe('WorkflowHistoryEventStatusBadge', () => {
+  it('should show loading skeleton when status is not ready', () => {
+    render(
+      <WorkflowHistoryEventStatusBadge status="CANCELED" statusReady={false} />
+    );
+
+    expect(screen.getByText('Loading Skeleton')).toBeInTheDocument();
+  });
+
   it('should match snapshot when status is not valid and badge should not be rendered', () => {
     const { container } = render(
       // @ts-expect-error invalid status
-      <WorkflowHistoryEventStatusBadge status="INVALID_STATUS" />,
+      <WorkflowHistoryEventStatusBadge status="INVALID_STATUS" statusReady />,
       { isSnapshotTest: true }
     );
     expect(container).toMatchSnapshot();
   });
 
   it('should match snapshot when size is not valid and icon should not be rendered', () => {
     const { container } = render(
-      // @ts-expect-error invalid size
-      <WorkflowHistoryEventStatusBadge status="COMPLETE" size="invalid" />
+      <WorkflowHistoryEventStatusBadge
+        // @ts-expect-error invalid status
+        status="COMPLETE"
+        statusReady
+        // @ts-expect-error invalid size
+        size="invalid"
+      />
     );
     expect(container).toMatchSnapshot();
   });
@@ -30,7 +47,11 @@ describe('WorkflowHistoryEventStatusBadge', () => {
     for (const size of Object.values(WORKFLOW_EVENT_STATUS_BADGE_SIZES)) {
       it(`should match snapshot when status is ${status} and size is ${size}`, () => {
         const { container } = render(
-          <WorkflowHistoryEventStatusBadge status={status} size={size} />,
+          <WorkflowHistoryEventStatusBadge
+            status={status}
+            statusReady
+            size={size}
+          />,
           { isSnapshotTest: true }
         );
         expect(container).toMatchSnapshot();