anmolbabu has posted comments on this change.

Change subject: webadmin : Create Geo-rep pop up
......................................................................


Patch Set 33:

(14 comments)

http://gerrit.ovirt.org/#/c/29691/33/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeGeoRepCreateModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeGeoRepCreateModel.java:

Line 113:                     
setQueryFailureMessage(vdcReturnValue.getExceptionString());
Line 114:                     onPropertyChanged(new 
PropertyChangedEventArgs("queryFailed"));//$NON-NLS-1$
Line 115:                 } else {
Line 116:                     List<GlusterVolumeEntity> eligibleVolumesList = 
(List<GlusterVolumeEntity>) vdcReturnValue.getReturnValue();
Line 117:                     recommendationViolations = null;
> use the setter
Done
Line 118:                     setVolumeList(eligibleVolumesList);
Line 119:                     
getSlaveClusters().setItems(GlusterVolumeGeoRepCreateModel.this.getClusterForVolumes(eligibleVolumesList));
Line 120:                 }
Line 121:             }


Line 130:         getStartSession().setIsAvailable(true);
Line 131:         getSlaveClusters().setIsAvailable(true);
Line 132:         getSlaveVolumes().setIsAvailable(true);
Line 133:         getSlaveHosts().setIsAvailable(true);
Line 134:         getShowEligibleVolumes().setIsAvailable(true);
> Is the visibility modified anywhere else?
Done
Line 135:     }
Line 136: 
Line 137:     public ListModel<String> getSlaveClusters() {
Line 138:         return slaveClusters;


Line 171:         setSlaveUserName(new EntityModel<String>());
Line 172:         setSlaveHostRootPassword(new EntityModel<String>());
Line 173: 
Line 174:         getShowEligibleVolumes().setEntity(false);
Line 175:         getSlaveUserName().setIsChangable(true);
> 'true' is the default value, no need to set it explicitly
Done
Line 176:         getSlaveUserName().setIsAvailable(true);
Line 177:         getSlaveUserName().setEntity("root");//$NON-NLS-1$
Line 178:     }
Line 179: 


Line 172:         setSlaveHostRootPassword(new EntityModel<String>());
Line 173: 
Line 174:         getShowEligibleVolumes().setEntity(false);
Line 175:         getSlaveUserName().setIsChangable(true);
Line 176:         getSlaveUserName().setIsAvailable(true);
> same here
Done
Line 177:         getSlaveUserName().setEntity("root");//$NON-NLS-1$
Line 178:     }
Line 179: 
Line 180:     public boolean validate() {


Line 237:         this.recommendationViolations = recommendationViolations;
Line 238:         onPropertyChanged(new 
PropertyChangedEventArgs("RecommendationViolations"));//$NON-NLS-1$
Line 239:     }
Line 240: 
Line 241:     public void fetchRecommendatonViolations() {
> updateRecommendatonViolations
Done
Line 242:         startProgress(constants.fetchingDataMessage());
Line 243:         AsyncQuery aQuery = new AsyncQuery(new INewAsyncCallback() {
Line 244:             @Override
Line 245:             public void onSuccess(Object model, Object returnValue) {


http://gerrit.ovirt.org/#/c/29691/33/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 157:         boolean allowNewSessionCommand = true;
Line 158:         if(volumeEntity == null) {
Line 159:             allowNewSessionCommand = false;
Line 160:         }
Line 161:         if(getEntity().getStatus() != GlusterStatus.UP) {
> volumeEntity.getStatus()
Done
Line 162:             allowNewSessionCommand = false;
Line 163:         }
Line 164:         getNewSessionCommand().setIsAvailable(allowNewSessionCommand);
Line 165:         getRemoveSessionCommand().setIsAvailable(false);


Line 160:         }
Line 161:         if(getEntity().getStatus() != GlusterStatus.UP) {
Line 162:             allowNewSessionCommand = false;
Line 163:         }
Line 164:         getNewSessionCommand().setIsAvailable(allowNewSessionCommand);
> This will hide the action itself. I think you just need to disable it inste
Done
Line 165:         getRemoveSessionCommand().setIsAvailable(false);
Line 166:         getStartSessionCommand().setIsAvailable(false);
Line 167:         getStopSessionCommand().setIsAvailable(false);
Line 168:         getSessionOptionsCommand().setIsAvailable(false);


http://gerrit.ovirt.org/#/c/29691/33/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 227:         setStartVolumeProfilingCommand(new 
UICommand("startProfiling", this));//$NON-NLS-1$
Line 228:         setShowVolumeProfileDetailsCommand(new 
UICommand("showProfileDetails", this));//$NON-NLS-1$
Line 229:         setStopVolumeProfilingCommand(new UICommand("stopProfiling", 
this));//$NON-NLS-1$
Line 230:         setOptimizeForVirtStoreCommand(new 
UICommand("OptimizeForVirtStore", this)); //$NON-NLS-1$
Line 231:         setNewGeoRepSessionCommand(new 
UICommand("createGeoREpSession", this));//$NON-NLS-1$
> CreateGeoRepSession
Done
Line 232: 
Line 233:         getRemoveVolumeCommand().setIsExecutionAllowed(false);
Line 234:         getStartCommand().setIsExecutionAllowed(false);
Line 235:         getStopCommand().setIsExecutionAllowed(false);


Line 506:         
getStartRebalanceCommand().setIsExecutionAllowed(allowStartRebalance);
Line 507:         
getStopRebalanceCommand().setIsExecutionAllowed(allowStopRebalance);
Line 508:         
getStatusRebalanceCommand().setIsExecutionAllowed(allowStatusRebalance);
Line 509:         
getOptimizeForVirtStoreCommand().setIsExecutionAllowed(allowOptimize);
Line 510:         
getNewGeoRepSessionCommand().setIsAvailable(allowCreateGeoRepSession);
> you might just need to disable instead of hiding
Done
Line 511: 
Line 512:         // System tree dependent actions.
Line 513:         boolean isAvailable =
Line 514:                 !(getSystemTreeSelectedItem() != null && 
getSystemTreeSelectedItem().getType() == SystemTreeItemType.Volume);


http://gerrit.ovirt.org/#/c/29691/33/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/popup/gluster/GlusterVolumeGeoRepCreateSessionPopupPresenterWidget.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/popup/gluster/GlusterVolumeGeoRepCreateSessionPopupPresenterWidget.java:

Line 37:             @Override
Line 38:             public void eventRaised(Event<? extends 
PropertyChangedEventArgs> ev, Object sender, PropertyChangedEventArgs args) {
Line 39:                 
if(args.propertyName.equalsIgnoreCase("RecommendationViolations")) 
{//$NON-NLS-1$
Line 40:                     
getView().setSuggestedConfigViolations(model.getRecommendationViolations());
Line 41:                 } else 
if(args.propertyName.equalsIgnoreCase("queryFailed")) {//$NON-NLS-1$
> change the event name to "QueryFailed"
Done
Line 42:                     
getView().setQueryFailureMessage(model.getQueryFailureMessage());
Line 43:                 }
Line 44:             }
Line 45:         });


