Fred Rolland has posted comments on this change. Change subject: webadmin: Warning on dependent VMs on delete template ......................................................................
Patch Set 8: (8 comments) fixed comments https://gerrit.ovirt.org/#/c/37514/8/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/TemplateBackupModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/TemplateBackupModel.java: Line 104: TemplateBackupModel templateBackupModel = (TemplateBackupModel) model; Line 105: ArrayList<StoragePool> pools = (ArrayList<StoragePool>) returnValue; Line 106: if (pools != null && pools.size() > 0) { Line 107: StoragePool pool = pools.get(0); Line 108: getVmsFromExportDomain(pool.getId(), getEntity().getId()); > this method should be renamed to checkVmsDependentOnTemplate() or something Done Line 109: } Line 110: } Line 111: }), Line 112: getEntity().getId()); Line 109: } Line 110: } Line 111: }), Line 112: getEntity().getId()); Line 113: cancel(); > should be removed. the window should be closed only upon action completion. We need it to close the first confirmation dialog Line 114: } Line 115: Line 116: private void getVmsFromExportDomain(Guid dataCenterId, Guid storageDomainId) { Line 117: Frontend.getInstance().runQuery(VdcQueryType.GetVmsFromExportDomain, Line 122: private INewAsyncCallback createGetVmsFromExportDomainCallback() { Line 123: return new INewAsyncCallback() { Line 124: @Override Line 125: public void onSuccess(Object model, Object returnValue) { Line 126: List<VM> vmsInExportDomain; > you can define it on line 131 instead of here Done Line 127: VdcQueryReturnValue retVal = (VdcQueryReturnValue) returnValue; Line 128: if (retVal == null || retVal.getReturnValue() == null || !retVal.getSucceeded()) { Line 129: return; Line 130: } Line 128: if (retVal == null || retVal.getReturnValue() == null || !retVal.getSucceeded()) { Line 129: return; Line 130: } Line 131: vmsInExportDomain = retVal.getReturnValue(); Line 132: if ((vmsInExportDomain.size() > 0)) { > i'd remove this if condition from here, it can reside in the begining of ge Done Line 133: TemplateBackupModel templateBackupModel = (TemplateBackupModel) model; Line 134: HashMap<String, List<String>> problematicVmNames = Line 135: getDependentVMsForTemplate(vmsInExportDomain, templateBackupModel); Line 136: if (!problematicVmNames.isEmpty()) { Line 177: confirmModel.getCommands().add(cancelConfirmationUiCommand); Line 178: } Line 179: Line 180: private HashMap<String, List<String>> getDependentVMsForTemplate(List<VM> vmsInExportDomain, Line 181: TemplateBackupModel templateBackupModel) { > this method can get just the template list, it doesn't need the whole model Done Line 182: // Build a map between the template ID and the template instance Line 183: Map<Guid, VmTemplate> templateMap = Line 184: Entities.businessEntitiesById((List<VmTemplate>) templateBackupModel.getSelectedItems()); Line 185: // Build a map between the template ID and a list of dependent VMs names Line 203: setConfirmWindow(null); Line 204: } Line 205: Line 206: private void removeTemplateBackup() { Line 207: AsyncDataProvider.getInstance().getDataCentersByStorageDomain(new AsyncQuery(this, new INewAsyncCallback() { > why do u need to call it again (it's already called in line 100) ? Done. Added a local variable for the pool. Line 208: Line 209: @Override Line 210: public void onSuccess(Object model, Object returnValue) { Line 211: TemplateBackupModel templateBackupModel = (TemplateBackupModel) model; Line 221: pool.getId())); Line 222: } Line 223: Line 224: Frontend.getInstance().runMultipleAction(VdcActionType.RemoveVmTemplateFromImportExport, prms); Line 225: cancel(); > call cancel either way (i.e. move to line 227) - just in case... Done Line 226: } Line 227: Line 228: } Line 229: }), https://gerrit.ovirt.org/#/c/37514/8/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java File frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java: Line 2 Line 3 Line 4 Line 5 Line 6 > Why was it removed? I put it back -- To view, visit https://gerrit.ovirt.org/37514 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8cba5df5a41973741a855ae0c9efddd56100d2fd Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Candace Sheremeta <csher...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Fred Rolland <froll...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
