----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12907/ -----------------------------------------------------------
(Updated July 24, 2013, 5:52 p.m.) Review request for cloudstack, Abhinandan Prateek, Brian Federle, Jessica Wang, Pranav Saxena, and Sebastien Goasguen. Repository: cloudstack-git Description (updated) ------- 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? I'm not sure how to get the newly created user to pop up in the table after the add account screen is closed. I'm not sure how to notify the administrator that their user has successfully created or failed to create via the little pop up box. 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
