Resolves: MTV-4481 | Add appliance management UI input for OVA providers by avivtur · Pull Request #2229 · kubev2v/forklift-console-plugin

avivtur · 2026-02-03T11:39:09Z

📝 Links

References: https://issues.redhat.com/browse/MTV-4481

📝 Description

Add UI input for the applianceManagement setting on OVA providers, which controls whether users can upload OVA files directly to the provider.

Changes:

Add checkbox field in the create provider form when OVA type is selected
Add details item with edit capability in the OVA provider details page
Add shared constants for label and description text
Add selector for reading applianceManagement from provider spec
Add E2E test verification for the appliance management setting

When enabled, the "Upload local OVA files" section becomes visible on the provider details page.

🎥 Demo

📝 CC://

codecov-commenter · 2026-02-03T11:41:03Z

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

Codecov Report

❌ Patch coverage is 25.00000% with 63 lines in your changes missing coverage. Please review.
✅ Project coverage is 14.58%. Comparing base (13484d0) to head (780e66c).
⚠️ Report is 965 commits behind head on main.

Files with missing lines	Patch %	Lines
.../DetailsSection/ApplianceManagementDetailsItem.tsx	0.00%	20 Missing ⚠️
...ponents/DetailsSection/EditApplianceManagement.tsx	0.00%	17 Missing ⚠️
...ents/DetailsSection/onUpdateApplianceManagement.ts	0.00%	14 Missing ⚠️
...roviders/create/utils/buildOvaProviderResources.ts	25.00%	3 Missing ⚠️
...ls/components/DetailsSection/OVADetailsSection.tsx	0.00%	3 Missing ⚠️
src/components/OvaFileUploader/OvaFileUploader.tsx	0.00%	2 Missing ⚠️
.../create/fields/ova/OvaApplianceManagementField.tsx	84.61%	2 Missing ⚠️
src/utils/crds/common/selectors.ts	50.00%	2 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2229       +/-   ##
===========================================
- Coverage   36.81%   14.58%   -22.23%     
===========================================
  Files         158     1127      +969     
  Lines        2548    20175    +17627     
  Branches      599     3767     +3168     
===========================================
+ Hits          938     2943     +2005     
- Misses       1428    17222    +15794     
+ 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.

Pedro-S-Abreu · 2026-02-03T14:10:51Z

LGTM

jpuzz0 · 2026-02-03T14:42:15Z

Why is it that in the create provider form there is no ability to upload local OVA files but in the details after creation there is?

jpuzz0 · 2026-02-03T14:45:32Z

There isn't supposed to be a minus sign next to the helper text. Our other helper text does not have this and I don't believe its meant for this use-case regardless.

jpuzz0 · 2026-02-03T14:47:17Z

Is there meant to be more than 1 field contained in this section called "Upload local OVA files"? Seems redundant to have that title then another just below it called "Upload local OVA file". Should we just have 1 or the other?

jpuzz0 · 2026-02-03T14:48:32Z

I think the bottom section of this tooltip text with the arrows would be more readable if we make sure it doesn't wrap.

jpuzz0 · 2026-02-03T14:50:27Z

Also, why not contain the OVA local file upload within the edit more for appliance management to keep the details page more simple?

jpuzz0 · 2026-02-03T14:53:42Z

src/providers/details/tabs/Details/components/DetailsSection/ApplianceManagementDetailsItem.tsx

