From c1d6674d04c871450bbdd437718c0a22651b11b8 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Tue, 22 Mar 2016 16:53:44 +0100 Subject: [PATCH] [WIP] Don't wait for downloading user data when opening the app --- app/components/branch-row.js | 16 +++--------- app/components/dashboard-row.js | 14 ++++++----- app/components/log-content.js | 9 +++---- app/components/repo-show-tools.js | 15 +++++------ app/services/permissions.js | 29 ++++++++++++++++++++++ app/utils/permission.js | 33 +++++++++---------------- tests/unit/services/permissions-test.js | 12 +++++++++ tests/unit/utils/permission-test.js | 30 ++++++++++++++++++++++ 8 files changed, 107 insertions(+), 51 deletions(-) create mode 100644 app/services/permissions.js create mode 100644 tests/unit/services/permissions-test.js diff --git a/app/components/branch-row.js b/app/components/branch-row.js index d4ae6c98..3c5c6f85 100644 --- a/app/components/branch-row.js +++ b/app/components/branch-row.js @@ -5,6 +5,8 @@ import config from 'travis/config/environment'; export default Ember.Component.extend({ routing: Ember.inject.service('-routing'), + permissions: Ember.inject.service(), + tagName: 'li', classNameBindings: ['build.last_build.state'], classNames: ['branch-row', 'row-li'], @@ -54,18 +56,8 @@ export default Ember.Component.extend({ }.property(), canTrigger: function() { - var permissions; - if (!this.get('auth.signedIn')) { - return false; - } else { - permissions = this.get('auth.currentUser.permissions'); - if (permissions.contains(parseInt(this.get('build.repository.id')))) { - return true; - } else { - return false; - } - } - }.property(), + return this.get('permissions').hasPermission(this.get('build.repository')); + }.property('permissions.all', 'build.repository'), triggerBuild: function() { var apiEndpoint, options, repoId; diff --git a/app/components/dashboard-row.js b/app/components/dashboard-row.js index 96e6c625..cd688454 100644 --- a/app/components/dashboard-row.js +++ b/app/components/dashboard-row.js @@ -4,6 +4,8 @@ import config from 'travis/config/environment'; import { hasAdminPermission, hasPushPermission } from 'travis/utils/permission'; export default Ember.Component.extend({ + permissions: Ember.inject.service(), + tagName: 'li', classNameBindings: ['repo.default_branch.last_build.state'], classNames: ['rows', 'rows--dashboard'], @@ -16,13 +18,13 @@ export default Ember.Component.extend({ return githubCommitUrl(this.get('repo.slug'), this.get('repo.default_branch.last_build.commit.sha')); }.property('repo'), - displayMenuTofu: function() { - return hasPushPermission(this.get('currentUser'), this.get('repo.id')); - }, + displayMenuTofu: Ember.computed('permissions.all', 'repo', function() { + return this.get('permissions').hasPushPermission(this.get('repo')); + }), - displayActivateLink: function() { - return hasAdminPermission(this.get('currentUser'), this.get('repo.id')); - }, + displayActivateLink: Ember.computed('permissions.all', 'repo', function() { + return this.get('permissions').hasAdminPermission(this.get('repo')); + }), actions: { tiggerBuild(branch) { diff --git a/app/components/log-content.js b/app/components/log-content.js index 9db46eba..4ef694a5 100644 --- a/app/components/log-content.js +++ b/app/components/log-content.js @@ -60,6 +60,8 @@ Object.defineProperty(Log.Limit.prototype, 'limited', { export default Ember.Component.extend({ popup: Ember.inject.service(), + permissions: Ember.inject.service(), + classNameBindings: ['logIsVisible:is-open'], logIsVisible: false, currentUserBinding: 'auth.currentUser', @@ -190,11 +192,8 @@ export default Ember.Component.extend({ }.property('job.log.id', 'job.log.token'), hasPermission: function() { - var permissions; - if (permissions = this.get('currentUser.permissions')) { - return permissions.contains(parseInt(this.get('job.repo.id'))); - } - }.property('currentUser.permissions.length', 'job.repo.id'), + return this.get('permissions').hasPermission(this.get('job.repo')); + }.property('permissions.all', 'job.repo'), canRemoveLog: function() { var job; diff --git a/app/components/repo-show-tools.js b/app/components/repo-show-tools.js index 741451bd..80ad7a84 100644 --- a/app/components/repo-show-tools.js +++ b/app/components/repo-show-tools.js @@ -1,9 +1,10 @@ import Ember from 'ember'; import config from 'travis/config/environment'; -import { hasPermission, hasPushPermission } from 'travis/utils/permission'; export default Ember.Component.extend({ popup: Ember.inject.service(), + permissions: Ember.inject.service(), + classNames: ['option-button'], classNameBindings: ['isOpen:display'], isOpen: false, @@ -24,15 +25,15 @@ export default Ember.Component.extend({ } }, displaySettingsLink: function() { - return hasPushPermission(this.get('currentUser'), this.get('repo.id')); - }.property('currentUser.pushPermissions', 'repo.id'), + return this.get('permissions').hasPushPermission(this.get('repo')); + }.property('permissions.all', 'repo'), displayCachesLink: function() { - return hasPushPermission(this.get('currentUser'), this.get('repo.id')) && config.endpoints.caches; - }.property('currentUser.pushPermissions', 'repo.id'), + return this.get('permissions').hasPushPermission(this.get('repo')) && config.endpoints.caches; + }.property('permissions.all', 'repo'), displayStatusImages: function() { - return hasPermission(this.get('currentUser'), this.get('repo.id')); - }.property('currentUser.permissions', 'repo.id') + return this.get('permissions').hasPermission(this.get('repo')); + }.property('permissions.all', 'repo'), }); diff --git a/app/services/permissions.js b/app/services/permissions.js new file mode 100644 index 00000000..e34185a3 --- /dev/null +++ b/app/services/permissions.js @@ -0,0 +1,29 @@ +import Ember from 'ember'; +import { hasPermission, hasPushPermission, hasAdminPermission } from 'travis/utils/permission'; + +export default Ember.Service.extend({ + auth: Ember.inject.service(), + + currentUser: Ember.computed.alias('auth.currentUser'), + // This is computed property that can be used to allow any properties that + // use permissions service to add dependencies easier. So instead of depending + // on each of these things separately, we can depend on all + all: Ember.computed('currentUser.permissions', 'currentUser.permissions.[]', + 'currentUser.pushPermissions', 'currentUser.pushPermissions.[]', + 'currentUser.adminPermissions', 'currentUser.adminPermissions.[]', + function() { + return; + }), + + hasPermission(repo) { + return hasPermission(this.get('currentUser'), repo); + }, + + hasPushPermission(repo) { + return hasPushPermission(this.get('currentUser'), repo); + }, + + hasAdminPermission(repo) { + return hasAdminPermission(this.get('currentUser'), repo); + } +}); diff --git a/app/utils/permission.js b/app/utils/permission.js index cbe6785a..b8dc87ab 100644 --- a/app/utils/permission.js +++ b/app/utils/permission.js @@ -1,31 +1,22 @@ -var hasPermission = function(user, repoId) { - var id = parseInt(repoId); - var permissions; - if (user) { - if (permissions = user.get('permissions')) { - return permissions.contains(id); - } +var hasPermission = function(user, repo) { + let id = isNaN(repo) ? repo.get('id') : parseInt(repo); + if(user) { + return user.get('permissions').contains(id); } }; -var hasPushPermission = function(user, repoId) { - var id = parseInt(repoId); - var permissions; - if (user) { - if (permissions = user.get('pushPermissions')) { - return permissions.contains(id); - } +var hasPushPermission = function(user, repo) { + let id = isNaN(repo) ? repo.get('id') : parseInt(repo); + if(user) { + return user.get('pushPermissions').contains(id); } }; -var hasAdminPermission = function(user, repoId) { - var id = parseInt(repoId); - var permissions; - if (user) { - if (permissions = user.get('adminPermissions')) { - return permissions.contains(id); - } +var hasAdminPermission = function(user, repo) { + let id = isNaN(repo) ? repo.get('id') : parseInt(repo); + if(user) { + return user.get('adminPermissions').contains(id); } }; diff --git a/tests/unit/services/permissions-test.js b/tests/unit/services/permissions-test.js new file mode 100644 index 00000000..ae0af668 --- /dev/null +++ b/tests/unit/services/permissions-test.js @@ -0,0 +1,12 @@ +import { moduleFor, test } from 'ember-qunit'; + +moduleFor('service:permissions', 'Unit | Service | permissions', { + // Specify the other units that are required for this test. + // needs: ['service:foo'] +}); + +// Replace this with your real tests. +test('it exists', function(assert) { + let service = this.subject(); + assert.ok(service); +}); diff --git a/tests/unit/utils/permission-test.js b/tests/unit/utils/permission-test.js index 7ae1af92..e80a639a 100644 --- a/tests/unit/utils/permission-test.js +++ b/tests/unit/utils/permission-test.js @@ -6,21 +6,51 @@ module('Unit | Utility | permission'); test('it checks permissions', function(assert) { let user = Ember.Object.create({permissions: [1]}); + assert.ok(hasPermission(user, 1)); assert.ok(!hasPermission(user, 2)); assert.ok(!hasPermission(null, 1)); }); +test('it checks permissions if a repo object is given', function(assert) { + let repo1 = Ember.Object.create({ id: 1 }), + repo2 = Ember.Object.create({ id: 2 }), + user = Ember.Object.create({permissions: [1]}); + + assert.ok(hasPermission(user, repo1)); + assert.ok(!hasPermission(user, repo2)); +}); + test('it checks push permissions', function(assert) { let user = Ember.Object.create({pushPermissions: [1]}); + assert.ok(hasPushPermission(user, 1)); assert.ok(!hasPushPermission(user, 2)); assert.ok(!hasPushPermission(null, 1)); }); +test('it checks push permissions if a repo object is given', function(assert) { + let repo1 = Ember.Object.create({ id: 1 }), + repo2 = Ember.Object.create({ id: 2 }), + user = Ember.Object.create({pushPermissions: [1]}); + + assert.ok(hasPushPermission(user, repo1)); + assert.ok(!hasPushPermission(user, repo2)); +}); + test('it checks admin permissions', function(assert) { let user = Ember.Object.create({adminPermissions: [1]}); + assert.ok(hasAdminPermission(user, 1)); assert.ok(!hasAdminPermission(user, 2)); assert.ok(!hasAdminPermission(null, 1)); }); + +test('it checks admin permissions if a repo object is given', function(assert) { + let repo1 = Ember.Object.create({ id: 1 }), + repo2 = Ember.Object.create({ id: 2 }), + user = Ember.Object.create({adminPermissions: [1]}); + + assert.ok(hasAdminPermission(user, repo1)); + assert.ok(!hasAdminPermission(user, repo2)); +});