anmolbabu has posted comments on this change. Change subject: webadmin: UI for gluster volume snapshot create ......................................................................
Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/35082/6/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/volumes/VolumeListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/volumes/VolumeListModel.java: Line 354: snapshotModel.setHelpTag(HelpTag.new_snapshot); Line 355: snapshotModel.setHashName("new_snapshot"); //$NON-NLS-1$ Line 356: snapshotModel.setTitle(ConstantsManager.getInstance().getConstants().createSnapshotTitle()); Line 357: setWindow(snapshotModel); Line 358: GlusterVolumeEntity volumeEntity = (GlusterVolumeEntity) getSelectedItem(); Kindly check if this works. Bcoz, I feel the intended purpose of getSelectedItem may not be accomplished by doing it after setWindow. if works : please ignore; else place it above setWindow Line 359: snapshotModel.getClusterName().setEntity(volumeEntity.getVdsGroupName()); Line 360: snapshotModel.getVolumeName().setEntity(volumeEntity.getName()); Line 361: Line 362: UICommand command = new UICommand("onCreateSnapshot", this); //$NON-NLS-1$ Line 667: for (Object selectedVolume : getSelectedItems()) { Line 668: selectedVolumes.add((GlusterVolumeEntity) selectedVolume); Line 669: } Line 670: optimizeVolumesForVirtStore(selectedVolumes); Line 671: } else if (command.getName().equalsIgnoreCase("createSnapshot")) {//$NON-NLS-1$ you could probably check for command.equals(getNewSnapshotCommand()) instead of comparing based on string. I however see no harm in doing this HERE. Line 672: newSnapshot(); Line 673: } else if (command.getName().equalsIgnoreCase("onCreateSnapshot")) {//$NON-NLS-1$ Line 674: onCreateSnapshot(); Line 675: } Line 1175: Line 1176: private void onCreateSnapshot() { Line 1177: SnapshotModel snapshotModel = (SnapshotModel) getWindow(); Line 1178: Line 1179: if (!snapshotModel.validate()) I feel it is better not to ignore this invalidity but instead handle it as a warning probably on key out. Here,In this case I feel neither is the window closed nor is any other operation done. Line 1180: { Line 1181: return; Line 1182: } Line 1183: http://gerrit.ovirt.org/#/c/35082/6/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterVolumeSnapshotCreatePopupView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterVolumeSnapshotCreatePopupView.java: Line 62: @Path(value = "forceCreate.entity") Line 63: @WithElementId Line 64: EntityModelCheckBoxEditor forceCreateEditor; Line 65: Line 66: ApplicationMessages messages; This field isn't used Line 67: ApplicationConstants constants; Line 68: CommonApplicationTemplates templates; Line 69: Line 70: private final Driver driver = GWT.create(Driver.class); -- To view, visit http://gerrit.ovirt.org/35082 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I823580ecb127e48e075c437668bfb19ff8ec4467 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: anmolbabu <anb...@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
