Shubhendu Tripathi has posted comments on this change. Change subject: webadmin: UI for gluster volume snapshot creation ......................................................................
Patch Set 31: (8 comments) https://gerrit.ovirt.org/#/c/35082/31/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java: Line 1642: public void getVolumeSnapshotSchedule(AsyncQuery aQuery, Guid volumeId) { Line 1643: aQuery.converterCallback = new IAsyncConverter() { Line 1644: @Override Line 1645: public Object Convert(Object source, AsyncQuery asyncQuery) { Line 1646: return source != null ? source : null; > This converter does not have any effect Got it. Will remove this. Line 1647: } Line 1648: }; Line 1649: Frontend.getInstance().runQuery(VdcQueryType.GetGlusterVolumeSnapshotScheduleByVolumeId, Line 1650: new IdQueryParameters(volumeId), https://gerrit.ovirt.org/#/c/35082/31/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeSnapshotListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeSnapshotListModel.java: Line 228: setConfirmWindow(null); Line 229: } else if (command.equals(getNewSnapshotCommand())) { Line 230: newSnapshot(); Line 231: } else if (command.getName().equalsIgnoreCase("onCreateSnapshot")) {//$NON-NLS-1$ Line 232: onCreateSnapshot(); > You can have consistent names for newSnapshot and onCreateSnapshot. Done Line 233: } else if (command.getName().equalsIgnoreCase("cancel")) {//$NON-NLS-1$ Line 234: setWindow(null); Line 235: } else if (command.equals(getEditSnapshotScheduleCommand())) { Line 236: editSnapshotSchedule(); Line 431: GlusterVolumeEntity volumeEntity = getEntity(); Line 432: boolean scheduleTabVisible = false; Line 433: if (!(volumeEntity.getSnapshotScheduled())) { Line 434: scheduleTabVisible = true; Line 435: } > instead of this 'if' block you could just do Done Line 436: GlusterVolumeSnapshotModel snapshotModel = Line 437: new GlusterVolumeSnapshotModel(true, scheduleTabVisible); Line 438: Line 439: snapshotModel.setHelpTag(HelpTag.new_volume_snapshot); Line 585: GlusterVolumeSnapshotModel snapshotModel) { Line 586: if (returnValue != null && returnValue.getSucceeded()) { Line 587: setWindow(null); Line 588: } Line 589: } > Above two functions does the same. Can this be combined and named something Done Line 590: Line 591: public void editSnapshotSchedule() { Line 592: if (getWindow() != null) { Line 593: return; https://gerrit.ovirt.org/#/c/35082/31/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeSnapshotModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeSnapshotModel.java: Line 254: && validWeekDays && validMonthDays && validEndDate; Line 255: } Line 256: Line 257: public enum EndDateOptions { Line 258: HasEndDate(ConstantsManager.getInstance().getConstants().endDateRadioText()), > endDateOpionText Done Line 259: NoEndDate(ConstantsManager.getInstance().getConstants().noEndDateRadioText()); Line 260: Line 261: private String description; Line 262: Line 255: } Line 256: Line 257: public enum EndDateOptions { Line 258: HasEndDate(ConstantsManager.getInstance().getConstants().endDateRadioText()), Line 259: NoEndDate(ConstantsManager.getInstance().getConstants().noEndDateRadioText()); > noEndDateOptionText Done Line 260: Line 261: private String description; Line 262: Line 263: private EndDateOptions(String description) { https://gerrit.ovirt.org/#/c/35082/31/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 2661: Line 2662: @DefaultStringValue("End by date cannot be before start date") Line 2663: String endDateBeforeStartDate(); Line 2664: Line 2665: @DefaultStringValue("Unable to fethc gluster volume snapshot schedule") > fetch Done Line 2666: String unableToFetchVolumeSnapshotSchedule(); https://gerrit.ovirt.org/#/c/35082/31/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 261: @Override Line 262: public void setErrorMsgLabelVisibility(boolean visible, String msg) { Line 263: errorMsgLabel.setVisible(visible); Line 264: errorMsgLabel.setText(msg); Line 265: } > I think there is already a setMessage() method from super class is availabl Will check that and use if possible Line 266: Line 267: private void updateTabVisibilities(GlusterVolumeSnapshotModel object) { Line 268: generalTab.setVisible(object.isGeneralTabVisible()); Line 269: scheduleTab.setVisible(object.isScheduleTabVisible()); -- To view, visit https://gerrit.ovirt.org/35082 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I823580ecb127e48e075c437668bfb19ff8ec4467 Gerrit-PatchSet: 31 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@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
