Tomas Jelinek has posted comments on this change. Change subject: webadmin: warn when exporting template version if base is missing ......................................................................
Patch Set 1: (11 comments) http://gerrit.ovirt.org/#/c/26000/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/TemplateListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/TemplateListModel.java: Line 252: public void onSuccess(Object target, Object returnValue) { Line 253: TemplateListModel templateListModel = (TemplateListModel) target; Line 254: HashMap<VmTemplate, ArrayList<DiskImage>> templatesDiskSet = Line 255: (HashMap<VmTemplate, ArrayList<DiskImage>>) returnValue; Line 256: HashMap<String, ArrayList<String>> templateDic = the templateDic is not used Line 257: new HashMap<String, ArrayList<String>>(); Line 258: ArrayList<String> verTempMissingBase = new ArrayList<String>(); Line 259: Line 260: // check if relevant templates are already there Line 292: { Line 293: ExportVmModel model = (ExportVmModel) getWindow(); Line 294: ArrayList<VdcActionParametersBase> parameters = new ArrayList<VdcActionParametersBase>(); Line 295: Line 296: model.stopProgress(); this stop and than the start again would cause a "blink" in the screen. I would remove it from here (see my following comments where to put it) Line 297: Line 298: for (Object item : getSelectedItems()) Line 299: { Line 300: VmTemplate a = (VmTemplate) item; Line 295: Line 296: model.stopProgress(); Line 297: Line 298: for (Object item : getSelectedItems()) Line 299: { This code to prepare the parameters and the code to call the export (the else statement to the if ((missingTemplatesFromVms == null || missingTemplatesFromVms.size() > 0)) is the same as the code in the onExportNoTemplates(). So if I'm not mistaken, you could just delete this code, than delete the whole body of the else statement and in the else statement just call the onExportNoTemplates() method. Line 300: VmTemplate a = (VmTemplate) item; Line 301: if (a.getId().equals(Guid.Empty)) Line 302: { Line 303: continue; Line 308: tempVar.setForceOverride((Boolean) model.getForceOverride().getEntity()); Line 309: parameters.add(tempVar); Line 310: } Line 311: Line 312: if ((missingTemplatesFromVms == null || missingTemplatesFromVms.size() > 0)) The missingTemplatesFromVms can not be null. And also, the outer parenthesis can be removed Line 313: { Line 314: ConfirmationModel confirmModel = new ConfirmationModel(); Line 315: setConfirmWindow(confirmModel); Line 316: confirmModel.setTitle(ConstantsManager.getInstance() Line 309: parameters.add(tempVar); Line 310: } Line 311: Line 312: if ((missingTemplatesFromVms == null || missingTemplatesFromVms.size() > 0)) Line 313: { so I would put the stopProgress here since no more async calls will be executed Line 314: ConfirmationModel confirmModel = new ConfirmationModel(); Line 315: setConfirmWindow(confirmModel); Line 316: confirmModel.setTitle(ConstantsManager.getInstance() Line 317: .getConstants() Line 318: .baseTemplatesNotFoundOnExportDomainTitle()); Line 319: confirmModel.setHelpTag(HelpTag.base_template_not_found_on_export_domain); Line 320: confirmModel.setHashName("base_template_not_found_on_export_domain"); //$NON-NLS-1$ Line 321: Line 322: confirmModel.setMessage(missingTemplatesFromVms == null ? ConstantsManager.getInstance() the missingTemplatesFromVms can never be null so the condition is not needed Line 323: .getConstants() Line 324: .couldNotReadTemplatesFromExportDomainMsg() Line 325: : ConstantsManager.getInstance() Line 326: .getConstants() Line 338: } Line 339: else Line 340: { Line 341: if (model.getProgress() != null) Line 342: { this condition is not needed. Line 343: return; Line 344: } Line 345: Line 346: model.startProgress(null); Line 342: { Line 343: return; Line 344: } Line 345: Line 346: model.startProgress(null); this start progress is also not needed since it is still progressing Line 347: Line 348: Frontend.getInstance().runMultipleAction(VdcActionType.ExportVmTemplate, parameters, Line 349: new IFrontendMultipleActionAsyncCallback() { Line 350: @Override Line 344: } Line 345: Line 346: model.startProgress(null); Line 347: Line 348: Frontend.getInstance().runMultipleAction(VdcActionType.ExportVmTemplate, parameters, so, this whole block could be removed and just call the onExportNoTemplats() Line 349: new IFrontendMultipleActionAsyncCallback() { Line 350: @Override Line 351: public void executed(FrontendMultipleActionAsyncResult result) { Line 352: Line 359: } Line 360: } Line 361: Line 362: private void onExportNoTemplates() Line 363: { since this has no logic specific to the fact that there are "NoTemplates" what about calling it just "doExport"? Line 364: ExportVmModel model = (ExportVmModel) getWindow(); Line 365: Line 366: if (model.getProgress() != null) Line 367: { Line 362: private void onExportNoTemplates() Line 363: { Line 364: ExportVmModel model = (ExportVmModel) getWindow(); Line 365: Line 366: if (model.getProgress() != null) not needed - it is not a problem if the startProgress is called more times Line 367: { Line 368: return; Line 369: } Line 370: -- To view, visit http://gerrit.ovirt.org/26000 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I028f9e98a31cca391e05e2e02c535f5f1b2068cf Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Omer Frenkel <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[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
