Alerting: Sort StateHistoryItem after fetch instead of on render. (#47842)

PR #47674 attempted to sort a read-only managed async array. This change
moves the sort logic to the fetch code so sort happens once on fetch, to
a mutable array, rather than trying on each render for an immutable
array.

Signed-off-by: Joe Blubaugh <joe.blubaugh@grafana.com>

public/app/features/alerting/unified/api/annotations.test.ts

@@ -1,10 +1,17 @@
 import '@grafana/runtime';
-import { fetchAnnotations } from './annotations';
+import { fetchAnnotations, sortStateHistory } from './annotations';
+import { StateHistoryItem } from 'app/types/unified-alerting';
 
-const get = jest.fn();
+const get = jest.fn(() => {
+  return new Promise((resolve) => {
+    resolve(undefined);
+  });
+});
 
 jest.mock('@grafana/runtime', () => ({
-  getBackendSrv: () => ({ get }),
+  getBackendSrv: () => ({
+    get,
+  }),
 }));
 
 describe('annotations', () => {
@@ -16,3 +23,41 @@ describe('annotations', () => {
     expect(get).toBeCalledWith('/api/annotations', { alertId: ALERT_ID });
   });
 });
+
+describe(sortStateHistory, () => {
+  describe('should stably sort', () => {
+    describe('when timeEnd is different', () => {
+      it('should not sort by rule id', () => {
+        let data: StateHistoryItem[] = [
+          { timeEnd: 23, time: 22, id: 1 } as StateHistoryItem,
+          { timeEnd: 22, time: 21, id: 3 } as StateHistoryItem,
+          { timeEnd: 22, time: 22, id: 2 } as StateHistoryItem,
+          { timeEnd: 24, id: 3 } as StateHistoryItem,
+        ];
+
+        data.sort(sortStateHistory);
+        expect(data[0].timeEnd).toBe(24);
+        expect(data[1].timeEnd).toBe(23);
+        expect(data[2].time).toBe(22);
+        expect(data[3].id).toBe(3);
+      });
+    });
+
+    describe('when only the rule id is different', () => {
+      it('should sort by rule id', () => {
+        let data: StateHistoryItem[] = [
+          { timeEnd: 23, time: 22, id: 1 } as StateHistoryItem,
+          { timeEnd: 23, time: 22, id: 3 } as StateHistoryItem,
+          { timeEnd: 23, time: 22, id: 2 } as StateHistoryItem,
+          { timeEnd: 23, time: 22, id: 6 } as StateHistoryItem,
+        ];
+
+        data.sort(sortStateHistory);
+        expect(data[0].id).toBe(6);
+        expect(data[1].id).toBe(3);
+        expect(data[2].id).toBe(2);
+        expect(data[3].id).toBe(1);
+      });
+    });
+  });
+});

public/app/features/alerting/unified/api/annotations.ts

@@ -2,7 +2,37 @@ import { getBackendSrv } from '@grafana/runtime';
 import { StateHistoryItem } from 'app/types/unified-alerting';
 
 export function fetchAnnotations(alertId: string): Promise<StateHistoryItem[]> {
-  return getBackendSrv().get('/api/annotations', {
+  return getBackendSrv()
+    .get('/api/annotations', {
       alertId,
+    })
+    .then((result) => {
+      return result?.sort(sortStateHistory);
     });
 }
+
+export function sortStateHistory(a: StateHistoryItem, b: StateHistoryItem): number {
+  const compareDesc = (a: number, b: number): number => {
+    // Larger numbers first.
+    if (a > b) {
+      return -1;
+    }
+
+    if (b > a) {
+      return 1;
+    }
+    return 0;
+  };
+
+  const endNeq = compareDesc(a.timeEnd, b.timeEnd);
+  if (endNeq) {
+    return endNeq;
+  }
+
+  const timeNeq = compareDesc(a.time, b.time);
+  if (timeNeq) {
+    return timeNeq;
+  }
+
+  return compareDesc(a.id, b.id);
+}

public/app/features/alerting/unified/components/rules/StateHistory.test.tsx

@@ -1,40 +0,0 @@
-import { StateHistoryItem } from 'app/types/unified-alerting';
-import { sortStateHistory } from './StateHistory';
-
-describe(sortStateHistory, () => {
-  describe('should stably sort', () => {
-    describe('when timeEnd is different', () => {
-      it('should not sort by rule id', () => {
-        let data: StateHistoryItem[] = [
-          { timeEnd: 23, time: 22, id: 1 } as StateHistoryItem,
-          { timeEnd: 22, time: 21, id: 3 } as StateHistoryItem,
-          { timeEnd: 22, time: 22, id: 2 } as StateHistoryItem,
-          { timeEnd: 24, id: 3 } as StateHistoryItem,
-        ];
-
-        data.sort(sortStateHistory);
-        expect(data[0].timeEnd).toBe(24);
-        expect(data[1].timeEnd).toBe(23);
-        expect(data[2].time).toBe(22);
-        expect(data[3].id).toBe(3);
-      });
-    });
-
-    describe('when only the rule id is different', () => {
-      it('should sort by rule id', () => {
-        let data: StateHistoryItem[] = [
-          { timeEnd: 23, time: 22, id: 1 } as StateHistoryItem,
-          { timeEnd: 23, time: 22, id: 3 } as StateHistoryItem,
-          { timeEnd: 23, time: 22, id: 2 } as StateHistoryItem,
-          { timeEnd: 23, time: 22, id: 6 } as StateHistoryItem,
-        ];
-
-        data.sort(sortStateHistory);
-        expect(data[0].id).toBe(6);
-        expect(data[1].id).toBe(3);
-        expect(data[2].id).toBe(2);
-        expect(data[3].id).toBe(1);
-      });
-    });
-  });
-});

public/app/features/alerting/unified/components/rules/StateHistory.tsx

@@ -24,32 +24,6 @@ interface RuleStateHistoryProps {
   alertId: string;
 }
 
-function sortStateHistory(a: StateHistoryItem, b: StateHistoryItem): number {
-  const compareDesc = (a: number, b: number): number => {
-    // Larger numbers first.
-    if (a > b) {
-      return -1;
-    }
-
-    if (b > a) {
-      return 1;
-    }
-    return 0;
-  };
-
-  const endNeq = compareDesc(a.timeEnd, b.timeEnd);
-  if (endNeq) {
-    return endNeq;
-  }
-
-  const timeNeq = compareDesc(a.time, b.time);
-  if (timeNeq) {
-    return timeNeq;
-  }
-
-  return compareDesc(a.id, b.id);
-}
-
 const StateHistory: FC<RuleStateHistoryProps> = ({ alertId }) => {
   const { loading, error, result = [] } = useManagedAlertStateHistory(alertId);
 
@@ -68,7 +42,6 @@ const StateHistory: FC<RuleStateHistoryProps> = ({ alertId }) => {
   ];
 
   const items: StateHistoryRow[] = result
-    .sort(sortStateHistory)
     .reduce((acc: StateHistoryRowItem[], item, index) => {
       acc.push({
         id: String(item.id),
@@ -150,7 +123,4 @@ function hasMatchingPrecedingState(index: number, items: StateHistoryItem[]): bo
   return previousHistoryItem.newState === currentHistoryItem.prevState;
 }
 
-export {
+export { StateHistory };
-  StateHistory,
-  sortStateHistory, // exported for testing.
-};