Resolves: MTV-3743 | Standartize plan mappings tab #2147

avivtur · 2025-12-10T12:29:42Z

📝 Links

https://issues.redhat.com/browse/MTV-3743

📝 Description

Standardize the mappings tab under plan details page

🎥 Demo

plan-mappings-section.mp4

plan-mappings-section2.mp4

plan-mappings-section3.mp4

📝 CC://

codecov-commenter · 2025-12-10T12:33:11Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 3.44828% with 420 lines in your changes missing coverage. Please review.
✅ Project coverage is 13.40%. Comparing base (13484d0) to head (9b6a144).
⚠️ Report is 935 commits behind head on main.

Files with missing lines	Patch %	Lines
...geMapEdit/components/PlanStorageMapFieldsTable.tsx	0.00%	63 Missing ⚠️
...rkMapEdit/components/PlanNetworkMapFieldsTable.tsx	0.00%	57 Missing ⚠️
...c/plans/details/tabs/Mappings/PlanMappingsPage.tsx	0.00%	50 Missing ⚠️
...s/mappings/network-mappings/SourceNetworkField.tsx	0.00%	33 Missing ⚠️
...s/mappings/network-mappings/TargetNetworkField.tsx	0.00%	31 Missing ⚠️
...mponents/PlanNetworkMapEdit/PlanNetworkMapEdit.tsx	0.00%	26 Missing ⚠️
...mponents/PlanStorageMapEdit/PlanStorageMapEdit.tsx	0.00%	25 Missing ⚠️
.../tabs/Mappings/hooks/usePlanStorageMapResources.ts	0.00%	23 Missing ⚠️
.../tabs/Mappings/hooks/usePlanNetworkMapResources.ts	0.00%	20 Missing ⚠️
...pings/components/PlanStorageMapEdit/utils/utils.ts	0.00%	18 Missing ⚠️
... and 19 more
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2147       +/-   ##
===========================================
- Coverage   36.81%   13.40%   -23.41%     
===========================================
  Files         158     1065      +907     
  Lines        2548    20095    +17547     
  Branches      599     3900     +3301     
===========================================
+ Hits          938     2694     +1756     
- Misses       1428    17391    +15963     
+ Partials      182       10      -172

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sgratch

Generally, the mappings tab works much better now so this is a great improvement.
Please see my comments below.

sgratch · 2025-12-15T22:43:22Z

...details/tabs/Mappings/components/PlanNetworkMapEdit/components/PlanNetworkMapFieldsTable.tsx

