From 30c831b84b22637c3c71c9a1b0056676e948e1ae Mon Sep 17 00:00:00 2001 From: rpletz Date: Tue, 8 Oct 2024 13:09:22 -0700 Subject: [PATCH] Revert "Revert "feature/DEVSU-2384-permission-storage-improvement"" This reverts commit cd9869d7837f5596505f3ab1a8585079653a1e34. --- app/constants.js | 13 ++ app/libs/__mocks__/getUser.js | 1 - app/libs/getUser.js | 3 +- app/middleware/group.js | 11 +- app/models/clearCache.js | 2 +- app/models/index.js | 14 +- app/models/notification/notification.js | 17 +- app/models/user/userGroup.js | 24 ++- app/models/user/userGroupMember.js | 32 --- app/routes/notification/notification.js | 25 ++- app/routes/swagger/schemas.js | 4 +- app/routes/swagger/swagger.json | 195 +----------------- app/routes/user/group.js | 106 +--------- app/routes/user/member.js | 27 +-- app/routes/user/user.js | 20 +- ...46-DEVSU-2384-permission-storage-update.js | 81 ++++++++ ...14165504-DEVSU-2400-remove-owner-column.js | 17 -- ...36-DEVSU-2384-permission-storage-update.js | 27 +++ .../routes/notification/notifications.test.js | 32 +-- test/routes/user/group.test.js | 122 +---------- test/routes/user/member.test.js | 106 +++++----- test/routes/user/user.test.js | 30 ++- test/testSetup.js | 22 +- 23 files changed, 286 insertions(+), 645 deletions(-) delete mode 100644 app/models/user/userGroupMember.js create mode 100644 migrations/20240725223946-DEVSU-2384-permission-storage-update.js delete mode 100644 migrations/20240814165504-DEVSU-2400-remove-owner-column.js create mode 100644 migrations/20240828195736-DEVSU-2384-permission-storage-update.js diff --git a/app/constants.js b/app/constants.js index 5808dad85..3eb2324c2 100644 --- a/app/constants.js +++ b/app/constants.js @@ -55,4 +55,17 @@ module.exports = { MASTER_REPORT_ACCESS: ['admin', 'manager'], ALL_PROJECTS_ACCESS: ['admin', 'all projects access'], UPDATE_METHODS: ['POST', 'PUT', 'DELETE'], + USER_GROUPS: [ + 'admin', + 'manager', + 'report assignment access', + 'create report access', + 'germline access', + 'non-production access', + 'unreviewed access', + 'all projects access', + 'template edit access', + 'appendix edit access', + 'variant-text edit access', + ], }; diff --git a/app/libs/__mocks__/getUser.js b/app/libs/__mocks__/getUser.js index edeb7ba89..6c21dfc5f 100644 --- a/app/libs/__mocks__/getUser.js +++ b/app/libs/__mocks__/getUser.js @@ -7,7 +7,6 @@ const include = [ attributes: { exclude: ['id', 'deletedAt', 'updatedAt', 'createdAt', 'updatedBy'], }, - through: {attributes: []}, }, { model: db.models.project, diff --git a/app/libs/getUser.js b/app/libs/getUser.js index 347eeee25..f8cfa6a1b 100644 --- a/app/libs/getUser.js +++ b/app/libs/getUser.js @@ -15,9 +15,8 @@ const include = [ model: db.models.userGroup, as: 'groups', attributes: { - exclude: ['id', 'deletedAt', 'updatedAt', 'createdAt', 'updatedBy'], + exclude: ['id', 'deletedAt', 'updatedAt', 'createdAt', 'updatedBy', 'userId'], }, - through: {attributes: []}, }, { model: db.models.project, diff --git a/app/middleware/group.js b/app/middleware/group.js index 0fc6cae30..21959df56 100644 --- a/app/middleware/group.js +++ b/app/middleware/group.js @@ -3,13 +3,12 @@ const db = require('../models'); const logger = require('../log'); // Middleware for user groups -module.exports = async (req, res, next, ident) => { +module.exports = async (req, res, next, group) => { let result; try { - result = await db.models.userGroup.findOne({ - where: {ident}, + result = await db.models.user.scope('public').findAll({ include: [ - {as: 'users', model: db.models.user, attributes: {exclude: ['id', 'deletedAt', 'password', 'updatedBy']}, through: {attributes: []}}, + {as: 'groups', model: db.models.userGroup, attributes: [], where: {name: group}}, ], }); } catch (error) { @@ -20,12 +19,12 @@ module.exports = async (req, res, next, ident) => { } if (!result) { - logger.error(`Unable to find group ${ident}`); + logger.error(`Unable to find group ${group}`); return res.status(HTTP_STATUS.NOT_FOUND).json({ error: {message: 'Unable to find group'}, }); } - req.group = result; + req.group = {name: group, users: result}; return next(); }; diff --git a/app/models/clearCache.js b/app/models/clearCache.js index 8ce7c9ab4..4aad3b87e 100644 --- a/app/models/clearCache.js +++ b/app/models/clearCache.js @@ -12,7 +12,7 @@ const CLEAR_GERMLINE_CACHE_MODELS = [ ]; const CLEAR_USER_CACHE_MODELS = [ - 'userGroup', 'userGroupMember', 'userProject', + 'userGroup', 'userProject', ]; /** diff --git a/app/models/index.js b/app/models/index.js index 915912bb8..b44be7ebc 100644 --- a/app/models/index.js +++ b/app/models/index.js @@ -80,13 +80,9 @@ user.belongsToMany(analysisReports, { }); const userGroup = require('./user/userGroup')(sequelize, Sq); -const userGroupMember = require('./user/userGroupMember')(sequelize, Sq); -user.belongsToMany(userGroup, { - as: 'groups', through: {model: userGroupMember, unique: false}, foreignKey: 'user_id', otherKey: 'group_id', onDelete: 'CASCADE', -}); -userGroup.belongsToMany(user, { - as: 'users', through: {model: userGroupMember, unique: false}, foreignKey: 'group_id', otherKey: 'user_id', onDelete: 'CASCADE', +user.hasMany(userGroup, { + as: 'groups', foreignKey: 'userId', onDelete: 'CASCADE', constraints: true, }); // IMPORTANT must be done before the variant models are defined @@ -487,9 +483,6 @@ const notification = require('./notification/notification')(sequelize, Sq); user.hasMany(notification, { as: 'notifications', foreignKey: 'userId', onDelete: 'CASCADE', constraints: true, }); -userGroup.hasMany(notification, { - as: 'notifications', foreignKey: 'userGroupId', onDelete: 'CASCADE', constraints: true, -}); project.hasMany(notification, { as: 'notifications', foreignKey: 'projectId', onDelete: 'CASCADE', constraints: true, }); @@ -505,9 +498,6 @@ notification.belongsTo(user, { notification.belongsTo(project, { as: 'project', foreignKey: 'projectId', targetKey: 'id', onDelete: 'CASCADE', constraints: true, }); -notification.belongsTo(userGroup, { - as: 'userGroup', foreignKey: 'userGroupId', targetKey: 'id', onDelete: 'CASCADE', constraints: true, -}); const notificationTrack = require('./notification/notificationTrack')(sequelize, Sq); diff --git a/app/models/notification/notification.js b/app/models/notification/notification.js index 1145470ab..9fd60642e 100644 --- a/app/models/notification/notification.js +++ b/app/models/notification/notification.js @@ -1,4 +1,5 @@ const {DEFAULT_COLUMNS, DEFAULT_OPTIONS} = require('../base'); +const {USER_GROUPS} = require('../../constants'); module.exports = (sequelize, Sq) => { const notification = sequelize.define( @@ -34,16 +35,12 @@ module.exports = (sequelize, Sq) => { key: 'id', }, }, - userGroupId: { - type: Sq.INTEGER, - name: 'userGroupId', - field: 'user_group_id', + userGroup: { + name: 'userGroup', + field: 'user_group', unique: false, allowNull: true, - references: { - model: 'user_groups', - key: 'id', - }, + type: Sq.ENUM(USER_GROUPS), }, templateId: { type: Sq.INTEGER, @@ -62,11 +59,10 @@ module.exports = (sequelize, Sq) => { scopes: { public: { attributes: { - exclude: ['id', 'deletedAt', 'updatedBy', 'userId', 'projectId', 'userGroupId', 'templateId'], + exclude: ['id', 'deletedAt', 'updatedBy', 'userId', 'projectId', 'templateId'], }, include: [ {model: sequelize.models.user.scope('minimal'), as: 'user'}, - {model: sequelize.models.userGroup.scope('minimal'), as: 'userGroup'}, {model: sequelize.models.template.scope('minimal'), as: 'template'}, {model: sequelize.models.project.scope('minimal'), as: 'project'}, ], @@ -74,7 +70,6 @@ module.exports = (sequelize, Sq) => { extended: { include: [ {model: sequelize.models.user, as: 'user'}, - {model: sequelize.models.userGroup, as: 'userGroup', include: {model: sequelize.models.user, as: 'users', through: {attributes: []}}}, {model: sequelize.models.template, as: 'template'}, {model: sequelize.models.project, as: 'project'}, ], diff --git a/app/models/user/userGroup.js b/app/models/user/userGroup.js index b824413e9..f5cf367d4 100644 --- a/app/models/user/userGroup.js +++ b/app/models/user/userGroup.js @@ -1,32 +1,36 @@ const {DEFAULT_COLUMNS, DEFAULT_OPTIONS} = require('../base'); +const {USER_GROUPS} = require('../../constants'); module.exports = (sequelize, Sq) => { const userGroup = sequelize.define('userGroup', { ...DEFAULT_COLUMNS, - name: { - type: Sq.STRING, + userId: { + name: 'userId', + field: 'user_id', + type: Sq.INTEGER, + references: { + model: 'users', + key: 'id', + }, allowNull: false, }, - description: { - type: Sq.STRING, - allowNull: true, + name: { + type: Sq.ENUM(USER_GROUPS), + allowNull: false, }, }, { ...DEFAULT_OPTIONS, tableName: 'user_groups', scopes: { public: { - order: [['name', 'ASC']], + order: [['user_id', 'ASC']], attributes: { exclude: ['id', 'deletedAt', 'updatedBy'], }, include: [ - {as: 'users', model: sequelize.models.user, attributes: {exclude: ['id', 'deletedAt', 'password', 'updatedBy']}, through: {attributes: []}}, + {as: 'users', model: sequelize.models.user, attributes: {exclude: ['id', 'deletedAt', 'password', 'updatedBy']}}, ], }, - minimal: { - attributes: ['ident', 'name'], - }, }, }); diff --git a/app/models/user/userGroupMember.js b/app/models/user/userGroupMember.js deleted file mode 100644 index 557fc274d..000000000 --- a/app/models/user/userGroupMember.js +++ /dev/null @@ -1,32 +0,0 @@ -const {DEFAULT_MAPPING_COLUMNS, DEFAULT_MAPPING_OPTIONS} = require('../base'); - -module.exports = (sequelize, Sq) => { - return sequelize.define( - 'userGroupMember', - { - ...DEFAULT_MAPPING_COLUMNS, - user_id: { - type: Sq.INTEGER, - unique: false, - allowNull: false, - references: { - model: 'users', - key: 'id', - }, - }, - group_id: { - type: Sq.INTEGER, - unique: false, - allowNull: false, - references: { - model: 'user_groups', - key: 'id', - }, - }, - }, - { - ...DEFAULT_MAPPING_OPTIONS, - tableName: 'user_group_members', - }, - ); -}; diff --git a/app/routes/notification/notification.js b/app/routes/notification/notification.js index 54670f0b2..31daac97a 100644 --- a/app/routes/notification/notification.js +++ b/app/routes/notification/notification.js @@ -1,5 +1,6 @@ const HTTP_STATUS = require('http-status-codes'); const express = require('express'); +const {USER_GROUPS} = require('../../constants'); const db = require('../../models'); const logger = require('../../log'); @@ -12,7 +13,6 @@ const pairs = { user: db.models.user, project: db.models.project, template: db.models.template, - user_group: db.models.userGroup, }; // for each entry in pairs, assumes the key-named value in @@ -72,10 +72,18 @@ router.route('/') }); } } + + if (!USER_GROUPS.includes(req.body.user_group) && req.body.user_group) { + logger.error(`${req.body.user_group} group not found`); + return res.status(HTTP_STATUS.NOT_FOUND).json({ + error: {message: `${req.body.user_group} group not found`}, + }); + } + try { const whereClause = { ...((req.body.user_id == null) ? {} : {user_id: req.body.user_id}), - ...((req.body.user_group_id == null) ? {} : {user_group_id: req.body.user_group_id}), + ...((req.body.user_group == null) ? {} : {user_group: req.body.user_group}), ...((req.body.template_id == null) ? {} : {template_id: req.body.template_id}), ...((req.body.project_id == null) ? {} : {project_id: req.body.project_id}), }; @@ -100,13 +108,13 @@ router.route('/') return res.status(HTTP_STATUS.FORBIDDEN); } - if (req.body.user_id && req.body.user_group_id) { + if (req.body.user_id && req.body.user_group) { return res.status(HTTP_STATUS.CONFLICT).json({ error: {message: 'Only one of user and user group should be specified'}, }); } - if (!req.body.user_id && !req.body.user_group_id) { + if (!req.body.user_id && !req.body.user_group) { return res.status(HTTP_STATUS.CONFLICT).json({ error: {message: 'Exactly one of user and user group should be specified'}, }); @@ -130,6 +138,13 @@ router.route('/') }); } + if (!USER_GROUPS.includes(req.body.user_group) && req.body.user_group) { + logger.error(`${req.body.user_group} group not found`); + return res.status(HTTP_STATUS.NOT_FOUND).json({ + error: {message: `${req.body.user_group} group not found`}, + }); + } + // check the given user, if any, is bound to the given project (or is admin) if (req.body.user_id) { let notifUser; @@ -173,7 +188,7 @@ router.route('/') where: { projectId: req.body.project_id, userId: req.body.user_id ? req.body.user_id : null, - userGroupId: req.body.user_group_id ? req.body.user_group_id : null, + userGroup: req.body.user_group ? req.body.user_group : null, eventType: req.body.event_type, templateId: req.body.template_id, }, diff --git a/app/routes/swagger/schemas.js b/app/routes/swagger/schemas.js index 8ecd2181c..7b30155cc 100644 --- a/app/routes/swagger/schemas.js +++ b/app/routes/swagger/schemas.js @@ -14,7 +14,7 @@ const ID_FIELDS = [ ]; const PUBLIC_VIEW_EXCLUDE = [...ID_FIELDS, 'id', 'reportId', 'geneId', 'deletedAt', 'updatedBy']; const GENERAL_EXCLUDE = REPORT_EXCLUDE.concat(ID_FIELDS); -const GENERAL_EXCLUDE_ASSOCIATIONS = ['report', 'reports', 'germlineReport', 'userProject', 'userGroupMember']; +const GENERAL_EXCLUDE_ASSOCIATIONS = ['report', 'reports', 'germlineReport', 'userProject']; const MODELS_WITH_VARIANTS = ['kbMatches', 'genes']; @@ -96,7 +96,7 @@ const getExcludes = (model) => { // Remove joining models from the list of models to use for generating schemas const { - userGroupMember, reportProject, userProject, ...models + reportProject, userProject, ...models } = db.models; // Generate schemas from Sequelize models. One for the public returned value, one diff --git a/app/routes/swagger/swagger.json b/app/routes/swagger/swagger.json index 66255e370..96f558317 100644 --- a/app/routes/swagger/swagger.json +++ b/app/routes/swagger/swagger.json @@ -364,196 +364,6 @@ "description": "You are not authorized to view this resource" } } - }, - "post": { - "summary": "Create new group", - "description": "Create a new ACL group in the Report API", - "tags": [ - "Group" - ], - "security": [ - { - "basicAuth": [] - } - ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "type": "object", - "properties": { - "name": { - "description": "Name of the new group", - "type": "string" - } - }, - "required": [ - "name" - ] - } - } - } - }, - "responses": { - "200": { - "description": "Group successfully added to database", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/userGroupAssociations" - } - } - } - }, - "401": { - "$ref": "#/components/responses/UnauthorizedError" - }, - "403": { - "description": "You are not allowed to create new group entries." - } - } - } - }, - "/user/group/{group}": { - "get": { - "summary": "Get a group", - "description": "Get detailed group object from ident.", - "tags": [ - "Group" - ], - "security": [ - { - "basicAuth": [] - } - ], - "parameters": [ - { - "name": "group", - "in": "path", - "required": true, - "description": "Group UUID string", - "schema": { - "type": "string" - } - } - ], - "responses": { - "200": { - "description": "Group found, API will return the detailed group object.", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/userGroupAssociations" - } - } - } - }, - "401": { - "$ref": "#/components/responses/UnauthorizedError" - }, - "403": { - "description": "You are not authorized to view the resource" - } - } - }, - "put": { - "summary": "Update a group", - "description": "Update the groups definition in the IPR API database", - "tags": [ - "Group" - ], - "security": [ - { - "basicAuth": [] - } - ], - "parameters": [ - { - "name": "group", - "in": "path", - "required": true, - "description": "Group UUID string", - "schema": { - "type": "string" - } - } - ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "type": "object", - "properties": { - "name": { - "description": "Name of the new group", - "type": "string" - }, - "required": [ - "name" - ] - } - } - } - } - }, - "responses": { - "200": { - "description": "Successfully updated the group.", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/userGroupAssociations" - } - } - } - }, - "401": { - "$ref": "#/components/responses/UnauthorizedError" - }, - "403": { - "description": "You are not authorized to perform this action" - }, - "404": { - "description": "Unable to find the group" - } - } - }, - "delete": { - "summary": "Remove a group", - "description": "Remove a group entry from the database. Doing this will immediately\nrevoke group member's access to defined resources.", - "tags": [ - "Group" - ], - "security": [ - { - "basicAuth": [] - } - ], - "parameters": [ - { - "name": "group", - "in": "path", - "required": true, - "description": "Group UUID string", - "schema": { - "type": "string" - } - } - ], - "responses": { - "204": { - "description": "The group was successfully removed" - }, - "401": { - "$ref": "#/components/responses/UnauthorizedError" - }, - "403": { - "description": "You are not authorized to perform this action" - }, - "404": { - "description": "Unable to find the group or user" - } - } } }, "/user/group/{group}/member": { @@ -7721,8 +7531,7 @@ } } }, - "userProject": {}, - "userGroupMember": {} + "userProject": {} }, "responses": { "UnauthorizedError": { @@ -7745,4 +7554,4 @@ "basicAuth": [] } ] -} +} \ No newline at end of file diff --git a/app/routes/user/group.js b/app/routes/user/group.js index 6c6e5de36..07e88150a 100644 --- a/app/routes/user/group.js +++ b/app/routes/user/group.js @@ -1,112 +1,20 @@ -const HTTP_STATUS = require('http-status-codes'); const express = require('express'); - -const db = require('../../models'); -const logger = require('../../log'); - -const groupMiddleware = require('../../middleware/group'); +const {USER_GROUPS} = require('../../constants'); const router = express.Router({mergeParams: true}); -const schemaGenerator = require('../../schemas/schemaGenerator'); -const validateAgainstSchema = require('../../libs/validateAgainstSchema'); -const {BASE_EXCLUDE} = require('../../schemas/exclude'); - -// Generate schema's -const groupCreateSchema = schemaGenerator(db.models.userGroup, { - baseUri: '/create', - exclude: [...BASE_EXCLUDE], - nothingRequired: true, -}); - -const groupUpdateSchema = schemaGenerator(db.models.userGroup, { - baseUri: '/update', - exclude: [...BASE_EXCLUDE], - nothingRequired: true, -}); - -// Register group middleware -router.param('group', groupMiddleware); - -// Routes for operating on a single user group -router.route('/:group([A-z0-9-]{36})') - .get((req, res) => { - return res.json(req.group.view('public')); - }) - .put(async (req, res) => { - try { - // Validate input - validateAgainstSchema(groupUpdateSchema, req.body, false); - } catch (error) { - const message = `There was an error validating the user group update request ${error}`; - logger.error(message); - return res.status(HTTP_STATUS.BAD_REQUEST).json({error: {message}}); - } - - // Update Group - try { - await req.group.update(req.body, {userId: req.user.id}); - await req.group.reload(); - return res.json(req.group.view('public')); - } catch (error) { - logger.error(`Error while updating user group ${error}`); - return res.status(HTTP_STATUS.INTERNAL_SERVER_ERROR).json({ - error: {message: 'Error while updating user group'}, - }); - } - }) - - .delete(async (req, res) => { - try { - await req.group.destroy(); - return res.status(HTTP_STATUS.NO_CONTENT).send(); - } catch (error) { - logger.error(`Error while trying to remove user group ${error}`); - return res.status(HTTP_STATUS.INTERNAL_SERVER_ERROR).json({ - error: {message: 'Error while trying to remove user group'}, - }); - } - }); - // Routes for operating on all user groups router.route('/') // Get all user groups .get(async (req, res) => { - try { - const groups = await db.models.userGroup.scope('public').findAll(); - return res.json(groups); - } catch (error) { - logger.error(`Error while getting all user groups ${error}`); - return res.status(HTTP_STATUS.INTERNAL_SERVER_ERROR).json({ - error: {message: 'Error while getting all user groups'}, - }); - } - }) - .post(async (req, res) => { - try { - // Validate input - validateAgainstSchema(groupCreateSchema, req.body); - } catch (error) { - const message = `There was an error validating the user group create request ${error}`; - logger.error(message); - return res.status(HTTP_STATUS.BAD_REQUEST).json({error: {message}}); - } - - try { - // Add new group - const newGroup = await db.models.userGroup.create(req.body); + // Format return to be as close as possible to the previous version + const returnArray = []; - const result = await db.models.userGroup.scope('public').findOne({ - where: {ident: newGroup.ident}, - }); - - return res.status(HTTP_STATUS.CREATED).json(result); - } catch (error) { - logger.error(`Error while creating new user group ${error}`); - return res.status(HTTP_STATUS.INTERNAL_SERVER_ERROR).json({ - error: {message: 'Error while creating new user group'}, - }); + for (const group of USER_GROUPS) { + returnArray.push({name: group}); } + + return res.json(returnArray); }); module.exports = router; diff --git a/app/routes/user/member.js b/app/routes/user/member.js index 1052aa296..2520e3756 100644 --- a/app/routes/user/member.js +++ b/app/routes/user/member.js @@ -11,9 +11,9 @@ const validateAgainstSchema = require('../../libs/validateAgainstSchema'); const {BASE_EXCLUDE} = require('../../schemas/exclude'); // Generate schema -const memberSchema = schemaGenerator(db.models.userGroupMember, { +const memberSchema = schemaGenerator(db.models.userGroup, { baseUri: '/create-delete', - exclude: [...BASE_EXCLUDE, 'user_id', 'group_id'], + exclude: [...BASE_EXCLUDE, 'userId', 'name'], properties: { user: {type: 'string', format: 'uuid'}, }, @@ -45,9 +45,8 @@ router.route('/') model: db.models.userGroup, as: 'groups', attributes: { - exclude: ['id', 'deletedAt', 'updatedAt', 'createdAt', 'updatedBy'], + exclude: ['id', 'userId', 'deletedAt', 'updatedAt', 'createdAt', 'updatedBy'], }, - through: {attributes: []}, }, ], }); @@ -81,11 +80,18 @@ router.route('/') try { // Add user to group - await db.models.userGroupMember.create({group_id: req.group.id, user_id: user.id}); + await db.models.userGroup.create({name: req.group.name, userId: user.id}); await user.reload(); return res.status(HTTP_STATUS.CREATED).json(user.view('public')); } catch (error) { logger.error(`Error while trying to add user to group ${error}`); + + if (`${error}` === 'SequelizeUniqueConstraintError: Validation error') { + return res.status(HTTP_STATUS.BAD_REQUEST).json({ + error: {message: 'Error while trying to add user to group: User already in group'}, + }); + } + return res.status(HTTP_STATUS.INTERNAL_SERVER_ERROR).json({ error: {message: 'Error while trying to add user to group'}, }); @@ -129,11 +135,8 @@ router.route('/') return res.status(HTTP_STATUS.FORBIDDEN).json({error: {message: msg}}); } - const adminGroup = await db.models.userGroup.findOne({ - where: {name: 'admin'}, - }); - const subjectUserIsAdmin = await db.models.userGroupMember.findOne({ - where: {group_id: adminGroup.id, user_id: user.id}, + const subjectUserIsAdmin = await db.models.userGroup.findOne({ + where: {name: 'admin', userId: user.id}, }); if (!reqUserIsAdmin && subjectUserIsAdmin) { @@ -143,8 +146,8 @@ router.route('/') } try { // Find membership - const membership = await db.models.userGroupMember.findOne({ - where: {group_id: req.group.id, user_id: user.id}, + const membership = await db.models.userGroup.findOne({ + where: {name: req.group.name, userId: user.id}, }); if (!membership) { diff --git a/app/routes/user/user.js b/app/routes/user/user.js index 067a95583..56aa99ce7 100644 --- a/app/routes/user/user.js +++ b/app/routes/user/user.js @@ -26,9 +26,8 @@ router.param('userByIdent', async (req, res, next, ident) => { model: db.models.userGroup, as: 'groups', attributes: { - exclude: ['id', 'deletedAt', 'updatedAt', 'createdAt', 'updatedBy'], + exclude: ['id', 'deletedAt', 'updatedAt', 'createdAt', 'updatedBy', 'userId'], }, - through: {attributes: []}, }, { model: db.models.project, @@ -102,11 +101,8 @@ router.route('/:userByIdent([A-z0-9-]{36})') return res.json(req.userByIdent.view('public')); }) .put(async (req, res) => { - const adminGroup = await db.models.userGroup.findOne({ - where: {name: 'admin'}, - }); - const subjectUserIsAdmin = await db.models.userGroupMember.findOne({ - where: {group_id: adminGroup.id, user_id: req.userByIdent.id}, + const subjectUserIsAdmin = await db.models.userGroup.findOne({ + where: {name: 'admin', userId: req.userByIdent.id}, }); if (!isAdmin(req.user) && subjectUserIsAdmin) { @@ -165,11 +161,8 @@ router.route('/:userByIdent([A-z0-9-]{36})') } }) .delete(async (req, res) => { - const adminGroup = await db.models.userGroup.findOne({ - where: {name: 'admin'}, - }); - const subjectUserIsAdmin = await db.models.userGroupMember.findOne({ - where: {group_id: adminGroup.id, user_id: req.userByIdent.id}, + const subjectUserIsAdmin = await db.models.userGroup.findOne({ + where: {name: 'admin', userId: req.userByIdent.id}, }); if (!isAdmin(req.user) && subjectUserIsAdmin) { @@ -200,8 +193,7 @@ router.route('/') { as: 'groups', model: db.models.userGroup, - attributes: {exclude: ['id', 'user_id', 'deletedAt', 'updatedAt', 'createdAt', 'updatedBy']}, - through: {attributes: []}, + attributes: {exclude: ['id', 'userId', 'deletedAt', 'updatedAt', 'createdAt', 'updatedBy']}, }, { as: 'projects', diff --git a/migrations/20240725223946-DEVSU-2384-permission-storage-update.js b/migrations/20240725223946-DEVSU-2384-permission-storage-update.js new file mode 100644 index 000000000..534077857 --- /dev/null +++ b/migrations/20240725223946-DEVSU-2384-permission-storage-update.js @@ -0,0 +1,81 @@ +const NEW_TABLE = 'user_groups'; +const OLD_TABLE = 'user_groups_old'; +const {DEFAULT_COLUMNS} = require('../app/models/base'); + +module.exports = { + up: (queryInterface, Sq) => { + // Create new variant text table + return queryInterface.sequelize.transaction(async (transaction) => { + queryInterface.renameTable(NEW_TABLE, OLD_TABLE, {transaction}); + + console.log('Creating new table'); + + await queryInterface.createTable(NEW_TABLE, { + ...DEFAULT_COLUMNS, + user_id: { + allowNull: false, + name: 'userId', + field: 'user_id', + type: Sq.INTEGER, + references: { + model: 'users', + key: 'id', + }, + }, + name: { + type: Sq.ENUM( + 'admin', + 'manager', + 'report assignment access', + 'create report access', + 'germline access', + 'non-production access', + 'unreviewed access', + 'all projects access', + 'template edit access', + 'appendix edit access', + 'variant-text edit access', + ), + allowNull: false, + }, + }, {transaction}); + + await queryInterface.sequelize + .query( + ` + create function cast_to_text(enum_user_groups_name) returns text as + $$ select $1::text; $$ + language sql cost 1 immutable;`, + { + transaction, + }, + ); + + await queryInterface.sequelize.query( + 'CREATE UNIQUE INDEX groups_unique ON user_groups ((ARRAY[cast_to_text("name"), user_id::text])) where deleted_at is null;', + { + transaction, + }, + ); + + console.log('Migrating data'); + + await queryInterface.sequelize.query( + // eslint-disable-next-line no-multi-str + `insert into user_groups + (ident, created_at, updated_at, user_id, "name") + select gen_random_uuid(), now(), now(), user_id, LOWER(ug.name)::enum_user_groups_name from user_group_members ugm + join user_groups_old ug on (ugm.group_id = ug.id) + where ugm.deleted_at is null;`, + { + type: queryInterface.sequelize.QueryTypes.SELECT, + transaction, + }, + ); + }); + }, + + down: () => { + throw new Error('Not Implemented!'); + }, +}; diff --git a/migrations/20240814165504-DEVSU-2400-remove-owner-column.js b/migrations/20240814165504-DEVSU-2400-remove-owner-column.js deleted file mode 100644 index e3ac42284..000000000 --- a/migrations/20240814165504-DEVSU-2400-remove-owner-column.js +++ /dev/null @@ -1,17 +0,0 @@ -const TABLE = 'user_groups'; - -module.exports = { - up: async (queryInterface) => { - return queryInterface.sequelize.transaction(async (transaction) => { - await queryInterface.removeColumn( - TABLE, - 'owner_id', - {transaction}, - ); - }); - }, - - down: async () => { - throw new Error('Not Implemented!'); - }, -}; diff --git a/migrations/20240828195736-DEVSU-2384-permission-storage-update.js b/migrations/20240828195736-DEVSU-2384-permission-storage-update.js new file mode 100644 index 000000000..3c4f3f4a2 --- /dev/null +++ b/migrations/20240828195736-DEVSU-2384-permission-storage-update.js @@ -0,0 +1,27 @@ +const TABLE = 'notifications'; +const {USER_GROUPS} = require('../app/constants'); + +module.exports = { + up: async (queryInterface, Sq) => { + return queryInterface.sequelize.transaction(async (transaction) => { + await queryInterface.removeColumn( + TABLE, + 'user_group_id', + {transaction}, + ); + + await queryInterface.addColumn( + TABLE, + 'user_group', + { + type: Sq.ENUM(USER_GROUPS), + }, + {transaction}, + ); + }); + }, + + down: async () => { + throw new Error('Not Implemented!'); + }, +}; diff --git a/test/routes/notification/notifications.test.js b/test/routes/notification/notifications.test.js index 936018369..158bf43c1 100644 --- a/test/routes/notification/notifications.test.js +++ b/test/routes/notification/notifications.test.js @@ -15,7 +15,7 @@ const punProperties = [ 'ident', 'createdAt', 'updatedAt', 'eventType', 'user', 'userGroup', 'project', 'template', ]; -const punObjectProperties = ['userGroup', 'project', 'template']; +const punObjectProperties = ['project', 'template']; const punUserProperties = [ 'ident', 'username', ]; @@ -57,6 +57,8 @@ beforeAll(async () => { // Tests for project user endpoints describe('/notification/notifications', () => { + const userGroup1 = 'create report access'; + const userGroup2 = 'germline access'; let testUser; let project; let project2; @@ -71,8 +73,6 @@ describe('/notification/notifications', () => { let binding1; let binding2; let binding3; - let userGroup1; - let userGroup2; beforeAll(async () => { // get test user @@ -112,16 +112,6 @@ describe('/notification/notifications', () => { binding2 = await db.models.userProject.create({project_id: project.id, user_id: user01.id}); binding3 = await db.models.userProject.create({project_id: project2.id, user_id: user01.id}); - // Create userGroups - userGroup1 = await db.models.userGroup.create({ - ident: uuidv4(), - name: uuidv4(), - }); - userGroup2 = await db.models.userGroup.create({ - ident: uuidv4(), - name: uuidv4(), - }); - pun1 = await db.models.notification.create({ ident: uuidv4(), projectId: project.id, @@ -149,7 +139,7 @@ describe('/notification/notifications', () => { pun4 = await db.models.notification.create({ ident: uuidv4(), projectId: project.id, - userGroupId: userGroup1.id, + userGroup: userGroup1, templateId: template.id, eventType: 'test event 1', }); @@ -157,7 +147,7 @@ describe('/notification/notifications', () => { pun5 = await db.models.notification.create({ ident: uuidv4(), projectId: project.id, - userGroupId: userGroup2.id, + userGroup: userGroup2, templateId: template.id, eventType: 'test event 2', }); @@ -165,7 +155,7 @@ describe('/notification/notifications', () => { pun6 = await db.models.notification.create({ ident: uuidv4(), projectId: project2.id, - userGroupId: userGroup1.id, + userGroup: userGroup1, templateId: template.id, eventType: 'test event 3', }); @@ -183,8 +173,6 @@ describe('/notification/notifications', () => { binding1.destroy({force: true}), binding2.destroy({force: true}), binding3.destroy({force: true}), - userGroup1.destroy({force: true}), - userGroup2.destroy({force: true}), pun4.destroy({force: true}), pun5.destroy({force: true}), pun6.destroy({force: true}), @@ -213,7 +201,7 @@ describe('/notification/notifications', () => { .get('/api/notification/notifications') .auth(username, password) .type('json') - .send({user_group: userGroup1.ident}) + .send({user_group: userGroup1}) .expect(HTTP_STATUS.OK); expect(Array.isArray(res.body)).toBe(true); @@ -252,7 +240,7 @@ describe('/notification/notifications', () => { .get('/api/notification/notifications') .auth(username, password) .type('json') - .send({user_group: userGroup1.ident, project: project.ident}) + .send({user_group: userGroup1, project: project.ident}) .expect(HTTP_STATUS.OK); expect(Array.isArray(res.body)).toBe(true); @@ -316,12 +304,12 @@ describe('/notification/notifications', () => { .post('/api/notification/notifications') .auth(username, password) .type('json') - .send({user_group: userGroup1.ident, project: project.ident, event_type: 'test event 5', template: template.ident}) + .send({user_group: userGroup1, project: project.ident, event_type: 'test event 5', template: template.ident}) .expect(HTTP_STATUS.CREATED); // Check the binding was created const result = await db.models.notification.findOne({ - where: {project_id: project.id, user_group_id: userGroup1.id, event_type: 'test event 5'}, + where: {project_id: project.id, user_group: userGroup1, event_type: 'test event 5'}, }); expect(result).not.toBeNull(); diff --git a/test/routes/user/group.test.js b/test/routes/user/group.test.js index 99ab1b8c2..b2d4cb832 100644 --- a/test/routes/user/group.test.js +++ b/test/routes/user/group.test.js @@ -12,7 +12,7 @@ CONFIG.set('env', 'test'); const {username, password} = CONFIG.get('testing'); const groupProperties = [ - 'ident', 'createdAt', 'updatedAt', 'name', 'users', + 'name', ]; const checkUserGroup = (groupObject) => { @@ -22,7 +22,6 @@ const checkUserGroup = (groupObject) => { expect(groupObject).toEqual(expect.not.objectContaining({ id: expect.any(Number), deletedAt: expect.any(String), - description: expect.any(String), })); }; @@ -36,9 +35,7 @@ let server; let request; // Variables to be used for group tests -let user01; -let user02; -let group; +let user; // Start API beforeAll(async () => { @@ -47,7 +44,7 @@ beforeAll(async () => { request = supertest(server); // make sure there are at least 2 users - user01 = await db.models.user.create({ + user = await db.models.user.create({ ident: uuidv4(), username: uuidv4(), firstName: 'firstName01', @@ -55,15 +52,10 @@ beforeAll(async () => { email: 'email01@email.com', }); - user02 = await db.models.user.create({ - ident: uuidv4(), - username: uuidv4(), - firstName: 'firstName02', - lastName: 'lastName02', - email: 'email02@email.com', + await db.models.userGroup.create({ + userId: user.id, + name: 'germline access', }); - - group = await db.models.userGroup.create({name: 'Test group', description: 'test'}); }); // Tests for user group related endpoints @@ -79,111 +71,11 @@ describe('/user/group', () => { checkUserGroups(res.body); }); - - test('/{group} - 200 Success', async () => { - await request - .get(`/api/user/group/${group.ident}`) - .auth(username, password) - .type('json') - .expect(HTTP_STATUS.OK); - }); - - test('/{group} - 404 Not Found', async () => { - await request - .get(`/api/user/group/${uuidv4()}`) - .auth(username, password) - .type('json') - .expect(HTTP_STATUS.NOT_FOUND); - }); - }); - - // Tests for POST endpoint - describe('POST', () => { - test('/ - 200 Success', async () => { - const res = await request - .post('/api/user/group') - .auth(username, password) - .type('json') - .send({name: 'testGroup'}) - .expect(HTTP_STATUS.CREATED); - - checkUserGroup(res.body); - - await db.models.userGroup.destroy({where: {ident: res.body.ident}, force: true}); - }); - }); - - // Tests for PUT endpoint - describe('PUT', () => { - let updateGroup; - - beforeEach(async () => { - updateGroup = await db.models.userGroup.create({ - name: 'updateTestGroup', - }); - }); - - afterEach(async () => { - return db.models.userGroup.destroy({where: {ident: updateGroup.ident}, force: true}); - }); - - test('/{group} - 200 Success', async () => { - const UPDATE_DATA = {name: 'testGroupUpdated'}; - - const res = await request - .put(`/api/user/group/${updateGroup.ident}`) - .auth(username, password) - .type('json') - .send(UPDATE_DATA) - .expect(HTTP_STATUS.OK); - - checkUserGroup(res.body); - expect(res.body.name).toBe(UPDATE_DATA.name); - }); - - test('/{group} - 404 Not Found', async () => { - await request - .put(`/api/user/group/${uuidv4()}`) - .auth(username, password) - .type('json') - .send({name: 'testGroup'}) - .expect(HTTP_STATUS.NOT_FOUND); - }); - }); - - // Tests for DELETE endpoint - describe('DELETE', () => { - let deleteGroup; - - beforeEach(async () => { - deleteGroup = await db.models.userGroup.create({ - name: 'testGroup', - }); - }); - - afterEach(async () => { - return db.models.userGroup.destroy({where: {ident: deleteGroup.ident}, force: true}); - }); - - test('/{group} - 204 Success', async () => { - await request - .delete(`/api/user/group/${deleteGroup.ident}`) - .auth(username, password) - .type('json') - .expect(HTTP_STATUS.NO_CONTENT); - - // Verify group is soft-deleted - const delGroup = await db.models.userGroup.findOne({ - where: {ident: deleteGroup.ident}, paranoid: false, - }); - expect(delGroup.deletedAt).not.toBeNull(); - }); }); }); afterAll(async () => { // Delete group and users - await db.models.userGroup.destroy({where: {ident: group.ident}, force: true}); - await db.models.user.destroy({where: {ident: [user01.ident, user02.ident]}, force: true}); + await db.models.user.destroy({where: {ident: user.ident}}); await server.close(); }); diff --git a/test/routes/user/member.test.js b/test/routes/user/member.test.js index 7bd8bc4cc..9559ef35a 100644 --- a/test/routes/user/member.test.js +++ b/test/routes/user/member.test.js @@ -67,15 +67,12 @@ beforeAll(async () => { email: 'email02@email.com', }); - // Create groups - group = await db.models.userGroup.create({name: 'Test group'}); - adminGroup = await db.models.userGroup.findOne({ - where: {name: 'admin'}, - }); + group = {name: 'germline access'}; + adminGroup = {name: 'admin'}; // Make users group members - await db.models.userGroupMember.create({user_id: user01.id, group_id: group.id}); - await db.models.userGroupMember.create({user_id: user02.id, group_id: group.id}); + await db.models.userGroup.create({userId: user01.id, name: group.name}); + await db.models.userGroup.create({userId: user02.id, name: group.name}); }); // Tests for user group member related endpoints @@ -84,7 +81,7 @@ describe('/user/group/{group}/member', () => { describe('GET', () => { test('/ - 200 Success', async () => { const res = await request - .get(`/api/user/group/${group.ident}/member`) + .get(`/api/user/group/${group.name}/member`) .auth(username, password) .type('json') .expect(HTTP_STATUS.OK); @@ -106,7 +103,7 @@ describe('/user/group/{group}/member', () => { }); const res = await request - .post(`/api/user/group/${group.ident}/member`) + .post(`/api/user/group/${group.name}/member`) .auth(username, password) .type('json') .send({user: updateUser.ident}) @@ -115,13 +112,13 @@ describe('/user/group/{group}/member', () => { checkUser(res.body); // Check user belongs to group - const groupIdents = res.body.groups.map((g) => { - return g.ident; + const groupNames = res.body.groups.map((g) => { + return g.name; }); - expect(groupIdents.includes(group.ident)).toBe(true); + expect(groupNames.includes(group.name)).toBe(true); // Remove user - await db.models.user.destroy({where: {ident: updateUser.ident}, force: true}); + await db.models.user.destroy({where: {ident: updateUser.ident}}); }); test('/ - 200 Success by manager', async () => { @@ -135,7 +132,7 @@ describe('/user/group/{group}/member', () => { }); const res = await request - .post(`/api/user/group/${group.ident}/member`) + .post(`/api/user/group/${group.name}/member`) .auth(managerUsername, password) .type('json') .send({user: updateUser.ident}) @@ -144,13 +141,13 @@ describe('/user/group/{group}/member', () => { checkUser(res.body); // Check user belongs to group - const groupIdents = res.body.groups.map((g) => { - return g.ident; + const groupNames = res.body.groups.map((g) => { + return g.name; }); - expect(groupIdents.includes(group.ident)).toBe(true); + expect(groupNames.includes(group.name)).toBe(true); // Remove user - await db.models.user.destroy({where: {ident: updateUser.ident}, force: true}); + await db.models.user.destroy({where: {ident: updateUser.ident}}); }); test('/ - 403 forbidden to non-admin to give admin role', async () => { @@ -164,14 +161,14 @@ describe('/user/group/{group}/member', () => { }); await request - .post(`/api/user/group/${adminGroup.ident}/member`) + .post(`/api/user/group/${adminGroup.name}/member`) .auth(managerUsername, password) .type('json') .send({user: updateUser.ident}) .expect(HTTP_STATUS.FORBIDDEN); // Remove user - await db.models.user.destroy({where: {ident: updateUser.ident}, force: true}); + await db.models.user.destroy({where: {ident: updateUser.ident}}); }); test('/ - 403 forbidden to non-admin to give role to admin user', async () => { @@ -184,16 +181,16 @@ describe('/user/group/{group}/member', () => { email: 'updateUser2@email.com', }); - await db.models.userGroupMember.create({user_id: updateUser.id, group_id: adminGroup.id}); + await db.models.userGroup.create({userId: updateUser.id, name: adminGroup.name}); await request - .post(`/api/user/group/${group.ident}/member`) + .post(`/api/user/group/${group.name}/member`) .auth(managerUsername, password) .type('json') .send({user: updateUser.ident}) .expect(HTTP_STATUS.FORBIDDEN); // Remove user - await db.models.user.destroy({where: {ident: updateUser.ident}, force: true}); + await db.models.user.destroy({where: {ident: updateUser.ident}}); }); test('/ - 403 forbidden to bioinformatician', async () => { @@ -207,19 +204,19 @@ describe('/user/group/{group}/member', () => { }); await request - .post(`/api/user/group/${group.ident}/member`) + .post(`/api/user/group/${group.name}/member`) .auth(bioinformaticianUsername, password) .type('json') .send({user: updateUser.ident}) .expect(HTTP_STATUS.FORBIDDEN); // Remove user - await db.models.user.destroy({where: {ident: updateUser.ident}, force: true}); + await db.models.user.destroy({where: {ident: updateUser.ident}}); }); test('/ - 400 Bad Request - User is required', async () => { await request - .post(`/api/user/group/${group.ident}/member`) + .post(`/api/user/group/${group.name}/member`) .auth(username, password) .type('json') .send({}) @@ -228,7 +225,7 @@ describe('/user/group/{group}/member', () => { test('/ - 400 Bad Request - User must be a uuid', async () => { await request - .post(`/api/user/group/${group.ident}/member`) + .post(`/api/user/group/${group.name}/member`) .auth(username, password) .type('json') .send({user: 'NOT_UUID'}) @@ -237,7 +234,7 @@ describe('/user/group/{group}/member', () => { test('/ - 404 Not Found - User does not exist', async () => { await request - .post(`/api/user/group/${group.ident}/member`) + .post(`/api/user/group/${group.name}/member`) .auth(username, password) .type('json') .send({user: uuidv4()}) @@ -276,59 +273,59 @@ describe('/user/group/{group}/member', () => { email: 'deleteUser1@email.com', }); - await db.models.userGroupMember.create({ - user_id: deleteUser.id, group_id: group.id, + await db.models.userGroup.create({ + userId: deleteUser.id, name: group.name, }); - await db.models.userGroupMember.create({ - user_id: deleteUser2.id, group_id: group.id, + await db.models.userGroup.create({ + userId: deleteUser2.id, name: group.name, }); - await db.models.userGroupMember.create({ - user_id: adminDeleteUser.id, group_id: group.id, + await db.models.userGroup.create({ + userId: adminDeleteUser.id, name: group.name, }); - await db.models.userGroupMember.create({ - user_id: adminDeleteUser.id, group_id: adminGroup.id, + await db.models.userGroup.create({ + userId: adminDeleteUser.id, name: adminGroup.name, }); }); afterEach(async () => { - await db.models.user.destroy({where: {ident: deleteUser.ident}, force: true}); - await db.models.user.destroy({where: {ident: deleteUser2.ident}, force: true}); - await db.models.user.destroy({where: {ident: adminDeleteUser.ident}, force: true}); + await db.models.user.destroy({where: {ident: deleteUser.ident}}); + await db.models.user.destroy({where: {ident: deleteUser2.ident}}); + await db.models.user.destroy({where: {ident: adminDeleteUser.ident}}); }); test('/ - 204 Success', async () => { await request - .delete(`/api/user/group/${group.ident}/member`) + .delete(`/api/user/group/${group.name}/member`) .auth(username, password) .type('json') .send({user: deleteUser.ident}) .expect(HTTP_STATUS.NO_CONTENT); // Verify group-member association is deleted - const delGroupMember = await db.models.userGroupMember.findOne({ - where: {user_id: deleteUser.id, group_id: group.id}, paranoid: false, + const delGroupMember = await db.models.userGroup.findOne({ + where: {userId: deleteUser.id, name: group.name}, paranoid: false, }); expect(delGroupMember.deletedAt).not.toBeNull(); }); test('/ - 204 Success by manager', async () => { await request - .delete(`/api/user/group/${group.ident}/member`) + .delete(`/api/user/group/${group.name}/member`) .auth(managerUsername, password) .type('json') .send({user: deleteUser2.ident}) .expect(HTTP_STATUS.NO_CONTENT); // Verify group-member association is deleted - const delGroupMember = await db.models.userGroupMember.findOne({ - where: {user_id: deleteUser2.id, group_id: group.id}, paranoid: false, + const delGroupMember = await db.models.userGroup.findOne({ + where: {userId: deleteUser2.id, name: group.name}, paranoid: false, }); expect(delGroupMember.deletedAt).not.toBeNull(); }); test('/ - 403 forbidden to non-admin to remove admin role', async () => { await request - .delete(`/api/user/group/${adminGroup.ident}/member`) + .delete(`/api/user/group/${adminGroup.name}/member`) .auth(managerUsername, password) .type('json') .send({user: adminDeleteUser.ident}) @@ -337,7 +334,7 @@ describe('/user/group/{group}/member', () => { test('/ - 403 forbidden to non-admin to remove any role from admin user', async () => { await request - .delete(`/api/user/group/${group.ident}/member`) + .delete(`/api/user/group/${group.name}/member`) .auth(managerUsername, password) .type('json') .send({user: adminDeleteUser.ident}) @@ -346,7 +343,7 @@ describe('/user/group/{group}/member', () => { test('/ - 403 forbidden to bioinformatician', async () => { await request - .delete(`/api/user/group/${group.ident}/member`) + .delete(`/api/user/group/${group.name}/member`) .auth(bioinformaticianUsername, password) .type('json') .send({user: deleteUser.ident}) @@ -355,7 +352,7 @@ describe('/user/group/{group}/member', () => { test('/ - 400 Bad Request', async () => { await request - .delete(`/api/user/group/${group.ident}/member`) + .delete(`/api/user/group/${group.name}/member`) .auth(username, password) .type('json') .send({user: 'INVALID_UUID'}) @@ -364,7 +361,7 @@ describe('/user/group/{group}/member', () => { test('/ - 404 Not Found - User does not exist', async () => { await request - .delete(`/api/user/group/${group.ident}/member`) + .delete(`/api/user/group/${group.name}/member`) .auth(username, password) .type('json') .send({user: uuidv4()}) @@ -373,12 +370,12 @@ describe('/user/group/{group}/member', () => { test('/ - 404 Not Found - User is not group member', async () => { // Delete group-member binding - await db.models.userGroupMember.destroy({ - where: {user_id: deleteUser.id, group_id: group.id}, + await db.models.userGroup.destroy({ + where: {userId: deleteUser.id, name: group.name}, }); await request - .delete(`/api/user/group/${group.ident}/member`) + .delete(`/api/user/group/${group.name}/member`) .auth(username, password) .type('json') .send({user: deleteUser.ident}) @@ -388,7 +385,6 @@ describe('/user/group/{group}/member', () => { }); afterAll(async () => { - await db.models.userGroup.destroy({where: {ident: group.ident}, force: true}); - await db.models.user.destroy({where: {ident: [user01.ident, user02.ident]}, force: true}); + await db.models.user.destroy({where: {ident: [user01.ident, user02.ident]}}); await server.close(); }); diff --git a/test/routes/user/user.test.js b/test/routes/user/user.test.js index 4d2d6ba04..b9f40a28d 100644 --- a/test/routes/user/user.test.js +++ b/test/routes/user/user.test.js @@ -46,16 +46,12 @@ beforeAll(async () => { // Tests for user related endpoints describe('/user', () => { let testUser; - let adminGroup; beforeAll(async () => { // get test user testUser = await db.models.user.findOne({ where: {username}, }); - adminGroup = await db.models.userGroup.findOne({ - where: {name: 'admin'}, - }); }); describe('GET', () => { @@ -143,7 +139,7 @@ describe('/user', () => { expect(res.body[0].firstName).toBe(firstName); // Delete user - await db.models.user.destroy({where: {ident: searchUser.ident}, force: true}); + await db.models.user.destroy({where: {ident: searchUser.ident}}); }); }); @@ -163,7 +159,7 @@ describe('/user', () => { .expect(HTTP_STATUS.CREATED); // Remove test user from db - await db.models.user.destroy({where: {ident: res.body.ident}, force: true}); + await db.models.user.destroy({where: {ident: res.body.ident}}); }); test('/ - 200 Success by manager', async () => { @@ -181,7 +177,7 @@ describe('/user', () => { .expect(HTTP_STATUS.CREATED); // Remove test user from db - await db.models.user.destroy({where: {ident: res.body.ident}, force: true}); + await db.models.user.destroy({where: {ident: res.body.ident}}); }); test('/ - 403 forbidden for bioinformatician', async () => { @@ -309,14 +305,14 @@ describe('/user', () => { lastName: 'putTestLastName', }); - await db.models.userGroupMember.create({ - user_id: adminPutTestUser.id, group_id: adminGroup.id, + await db.models.userGroup.create({ + userId: adminPutTestUser.id, name: 'admin', }); }); afterEach(async () => { - await db.models.user.destroy({where: {ident: putTestUser.ident}, force: true}); - await db.models.user.destroy({where: {ident: adminPutTestUser.ident}, force: true}); + await db.models.user.destroy({where: {ident: putTestUser.ident}}); + await db.models.user.destroy({where: {ident: adminPutTestUser.ident}}); }); test('/{user} - 200 Success', async () => { @@ -404,16 +400,16 @@ describe('/user', () => { lastName: 'delete3LastName01', email: 'delete3@email.com', }); - await db.models.userGroupMember.create({ - user_id: deleteTestUserWhoIsAdmin.id, group_id: adminGroup.id, + await db.models.userGroup.create({ + userId: deleteTestUserWhoIsAdmin.id, name: 'admin', }); }); afterEach(async () => { - await db.models.user.destroy({where: {ident: deleteTestUser.ident}, force: true}); - await db.models.user.destroy({where: {ident: deleteTestUserManager.ident}, force: true}); - await db.models.user.destroy({where: {ident: deleteTestUserForbidden.ident}, force: true}); - await db.models.user.destroy({where: {ident: deleteTestUserWhoIsAdmin.ident}, force: true}); + await db.models.user.destroy({where: {ident: deleteTestUser.ident}}); + await db.models.user.destroy({where: {ident: deleteTestUserManager.ident}}); + await db.models.user.destroy({where: {ident: deleteTestUserForbidden.ident}}); + await db.models.user.destroy({where: {ident: deleteTestUserWhoIsAdmin.ident}}); }); test('/{user} - 204 Success', async () => { diff --git a/test/testSetup.js b/test/testSetup.js index 6a899f89f..2666099a6 100644 --- a/test/testSetup.js +++ b/test/testSetup.js @@ -15,17 +15,11 @@ beforeAll(async () => { }, }); - const [managerGroup] = await db.models.userGroup.findOrCreate({ - where: { - name: 'manager', - }, + await db.models.userGroup.findOrCreate({ + where: {userId: managerUser.id, name: 'manager'}, }); - await db.models.userGroupMember.findOrCreate({ - where: {user_id: managerUser.id, group_id: managerGroup.id}, - }); - - const [bioinformaticianUser] = await db.models.user.findOrCreate({ + await db.models.user.findOrCreate({ where: { username: bioinformaticianUsername, firstName: bioinformaticianUsername, @@ -33,14 +27,4 @@ beforeAll(async () => { email: 'ipr@bcgsc.ca', }, }); - - const [bioinformaticianGroup] = await db.models.userGroup.findOrCreate({ - where: { - name: 'Bioinformatician', - }, - }); - - await db.models.userGroupMember.findOrCreate({ - where: {user_id: bioinformaticianUser.id, group_id: bioinformaticianGroup.id}, - }); });