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

feat: fap data export for a call and refactor data collections for export #629

Merged
merged 64 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from 57 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
f1964aa
BREAKING CHANGE: multiple FAPs per proposal
martin-trajanovski Apr 3, 2024
fa1d0d1
Merge branch 'develop' of https://github.com/UserOfficeProject/user-o…
martin-trajanovski Apr 4, 2024
a742436
fix the assignment of proposal to multiple FAPs
martin-trajanovski Apr 4, 2024
20d1974
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski Apr 5, 2024
f88684b
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski Apr 8, 2024
5e0b5be
restructure and re-design of the proposal FAP assignments
martin-trajanovski Apr 9, 2024
27a8596
resolve merge conflicts
martin-trajanovski Apr 10, 2024
4dad407
try to improve the whole structure of the proposal view
martin-trajanovski Apr 10, 2024
1e2881e
fixing lint issues and finalizing the refactor
martin-trajanovski Apr 11, 2024
24a0909
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski Apr 11, 2024
0d09759
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski Apr 12, 2024
6e9c6bf
reduce data dupplication and make filters work with jsonb fields
martin-trajanovski Apr 12, 2024
4a0e8b8
Merge branch 'SWAP-3734-multi-fap-proposals' of https://github.com/Us…
martin-trajanovski Apr 12, 2024
aaf63a6
fix linting issues
martin-trajanovski Apr 15, 2024
a79d8a2
proposal model change to faps, do some cleanup and start fixing tests
martin-trajanovski Apr 15, 2024
98d310e
fix backend unit tests
martin-trajanovski Apr 15, 2024
a279e63
fix some bugs found in the e2e testing
martin-trajanovski Apr 16, 2024
5b96972
fix some more bugs found while testing
martin-trajanovski Apr 16, 2024
5342ade
improve FAP e2e tests
martin-trajanovski Apr 16, 2024
07aadb1
fix failing e2e tests
martin-trajanovski Apr 16, 2024
861d8a0
refactor and fix last failing e2e tests
martin-trajanovski Apr 17, 2024
fbef88b
find and fix problematic e2e test
martin-trajanovski Apr 17, 2024
663448c
fixing very last bits of failing tests
martin-trajanovski Apr 17, 2024
45f0fc1
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski Apr 17, 2024
048e5e8
fix the auto assignment of multiple FAPs and add e2e test
martin-trajanovski Apr 17, 2024
ec5bf9e
Merge branch 'SWAP-3734-multi-fap-proposals' of https://github.com/Us…
martin-trajanovski Apr 17, 2024
71aca5a
FAP auto assignment test fix
martin-trajanovski Apr 18, 2024
5947277
fix newly added test
martin-trajanovski Apr 18, 2024
af6ede6
resolve merge conflicts
martin-trajanovski Apr 18, 2024
edcc8d3
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski Apr 18, 2024
2ae0866
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski Apr 19, 2024
3a7139b
fix failing e2e test
martin-trajanovski Apr 22, 2024
4870b3e
Update 0151_MultipleFapPerProposal.sql
martin-trajanovski Apr 23, 2024
42a8d9e
resolve merge conflicts
martin-trajanovski Apr 24, 2024
5c844e9
fix proposal table name in select
martin-trajanovski Apr 24, 2024
80753a1
improve auto fap assignment after instrument
martin-trajanovski Apr 24, 2024
c6d1f9b
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski Apr 24, 2024
8d8732f
remove some leftover commented code
martin-trajanovski Apr 24, 2024
a0eb9f0
refactor: make fap data export view and clean up code
TCMeldrum Apr 29, 2024
7acfd8d
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski Apr 30, 2024
15a94b0
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski May 2, 2024
2f87c1d
fix review comments and refactor
martin-trajanovski May 8, 2024
9c7574e
fixing review comments part 2
martin-trajanovski May 8, 2024
843a55f
add e2e test that covers some review improvements
martin-trajanovski May 8, 2024
be781c7
fix unit and e2e tests
martin-trajanovski May 8, 2024
4140ac2
wip
TCMeldrum May 8, 2024
f2f2bf3
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski May 13, 2024
5f04967
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski May 16, 2024
4e6e8e0
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski May 20, 2024
4a05582
Merge branch 'develop' into SWAP-3734-multi-fap-proposals
martin-trajanovski May 21, 2024
8abe212
Merge branch 'develop' of github.com:UserOfficeProject/user-office-co…
TCMeldrum May 21, 2024
0b3c7fc
Merge branch 'SWAP-3734-multi-fap-proposals' of github.com:UserOffice…
TCMeldrum May 23, 2024
52f8794
wip
TCMeldrum May 23, 2024
1a6dfae
feat: add all call fap export
TCMeldrum May 24, 2024
fa15274
Merge branch 'develop' of github.com:UserOfficeProject/user-office-co…
TCMeldrum Jun 13, 2024
d8e9b40
missed merge clean up
TCMeldrum Jun 13, 2024
6d1b69c
Merge branch 'develop' into 1050-fap-data-export
TCMeldrum Jun 14, 2024
a95dee0
small fixes
TCMeldrum Jun 24, 2024
3d2bfa5
Merge branch 'develop' into 1050-fap-data-export
TCMeldrum Jun 24, 2024
5f7e7b4
Merge remote-tracking branch 'origin' into 1050-fap-data-export
TCMeldrum Jul 22, 2024
1690474
small improvments
TCMeldrum Jul 22, 2024
dc5d8ae
update to work with non stfc modes
TCMeldrum Aug 1, 2024
c05c9f1
add test
TCMeldrum Aug 1, 2024
cf8f670
Merge branch 'develop' of github.com:UserOfficeProject/user-office-co…
TCMeldrum Aug 9, 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
40 changes: 40 additions & 0 deletions apps/backend/db_patches/0156_CreateFAPProposalView.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
DO
$$
BEGIN
IF register_patch('0156_CreateFAPProposalView.sql', 'Thomas Cottee Meldrum', 'Create View for FAP data', '2024-04-05') THEN

