Kanagaraj M has posted comments on this change.

Change subject: webadmin: gluster volume rebalance status popup
......................................................................


Patch Set 30:

(6 comments)

....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/volumes/VolumeListModel.java
Line 550:     }
Line 551: 
Line 552:     UICommand cancelRebalance;
Line 553: 
Line 554:     UICommand rebalanceStatusOk;
Move this fields to method as local fields
Line 555: 
Line 556:     private void getRebalanceDetails(final GlusterVolumeEntity 
volumeEntity) {
Line 557:         // TODO Auto-generated method stub
Line 558:         final ConfirmationModel cModel = new ConfirmationModel();


Line 552:     UICommand cancelRebalance;
Line 553: 
Line 554:     UICommand rebalanceStatusOk;
Line 555: 
Line 556:     private void getRebalanceDetails(final GlusterVolumeEntity 
volumeEntity) {
Why do you need a new method here?

If required, please change the name of the method. Also remove TODO
Line 557:         // TODO Auto-generated method stub
Line 558:         final ConfirmationModel cModel = new ConfirmationModel();
Line 559:         setConfirmWindow(cModel);
Line 560:         
cModel.setTitle(ConstantsManager.getInstance().getConstants().rebalanceStatusTitle());


Line 557:         // TODO Auto-generated method stub
Line 558:         final ConfirmationModel cModel = new ConfirmationModel();
Line 559:         setConfirmWindow(cModel);
Line 560:         
cModel.setTitle(ConstantsManager.getInstance().getConstants().rebalanceStatusTitle());
Line 561:         cModel.startProgress("Fetching Data");//$NON-NLS-1$
This text should be moved to Constants
Line 562:         AsyncDataProvider.getGlusterRebalanceStatus(new 
AsyncQuery(this, new INewAsyncCallback() {
Line 563:             @Override
Line 564:             public void onSuccess(Object model, Object returnValue) {
Line 565:                 GlusterVolumeTaskStatusEntity rebalanceStatusEntity =


Line 563:             @Override
Line 564:             public void onSuccess(Object model, Object returnValue) {
Line 565:                 GlusterVolumeTaskStatusEntity rebalanceStatusEntity =
Line 566:                         (GlusterVolumeTaskStatusEntity) returnValue;
Line 567:                 if 
((rebalanceStatusEntity.getStatusSummary().getStatus() == 
JobExecutionStatus.UNKNOWN) || rebalanceStatusEntity.equals(null)) {
There is chance of NPE here

 rebalanceStatusEntity == null || 
rebalanceStatusEntity.getStatusSummary().getStatus() == JobExecutionStatus.UNKN
OWN
Line 568:                     cModel.stopProgress();
Line 569:                     cModel.setMessage(ConstantsManager.getInstance()
Line 570:                             .getConstants()
Line 571:                             .rebalanceNotStartedMessage() + 
volumeEntity.getName());


Line 567:                 if 
((rebalanceStatusEntity.getStatusSummary().getStatus() == 
JobExecutionStatus.UNKNOWN) || rebalanceStatusEntity.equals(null)) {
Line 568:                     cModel.stopProgress();
Line 569:                     cModel.setMessage(ConstantsManager.getInstance()
Line 570:                             .getConstants()
Line 571:                             .rebalanceNotStartedMessage() + 
volumeEntity.getName());
Use Messages instead of appending volume name
Line 572: 
Line 573:                     rebalanceStatusOk = new 
UICommand("rebalanceNotStarted", VolumeListModel.this);//$NON-NLS-1$
Line 574:                     
rebalanceStatusOk.setTitle(ConstantsManager.getInstance().getConstants().ok());
Line 575:                     rebalanceStatusOk.setIsDefault(true);


....................................................
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/VolumeRebalanceStatusPopupView.java
Line 192:             @Override
Line 193:             public void eventRaised(Event ev, Object sender, 
EventArgs args) {
Line 194:                 // TODO Auto-generated method stub
Line 195:                 PropertyChangedEventArgs e = 
(PropertyChangedEventArgs) args;
Line 196:                 if(e.PropertyName == "IS_STATUS_APPLICABLE") 
{//$NON-NLS-1$
It should be e.PropertyName.equals(...)

Also statusLabel.setVisible(object.getStatusAvailable());
Line 197:                     statusLabel.setVisible(true);
Line 198:                 }else {
Line 199:                     statusLabel.setVisible(false);
Line 200:                 }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d1f63b21dfbed88fa2b4e8c077e3e5e54ba7b09
Gerrit-PatchSet: 30
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com>
Gerrit-Reviewer: anmolbabu <anb...@redhat.com>
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