Address remaining review feedback on service offering categories

Fixes the outstanding reviewer/Copilot comments that were not covered by the earlier "apply copilot suggestions" commit: - Remove trailing space in ListServiceOfferingsCmd (checkstyle build failure) - Add dedicated EVENT_SERVICE_OFFERING_CATEGORY_{CREATE,EDIT,DELETE} event types mapped to ServiceOfferingCategory, and use them for category CRUD instead of the ServiceOffering event types (audit-trail misclassification) - Eliminate the N+1 category lookup in ServiceOfferingJoinDaoImpl by joining service_offering_category into service_offering_view and reading the category uuid/name directly from the join VO - Add missing ServiceOfferingDao.listByCategoryId used by the delete usage-check (the branch did not compile without it) - Add UNIQUE constraint on service_offering_category.name - UI: drop dead params.categoryid=null branch in DeployVM; fix double space in ListView category condition; fix radio-option__icon CSS typo; remove unused categories/categoryLoading state and fetchCategories() from AddComputeOffering and CloneComputeOffering
2026-07-01 21:22:33 +02:00 · 2026-07-01 21:22:33 +02:00 · 8260735251
parent 066cc1f4b2
commit 8260735251
14 changed files with 51 additions and 46 deletions
--- a/api/src/main/java/com/cloud/event/EventTypes.java
+++ b/api/src/main/java/com/cloud/event/EventTypes.java
@ -83,6 +83,7 @@ import com.cloud.network.vpc.VpcOffering;
 import com.cloud.offering.DiskOffering;
 import com.cloud.offering.NetworkOffering;
 import com.cloud.offering.ServiceOffering;
+import com.cloud.offering.ServiceOfferingCategory;
 import com.cloud.projects.Project;
 import com.cloud.server.ResourceTag;
 import com.cloud.storage.GuestOS;