CREATE VIEW review_data AS
Select proposal.*, grade.avg as average_grade from (
Select
fp.proposal_pk,
p.proposal_id,
p.title,
i.name as instrument_name,
chi.availability_time,
tr.time_allocation,
f.fap_id,
fmd.rank_order,
c.call_id,
p.proposer_id,
i.instrument_id,
fp.fap_time_allocation,
p.questionary_id
from fap_proposals as fp
join faps as f on f.fap_id = fp.fap_id
join call c on c.call_id = fp.call_id
join proposals p on p.proposal_pk = fp.proposal_pk
join technical_review tr on tr.proposal_pk = p.proposal_pk and tr.instrument_id = fp.instrument_id
left join fap_meeting_decisions as fmd on fmd.proposal_pk = p.proposal_pk
join call_has_instruments as chi on chi.instrument_id = fp.instrument_id and chi.call_id = c.call_id
join instruments as i on i.instrument_id = chi.instrument_id) as proposal
left join (
Select fr.proposal_pk, AVG(fr.grade) from
fap_proposals as fp
join fap_reviews as fr on fr.proposal_pk = fp.proposal_pk
group by fr.proposal_pk
) grade on grade.proposal_pk = proposal.proposal_pk;

END IF;
END;
$$
LANGUAGE plpgsql;
6 changes: 5 additions & 1 deletion apps/backend/src/datasources/FapDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ import {
import { RemoveProposalsFromFapsArgs } from '../resolvers/mutations/AssignProposalsToFapsMutation';
import { SaveFapMeetingDecisionInput } from '../resolvers/mutations/FapMeetingDecisionMutation';
import { FapsFilter } from '../resolvers/queries/FapsQuery';
import { AssignProposalsToFapsInput } from './postgres/records';
import {
FapReviewsRecord,
AssignProposalsToFapsInput,
} from './postgres/records';

export interface FapDataSource {
create(
Expand Down Expand Up @@ -152,4 +155,5 @@ export interface FapDataSource {
reviewerId: number,
rank: number
): Promise<boolean>;
getFapReviewData(callId: number, fapId: number): Promise<FapReviewsRecord[]>;
}
9 changes: 8 additions & 1 deletion apps/backend/src/datasources/mockups/FapDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ import { RemoveProposalsFromFapsArgs } from '../../resolvers/mutations/AssignPro
import { SaveFapMeetingDecisionInput } from '../../resolvers/mutations/FapMeetingDecisionMutation';
import { FapsFilter } from '../../resolvers/queries/FapsQuery';
import { FapDataSource } from '../FapDataSource';
import { AssignProposalsToFapsInput } from '../postgres/records';
import {
FapReviewsRecord,
AssignProposalsToFapsInput,
} from '../postgres/records';
import { basicDummyUser } from './UserDataSource';

export const dummyFap = new Fap(
Expand Down Expand Up @@ -738,4 +741,8 @@ export class FapDataSourceMock implements FapDataSource {
): Promise<boolean> {
throw new Error('Method not implemented.');
}

getFapReviewData(callId: number, fapId: number): Promise<FapReviewsRecord[]> {
throw new Error('Method not implemented.');
}
}
12 changes: 12 additions & 0 deletions apps/backend/src/datasources/postgres/FapDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import {
InstitutionRecord,
AssignProposalsToFapsInput,
CountryRecord,
FapReviewsRecord,
} from './records';

@injectable()
Expand Down Expand Up @@ -1275,4 +1276,15 @@ export default class PostgresFapDataSource implements FapDataSource {
return true;
});
}

