Skip to content

Commit

Permalink
🎨 unique constraint for permission and role name (#7674)
Browse files Browse the repository at this point in the history
refs #7494,  refs #7495 

I saw tests adding permissions and roles twice. (see screenshots)
That happened because the setup in the test was mis-used and there is no restriction for static resources to create duplicates.
With this PR i suggest to make name unique.
  • Loading branch information
kirrg001 authored and ErisDS committed Nov 9, 2016
1 parent 48387e4 commit b48031f
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 19 deletions.
4 changes: 2 additions & 2 deletions core/server/data/schema/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ module.exports = {
roles: {
id: {type: 'increments', nullable: false, primary: true},
uuid: {type: 'string', maxlength: 36, nullable: false, validations: {isUUID: true}},
name: {type: 'string', maxlength: 150, nullable: false},
name: {type: 'string', maxlength: 150, nullable: false, unique: true},
description: {type: 'string', maxlength: 200, nullable: true},
created_at: {type: 'dateTime', nullable: false},
created_by: {type: 'integer', nullable: false},
Expand All @@ -70,7 +70,7 @@ module.exports = {
permissions: {
id: {type: 'increments', nullable: false, primary: true},
uuid: {type: 'string', maxlength: 36, nullable: false, validations: {isUUID: true}},
name: {type: 'string', maxlength: 150, nullable: false},
name: {type: 'string', maxlength: 150, nullable: false, unique: true},
object_type: {type: 'string', maxlength: 150, nullable: false},
action_type: {type: 'string', maxlength: 150, nullable: false},
object_id: {type: 'integer', nullable: true},
Expand Down
2 changes: 1 addition & 1 deletion core/test/functional/routes/api/themes_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('Themes API', function () {
ghost().then(function (ghostServer) {
request = supertest.agent(ghostServer.rootApp);
}).then(function () {
return testUtils.doAuth(request, 'perms:theme', 'perms:init', 'users:roles:no-owner');
return testUtils.doAuth(request, 'perms:init', 'users:no-owner');
}).then(function (token) {
scope.ownerAccessToken = token;

Expand Down
2 changes: 1 addition & 1 deletion core/test/functional/routes/api/users_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('User API', function () {
ghost().then(function (ghostServer) {
request = supertest.agent(ghostServer.rootApp);
}).then(function () {
return testUtils.doAuth(request, 'users:roles:no-owner');
return testUtils.doAuth(request, 'users:no-owner');
}).then(function (token) {
ownerAccessToken = token;

Expand Down
4 changes: 2 additions & 2 deletions core/test/integration/api/api_authentication_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ describe('Authentication API', function () {
});

describe('Not Owner', function () {
beforeEach(testUtils.setup('roles', 'users:roles', 'settings', 'perms:setting', 'perms:init', 'perms:user'));
beforeEach(testUtils.setup('users:roles', 'settings', 'perms:setting', 'perms:init', 'perms:user'));

it('should report that setup has been completed', function (done) {
AuthAPI.isSetup().then(function (result) {
Expand Down Expand Up @@ -586,7 +586,7 @@ describe('Authentication API', function () {
});

describe('Owner', function () {
beforeEach(testUtils.setup('roles', 'users:roles', 'settings', 'perms:setting', 'perms:init'));
beforeEach(testUtils.setup('users:roles', 'settings', 'perms:setting', 'perms:init'));

it('should report that setup has been completed', function (done) {
AuthAPI.isSetup().then(function (result) {
Expand Down
24 changes: 11 additions & 13 deletions core/test/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,10 @@ fixtures = {
user = DataGenerator.forKnex.createBasic(user);
user = _.extend({}, user, {status: 'inactive'});

return db.knex('roles').insert(DataGenerator.forKnex.roles).then(function () {
return db.knex('users').insert(user);
}).then(function () {
return db.knex('roles_users').insert(DataGenerator.forKnex.roles_users[0]);
});
return db.knex('users').insert(user)
.then(function () {
return db.knex('roles_users').insert(DataGenerator.forKnex.roles_users[0]);
});
},

insertOwnerUser: function insertOwnerUser() {
Expand Down Expand Up @@ -243,14 +242,13 @@ fixtures = {
});
},

createUsersWithRolesWithoutOwner: function createUsersWithRolesWithoutOwner() {
createUsersWithoutOwner: function createUsersWithoutOwner() {
var usersWithoutOwner = DataGenerator.forKnex.users.slice(1);

return db.knex('roles').insert(DataGenerator.forKnex.roles).then(function () {
return db.knex('users').insert(usersWithoutOwner);
}).then(function () {
return db.knex('roles_users').insert(DataGenerator.forKnex.roles_users);
});
return db.knex('users').insert(usersWithoutOwner)
.then(function () {
return db.knex('roles_users').insert(DataGenerator.forKnex.roles_users);
});
},

createExtraUsers: function createExtraUsers() {
Expand Down Expand Up @@ -443,7 +441,7 @@ toDoList = {
return models.Settings.populateDefaults().then(function () { return SettingsAPI.updateSettingsCache(); });
},
'users:roles': function createUsersWithRoles() { return fixtures.createUsersWithRoles(); },
'users:roles:no-owner': function createUsersWithRoles() { return fixtures.createUsersWithRolesWithoutOwner(); },
'users:no-owner': function createUsersWithoutOwner() { return fixtures.createUsersWithoutOwner(); },
users: function createExtraUsers() { return fixtures.createExtraUsers(); },
'user:token': function createTokensForUser() { return fixtures.createTokensForUser(); },
owner: function insertOwnerUser() { return fixtures.insertOwnerUser(); },
Expand Down Expand Up @@ -503,7 +501,7 @@ getFixtureOps = function getFixtureOps(toDos) {
fixtureOps.push(toDoList[tmp[0]](tmp[1]));
} else {
if (!toDoList[toDo]) {
throw new Error('setup todo does not exist - spell mistake?');
throw new Error('setup todo does not exist - spell mistake? --> ' + toDo);
}

fixtureOps.push(toDoList[toDo]);
Expand Down

0 comments on commit b48031f

Please sign in to comment.