Skip to content

Commit fca6bcf

Browse files
ChongJiaChuaChongJiaChua
authored
fix: allow instrument contacts to filter by their assigned experimental areas (#1553)
2 parents bc36df0 + 0dfd57c commit fca6bcf

4 files changed

Lines changed: 242 additions & 26 deletions

File tree

‎apps/backend/src/datasources/postgres/CallDataSource.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ export default class PostgresCallDataSource implements CallDataSource {
572572
'call_has_instruments.call_id',
573573
'call.call_id'
574574
)
575-
.join(
575+
.leftJoin(
576576
'instrument_has_scientists',
577577
'instrument_has_scientists.instrument_id',
578578
'call_has_instruments.instrument_id'
@@ -582,8 +582,12 @@ export default class PostgresCallDataSource implements CallDataSource {
582582
'instruments.instrument_id',
583583
'call_has_instruments.instrument_id'
584584
)
585-
.where('instrument_has_scientists.user_id', scientistId)
586-
.orWhere('instruments.manager_user_id', scientistId);
585+
.where(function () {
586+
this.where('instrument_has_scientists.user_id', scientistId).orWhere(
587+
'instruments.manager_user_id',
588+
scientistId
589+
);
590+
});
587591

588592
return records.map(createCallObject);
589593
}

‎apps/backend/src/datasources/postgres/ExperimentDataSource.ts‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ export default class PostgresExperimentDataSource
577577
//print all arguments
578578

579579
const query = database('experiments')
580-
.select(['*', database.raw('count(*) OVER() AS full_count')])
580+
.select(['experiments.*', database.raw('count(*) OVER() AS full_count')])
581581
.join(
582582
'proposals',
583583
'proposals.proposal_pk',
@@ -587,16 +587,31 @@ export default class PostgresExperimentDataSource
587587

588588
// Add instrument scientist filtering if provided
589589
if (filter?.instrumentScientistUserId) {
590+
const instrumentScientistUserId = filter.instrumentScientistUserId;
591+
590592
query
593+
.leftJoin('instrument_has_scientists', function () {
594+
this.on(
595+
'experiments.instrument_id',
596+
'=',
597+
'instrument_has_scientists.instrument_id'
598+
).andOnVal(
599+
'instrument_has_scientists.user_id',
600+
'=',
601+
instrumentScientistUserId
602+
);
603+
})
591604
.join(
592-
'instrument_has_scientists',
605+
'instruments',
593606
'experiments.instrument_id',
594-
'instrument_has_scientists.instrument_id'
607+
'instruments.instrument_id'
595608
)
596-
.where(
597-
'instrument_has_scientists.user_id',
598-
filter.instrumentScientistUserId
599-
);
609+
.where(function () {
610+
this.whereNotNull('instrument_has_scientists.user_id').orWhere(
611+
'instruments.manager_user_id',
612+
instrumentScientistUserId
613+
);
614+
});
600615
}
601616

602617
return query

‎apps/backend/src/datasources/postgres/InstrumentDataSource.ts‎

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export default class PostgresInstrumentDataSource
3737
constructor(
3838
@inject(Tokens.FapDataSource) private fapDataSource: FapDataSource
3939
) {}
40+
4041
private createInstrumentObject(instrument: InstrumentRecord) {
4142
return new Instrument(
4243
instrument.instrument_id,
@@ -296,7 +297,22 @@ export default class PostgresInstrumentDataSource
296297
.whereIn('tag_id', tagIds)
297298
.select('i.*');
298299

299-
return instruments.map(this.createInstrumentWithAvailabilityTimeObject);
300+
const managerInstruments = await database<InstrumentRecord>(
301+
'instruments as i'
302+
)
303+
.where('i.manager_user_id', userId)
304+
.select('i.*');
305+
306+
const allInstruments = [...instruments, ...managerInstruments];
307+
const uniqueInstruments = allInstruments.filter(
308+
(inst, index, self) =>
309+
self.findIndex((i) => i.instrument_id === inst.instrument_id) ===
310+
index
311+
);
312+
313+
return uniqueInstruments.map(
314+
this.createInstrumentWithAvailabilityTimeObject
315+
);
300316
} else {
301317
const instruments =
302318
await database<InstrumentRecord>('instruments as i').select('i.*');
@@ -314,11 +330,12 @@ export default class PostgresInstrumentDataSource
314330
'manager_user_id',
315331
])
316332
.from('instruments as i')
317-
.join('instrument_has_scientists as ihs', {
333+
.leftJoin('instrument_has_scientists as ihs', {
318334
'i.instrument_id': 'ihs.instrument_id',
319335
})
320-
.where('ihs.user_id', userId)
321-
.orWhere('i.manager_user_id', userId)
336+
.where(function () {
337+
this.where('ihs.user_id', userId).orWhere('i.manager_user_id', userId);
338+
})
322339
.distinct('i.instrument_id')
323340
.then((instruments: InstrumentRecord[]) => {
324341
const result = instruments.map((instrument) =>
@@ -803,14 +820,20 @@ export default class PostgresInstrumentDataSource
803820
await database
804821
.count({ count: '*' })
805822
.from('instruments as i')
806-
.join('instrument_has_scientists as ihs', {
807-
'i.instrument_id': 'ihs.instrument_id',
823+
.leftJoin('instrument_has_scientists as ihs', function () {
824+
this.on('i.instrument_id', '=', 'ihs.instrument_id').andOnVal(
825+
'ihs.user_id',
826+
'=',
827+
userId
828+
);
808829
})
809-
.where('ihs.user_id', userId)
810830
.where('i.instrument_id', instrumentId)
831+
.andWhere(function () {
832+
this.whereNotNull('ihs.user_id').orWhere('i.manager_user_id', userId);
833+
})
811834
.first();
812835

813-
return result?.count === '1';
836+
return Number(result?.count) >= 1;
814837
}
815838

816839
async hasInstrumentScientistAccess(
@@ -821,19 +844,27 @@ export default class PostgresInstrumentDataSource
821844
return database
822845
.select([database.raw('count(*) OVER() AS count')])
823846
.from('proposals')
824-
.join('instrument_has_scientists', {
825-
'instrument_has_scientists.user_id': scientistId,
826-
})
827847
.join('instrument_has_proposals', {
828848
'instrument_has_proposals.proposal_pk': 'proposals.proposal_pk',
829-
'instrument_has_proposals.instrument_id':
830-
'instrument_has_scientists.instrument_id',
849+
})
850+
.join('instruments', {
851+
'instruments.instrument_id': 'instrument_has_proposals.instrument_id',
852+
})
853+
.leftJoin('instrument_has_scientists', {
854+
'instrument_has_scientists.instrument_id': 'instruments.instrument_id',
855+
'instrument_has_scientists.user_id': scientistId,
831856
})
832857
.where('proposals.proposal_pk', '=', proposalPk)
833-
.where('instrument_has_scientists.instrument_id', '=', instrumentId)
858+
.where('instrument_has_proposals.instrument_id', '=', instrumentId)
859+
.andWhere(function () {
860+
this.where('instrument_has_scientists.user_id', scientistId).orWhere(
861+
'instruments.manager_user_id',
862+
scientistId
863+
);
864+
})
834865
.first()
835-
.then((result: undefined | { count: string }) => {
836-
return result?.count === '1';
866+
.then((result) => {
867+
return Number(result?.count) >= 1;
837868
});
838869
}
839870

‎apps/e2e/cypress/e2e/instruments.cy.ts‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2002,4 +2002,170 @@ context('Instrument tests', () => {
20022002
cy.should('not.contain', proposal1.title);
20032003
});
20042004
});
2005+
2006+
describe('Instrument contact visibility tests', () => {
2007+
let contactOnlyInstrumentId: number;
2008+
let scientistInstrumentId: number;
2009+
let createdProposalPk: number;
2010+
let createdScientistProposalPk: number;
2011+
2012+
const contactOnlyInstrument = {
2013+
name: faker.word.words(2),
2014+
shortCode: faker.string.alphanumeric(15),
2015+
description: faker.word.words(5),
2016+
managerUserId: scientist2.id,
2017+
};
2018+
2019+
const scientistInstrument = {
2020+
name: faker.word.words(2),
2021+
shortCode: faker.string.alphanumeric(15),
2022+
description: faker.word.words(5),
2023+
managerUserId: scientist1.id,
2024+
};
2025+
2026+
beforeEach(function () {
2027+
if (featureFlags.getEnabledFeatures().get(FeatureId.SCHEDULER)) {
2028+
cy.updateUserRoles({
2029+
id: scientist2.id,
2030+
roles: [initialDBData.roles.instrumentScientist],
2031+
});
2032+
}
2033+
2034+
//create an instrument where scientist2 is only the contact and not a scientist
2035+
cy.createInstrument(contactOnlyInstrument).then((result) => {
2036+
if (result.createInstrument) {
2037+
contactOnlyInstrumentId = result.createInstrument.id;
2038+
2039+
cy.assignInstrumentToCall({
2040+
callId: initialDBData.call.id,
2041+
instrumentFapIds: [{ instrumentId: contactOnlyInstrumentId }],
2042+
});
2043+
}
2044+
});
2045+
2046+
//create an instrument where scientist2 is an instrument scientist
2047+
cy.createInstrument(scientistInstrument).then((result) => {
2048+
if (result.createInstrument) {
2049+
scientistInstrumentId = result.createInstrument.id;
2050+
2051+
cy.assignInstrumentToCall({
2052+
callId: initialDBData.call.id,
2053+
instrumentFapIds: [{ instrumentId: scientistInstrumentId }],
2054+
});
2055+
2056+
cy.assignScientistsToInstrument({
2057+
instrumentId: scientistInstrumentId,
2058+
scientistIds: [scientist2.id],
2059+
});
2060+
}
2061+
});
2062+
2063+
//create a proposal and assign it to the contact only instrument
2064+
cy.createProposal({ callId: initialDBData.call.id }).then((result) => {
2065+
if (result.createProposal) {
2066+
createdProposalPk = result.createProposal.primaryKey;
2067+
2068+
cy.updateProposal({
2069+
proposalPk: createdProposalPk,
2070+
title: proposal1.title,
2071+
abstract: proposal1.abstract,
2072+
});
2073+
2074+
cy.assignProposalsToInstruments({
2075+
proposalPks: [createdProposalPk],
2076+
instrumentIds: [contactOnlyInstrumentId],
2077+
});
2078+
2079+
cy.updateTechnicalReviewAssignee({
2080+
proposalPks: [createdProposalPk],
2081+
userId: scientist2.id,
2082+
instrumentId: contactOnlyInstrumentId,
2083+
});
2084+
}
2085+
});
2086+
2087+
//create a proposal and assign it to the instrument where scientist2 is a scientist
2088+
cy.createProposal({ callId: initialDBData.call.id }).then((result) => {
2089+
if (result.createProposal) {
2090+
createdScientistProposalPk = result.createProposal.primaryKey;
2091+
2092+
cy.updateProposal({
2093+
proposalPk: createdScientistProposalPk,
2094+
title: proposal2.title,
2095+
abstract: proposal2.abstract,
2096+
});
2097+
2098+
cy.assignProposalsToInstruments({
2099+
proposalPks: [createdScientistProposalPk],
2100+
instrumentIds: [scientistInstrumentId],
2101+
});
2102+
2103+
cy.updateTechnicalReviewAssignee({
2104+
proposalPks: [createdScientistProposalPk],
2105+
userId: scientist2.id,
2106+
instrumentId: scientistInstrumentId,
2107+
});
2108+
}
2109+
});
2110+
2111+
cy.login(scientist2);
2112+
cy.visit('/');
2113+
});
2114+
2115+
it('Instrument contact should see their instrument in the instrument filter dropdown', () => {
2116+
cy.contains('Proposals');
2117+
2118+
selectAllProposalsFilterStatus();
2119+
2120+
cy.finishedLoading();
2121+
2122+
cy.get('[data-cy="instrument-filter"]').click();
2123+
2124+
cy.get('[role="listbox"]')
2125+
.contains(contactOnlyInstrument.name)
2126+
.should('exist');
2127+
2128+
cy.get('[role="listbox"]')
2129+
.contains(scientistInstrument.name)
2130+
.should('exist');
2131+
});
2132+
2133+
it('Instrument contact should see proposals when filtering by their instrument', () => {
2134+
cy.contains('Proposals');
2135+
2136+
selectAllProposalsFilterStatus();
2137+
2138+
cy.finishedLoading();
2139+
2140+
cy.get('[data-cy="instrument-filter"]').click();
2141+
cy.get('[role="listbox"]').contains(contactOnlyInstrument.name).click();
2142+
cy.finishedLoading();
2143+
2144+
cy.contains(proposal1.title).should('exist');
2145+
});
2146+
2147+
it('Instrument scientist should see proposals when filtering by their scientist instrument', () => {
2148+
cy.contains('Proposals');
2149+
2150+
selectAllProposalsFilterStatus();
2151+
2152+
cy.finishedLoading();
2153+
2154+
cy.get('[data-cy="instrument-filter"]').click();
2155+
cy.get('[role="listbox"]').contains(scientistInstrument.name).click();
2156+
cy.finishedLoading();
2157+
2158+
cy.contains(proposal2.title).should('exist');
2159+
});
2160+
2161+
it('Instrument contact should see their instrument on the instruments page', () => {
2162+
cy.visit('/Instruments');
2163+
2164+
cy.finishedLoading();
2165+
2166+
cy.contains(contactOnlyInstrument.name).should('exist');
2167+
2168+
cy.contains(scientistInstrument.name).should('exist');
2169+
});
2170+
});
20052171
});

0 commit comments

Comments
 (0)