async getFapReviewData(
callId: number,
fapId: number
): Promise<FapReviewsRecord[]> {
return await database
.select('*')
.from('review_data')
.where('fap_id', fapId)
.andWhere('call_id', callId);
}
}
17 changes: 17 additions & 0 deletions apps/backend/src/datasources/postgres/records.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,23 @@ export interface FapAssignmentRecord {
readonly rank: number | null;
}

export interface FapReviewsRecord {
readonly proposal_pk: number;
readonly proposal_id: number;
readonly title: string;
readonly instrument_name: string;
readonly availability_time: number;
readonly time_allocation: number;
readonly fap_id: number;
readonly rank_order: number;
readonly call_id: number;
readonly proposer_id: number;
readonly instrument_id: number;
readonly fap_time_allocation: number;
readonly average_grade: number;
readonly questionary_id: number;
}

export interface FapReviewerRecord {
readonly user_id: number;
readonly fap_id: number;
Expand Down
3 changes: 2 additions & 1 deletion apps/backend/src/factory/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ export enum DownloadType {

export enum XLSXType {
PROPOSAL = 'proposal',
Fap = 'fap',
FAP = 'fap',
CALL_FAP = 'call_fap',
}

export enum PDFType {
Expand Down
37 changes: 18 additions & 19 deletions apps/backend/src/factory/xlsx/FapDataRow.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,28 @@
import { FapProposal } from '../../models/Fap';
import { FapMeetingDecision } from '../../models/FapMeetingDecision';
import { InstrumentWithAvailabilityTime } from '../../models/Instrument';
import { Proposal } from '../../models/Proposal';
import { TechnicalReview } from '../../models/TechnicalReview';
import { RowObj } from './fap';

export function getDataRow(
proposalPk: number,
piName: string,
proposalAverageScore: number,
instrument: InstrumentWithAvailabilityTime,
fapMeetingDecision: FapMeetingDecision | null,
proposal: Proposal | null,
technicalReview: TechnicalReview | null,
fapProposal: FapProposal | null
) {
instrumentName: string,
instrumentAvailabilityTime: number,
fapTimeAllocation: number | null,
proposalTitle: string,
proposalId: number | null,
techReviewTimeAllocation: number | null,
propFapRankOrder: number | null
): RowObj {
return {
propShortCode: proposal?.proposalId,
propTitle: proposal?.title,
proposalPk: proposalPk,
propShortCode: proposalId?.toString(),
propTitle: proposalTitle,
principalInv: piName,
instrName: instrument.name,
instrAvailTime: instrument.availabilityTime,
techReviewTimeAllocation: technicalReview?.timeAllocation,
fapTimeAllocation: fapProposal?.fapTimeAllocation ?? null,
propReviewAvgScore: proposalAverageScore,
propFapRankOrder: fapMeetingDecision?.rankOrder ?? null,
instrName: instrumentName,
instrAvailTime: instrumentAvailabilityTime,
techReviewTimeAllocation: techReviewTimeAllocation,
fapTimeAllocation: fapTimeAllocation ?? null,
propReviewAvgScore: proposalAverageScore ?? 0,
propFapRankOrder: propFapRankOrder ?? null,
inAvailZone: null,
};
}
Expand Down
177 changes: 177 additions & 0 deletions apps/backend/src/factory/xlsx/callFaps.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
// import { Review } from '../../models/Review';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be removed?

import { stripHtml } from 'string-strip-html';
import { container } from 'tsyringe';

import baseContext from '../../buildContext';
import { Tokens } from '../../config/Tokens';
import { FapDataSource } from '../../datasources/FapDataSource';
import { ProposalEndStatus } from '../../models/Proposal';
import { UserWithRole } from '../../models/User';
import { RowObj, collectFapXLSXRowData } from './fap';

const fapDataSource: FapDataSource = container.resolve(Tokens.FapDataSource);

const ProposalEndStatusStringValue = {
[ProposalEndStatus.UNSET]: 'Unset',
[ProposalEndStatus.ACCEPTED]: 'Accepted',
[ProposalEndStatus.RESERVED]: 'Reserved',
[ProposalEndStatus.REJECTED]: 'Rejected',
};

export type CallRowRowObj = RowObj & {
TCMeldrum marked this conversation as resolved.
Show resolved Hide resolved
fapMeetingDecision?: string | null;
fapMeetingExComment?: string | null;
fapMeetingInComment?: string | null;
};

export const collectCallFapXLSXData = async (
callId: number,
user: UserWithRole
) => {
const faps = await baseContext.queries.fap.dataSource.getFapsByCallId(callId);
const call = await baseContext.queries.call.get(user, callId);
const filename = `${call?.shortCode}_FAP_Results.xlsx`;

const baseData = await Promise.all(
faps.map(async (fap) => {
return {
sheetName: fap.code,
rows: await collectFAPRowData(fap.id, callId, user),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this collectFAPRowData function is used before it is defined. Maybe it is good to move it on the top.

};
})
);

return { data: baseData, filename: filename.replace(/\s+/g, '_') };
};

const collectFAPRowData = async (
fapId: number,
callId: number,
user: UserWithRole
) => {
const data = await collectFapXLSXRowData(fapId, callId, user);

const extraData = await Promise.all(
data.map(async (sheet) => {
return {
sheetName: sheet.sheetName,
rows: await Promise.all(
sheet.rows.map(async (proposal) => {
const fapMeetingDecision =
await fapDataSource.getProposalsFapMeetingDecisions([
proposal.proposalPk,
]);

return {
...proposal,
fapMeetingDecision: fapMeetingDecision[0]
? ProposalEndStatusStringValue[
fapMeetingDecision[0].recommendation
]
: null,
fapMeetingExComment: fapMeetingDecision[0]
? stripHtml(fapMeetingDecision[0].commentForUser).result
: null,
fapMeetingInComment: fapMeetingDecision[0]
? stripHtml(fapMeetingDecision[0].commentForManagement).result
: null,
};
})
),
};
})
);

const allRowData = extraData.map((inst) => {
const instName: (string | number)[][] = [[inst.sheetName]];

const sortedData = sortByRankOrAverageScore(inst.rows).map(
(row: CallRowRowObj) => populateRow(row)
);

instName.concat(instName);
TCMeldrum marked this conversation as resolved.
Show resolved Hide resolved

return instName.concat(sortedData);
});

return allRowData.length
? allRowData.reduce((arr, inst) => {
return arr.concat(inst);
})
: allRowData;
};

function populateRow(row: CallRowRowObj): (string | number)[] {
const individualReviews = row.reviews?.flatMap((rev) => [
rev.grade,
rev.comment && stripHtml(rev.comment).result,
]);

return [
row.propShortCode ?? '<missing>',
row.principalInv ?? '<missing>',
row.piCountry ?? '<missing>',
row.instrName ?? '<missing>',
row.daysRequested ?? '<missing>',
row.propTitle ?? '<missing>',
row.propReviewAvgScore ?? '<missing>',
row.fapTimeAllocation ?? row.daysRequested ?? '<missing>',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be made clear whether the user is looking at the time allocated or requested.

row.fapMeetingDecision ?? '<missing>',
row.fapMeetingInComment ?? '<missing>',
row.fapMeetingExComment ?? '<missing>',
].concat(individualReviews ? individualReviews : []);
}

export const CallFapDataColumns = [
'Proposal Reference Number',
'Principal Investigator',
'PI Country',
'Instrument Name',
'Requested Time',
'Proposal Title',
'Average score',
'Fap Time allocation',
'Fap Meeting Decision',
'Fap Meeting Comment for User',
'Fap Meeting Internal Comment',
'Reviewer 1 score',
'Reviewer 1 review comment',
'Reviewer 2 score',
'Reviewer 2 review comment',
];

const sortByRankOrder = (a: RowObj, b: RowObj) => {
if (a.propFapRankOrder === b.propFapRankOrder) {
return -1;
} else if (a.propFapRankOrder === null) {
return 1;
} else if (b.propFapRankOrder === null) {
return -1;
} else {
return a.propFapRankOrder > b.propFapRankOrder ? 1 : -1;
}
};

const sortByRankOrAverageScore = (data: RowObj[]) => {
let allocationTimeSum = 0;

return data
.sort((a, b) =>
(a.propReviewAvgScore || 0) > (b.propReviewAvgScore || 0) ? 1 : -1
)
.sort(sortByRankOrder)
.map((row) => {
const proposalAllocationTime =
row.fapTimeAllocation !== null
? row.fapTimeAllocation
: row.techReviewTimeAllocation || 0;

const isInAvailabilityZone =
allocationTimeSum + proposalAllocationTime <= (row.instrAvailTime || 0);
allocationTimeSum = allocationTimeSum + proposalAllocationTime;

row.inAvailZone = isInAvailabilityZone ? 'yes' : 'no';

return row;
});
};
Loading
Loading