Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/DEVSU-2384-permission-storage-improvement #381

Merged
merged 26 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
5293d86
Add migration
Nithriel Jul 29, 2024
36d8933
Merge branch 'feature/DEVSU-2400-remove-owner-from-groups-table' into…
Nithriel Aug 21, 2024
0025efd
Remove mentions of userGroupMembers and fix /user/me endpoint
Nithriel Aug 21, 2024
fa56457
Fix user endpoint
Nithriel Aug 21, 2024
6e8585f
Fix typo
Nithriel Aug 21, 2024
832a1a2
rename group to name
Nithriel Aug 21, 2024
b7cee43
Implement new group handling improvements
Nithriel Aug 27, 2024
43eb720
Apply permission improvement to tests
Nithriel Aug 27, 2024
3ae8744
Update swagger
Nithriel Aug 27, 2024
e48a67a
Move out of transaction
Nithriel Aug 27, 2024
469c3f6
Move back to transaction
Nithriel Aug 27, 2024
d4eab2d
fix table name
Nithriel Aug 27, 2024
73755e4
Merge branch 'develop' into feature/DEVSU-2384-permission-storage-imp…
Nithriel Aug 27, 2024
6e5e915
Remove not needed migration
Nithriel Aug 27, 2024
d4af7a2
Fix user test
Nithriel Aug 27, 2024
ca44fdc
Fix group tests
Nithriel Aug 27, 2024
57c1f42
Update notifications
Nithriel Aug 28, 2024
2f184df
Remove debugging code
Nithriel Aug 28, 2024
96c51e1
Fix notification
Nithriel Aug 28, 2024
54b1f51
comment out force
Nithriel Aug 28, 2024
a1de98c
Remove force true
Nithriel Aug 28, 2024
56617e2
Merge branch 'develop' into feature/DEVSU-2384-permission-storage-imp…
bnguyen-bcgsc Aug 29, 2024
61246bc
Merge branch 'develop' into feature/DEVSU-2384-permission-storage-imp…
elewis2 Sep 20, 2024
11d6062
Remove table deletions
Nithriel Sep 24, 2024
478584c
Merge branch 'feature/DEVSU-2384-permission-storage-improvement' of g…
Nithriel Sep 24, 2024
e5e8de5
Merge branch 'develop' into feature/DEVSU-2384-permission-storage-imp…
Nithriel Oct 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions app/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
],
};
1 change: 0 additions & 1 deletion app/libs/__mocks__/getUser.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ const include = [
attributes: {
exclude: ['id', 'deletedAt', 'updatedAt', 'createdAt', 'updatedBy'],
},
through: {attributes: []},
},
{
model: db.models.project,
Expand Down
3 changes: 1 addition & 2 deletions app/libs/getUser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 5 additions & 6 deletions app/middleware/group.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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();
};
2 changes: 1 addition & 1 deletion app/models/clearCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const CLEAR_GERMLINE_CACHE_MODELS = [
];

const CLEAR_USER_CACHE_MODELS = [
'userGroup', 'userGroupMember', 'userProject',
'userGroup', 'userProject',
];

/**
Expand Down
14 changes: 2 additions & 12 deletions app/models/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
});
Expand All @@ -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);

Expand Down
17 changes: 6 additions & 11 deletions app/models/notification/notification.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const {DEFAULT_COLUMNS, DEFAULT_OPTIONS} = require('../base');
const {USER_GROUPS} = require('../../constants');

module.exports = (sequelize, Sq) => {
const notification = sequelize.define(
Expand Down Expand Up @@ -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,
Expand All @@ -62,19 +59,17 @@ 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'},
],
},
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'},
],
Expand Down
24 changes: 14 additions & 10 deletions app/models/user/userGroup.js
Original file line number Diff line number Diff line change
@@ -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'],
},
},
});

Expand Down
32 changes: 0 additions & 32 deletions app/models/user/userGroupMember.js

This file was deleted.

25 changes: 20 additions & 5 deletions app/routes/notification/notification.js
Original file line number Diff line number Diff line change
@@ -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');
Expand All @@ -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
Expand Down Expand Up @@ -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}),
};
Expand All @@ -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'},
});
Expand All @@ -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;
Expand Down Expand Up @@ -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,
},
Expand Down
4 changes: 2 additions & 2 deletions app/routes/swagger/schemas.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'];

Expand Down Expand Up @@ -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
Expand Down
Loading
Loading