Merge branch 'main' into ms&sy_first

.github/workflows/ci-global.yml

@@ -6,6 +6,10 @@ on:
       - "main"
 
   pull_request:
+    paths-ignore:
+      - "docs/**"
+      - "hack/**"
+      - "*.md"
     branches:
       - "main"
 

.github/workflows/ci-image-build.yml

@@ -2,6 +2,10 @@ name: CI (test image build for a PR with build related changes)
 
 on:
   pull_request:
+    paths-ignore:
+      - "docs/**"
+      - "hack/**"
+      - "*.md"
     branches:
       - "main"
       - "release-*"

.github/workflows/ci-repo.yml

@@ -20,26 +20,77 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  unit-test-lookup-image:
+  unit-test-lookup:
     runs-on: ubuntu-latest
     outputs:
       builder-image: ${{ steps.grepBuilder.outputs.builder }}
+      should-test: ${{ steps.check-changes.outputs.should-test }}
+
     steps:
       - uses: actions/checkout@v4
 
       - name: Lookup builder image from the project's Dockerfile
         id: grepBuilder
         run: |
           builder=$(grep 'as builder' Dockerfile | sed -e 's/^FROM \(.*\) as builder$/\1/')
-          echo "Builder image: \`$builder\`" >> "$GITHUB_STEP_SUMMARY"
           echo "builder=$builder" >> "$GITHUB_OUTPUT"
 
