[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7057: server/UI: fix some issues with network offering details

[GitBox <gi...@apache.org>]

DaanHoogland commented on code in PR #7057:
URL: https://github.com/apache/cloudstack/pull/7057#discussion_r1064743445


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -6408,7 +6408,9 @@ public NetworkOfferingVO doInTransaction(final TransactionStatus status) {
                             detailsVO.add(new NetworkOfferingDetailsVO(offering.getId(), Detail.internetProtocol, String.valueOf(internetProtocol), true));
                         }
                         if (!detailsVO.isEmpty()) {
-                            networkOfferingDetailsDao.saveDetails(detailsVO);

Review Comment:
   This is one of those mega methods that need disecting to understand (out of scope). I think @weizhouapache is right,a though it is strange that some of the details should already exist for a network offering we are busy creating.



##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -6769,6 +6771,7 @@ public boolean deleteNetworkOffering(final DeleteNetworkOfferingCmd cmd) {
 
         annotationDao.removeByEntityType(AnnotationService.EntityType.NETWORK_OFFERING.name(), offering.getUuid());
         if (_networkOfferingDao.remove(offeringId)) {
+            networkOfferingDetailsDao.removeDetails(offeringId);

Review Comment:
   shouldn´t we first remove the details and than the oddering?



-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org