Copilot commented on code in PR #14062:
URL: https://github.com/apache/cloudstack/pull/14062#discussion_r4350902451
##########
ui/src/views/offering/CloneVpcOffering.vue:
##########
@@ -656,6 +666,45 @@ export default {
this.form.nsxsupportlb = Boolean(this.serviceProviderMap.Lb)
}
},
+ syncServiceSelectionsForCurrentMode () {
+ const updatedServices = this.supportedServices.map(svc => {
+ const serviceCopy = { ...svc, provider: [...svc.provider] }
+ const providerName = this.selectedServiceProviderMap[serviceCopy.name]
+
+ if (providerName) {
+ const providerIndex = serviceCopy.provider.findIndex(p => p.name ===
providerName)
+ if (providerIndex > 0) {
+ const targetProvider = serviceCopy.provider[providerIndex]
+ serviceCopy.provider.splice(providerIndex, 1)
+ serviceCopy.provider.unshift(targetProvider)
+ }
+ serviceCopy.defaultChecked = true
+ serviceCopy.selectedProvider = providerName
+ } else {
+ serviceCopy.defaultChecked = false
+ serviceCopy.selectedProvider = null
+ }
Review Comment:
When `providerName` exists in `selectedServiceProviderMap` but is no longer
present in `serviceCopy.provider` (i.e., `providerIndex === -1`), the code
still sets `defaultChecked = true` and `selectedProvider = providerName`. That
can leave the UI (and later payload generation) in an invalid state with a
selected provider that isn’t available for that service/mode. Handle the
`providerIndex === -1` case explicitly (e.g., clear the selection, or fall back
to the first available provider and update `selectedServiceProviderMap`
accordingly).
##########
ui/src/views/offering/CloneVpcOffering.vue:
##########
@@ -656,6 +666,45 @@ export default {
this.form.nsxsupportlb = Boolean(this.serviceProviderMap.Lb)
}
},
+ syncServiceSelectionsForCurrentMode () {
+ const updatedServices = this.supportedServices.map(svc => {
+ const serviceCopy = { ...svc, provider: [...svc.provider] }
+ const providerName = this.selectedServiceProviderMap[serviceCopy.name]
+
+ if (providerName) {
+ const providerIndex = serviceCopy.provider.findIndex(p => p.name ===
providerName)
+ if (providerIndex > 0) {
+ const targetProvider = serviceCopy.provider[providerIndex]
+ serviceCopy.provider.splice(providerIndex, 1)
+ serviceCopy.provider.unshift(targetProvider)
+ }
+ serviceCopy.defaultChecked = true
+ serviceCopy.selectedProvider = providerName
+ } else {
+ serviceCopy.defaultChecked = false
+ serviceCopy.selectedProvider = null
+ }
+ return serviceCopy
+ })
+ this.supportedServices = updatedServices
+
+ const availableNames = new Set(updatedServices.map(svc => svc.name))
+ Object.keys(this.selectedServiceProviderMap).forEach(name => {
+ if (!availableNames.has(name)) {
+ delete this.selectedServiceProviderMap[name]
+ }
+ })
+
+ this.connectivityServiceChecked =
Boolean(this.selectedServiceProviderMap.Connectivity)
+ this.sourceNatServiceChecked =
Boolean(this.selectedServiceProviderMap.SourceNat)
+
+ this.$nextTick(() => {
+ this.servicesReady = true
+ this.$nextTick(() => {
+ this.checkVpcVirtualRouterForServices()
+ })
+ })
Review Comment:
The nested `$nextTick` calls make the update flow harder to follow and
reason about. Consider collapsing this into a single tick (or making the method
`async` and `await this.$nextTick()` once) so it’s clear when `servicesReady`
is set relative to `checkVpcVirtualRouterForServices()`. This reduces
control-flow complexity and makes future changes less error-prone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