+        },
+        tooltip: (index) => {
+          if (networkMappingFields.length <= 1) {
+            return t('At least one network mapping must be provided.');


This validation is not accurate.
There are cases in which there is no network mappings exists.

you mean we should allow an empty mapping?

this tooltip appears on the delete button when disabling the delete button when we have just one mapping

sgratch · 2025-12-16T17:01:03Z

src/networkMaps/create/utils/buildNetworkMappings.ts

 };

-export const buildFormNetworkMapping = (
+export const getNetworkMappingValues = (


the calculation of the source network entries for oVirt is not taking into account the data center (i.e. it's set to net1 instead of data-center1/net1)

fixed I was getting the name from the inventory network and not the name from the map resource

sgratch · 2025-12-16T17:04:34Z

src/plans/details/tabs/Mappings/hooks/usePlanMappingVms.ts

+  plan: V1beta1Plan,
+  sourceProvider: V1beta1Provider,
+): Record<string, ProviderVirtualMachine> => {
+  const [providerVmData, isVmDataLoading] = useInventoryVms({ provider: sourceProvider });


Suggested change

const [providerVmData, isVmDataLoading] = useInventoryVms({ provider: sourceProvider });

const [providerVmData, isVmDataLoading, error] = useInventoryVms({ provider: sourceProvider });

Worth adding error handling for fetching vms from the inventory

sgratch · 2025-12-16T17:11:20Z

src/plans/details/tabs/Mappings/PlanMappingsPage.tsx

+    [availableTargetNetworks, targetProject],
+  );
+
+  const message = useMemo(


please move lines 92 -115 to be located after calculating the isLoading.

I can't, everything is wrapped in useMemo as we have get all the data here, and all hooks must run all the times meaning I can't move that block

sgratch · 2025-12-16T17:15:16Z

src/plans/details/tabs/Mappings/PlanMappingsPage.tsx

-        sourceStorages={sourceStorages}
-        targetNetworks={targetNetworks}
-        targetStorages={targetStorages}
+      <SectionHeadingWithEdit


Worth adding a link ot a string of the created mappings entity name for both network and storage, the same as was in old UI. Otherwise it will be hard for the user to find this mapping in the network, storage maps list page.

I added it to the UI, this is the best location I could think of, @mmenestr can you have an opinion on this

@sgratch @avivtur I think adding a link to it is fine, but I think what we can do is add a read only field called "Network map name" like this:

and if we add that, we should probably also add the option to edit the name in the modal:

Let me know what you think!

Editing names of resources in kubernetes like network maps/plan/providers is not allowed, but I will change the display accordingly, Thank you

sgratch · 2025-12-16T17:34:51Z

...details/tabs/Mappings/components/PlanNetworkMapEdit/components/PlanNetworkMapFieldsTable.tsx

+  loadError: Error | null;
+};
+
+const PlanNetworkMapFieldsTable: FC<PlanNetworkMapFieldsTableProps> = ({


This PlanNetworkMapFieldsTable component and PlanStorageMapFieldsTableshare a lot of logic. Can we combine it into a shared component used by both or is it too complicated?

I've explored this possibility myself, however I already use a similar component, soI can extract the logic above the component to a custom hook but it wasn't saving code, infact it doubled the code, and the the readability didn't improved (since we now also have another generic-ish hook that needs to be in use)

sgratch · 2025-12-16T17:49:08Z

src/plans/details/tabs/Mappings/components/PlanNetworkMapEdit/components/TargetNetworkField.tsx

+  targetNetworks: Record<string, MappingValue>;
+};
+
+const TargetNetworkField: FC<TargetNetworkFieldProps> = ({ fieldId, targetNetworks }) => {


There are 3 components that at least 2 of them look identical, one for create and one for edit plans and the 3d for network maps editing:

- networkMaps/create/fields/TargetNetworkField
- plans/create/steps/network-map/TargetNetworkField
-plans/details/tabs/Mappings/components/PlanNetworkMapEdit/components TargetNetworkField

Can we combine them and use one component for all?

sgratch · 2025-12-16T17:49:19Z

src/plans/details/tabs/Mappings/components/PlanNetworkMapEdit/components/SourceNetworkField.tsx

+  otherSourceNetworks: MappingValue[];
+};
+
+const SourceNetworkField: FC<SourceNetworkFieldProps> = ({


There are two components that looks identical, one for create and one for edit plans:

details/tabs/Mappings/components/PlanNetworkMapEdit/components/SourceNetworkField

create/steps/network-map/SourceNetworkField

Can we combine them and use one component for both?

sgratch · 2025-12-16T17:57:29Z

src/plans/details/tabs/Mappings/components/PlanNetworkMapEdit/PlanNetworkMapEdit.tsx

+        title={t('Edit network map')}
+        closeModal={closeModal}
+        variant={ModalVariant.medium}
+        isDisabled={!isValid || !isDirty}


Please handle cases in which you can't edit the plan mappings and it should be disabled. E.g. plan is completed, archived, running while at least one vm was finished the migration successfully..)

Maybe even better to handle that in SectionHeadingWithEdit and not here.

sgratch · 2025-12-16T17:59:20Z

src/plans/details/tabs/Mappings/components/PlanStorageMapEdit/PlanStorageMapEdit.tsx

+        title={t('Edit storage map')}
+        closeModal={closeModal}
+        variant={ModalVariant.medium}
+        isDisabled={!isValid || !isDirty}


Please handle cases in which you can't edit the plan mappings and it should be disabled. E.g. plan is completed, archived, running while at least one vm was finished the migration successfully..)

Maybe even better to handle that in SectionHeadingWithEdit and not here.

src/networkMaps/create/utils/buildNetworkMappings.ts

metalice · 2025-12-22T12:10:37Z

...details/tabs/Mappings/components/PlanStorageMapEdit/components/PlanStorageMapFieldsTable.tsx

+  });
+
+  useEffect(() => {
+    setTimeout(async () => {


when trying the pattern of const func = async () => { await trigger(); }; func() I'm getting the following eslint error:
Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the void operator @typescript-eslint/no-floating-promises

u sure u need the await?

src/components/mappings/network-mappings/SourceNetworkField.tsx

src/components/mappings/network-mappings/TargetNetworkField.tsx

src/networkMaps/create/utils/__tests__/buildNetworkMappings.test.ts

src/plans/create/hooks/useInitializeMappings.ts

src/plans/create/steps/review/StorageMapReviewTable.tsx

avivtur · 2025-12-23T11:47:08Z

/retest

avivtur · 2025-12-23T15:23:49Z

/retest

src/plans/create/steps/review/StorageMapReviewTable.tsx

src/components/mappings/network-mappings/SourceNetworkField.tsx

src/components/mappings/network-mappings/TargetNetworkField.tsx

src/plans/create/steps/review/StorageMapReviewTable.tsx

...details/tabs/Mappings/components/PlanNetworkMapEdit/components/PlanNetworkMapFieldsTable.tsx

metalice · 2026-01-05T10:46:14Z

...details/tabs/Mappings/components/PlanNetworkMapEdit/components/PlanNetworkMapFieldsTable.tsx

+  useEffect(() => {
+    setTimeout(async () => {
+      await trigger();
+    }, 0);
+  }, [trigger]);


why? we dont any other way then this effect and timeout? IIRC check other hooks such as useLAyouteffect or others

well we do have something like:

useEffect(() => { const triggerFunc = async () => { try { await trigger(); } catch { // handle error if needed } }; (async () => { await triggerFunc(); })(); }, [trigger]);

src/plans/details/tabs/Mappings/components/PlanNetworkMapEdit/PlanNetworkMapEdit.tsx

src/plans/details/tabs/Mappings/components/PlanStorageMapEdit/PlanStorageMapEdit.tsx

metalice · 2026-01-05T10:50:32Z

src/plans/details/tabs/Mappings/hooks/usePlanNetworkMapResources.ts

+  const [networkMap] = networkMapResult ?? [];
+  const [sourceNetworks] = sourceNetworksResult ?? [];
+  const [targetNetworks] = targetNetworksResult ?? [];


seems redundent, do it directly getNetworkMappingValues

the getNetworkMappingValues is also used elsewhere so that would cause more changes

src/plans/details/tabs/Mappings/hooks/usePlanStorageMapResources.ts

jpuzz0 · 2026-01-07T16:51:45Z

...details/tabs/Mappings/components/PlanStorageMapEdit/components/PlanStorageMapFieldsTable.tsx

+        },
+        tooltip: (index) => {
+          if (storageMappingFields.length <= 1) {
+            return t('At least one network mapping must be provided.');


This message should read "storage" mapping instead of "network".

src/plans/details/tabs/Mappings/PlanMappingsPage.tsx

...details/tabs/Mappings/components/PlanNetworkMapEdit/components/PlanNetworkMapFieldsTable.tsx

...details/tabs/Mappings/components/PlanStorageMapEdit/components/PlanStorageMapFieldsTable.tsx

Signed-off-by: Aviv Turgeman <[email protected]>

sonarqubecloud · 2026-01-08T09:58:53Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.9% Duplication on New Code

See analysis details on SonarQube Cloud

avivtur force-pushed the standardize-mappings-tab branch 5 times, most recently from 66a0635 to 78f1a53 Compare December 16, 2025 11:27

sgratch reviewed Dec 16, 2025

View reviewed changes

avivtur force-pushed the standardize-mappings-tab branch 4 times, most recently from 0689873 to 5378a2a Compare December 18, 2025 09:53

metalice reviewed Dec 22, 2025

View reviewed changes

src/networkMaps/create/utils/buildNetworkMappings.ts Outdated Show resolved Hide resolved

metalice reviewed Dec 22, 2025

View reviewed changes

galkremer1 reviewed Dec 22, 2025

View reviewed changes

avivtur force-pushed the standardize-mappings-tab branch from 5378a2a to 782530f Compare December 23, 2025 11:04

avivtur force-pushed the standardize-mappings-tab branch from 782530f to e1b91bc Compare December 23, 2025 12:14