+            {t('Enabled')}
+          </Label>
+        ) : (
+          <Label isCompact status="info">


I'm not sure it makes sense to have "info" status for something labeled as "Disabled". Do we do this elsewhere? Otherwise I would suggest a default status.

jpuzz0 · 2026-02-03T15:03:06Z

src/providers/details/tabs/Details/components/DetailsSection/onUpdateApplianceManagement.ts

+  const obj = await k8sPatch({
+    data: [
+      {
+        op: currentValue ? REPLACE : ADD,


I'm not exactly sure why the Modal utils possess REPLACE/ADD/REMOVE operations for k8sPatch operations (something we might want to consider moving to a place for this just pertaining to k8s operations), but either way, it looks like REMOVE is an option, and wouldn't that make more sense to use when appliance management is "disabled" vs REPLACE?

It cluters a bit the code, I made the changes what do you think?

jpuzz0 · 2026-02-03T15:06:03Z

src/providers/details/tabs/Details/components/DetailsSection/onUpdateApplianceManagement.ts

+  provider: V1beta1Provider,
+  enabled: boolean,
+): Promise<V1beta1Provider> => {
+  const currentValue = getApplianceManagement(provider);


nit: since this is used to determine replace vs add, I wonder if its a better read to convert to value returned to a boolean and call it something like isAppManagementEnabled?

I added this selector because it makes sense, but in this specific case I want to see if I have any value in there (including false value) so I would know the patch operation should replace or delete

avivtur · 2026-02-03T18:14:15Z

Why is it that in the create provider form there is no ability to upload local OVA files but in the details after creation there is?

the service to upload the OVA files is created only after the provider creation, not during.

Also, why not contain the OVA local file upload within the edit more for appliance management to keep the details page more simple?

Not sure if we should hide this upload if I remember they wanted it exposed in a way a user would not miss it, but I do think that's just temporary just to make them aware of it, so we can ask Margot about this

jpuzz0 · 2026-02-03T18:18:21Z

src/providers/details/tabs/Details/components/DetailsSection/EditApplianceManagement.tsx

+  const [enabled, setEnabled] = useState(isApplianceManagementEnabled(provider));
+
+  const onSubmit = async (): Promise<void> => {
+    await onUpdateApplianceManagement(provider, enabled);


nit: no point of awaiting here. can just return onUpdateApplianceManagement(provider, enabled);

jpuzz0 · 2026-02-03T18:20:11Z

src/providers/details/tabs/Details/components/DetailsSection/onUpdateApplianceManagement.ts

+  const currentValue = getApplianceManagement(provider);
+  const getOpAndValue = () => {
+    if (currentValue === enabled.toString()) return undefined;
+    if (isEmpty(currentValue) && enabled) return { op: ADD, value: TRUE_VALUE };


Isn't currentValue just a string? Do we need to use isEmpty on strings? I thought it was mainly for objects and arrays.

it can be also used with strings, any reason not to?
this function is meant to replace the one in loadsh: https://lodash.com/docs/#isEmpty (we wanted to avoid adding the entire package for just some bits of the package)

jpuzz0 · 2026-02-03T18:21:52Z

src/providers/details/tabs/Details/components/DetailsSection/onUpdateApplianceManagement.ts

+  const getOpAndValue = () => {
+    if (currentValue === enabled.toString()) return undefined;
+    if (isEmpty(currentValue) && enabled) return { op: ADD, value: TRUE_VALUE };
+    if (!isEmpty(currentValue) && enabled) return { op: REPLACE, value: TRUE_VALUE };


nit: I personally think this is less readable with all if statements being single lines with their return statements

thought so, will revert

jpuzz0

Few minor comments, but otherwise LGTM

Add checkbox field to enable appliance management when creating OVA providers and in the provider details page. This setting controls whether users can upload OVA files directly to the provider. Changes: - Add checkbox in create provider form for OVA type - Add details item with edit capability in OVA provider details page - Add E2E test verification for appliance management setting Resolves: MTV-4481 Signed-off-by: Aviv Turgeman <[email protected]>

sonarqubecloud · 2026-02-04T06:37:10Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

avivtur · 2026-02-04T08:03:49Z

/backport release-2.11

github-actions · 2026-02-04T08:04:04Z

🔄 Backport Status

Starting backport of PR #2229 to release-2.11
🚀 Live mode - Changes will be applied

View workflow run

github-actions · 2026-02-04T08:04:13Z

✅ Backport Completed Successfully

PR #2229 has been successfully backported to release-2.11.

A new pull request should have been created with the backported changes.

avivtur force-pushed the expose-checkbox-for-ova-upload branch 2 times, most recently from e3233c8 to 4305cce Compare February 3, 2026 13:59

jpuzz0 reviewed Feb 3, 2026

View reviewed changes

avivtur force-pushed the expose-checkbox-for-ova-upload branch 2 times, most recently from e2dbc99 to 7840390 Compare February 3, 2026 18:11

jpuzz0 reviewed Feb 3, 2026

View reviewed changes

jpuzz0 approved these changes Feb 3, 2026

View reviewed changes

avivtur force-pushed the expose-checkbox-for-ova-upload branch from 7840390 to 780e66c Compare February 4, 2026 06:36

avivtur added this pull request to the merge queue Feb 4, 2026

Merged via the queue into kubev2v:main with commit 9619084 Feb 4, 2026
10 checks passed

avivtur deleted the expose-checkbox-for-ova-upload branch February 4, 2026 08:03

github-actions bot mentioned this pull request Feb 4, 2026

[release-2.11] Resolves: MTV-4481 | Add appliance management UI input for OVA providers #2232

Merged

Conversation

avivtur commented Feb 3, 2026

📝 Links

📝 Description

🎥 Demo

📝 CC://

Uh oh!

codecov-commenter commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Pedro-S-Abreu commented Feb 3, 2026

Uh oh!

jpuzz0 commented Feb 3, 2026

Uh oh!

jpuzz0 commented Feb 3, 2026

Uh oh!

jpuzz0 commented Feb 3, 2026

Uh oh!

jpuzz0 commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jpuzz0 commented Feb 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avivtur commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpuzz0 left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 4, 2026

Quality Gate passed

Uh oh!

Uh oh!

avivtur commented Feb 4, 2026

Uh oh!

github-actions bot commented Feb 4, 2026

Uh oh!

github-actions bot commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Feb 3, 2026 •

edited

Loading

jpuzz0 commented Feb 3, 2026 •

edited

Loading

avivtur commented Feb 3, 2026 •

edited

Loading