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

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


Repository: cloudstack-git


Description
-------

Note: JS files were formatted with same formatting that was applied to 
origin/master.

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.
I'm not sure how to detail with cases where the user might not have  firstname, 
lastname or email set in ldap. 
I'm not sure what happens if listAllLdapUsers returns a massive list of 
users... will it load on scroll? Does paging need to be implemented API side? 
How is this done?

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.basedn = dc=cloudstack,dc=org
Also global settings -> LDAP Configuration, hostname: localhost, port: 10389.


Diffs
-----

  client/tomcatconf/commands.properties.in 9e14d0f 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapAddConfigurationCmd.java
 62736b16087561a7e25893cd46115795100c609e 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccount.java
 PRE-CREATION 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapDeleteConfigurationCmd.java
 329b91b 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapListAllUsersCmd.java
 087d156 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapListConfigurationCmd.java
 6707878 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapUserSearchCmd.java
 e6a40d0 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LdapConfigurationResponse.java
 d583346 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LdapUserResponse.java
 40ba0ce 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java
 2916202 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java
 8f31ce5 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfigurationVO.java
 d3ff820 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java
 30bdc5b 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManager.java
 c961d2c 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
 be9b3d5 
  plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUser.java 
7c65e60 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManager.java
 54802cf 
  
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUtils.java 
453dc0a 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAddConfigurationCmdSpec.groovy
 ebade1e 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapConfigurationSpec.groovy
 91c9baf 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapConfigurationVO.groovy
 8135901 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapContextFactorySpec.groovy
 bb20fb3 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapDeleteConfigurationCmdSpec.groovy
 664fd64 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapListAllUsersCmdSpec.groovy
 30cd7cc 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapListConfigurationCmdSpec.groovy
 a7c1979 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy
 5dfecb9 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapSearchUserCmdSpec.groovy
 d72878b 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerSpec.groovy
 489c250 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserResponseSpec.groovy
 105203b 
  
plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserSpec.groovy
 6131267 
  ui/css/cloudstack3.css 4545e96 
  ui/index.jsp 34f0c54 
  ui/scripts/accounts.js bad8435 
  ui/scripts/accountsWizard.js PRE-CREATION 
  ui/scripts/affinity.js a9c6695 
  ui/scripts/autoscaler.js 15a9dac 
  ui/scripts/cloud.core.callbacks.js 6eb7644 
  ui/scripts/cloudStack.js c0ff7f2 
  ui/scripts/configuration.js 8bc40d6 
  ui/scripts/dashboard.js e8ab6c5 
  ui/scripts/docs.js c8ef0d9 
  ui/scripts/domains.js 01f4236 
  ui/scripts/events.js bd50887 
  ui/scripts/globalSettings.js ac63015 
  ui/scripts/installWizard.js 46769fa 
  ui/scripts/instanceWizard.js ff130d3 
  ui/scripts/instances.js 9b27d93 
  ui/scripts/lbStickyPolicy.js c0e2bfa 
  ui/scripts/network.js 95a93bc 
  ui/scripts/plugins.js 3c5bc0f 
  ui/scripts/projects.js ea1e6db 
  ui/scripts/regions.js 4be600f 
  ui/scripts/sharedFunctions.js a9f833c 
  ui/scripts/storage.js ad0965a 
  ui/scripts/system.js 3038a8a 
  ui/scripts/templates.js dbb0083 
  ui/scripts/ui-custom/accountsWizard.js PRE-CREATION 
  ui/scripts/ui-custom/affinity.js 1a23ff7 
  ui/scripts/ui-custom/autoscaler.js 2f6ce38 
  ui/scripts/ui-custom/dashboard.js 6d92318 
  ui/scripts/ui-custom/enableStaticNAT.js 1b2bf7b 
  ui/scripts/ui-custom/granularSettings.js 02d5c1f 
  ui/scripts/ui-custom/healthCheck.js 4b42fa7 
  ui/scripts/ui-custom/installWizard.js c53a642 
  ui/scripts/ui-custom/instanceWizard.js 31b4baa 
  ui/scripts/ui-custom/ipRules.js 34b2398 
  ui/scripts/ui-custom/login.js 0dbbf82 
  ui/scripts/ui-custom/physicalResources.js 5173172 
  ui/scripts/ui-custom/pluginListing.js 3dcce98 
  ui/scripts/ui-custom/projectSelect.js aef49ed 
  ui/scripts/ui-custom/projects.js f1f9eba 
  ui/scripts/ui-custom/recurringSnapshots.js 985f369 
  ui/scripts/ui-custom/regions.js 9fc36f3 
  ui/scripts/ui-custom/securityRules.js 2e2c9ac 
  ui/scripts/ui-custom/uploadVolume.js 996d8ac 
  ui/scripts/ui-custom/vpc.js 4edccf1 
  ui/scripts/ui-custom/zoneChart.js 5d4e0c0 
  ui/scripts/ui-custom/zoneFilter.js 9e6a493 
  ui/scripts/ui-custom/zoneWizard.js 877dbc0 
  ui/scripts/ui/core.js 18c3363 
  ui/scripts/ui/dialog.js 7f82eea 
  ui/scripts/ui/events.js bd609d2 
  ui/scripts/ui/utils.js 39ef3e3 
  ui/scripts/ui/widgets/cloudBrowser.js 9a56bb3 
  ui/scripts/ui/widgets/dataTable.js 1b3ea82 
  ui/scripts/ui/widgets/detailView.js 0bccef5 
  ui/scripts/ui/widgets/listView.js bc68a72 
  ui/scripts/ui/widgets/multiEdit.js 08bd0bf 
  ui/scripts/ui/widgets/notifications.js 0299603 
  ui/scripts/ui/widgets/overlay.js ecf12e6 
  ui/scripts/ui/widgets/tagger.js 9af6fb7 
  ui/scripts/ui/widgets/toolTip.js 6967acc 
  ui/scripts/ui/widgets/treeView.js fa1ceb6 
  ui/scripts/vm_snapshots.js c50c7e1 
  ui/scripts/vpc.js e90d8a7 
  ui/scripts/zoneWizard.js 04687fe 

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


Testing
-------

Compiled... unit tests passed, integration tests passed.
Viewed in browser, got expected results.


Thanks,

Ian Duffy

Reply via email to