@ -379,6 +380,11 @@ public class EventTypes {
    public static final String EVENT_SERVICE_OFFERING_EDIT = "SERVICE.OFFERING.EDIT";
    public static final String EVENT_SERVICE_OFFERING_DELETE = "SERVICE.OFFERING.DELETE";

+    // Service Offering Categories
+    public static final String EVENT_SERVICE_OFFERING_CATEGORY_CREATE = "SERVICE.OFFERING.CATEGORY.CREATE";
+    public static final String EVENT_SERVICE_OFFERING_CATEGORY_EDIT = "SERVICE.OFFERING.CATEGORY.EDIT";
+    public static final String EVENT_SERVICE_OFFERING_CATEGORY_DELETE = "SERVICE.OFFERING.CATEGORY.DELETE";
+
    // Disk Offerings
    public static final String EVENT_DISK_OFFERING_CREATE = "DISK.OFFERING.CREATE";
    public static final String EVENT_DISK_OFFERING_CLONE = "DISK.OFFERING.CLONE";
@ -1054,6 +1060,9 @@ public class EventTypes {
        entityEventDetails.put(EVENT_SERVICE_OFFERING_CLONE, ServiceOffering.class);
        entityEventDetails.put(EVENT_SERVICE_OFFERING_EDIT, ServiceOffering.class);
        entityEventDetails.put(EVENT_SERVICE_OFFERING_DELETE, ServiceOffering.class);
+        entityEventDetails.put(EVENT_SERVICE_OFFERING_CATEGORY_CREATE, ServiceOfferingCategory.class);
+        entityEventDetails.put(EVENT_SERVICE_OFFERING_CATEGORY_EDIT, ServiceOfferingCategory.class);
+        entityEventDetails.put(EVENT_SERVICE_OFFERING_CATEGORY_DELETE, ServiceOfferingCategory.class);

        // Disk Offerings
        entityEventDetails.put(EVENT_DISK_OFFERING_CREATE, DiskOffering.class);
--- a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java
+++ b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java
@ -204,7 +204,7 @@ public class ListServiceOfferingsCmd extends BaseListProjectAndAccountResourcesC
    public Long getCategoryId() {
        return categoryId;
    }
-  
+
    public void setZoneId(Long zoneId) {
        this.zoneId = zoneId;
    }
--- a/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDao.java
+++ b/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDao.java
@ -59,5 +59,7 @@ public interface ServiceOfferingDao extends GenericDao<ServiceOfferingVO, Long>

    List<Long> listIdsByHostTag(String tag);

+    List<ServiceOfferingVO> listByCategoryId(Long categoryId);
+
    void addCheckForGpuEnabled(SearchBuilder<ServiceOfferingVO> serviceOfferingSearch, Boolean gpuEnabled);
 }
--- a/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/service/dao/ServiceOfferingDaoImpl.java
@ -56,6 +56,7 @@ public class ServiceOfferingDaoImpl extends GenericDaoBase<ServiceOfferingVO, Lo
    protected final SearchBuilder<ServiceOfferingVO> ServiceOfferingsByKeywordSearch;
    protected final SearchBuilder<ServiceOfferingVO> PublicCpuRamSearch;
    protected final SearchBuilder<ServiceOfferingVO> SearchComputeOfferingByComputeOnlyDiskOffering;
+    protected final SearchBuilder<ServiceOfferingVO> CategoryIdSearch;

    public ServiceOfferingDaoImpl() {
        super();
@ -79,6 +80,17 @@ public class ServiceOfferingDaoImpl extends GenericDaoBase<ServiceOfferingVO, Lo
        SearchComputeOfferingByComputeOnlyDiskOffering = createSearchBuilder();
        SearchComputeOfferingByComputeOnlyDiskOffering.and("disk_offering_id", SearchComputeOfferingByComputeOnlyDiskOffering.entity().getDiskOfferingId(), SearchCriteria.Op.EQ);
        SearchComputeOfferingByComputeOnlyDiskOffering.done();
+
+        CategoryIdSearch = createSearchBuilder();
+        CategoryIdSearch.and("category_id", CategoryIdSearch.entity().getCategoryId(), SearchCriteria.Op.EQ);
+        CategoryIdSearch.done();
+    }
+
+    @Override
+    public List<ServiceOfferingVO> listByCategoryId(Long categoryId) {
+        SearchCriteria<ServiceOfferingVO> sc = CategoryIdSearch.create();
+        sc.setParameters("category_id", categoryId);
+        return listBy(sc);
    }

    @Override
--- a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql
+++ b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql
@ -25,7 +25,8 @@ CREATE TABLE IF NOT EXISTS `cloud`.`service_offering_category` (
 `uuid` varchar(40),
 `sort_key` int NOT NULL DEFAULT 0,
 PRIMARY KEY  (`id`),
- CONSTRAINT `uc_service_offering_category__uuid` UNIQUE (`uuid`)
+ CONSTRAINT `uc_service_offering_category__uuid` UNIQUE (`uuid`),
+ CONSTRAINT `uc_service_offering_category__name` UNIQUE (`name`)
 ) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8;

 INSERT INTO `cloud`.`service_offering_category` (id, name, uuid) VALUES (1, 'Default', UUID());
--- a/engine/schema/src/main/resources/META-INF/db/views/cloud.service_offering_view.sql
+++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.service_offering_view.sql
@ -86,6 +86,8 @@ SELECT
    `service_offering`.`gpu_count` AS `gpu_count`,
    `service_offering`.`gpu_display` AS `gpu_display`,
    `service_offering`.`category_id` AS `category_id`,
+    `service_offering_category`.`uuid` AS `category_uuid`,
+    `service_offering_category`.`name` AS `category_name`,
    GROUP_CONCAT(DISTINCT(domain.id)) AS domain_id,
    GROUP_CONCAT(DISTINCT(domain.uuid)) AS domain_uuid,
    GROUP_CONCAT(DISTINCT(domain.name)) AS domain_name,
@ -106,6 +108,8 @@ FROM
        LEFT JOIN
    `cloud`.`gpu_card` ON vgpu_profile.card_id = gpu_card.id
        LEFT JOIN
+    `cloud`.`service_offering_category` ON service_offering.category_id = service_offering_category.id
+        LEFT JOIN
    `cloud`.`service_offering_details` AS `domain_details` ON `domain_details`.`service_offering_id` = `service_offering`.`id` AND `domain_details`.`name`='domainid'
        LEFT JOIN
    `cloud`.`domain` AS `domain` ON FIND_IN_SET(`domain`.`id`, `domain_details`.`value`)
--- a/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java
+++ b/server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java
@ -44,8 +44,6 @@ import com.cloud.dc.dao.VsphereStoragePolicyDao;
 import com.cloud.offering.ServiceOffering;
 import com.cloud.server.ResourceTag.ResourceObjectType;
 import com.cloud.storage.DiskOfferingVO;
-import com.cloud.service.dao.ServiceOfferingCategoryDao;
-import com.cloud.service.ServiceOfferingCategoryVO;
 import com.cloud.user.AccountManager;
 import com.cloud.utils.db.GenericDaoBase;
 import com.cloud.utils.db.Filter;
@ -66,8 +64,6 @@ public class ServiceOfferingJoinDaoImpl extends GenericDaoBase<ServiceOfferingJo
    private ConfigurationDao configDao;
    @Inject
    private AccountManager accountManager;
-    @Inject
-    private ServiceOfferingCategoryDao _serviceOfferingCategoryDao;

    private SearchBuilder<ServiceOfferingJoinVO> sofIdSearch;

@ -152,13 +148,10 @@ public class ServiceOfferingJoinDaoImpl extends GenericDaoBase<ServiceOfferingJo
        offeringResponse.setGpuCount(offering.getGpuCount());
        offeringResponse.setGpuDisplay(offering.getGpuDisplay());

-        // Set category information if available
-        if (offering.getCategoryId() != null) {
-            ServiceOfferingCategoryVO category = _serviceOfferingCategoryDao.findById(offering.getCategoryId());
-            if (category != null) {
-                offeringResponse.setCategoryId(category.getUuid());
-                offeringResponse.setCategoryName(category.getName());
-            }
+        // Set category information if available (joined via service_offering_view)
+        if (offering.getCategoryUuid() != null) {
+            offeringResponse.setCategoryId(offering.getCategoryUuid());
+            offeringResponse.setCategoryName(offering.getCategoryName());
        }

        offeringResponse.setNetworkRate(offering.getRateMbps());
--- a/server/src/main/java/com/cloud/api/query/vo/ServiceOfferingJoinVO.java
+++ b/server/src/main/java/com/cloud/api/query/vo/ServiceOfferingJoinVO.java
@ -266,6 +266,12 @@ public class ServiceOfferingJoinVO extends BaseViewVO implements InternalIdentit
    @Column(name = "category_id")
    private Long categoryId;

+    @Column(name = "category_uuid")
+    private String categoryUuid;
+
+    @Column(name = "category_name")
+    private String categoryName;
+
    public ServiceOfferingJoinVO() {
    }

@ -562,4 +568,8 @@ public class ServiceOfferingJoinVO extends BaseViewVO implements InternalIdentit
    }

    public Long getCategoryId() { return categoryId; }
+
+    public String getCategoryUuid() { return categoryUuid; }
+
+    public String getCategoryName() { return categoryName; }
 }
--- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
+++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
@ -9576,7 +9576,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
    }

    @Override
-    @ActionEvent(eventType = EventTypes.EVENT_SERVICE_OFFERING_CREATE, eventDescription = "creating service offering category")
+    @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();
@ -9598,7 +9598,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
    }

    @Override
-    @ActionEvent(eventType = EventTypes.EVENT_SERVICE_OFFERING_DELETE, eventDescription = "deleting service offering category")
+    @ActionEvent(eventType = EventTypes.EVENT_SERVICE_OFFERING_CATEGORY_DELETE, eventDescription = "deleting service offering category")
    public boolean deleteServiceOfferingCategory(DeleteServiceOfferingCategoryCmd cmd) {
        Long categoryId = cmd.getId();

@ -9627,7 +9627,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
    }

    @Override
-    @ActionEvent(eventType = EventTypes.EVENT_SERVICE_OFFERING_EDIT, eventDescription = "updating service offering category")
+    @ActionEvent(eventType = EventTypes.EVENT_SERVICE_OFFERING_CATEGORY_EDIT, eventDescription = "updating service offering category")
    public ServiceOfferingCategory updateServiceOfferingCategory(UpdateServiceOfferingCategoryCmd cmd) {
        Long categoryId = cmd.getId();
        String name = cmd.getName();
--- a/ui/src/components/view/ListView.vue
+++ b/ui/src/components/view/ListView.vue
@ -1056,7 +1056,7 @@
      <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'">
+      <template v-if="column.key === 'category' && $route.path.split('/')[1] === 'computeoffering'">
        <router-link :to="{ path: '/serviceofferingcategory/' + record.categoryid }">{{ text }}</router-link>
      </template>
    </template>
--- a/ui/src/views/compute/DeployVM.vue
+++ b/ui/src/views/compute/DeployVM.vue
@ -2315,8 +2315,6 @@ export default {
      }
      if (categoryId && categoryId !== '-1') {
        params.categoryid = categoryId
-      } else {
-        params.categoryid = null
      }
      this.handleSearchFilter('serviceOfferings', params)
    },