Line 43:                 }
Line 44:             }
Line 45:         });
Line 46: 
Line 47:         
model.getShowEligibleVolumes().getEntityChangedEvent().addListener(new 
IEventListener<EventArgs>() {
> This is not interacting with the view, so it should be done in CreateModel 
Done
Line 48:             @Override
Line 49:             public void eventRaised(Event<? extends EventArgs> ev, 
Object sender, EventArgs args) {
Line 50:                 boolean overrideSuggestedConfig = 
model.getShowEligibleVolumes().getEntity();
Line 51:                 if(!overrideSuggestedConfig) {


Line 70:                     
model.getSlaveVolumes().setItems(volumesInCurrentCluster);
Line 71:                 }
Line 72:             }
Line 73:         };
Line 74:         
model.getSlaveClusters().getSelectedItemChangedEvent().addListener(clusterEventListener);
> same goes here
Done
Line 75: 
Line 76:         IEventListener<EventArgs> slaveVolumeEventListener = new 
IEventListener<EventArgs>() {
Line 77:             @Override
Line 78:             public void eventRaised(Event<? extends EventArgs> ev, 
Object sender, EventArgs args) {


Line 87:                 model.getSlaveHosts().setItems(hostsInCurrentVolume);
Line 88:             }
Line 89:         };
Line 90:         
model.getSlaveVolumes().getSelectedItemChangedEvent().addListener(slaveVolumeEventListener);
Line 91: 
> same here.
Done
Line 92:     }


http://gerrit.ovirt.org/#/c/29691/33/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 174:     @Override
Line 175:     public void edit(final GlusterVolumeGeoRepCreateModel object) {
Line 176:         driver.edit(object);
Line 177:         
setSuggestedConfigViolations(object.getRecommendationViolations());
Line 178:         setQueryFailureMessage(object.getQueryFailureMessage());
> This is not required, already take care presenter
Done
Line 179:     }
Line 180: 
Line 181:     @Override
Line 182:     public void setQueryFailureMessage(String failureMessage) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied7317d456bf66db9a7800ba7106f2e8ec66429c
Gerrit-PatchSet: 33
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: anmolbabu <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Sahina Bose <[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