Repository: zeppelin Updated Branches: refs/heads/master 1b9c46dcf -> ca40e49aa
[ZEPPELIN-2778] Unit test for credential module (zeppelin-web) ### What is this PR for? Added few test cases for the credential module under `zeppelin-web/` Additionally, - modified `configuration.html` to add the missing `tbody`. - removed useless lodash dependency - extract some funcs to reuse them. ### What type of PR is it? [Improvement] ### What is the Jira issue? [ZEPPELIN-2778](https://issues.apache.org/jira/browse/ZEPPELIN-2778) ### How should this be tested? 1. cd `zeppelin-web` 2. `yarn install` (or `npm install`) 3. `yarn run test` (or `npm run test`) The test should pass. ### Questions: * Does the licenses files need update? - NO * Is there breaking changes for older versions? - NO * Does this needs documentation? - NO Author: 1ambda <1am...@gmail.com> Closes #2493 from 1ambda/ZEPPELIN-2778/add-unit-test-for-credential-page and squashes the following commits: a0dad5d5 [1ambda] fix: eslint f8bb092c [1ambda] fix: Add verify* to the end of api test 46294be6 [1ambda] test: Add credential.test.js b4111f89 [1ambda] fix: Add missing tbody in credential.html e857047d [1ambda] style: Re-indent 0c5a2202 [1ambda] refactor: Add showToast 644e49af [1ambda] fix: Add guard to update method 904e0513 [1ambda] refactor: Extract isValidCredential cce91f98 [1ambda] fix: Remove unused var 818d0970 [1ambda] fix: Remove lodash dependency Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/ca40e49a Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/ca40e49a Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/ca40e49a Branch: refs/heads/master Commit: ca40e49aa22048b4011a1198e4aa0fe6cfb1dad8 Parents: 1b9c46d Author: 1ambda <1am...@gmail.com> Authored: Sat Jul 15 01:27:15 2017 +0900 Committer: 1ambda <1am...@gmail.com> Committed: Sun Jul 23 12:44:53 2017 +0900 ---------------------------------------------------------------------- .../src/app/credential/credential.controller.js | 164 ++++++++++--------- zeppelin-web/src/app/credential/credential.html | 96 +++++------ .../src/app/credential/credential.test.js | 114 +++++++++++++ 3 files changed, 253 insertions(+), 121 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ca40e49a/zeppelin-web/src/app/credential/credential.controller.js ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/credential/credential.controller.js b/zeppelin-web/src/app/credential/credential.controller.js index 89cb1cf..102876e 100644 --- a/zeppelin-web/src/app/credential/credential.controller.js +++ b/zeppelin-web/src/app/credential/credential.controller.js @@ -12,49 +12,60 @@ * limitations under the License. */ -angular.module('zeppelinWebApp').controller('CredentialCtrl', CredentialCtrl) +angular.module('zeppelinWebApp').controller('CredentialCtrl', CredentialController) -function CredentialCtrl ($scope, $rootScope, $http, baseUrlSrv, ngToast) { +function CredentialController($scope, $http, baseUrlSrv, ngToast) { 'ngInject' - $scope._ = _ ngToast.dismiss() $scope.credentialInfo = [] $scope.showAddNewCredentialInfo = false $scope.availableInterpreters = [] + $scope.entity = '' + $scope.password = '' + $scope.username = '' + + $scope.hasCredential = () => { + return Array.isArray($scope.credentialInfo) && $scope.credentialInfo.length + } + let getCredentialInfo = function () { $http.get(baseUrlSrv.getRestApiBase() + '/credential') - .success(function (data, status, headers, config) { - $scope.credentialInfo = _.map(data.body.userCredentials, function (value, prop) { - return {entity: prop, password: value.password, username: value.username} + .success(function (data, status, headers, config) { + $scope.credentialInfo.length = 0 // keep the ref while cleaning + const returnedCredentials = data.body.userCredentials + + for (let key in returnedCredentials) { + const value = returnedCredentials[key] + $scope.credentialInfo.push({ + entity: key, + password: value.password, + username: value.username, + }) + } + + console.log('Success %o %o', status, $scope.credentialInfo) }) - console.log('Success %o %o', status, $scope.credentialInfo) - }) - .error(function (data, status, headers, config) { - if (status === 401) { - ngToast.danger({ - content: 'You don\'t have permission on this page', - verticalPosition: 'bottom', - timeout: '3000' - }) - setTimeout(function () { - window.location = baseUrlSrv.getBase() - }, 3000) - } - console.log('Error %o %o', status, data.message) - }) + .error(function (data, status, headers, config) { + if (status === 401) { + showToast('You do not have permission on this page', 'danger') + setTimeout(function () { + window.location = baseUrlSrv.getBase() + }, 3000) + } + console.log('Error %o %o', status, data.message) + }) + } + + $scope.isValidCredential = function() { + return $scope.entity.trim() !== '' && $scope.username.trim() !== '' } $scope.addNewCredentialInfo = function () { - if ($scope.entity && _.isEmpty($scope.entity.trim()) && - $scope.username && _.isEmpty($scope.username.trim())) { - ngToast.danger({ - content: 'Username \\ Entity can not be empty.', - verticalPosition: 'bottom', - timeout: '3000' - }) + if (!$scope.isValidCredential()) { + showToast('Username \\ Entity can not be empty.', 'danger') return } @@ -65,25 +76,17 @@ function CredentialCtrl ($scope, $rootScope, $http, baseUrlSrv, ngToast) { } $http.put(baseUrlSrv.getRestApiBase() + '/credential', newCredential) - .success(function (data, status, headers, config) { - ngToast.success({ - content: 'Successfully saved credentials.', - verticalPosition: 'bottom', - timeout: '3000' + .success(function (data, status, headers, config) { + showToast('Successfully saved credentials.', 'success') + $scope.credentialInfo.push(newCredential) + resetCredentialInfo() + $scope.showAddNewCredentialInfo = false + console.log('Success %o %o', status, data.message) }) - $scope.credentialInfo.push(newCredential) - resetCredentialInfo() - $scope.showAddNewCredentialInfo = false - console.log('Success %o %o', status, data.message) - }) - .error(function (data, status, headers, config) { - ngToast.danger({ - content: 'Error saving credentials', - verticalPosition: 'bottom', - timeout: '3000' + .error(function (data, status, headers, config) { + showToast('Error saving credentials', 'danger') + console.log('Error %o %o', status, data.message) }) - console.log('Error %o %o', status, data.message) - }) } let getAvailableInterpreters = function () { @@ -100,7 +103,9 @@ function CredentialCtrl ($scope, $rootScope, $http, baseUrlSrv, ngToast) { return false } }) - }).error(function (data, status, headers, config) { + }) + .error(function (data, status, headers, config) { + showToast(data.message, 'danger') console.log('Error %o %o', status, data.message) }) } @@ -125,35 +130,32 @@ function CredentialCtrl ($scope, $rootScope, $http, baseUrlSrv, ngToast) { } $scope.copyOriginCredentialsInfo = function () { - ngToast.info({ - content: 'Since entity is a unique key, you can edit only username & password', - verticalPosition: 'bottom', - timeout: '3000' - }) + showToast('Since entity is a unique key, you can edit only username & password', 'info') } $scope.updateCredentialInfo = function (form, data, entity) { - let request = { + if (!$scope.isValidCredential()) { + showToast('Username \\ Entity can not be empty.', 'danger') + return + } + + let credential = { entity: entity, username: data.username, password: data.password } - $http.put(baseUrlSrv.getRestApiBase() + '/credential/', request) - .success(function (data, status, headers, config) { - let index = _.findIndex($scope.credentialInfo, {'entity': entity}) - $scope.credentialInfo[index] = request - return true - }) - .error(function (data, status, headers, config) { - console.log('Error %o %o', status, data.message) - ngToast.danger({ - content: 'We couldn\'t save the credential', - verticalPosition: 'bottom', - timeout: '3000' + $http.put(baseUrlSrv.getRestApiBase() + '/credential/', credential) + .success(function (data, status, headers, config) { + const index = $scope.credentialInfo.findIndex(elem => elem.entity === entity) + $scope.credentialInfo[index] = credential + return true + }) + .error(function (data, status, headers, config) { + showToast('We could not save the credential', 'danger') + console.log('Error %o %o', status, data.message) + form.$show() }) - form.$show() - }) return false } @@ -167,19 +169,33 @@ function CredentialCtrl ($scope, $rootScope, $http, baseUrlSrv, ngToast) { callback: function (result) { if (result) { $http.delete(baseUrlSrv.getRestApiBase() + '/credential/' + entity) - .success(function (data, status, headers, config) { - let index = _.findIndex($scope.credentialInfo, {'entity': entity}) - $scope.credentialInfo.splice(index, 1) - console.log('Success %o %o', status, data.message) - }) - .error(function (data, status, headers, config) { - console.log('Error %o %o', status, data.message) - }) + .success(function (data, status, headers, config) { + const index = $scope.credentialInfo.findIndex(elem => elem.entity === entity) + $scope.credentialInfo.splice(index, 1) + console.log('Success %o %o', status, data.message) + }) + .error(function (data, status, headers, config) { + showToast(data.message, 'danger') + console.log('Error %o %o', status, data.message) + }) } } }) } + function showToast(message, type) { + const verticalPosition = 'bottom' + const timeout = '3000' + + if (type === 'success') { + ngToast.success({ content: message, verticalPosition: verticalPosition, timeout: timeout, }) + } else if (type === 'info') { + ngToast.info({ content: message, verticalPosition: verticalPosition, timeout: timeout, }) + } else { + ngToast.danger({ content: message, verticalPosition: verticalPosition, timeout: timeout, }) + } + } + let init = function () { getAvailableInterpreters() getCredentialInfo() http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ca40e49a/zeppelin-web/src/app/credential/credential.html ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/credential/credential.html b/zeppelin-web/src/app/credential/credential.html index deb0f3b..cf283d6 100644 --- a/zeppelin-web/src/app/credential/credential.html +++ b/zeppelin-web/src/app/credential/credential.html @@ -85,11 +85,11 @@ limitations under the License. <div class="box width-full"> <div class="row interpreter"> - <div ng-show="_.isEmpty(credentialInfo) || valueform.$hidden" + <div ng-show="!hasCredential(credentialInfo) || valueform.$hidden" class="col-md-12 gray40-message"> <em>Currently there is no credential information</em> </div> - <div class="col-md-12" ng-show="!_.isEmpty(credentialInfo) || valueform.$visible"> + <div class="col-md-12" ng-show="hasCredential(credentialInfo) || valueform.$visible"> <table class="table table-striped"> <thead> <tr> @@ -99,52 +99,54 @@ limitations under the License. <th></th> </tr> </thead> - <tr ng-repeat="credential in credentialInfo"> - <td> - <span> - {{credential.entity}} - </span> - </td> - <td> - <span editable-textarea="credential.username" e-name="username" e-form="valueform" - e-msd-elastic focus-if="credential.username.length == 0"> - {{credential.username}} - </span> - </td> - <td> - <span editable-password="credential.password" e-name="password" e-form="valueform" - e-msd-elastic focus-if="credential.password.length == 0"> - ********** - </span> - </td> - <td> - <!-- Edit credential info --> - <span style="float:right" ng-show="!valueform.$visible"> - <button class="btn btn-default btn-xs" - ng-click="valueform.$show(); - copyOriginCredentialsInfo();"> - <span class="fa fa-pencil"></span> edit</button> - <button class="btn btn-default btn-xs" - ng-click="removeCredentialInfo(credential.entity)"> - <span class="fa fa-trash"></span> remove</button> + <tbody> + <tr ng-repeat="credential in credentialInfo"> + <td> + <span> + {{credential.entity}} + </span> + </td> + <td> + <span editable-textarea="credential.username" e-name="username" e-form="valueform" + e-msd-elastic focus-if="credential.username.length == 0"> + {{credential.username}} + </span> + </td> + <td> + <span editable-password="credential.password" e-name="password" e-form="valueform" + e-msd-elastic focus-if="credential.password.length == 0"> + ********** + </span> + </td> + <td> + <!-- Edit credential info --> + <span style="float:right" ng-show="!valueform.$visible"> + <button class="btn btn-default btn-xs" + ng-click="valueform.$show(); + copyOriginCredentialsInfo();"> + <span class="fa fa-pencil"></span> edit</button> + <button class="btn btn-default btn-xs" + ng-click="removeCredentialInfo(credential.entity)"> + <span class="fa fa-trash"></span> remove</button> - </span> - <span style="float:right" ng-show="valueform.$visible"> - <form editable-form name="valueform" - onbeforesave="updateCredentialInfo(valueform, $data, credential.entity)" - ng-show="valueform.$visible"> - <button type="submit" class="btn btn-primary btn-xs"> - <span class="fa fa-check"></span> save - </button> - <button type="button" class="btn btn-default btn-xs" - ng-disabled="valueform.$waiting" - ng-click="valueform.$cancel();"> - <span class="fa fa-remove"></span> cancel - </button> - </form> - </span> - </td> - </tr> + </span> + <span style="float:right" ng-show="valueform.$visible"> + <form editable-form name="valueform" + onbeforesave="updateCredentialInfo(valueform, $data, credential.entity)" + ng-show="valueform.$visible"> + <button type="submit" class="btn btn-primary btn-xs"> + <span class="fa fa-check"></span> save + </button> + <button type="button" class="btn btn-default btn-xs" + ng-disabled="valueform.$waiting" + ng-click="valueform.$cancel();"> + <span class="fa fa-remove"></span> cancel + </button> + </form> + </span> + </td> + </tr> + </tbody> </table> </div> </div> http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ca40e49a/zeppelin-web/src/app/credential/credential.test.js ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/credential/credential.test.js b/zeppelin-web/src/app/credential/credential.test.js new file mode 100644 index 0000000..d90567b --- /dev/null +++ b/zeppelin-web/src/app/credential/credential.test.js @@ -0,0 +1,114 @@ + +describe('Controller: Credential', function () { + beforeEach(angular.mock.module('zeppelinWebApp')) + + let baseUrlSrvMock = { getRestApiBase: () => '' } + + let $scope + let $controller // controller generator + let $httpBackend + + beforeEach(inject((_$controller_, _$rootScope_, _$compile_, _$httpBackend_, _ngToast_) => { + $scope = _$rootScope_.$new() + $controller = _$controller_ + $httpBackend = _$httpBackend_ + })) + + const credentialResponse = { 'spark.testCredential': { username: 'user1', password: 'password1' }, } + const interpreterResponse = [ + { 'name': 'spark', 'group': 'spark', }, + { 'name': 'md', 'group': 'md', }, + ] // simplified + + function setupInitialization(credentialRes, interpreterRes) { + // requests should follow the exact order + $httpBackend + .when('GET', '/interpreter/setting') + .respond(200, { body: interpreterRes, }) + $httpBackend.expectGET('/interpreter/setting') + $httpBackend + .when('GET', '/credential') + .respond(200, { body: { userCredentials: credentialRes, } }) + $httpBackend.expectGET('/credential') + + // should flush after calling this function + } + + it('should get available interpreters and credentials initially', () => { + const ctrl = createController() + expect(ctrl).toBeDefined() + + setupInitialization(credentialResponse, interpreterResponse) + $httpBackend.flush() + + expect($scope.credentialInfo).toEqual( + [{ entity: 'spark.testCredential', username: 'user1', password: 'password1'}] + ) + expect($scope.availableInterpreters).toEqual( + ['spark.spark', 'md.md'] + ) + + $httpBackend.verifyNoOutstandingExpectation() + $httpBackend.verifyNoOutstandingRequest() + }) + + it('should toggle using toggleAddNewCredentialInfo', () => { + createController() + + expect($scope.showAddNewCredentialInfo).toBe(false) + $scope.toggleAddNewCredentialInfo() + expect($scope.showAddNewCredentialInfo).toBe(true) + $scope.toggleAddNewCredentialInfo() + expect($scope.showAddNewCredentialInfo).toBe(false) + }) + + it('should check empty credentials using isInvalidCredential', () => { + createController() + + $scope.entity = '' + $scope.username = '' + expect($scope.isValidCredential()).toBe(false) + + $scope.entity = 'spark1' + $scope.username = '' + expect($scope.isValidCredential()).toBe(false) + + $scope.entity = '' + $scope.username = 'user1' + expect($scope.isValidCredential()).toBe(false) + + $scope.entity = 'spark' + $scope.username = 'user1' + expect($scope.isValidCredential()).toBe(true) + }) + + it('should be able to add credential via addNewCredentialInfo', () => { + const ctrl = createController() + expect(ctrl).toBeDefined() + setupInitialization(credentialResponse, interpreterResponse) + + // when + const newCredential = { entity: 'spark.sql', username: 'user2', password: 'password2'} + + $httpBackend + .when('PUT', '/credential', newCredential) + .respond(200, { }) + $httpBackend.expectPUT('/credential', newCredential) + + $scope.entity = newCredential.entity + $scope.username = newCredential.username + $scope.password = newCredential.password + $scope.addNewCredentialInfo() + + $httpBackend.flush() + + expect($scope.credentialInfo[1]).toEqual(newCredential) + + $httpBackend.verifyNoOutstandingExpectation() + $httpBackend.verifyNoOutstandingRequest() + }) + + function createController() { + return $controller('CredentialCtrl', { $scope: $scope, baseUrlSrv: baseUrlSrvMock, }) + } +})
