anmolbabu has posted comments on this change.

Change subject: webadmin : Create + Delete Geo-rep session
......................................................................


Patch Set 39:

(6 comments)

https://gerrit.ovirt.org/#/c/29691/39/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeoRepListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeGeoRepListModel.java:

Line 224:     @Override
Line 225:     public void executeCommand(UICommand command) {
Line 226:         super.executeCommand(command);
Line 227:         if (command.equals(getNewSessionCommand())) {
Line 228:             createNewGeoRepSession();
> createGeoRepSession
Done
Line 229:         } else if (command.equals(getRemoveSessionCommand())) {
Line 230:             removeGeoRepSession();
Line 231:         } else if(command.equals(getStartSessionCommand())) {
Line 232:             startGeoRepSession();


Line 373:     }
Line 374: 
Line 375:     private void onCreateSession() {
Line 376:         final GlusterVolumeGeoRepCreateModel createModel = 
(GlusterVolumeGeoRepCreateModel) getWindow();
Line 377:         createModel.startProgress(null);
> You might want the validate the values filled in the ui, atleast not null
so basically all fields except user name and user group are a selected ones 
from the available list.
Line 378:         final Guid masterVolumeId = getEntity().getId();
Line 379:         final String remoteVolumeName = 
createModel.getSlaveVolumes().getSelectedItem().getName();
Line 380:         final String remoteHostName = 
createModel.getSlaveHosts().getSelectedItem();
Line 381:         String remoteUserName = 
createModel.getSlaveUserName().getEntity();


Line 375:     private void onCreateSession() {
Line 376:         final GlusterVolumeGeoRepCreateModel createModel = 
(GlusterVolumeGeoRepCreateModel) getWindow();
Line 377:         createModel.startProgress(null);
Line 378:         final Guid masterVolumeId = getEntity().getId();
Line 379:         final String remoteVolumeName = 
createModel.getSlaveVolumes().getSelectedItem().getName();
> Possible NPE if there are no volumes selected
Done
Line 380:         final String remoteHostName = 
createModel.getSlaveHosts().getSelectedItem();
Line 381:         String remoteUserName = 
createModel.getSlaveUserName().getEntity();
Line 382:         String remoteUserGroup = 
createModel.getSlaveUserGroupName().getEntity();
Line 383:         
Frontend.getInstance().runAction(VdcActionType.CreateGlusterVolumeGeoRepSession,


Line 399:                                                 remoteHostName),
Line 400:                                         new 
IFrontendActionAsyncCallback() {
Line 401:                                             @Override
Line 402:                                             public void 
executed(FrontendActionAsyncResult result) {
Line 403:                                                 closeWindow();
> I think you need not wait for the 'start' operation to close the dialog. Yo
We discussed this might not be feasible
Line 404:                                                 if 
(!result.getReturnValue().getSucceeded()) {
Line 405:                                                     
initializeGeoRepActionConfirmation(constants.geoReplicationStartTitle(),
Line 406:                                                             
HelpTag.volume_geo_rep_start_confirmation,
Line 407:                                                             
"volume_geo_rep_start_confirmation", constants.geoRepForceHelp(), 
messages.geoRepForceTitle(constants.startGeoRep()), 
"onStartGeoRepSession");//$NON-NLS-1$//$NON-NLS-2$


https://gerrit.ovirt.org/#/c/29691/39/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterVolumeGeoRepCreateSessionPopupView.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterVolumeGeoRepCreateSessionPopupView.java:

Line 184:         
setSuggestedConfigViolations(object.getRecommendationViolations());
Line 185: 
Line 186:         setUserGroupVisibility(false);
Line 187: 
Line 188:         
object.getShowEligibleVolumes().getEntityChangedEvent().addListener(new 
IEventListener<EventArgs>() {
> This should be done in the model
Done
Line 189:             @Override
Line 190:             public void eventRaised(Event<? extends EventArgs> ev, 
Object sender, EventArgs args) {
Line 191:                 boolean overrideSuggestedConfig = 
object.getShowEligibleVolumes().getEntity();
Line 192:                 if(!overrideSuggestedConfig) {


Line 228:                 object.getSlaveHosts().setItems(hostsInCurrentVolume);
Line 229:             }
Line 230:         };
Line 231:         
object.getSlaveVolumes().getSelectedItemChangedEvent().addListener(slaveVolumeEventListener);
Line 232: 
> Pls move the above code to model
Done
Line 233:     }
Line 234: 
Line 235:     @Override
Line 236:     public void setFailureMessage(String failureMessage) {


-- 
To view, visit https://gerrit.ovirt.org/29691
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied7317d456bf66db9a7800ba7106f2e8ec66429c
Gerrit-PatchSet: 39
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: anmolbabu <anb...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Ramesh N <rnach...@redhat.com>
Gerrit-Reviewer: Sahina Bose <sab...@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

Reply via email to