CNV-89816: Cancel all pending uploads when VM creation is canceled or VM is deleted#4077
CNV-89816: Cancel all pending uploads when VM creation is canceled or VM is deleted#4077galkremer1 wants to merge 1 commit into
Conversation
|
@galkremer1: This pull request references CNV-89816 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target either version "5.0." or "openshift-5.0.", but it targets "CNV v4.23.0" instead. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: galkremer1 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughExtends the upload progress store with VM-scoped and bulk cancellation actions, introducing helpers to match uploads by VM scope and cancel them on deletion or wizard reset. Upload key derivation gains cluster normalization for empty-cluster VMs, upload success/context links are gated on VM UID presence, and a new cleanup utility removes stale toast notifications for disappeared uploads. ChangesVM Upload Cancellation Lifecycle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
✅ Jira Validation Passed CNV-89816
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/hooks/useUploadProgressToast/uploadProgressStore.test.ts`:
- Around line 351-385: The test for cancelAllPendingUploads only validates the
success path where all cancelUpload callbacks resolve. Add a new test case
within the same describe block that sets up at least one upload with a
cancelUpload callback that rejects or throws an error, then call
cancelAllPendingUploads() and assert the entry is not silently dropped.
Explicitly verify the intended fallback or retention behavior when a
cancellation fails, ensuring this failure scenario is properly handled and
documented as a regression test.
In `@src/utils/hooks/useUploadProgressToast/utils/cancelUpload.ts`:
- Around line 26-28: The removeUpload call in the cancelUpload function is
executed immediately after initiating cancellation, but before confirming that
the cancellation actually succeeded. This creates a race condition where the
upload entry is removed from the store while the cancellation request is still
in flight, causing loss of visibility if the cancellation fails. Move the
get().removeUpload(uploadKey) call to execute only after the cancellation
operation completes successfully, ensuring the upload remains trackable if the
cancellation fails. Apply this same fix at the other occurrence in the file
around lines 66-68 where this pattern also exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 37527289-3317-453f-b030-2285c79fc8bd
⛔ Files ignored due to path filters (6)
locales/en/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/es/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/fr/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/ja/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/ko/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/zh/plugin__kubevirt-plugin.jsonis excluded by!locales/**
📒 Files selected for processing (17)
src/utils/components/DiskModal/UploadDiskModal.tsxsrc/utils/components/DiskModal/utils/submit.tssrc/utils/hooks/useUploadProgressToast/UploadProgressToastListener.tsxsrc/utils/hooks/useUploadProgressToast/uploadProgressStore.test.tssrc/utils/hooks/useUploadProgressToast/uploadProgressStore.tssrc/utils/hooks/useUploadProgressToast/utils/cancelPendingVmUploads.test.tssrc/utils/hooks/useUploadProgressToast/utils/cancelPendingVmUploads.tssrc/utils/hooks/useUploadProgressToast/utils/cancelUpload.tssrc/utils/hooks/useUploadProgressToast/utils/cleanupRemovedUploads.tssrc/utils/hooks/useUploadProgressToast/utils/types.tssrc/utils/hooks/useUploadProgressToast/utils/uploadCompletion.tssrc/utils/hooks/useUploadProgressToast/utils/uploadKeys.test.tssrc/utils/hooks/useUploadProgressToast/utils/uploadKeys.tssrc/utils/resources/template/utils/processOpenShiftTemplate.tssrc/views/virtualmachines/actions/actions.tssrc/views/virtualmachines/actions/components/DeleteVMModal/utils/deleteVM.tssrc/views/virtualmachines/creation-wizard/state/vm-wizard-store/useVMWizardStore.ts
galkremer1
force-pushed
the
CNV-89816-abort-pending-uploads
branch
from
5cd9dee to
d552f41
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/views/virtualmachines/actions/components/DeleteVMModal/utils/deleteVM.ts`:
- Line 36: The cancelPendingVmUploads function call is not being awaited, which
can cause race conditions where the delete proceeds before upload cancellation
completes. Add await before the cancelPendingVmUploads(vm) invocation to ensure
it finishes before continuing, and wrap it in a try-catch block to handle any
errors that may occur during cancellation rather than silently swallowing them.
Log any caught errors with meaningful context to aid debugging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ecf25837-e541-463f-9d2f-2cbb55f95b00
⛔ Files ignored due to path filters (6)
locales/en/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/es/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/fr/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/ja/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/ko/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/zh/plugin__kubevirt-plugin.jsonis excluded by!locales/**
📒 Files selected for processing (17)
src/utils/components/DiskModal/UploadDiskModal.tsxsrc/utils/components/DiskModal/utils/submit.tssrc/utils/hooks/useUploadProgressToast/UploadProgressToastListener.tsxsrc/utils/hooks/useUploadProgressToast/uploadProgressStore.test.tssrc/utils/hooks/useUploadProgressToast/uploadProgressStore.tssrc/utils/hooks/useUploadProgressToast/utils/cancelPendingVmUploads.test.tssrc/utils/hooks/useUploadProgressToast/utils/cancelPendingVmUploads.tssrc/utils/hooks/useUploadProgressToast/utils/cancelUpload.tssrc/utils/hooks/useUploadProgressToast/utils/cleanupRemovedUploads.tssrc/utils/hooks/useUploadProgressToast/utils/types.tssrc/utils/hooks/useUploadProgressToast/utils/uploadCompletion.tssrc/utils/hooks/useUploadProgressToast/utils/uploadKeys.test.tssrc/utils/hooks/useUploadProgressToast/utils/uploadKeys.tssrc/utils/resources/template/utils/processOpenShiftTemplate.tssrc/views/virtualmachines/actions/actions.tssrc/views/virtualmachines/actions/components/DeleteVMModal/utils/deleteVM.tssrc/views/virtualmachines/creation-wizard/state/vm-wizard-store/useVMWizardStore.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- src/utils/components/DiskModal/utils/submit.ts
- src/utils/hooks/useUploadProgressToast/utils/cleanupRemovedUploads.ts
- src/utils/components/DiskModal/UploadDiskModal.tsx
- src/utils/resources/template/utils/processOpenShiftTemplate.ts
- src/utils/hooks/useUploadProgressToast/utils/types.ts
- src/utils/hooks/useUploadProgressToast/utils/uploadKeys.test.ts
- src/views/virtualmachines/creation-wizard/state/vm-wizard-store/useVMWizardStore.ts
- src/utils/hooks/useUploadProgressToast/utils/cancelPendingVmUploads.test.ts
- src/utils/hooks/useUploadProgressToast/UploadProgressToastListener.tsx
- src/utils/hooks/useUploadProgressToast/utils/cancelPendingVmUploads.ts
- src/views/virtualmachines/actions/actions.ts
- src/utils/hooks/useUploadProgressToast/utils/cancelUpload.ts
- src/utils/hooks/useUploadProgressToast/utils/uploadKeys.ts
- src/utils/hooks/useUploadProgressToast/uploadProgressStore.test.ts
galkremer1
force-pushed
the
CNV-89816-abort-pending-uploads
branch
from
d552f41 to
5fcd9ed
Compare
|
CI Triage (auto-retest 1/5 for Classification: Infrastructure failure Log excerpt (infrastructure pattern matched)/retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066903109799514112/build-log.txt" | tail -300Length of output: 19984 The failure is an infrastructure/environment flake, not a code issue. The setup step failed because the Playwright test could not complete the login flow on the ephemeral CI cluster (
None of this is related to the code changes in this PR. This is a transient CI cluster readiness issue. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066934379787063296/build-log.txt" | tail -400Length of output: 13014 This is another CI infrastructure flake, unrelated to the code changes in this PR. The failure root cause is:
None of this is related to the PR's code changes. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066936583629574144/build-log.txt" | tail -400Length of output: 13468 This is the same CI infrastructure flake as the previous attempt — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066937873285779456/build-log.txt" | tail -400Length of output: 13468 This is the same CI infrastructure flake as the two previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066939639901458432/build-log.txt" | tail -400Length of output: 13465 This is the same CI infrastructure flake as the four previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066940722820419584/build-log.txt" | tail -400Length of output: 13464 This is the same CI infrastructure flake as the five previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066941742816432128/build-log.txt" | tail -400Length of output: 13466 This is the same CI infrastructure flake as all previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066943005801058304/build-log.txt" | tail -400Length of output: 14344 This is the same CI infrastructure flake as all previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066944130725974016/build-log.txt" | tail -400Length of output: 14347 This is the same CI infrastructure flake as all previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066945410357792768/build-log.txt" | tail -400Length of output: 14347 This is the same CI infrastructure flake as all previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066946624457805824/build-log.txt" | tail -400Length of output: 14344 This is the same CI infrastructure flake as all previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066947965666201600/build-log.txt" | tail -400Length of output: 14345 This is the same CI infrastructure flake as all previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066949021238300672/build-log.txt" | tail -400Length of output: 14347 This is the same CI infrastructure flake as all previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066950806975811584/build-log.txt" | tail -400Length of output: 14346 This is the same CI infrastructure flake as all previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
curl -sL "https://storage.googleapis.com/test-platform-results/pr-logs/pull/kubevirt-ui_kubevirt-plugin/4077/pull-ci-kubevirt-ui-kubevirt-plugin-main-kubevirt-e2e-aws/2066952073127137280/build-log.txt" | tail -400Length of output: 14345 This is the same CI infrastructure flake as all previous attempts — unrelated to the PR's code changes.
This is a CI platform-level environment injection issue, not a code problem. /retest |
|
CI Triage (auto-retest 2/5 for Classification: Infrastructure failure Log excerpt (infrastructure pattern matched)/retest |
|
CI Triage (auto-retest 3/5 for Classification: Infrastructure failure Log excerpt (infrastructure pattern matched)/retest |
galkremer1
force-pushed
the
CNV-89816-abort-pending-uploads
branch
from
5fcd9ed to
8464c95
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/components/DiskModal/utils/types.ts`:
- Around line 88-98: The BuildUploadTrackMetadataParams type declares file and t
as required properties, but the actual caller uploadDataVolume treats them as
optional, creating a runtime risk. In the buildUploadTrackMetadata function, add
a conditional guard for the translate parameter (the t parameter) before calling
getVmDiskUploadSuccessLinks, ensuring contextLinks is only constructed when
translate is defined, similar to how file is already guarded against undefined
values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0e154a50-5c0d-449a-8805-5b5a36eba88f
⛔ Files ignored due to path filters (6)
locales/en/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/es/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/fr/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/ja/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/ko/plugin__kubevirt-plugin.jsonis excluded by!locales/**locales/zh/plugin__kubevirt-plugin.jsonis excluded by!locales/**
📒 Files selected for processing (19)
src/utils/components/DiskModal/UploadDiskModal.tsxsrc/utils/components/DiskModal/utils/helpers.tssrc/utils/components/DiskModal/utils/submit.tssrc/utils/components/DiskModal/utils/types.tssrc/utils/hooks/useUploadProgressToast/UploadProgressToastListener.tsxsrc/utils/hooks/useUploadProgressToast/uploadProgressStore.test.tssrc/utils/hooks/useUploadProgressToast/uploadProgressStore.tssrc/utils/hooks/useUploadProgressToast/utils/cancelPendingVmUploads.test.tssrc/utils/hooks/useUploadProgressToast/utils/cancelPendingVmUploads.tssrc/utils/hooks/useUploadProgressToast/utils/cancelUpload.tssrc/utils/hooks/useUploadProgressToast/utils/cleanupRemovedUploads.tssrc/utils/hooks/useUploadProgressToast/utils/types.tssrc/utils/hooks/useUploadProgressToast/utils/uploadCompletion.tssrc/utils/hooks/useUploadProgressToast/utils/uploadKeys.test.tssrc/utils/hooks/useUploadProgressToast/utils/uploadKeys.tssrc/utils/resources/template/utils/processOpenShiftTemplate.tssrc/views/virtualmachines/actions/actions.tssrc/views/virtualmachines/actions/components/DeleteVMModal/utils/deleteVM.tssrc/views/virtualmachines/creation-wizard/state/vm-wizard-store/useVMWizardStore.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- src/utils/hooks/useUploadProgressToast/utils/types.ts
- src/utils/components/DiskModal/UploadDiskModal.tsx
- src/utils/resources/template/utils/processOpenShiftTemplate.ts
- src/utils/hooks/useUploadProgressToast/utils/uploadCompletion.ts
- src/utils/hooks/useUploadProgressToast/utils/cancelPendingVmUploads.ts
- src/utils/hooks/useUploadProgressToast/uploadProgressStore.ts
- src/utils/hooks/useUploadProgressToast/utils/cleanupRemovedUploads.ts
- src/utils/hooks/useUploadProgressToast/UploadProgressToastListener.tsx
- src/utils/hooks/useUploadProgressToast/utils/cancelPendingVmUploads.test.ts
- src/views/virtualmachines/actions/components/DeleteVMModal/utils/deleteVM.ts
- src/utils/hooks/useUploadProgressToast/utils/cancelUpload.ts
- src/utils/hooks/useUploadProgressToast/utils/uploadKeys.test.ts
- src/utils/hooks/useUploadProgressToast/utils/uploadKeys.ts
- src/utils/hooks/useUploadProgressToast/uploadProgressStore.test.ts
| export type BuildUploadTrackMetadataParams = { | ||
| abortTooltip: UploadDataVolumeOptions['abortTooltip']; | ||
| data: V1DiskFormState; | ||
| dataVolume: V1beta1DataVolume; | ||
| file: File; | ||
| isCDROM: boolean; | ||
| onCancelCleanup: UploadDataVolumeOptions['onCancelCleanup']; | ||
| t: TFunction; | ||
| uploadKey: string | undefined; | ||
| vm: V1VirtualMachine; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TypeScript compiler options =="
fd -i 'tsconfig*.json' | while read -r file; do
echo "--- ${file} ---"
jq '.compilerOptions | {strict, strictNullChecks}' "${file}"
done
echo
echo "== uploadDataVolume declarations and usages =="
rg -n --type=ts --type=tsx -C3 '\buploadDataVolume\s*\('
echo
echo "== buildUploadTrackMetadata usages =="
rg -n --type=ts --type=tsx -C3 '\bbuildUploadTrackMetadata\s*\('Repository: kubevirt-ui/kubevirt-plugin
Length of output: 352
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check tsconfig for strict mode =="
if [ -f tsconfig.json ]; then
jq -r '.compilerOptions | "\(.strict // "not set") strict, \(.strictNullChecks // "not set") strictNullChecks"' tsconfig.json
fi
echo
echo "== Current BuildUploadTrackMetadataParams definition =="
cat src/utils/components/DiskModal/utils/types.ts
echo
echo "== Search for uploadDataVolume with correct syntax =="
rg -n 'uploadDataVolume' --glob='**/*.ts' --glob='**/*.tsx' -C 3
echo
echo "== Search for buildUploadTrackMetadata calls =="
rg -n 'buildUploadTrackMetadata' --glob='**/*.ts' --glob='**/*.tsx' -C 3Repository: kubevirt-ui/kubevirt-plugin
Length of output: 12412
🏁 Script executed:
#!/bin/bash
echo "== uploadDataVolume full signature and buildUploadTrackMetadata call =="
sed -n '38,75p' src/utils/components/DiskModal/utils/submit.ts
echo
echo "== buildUploadTrackMetadata implementation =="
sed -n '479,510p' src/utils/components/DiskModal/utils/helpers.ts
echo
echo "== UploadDiskModal uploadDataVolume call context =="
sed -n '75,95p' src/utils/components/DiskModal/UploadDiskModal.tsx
echo
echo "== vmCdromBackgroundUpload uploadDataVolume call =="
sed -n '45,55p' src/utils/components/DiskModal/utils/vmCdromBackgroundUpload.tsRepository: kubevirt-ui/kubevirt-plugin
Length of output: 2921
🏁 Script executed:
#!/bin/bash
echo "== getVmDiskUploadSuccessLinks implementation =="
rg -n 'getVmDiskUploadSuccessLinks' --glob='**/*.ts' --glob='**/*.tsx' -A 8 -B 2 | head -50
echo
echo "== Check UploadDataProps type =="
rg -n 'UploadDataProps' --glob='**/*.ts' --glob='**/*.tsx' -A 5 -B 2Repository: kubevirt-ui/kubevirt-plugin
Length of output: 20454
Align BuildUploadTrackMetadataParams optionality with actual caller contract
file and t are required in BuildUploadTrackMetadataParams, but uploadDataVolume accepts both as optional (t?: TFunction and file = data?.uploadFile?.file). The type mismatch creates a runtime risk: buildUploadTrackMetadata guards against undefined file but not translate, so calling getVmDiskUploadSuccessLinks(translate, ...) with undefined translate could fail.
Suggested contract fix
export type BuildUploadTrackMetadataParams = {
abortTooltip: UploadDataVolumeOptions['abortTooltip'];
data: V1DiskFormState;
dataVolume: V1beta1DataVolume;
- file: File;
+ file?: File;
isCDROM: boolean;
onCancelCleanup: UploadDataVolumeOptions['onCancelCleanup'];
- t: TFunction;
- uploadKey: string | undefined;
+ t?: TFunction;
+ uploadKey?: string;
vm: V1VirtualMachine;
};// In buildUploadTrackMetadata (helpers.ts), gate link construction:
contextLinks:
translate && getUID(vm)
? getVmDiskUploadSuccessLinks(translate, vm, diskName, dvName, isCDROM)
: undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type BuildUploadTrackMetadataParams = { | |
| abortTooltip: UploadDataVolumeOptions['abortTooltip']; | |
| data: V1DiskFormState; | |
| dataVolume: V1beta1DataVolume; | |
| file: File; | |
| isCDROM: boolean; | |
| onCancelCleanup: UploadDataVolumeOptions['onCancelCleanup']; | |
| t: TFunction; | |
| uploadKey: string | undefined; | |
| vm: V1VirtualMachine; | |
| }; | |
| export type BuildUploadTrackMetadataParams = { | |
| abortTooltip: UploadDataVolumeOptions['abortTooltip']; | |
| data: V1DiskFormState; | |
| dataVolume: V1beta1DataVolume; | |
| file?: File; | |
| isCDROM: boolean; | |
| onCancelCleanup: UploadDataVolumeOptions['onCancelCleanup']; | |
| t?: TFunction; | |
| uploadKey?: string; | |
| vm: V1VirtualMachine; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/components/DiskModal/utils/types.ts` around lines 88 - 98, The
BuildUploadTrackMetadataParams type declares file and t as required properties,
but the actual caller uploadDataVolume treats them as optional, creating a
runtime risk. In the buildUploadTrackMetadata function, add a conditional guard
for the translate parameter (the t parameter) before calling
getVmDiskUploadSuccessLinks, ensuring contextLinks is only constructed when
translate is defined, similar to how file is already guarded against undefined
values.
4 participants
📝 Description
When a user cancels or navigates away from the VM creation wizard, or deletes an existing VM, all in-flight uploads (CD-ROM ISO, disk) associated with that VM should be automatically canceled.
🔗 Links
Jira ticket: CNV-89816
🎥 Demo
Demo.mov
Summary by CodeRabbit