Omer Frenkel 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
Done
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 w
Done
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 el
Done
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.
Done
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 exec
Done
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 neede
Done
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.
Done
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
Done
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(
Done
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" w
Done
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
Done
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

Reply via email to