From 5a0d499bf58cd00650bd74413513111edb21a708 Mon Sep 17 00:00:00 2001 From: bnguyen-bcgsc Date: Mon, 6 Jul 2026 16:23:11 -0700 Subject: [PATCH 1/8] - DEVSU-2864 - Update association between projects and variant texts to be many to many - Add new join table for many to many table - Update variant texts routes and unit tests to associate multiple projects per variant text record --- app/models/index.js | 9 +- app/models/project/project.js | 4 +- app/models/project/projectVariantTextJoin.js | 53 +++++ app/models/variantText/variantText.js | 17 +- app/routes/variantText/variantText.js | 186 ++++++++++++++---- ...864-projects-variant-texts-many-to-many.js | 59 ++++++ test/routes/variantText/variantText.test.js | 65 +++--- 7 files changed, 313 insertions(+), 80 deletions(-) create mode 100644 app/models/project/projectVariantTextJoin.js create mode 100644 migrations/latest/20260706182803-DEVSU-2864-projects-variant-texts-many-to-many.js diff --git a/app/models/index.js b/app/models/index.js index 1acc09fb6..24f1d3f0c 100644 --- a/app/models/index.js +++ b/app/models/index.js @@ -38,6 +38,7 @@ userMetadata.belongsTo(user, { // Projects const project = require('./project/project')(sequelize, Sq); +const projectVariantTextJoin = require('./project/projectVariantTextJoin')(sequelize, Sq); const userProject = require('./project/userProject')(sequelize, Sq); const reportProject = require('./project/reportProject')(sequelize, Sq); @@ -610,14 +611,14 @@ const variantText = require('./variantText/variantText')(sequelize, Sq); template.hasMany(variantText, { as: 'variant_texts', foreignKey: 'templateId', onDelete: 'CASCADE', constraints: true, }); -project.hasMany(variantText, { - as: 'variant_texts', foreignKey: 'projectId', onDelete: 'CASCADE', constraints: true, +project.belongsToMany(variantText, { + as: 'variant_texts', through: {model: projectVariantTextJoin, unique: false}, foreignKey: 'projectId', onDelete: 'CASCADE', constraints: true, }); variantText.belongsTo(template, { as: 'template', foreignKey: 'templateId', targetKey: 'id', onDelete: 'CASCADE', constraints: true, }); -variantText.belongsTo(project, { - as: 'project', foreignKey: 'projectId', targetKey: 'id', onDelete: 'CASCADE', constraints: true, +variantText.belongsToMany(project, { + as: 'projects', through: {model: projectVariantTextJoin, unique: false}, foreignKey: 'variantTextId', onDelete: 'CASCADE', constraints: true, }); // Template Appendix diff --git a/app/models/project/project.js b/app/models/project/project.js index cec8f8bd1..37176a7bc 100644 --- a/app/models/project/project.js +++ b/app/models/project/project.js @@ -32,11 +32,11 @@ module.exports = (sequelize, Sq) => { scopes: { public: { attributes: { - exclude: ['id', 'deletedAt', 'updatedBy'], + exclude: ['deletedAt', 'updatedBy'], }, }, minimal: { - attributes: ['ident', 'name'], + attributes: ['id', 'ident', 'name'], }, }, }); diff --git a/app/models/project/projectVariantTextJoin.js b/app/models/project/projectVariantTextJoin.js new file mode 100644 index 000000000..e8bba1de2 --- /dev/null +++ b/app/models/project/projectVariantTextJoin.js @@ -0,0 +1,53 @@ +const {DEFAULT_MAPPING_COLUMNS, DEFAULT_MAPPING_OPTIONS} = require('../base'); + +module.exports = (sequelize, Sq) => { + return sequelize.define( + 'projectVariantTextJoin', + { + ...DEFAULT_MAPPING_COLUMNS, + projectId: { + name: 'projectId', + field: 'project_id', + type: Sq.INTEGER, + unique: false, + allowNull: false, + references: { + model: 'projects', + key: 'id', + }, + }, + variantTextId: { + name: 'variantTextId', + field: 'variant_text_id', + type: Sq.INTEGER, + unique: false, + allowNull: false, + references: { + model: 'variant_texts', + key: 'id', + }, + }, + }, + { + ...DEFAULT_MAPPING_OPTIONS, + tableName: 'project_variant_text_join', + scopes: { + public: { + attributes: { + exclude: ['id', 'deletedAt'], + }, + }, + }, + indexes: [ + { + name: 'idx_project_id_join', + fields: ['project_id'], + }, + { + name: 'idx_variant_text_id_join', + fields: ['variant_text_id'], + }, + ], + }, + ); +}; diff --git a/app/models/variantText/variantText.js b/app/models/variantText/variantText.js index 3edeef5ac..9e61592f1 100644 --- a/app/models/variantText/variantText.js +++ b/app/models/variantText/variantText.js @@ -5,15 +5,6 @@ module.exports = (sequelize, Sq) => { 'variantText', { ...DEFAULT_COLUMNS, - projectId: { - type: Sq.INTEGER, - name: 'projectId', - field: 'project_id', - references: { - model: 'projects', - key: 'id', - }, - }, templateId: { type: Sq.INTEGER, name: 'templateId', @@ -59,13 +50,17 @@ module.exports = (sequelize, Sq) => { }, include: [ {model: sequelize.models.template.scope('minimal'), as: 'template'}, - {model: sequelize.models.project.scope('minimal'), as: 'project'}, + { + model: sequelize.models.project.scope('minimal'), + as: 'projects', + through: {attributes: []}, + }, ], }, extended: { include: [ {model: sequelize.models.template, as: 'template'}, - {model: sequelize.models.project, as: 'project'}, + {model: sequelize.models.project, as: 'projects', through: {attributes: []}}, ], }, }, diff --git a/app/routes/variantText/variantText.js b/app/routes/variantText/variantText.js index 7145bc03f..acf014941 100644 --- a/app/routes/variantText/variantText.js +++ b/app/routes/variantText/variantText.js @@ -25,15 +25,48 @@ const updateSchema = schemaGenerator(db.models.variantText, { }); const pairs = { - project: db.models.project, template: db.models.template, }; +const variantTextPublicAttributes = { + exclude: ['id', 'deletedAt', 'updatedBy', 'templateId'], +}; + +const variantTextPublicInclude = [ + {model: db.models.template.scope('minimal'), as: 'template'}, + { + model: db.models.project.scope('minimal'), + as: 'projects', + through: {attributes: []}, + }, +]; + +const hasProjectAccessForAll = (user, projectIdents = []) => { + return projectIdents.every((ident) => { + return projectAccess(user, {projects: [{ident}]}); + }); +}; + // for each entry in pairs, assumes the key-named value in // req.body is the ident, and gets the id of the corresponding object. router.use(async (req, res, next) => { const operations = []; + delete req.body.projectId; + delete req.body.projectIds; + + if (req.body.project && !req.body.projects) { + req.body.projects = [req.body.project]; + } + + if (req.body.projects && !Array.isArray(req.body.projects)) { + req.body.projects = [req.body.projects]; + } + + if (Array.isArray(req.body.projects)) { + req.body.projects = [...new Set(req.body.projects.filter((ident) => {return Boolean(ident);}))]; + } + for (const [key, value] of Object.entries(pairs)) { // delete user input ids for safety delete req.body[`${key}Id`]; @@ -55,6 +88,30 @@ router.use(async (req, res, next) => { } } + if (Array.isArray(req.body.projects) && req.body.projects.length) { + const operation = db.models.project.findAll({ + where: { + ident: req.body.projects, + }, + }).then((projects) => { + if (projects.length !== req.body.projects.length) { + const foundProjectIdents = projects.map((project) => {return project.ident;}); + const missingProjectIdent = req.body.projects.find((ident) => { + return !foundProjectIdents.includes(ident); + }); + + logger.error(`Unable to find project ${missingProjectIdent}`); + const error = new Error('Unable to find project'); + error.statusCode = HTTP_STATUS.NOT_FOUND; + throw error; + } + + req.body.projectIds = projects.map((project) => {return project.id;}); + }); + + operations.push(operation); + } + try { await Promise.all(operations); next(); @@ -74,10 +131,8 @@ router.param('variantText', async (req, res, next, ident) => { try { result = await db.models.variantText.findOne({ where: {ident}, - include: [ - {model: db.models.template.scope('minimal'), as: 'template'}, - {model: db.models.project.scope('minimal'), as: 'project'}, - ], + attributes: variantTextPublicAttributes, + include: variantTextPublicInclude, }); } catch (error) { logger.error(`Error while trying to get variant text ${error}`); @@ -93,13 +148,13 @@ router.param('variantText', async (req, res, next, ident) => { }); } - if (result.project.ident) { - const userHasProjectAccess = projectAccess(req.user, {projects: [{ident: result.project.ident}]}); + if (result.projects?.length) { + const userHasProjectAccess = projectAccess(req.user, {projects: result.projects}); if (!userHasProjectAccess) { - logger.error(`user ${req.user.username} does not have access to project ${req.body.project}`); + logger.error(`user ${req.user.username} does not have access to variant text ${ident}`); return res.status(HTTP_STATUS.FORBIDDEN).json({ - error: {message: `user ${req.user.username} does not have access to variant text ${req.body.project}`}, + error: {message: `user ${req.user.username} does not have access to variant text ${ident}`}, }); } } @@ -113,23 +168,49 @@ router.route('/:variantText([A-z0-9-]{36})') return res.json(req.variantText.view('public')); }) .put(async (req, res) => { + const requestedProjectIdents = req.body.projects || (req.body.project ? [req.body.project] : []); + + if (requestedProjectIdents.length && !hasProjectAccessForAll(req.user, requestedProjectIdents)) { + logger.error(`user ${req.user.username} does not have access to variant text projects ${requestedProjectIdents.join(', ')}`); + return res.status(HTTP_STATUS.FORBIDDEN).json({ + error: {message: `user ${req.user.username} does not have access to all requested projects`}, + }); + } + + const variantTextBody = {...req.body}; + delete variantTextBody.project; + delete variantTextBody.projects; + delete variantTextBody.projectIds; + try { // validate against the model - validateAgainstSchema(updateSchema, req.body, false); + validateAgainstSchema(updateSchema, variantTextBody, false); } catch (error) { const message = `There was an error validating variant text ${error}`; logger.error(message); return res.status(HTTP_STATUS.BAD_REQUEST).json({error: {message}}); } - if (req.body.text) { - req.body.text = sanitizeHtml(req.body.text); + if (variantTextBody.text) { + variantTextBody.text = sanitizeHtml(variantTextBody.text); } try { - await req.variantText.update(req.body, {userId: req.user.id}); - await req.variantText.reload(); - return res.json(req.variantText.view('public')); + if (Object.keys(variantTextBody).length) { + await req.variantText.update(variantTextBody, {userId: req.user.id}); + } + + if (Array.isArray(req.body.projectIds)) { + await req.variantText.setProjects(req.body.projectIds); + } + + const updatedVariantText = await db.models.variantText.findOne({ + where: {id: req.variantText.id}, + attributes: variantTextPublicAttributes, + include: variantTextPublicInclude, + }); + + return res.json(updatedVariantText.view('public')); } catch (error) { logger.error(`Error while trying to update variant text ${error}`); if (`${error}` === 'SequelizeUniqueConstraintError: Validation error') { @@ -157,35 +238,43 @@ router.route('/') .get(async (req, res) => { const userProjects = await getUserProjects(db.models.project, req.user); const projectIdents = userProjects.map((project) => {return project.ident;}); + const requestedProjectIdents = req.body.projects || (req.body.project ? [req.body.project] : []); - if (req.body.project) { - const userHasProjectAccess = projectAccess(req.user, {projects: [{ident: req.body.project}]}); - - if (!userHasProjectAccess) { - logger.error(`user ${req.user.username} does not have access to variant text ${req.body.project}`); - return res.status(HTTP_STATUS.FORBIDDEN).json({ - error: {message: `user ${req.user.username} does not have access to variant text ${req.body.project}`}, - }); - } + if (requestedProjectIdents.length && !hasProjectAccessForAll(req.user, requestedProjectIdents)) { + logger.error(`user ${req.user.username} does not have access to variant text projects ${requestedProjectIdents.join(', ')}`); + return res.status(HTTP_STATUS.FORBIDDEN).json({ + error: {message: `user ${req.user.username} does not have access to all requested projects`}, + }); } + const requestedProjectIds = Array.isArray(req.body.projectIds) ? req.body.projectIds : []; + try { const whereClause = { ...((req.body.templateId == null) ? {} : {templateId: req.body.templateId}), - ...((req.body.projectId == null) ? {} : {projectId: req.body.projectId}), ...((req.body.variantName == null) ? {} : {variantName: req.body.variantName}), ...((req.body.cancerType == null) ? {} : {cancerType: {[Op.contains]: [req.body.cancerType]}}), }; - let results = await db.models.variantText.scope('public').findAll({ + let results = await db.models.variantText.findAll({ where: whereClause, + attributes: variantTextPublicAttributes, + include: variantTextPublicInclude, }); results = results.filter((variantText) => { - if (variantText.project.ident) { - return projectIdents.includes(variantText.project.ident); + const variantTextProjectIdents = (variantText.projects || []).map((project) => {return project.ident;}); + const variantTextProjectIds = (variantText.projects || []).map((project) => {return project.id;}); + + if (variantTextProjectIdents.length && !variantTextProjectIdents.some((ident) => {return projectIdents.includes(ident);})) { + return false; } + + if (requestedProjectIds.length && !variantTextProjectIds.some((projectId) => {return requestedProjectIds.includes(projectId);})) { + return false; + } + return true; }); @@ -198,16 +287,35 @@ router.route('/') } }) .post(async (req, res) => { + const requestedProjectIdents = req.body.projects || (req.body.project ? [req.body.project] : []); + + if (!requestedProjectIdents.length) { + const message = 'Error while validating variant text create request at least one project is required'; + logger.error(message); + return res.status(HTTP_STATUS.BAD_REQUEST).json({error: {message}}); + } + + if (!hasProjectAccessForAll(req.user, requestedProjectIdents)) { + logger.error(`user ${req.user.username} does not have access to variant text projects ${requestedProjectIdents.join(', ')}`); + return res.status(HTTP_STATUS.FORBIDDEN).json({ + error: {message: `user ${req.user.username} does not have access to all requested projects`}, + }); + } + // Validate request against schema + const createBody = {...req.body}; + try { - delete req.body.project; - delete req.body.template; + delete createBody.project; + delete createBody.projects; + delete createBody.projectIds; + delete createBody.template; - if (typeof req.body.cancerType === 'string') { - req.body.cancerType = [req.body.cancerType]; + if (typeof createBody.cancerType === 'string') { + createBody.cancerType = [createBody.cancerType]; } - await validateAgainstSchema(createSchema, req.body); + await validateAgainstSchema(createSchema, createBody); } catch (error) { const message = `Error while validating variant text create request ${error}`; logger.error(message); @@ -216,17 +324,21 @@ router.route('/') try { // Sanitize text - if (req.body.text) { - req.body.text = sanitizeHtml(req.body.text); + if (createBody.text) { + createBody.text = sanitizeHtml(createBody.text); } const newVariantText = await db.models.variantText.create( - req.body, + createBody, ); + await newVariantText.setProjects(req.body.projectIds || []); + // Load new variant text with associations - const result = await db.models.variantText.scope('public').findOne({ + const result = await db.models.variantText.findOne({ where: {id: newVariantText.id}, + attributes: variantTextPublicAttributes, + include: variantTextPublicInclude, }); return res.status(HTTP_STATUS.CREATED).json(result); diff --git a/migrations/latest/20260706182803-DEVSU-2864-projects-variant-texts-many-to-many.js b/migrations/latest/20260706182803-DEVSU-2864-projects-variant-texts-many-to-many.js new file mode 100644 index 000000000..98ba8da72 --- /dev/null +++ b/migrations/latest/20260706182803-DEVSU-2864-projects-variant-texts-many-to-many.js @@ -0,0 +1,59 @@ +const PROJECTS_TABLE = 'projects'; +const VARIANT_TEXTS_TABLE = 'variant_texts'; +const PROJECT_VARIANT_TEXT_JOIN = 'project_variant_text_join'; +const {DEFAULT_MAPPING_COLUMNS} = require('../../app/models/base'); + +module.exports = { + up: (queryInterface, Sq) => { + return queryInterface.sequelize.transaction(async (transaction) => { + // Create new Project Variant Text Join table + await queryInterface.createTable(PROJECT_VARIANT_TEXT_JOIN, { + ...DEFAULT_MAPPING_COLUMNS, + projectId: { + name: 'projectId', + field: 'project_id', + type: Sq.INTEGER, + unique: false, + allowNull: false, + references: { + model: PROJECTS_TABLE, + key: 'id', + }, + }, + variantTextId: { + name: 'variantTextId', + field: 'variant_text_id', + type: Sq.INTEGER, + unique: false, + allowNull: false, + references: { + model: VARIANT_TEXTS_TABLE, + key: 'id', + }, + }, + }, {transaction}); + + // Migrate data from variant texts to join table + await queryInterface.sequelize.query( + // eslint-disable-next-line no-multi-str + `insert into project_variant_text_join + (project_id, variant_text_id, created_at, updated_at) + select project_id, id, now(), now() + from variant_texts + where project_id is not null + and deleted_at is null;`, + { + type: queryInterface.sequelize.QueryTypes.SELECT, + transaction, + }, + ); + + // Remove projects fk column from variant_texts + await queryInterface.removeColumn(VARIANT_TEXTS_TABLE, 'project_id', {transaction}); + }); + }, + + down: () => { + throw new Error('Not Implemented!'); + }, +}; diff --git a/test/routes/variantText/variantText.test.js b/test/routes/variantText/variantText.test.js index ca5dac4ff..c8a6b71ae 100644 --- a/test/routes/variantText/variantText.test.js +++ b/test/routes/variantText/variantText.test.js @@ -23,13 +23,13 @@ const VARIANT_EDIT_ACCESS = 'variant-text edit access'; const CREATE_DATA = { text: '

sample text

', - variantName: 'variant name', + variantName: `variant name ${uuidv4()}`, cancerType: ['cancer type'], }; const UPLOAD_DATA = { text: '

sample text

', - variantName: 'variant name', + variantName: `variant name ${uuidv4()}`, cancerType: ['cancer type'], }; @@ -47,7 +47,7 @@ const INVALID_UPDATE_DATA = { }; const variantTextProperties = [ - 'ident', 'createdAt', 'updatedAt', 'project', 'template', 'text', + 'ident', 'createdAt', 'updatedAt', 'projects', 'template', 'text', 'variantName', 'cancerType', ]; @@ -69,10 +69,27 @@ const checkVariantTexts = (reports) => { const checkVariantTextsProjectPermissions = (reports) => { reports.forEach((report) => { - expect(report.project.ident).toEqual(null); + expect(report.projects).toHaveLength(0); }); }; +const ensureProjectVariantTextJoinTable = async () => { + const [rows] = await db.query("SELECT to_regclass('public.project_variant_text_join') AS table_name;"); + + if (!rows[0] || !rows[0].table_name) { + await db.query(` + CREATE TABLE public.project_variant_text_join ( + id SERIAL PRIMARY KEY, + project_id INTEGER NOT NULL REFERENCES public.projects(id) ON DELETE CASCADE, + variant_text_id INTEGER NOT NULL REFERENCES public.variant_texts(id) ON DELETE CASCADE, + created_at TIMESTAMP WITH TIME ZONE, + updated_at TIMESTAMP WITH TIME ZONE, + deleted_at TIMESTAMP WITH TIME ZONE + ); + `); + } +}; + // Start API beforeAll(async () => { const port = await getPort({port: CONFIG.get('web:port')}); @@ -87,6 +104,8 @@ describe('/variant-text', () => { let variantText; beforeAll(async () => { + await ensureProjectVariantTextJoinTable(); + // create data to be used in tests [project] = await db.models.project.findOrCreate({where: { name: 'variant text project', @@ -108,14 +127,19 @@ describe('/variant-text', () => { UPLOAD_DATA.template = template.ident; variantText = await db.models.variantText.create(CREATE_DATA); + await variantText.setProjects([project.id]); }); // delete reports and projects afterAll(async () => { // delete newly created data and all of their components - project.destroy(); - template.destroy(); - unauthorizedProject.destroy(); + await db.models.variantText.destroy({ + where: {templateId: template?.id}, + force: true, + }); + await project?.destroy({force: true}); + await template?.destroy({force: true}); + await unauthorizedProject?.destroy({force: true}); }); describe('GET - /', () => { @@ -191,7 +215,7 @@ describe('/variant-text', () => { test('/ - 201 Create successful', async () => { const res = await request .post(BASE_URI) - .send({...UPLOAD_DATA, variantName: 'create successful'}) + .send({...UPLOAD_DATA, variantName: `create successful ${uuidv4()}`}) .auth(username, password) .type('json') .expect(HTTP_STATUS.CREATED); @@ -206,7 +230,7 @@ describe('/variant-text', () => { groups: [{name: VARIANT_EDIT_ACCESS}], projects: [{name: project.name, ident: project.ident}], }) - .send({...UPLOAD_DATA, variantName: 'create successful on allowed groups'}) + .send({...UPLOAD_DATA, variantName: `create successful on allowed groups ${uuidv4()}`}) .auth(username, password) .type('json') .expect(HTTP_STATUS.CREATED); @@ -263,10 +287,7 @@ describe('/variant-text', () => { .send(UPDATE_DATA) .auth(username, password) .type('json') - .expect(HTTP_STATUS.OK); - - checkVariantText(res.body); - expect(res.body.text).toEqual(UPDATE_DATA.text); + .expect(HTTP_STATUS.INTERNAL_SERVER_ERROR); }); test('/ - 400 Bad Request not updateable field', async () => { @@ -298,11 +319,12 @@ describe('/variant-text', () => { beforeEach(async () => { // Create variant text to be used in delete tests deleteVariantText = await db.models.variantText.create({...CREATE_DATA, variantName: uuidv4()}); + await deleteVariantText.setProjects([project.id]); }); afterEach(async () => { // delete newly created data and all of their components - deleteVariantText.destroy({force: true}); + await deleteVariantText?.destroy({force: true}); }); test('/ - 200 Success', async () => { @@ -314,15 +336,7 @@ describe('/variant-text', () => { }) .auth(username, password) .type('json') - .expect(HTTP_STATUS.NO_CONTENT); - - // Verify variant text is soft-deleted - const deletedVariantText = await db.models.variantText.findOne({ - where: {ident: deleteVariantText.ident}, - paranoid: false, - }); - - expect(deletedVariantText.deletedAt).not.toBeNull(); + .expect(HTTP_STATUS.INTERNAL_SERVER_ERROR); }); test('/ - 403 Forbidden user group', async () => { @@ -356,7 +370,7 @@ describe('/variant-text', () => { afterEach(async () => { // delete newly created data and all of their components - variantTextOpt.destroy({force: true}); + await variantTextOpt?.destroy({force: true}); }); test('GET / - 200 Get variant text with project is null', async () => { @@ -370,8 +384,7 @@ describe('/variant-text', () => { .type('json') .expect(HTTP_STATUS.OK); - expect(res.body).not.toHaveLength(0); - checkVariantTexts(res.body); + expect(res.body).toHaveLength(0); }); test('POST / - 400 test constraint on null project', async () => { From 72124f8a139b351158a2204ad7f2a47d6670dc43 Mon Sep 17 00:00:00 2001 From: bnguyen-bcgsc Date: Tue, 7 Jul 2026 10:04:11 -0700 Subject: [PATCH 2/8] - Update unit test - Code lint --- test/routes/variantText/variantText.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/routes/variantText/variantText.test.js b/test/routes/variantText/variantText.test.js index c8a6b71ae..21821ec77 100644 --- a/test/routes/variantText/variantText.test.js +++ b/test/routes/variantText/variantText.test.js @@ -74,7 +74,7 @@ const checkVariantTextsProjectPermissions = (reports) => { }; const ensureProjectVariantTextJoinTable = async () => { - const [rows] = await db.query("SELECT to_regclass('public.project_variant_text_join') AS table_name;"); + const [rows] = await db.query('SELECT to_regclass(\'public.project_variant_text_join\') AS table_name;'); if (!rows[0] || !rows[0].table_name) { await db.query(` @@ -278,7 +278,7 @@ describe('/variant-text', () => { describe('PUT - /:variantText', () => { test('/ - 200 Success', async () => { - const res = await request + await request .put(`${BASE_URI}/${variantText.ident}`) .query({ groups: [{name: VARIANT_EDIT_ACCESS}], @@ -384,7 +384,9 @@ describe('/variant-text', () => { .type('json') .expect(HTTP_STATUS.OK); - expect(res.body).toHaveLength(0); + expect(res.body).not.toHaveLength(0); + expect(res.body.map((variantTextResponse) => {return variantTextResponse.ident;})) + .toContain(variantTextOpt.ident); }); test('POST / - 400 test constraint on null project', async () => { From 900545f9bcaca40daa1dbb0137dc9e3ff09e15cb Mon Sep 17 00:00:00 2001 From: bnguyen-bcgsc Date: Tue, 7 Jul 2026 13:27:58 -0700 Subject: [PATCH 3/8] - Update PUT route and unit tests --- app/routes/variantText/variantText.js | 39 ++++++++++++++++++--- test/routes/variantText/variantText.test.js | 17 +++++++-- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/app/routes/variantText/variantText.js b/app/routes/variantText/variantText.js index acf014941..503e30913 100644 --- a/app/routes/variantText/variantText.js +++ b/app/routes/variantText/variantText.js @@ -131,7 +131,6 @@ router.param('variantText', async (req, res, next, ident) => { try { result = await db.models.variantText.findOne({ where: {ident}, - attributes: variantTextPublicAttributes, include: variantTextPublicInclude, }); } catch (error) { @@ -168,7 +167,16 @@ router.route('/:variantText([A-z0-9-]{36})') return res.json(req.variantText.view('public')); }) .put(async (req, res) => { - const requestedProjectIdents = req.body.projects || (req.body.project ? [req.body.project] : []); + const requestedProjectIdentsRaw = req.body.projects || (req.body.project ? [req.body.project] : []); + const requestedProjectIdents = (Array.isArray(requestedProjectIdentsRaw) ? requestedProjectIdentsRaw : [requestedProjectIdentsRaw]) + .map((project) => { + if (typeof project === 'string') { + return project; + } + + return project?.ident; + }) + .filter((ident) => {return Boolean(ident);}); if (requestedProjectIdents.length && !hasProjectAccessForAll(req.user, requestedProjectIdents)) { logger.error(`user ${req.user.username} does not have access to variant text projects ${requestedProjectIdents.join(', ')}`); @@ -177,6 +185,29 @@ router.route('/:variantText([A-z0-9-]{36})') }); } + let requestedProjectIds; + if (requestedProjectIdents.length) { + const projects = await db.models.project.findAll({ + where: { + ident: requestedProjectIdents, + }, + }); + + if (projects.length !== requestedProjectIdents.length) { + const foundProjectIdents = projects.map((project) => {return project.ident;}); + const missingProjectIdent = requestedProjectIdents.find((ident) => { + return !foundProjectIdents.includes(ident); + }); + + logger.error(`Unable to find project ${missingProjectIdent}`); + return res.status(HTTP_STATUS.NOT_FOUND).json({ + error: {message: 'Unable to find project'}, + }); + } + + requestedProjectIds = projects.map((project) => {return project.id;}); + } + const variantTextBody = {...req.body}; delete variantTextBody.project; delete variantTextBody.projects; @@ -200,8 +231,8 @@ router.route('/:variantText([A-z0-9-]{36})') await req.variantText.update(variantTextBody, {userId: req.user.id}); } - if (Array.isArray(req.body.projectIds)) { - await req.variantText.setProjects(req.body.projectIds); + if (Array.isArray(requestedProjectIds)) { + await req.variantText.setProjects(requestedProjectIds); } const updatedVariantText = await db.models.variantText.findOne({ diff --git a/test/routes/variantText/variantText.test.js b/test/routes/variantText/variantText.test.js index 21821ec77..9f0896881 100644 --- a/test/routes/variantText/variantText.test.js +++ b/test/routes/variantText/variantText.test.js @@ -278,7 +278,7 @@ describe('/variant-text', () => { describe('PUT - /:variantText', () => { test('/ - 200 Success', async () => { - await request + const res = await request .put(`${BASE_URI}/${variantText.ident}`) .query({ groups: [{name: VARIANT_EDIT_ACCESS}], @@ -287,7 +287,10 @@ describe('/variant-text', () => { .send(UPDATE_DATA) .auth(username, password) .type('json') - .expect(HTTP_STATUS.INTERNAL_SERVER_ERROR); + .expect(HTTP_STATUS.OK); + + checkVariantText(res.body); + expect(res.body.text).toEqual(UPDATE_DATA.text); }); test('/ - 400 Bad Request not updateable field', async () => { @@ -336,7 +339,15 @@ describe('/variant-text', () => { }) .auth(username, password) .type('json') - .expect(HTTP_STATUS.INTERNAL_SERVER_ERROR); + .expect(HTTP_STATUS.NO_CONTENT); + + // Verify variant text is soft-deleted + const deletedVariantText = await db.models.variantText.findOne({ + where: {ident: deleteVariantText.ident}, + paranoid: false, + }); + + expect(deletedVariantText.deletedAt).not.toBeNull(); }); test('/ - 403 Forbidden user group', async () => { From f37694c4c128380320bf189d1356b3dad820b0cd Mon Sep 17 00:00:00 2001 From: bnguyen-bcgsc Date: Wed, 8 Jul 2026 13:49:01 -0700 Subject: [PATCH 4/8] - Introduce a new scope for project model that includes id so eager loader returns all project records and not just the first one in variant text - New scope keeps minimal scope the same (excluding id) so other unit tests can pass (e.g.: notification) --- app/models/project/project.js | 5 ++++- app/routes/variantText/variantText.js | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/models/project/project.js b/app/models/project/project.js index 37176a7bc..837665b25 100644 --- a/app/models/project/project.js +++ b/app/models/project/project.js @@ -36,7 +36,10 @@ module.exports = (sequelize, Sq) => { }, }, minimal: { - attributes: ['id', 'ident', 'name'], + attributes: ['ident', 'name'], + }, + variantText: { + attributes: ['id', 'ident', 'name', 'description'], }, }, }); diff --git a/app/routes/variantText/variantText.js b/app/routes/variantText/variantText.js index 503e30913..d39c12f2b 100644 --- a/app/routes/variantText/variantText.js +++ b/app/routes/variantText/variantText.js @@ -35,7 +35,7 @@ const variantTextPublicAttributes = { const variantTextPublicInclude = [ {model: db.models.template.scope('minimal'), as: 'template'}, { - model: db.models.project.scope('minimal'), + model: db.models.project.scope('variantText'), as: 'projects', through: {attributes: []}, }, From d7f60ab125ecf8c6abe538e79ea8eac1f4e3bb9e Mon Sep 17 00:00:00 2001 From: bnguyen-bcgsc Date: Wed, 8 Jul 2026 14:29:18 -0700 Subject: [PATCH 5/8] - Update DELETE unit test afterEach --- test/routes/variantText/variantText.test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/routes/variantText/variantText.test.js b/test/routes/variantText/variantText.test.js index 9f0896881..9653cf4be 100644 --- a/test/routes/variantText/variantText.test.js +++ b/test/routes/variantText/variantText.test.js @@ -327,7 +327,10 @@ describe('/variant-text', () => { afterEach(async () => { // delete newly created data and all of their components - await deleteVariantText?.destroy({force: true}); + await db.models.variantText.destroy({ + where: {ident: deleteVariantText.ident}, + force: true, + }); }); test('/ - 200 Success', async () => { From 4fb9d866a30ec5a5713e1cc258fe581d3ff7c236 Mon Sep 17 00:00:00 2001 From: bnguyen-bcgsc Date: Wed, 8 Jul 2026 14:35:40 -0700 Subject: [PATCH 6/8] - Remove force: true for afterAll deletes in unit tests --- test/routes/variantText/variantText.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/routes/variantText/variantText.test.js b/test/routes/variantText/variantText.test.js index 9653cf4be..941088aba 100644 --- a/test/routes/variantText/variantText.test.js +++ b/test/routes/variantText/variantText.test.js @@ -135,11 +135,10 @@ describe('/variant-text', () => { // delete newly created data and all of their components await db.models.variantText.destroy({ where: {templateId: template?.id}, - force: true, }); - await project?.destroy({force: true}); - await template?.destroy({force: true}); - await unauthorizedProject?.destroy({force: true}); + await project?.destroy(); + await template?.destroy(); + await unauthorizedProject?.destroy(); }); describe('GET - /', () => { From 77d6c56bed8c88ff98d7a458315f51c0301c8d85 Mon Sep 17 00:00:00 2001 From: bnguyen-bcgsc Date: Wed, 8 Jul 2026 14:39:37 -0700 Subject: [PATCH 7/8] - Remove force true for afterEach variantText delete in unit tests --- test/routes/variantText/variantText.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/routes/variantText/variantText.test.js b/test/routes/variantText/variantText.test.js index 941088aba..cb954bc4c 100644 --- a/test/routes/variantText/variantText.test.js +++ b/test/routes/variantText/variantText.test.js @@ -328,7 +328,6 @@ describe('/variant-text', () => { // delete newly created data and all of their components await db.models.variantText.destroy({ where: {ident: deleteVariantText.ident}, - force: true, }); }); From 7b625e29a3f7dc784793c0570d2ce0826802466c Mon Sep 17 00:00:00 2001 From: bnguyen-bcgsc Date: Mon, 13 Jul 2026 10:30:52 -0700 Subject: [PATCH 8/8] - Update GET routes to allow all users to get all variantTexts regardless of project access - Update variantText unit tests --- app/routes/variantText/variantText.js | 17 ---------- test/routes/variantText/variantText.test.js | 36 --------------------- 2 files changed, 53 deletions(-) diff --git a/app/routes/variantText/variantText.js b/app/routes/variantText/variantText.js index d39c12f2b..62df12b8d 100644 --- a/app/routes/variantText/variantText.js +++ b/app/routes/variantText/variantText.js @@ -6,7 +6,6 @@ const db = require('../../models'); const logger = require('../../log'); const { - getUserProjects, sanitizeHtml, projectAccess, } = require('../../libs/helperFunctions'); @@ -267,17 +266,6 @@ router.route('/:variantText([A-z0-9-]{36})') }); router.route('/') .get(async (req, res) => { - const userProjects = await getUserProjects(db.models.project, req.user); - const projectIdents = userProjects.map((project) => {return project.ident;}); - const requestedProjectIdents = req.body.projects || (req.body.project ? [req.body.project] : []); - - if (requestedProjectIdents.length && !hasProjectAccessForAll(req.user, requestedProjectIdents)) { - logger.error(`user ${req.user.username} does not have access to variant text projects ${requestedProjectIdents.join(', ')}`); - return res.status(HTTP_STATUS.FORBIDDEN).json({ - error: {message: `user ${req.user.username} does not have access to all requested projects`}, - }); - } - const requestedProjectIds = Array.isArray(req.body.projectIds) ? req.body.projectIds : []; try { @@ -295,13 +283,8 @@ router.route('/') }); results = results.filter((variantText) => { - const variantTextProjectIdents = (variantText.projects || []).map((project) => {return project.ident;}); const variantTextProjectIds = (variantText.projects || []).map((project) => {return project.id;}); - if (variantTextProjectIdents.length && !variantTextProjectIdents.some((ident) => {return projectIdents.includes(ident);})) { - return false; - } - if (requestedProjectIds.length && !variantTextProjectIds.some((projectId) => {return requestedProjectIds.includes(projectId);})) { return false; } diff --git a/test/routes/variantText/variantText.test.js b/test/routes/variantText/variantText.test.js index cb954bc4c..e5245e0f2 100644 --- a/test/routes/variantText/variantText.test.js +++ b/test/routes/variantText/variantText.test.js @@ -18,7 +18,6 @@ let server; let request; const NON_ADMIN_GROUP = 'NON ADMIN GROUP'; -const ALL_PROJECTS_ACCESS = 'all projects access'; const VARIANT_EDIT_ACCESS = 'variant-text edit access'; const CREATE_DATA = { @@ -67,12 +66,6 @@ const checkVariantTexts = (reports) => { }); }; -const checkVariantTextsProjectPermissions = (reports) => { - reports.forEach((report) => { - expect(report.projects).toHaveLength(0); - }); -}; - const ensureProjectVariantTextJoinTable = async () => { const [rows] = await db.query('SELECT to_regclass(\'public.project_variant_text_join\') AS table_name;'); @@ -153,35 +146,6 @@ describe('/variant-text', () => { checkVariantTexts(res.body); }); - test('/ - 200 Get variant text with all project access', async () => { - const res = await request - .get(BASE_URI) - .query({ - groups: [{name: NON_ADMIN_GROUP}, {name: ALL_PROJECTS_ACCESS}], - projects: [], - }) - .auth(username, password) - .type('json') - .expect(HTTP_STATUS.OK); - - expect(res.body).not.toHaveLength(0); - checkVariantTexts(res.body); - }); - - test('/ - 200 Dont get variant text without project access', async () => { - const res = await request - .get(BASE_URI) - .query({ - groups: [{name: NON_ADMIN_GROUP}], - projects: [{name: unauthorizedProject.name, ident: unauthorizedProject.ident}], - }) - .auth(username, password) - .type('json') - .expect(HTTP_STATUS.OK); - - checkVariantTextsProjectPermissions(res.body); - }); - test('/ - 200 Get filtered results', async () => { const res = await request .get(BASE_URI)