diff --git a/src/lib/db/group-store.ts b/src/lib/db/group-store.ts index 5a1d39f688..7e33f480e7 100644 --- a/src/lib/db/group-store.ts +++ b/src/lib/db/group-store.ts @@ -42,7 +42,6 @@ const rowToGroupUser = (row) => { return { userId: row.user_id, groupId: row.group_id, - role: row.role, joinedAt: row.created_at, }; }; @@ -112,7 +111,7 @@ export default class GroupStore implements IGroupStore { async getAllUsersByGroups(groupIds: number[]): Promise { const rows = await this.db - .select('gu.group_id', 'u.id as user_id', 'role', 'gu.created_at') + .select('gu.group_id', 'u.id as user_id', 'gu.created_at') .from(`${T.GROUP_USER} AS gu`) .join(`${T.USERS} AS u`, 'u.id', 'gu.user_id') .whereIn('gu.group_id', groupIds); @@ -174,32 +173,12 @@ export default class GroupStore implements IGroupStore { return { group_id: groupId, user_id: user.user.id, - role: user.role, created_by: userName, }; }); return (transaction || this.db).batchInsert(T.GROUP_USER, rows); } - async updateExistingUsersInGroup( - groupId: number, - existingUsers: IGroupUserModel[], - transaction?: Transaction, - ): Promise { - const queries = []; - - existingUsers.forEach((user) => { - queries.push( - (transaction || this.db)(T.GROUP_USER) - .where({ group_id: groupId, user_id: user.user.id }) - .update({ role: user.role }) - .transacting(transaction), - ); - }); - - await Promise.all(queries); - } - async deleteOldUsersFromGroup( deletableUsers: IGroupUser[], transaction?: Transaction, @@ -221,7 +200,6 @@ export default class GroupStore implements IGroupStore { ): Promise { await this.db.transaction(async (tx) => { await this.addNewUsersToGroup(groupId, newUsers, userName, tx); - await this.updateExistingUsersInGroup(groupId, existingUsers, tx); await this.deleteOldUsersFromGroup(deletableUsers, tx); }); } diff --git a/src/lib/openapi/spec/group-user-model-schema.ts b/src/lib/openapi/spec/group-user-model-schema.ts index 2992e89624..6f287ac73e 100644 --- a/src/lib/openapi/spec/group-user-model-schema.ts +++ b/src/lib/openapi/spec/group-user-model-schema.ts @@ -5,15 +5,12 @@ export const groupUserModelSchema = { $id: '#/components/schemas/groupUserModelSchema', type: 'object', additionalProperties: false, - required: ['role', 'user'], + required: ['user'], properties: { joinedAt: { type: 'string', format: 'date-time', }, - role: { - type: 'string', - }, user: { $ref: '#/components/schemas/userSchema', }, diff --git a/src/lib/openapi/spec/groups-schema.test.ts b/src/lib/openapi/spec/groups-schema.test.ts index 58987c6be3..64b2b0036b 100644 --- a/src/lib/openapi/spec/groups-schema.test.ts +++ b/src/lib/openapi/spec/groups-schema.test.ts @@ -9,7 +9,6 @@ test('groupsSchema', () => { name: 'Group', users: [ { - role: 'Owner', user: { id: 3, }, diff --git a/src/lib/services/group-service.ts b/src/lib/services/group-service.ts index 7cfb81d221..6ec7514dbd 100644 --- a/src/lib/services/group-service.ts +++ b/src/lib/services/group-service.ts @@ -176,7 +176,7 @@ export class GroupService { } async validateGroup( - { name, users }: IGroupModel, + { name }: IGroupModel, existingGroup?: IGroup, ): Promise { if (!name) { @@ -188,10 +188,6 @@ export class GroupService { throw new NameExistsError('Group name already exists'); } } - - if (users.length == 0 || !users.some((u) => u.role == 'Owner')) { - throw new BadDataError('Group needs to have at least one Owner'); - } } async getRolesForProject(projectId: string): Promise { @@ -215,7 +211,6 @@ export class GroupService { return { user: user, joinedAt: roleUser.joinedAt, - role: roleUser.role, }; }); return { ...group, users: finalUsers }; diff --git a/src/lib/types/group.ts b/src/lib/types/group.ts index 2b3d0804e8..f107c9235e 100644 --- a/src/lib/types/group.ts +++ b/src/lib/types/group.ts @@ -13,7 +13,6 @@ export interface IGroup { export interface IGroupUser { groupId: number; userId: number; - role: string; joinedAt: Date; seenAt?: Date; } @@ -37,7 +36,6 @@ export interface IGroupProject { export interface IGroupUserModel { user: IUser; - role: string; joinedAt?: Date; } diff --git a/src/lib/types/stores/group-store.ts b/src/lib/types/stores/group-store.ts index 7569a20203..f4e9ec1664 100644 --- a/src/lib/types/stores/group-store.ts +++ b/src/lib/types/stores/group-store.ts @@ -40,11 +40,6 @@ export interface IGroupStore extends Store { userName: string, ): Promise; - updateExistingUsersInGroup( - groupId: number, - users: IGroupUserModel[], - ): Promise; - existsWithName(name: string): Promise; create(group: IStoreGroup): Promise; diff --git a/src/migrations/20220808084524-add-group-permissions.js b/src/migrations/20220808084524-add-group-permissions.js new file mode 100644 index 0000000000..2ee878a31e --- /dev/null +++ b/src/migrations/20220808084524-add-group-permissions.js @@ -0,0 +1,19 @@ +'use strict'; + +exports.up = function (db, cb) { + db.runSql( + ` + ALTER TABLE group_user DROP COLUMN IF EXISTS role; + `, + cb, + ); +}; + +exports.down = function (db, cb) { + db.runSql( + ` + ALTER TABLE group_user ADD COLUMN role text check(role in ('Owner', 'Member')) default 'Member'; + `, + cb, + ); +}; diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index bbdda67727..e1ba6d7c4c 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -1359,15 +1359,11 @@ Object { "format": "date-time", "type": "string", }, - "role": Object { - "type": "string", - }, "user": Object { "$ref": "#/components/schemas/userSchema", }, }, "required": Array [ - "role", "user", ], "type": "object", diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index f36e3e3133..148ea63956 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/src/test/e2e/services/access-service.e2e.test.ts @@ -882,7 +882,7 @@ test('Should be allowed move feature toggle to project when given access through await groupStore.addNewUsersToGroup( groupWithProjectAccess.id, - [{ user: viewerUser, role: 'Owner' }], + [{ user: viewerUser }], 'Admin', ); @@ -919,7 +919,7 @@ test('Should not lose user role access when given permissions from a group', asy await groupStore.addNewUsersToGroup( groupWithNoAccess.id, - [{ user: user, role: 'Owner' }], + [{ user: user }], 'Admin', ); @@ -968,13 +968,13 @@ test('Should allow user to take multiple group roles and have expected permissio await groupStore.addNewUsersToGroup( groupWithCreateAccess.id, - [{ user: viewerUser, role: 'Owner' }], + [{ user: viewerUser }], 'Admin', ); await groupStore.addNewUsersToGroup( groupWithDeleteAccess.id, - [{ user: viewerUser, role: 'Owner' }], + [{ user: viewerUser }], 'Admin', ); diff --git a/src/test/fixtures/fake-group-store.ts b/src/test/fixtures/fake-group-store.ts index fbbace4367..c4511231fe 100644 --- a/src/test/fixtures/fake-group-store.ts +++ b/src/test/fixtures/fake-group-store.ts @@ -50,13 +50,6 @@ export default class FakeGroupStore implements IGroupStore { throw new Error('Method not implemented.'); } - updateExistingUsersInGroup( - id: number, - users: IGroupUserModel[], - ): Promise { - throw new Error('Method not implemented.'); - } - getAllUsersByGroups(groupIds: number[]): Promise { throw new Error('Method not implemented.'); }