Fix/remove group owner concept (#1905)

* fix: remove group user role
Unleash · Aug 11, 2022 · 38e428d · 38e428d
1 parent 0c8ebd4
commit 38e428d
Show file tree

Hide file tree

Showing 10 changed files with 26 additions and 56 deletions.
diff --git 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<IGroupUser[]> {
         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<void> {
-        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<void> {
         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
@@ -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
@@ -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
@@ -176,7 +176,7 @@ export class GroupService {
     }
 
     async validateGroup(
-        { name, users }: IGroupModel,
+        { name }: IGroupModel,
         existingGroup?: IGroup,
     ): Promise<void> {
         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<IGroupRole[]> {
@@ -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
@@ -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
@@ -40,11 +40,6 @@ export interface IGroupStore extends Store<IGroup, number> {
         userName: string,
     ): Promise<void>;
 
-    updateExistingUsersInGroup(
-        groupId: number,
-        users: IGroupUserModel[],
-    ): Promise<void>;
-
     existsWithName(name: string): Promise<boolean>;
 
     create(group: IStoreGroup): Promise<IGroup>;

diff --git a/src/migrations/20220808084524-add-group-permissions.js 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
@@ -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
@@ -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
@@ -50,13 +50,6 @@ export default class FakeGroupStore implements IGroupStore {
         throw new Error('Method not implemented.');
     }
 
-    updateExistingUsersInGroup(
-        id: number,
-        users: IGroupUserModel[],
-    ): Promise<void> {
-        throw new Error('Method not implemented.');
-    }
-
     getAllUsersByGroups(groupIds: number[]): Promise<IGroupUser[]> {
         throw new Error('Method not implemented.');
     }