Daniel Erez has posted comments on this change. Change subject: engine:Warning on dependent VMs on delete template ......................................................................
Patch Set 6: (20 comments) http://gerrit.ovirt.org/#/c/37514/6//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2015-02-22 09:32:13 +0200 Line 4: Commit: Fred Rolland <[email protected]> Line 5: CommitDate: 2015-02-22 11:44:43 +0200 Line 6: Line 7: engine:Warning on dependent VMs on delete template s/'engine:'/'webadmin: ' Line 8: Line 9: When removing a backup template from an export domain, we check Line 10: if dependent backup VMs exist. Line 11: If such exists, a confirmation message will be prompt to the user http://gerrit.ovirt.org/#/c/37514/6/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 105: if (pools != null && pools.size() > 0) { Line 106: StoragePool pool = pools.get(0); Line 107: getVmsFromExportDomain(pool.getId(), getEntity().getId()); Line 108: } Line 109: redundant empty line Line 110: } Line 111: }), Line 112: getEntity().getId()); Line 113: cancel(); Line 109: Line 110: } Line 111: }), Line 112: getEntity().getId()); Line 113: cancel(); you should call cancel (close the dialog) only upon completing the operation Line 114: } Line 115: Line 116: private void getVmsFromExportDomain(Guid dataCenterId, Guid storageDomainId) { Line 117: Frontend.getInstance().runQuery(VdcQueryType.GetVmsFromExportDomain, Line 124: @Override Line 125: public void onSuccess(Object model, Object returnValue) { Line 126: List<VM> vmsInExportDomain; Line 127: VdcQueryReturnValue retVal = (VdcQueryReturnValue) returnValue; Line 128: if (retVal != null && retVal.getReturnValue() != null && retVal.getSucceeded()) { probably better use early return here, just to avoid further indention.. Line 129: vmsInExportDomain = retVal.getReturnValue(); Line 130: if ((vmsInExportDomain.size() > 0)) { Line 131: // Build a map between the template ID and the template instance Line 132: TemplateBackupModel templateBackupModel = (TemplateBackupModel) model; Line 129: vmsInExportDomain = retVal.getReturnValue(); Line 130: if ((vmsInExportDomain.size() > 0)) { Line 131: // Build a map between the template ID and the template instance Line 132: TemplateBackupModel templateBackupModel = (TemplateBackupModel) model; Line 133: HashMap<Guid, VmTemplate> templateMap = new HashMap<Guid, VmTemplate>(); you can remove 'Guid, VmTemplate' from generics in second clause Line 134: for (Object a : templateBackupModel.getSelectedItems()) { Line 135: VmTemplate template = (VmTemplate) a; Line 136: templateMap.put(template.getId(), template); Line 137: } Line 130: if ((vmsInExportDomain.size() > 0)) { Line 131: // Build a map between the template ID and the template instance Line 132: TemplateBackupModel templateBackupModel = (TemplateBackupModel) model; Line 133: HashMap<Guid, VmTemplate> templateMap = new HashMap<Guid, VmTemplate>(); Line 134: for (Object a : templateBackupModel.getSelectedItems()) { rename 'a' var.. Line 135: VmTemplate template = (VmTemplate) a; Line 136: templateMap.put(template.getId(), template); Line 137: } Line 138: // Build a map between the template ID and a list of dependent VMs names Line 132: TemplateBackupModel templateBackupModel = (TemplateBackupModel) model; Line 133: HashMap<Guid, VmTemplate> templateMap = new HashMap<Guid, VmTemplate>(); Line 134: for (Object a : templateBackupModel.getSelectedItems()) { Line 135: VmTemplate template = (VmTemplate) a; Line 136: templateMap.put(template.getId(), template); actually, try using 'Entities -> businessEntitiesById' instead Line 137: } Line 138: // Build a map between the template ID and a list of dependent VMs names Line 139: HashMap<String, List<String>> problematicVmNames = new HashMap<String, List<String>>(); Line 140: Line 135: VmTemplate template = (VmTemplate) a; Line 136: templateMap.put(template.getId(), template); Line 137: } Line 138: // Build a map between the template ID and a list of dependent VMs names Line 139: HashMap<String, List<String>> problematicVmNames = new HashMap<String, List<String>>(); same - remove 'String, List<String>' Line 140: Line 141: for (VM vm : vmsInExportDomain) { Line 142: VmTemplate template = templateMap.get(vm.getVmtGuid()); Line 143: if (template != null) { Line 137: } Line 138: // Build a map between the template ID and a list of dependent VMs names Line 139: HashMap<String, List<String>> problematicVmNames = new HashMap<String, List<String>>(); Line 140: Line 141: for (VM vm : vmsInExportDomain) { try to extract this logic - could make it a bit cleaner Line 142: VmTemplate template = templateMap.get(vm.getVmtGuid()); Line 143: if (template != null) { Line 144: Line 145: List<String> vms = problematicVmNames.get(template.getName()); Line 140: Line 141: for (VM vm : vmsInExportDomain) { Line 142: VmTemplate template = templateMap.get(vm.getVmtGuid()); Line 143: if (template != null) { Line 144: redundant empty line Line 145: List<String> vms = problematicVmNames.get(template.getName()); Line 146: if (vms == null) { Line 147: vms = new ArrayList<String>(); Line 148: problematicVmNames.put(template.getName(), vms); Line 155: Line 156: for (String templateName : problematicVmNames.keySet()) Line 157: { Line 158: List<String> vms = problematicVmNames.get(templateName); Line 159: StringBuilder sb = new StringBuilder("Template " + templateName + " (for VM(s) :"); //$NON-NLS-1$ //$NON-NLS-2$ use UIMessages for pattern and localization Line 160: sb.append(StringUtils.join(vms, ",")); //$NON-NLS-1$ Line 161: sb.append(")"); //$NON-NLS-1$ Line 162: missingTemplatesFromVms.add(sb.toString()); Line 163: } Line 161: sb.append(")"); //$NON-NLS-1$ Line 162: missingTemplatesFromVms.add(sb.toString()); Line 163: } Line 164: Line 165: ConfirmationModel confirmModel = new ConfirmationModel(); try to extract this logic (model and command creation) - should make it cleaner Line 166: setConfirmWindow(confirmModel); Line 167: confirmModel.setTitle(ConstantsManager.getInstance() Line 168: .getConstants() Line 169: .removeBackedUpTemplatesWithDependentsVMTitle()); Line 177: .getConstants() Line 178: .theFollowingTemplatesHaveDependentVmsBackupOnExportDomainMsg()); Line 179: confirmModel.setItems(missingTemplatesFromVms); Line 180: Line 181: UICommand tempVar = rename var Line 182: UICommand.createDefaultOkUiCommand("RemoveVmTemplates", TemplateBackupModel.this); //$NON-NLS-1$ Line 183: confirmModel.getCommands().add(tempVar); Line 184: UICommand tempVar2 = Line 185: UICommand.createCancelUiCommand("CancelConfirmation", TemplateBackupModel.this); //$NON-NLS-1$ Line 180: Line 181: UICommand tempVar = Line 182: UICommand.createDefaultOkUiCommand("RemoveVmTemplates", TemplateBackupModel.this); //$NON-NLS-1$ Line 183: confirmModel.getCommands().add(tempVar); Line 184: UICommand tempVar2 = rename var Line 185: UICommand.createCancelUiCommand("CancelConfirmation", TemplateBackupModel.this); //$NON-NLS-1$ Line 186: confirmModel.getCommands().add(tempVar2); Line 187: } else Line 188: { Line 184: UICommand tempVar2 = Line 185: UICommand.createCancelUiCommand("CancelConfirmation", TemplateBackupModel.this); //$NON-NLS-1$ Line 186: confirmModel.getCommands().add(tempVar2); Line 187: } else Line 188: { style - move bracket to eol Line 189: removeTemplateBackup(); Line 190: } Line 191: } Line 192: else Line 189: removeTemplateBackup(); Line 190: } Line 191: } Line 192: else Line 193: { same Line 194: removeTemplateBackup(); Line 195: } Line 196: Line 197: } Line 224: } Line 225: Line 226: Frontend.getInstance().runMultipleAction(VdcActionType.RemoveVmTemplateFromImportExport, prms); Line 227: } Line 228: redundant empty line Line 229: } Line 230: }), Line 231: getEntity().getId()); Line 232: setConfirmWindow(null); Line 472: { Line 473: onRestore(); Line 474: } Line 475: else if ("CancelConfirmation".equals(command.getName())) //$NON-NLS-1$ Line 476: { style - move bracket to eol Line 477: cancelConfirmation(); Line 478: } Line 479: else if ("RemoveVmTemplates".equals(command.getName())) //$NON-NLS-1$ Line 480: { Line 476: { Line 477: cancelConfirmation(); Line 478: } Line 479: else if ("RemoveVmTemplates".equals(command.getName())) //$NON-NLS-1$ Line 480: { same Line 481: removeTemplateBackup(); Line 482: } Line 483: else { Line 484: super.executeCommand(command); Line 482: } Line 483: else { Line 484: super.executeCommand(command); Line 485: } Line 486: redundant empty line Line 487: } Line 488: Line 489: @Override Line 490: protected String getListName() { -- To view, visit http://gerrit.ovirt.org/37514 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8cba5df5a41973741a855ae0c9efddd56100d2fd Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Freddy Rolland <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Amit Aviram <[email protected]> Gerrit-Reviewer: Candace Sheremeta <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Fred Rolland <[email protected]> Gerrit-Reviewer: Freddy Rolland <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
