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]