From 4d8caf79de65c4802194d13c123dd78368209afc Mon Sep 17 00:00:00 2001 From: Scott J Dickerson Date: Fri, 7 Nov 2025 22:37:36 -0500 Subject: [PATCH 1/2] :bug: Fix custom target order handling Resolves: https://issues.redhat.com/browse/MTA-6287 Resolves: #2708 All targets will new be displayed, even if they are not in the target order setting. The target query hook now will return the server ordering of the targets, and ordering by the "ui.target.order" setting. All targets are included in both lists. Any unordered targets is added to the end of the ordered list. By putting this logic in the target query hook, target order can be used the same way everywhere it is needed. On the migration-targets page, the targets are now displayed in the order specified by the "ui.target.order" setting. Any targets not in the setting are added to the end of the list. On the custom-target-form, updated the onSubmit handler to both create a new target and add it to the target order. This is done awaiting async mutations to help ensure the operations finish before the create modal is closed. Signed-off-by: Scott J Dickerson --- .../analysis-wizard/set-targets.tsx | 20 +--- .../components/custom-target-form.tsx | 107 ++++++++++++------ .../migration-targets/migration-targets.tsx | 39 +++---- client/src/app/queries/settings.ts | 4 +- client/src/app/queries/targets.ts | 41 ++++++- 5 files changed, 134 insertions(+), 77 deletions(-) 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 ab089d113..6107412fa 100644 --- a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx +++ b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx @@ -37,6 +37,7 @@ interface SetTargetsProps { const useEnhancedTargets = (applications: Application[]) => { const { targets, + targetsInOrder, isFetching: isTargetsLoading, fetchError: isTargetsError, } = useFetchTargets(); @@ -69,23 +70,6 @@ const useEnhancedTargets = (applications: Application[]) => { [applications, languageTags, languageProviders] ); - // 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 @@ -93,7 +77,7 @@ const useEnhancedTargets = (applications: Application[]) => { isTagCategoriesLoading || isTargetsLoading || isTargetOrderLoading, // missing targets are the only blocker isError: !!isTargetsError, - targets: targetsWithOrder, + targets: targetsInOrder, applicationProviders, languageProviders, }; diff --git a/client/src/app/pages/migration-targets/components/custom-target-form.tsx b/client/src/app/pages/migration-targets/components/custom-target-form.tsx index bdfb185cb..ec7ba52a9 100644 --- a/client/src/app/pages/migration-targets/components/custom-target-form.tsx +++ b/client/src/app/pages/migration-targets/components/custom-target-form.tsx @@ -27,6 +27,7 @@ import { NotificationsContext } from "@app/components/NotificationsContext"; import { OptionWithValue, SimpleSelect } from "@app/components/SimpleSelect"; import { UploadFileSchema } from "@app/pages/applications/analysis-wizard/schema"; import { useFetchIdentities } from "@app/queries/identities"; +import { useSettingMutation } from "@app/queries/settings"; import { useCreateTargetMutation, useFetchTargets, @@ -47,6 +48,7 @@ import defaultImage from "./default.png"; export interface CustomTargetFormProps { target?: Target | null; + targetOrder: number[]; onSaved: (response: AxiosResponse) => void; onCancel: () => void; } @@ -77,12 +79,58 @@ const targetRulesetsToReadFiles = (target?: Target) => }; }) || []; +const useTargetQueryHooks = ( + onSaved: (response: AxiosResponse) => void, + reset: () => void +) => { + const { pushNotification } = useContext(NotificationsContext); + + const { mutateAsync: createImageFileAsync } = useCreateFileMutation(); + + const { mutateAsync: createTargetAsync } = useCreateTargetMutation( + (response: AxiosResponse) => { + // TODO: Consider any removed files and delete them from hub if necessary + onSaved(response); + reset(); + }, + (error: AxiosError) => { + pushNotification({ + title: getAxiosErrorMessage(error), + variant: "danger", + }); + } + ); + + const onUpdateTargetSuccess = (response: AxiosResponse) => { + // TODO: Consider any removed files and delete them from hub if necessary + onSaved(response); + reset(); + }; + + const onUpdateTargetFailure = (_error: AxiosError) => {}; + + const { mutate: updateTarget } = useUpdateTargetMutation( + onUpdateTargetSuccess, + onUpdateTargetFailure + ); + + const { mutateAsync: targetOrderMutateAsync } = + useSettingMutation("ui.target.order"); + + return { + createTargetAsync, + updateTarget, + targetOrderMutateAsync, + createImageFileAsync, + }; +}; + export const CustomTargetForm: React.FC = ({ target: initialTarget, + targetOrder, onSaved, onCancel, }) => { - const { pushNotification } = useContext(NotificationsContext); const baseProviderList = useMigrationProviderList(); const providerList = useMemo( () => @@ -261,6 +309,13 @@ export const CustomTargetForm: React.FC = ({ const values = getValues(); + const { + createTargetAsync, + updateTarget, + targetOrderMutateAsync, + createImageFileAsync, + } = useTargetQueryHooks(onSaved, reset); + const onSubmit = async (formValues: CustomTargetFormValues) => { const rules: Rule[] = formValues.customRulesFiles .map((file) => { @@ -339,7 +394,22 @@ export const CustomTargetForm: React.FC = ({ if (target) { updateTarget({ id: target.id, ...payload }); } else { - createTarget(payload); + let newTargetId: number | undefined; + try { + const response = await createTargetAsync(payload); + newTargetId = response.data.id; + } catch { + /* ignore */ + } + + // Add the new target to the target order and wait for it to complete before continuing. + if (newTargetId !== undefined) { + try { + await targetOrderMutateAsync([...targetOrder, newTargetId]); + } catch { + /* ignore */ + } + } } }; @@ -348,39 +418,6 @@ export const CustomTargetForm: React.FC = ({ onCancel(); }; - const { mutateAsync: createImageFileAsync } = useCreateFileMutation(); - - const onCreateTargetSuccess = (response: AxiosResponse) => { - // TODO: Consider any removed files and delete them from hub if necessary - onSaved(response); - reset(); - }; - - const onCreateTargetFailure = (error: AxiosError) => { - pushNotification({ - title: getAxiosErrorMessage(error), - variant: "danger", - }); - }; - - const { mutate: createTarget } = useCreateTargetMutation( - onCreateTargetSuccess, - onCreateTargetFailure - ); - - const onUpdateTargetSuccess = (response: AxiosResponse) => { - // TODO: Consider any removed files and delete them from hub if necessary - onSaved(response); - reset(); - }; - - const onUpdateTargetFailure = (_error: AxiosError) => {}; - - const { mutate: updateTarget } = useUpdateTargetMutation( - onUpdateTargetSuccess, - onUpdateTargetFailure - ); - const handleImageFileUpload = async (file: File) => { setImageFilename(file.name); return createImageFileAsync({ file }); diff --git a/client/src/app/pages/migration-targets/migration-targets.tsx b/client/src/app/pages/migration-targets/migration-targets.tsx index 942399a3a..e6144feda 100644 --- a/client/src/app/pages/migration-targets/migration-targets.tsx +++ b/client/src/app/pages/migration-targets/migration-targets.tsx @@ -50,7 +50,7 @@ export const MigrationTargets: React.FC = () => { const { t } = useTranslation(); const { pushNotification } = React.useContext(NotificationsContext); - const { targets, refetch: refetchTargets } = useFetchTargets(); + const { targetsInOrder } = useFetchTargets(); const targetOrderSetting = useSetting("ui.target.order"); const targetOrderSettingMutation = useSettingMutation("ui.target.order"); @@ -94,7 +94,7 @@ export const MigrationTargets: React.FC = () => { onDeleteTargetError ); - const onCustomTargetModalSaved = (response: AxiosResponse) => { + const onCustomTargetModalSaved = async (response: AxiosResponse) => { if (targetToUpdate) { pushNotification({ title: t("toastr.success.saveWhat", { @@ -121,18 +121,6 @@ export const MigrationTargets: React.FC = () => { ), }); - // update target order - if ( - targetOrderSetting.isSuccess && - response.data.id && - targetOrderSetting.data - ) { - targetOrderSettingMutation.mutate([ - ...targetOrderSetting.data, - response.data.id, - ]); - } - // Make sure the new target's provider is part of the providers filter so it can be seen if (filterState.filterValues["provider"]) { const targetProvider = response.data.provider; @@ -149,8 +137,8 @@ export const MigrationTargets: React.FC = () => { }); } } + setCreateUpdateModalState(null); - refetchTargets(); }; const sensors = useSensors(useSensor(MouseSensor), useSensor(TouchSensor)); @@ -180,13 +168,15 @@ export const MigrationTargets: React.FC = () => { const handleDragStart = ({ active }: DragStartEvent) => { const activeId = active.id as number; - const activeTarget = targets.find((target) => target.id === activeId); + const activeTarget = targetsInOrder.find( + (target) => target.id === activeId + ); setActiveTarget(activeTarget ?? null); }; const tableControls = useLocalTableControls({ tableName: "target-cards", - items: targets, + items: targetsInOrder, idProperty: "name", initialFilterValues: { provider: [DEFAULT_PROVIDER] }, columnNames: { @@ -200,7 +190,7 @@ export const MigrationTargets: React.FC = () => { filterCategories: [ { selectOptions: unique( - targets.map((target) => target.provider).filter(Boolean) + targetsInOrder.map((target) => target.provider).filter(Boolean) ).map((target) => ({ value: target })), placeholderText: "Filter by language...", categoryKey: "provider", @@ -230,9 +220,19 @@ export const MigrationTargets: React.FC = () => { return []; } - return targetOrderSetting.data + // Get targets in the order specified by the setting + const orderedTargets = targetOrderSetting.data .map((id) => filteredTargets.find((target) => target.id === id)) .filter(Boolean); + + // Find targets that are in filteredTargets but not in the ordered list + const orderedTargetIds = new Set(orderedTargets.map((target) => target.id)); + const unorderedTargets = filteredTargets.filter( + (target) => !orderedTargetIds.has(target.id) + ); + + // Combine ordered targets with unordered ones at the end + return [...orderedTargets, ...unorderedTargets]; }, [filteredTargets, targetOrderSetting.data, targetOrderSetting.isSuccess]); return ( @@ -315,6 +315,7 @@ export const MigrationTargets: React.FC = () => { > setCreateUpdateModalState(null)} /> diff --git a/client/src/app/queries/settings.ts b/client/src/app/queries/settings.ts index fdc46da05..c74f4a222 100644 --- a/client/src/app/queries/settings.ts +++ b/client/src/app/queries/settings.ts @@ -23,10 +23,10 @@ export const useSettingMutation = (key: K) => { return useMutation({ mutationFn: (value: SettingTypes[K]) => updateSetting({ key, value }), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: [SettingQueryKey] }); + queryClient.invalidateQueries({ queryKey: [SettingQueryKey, key] }); }, onError: () => { - queryClient.invalidateQueries({ queryKey: [SettingQueryKey] }); + queryClient.invalidateQueries({ queryKey: [SettingQueryKey, key] }); }, }); }; diff --git a/client/src/app/queries/targets.ts b/client/src/app/queries/targets.ts index a10169c2f..8c745a290 100644 --- a/client/src/app/queries/targets.ts +++ b/client/src/app/queries/targets.ts @@ -1,3 +1,4 @@ +import { useMemo } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { AxiosError, AxiosResponse } from "axios"; @@ -11,20 +12,53 @@ import { updateTarget, } from "@app/api/rest"; +import { useSetting } from "./settings"; + export const TargetsQueryKey = "targets"; export const useFetchTargets = ( refetchInterval: number | false = DEFAULT_REFETCH_INTERVAL ) => { - const { data, isLoading, isSuccess, error, refetch } = useQuery({ + const { + data: targets, + isLoading, + isSuccess, + error, + refetch, + } = useQuery({ queryKey: [TargetsQueryKey], queryFn: async () => await getTargets(), onError: (err) => console.log(err), refetchInterval, }); + const { data: targetOrder = [] } = useSetting("ui.target.order"); + + const targetsInOrder = useMemo( + () => + !targets + ? [] + : targets + .map((target, targetIndex) => { + const index = targetOrder.findIndex((id) => id === target.id); + return { + target, + order: index === -1 ? targets.length + targetIndex : index, + }; + }) + .sort((a, b) => a.order - b.order) + .map(({ target }) => target), + [targets, targetOrder] + ); + return { - targets: data || [], + /** Targets in their original order */ + targets: targets || [], + /** + * Targets in the order specified by the setting "ui.target.order" including targets + * that are not in the setting at the end of the list. + */ + targetsInOrder, isFetching: isLoading, isSuccess, fetchError: error, @@ -83,7 +117,7 @@ export const useCreateTargetMutation = ( onError: (err: AxiosError) => void ) => { const queryClient = useQueryClient(); - const { isPending, mutate, error } = useMutation({ + const { isPending, mutate, mutateAsync, error } = useMutation({ mutationFn: createTarget, onSuccess: (res) => { onSuccess(res); @@ -95,6 +129,7 @@ export const useCreateTargetMutation = ( }); return { mutate, + mutateAsync, isPending, error, }; From 0b79f1c1ff3337e25b69b96bd8be76525cdb1fee Mon Sep 17 00:00:00 2001 From: Scott J Dickerson Date: Mon, 10 Nov 2025 09:18:21 -0500 Subject: [PATCH 2/2] remove unused queries Signed-off-by: Scott J Dickerson --- .../app/pages/applications/analysis-wizard/set-targets.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) 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 6107412fa..6762d4166 100644 --- a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx +++ b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx @@ -20,7 +20,6 @@ import { FilterToolbar, FilterType } from "@app/components/FilterToolbar"; import { StateError } from "@app/components/StateError"; import { TargetCard } from "@app/components/target-card/target-card"; import { useLocalTableControls } from "@app/hooks/table-controls"; -import { useSetting } from "@app/queries/settings"; import { useFetchTagCategories } from "@app/queries/tags"; import { useFetchTargets } from "@app/queries/targets"; import { toLabelValue } from "@app/utils/rules-utils"; @@ -43,8 +42,6 @@ const useEnhancedTargets = (applications: Application[]) => { } = 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)), @@ -73,8 +70,7 @@ const useEnhancedTargets = (applications: Application[]) => { 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, + isLoading: isTagCategoriesLoading || isTargetsLoading, // missing targets are the only blocker isError: !!isTargetsError, targets: targetsInOrder,