From ab89b159b07865c383b523bd8c38f79c25546515 Mon Sep 17 00:00:00 2001 From: Radoslaw Szwajkowski Date: Wed, 14 Aug 2024 19:10:08 +0200 Subject: [PATCH] :bug: Use standard filters for languages in the Set Targets step (#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: https://github.com/konveyor/tackle2-ui/issues/2009 Resolves: https://issues.redhat.com/browse/MTA-3515 --------- Signed-off-by: Radoslaw Szwajkowski Signed-off-by: Scott J Dickerson Co-authored-by: Scott J Dickerson --- .../FilterToolbar/FilterToolbar.tsx | 5 +- .../app/components/SimpleSelectCheckbox.tsx | 2 +- .../analysis-wizard/set-targets.tsx | 258 ++++++++++++------ client/src/app/queries/targets.ts | 3 +- 4 files changed, 186 insertions(+), 82 deletions(-) diff --git a/client/src/app/components/FilterToolbar/FilterToolbar.tsx b/client/src/app/components/FilterToolbar/FilterToolbar.tsx index 404642aa72..c04803fa64 100644 --- 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 { pagination?: JSX.Element; showFiltersSideBySide?: boolean; isDisabled?: boolean; + breakpoint?: ToolbarToggleGroupProps["breakpoint"]; } export const FilterToolbar = ({ @@ -119,6 +121,7 @@ export const FilterToolbar = ({ pagination, showFiltersSideBySide = false, isDisabled = false, + breakpoint = "2xl", }: React.PropsWithChildren< IFilterToolbarProps >): JSX.Element | null => { @@ -192,7 +195,7 @@ export const FilterToolbar = ({ } - 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 index d95d903822..1220fcef86 100644 --- 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 index 4faca34b7d..75d5730647 100644 --- 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 = ({ 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 = ({ + targets, + isLoading, + isError, + languageProviders, + initialFilters = [], +}) => { + const { t } = useTranslation(); const { watch, setValue, getValues } = useFormContext(); @@ -39,32 +120,6 @@ export const SetTargets: React.FC = ({ 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 = ({ 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 (
= ({ applications }) => { {t("wizard.label.setTargets")} - { - return { - children:
{language}
, - value: language, - }; - })} - onChange={(selection) => { - setProviders(selection as string[]); - }} - id="filter-by-language" - toggleId="action-select-toggle" - /> + filterToolbarProps.setFilterValues({})} + > + + + + {values.selectedTargets.length === 0 && values.customRulesFiles.length === 0 && @@ -169,24 +234,59 @@ export const SetTargets: React.FC = ({ applications }) => { /> )} - - {targetsToRender.map((target) => ( - - id === target.id)} - onSelectedCardTargetChange={(selectedTarget) => { - handleOnSelectedCardTargetChange(selectedTarget); - }} - onCardClick={(isSelecting, selectedLabelName, target) => { - handleOnCardClick(isSelecting, selectedLabelName, target); - }} - formLabels={formLabels} - /> - - ))} - + {isError && } + {!isError && ( + + {currentPageItems.map((target) => ( + + id === target.id + )} + onSelectedCardTargetChange={(selectedTarget) => { + handleOnSelectedCardTargetChange(selectedTarget); + }} + onCardClick={(isSelecting, selectedLabelName, target) => { + handleOnCardClick(isSelecting, selectedLabelName, target); + }} + formLabels={formLabels} + /> + + ))} + + )} ); }; + +export const SetTargets: React.FC = ({ 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 ( +
+ +
+ ); + } + + return ( + + ); +}; diff --git a/client/src/app/queries/targets.ts b/client/src/app/queries/targets.ts index 8b6315585d..9dee0ea017 100644 --- 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({ + const { data, isLoading, isSuccess, error, refetch } = useQuery({ 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, };