Copilot commented on code in PR #12144:
URL: https://github.com/apache/cloudstack/pull/12144#discussion_r3552654546
##########
ui/src/views/compute/DeployVM.vue:
##########
@@ -261,8 +261,11 @@
:minimum-cpunumber="templateConfigurationExists &&
selectedTemplateConfiguration && selectedTemplateConfiguration.cpunumber ?
selectedTemplateConfiguration.cpunumber : 0"
:minimum-cpuspeed="templateConfigurationExists &&
selectedTemplateConfiguration && selectedTemplateConfiguration.cpuspeed ?
selectedTemplateConfiguration.cpuspeed : 0"
:minimum-memory="templateConfigurationExists &&
selectedTemplateConfiguration && selectedTemplateConfiguration.memory ?
selectedTemplateConfiguration.memory : 0"
+
:service-offering-categories="options.serviceOfferingCategories"
+
:selected-service-offering-category-id="selectedServiceOfferingCategoryId"
@select-compute-item="($event) =>
updateComputeOffering($event)"
@handle-search-filter="($event) =>
handleSearchFilter('serviceOfferings', $event)"
+ @service-offering-category-change="($event) =>
handleServiceOfferingCategoryChange($event)"
Review Comment:
`selectedServiceOfferingCategoryId` and `options.serviceOfferingCategories`
are referenced here (and mutated in `handleServiceOfferingCategoryChange`) but
are not declared in this component’s reactive state. In Vue this will trigger
runtime warnings (property accessed during render but not defined) and the
category selection may not update reactively. Declare
`selectedServiceOfferingCategoryId` plus `options.serviceOfferingCategories`
and `loading.serviceOfferingCategories` in `data()` (similar to
DeployVnfAppliance.vue) so the selection and loading state are tracked.
##########
ui/src/components/view/ListView.vue:
##########
@@ -1056,6 +1056,9 @@
<template v-if="column.key === 'vgpuActions'">
<slot name="actionButtons" :record="record" :actions="actions"></slot>
</template>
+ <template v-if="column.key === 'category' && $route.path.split('/')[1]
=== 'computeoffering'">
+ <router-link :to="{ path: '/serviceofferingcategory/' +
record.categoryid }">{{ text }}</router-link>
+ </template>
Review Comment:
This always renders a link even when `record.categoryid` is missing,
producing a route like `/serviceofferingcategory/undefined`. Guard the link on
`record.categoryid` and fall back to plain text when it’s not available.
##########
server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java:
##########
@@ -168,6 +172,24 @@ public List<Long> getServiceOfferingZones(Long
serviceOfferingId) {
return null;
}
+ @Override
+ public ServiceOfferingCategory
createServiceOfferingCategory(CreateServiceOfferingCategoryCmd cmd) {
+ // TODO Auto-generated method stub
+ return null;
+ }
Review Comment:
This new mock method is left as an auto-generated TODO stub returning null.
If any test path accidentally hits it, failures will be confusing (NPEs later)
rather than clearly indicating the mock isn’t supposed to be used for this
call. Prefer throwing `UnsupportedOperationException` with a clear message (or
implement a minimal fake) to fail fast.
##########
server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java:
##########
@@ -168,6 +172,24 @@ public List<Long> getServiceOfferingZones(Long
serviceOfferingId) {
return null;
}
+ @Override
+ public ServiceOfferingCategory
createServiceOfferingCategory(CreateServiceOfferingCategoryCmd cmd) {
+ // TODO Auto-generated method stub
+ return null;
+ }
+
+ @Override
+ public boolean
deleteServiceOfferingCategory(DeleteServiceOfferingCategoryCmd cmd) {
+ // TODO Auto-generated method stub
+ return false;
+ }
+
+ @Override
+ public ServiceOfferingCategory
updateServiceOfferingCategory(UpdateServiceOfferingCategoryCmd cmd) {
+ // TODO Auto-generated method stub
+ return null;
+ }
Review Comment:
This new mock method is left as an auto-generated TODO stub returning null.
If a test unexpectedly calls it, it will likely fail later with a null
dereference. Prefer throwing `UnsupportedOperationException` (or implement a
minimal fake) to fail fast with a clear reason.
##########
server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java:
##########
@@ -168,6 +172,24 @@ public List<Long> getServiceOfferingZones(Long
serviceOfferingId) {
return null;
}
+ @Override
+ public ServiceOfferingCategory
createServiceOfferingCategory(CreateServiceOfferingCategoryCmd cmd) {
+ // TODO Auto-generated method stub
+ return null;
+ }
+
+ @Override
+ public boolean
deleteServiceOfferingCategory(DeleteServiceOfferingCategoryCmd cmd) {
+ // TODO Auto-generated method stub
+ return false;
+ }
Review Comment:
This new mock method is left as an auto-generated TODO stub returning false.
If a test unexpectedly calls it, it will silently look like a legitimate delete
failure. Prefer throwing `UnsupportedOperationException` (or implement a
minimal fake) so unintended usage is obvious.
--
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]
