You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Ian Duffy <ia...@ianduffy.ie> on 2013/07/22 18:58:47 UTC

Review Request 12819: WIP: Add LDAP Account addition UI. Feedback and Help wanted.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12819/
-----------------------------------------------------------

Review request for cloudstack, Abhinandan Prateek, Brian Federle, Jessica Wang, Pranav Saxena, and Sebastien Goasguen.


Repository: cloudstack-git


Description
-------

This is purely a work in progress. I'm submitting it as I'd like somebody to give *detailed* feedback/reviewal before I go much further.

The code works/does-what-I-want but I'm not sure if its "correct" and follows cloudstack UI practices.
I had to do some ugly css in order to get the display I wanted.
I'm currently having issues with fields marked as "required" for some reason the requirement doesn't seem to be enforced.
I'm not sure if a loader appears should the request to listAllLdapUsers be slow to respond. 
I'm not sure how to add a if ldapEnabled display this view else display old view condition.

For testing purposes there is a ldap server included in this branch. You can launch it with:
mvn -pl :cloud-plugin-user-authenticator-ldap ldap:run

and then configure it at Global Settings -> LDAP Configuration, hostname: localhost, port: 10389.


Diffs
-----

  ui/css/cloudstack3.css 4545e96 
  ui/index.jsp 34f0c54 
  ui/scripts/accounts.js e5faf33 
  ui/scripts/accountsWizard.js PRE-CREATION 
  ui/scripts/ui-custom/accountsWizard.js PRE-CREATION 

Diff: https://reviews.apache.org/r/12819/diff/


Testing
-------

Complied... view in browser, got expected results.


Thanks,

Ian Duffy


Re: Review Request 12819: WIP: Add LDAP Account addition UI. Feedback and Help wanted.

Posted by "Jenkins Cloudstack.org" <hu...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12819/#review23679
-----------------------------------------------------------


Review 12819 PASSED the build test
The url of build cloudstack-master-with-patch #18 is : http://jenkins.cloudstack.org/job/cloudstack-master-with-patch/18/

- Jenkins Cloudstack.org


On July 22, 2013, 5 p.m., Ian Duffy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12819/
> -----------------------------------------------------------
> 
> (Updated July 22, 2013, 5 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Brian Federle, Jessica Wang, Pranav Saxena, and Sebastien Goasguen.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> This is purely a work in progress. I'm submitting it as I'd like somebody to give *detailed* feedback/reviewal before I go much further.
> 
> The code works/does-what-I-want but I'm not sure if its "correct" and follows cloudstack UI practices.
> I had to do some ugly css in order to get the display I wanted.
> I'm currently having issues with fields marked as "required" for some reason the requirement doesn't seem to be enforced.
> I'm not sure if a loader appears should the request to listAllLdapUsers be slow to respond. 
> I'm not sure how to add a if ldapEnabled display this view else display old view condition.
> 
> For testing purposes there is a ldap server included in this branch. You can launch it with:
> mvn -pl :cloud-plugin-user-authenticator-ldap ldap:run
> 
> and then configure it at Global Settings -> LDAP Configuration, hostname: localhost, port: 10389.
> 
> 
> Diffs
> -----
> 
>   ui/css/cloudstack3.css 4545e96 
>   ui/index.jsp 34f0c54 
>   ui/scripts/accounts.js e5faf33 
>   ui/scripts/accountsWizard.js PRE-CREATION 
>   ui/scripts/ui-custom/accountsWizard.js PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/12819/diff/
> 
> 
> Testing
> -------
> 
> Complied... view in browser, got expected results.
> 
> 
> File Attachments
> ----------------
> 
> Current_Output
>   https://reviews.apache.org/media/uploaded/files/2013/07/22/Capture.PNG
> 
> 
> Thanks,
> 
> Ian Duffy
> 
>


Re: Review Request 12819: WIP: Add LDAP Account addition UI. Feedback and Help wanted.

Posted by Ian Duffy <ia...@ianduffy.ie>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12819/
-----------------------------------------------------------

(Updated July 22, 2013, 5 p.m.)


Review request for cloudstack, Abhinandan Prateek, Brian Federle, Jessica Wang, Pranav Saxena, and Sebastien Goasguen.


Repository: cloudstack-git


Description
-------

This is purely a work in progress. I'm submitting it as I'd like somebody to give *detailed* feedback/reviewal before I go much further.

The code works/does-what-I-want but I'm not sure if its "correct" and follows cloudstack UI practices.
I had to do some ugly css in order to get the display I wanted.
I'm currently having issues with fields marked as "required" for some reason the requirement doesn't seem to be enforced.
I'm not sure if a loader appears should the request to listAllLdapUsers be slow to respond. 
I'm not sure how to add a if ldapEnabled display this view else display old view condition.

For testing purposes there is a ldap server included in this branch. You can launch it with:
mvn -pl :cloud-plugin-user-authenticator-ldap ldap:run

and then configure it at Global Settings -> LDAP Configuration, hostname: localhost, port: 10389.


Diffs
-----

  ui/css/cloudstack3.css 4545e96 
  ui/index.jsp 34f0c54 
  ui/scripts/accounts.js e5faf33 
  ui/scripts/accountsWizard.js PRE-CREATION 
  ui/scripts/ui-custom/accountsWizard.js PRE-CREATION 

Diff: https://reviews.apache.org/r/12819/diff/


Testing
-------

Complied... view in browser, got expected results.


File Attachments
----------------

Current_Output
  https://reviews.apache.org/media/uploaded/files/2013/07/22/Capture.PNG


Thanks,

Ian Duffy


Re: Review Request 12819: WIP: Add LDAP Account addition UI. Feedback and Help wanted.

Posted by Ian Duffy <ia...@ianduffy.ie>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12819/
-----------------------------------------------------------

(Updated July 22, 2013, 4:59 p.m.)


Review request for cloudstack, Abhinandan Prateek, Brian Federle, Jessica Wang, Pranav Saxena, and Sebastien Goasguen.


Repository: cloudstack-git


Description
-------

This is purely a work in progress. I'm submitting it as I'd like somebody to give *detailed* feedback/reviewal before I go much further.

The code works/does-what-I-want but I'm not sure if its "correct" and follows cloudstack UI practices.
I had to do some ugly css in order to get the display I wanted.
I'm currently having issues with fields marked as "required" for some reason the requirement doesn't seem to be enforced.
I'm not sure if a loader appears should the request to listAllLdapUsers be slow to respond. 
I'm not sure how to add a if ldapEnabled display this view else display old view condition.

For testing purposes there is a ldap server included in this branch. You can launch it with:
mvn -pl :cloud-plugin-user-authenticator-ldap ldap:run

and then configure it at Global Settings -> LDAP Configuration, hostname: localhost, port: 10389.


Diffs
-----

  ui/css/cloudstack3.css 4545e96 
  ui/index.jsp 34f0c54 
  ui/scripts/accounts.js e5faf33 
  ui/scripts/accountsWizard.js PRE-CREATION 
  ui/scripts/ui-custom/accountsWizard.js PRE-CREATION 

Diff: https://reviews.apache.org/r/12819/diff/


Testing
-------

Complied... view in browser, got expected results.


File Attachments (updated)
----------------

Current_Output
  https://reviews.apache.org/media/uploaded/files/2013/07/22/Capture.PNG


Thanks,

Ian Duffy