PR feedback, better fix than “page size= 29”

dagster-io · Nov 7, 2024 · de7719f · de7719f
1 parent e06149c
commit de7719f
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 17 deletions.
diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/RunsFeedRow.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/RunsFeedRow.tsx
@@ -105,9 +105,7 @@ export const RunsFeedRow = ({
             style={{gap: '4px 8px', lineHeight: 0}}
           >
             {entry.__typename === 'PartitionBackfill' ? (
-              <Tag intent="none">
-                <span>Backfill</span>
-              </Tag>
+              <Tag intent="none">Backfill</Tag>
             ) : undefined}
 
             <RunRowTags

diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/useCursorPaginatedQuery.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/useCursorPaginatedQuery.tsx
@@ -24,23 +24,28 @@ interface CursorPaginationQueryVariables {
  */
 export function useCursorPaginatedQuery<T, TVars extends CursorPaginationQueryVariables>(options: {
   query: DocumentNode;
-  nextCursorForResult: (result: T) => string | undefined;
   skip?: boolean;
   variables: Omit<TVars, 'cursor' | 'limit'>;
   pageSize: number;
-  getResultArray: (result: T | undefined) => any[];
   queryKey?: string;
+  getResultArray: (result: T | undefined) => any[];
+  nextCursorForResult: (result: T) => string | undefined;
+  hasMoreForResult?: (result: T) => boolean;
 }) {
   const [cursorStack, setCursorStack] = useState<string[]>(() => []);
   const [cursor, setCursor] = useQueryPersistedState<string | undefined>({
     queryKey: options.queryKey || 'cursor',
   });
 
-  const queryVars: any = {
-    ...options.variables,
-    cursor,
-    limit: options.pageSize + 1,
-  };
+  // If you don't provide a hasMoreForResult function for extracting hasMore from
+  // the response, we fall back to an old approach that fetched one extra item
+  // and used it's presence to determine if more items were available. If you use
+  // the old approach, your `nextCursorForResult` method needs to use
+  // `items[pageSize - 1]` NOT `items[items.length - 1]` to get the next cursor,
+  // or an item will be skipped when you advance.
+  //
+  const limit = options.hasMoreForResult ? options.pageSize : options.pageSize + 1;
+  const queryVars: any = {...options.variables, cursor, limit};
 
   const queryResult = useQuery<T, TVars>(options.query, {
     skip: options.skip,
@@ -49,9 +54,17 @@ export function useCursorPaginatedQuery<T, TVars extends CursorPaginationQueryVa
   });
 
   const resultArray = options.getResultArray(queryResult.data);
+
+  let hasNextCursor = false;
+  if (options.hasMoreForResult) {
+    hasNextCursor = queryResult.data ? options.hasMoreForResult(queryResult.data) : false;
+  } else {
+    hasNextCursor = resultArray.length === options.pageSize + 1;
+  }
+
   const paginationProps: CursorPaginationProps = {
     hasPrevCursor: !!cursor,
-    hasNextCursor: resultArray.length === options.pageSize + 1,
+    hasNextCursor,
     popCursor: () => {
       const nextStack = [...cursorStack];
       setCursor(nextStack.pop());

diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/useRunsFeedEntries.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/useRunsFeedEntries.tsx
@@ -1,3 +1,5 @@
+import {useMemo} from 'react';
+
 import {RUNS_FEED_TABLE_ENTRY_FRAGMENT} from './RunsFeedRow';
 import {useSelectedRunsFeedTab} from './RunsFeedTabs';
 import {SCHEDULED_RUNS_LIST_QUERY} from './ScheduledRunListRoot';
@@ -11,7 +13,7 @@ import {gql, useQuery} from '../apollo-client';
 import {PYTHON_ERROR_FRAGMENT} from '../app/PythonErrorFragment';
 import {RunsFilter} from '../graphql/types';
 
-const PAGE_SIZE = 29;
+const PAGE_SIZE = 30;
 
 export const RUNS_FEED_CURSOR_KEY = `runs_before`;
 
@@ -30,11 +32,17 @@ export function useRunsFeedEntries(
     pageSize: PAGE_SIZE,
     variables: {filter, includeRunsFromBackfills},
     skip: isScheduled,
-    nextCursorForResult: (runs) => {
-      if (runs.runsFeedOrError.__typename !== 'RunsFeedConnection') {
+    nextCursorForResult: (data) => {
+      if (data.runsFeedOrError.__typename !== 'RunsFeedConnection') {
         return undefined;
       }
-      return runs.runsFeedOrError.hasMore ? runs.runsFeedOrError.cursor : undefined;
+      return data.runsFeedOrError.hasMore ? data.runsFeedOrError.cursor : undefined;
+    },
+    hasMoreForResult: (data) => {
+      if (data.runsFeedOrError.__typename !== 'RunsFeedConnection') {
+        return false;
+      }
+      return data.runsFeedOrError.hasMore;
     },
     getResultArray: (data) => {
       if (!data || data.runsFeedOrError.__typename !== 'RunsFeedConnection') {
@@ -46,8 +54,11 @@ export function useRunsFeedEntries(
 
   const data = queryResult.data || queryResult.previousData;
 
-  const entries =
-    data?.runsFeedOrError.__typename === 'RunsFeedConnection' ? data?.runsFeedOrError.results : [];
+  const entries = useMemo(() => {
+    return data?.runsFeedOrError.__typename === 'RunsFeedConnection'
+      ? data?.runsFeedOrError.results
+      : [];
+  }, [data]);
 
   const scheduledQueryResult = useQuery<ScheduledRunsListQuery, ScheduledRunsListQueryVariables>(
     SCHEDULED_RUNS_LIST_QUERY,