Skip to content

Commit 9e246b9

Browse files
committed
bugfix: add hook to therapueticTargets on update to remove signatures
DEVSU-2953
1 parent 64b740e commit 9e246b9

2 files changed

Lines changed: 105 additions & 1 deletion

File tree

app/routes/report/therapeuticTargets.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ router.route('/')
142142
const {report: {id: reportId}, body} = req;
143143
try {
144144
await db.transaction(async (transaction) => {
145-
return Promise.all(body.map((target) => {
145+
await Promise.all(body.map((target) => {
146146
return db.models.therapeuticTarget.update(
147147
{rank: target.rank},
148148
{
@@ -151,10 +151,44 @@ router.route('/')
151151
ident: target.ident,
152152
},
153153
transaction,
154+
// Hooks are disabled so each reorder is a single atomic
155+
// statement (deferred exclusion-constraint friendly, no
156+
// per-row history spam). The model afterUpdate hook that
157+
// would normally remove signatures is bypassed, so the
158+
// signatures are removed explicitly below instead.
154159
hooks: false,
155160
},
156161
);
157162
}));
163+
164+
// Reordering targets edits the report, so revoke the author +
165+
// reviewer signatures once for the report (mirrors the afterUpdate
166+
// hook in models/base.js that hooks: false bypasses above).
167+
// Sequentially awaited - not concurrent - to keep the shared
168+
// transaction connection stable.
169+
await db.models.signatures.update({
170+
authorId: null,
171+
authorSignedAt: null,
172+
reviewerId: null,
173+
reviewerSignedAt: null,
174+
}, {
175+
where: {reportId},
176+
individualHooks: true,
177+
paranoid: true,
178+
transaction,
179+
userId: req.user.id,
180+
});
181+
182+
// If the report was "reviewed" send it back to "ready"
183+
await db.models.report.update({
184+
state: 'ready',
185+
}, {
186+
where: {id: reportId, state: 'reviewed'},
187+
individualHooks: true,
188+
paranoid: true,
189+
transaction,
190+
userId: req.user.id,
191+
});
158192
});
159193

160194
return res.json({updated: true});

test/routes/report/therapeuticTargets.test.js

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,76 @@ describe('/therapeutic-targets', () => {
390390
.type('json')
391391
.expect(HTTP_STATUS.BAD_REQUEST);
392392
}, LONGER_TIMEOUT);
393+
394+
describe('rank update and report signatures', () => {
395+
let signature;
396+
let user;
397+
let originalState;
398+
399+
beforeEach(async () => {
400+
user = await db.models.user.findOne({where: {username}});
401+
signature = await db.models.signatures.create({
402+
reportId: report.id,
403+
authorId: user.id,
404+
authorSignedAt: new Date(),
405+
reviewerId: user.id,
406+
reviewerSignedAt: new Date(),
407+
creatorId: user.id,
408+
creatorSignedAt: new Date(),
409+
});
410+
// state is excluded from signature removal, so setting it
411+
// here does not wipe the signature we just created
412+
({state: originalState} = await db.models.report.findOne({
413+
where: {id: report.id},
414+
}));
415+
await db.models.report.update(
416+
{state: 'reviewed'},
417+
{where: {id: report.id}, hooks: false},
418+
);
419+
});
420+
421+
afterEach(async () => {
422+
if (signature) {
423+
await db.models.signatures.destroy({
424+
where: {ident: signature.ident},
425+
force: true,
426+
});
427+
}
428+
// restore the original state - report.state is allowNull: false
429+
await db.models.report.update(
430+
{state: originalState},
431+
{where: {id: report.id}, hooks: false},
432+
);
433+
});
434+
435+
test('reorder revokes author + reviewer signatures, keeps creator', async () => {
436+
await request
437+
.put(`/api/reports/${report.ident}/therapeutic-targets`)
438+
.auth(username, password)
439+
.send([
440+
{ident: originalGene.ident, rank: newTarget.rank},
441+
{ident: newTarget.ident, rank: originalGene.rank},
442+
])
443+
.type('json')
444+
.expect(HTTP_STATUS.OK);
445+
446+
const updatedSignature = await db.models.signatures.findOne({
447+
where: {reportId: report.id},
448+
});
449+
expect(updatedSignature.authorId).toBe(null);
450+
expect(updatedSignature.authorSignedAt).toBe(null);
451+
expect(updatedSignature.reviewerId).toBe(null);
452+
expect(updatedSignature.reviewerSignedAt).toBe(null);
453+
// creator signature is intentionally preserved
454+
expect(updatedSignature.creatorId).toBe(user.id);
455+
expect(updatedSignature.creatorSignedAt).not.toBe(null);
456+
457+
const updatedReport = await db.models.report.findOne({
458+
where: {id: report.id},
459+
});
460+
expect(updatedReport.state).toBe('ready');
461+
}, LONGER_TIMEOUT);
462+
});
393463
});
394464

395465
test.todo('Bad request on update and set gene to null');

0 commit comments

Comments
 (0)