[RAM] fix bug by simplifying logic (#132867)

* fix bug by simplifying logic

* simplify more with more test

x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_table/alerts_table.tsx

@@ -55,7 +55,6 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = (props: AlertsTab
     onPageChange,
     pageIndex: activePage,
     pageSize: props.pageSize,
-    alertsCount,
   });
 
   const [visibleColumns, setVisibleColumns] = useState(props.visibleColumns);
@@ -125,7 +124,7 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = (props: AlertsTab
 
   const handleFlyoutClose = useCallback(() => setFlyoutAlertIndex(-1), [setFlyoutAlertIndex]);
 
-  // TODO when every solution is using this table, we will be bale to simplify it by just passing the alert index
+  // TODO when every solution is using this table, we will be able to simplify it by just passing the alert index
   const handleFlyoutAlert = useCallback(
     (alert) => {
       const idx = alerts.findIndex((a) =>

x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_table/alerts_table_state.tsx

@@ -193,7 +193,7 @@ const AlertsTableState = ({
       disabledCellActions: [],
       flyoutState,
       pageSize: pagination.pageSize,
-      pageSizeOptions: [1, 2, 5, 10, 20, 50, 100],
+      pageSizeOptions: [10, 20, 50, 100],
       leadingControlColumns: [],
       showCheckboxes,
       showExpandToDetails,

x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_table/hooks/use_pagination.test.ts

@@ -11,25 +11,20 @@ describe('usePagination', () => {
   const onPageChange = jest.fn();
   const pageIndex = 0;
   const pageSize = 10;
-  const alertsCount = 5;
 
   beforeEach(() => {
     onPageChange.mockClear();
   });
 
   it('should return the pagination information and callback functions', () => {
-    const { result } = renderHook(() =>
-      usePagination({ onPageChange, pageIndex, pageSize, alertsCount })
-    );
+    const { result } = renderHook(() => usePagination({ onPageChange, pageIndex, pageSize }));
     expect(result.current.pagination).toStrictEqual({ pageIndex, pageSize });
     expect(result.current.onChangePageSize).toBeDefined();
     expect(result.current.onChangePageIndex).toBeDefined();
   });
 
   it('should change the pagination when `onChangePageSize` is called', () => {
-    const { result } = renderHook(() =>
-      usePagination({ onPageChange, pageIndex, pageSize, alertsCount })
-    );
+    const { result } = renderHook(() => usePagination({ onPageChange, pageIndex, pageSize }));
 
     act(() => {
       result.current.onChangePageSize(20);
@@ -39,9 +34,7 @@ describe('usePagination', () => {
   });
 
   it('should change the pagination when `onChangePageIndex` is called', () => {
-    const { result } = renderHook(() =>
-      usePagination({ onPageChange, pageIndex, pageSize, alertsCount })
-    );
+    const { result } = renderHook(() => usePagination({ onPageChange, pageIndex, pageSize }));
 
     act(() => {
       result.current.onChangePageIndex(1);
@@ -51,9 +44,7 @@ describe('usePagination', () => {
   });
 
   it('should paginate the alert flyout', () => {
-    const { result } = renderHook(() =>
-      usePagination({ onPageChange, pageIndex, pageSize, alertsCount })
-    );
+    const { result } = renderHook(() => usePagination({ onPageChange, pageIndex, pageSize }));
 
     expect(result.current.flyoutAlertIndex).toBe(-1);
 
@@ -76,26 +67,8 @@ describe('usePagination', () => {
     expect(result.current.flyoutAlertIndex).toBe(0);
   });
 
-  it('should paginate the flyout when we need to change the page index going back', () => {
-    const { result } = renderHook(() =>
-      usePagination({ onPageChange, pageIndex: 0, pageSize: 1, alertsCount })
-    );
-
-    act(() => {
-      result.current.onPaginateFlyout(-2);
-    });
-
-    // It should reset to the first alert in the table
-    expect(result.current.flyoutAlertIndex).toBe(0);
-
-    // It should go to the last page
-    expect(result.current.pagination).toStrictEqual({ pageIndex: 4, pageSize: 1 });
-  });
-
   it('should paginate the flyout when we need to change the page index going forward', () => {
-    const { result } = renderHook(() =>
-      usePagination({ onPageChange, pageIndex: 0, pageSize: 1, alertsCount })
-    );
+    const { result } = renderHook(() => usePagination({ onPageChange, pageIndex: 0, pageSize: 1 }));
 
     act(() => {
       result.current.onPaginateFlyout(1);
@@ -107,4 +80,53 @@ describe('usePagination', () => {
     // It should go to the first page
     expect(result.current.pagination).toStrictEqual({ pageIndex: 1, pageSize: 1 });
   });
+
+  it('should paginate the flyout when we need to change the page index going forward using odd-count data', () => {
+    const { result } = renderHook(() => usePagination({ onPageChange, pageIndex: 0, pageSize: 7 }));
+
+    act(() => {
+      result.current.onPaginateFlyout(1);
+    });
+    expect(result.current.flyoutAlertIndex).toBe(1);
+
+    /*
+     * 7 is the next first item in the next page
+     */
+    act(() => {
+      result.current.onPaginateFlyout(7);
+    });
+    expect(result.current.flyoutAlertIndex).toBe(0);
+    expect(result.current.pagination).toStrictEqual({ pageIndex: 1, pageSize: 7 });
+    act(() => {
+      result.current.onPaginateFlyout(8);
+    });
+    expect(result.current.flyoutAlertIndex).toBe(1);
+    expect(result.current.pagination).toStrictEqual({ pageIndex: 1, pageSize: 7 });
+
+    // Let's make sure we are not breaking the logic when we ask for the same index
+    act(() => {
+      result.current.onPaginateFlyout(8);
+    });
+    expect(result.current.flyoutAlertIndex).toBe(1);
+
+    // Now let's make sure that we get an older page
+    act(() => {
+      result.current.onPaginateFlyout(12);
+    });
+    expect(result.current.flyoutAlertIndex).toBe(5);
+  });
+
+  it('should paginate the flyout when we need to change the page index going back', () => {
+    const { result } = renderHook(() => usePagination({ onPageChange, pageIndex: 0, pageSize: 1 }));
+
+    act(() => {
+      result.current.onPaginateFlyout(-1);
+    });
+
+    // It should reset to the first alert in the table
+    expect(result.current.flyoutAlertIndex).toBe(0);
+
+    // It should go to the last page
+    expect(result.current.pagination).toStrictEqual({ pageIndex: 0, pageSize: 1 });
+  });
 });

x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_table/hooks/use_pagination.ts

@@ -9,7 +9,6 @@ import { RuleRegistrySearchRequestPagination } from '@kbn/rule-registry-plugin/c
 
 type PaginationProps = RuleRegistrySearchRequestPagination & {
   onPageChange: (pagination: RuleRegistrySearchRequestPagination) => void;
-  alertsCount: number;
 };
 
 export type UsePagination = (props: PaginationProps) => {
@@ -22,7 +21,7 @@ export type UsePagination = (props: PaginationProps) => {
   setFlyoutAlertIndex: (alertIndex: number) => void;
 };
 
-export function usePagination({ onPageChange, pageIndex, pageSize, alertsCount }: PaginationProps) {
+export function usePagination({ onPageChange, pageIndex, pageSize }: PaginationProps) {
   const [pagination, setPagination] = useState<RuleRegistrySearchRequestPagination>({
     pageIndex,
     pageSize,
@@ -47,34 +46,28 @@ export function usePagination({ onPageChange, pageIndex, pageSize, alertsCount }
     [setPagination, onPageChange, pagination.pageSize]
   );
 
-  const paginateFlyout = useCallback(
-    (newFlyoutAlertIndex: number) => {
-      const lastPage = Math.floor(alertsCount / pagination.pageSize) - 1;
-      if (newFlyoutAlertIndex < 0) {
-        setFlyoutAlertIndex(pagination.pageSize - 1);
-        onChangePageIndex(pagination.pageIndex === 0 ? lastPage : pagination.pageIndex - 1);
-        return;
-      }
-
-      if (newFlyoutAlertIndex >= pagination.pageSize) {
-        setFlyoutAlertIndex(0);
-        onChangePageIndex(
-          pagination.pageIndex === lastPage ? 0 : Math.min(pagination.pageIndex + 1, lastPage)
-        );
-        return;
-      }
-
-      setFlyoutAlertIndex(newFlyoutAlertIndex);
-    },
-    [pagination, alertsCount, onChangePageIndex]
-  );
-
   const onPaginateFlyout = useCallback(
     (nextPageIndex: number) => {
-      nextPageIndex -= pagination.pageSize * pagination.pageIndex;
-      paginateFlyout(nextPageIndex);
+      setFlyoutAlertIndex((prevFlyoutAlertIndex) => {
+        if (nextPageIndex < 0) {
+          onChangePageIndex(0);
+          return 0;
+        }
+        const actualPageIndex = pagination.pageSize * pagination.pageIndex + prevFlyoutAlertIndex;
+        if (nextPageIndex === actualPageIndex) {
+          return prevFlyoutAlertIndex;
+        }
+
+        const newPageIndex = Math.floor(nextPageIndex / pagination.pageSize);
+        const newAlertIndex =
+          nextPageIndex >= pagination.pageSize * newPageIndex
+            ? nextPageIndex - pagination.pageSize * newPageIndex
+            : nextPageIndex;
+        onChangePageIndex(newPageIndex);
+        return newAlertIndex;
+      });
     },
-    [paginateFlyout, pagination.pageSize, pagination.pageIndex]
+    [onChangePageIndex, pagination.pageIndex, pagination.pageSize]
   );
 
   return {