+      - name: Did docs and hacks change?
+        id: docs-and-hacks
+        uses: tj-actions/changed-files@v44
+        with:
+          files: |
+            docs/**
+            hack/**
+            *.md
+
+      - name: Check if only docs and hacks changes have been made in a PR
+        id: check-changes
+        env:
+          IS_PR: ${{ !!github.event.pull_request }}
+          ONLY_DOCS: ${{ steps.docs-and-hacks.outputs.only_modified }}
+        run: |
+          SHOULD_TEST=$(
+            if [[ $IS_PR == true ]] && [[ $ONLY_DOCS == true ]]; then
+              echo "false"
+            else
+              echo "true"
+            fi
+          )
+
+          echo "is-pr=$IS_PR" >> "$GITHUB_OUTPUT"
+          echo "changes_only_docs=${ONLY_DOCS:-false}" >> "$GITHUB_OUTPUT"
+          echo "should-test=$SHOULD_TEST" >> "$GITHUB_OUTPUT"
+
+      - name: Summarize findings
+        env:
+          ONLY_DOCS: ${{ steps.docs-and-hacks.outputs.only_modified }}
+          MODIFIED_FILES: ${{ steps.docs-and-hacks.outputs.all_modified_files }}
+        run: |
+          cat >> "$GITHUB_STEP_SUMMARY" <<EOF
+          ## Build container
+            - CI will run on container: \`${{ steps.grepBuilder.outputs.builder }}\`
+
+          ## Findings
+            - The action is PR triggered? \`${{ steps.check-changes.outputs.is-pr }}\`
+            - Changes are only to docs and hacks? \`${{ steps.check-changes.outputs.changes_only_docs }}\`
+            - Should the unit test run? \`${{ steps.check-changes.outputs.should-test }}\`
+          EOF
+
+          if [[ $ONLY_DOCS == true ]] && [[ -n "$MODIFIED_FILES" ]]; then
+            echo "## Modified docs and hacks files that do not impact the build" >> "$GITHUB_STEP_SUMMARY"
+            for file in ${MODIFIED_FILES}; do
+              echo "  - \`$file\`" >> "$GITHUB_STEP_SUMMARY"
+            done
+          fi
+
   unit-test:
     runs-on: ubuntu-latest
-    needs: unit-test-lookup-image
+    needs: unit-test-lookup
+    if: ${{ needs.unit-test-lookup.outputs.should-test == 'true' }}
 
     # Use the same container as the Dockerfile's "FROM * as builder"
-    container: ${{ needs.unit-test-lookup-image.outputs.builder-image }}
+    container: ${{ needs.unit-test-lookup.outputs.builder-image }}
 
     steps:
       - uses: actions/checkout@v4

.github/workflows/image-build.yaml

@@ -17,8 +17,8 @@ jobs:
   image-build:
     uses: konveyor/release-tools/.github/workflows/build-push-images.yaml@main
     with:
-      registry: "quay.io/konveyor"
-      image_name: "tackle2-ui"
+      registry: ${{ vars.IMAGE_BUILD_REGISTRY || 'quay.io/konveyor' }}
+      image_name: ${{ vars.IMAGE_BUILD_IMAGE_NAME || 'tackle2-ui' }}
       containerfile: "./Dockerfile"
 
       # keep the architectures in sync with `ci-image-build.yml`

client/src/app/components/FilterToolbar/SelectFilterControl.tsx

@@ -5,6 +5,7 @@ import {
   Select,
   SelectList,
   SelectOption,
+  ToolbarChip,
   ToolbarFilter,
 } from "@patternfly/react-core";
 import { IFilterControlProps } from "./FilterControl";
@@ -56,8 +57,9 @@ export const SelectFilterControl = <TItem, TFilterCategoryKey extends string>({
     setIsFilterDropdownOpen(false);
   };
 
-  const onFilterClear = (chip: string) => {
-    const newValue = filterValue?.filter((val) => val !== chip);
+  const onFilterClear = (chip: string | ToolbarChip) => {
+    const chipValue = typeof chip === "string" ? chip : chip.key;
+    const newValue = filterValue?.filter((val) => val !== chipValue);
     setFilterValue(newValue?.length ? newValue : null);
   };
 
@@ -90,7 +92,7 @@ export const SelectFilterControl = <TItem, TFilterCategoryKey extends string>({
     <ToolbarFilter
       id={`filter-control-${category.categoryKey}`}
       chips={chips}
-      deleteChip={(_, chip) => onFilterClear(chip as string)}
+      deleteChip={(_, chip) => onFilterClear(chip)}
       categoryName={category.title}
       showToolbarItem={showToolbarItem}
     >

client/src/app/components/InfiniteScroller/InfiniteScroller.tsx

@@ -1,4 +1,4 @@
-import React, { ReactNode, useEffect, useRef } from "react";
+import React, { ReactNode, useEffect, useState } from "react";
 import { useTranslation } from "react-i18next";
 import { useVisibilityTracker } from "./useVisibilityTracker";
 import "./InfiniteScroller.css";
@@ -12,47 +12,46 @@ export interface InfiniteScrollerProps {
   hasMore: boolean;
   // number of items currently displayed/known
   itemCount: number;
+  pageSize: number;
 }
 
 export const InfiniteScroller = ({
   children,
   fetchMore,
   hasMore,
   itemCount,
+  pageSize,
 }: InfiniteScrollerProps) => {
   const { t } = useTranslation();
-  // Track how many items were known at time of triggering the fetch.
-  // This allows to detect edge case when second(or more) fetchMore() is triggered before
-  // IntersectionObserver is able to detect out-of-view event.
-  // Initializing with zero ensures that the effect will be triggered immediately
-  // (parent is expected to display empty state until some items are available).
-  const itemCountRef = useRef(0);
+  const [readyForFetch, setReadyForFetch] = useState(false);
   const { visible: isSentinelVisible, nodeRef: sentinelRef } =
     useVisibilityTracker({
       enable: hasMore,
     });
+  useEffect(() => {
+    // enable or clear the flag depending on the sentinel visibility
+    setReadyForFetch(!!isSentinelVisible);
+  }, [isSentinelVisible]);
 
-  useEffect(
-    () => {
-      if (
-        isSentinelVisible &&
-        itemCountRef.current !== itemCount &&
-        fetchMore() // fetch may be blocked if background refresh is in progress (or other manual fetch)
-      ) {
-        // fetchMore call was triggered (it may fail but will be subject to React Query retry policy)
-        itemCountRef.current = itemCount;
-      }
-    },
+  useEffect(() => {
+    if (readyForFetch) {
+      // clear the flag if fetch request is accepted
+      setReadyForFetch(!fetchMore());
+    }
     // reference to fetchMore() changes based on query state and ensures that the effect is triggered in the right moment
     // i.e. after fetch triggered by the previous fetchMore() call finished
-    [isSentinelVisible, fetchMore, itemCount]
-  );
+  }, [fetchMore, readyForFetch]);
 
   return (
     <div>
       {children}
       {hasMore && (
-        <div ref={sentinelRef} className="infinite-scroll-sentinel">
+        <div
+          ref={sentinelRef}
+          // re-create the node with every page change to force new Intersection observer
+          key={Math.ceil(itemCount / pageSize)}
+          className="infinite-scroll-sentinel"
+        >
           {t("message.loadingTripleDot")}
         </div>
       )}

client/src/app/components/InfiniteScroller/useVisibilityTracker.tsx

@@ -2,7 +2,7 @@ import { useEffect, useRef, useState, useCallback } from "react";
 
 export function useVisibilityTracker({ enable }: { enable: boolean }) {
   const nodeRef = useRef<HTMLDivElement>(null);
-  const [visible, setVisible] = useState(false);
+  const [visible, setVisible] = useState<boolean | undefined>(false);
   const node = nodeRef.current;
 
   // state is set from IntersectionObserver callbacks which may not align with React lifecycle
@@ -22,14 +22,19 @@ export function useVisibilityTracker({ enable }: { enable: boolean }) {
   }, []);
 
   useEffect(() => {
+    if (enable && !node) {
+      // use falsy value different than initial value - state change will trigger render()
+      // otherwise we need to wait for the next render() to read node ref
+      setVisibleSafe(undefined);
+      return undefined;
+    }
+
     if (!enable || !node) {
       return undefined;
     }
 
     // Observer with default options - the whole view port used.
     // Note that if root element is used then it needs to be the ancestor of the target.
-    // In case of infinite scroller the target is always within the (scrollable!)parent
-    // even if the node is technically hidden from the user.
     // https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API#root
     const observer = new IntersectionObserver(
       (entries: IntersectionObserverEntry[]) =>

client/src/app/components/task-manager/TaskManagerDrawer.tsx

@@ -66,7 +66,8 @@ interface TaskManagerDrawerProps {
 export const TaskManagerDrawer: React.FC<TaskManagerDrawerProps> = forwardRef(
   (_props, ref) => {
     const { isExpanded, setIsExpanded, queuedCount } = useTaskManagerContext();
-    const { tasks, hasNextPage, fetchNextPage } = useTaskManagerData();
+    const { tasks, hasNextPage, fetchNextPage, pageSize } =
+      useTaskManagerData();
 
     const [expandedItems, setExpandedItems] = useState<number[]>([]);
     const [taskWithExpandedActions, setTaskWithExpandedAction] = useState<
@@ -106,6 +107,7 @@ export const TaskManagerDrawer: React.FC<TaskManagerDrawerProps> = forwardRef(
               fetchMore={fetchNextPage}
               hasMore={hasNextPage}
               itemCount={tasks?.length ?? 0}
+              pageSize={pageSize}
             >
               <NotificationDrawerList>
                 {tasks.map((task) => (
@@ -282,6 +284,7 @@ const useTaskManagerData = () => {
     [data]
   );
 
+  // note that the callback will change when query fetching state changes
   const fetchMore = useCallback(() => {
     // forced fetch is not allowed when background fetch or other forced fetch is in progress
     if (!isFetching && !isFetchingNextPage) {
@@ -297,6 +300,6 @@ const useTaskManagerData = () => {
     isFetching,
     hasNextPage,
     fetchNextPage: fetchMore,
-    isReadyToFetch: !isFetching && !isFetchingNextPage,
+    pageSize: PAGE_SIZE,
   };
 };

client/src/app/hooks/table-controls/active-item/useActiveItemState.ts

@@ -1,5 +1,5 @@
 import { parseMaybeNumericString } from "@app/utils/utils";
-import { IFeaturePersistenceArgs } from "../types";
+import { IFeaturePersistenceArgs, isPersistenceProvider } from "../types";
 import { usePersistentState } from "@app/hooks/usePersistentState";
 
 /**
@@ -76,7 +76,13 @@ export const useActiveItemState = <
           persistTo,
           key: "activeItem",
         }
-      : { persistTo }),
+      : isPersistenceProvider(persistTo)
+      ? {
+          persistTo: "provider",
+          serialize: persistTo.write,
+          deserialize: () => persistTo.read() as string | number | null,
+        }
+      : { persistTo: "state" }),
   });
   return { activeItemId, setActiveItemId };
 };

client/src/app/hooks/table-controls/expansion/useExpansionState.ts

@@ -1,6 +1,6 @@
 import { usePersistentState } from "@app/hooks/usePersistentState";
 import { objectKeys } from "@app/utils/utils";
-import { IFeaturePersistenceArgs } from "../types";
+import { IFeaturePersistenceArgs, isPersistenceProvider } from "../types";
 import { DiscriminatedArgs } from "@app/utils/type-utils";
 
 /**
@@ -93,7 +93,9 @@ export const useExpansionState = <
       ? {
           persistTo,
           keys: ["expandedCells"],
-          serialize: (expandedCellsObj) => {
+          serialize: (
+            expandedCellsObj: Partial<TExpandedCells<TColumnKey>>
+          ) => {
             if (!expandedCellsObj || objectKeys(expandedCellsObj).length === 0)
               return { expandedCells: null };
             return { expandedCells: JSON.stringify(expandedCellsObj) };
@@ -111,7 +113,13 @@ export const useExpansionState = <
           persistTo,
           key: "expandedCells",
         }
-      : { persistTo }),
+      : isPersistenceProvider(persistTo)
+      ? {
+          persistTo: "provider",
+          serialize: persistTo.write,
+          deserialize: () => persistTo.read() as TExpandedCells<TColumnKey>,
+        }
+      : { persistTo: "state" }),
   });
   return { expandedCells, setExpandedCells };
 };

client/src/app/hooks/table-controls/filtering/useFilterState.ts

@@ -1,5 +1,5 @@
 import { FilterCategory, IFilterValues } from "@app/components/FilterToolbar";
-import { IFeaturePersistenceArgs } from "../types";
+import { IFeaturePersistenceArgs, isPersistenceProvider } from "../types";
 import { usePersistentState } from "@app/hooks/usePersistentState";
 import { serializeFilterUrlParams } from "./helpers";
 import { deserializeFilterUrlParams } from "./helpers";
@@ -90,7 +90,6 @@ export const useFilterState = <
     "filters"
   >({
     isEnabled: !!isFilterEnabled,
-    defaultValue: initialFilterValues,
     persistenceKeyPrefix,
     // Note: For the discriminated union here to work without TypeScript getting confused
     //       (e.g. require the urlParams-specific options when persistTo === "urlParams"),
@@ -99,12 +98,21 @@ export const useFilterState = <
       ? {
           persistTo,
           keys: ["filters"],
+          defaultValue: initialFilterValues,
           serialize: serializeFilterUrlParams,
           deserialize: deserializeFilterUrlParams,
         }
       : persistTo === "localStorage" || persistTo === "sessionStorage"
-      ? { persistTo, key: "filters" }
-      : { persistTo }),
+      ? { persistTo, key: "filters", defaultValue: initialFilterValues }
+      : isPersistenceProvider(persistTo)
+      ? {
+          persistTo: "provider",
+          serialize: persistTo.write,
+          deserialize: () =>
+            persistTo.read() as IFilterValues<TFilterCategoryKey>,
+          defaultValue: isFilterEnabled ? args?.initialFilterValues ?? {} : {},
+        }
+      : { persistTo: "state", defaultValue: initialFilterValues }),
   });
   return { filterValues, setFilterValues };
 };