From 8bb7c88e075bdd432d5de7de5490fed5d56a54db Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Tue, 19 Apr 2022 03:22:35 -0500 Subject: [PATCH] Alerting: Sort StateHistoryItem after fetch instead of on render. (#47842) (#47864) 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 (cherry picked from commit 7d5cb170c6f5b4352a4e3390644458dedf7132b1) Co-authored-by: Joe Blubaugh --- .../alerting/unified/api/annotations.test.ts | 51 +++++++++++++++++-- .../alerting/unified/api/annotations.ts | 36 +++++++++++-- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/public/app/features/alerting/unified/api/annotations.test.ts b/public/app/features/alerting/unified/api/annotations.test.ts index ca52b08ca65..183c103f58c 100644 --- a/public/app/features/alerting/unified/api/annotations.test.ts +++ b/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); + }); + }); + }); +}); diff --git a/public/app/features/alerting/unified/api/annotations.ts b/public/app/features/alerting/unified/api/annotations.ts index 2df5ee2cacd..39195dc1f48 100644 --- a/public/app/features/alerting/unified/api/annotations.ts +++ b/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 { - return getBackendSrv().get('/api/annotations', { - alertId, - }); + 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); }