🐛 Use standard filters for languages in the Set Targets step (konveyo…

…r#2045) Before, in the Set Targets step, if the languages were not pre-selected automatically then no cards were visible. In order to see target cards user had to select at least one language. After this fix, all target cards will be visible. Additional changes: 1. wait (display a spinner) until all necessary queries finished before displaying Set Targets step 2. improve error handling 1. display error when targets could not be loaded 2. silently ignore failed optional queries and continue without extra featues 3. extract data fetching into one hook Resolves: konveyor#2009 Resolves: https://issues.redhat.com/browse/MTA-3515 --------- Signed-off-by: Radoslaw Szwajkowski <[email protected]> Signed-off-by: Scott J Dickerson <[email protected]> Co-authored-by: Scott J Dickerson <[email protected]>
sjd78 · Aug 20, 2024 · 206bd40 · 206bd40
1 parent 192d3b8
commit 206bd40
Show file tree

Hide file tree

Showing 4 changed files with 186 additions and 82 deletions.
diff --git a/client/src/app/components/FilterToolbar/FilterToolbar.tsx b/client/src/app/components/FilterToolbar/FilterToolbar.tsx
@@ -8,6 +8,7 @@ import {
   SelectOptionProps,
   ToolbarToggleGroup,
   ToolbarItem,
+  ToolbarToggleGroupProps,
 } from "@patternfly/react-core";
 import FilterIcon from "@patternfly/react-icons/dist/esm/icons/filter-icon";
 
@@ -110,6 +111,7 @@ export interface IFilterToolbarProps<TItem, TFilterCategoryKey extends string> {
   pagination?: JSX.Element;
   showFiltersSideBySide?: boolean;
   isDisabled?: boolean;
+  breakpoint?: ToolbarToggleGroupProps["breakpoint"];
 }
 
 export const FilterToolbar = <TItem, TFilterCategoryKey extends string>({
@@ -119,6 +121,7 @@ export const FilterToolbar = <TItem, TFilterCategoryKey extends string>({
   pagination,
   showFiltersSideBySide = false,
   isDisabled = false,
+  breakpoint = "2xl",
 }: React.PropsWithChildren<
   IFilterToolbarProps<TItem, TFilterCategoryKey>
 >): JSX.Element | null => {
@@ -192,7 +195,7 @@ export const FilterToolbar = <TItem, TFilterCategoryKey extends string>({
       <ToolbarToggleGroup
         variant="filter-group"
         toggleIcon={<FilterIcon />}
-        breakpoint="2xl"
+        breakpoint={breakpoint}
         spaceItems={
           showFiltersSideBySide ? { default: "spaceItemsMd" } : undefined
         }

diff --git a/client/src/app/components/SimpleSelectCheckbox.tsx b/client/src/app/components/SimpleSelectCheckbox.tsx
@@ -11,7 +11,7 @@ import {
 import spacing from "@patternfly/react-styles/css/utilities/Spacing/spacing";
 
 export interface ISimpleSelectBasicProps {
-  onChange: (selection: string | string[]) => void;
+  onChange: (selection: string[]) => void;
   options: SelectOptionProps[];
   value?: string[];
   placeholderText?: string;

diff --git a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx
@@ -1,4 +1,4 @@
-import React, { useState } from "react";
+import React, { useMemo } from "react";
 import {
   Title,
   TextContent,
@@ -7,30 +7,111 @@ import {
   GalleryItem,
   Form,
   Alert,
-  SelectOptionProps,
+  Toolbar,
+  ToolbarContent,
 } from "@patternfly/react-core";
 import { useTranslation } from "react-i18next";
 import { useFormContext } from "react-hook-form";
 
 import { TargetCard } from "@app/components/target-card/target-card";
 import { AnalysisWizardFormValues } from "./schema";
-import { useSetting } from "@app/queries/settings";
 import { useFetchTargets } from "@app/queries/targets";
-import { Application, TagCategory, Target } from "@app/api/models";
+import { Application, Target } from "@app/api/models";
 import { useFetchTagCategories } from "@app/queries/tags";
-import { SimpleSelectCheckbox } from "@app/components/SimpleSelectCheckbox";
 import { getUpdatedFormLabels, toggleSelectedTargets } from "./utils";
+import { unique } from "radash";
+import { FilterToolbar, FilterType } from "@app/components/FilterToolbar";
+import { useLocalTableControls } from "@app/hooks/table-controls";
+import { useSetting } from "@app/queries/settings";
+import { AppPlaceholder } from "@app/components/AppPlaceholder";
+import { StateError } from "@app/components/StateError";
 
 interface SetTargetsProps {
   applications: Application[];
+  initialFilters?: string[];
 }
 
-export const SetTargets: React.FC<SetTargetsProps> = ({ applications }) => {
-  const { t } = useTranslation();
+const useEnhancedTargets = (applications: Application[]) => {
+  const {
+    targets,
+    isFetching: isTargetsLoading,
+    fetchError: isTargetsError,
+  } = useFetchTargets();
+  const { tagCategories, isFetching: isTagCategoriesLoading } =
+    useFetchTagCategories();
+  const { data: targetOrder = [], isLoading: isTargetOrderLoading } =
+    useSetting("ui.target.order");
+
+  const languageProviders = useMemo(
+    () => unique(targets.map(({ provider }) => provider).filter(Boolean)),
+    [targets]
+  );
+
+  const languageTags = useMemo(
+    () =>
+      tagCategories?.find((category) => category.name === "Language")?.tags ??
+      [],
+    [tagCategories]
+  );
+
+  const applicationProviders = useMemo(
+    () =>
+      unique(
+        applications
+          .flatMap((app) => app.tags || [])
+          .filter((tag) => languageTags.find((lt) => lt.id === tag.id))
+          .map((languageTag) => languageTag.name)
+          .filter((language) => languageProviders.includes(language))
+      ),
+    [applications, languageTags, languageProviders]
+  );
 
-  const { targets } = useFetchTargets();
+  // 1. missing target order setting is not a blocker (only lowers user experience)
+  // 2. targets without assigned position (if any) are put at the end
+  const targetsWithOrder = useMemo(
+    () =>
+      targets
+        .map((target) => {
+          const index = targetOrder.findIndex((id) => id === target.id);
+          return {
+            target,
+            order: index === -1 ? targets.length : index,
+          };
+        })
+        .sort((a, b) => a.order - b.order)
+        .map(({ target }) => target),
+    [targets, targetOrder]
+  );
+
+  return {
+    // true if some queries are still fetching data for the first time (initial load)
+    // note that the default re-try count (3) is used
+    isLoading:
+      isTagCategoriesLoading || isTargetsLoading || isTargetOrderLoading,
+    // missing targets are the only blocker
+    isError: !!isTargetsError,
+    targets: targetsWithOrder,
+    applicationProviders,
+    languageProviders,
+  };
+};
+
+interface SetTargetsInternalProps {
+  targets: Target[];
+  isLoading: boolean;
+  isError: boolean;
+  languageProviders: string[];
+  initialFilters: string[];
+}
 
-  const targetOrderSetting = useSetting("ui.target.order");
+const SetTargetsInternal: React.FC<SetTargetsInternalProps> = ({
+  targets,
+  isLoading,
+  isError,
+  languageProviders,
+  initialFilters = [],
+}) => {
+  const { t } = useTranslation();
 
   const { watch, setValue, getValues } =
     useFormContext<AnalysisWizardFormValues>();
@@ -39,32 +120,6 @@ export const SetTargets: React.FC<SetTargetsProps> = ({ applications }) => {
   const formLabels = watch("formLabels");
   const selectedTargets = watch("selectedTargets");
 
-  const { tagCategories } = useFetchTagCategories();
-
-  const findCategoryForTag = (tagId: number) => {
-    return tagCategories.find(
-      (category: TagCategory) =>
-        category.tags?.some((categoryTag) => categoryTag.id === tagId)
-    );
-  };
-
-  const initialProviders = Array.from(
-    new Set(
-      applications
-        .flatMap((app) => app.tags || [])
-        .map((tag) => {
-          return {
-            category: findCategoryForTag(tag.id),
-            tag,
-          };
-        })
-        .filter((tagWithCat) => tagWithCat?.category?.name === "Language")
-        .map((tagWithCat) => tagWithCat.tag.name)
-    )
-  ).filter(Boolean);
-
-  const [providers, setProviders] = useState(initialProviders);
-
   const handleOnSelectedCardTargetChange = (selectedLabelName: string) => {
     const otherSelectedLabels = formLabels?.filter((formLabel) => {
       return formLabel.name !== selectedLabelName;
@@ -118,17 +173,35 @@ export const SetTargets: React.FC<SetTargetsProps> = ({ applications }) => {
     setValue("formLabels", updatedFormLabels);
   };
 
-  const allProviders = targets.flatMap((target) => target.provider);
-  const languageOptions = Array.from(new Set(allProviders));
+  const tableControls = useLocalTableControls({
+    tableName: "target-cards",
+    items: targets,
+    idProperty: "name",
+    initialFilterValues: { name: initialFilters },
+    columnNames: {
+      name: "name",
+    },
+    isFilterEnabled: true,
+    isPaginationEnabled: false,
+    isLoading,
+    filterCategories: [
+      {
+        selectOptions: languageProviders?.map((language) => ({
+          value: language,
+        })),
+        placeholderText: "Filter by language...",
+        categoryKey: "name",
+        title: "Languages",
+        type: FilterType.multiselect,
+        matcher: (filter, target) => !!target.provider?.includes(filter),
+      },
+    ],
+  });
 
-  const targetsToRender: Target[] = !targetOrderSetting.isSuccess
-    ? []
-    : targetOrderSetting.data
-        .map((targetId) => targets.find((target) => target.id === targetId))
-        .filter(Boolean)
-        .filter((target) =>
-          providers.some((p) => target.provider?.includes(p) ?? false)
-        );
+  const {
+    currentPageItems,
+    propHelpers: { toolbarProps, filterToolbarProps },
+  } = tableControls;
 
   return (
     <Form
@@ -142,22 +215,14 @@ export const SetTargets: React.FC<SetTargetsProps> = ({ applications }) => {
         </Title>
         <Text>{t("wizard.label.setTargets")}</Text>
       </TextContent>
-      <SimpleSelectCheckbox
-        placeholderText="Filter by language..."
-        width={300}
-        value={providers}
-        options={languageOptions?.map((language): SelectOptionProps => {
-          return {
-            children: <div>{language}</div>,
-            value: language,
-          };
-        })}
-        onChange={(selection) => {
-          setProviders(selection as string[]);
-        }}
-        id="filter-by-language"
-        toggleId="action-select-toggle"
-      />
+      <Toolbar
+        {...toolbarProps}
+        clearAllFilters={() => filterToolbarProps.setFilterValues({})}
+      >
+        <ToolbarContent>
+          <FilterToolbar {...filterToolbarProps} breakpoint="md" />
+        </ToolbarContent>
+      </Toolbar>
 
       {values.selectedTargets.length === 0 &&
         values.customRulesFiles.length === 0 &&
@@ -169,24 +234,59 @@ export const SetTargets: React.FC<SetTargetsProps> = ({ applications }) => {
           />
         )}
 
-      <Gallery hasGutter>
-        {targetsToRender.map((target) => (
-          <GalleryItem key={target.id}>
-            <TargetCard
-              readOnly
-              item={target}
-              cardSelected={selectedTargets.some(({ id }) => id === target.id)}
-              onSelectedCardTargetChange={(selectedTarget) => {
-                handleOnSelectedCardTargetChange(selectedTarget);
-              }}
-              onCardClick={(isSelecting, selectedLabelName, target) => {
-                handleOnCardClick(isSelecting, selectedLabelName, target);
-              }}
-              formLabels={formLabels}
-            />
-          </GalleryItem>
-        ))}
-      </Gallery>
+      {isError && <StateError />}
+      {!isError && (
+        <Gallery hasGutter>
+          {currentPageItems.map((target) => (
+            <GalleryItem key={target.id}>
+              <TargetCard
+                readOnly
+                item={target}
+                cardSelected={selectedTargets.some(
+                  ({ id }) => id === target.id
+                )}
+                onSelectedCardTargetChange={(selectedTarget) => {
+                  handleOnSelectedCardTargetChange(selectedTarget);
+                }}
+                onCardClick={(isSelecting, selectedLabelName, target) => {
+                  handleOnCardClick(isSelecting, selectedLabelName, target);
+                }}
+                formLabels={formLabels}
+              />
+            </GalleryItem>
+          ))}
+        </Gallery>
+      )}
     </Form>
   );
 };
+
+export const SetTargets: React.FC<SetTargetsProps> = ({ applications }) => {
+  // wait for the initial load but leave error handling to the real page
+  const {
+    isLoading,
+    targets,
+    isError,
+    applicationProviders,
+    languageProviders,
+  } = useEnhancedTargets(applications);
+  if (isLoading) {
+    return (
+      <div style={{ marginTop: "100px" }}>
+        <AppPlaceholder />
+      </div>
+    );
+  }
+
+  return (
+    <SetTargetsInternal
+      {...{
+        initialFilters: applicationProviders,
+        targets,
+        isError,
+        isLoading,
+        languageProviders,
+      }}
+    />
+  );
+};
diff --git a/client/src/app/queries/targets.ts b/client/src/app/queries/targets.ts
@@ -12,7 +12,7 @@ import { AxiosError } from "axios";
 export const TargetsQueryKey = "targets";
 
 export const useFetchTargets = () => {
-  const { data, isLoading, error, refetch } = useQuery<Target[]>({
+  const { data, isLoading, isSuccess, error, refetch } = useQuery<Target[]>({
     queryKey: [TargetsQueryKey],
     queryFn: async () => await getTargets(),
     onError: (err) => console.log(err),
@@ -21,6 +21,7 @@ export const useFetchTargets = () => {
   return {
     targets: data || [],
     isFetching: isLoading,
+    isSuccess,
     fetchError: error,
     refetch,
   };