🐛 Use standard filters for languages in the Set Targets step (#2045) (#…

…2055)

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: #2009
Resolves: https://issues.redhat.com/browse/MTA-3515

---------

Signed-off-by: Radoslaw Szwajkowski <rszwajko@redhat.com>
Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
Co-authored-by: Radoslaw Szwajkowski <rszwajko@redhat.com>
Co-authored-by: Scott J Dickerson <sdickers@redhat.com>
Signed-off-by: Cherry Picker <noreply@github.com>

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
         }

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;

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,
+      }}
+    />
+  );
+};

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,
   };