Martin Betak has posted comments on this change.

Change subject: userportal, webadmin: VM console key management
......................................................................


Patch Set 4:

(7 comments)

Please consider adding a @Produces method for UserProfileDao to BllCDIAdapter. 
This will enable you to

  @Inject
  private UserProfileDao userProfileDao;

in Commands/Queries that want to utilize it (GetSshPublicKeyQuery and 
SetSshPublicKeyCommand in tihs case) - instead of getting it via DbFacade or 
adding getUserProfileDao() method in AuditLogableBase.

https://gerrit.ovirt.org/#/c/35810/4//COMMIT_MSG
Commit Message:

Line 16: public key assigned to the user in the database
Line 17: 
Line 18: - A new command to implement the functionality of this button was
Line 19: implemented
Line 20: 
is there a bug/wiki-page related to this feature?
Line 21: Change-Id: Id2364cafc687ba6dee2504322234067ff98dc00c


https://gerrit.ovirt.org/#/c/35810/4/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java:

Line 225:     {
Line 226:         privateNewTemplateCommand = value;
Line 227:     }
Line 228: 
Line 229:     private UICommand privateSetConsoleKeyCommand;
please don't use the 'private' prefix in new code
Line 230: 
Line 231:     public UICommand getSetConsoleKeyCommand() {
Line 232:         return privateSetConsoleKeyCommand;
Line 233:     }


Line 540:         model.getCommands().add(tempVar);
Line 541:         UICommand tempVar2 = new UICommand("Cancel", this); 
//$NON-NLS-1$
Line 542:         
tempVar2.setTitle(ConstantsManager.getInstance().getConstants().cancel());
Line 543:         tempVar2.setIsCancel(true);
Line 544:         model.getCommands().add(tempVar2);
you can simplify this to:

  model.getCommands().add(UICommand.createDefaultOkUiCommand("OnSetConsoleKey", 
this)); //$NON-NLS-1$       

  model.getCommands().add(UICommand.createCancelUiCommand("Cancel", this)); 
//$NON-NLS-1$
Line 545: 
Line 546:         AsyncDataProvider.getInstance().getConsolePublicKey(new 
AsyncQuery(model,
Line 547:                 new INewAsyncCallback() {
Line 548:                     @Override


Line 546:         AsyncDataProvider.getInstance().getConsolePublicKey(new 
AsyncQuery(model,
Line 547:                 new INewAsyncCallback() {
Line 548:                     @Override
Line 549:                     public void onSuccess(Object target, Object 
returnValue) {
Line 550:                         PublicKeyModel model = (PublicKeyModel)target;
please avoid using the 'target' facility from AsyncQuery. It is a remnant from 
C# code. In java you can directly access the outer model just by making it 
final (on line 529). 

It is always better (when possible) to access such variables directly rather 
than passing it via AsyncQuery and then using casts to obtain it.
Line 551: 
Line 552:                         Object publicKey = ((VdcQueryReturnValue) 
returnValue).getReturnValue();
Line 553: 
Line 554:                         
model.getTextInput().setEntity((String)publicKey);


Line 548:                     @Override
Line 549:                     public void onSuccess(Object target, Object 
returnValue) {
Line 550:                         PublicKeyModel model = (PublicKeyModel)target;
Line 551: 
Line 552:                         Object publicKey = ((VdcQueryReturnValue) 
returnValue).getReturnValue();
since VdcQueryReturnValue#getReturnValue() is generic, you can make publicKey 
of type String and avoid the cast on line 554.
Line 553: 
Line 554:                         
model.getTextInput().setEntity((String)publicKey);
Line 555:                     }
Line 556:                 }));


Line 568:                 new IFrontendActionAsyncCallback() {
Line 569:                     @Override
Line 570:                     public void executed(FrontendActionAsyncResult 
result) {
Line 571: 
Line 572:                         PublicKeyModel model = (PublicKeyModel) 
result.getState();
similarly: by making model final on line 561, you can capture the model and use 
it directly in this callback.
Line 573:                         model.stopProgress();
Line 574:                         cancel();
Line 575:                     }
Line 576:                 }, model);


https://gerrit.ovirt.org/#/c/35810/4/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java:

Line 1818:                         cancel();
Line 1819:                     }
Line 1820:                 }, model);
Line 1821:     }
Line 1822: 
same set of comments for #editConsoleKey and #onSetConsoleKey as in 
UserPortalListModel :-)
Line 1823:     private void editConsoleKey() {
Line 1824: 
Line 1825:         PublicKeyModel model = new PublicKeyModel();
Line 1826: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2364cafc687ba6dee2504322234067ff98dc00c
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vitor de Lima <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Martin Betak <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Shahar Havivi <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[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