Copilot commented on code in PR #12144:
URL: https://github.com/apache/cloudstack/pull/12144#discussion_r3521599011


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -9546,4 +9574,97 @@ public void setScope(String scope) {
             this.scope = scope;
         }
     }
+
+    @Override
+    @ActionEvent(eventType = 
EventTypes.EVENT_SERVICE_OFFERING_CATEGORY_CREATE, eventDescription = "creating 
service offering category")
+    public ServiceOfferingCategory 
createServiceOfferingCategory(CreateServiceOfferingCategoryCmd cmd) {
+        String name = cmd.getName();
+        Integer sortKey = cmd.getSortKey();
+

Review Comment:
   This PR adds new category CRUD behavior (create/update/delete + in-use 
checks) but there are no accompanying unit/integration tests covering the new 
API surface and validation paths (duplicate name, default-category protection, 
and in-use prevention). Adding tests will help prevent regressions in future 
schema/API changes.



##########
ui/src/views/compute/wizard/ComputeOfferingSelection.vue:
##########
@@ -17,6 +17,25 @@
 
 <template>
   <div>
+    <a-form-item
+      :label="$t('label.category')"
+      name="serviceofferingcategoryid"
+      ref="serviceofferingcategoryid"
+      v-if="serviceOfferingCategories && serviceOfferingCategories.length > 0"
+      style="margin-bottom: 16px;">
+      <block-radio-group-select
+        :maxBlocks="16"
+        :items="serviceOfferingCategories"
+        :selectedValue="selectedServiceOfferingCategoryId"
+        :horizontalGutter="6"
+        :verticalGutter="6"
+        blockSize="medium"
+        @change="handleServiceOfferingCategoryChange">
+        <template #radio-option="{ item }">
+              {{ item.name }}
+        </template>

Review Comment:
   The new `.radio-option` / `.ellipsis` styles added below won’t apply because 
the `radio-option` slot currently renders only raw text (no matching classes). 
As a result, long category names won’t get the intended ellipsis/centering 
behavior.



##########
ui/src/views/compute/DeployVM.vue:
##########
@@ -2277,6 +2280,46 @@ export default {
           console.error('Error fetching guestOsCategories:', e)
         })
     },
+    fetchServiceOfferingCategories () {
+      this.loading.serviceOfferingCategories = true
+      return new Promise((resolve, reject) => {
+        getAPI('listServiceOfferingCategories').then(json => {
+          const categories = 
json.listserviceofferingcategoriesresponse.serviceofferingcategory || []

Review Comment:
   `selectedServiceOfferingCategoryId`, `options.serviceOfferingCategories`, 
and `loading.serviceOfferingCategories` are used here but are not declared in 
`data()`. In DeployVM.vue this means the prop passed to 
`ComputeOfferingSelection` starts as `undefined` and the loading flag/category 
options are added dynamically, which can cause warnings and non-reactive 
updates (depending on Vue reactivity mode). These should be initialized in 
`data()` (e.g., selectedServiceOfferingCategoryId: '-1', 
options.serviceOfferingCategories: [], loading.serviceOfferingCategories: 
false).



-- 
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]

Reply via email to