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


##########
framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java:
##########
@@ -778,6 +783,19 @@ protected void loadPresetVariableValueForNetwork(UsageVO 
usageRecord, Value valu
         value.setId(network.getUuid());
         value.setName(network.getName());
         value.setState(usageRecord.getState());
+
+        
value.setNetworkOffering(getPresetVariableValueNetworkOffering(network.getNetworkOfferingId()));

Review Comment:
   The new networkOffering field is being set in 
loadPresetVariableValueForNetwork, but there are no tests verifying this 
integration. Similar methods like loadPresetVariableValueForVolume and 
loadPresetVariableValueForBackup have comprehensive tests that verify all 
fields are properly set. Consider adding a test similar to 
loadPresetVariableValueForBackupTestRecordIsBackupSetAllFields that verifies 
the networkOffering field is correctly populated.



##########
framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java:
##########
@@ -793,6 +811,18 @@ protected void loadPresetVariableValueForVpc(UsageVO 
usageRecord, Value value) {
 
         value.setId(vpc.getUuid());
         value.setName(vpc.getName());
+        
value.setVpcOffering(getPresetVariableValueVpcOffering(vpc.getVpcOfferingId()));

Review Comment:
   The new vpcOffering field is being set in loadPresetVariableValueForVpc, but 
there are no tests verifying this integration. Similar methods like 
loadPresetVariableValueForVolume and loadPresetVariableValueForBackup have 
comprehensive tests that verify all fields are properly set. Consider adding a 
test similar to loadPresetVariableValueForBackupTestRecordIsBackupSetAllFields 
that verifies the vpcOffering field is correctly populated.



##########
framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java:
##########
@@ -793,6 +811,18 @@ protected void loadPresetVariableValueForVpc(UsageVO 
usageRecord, Value value) {
 
         value.setId(vpc.getUuid());
         value.setName(vpc.getName());
+        
value.setVpcOffering(getPresetVariableValueVpcOffering(vpc.getVpcOfferingId()));
+    }
+
+    protected GenericPresetVariable getPresetVariableValueVpcOffering(long 
vpcOfferingId) {

Review Comment:
   The parameter type should be `Long` instead of `long` to maintain 
consistency with similar methods in this class. Other methods like 
`getPresetVariableValueNetworkOffering`, `getPresetVariableZone`, and 
`getPresetVariableValueTemplate` all use `Long` as the parameter type.
   ```suggestion
       protected GenericPresetVariable getPresetVariableValueVpcOffering(Long 
vpcOfferingId) {
   ```



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