Skip to content

Commit

Permalink
🐛 Use standard filters for languages in the Set Targets step (#2045)
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Co-authored-by: Scott J Dickerson <[email protected]>
  • Loading branch information
@rszwajko @sjd78
rszwajko and sjd78 authored Aug 14, 2024
1 parent 192d3b8 commit ab89b15
Show file tree
Hide file tree
Showing 4 changed files with 186 additions and 82 deletions.
5 changes: 4 additions & 1 deletion client/src/app/components/FilterToolbar/FilterToolbar.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
SelectOptionProps,
ToolbarToggleGroup,
ToolbarItem,
ToolbarToggleGroupProps,
} from "@patternfly/react-core";
import FilterIcon from "@patternfly/react-icons/dist/esm/icons/filter-icon";

Expand Down Expand Up @@ -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>({
Expand All @@ -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 => {
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion client/src/app/components/SimpleSelectCheckbox.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
258 changes: 179 additions & 79 deletions client/src/app/pages/applications/analysis-wizard/set-targets.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState } from "react";
import React, { useMemo } from "react";
import {
Title,
TextContent,
Expand All @@ -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>();
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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 &&
Expand All @@ -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,
}}
/>
);
};
3 changes: 2 additions & 1 deletion client/src/app/queries/targets.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -21,6 +21,7 @@ export const useFetchTargets = () => {
return {
targets: data || [],
isFetching: isLoading,
isSuccess,
fetchError: error,
refetch,
};
Expand Down

0 comments on commit ab89b15

Please sign in to comment.