🐛 Fix custom target order handling

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";
@@ -37,13 +36,12 @@ interface SetTargetsProps {
 const useEnhancedTargets = (applications: Application[]) => {
   const {
     targets,
+    targetsInOrder,
     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)),
@@ -69,31 +67,13 @@ 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
-    isLoading:
-      isTagCategoriesLoading || isTargetsLoading || isTargetOrderLoading,
+    isLoading: isTagCategoriesLoading || isTargetsLoading,
     // missing targets are the only blocker
     isError: !!isTargetsError,
-    targets: targetsWithOrder,
+    targets: targetsInOrder,
     applicationProviders,
     languageProviders,
   };

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<Target>) => void;
   onCancel: () => void;
 }
@@ -77,12 +79,58 @@ const targetRulesetsToReadFiles = (target?: Target) =>
     };
   }) || [];
 
+const useTargetQueryHooks = (
+  onSaved: (response: AxiosResponse<Target>) => void,
+  reset: () => void
+) => {
+  const { pushNotification } = useContext(NotificationsContext);
+
+  const { mutateAsync: createImageFileAsync } = useCreateFileMutation();
+
+  const { mutateAsync: createTargetAsync } = useCreateTargetMutation(
+    (response: AxiosResponse<Target>) => {
+      // 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<Target>) => {
+    // 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<CustomTargetFormProps> = ({
   target: initialTarget,
+  targetOrder,
   onSaved,
   onCancel,
 }) => {
-  const { pushNotification } = useContext(NotificationsContext);
   const baseProviderList = useMigrationProviderList();
   const providerList = useMemo(
     () =>
@@ -261,6 +309,13 @@ export const CustomTargetForm: React.FC<CustomTargetFormProps> = ({
 
   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<CustomTargetFormProps> = ({
     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<CustomTargetFormProps> = ({
     onCancel();
   };
 
-  const { mutateAsync: createImageFileAsync } = useCreateFileMutation();
-
-  const onCreateTargetSuccess = (response: AxiosResponse<Target>) => {
-    // 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<Target>) => {
-    // 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 });

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<Target>) => {
+  const onCustomTargetModalSaved = async (response: AxiosResponse<Target>) => {
     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 = () => {
       >
         <CustomTargetForm
           target={targetToUpdate}
+          targetOrder={targetOrderSetting.data ?? []}
           onSaved={onCustomTargetModalSaved}
           onCancel={() => setCreateUpdateModalState(null)}
         />

client/src/app/queries/settings.ts

@@ -23,10 +23,10 @@ export const useSettingMutation = <K extends keyof SettingTypes>(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] });
     },
   });
 };