--- a/ui/src/views/compute/wizard/ComputeOfferingSelection.vue
+++ b/ui/src/views/compute/wizard/ComputeOfferingSelection.vue
@ -446,7 +446,7 @@ export default {
    width: 100%;
    height: 100%;

-    .radio-opion__icon {
+    .radio-option__icon {
      font-size: 2rem;
      margin-bottom: 0.5rem;
    }
--- a/ui/src/views/offering/AddComputeOffering.vue
+++ b/ui/src/views/offering/AddComputeOffering.vue
@ -117,9 +117,7 @@ export default {
      leaseexpiryaction: undefined,
      vgpuProfiles: [],
      vgpuProfileLoading: false,
-      externalDetailsEnabled: false,
-      categories: [],
-      categoryLoading: false
+      externalDetailsEnabled: false
    }
  },
  beforeCreate () {
@ -231,7 +229,6 @@ export default {
      this.fetchDomainData()
      this.fetchZoneData()
      this.fetchGPUCards()
-      this.fetchCategories()
      if (isAdmin()) {
        this.fetchStorageTagData()
        this.fetchDeploymentPlannerData()
@ -257,15 +254,6 @@ export default {
        this.gpuCardLoading = false
      })
    },
-    fetchCategories () {
-      this.categoryLoading = true
-      getAPI('listServiceOfferingCategories', {
-      }).then(json => {
-        this.categories = json.listserviceofferingcategoriesresponse.serviceofferingcategory || []
-      }).finally(() => {
-        this.categoryLoading = false
-      })
-    },
    fetchDiskOfferings () {
      this.diskOfferingLoading = true
      getAPI('listDiskOfferings', {
--- a/ui/src/views/offering/CloneComputeOffering.vue
+++ b/ui/src/views/offering/CloneComputeOffering.vue
@ -140,9 +140,7 @@ export default {
      leaseexpiryaction: undefined,
      vgpuProfiles: [],
      vgpuProfileLoading: false,
-      externalDetailsEnabled: false,
-      categories: [],
-      categoryLoading: false
+      externalDetailsEnabled: false
    }
  },
  beforeCreate () {
@ -254,7 +252,6 @@ export default {
      this.fetchDomainData()
      this.fetchZoneData()
      this.fetchGPUCards()
-      this.fetchCategories()
      if (isAdmin()) {
        this.fetchStorageTagData()
        this.fetchDeploymentPlannerData()
@ -400,15 +397,6 @@ export default {
        this.gpuCardLoading = false
      })
    },
-    fetchCategories () {
-      this.categoryLoading = true
-      getAPI('listServiceOfferingCategories', {
-      }).then(json => {
-        this.categories = json.listserviceofferingcategoriesresponse.serviceofferingcategory || []
-      }).finally(() => {
-        this.categoryLoading = false
-      })
-    },
    fetchDiskOfferings () {
      this.diskOfferingLoading = true
      getAPI('listDiskOfferings', {