Shubhendu Tripathi has posted comments on this change.

Change subject: webadmin: UI for gluster volume snapshots
......................................................................


Patch Set 13:

(10 comments)

http://gerrit.ovirt.org/#/c/35082/13/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 88:         boolean allowCreateSnapshot = true;
Line 89: 
Line 90:         if (getEntity() == null) {
Line 91:             allowCreateSnapshot = false;
Line 92:         }
> you might want to disable "New Snapshot" when volume is down
yes may be. will do that.
Line 93: 
Line 94:         getNewSnapshotCommand().setIsAvailable(allowCreateSnapshot);
Line 95:     }
Line 96: 


Line 122:         }
Line 123: 
Line 124:         final UIConstants constants = 
ConstantsManager.getInstance().getConstants();
Line 125: 
Line 126:         GlusterVolumeEntity volumeEntity = (GlusterVolumeEntity) 
getEntity();
> Casting is not required here
done
Line 127:         GlusterVolumeSnapshotModel snapshotModel = new 
GlusterVolumeSnapshotModel();
Line 128:         snapshotModel.setHelpTag(HelpTag.new_volume_snapshot);
Line 129:         snapshotModel.setHashName("new_volume_snapshot"); 
//$NON-NLS-1$
Line 130:         
snapshotModel.setTitle(ConstantsManager.getInstance().getConstants().createSnapshotTitle());


Line 159:         
snapshot.setDescription(snapshotModel.getDescription().getEntity());
Line 160:         boolean force = snapshotModel.getForceCreate().getEntity();
Line 161: 
Line 162:         CreateGlusterVolumeSnapshotParameters parameter =
Line 163:                 new CreateGlusterVolumeSnapshotParameters(snapshot, 
force, true);
> you need to place a progress spinner on the dialog to stop the user from cl
done
Line 164: 
Line 165:         
Frontend.getInstance().runAction(VdcActionType.CreateGlusterVolumeSnapshot,
Line 166:                 parameter,
Line 167:                 new IFrontendActionAsyncCallback() {


Line 166:                 parameter,
Line 167:                 new IFrontendActionAsyncCallback() {
Line 168:                     @Override
Line 169:                     public void executed(FrontendActionAsyncResult 
result) {
Line 170:                         GlusterVolumeSnapshotListModel localModel = 
(GlusterVolumeSnapshotListModel) result.getState();
> And
done
Line 171:                         
localModel.postOnCreateSnapshot(result.getReturnValue(), snapshot);
Line 172:                     }
Line 173:                 },
Line 174:                 this);


http://gerrit.ovirt.org/#/c/35082/13/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 15:     private EntityModel<String> description;
Line 16:     private EntityModel<Boolean> forceCreate;
Line 17:     private EntityModel<Boolean> activate;
Line 18:     private String volumeQuotaMessage = "";
Line 19:     private String validationFailureMessage = "";
> Is this being used?
not required. will remove this.
Line 20: 
Line 21:     public GlusterVolumeSnapshotModel() {
Line 22:         init();
Line 23:         setAvailabilities();


http://gerrit.ovirt.org/#/c/35082/13/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/popup/gluster/GlusterVolumeSnapshotCreatePopupPresenterWidget.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/popup/gluster/GlusterVolumeSnapshotCreatePopupPresenterWidget.java:

Line 19: 
Line 20:     @Override
Line 21:     public void init(final GlusterVolumeSnapshotModel model) {
Line 22:         super.init(model);
Line 23:         model.getForceCreate().getEntityChangedEvent().addListener(new 
IEventListener<EventArgs>() {
> +1 for moving model listener registration into PresenterWidget =)
Thanks for your suggestion :)
Line 24:             @Override
Line 25:             public void eventRaised(Event<? extends EventArgs> ev, 
Object sender, EventArgs args) {
Line 26:                 
getView().setForceLabelVisibility(model.getForceCreate().getEntity());
Line 27:             }


http://gerrit.ovirt.org/#/c/35082/13/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 104: 
Line 105:     @Override
Line 106:     public void edit(final GlusterVolumeSnapshotModel object) {
Line 107:         driver.edit(object);
Line 108:         
forceWarningLabel.setVisible(object.getForceCreate().getEntity());
> We already have method for this, so to avoid duplicating code:
ok. will do that.
Line 109:     }
Line 110: 
Line 111:     @Override
Line 112:     public GlusterVolumeSnapshotModel flush() {


http://gerrit.ovirt.org/#/c/35082/13/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterVolumeSnapshotCreatePopupView.ui.xml
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterVolumeSnapshotCreatePopupView.ui.xml:

Line 27:         }
Line 28: 
Line 29:                .checkBoxEditorWidget{
Line 30:                width: 450px;
Line 31:                }
> Fix alignments
done
Line 32:        </ui:style>
Line 33: 
Line 34:        <d:SimpleDialogPanel width="480px" height="400px">
Line 35:                <d:content>


Line 49:                                <g:VerticalPanel 
addStyleNames="{style.panelStyle}">
Line 50:                                    <g:TextArea 
ui:field="forceWarningLabel" addStyleNames="{style.forceWarningLabel}"/>
Line 51:                                </g:VerticalPanel>
Line 52: 
Line 53:                        </g:VerticalPanel>
> Indeed, we should avoid "classic" GWT layout widgets like VerticalPanel and
Sure. Kanagaraj, I would take support on this and do the changes.
Line 54:                </d:content>
Line 55:        </d:SimpleDialogPanel>


http://gerrit.ovirt.org/#/c/35082/13/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/gluster/SubTabGlusterVolumeSnapshotView.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/gluster/SubTabGlusterVolumeSnapshotView.java:

Line 112:             protected UICommand resolveCommand() {
Line 113:                 // return 
getDetailModel().getDeactivateSnapshotCommand();
Line 114:                 return null;
Line 115:             }
Line 116:         });
> Can these actions be disabled till the functionality is implemented?
Sure.
Line 117:     }


-- 
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: 13
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: anmolbabu <[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