feat: add export to excel functionality in reviewers and assignments tab in fap page - #1839
Bhaswati1148 wants to merge 15 commits into
Conversation
…ffice-core into 1574-export-fap-reviewers-review
…ffice-core into 1574-export-fap-reviewers-review
Bhaswati1148
changed the title
| .andWhere('call_id', callId); | ||
| .where('fap_id', fapId); | ||
|
|
||
| if (callId != 0) { |
There was a problem hiding this comment.
Is there a legitimate case where callId is 0?
There was a problem hiding this comment.
The callId is set to 0 when no calls are selected in the frontend.
|
|
||
| cy.get('[data-cy="submit"]').click(); | ||
|
|
||
| cy.notification({ |
There was a problem hiding this comment.
What's the motivation behind removing this?
There was a problem hiding this comment.
I removed the notification assertion because the success notification appears and disappears very quickly, and I couldn't find a reliable way for Cypress to get hold of it before it disappeared. This was making the test unreliable. The assignment itself is still verified by checking that the proposal appears in the FAP assignments table, so the test continues to validate the operation successfully.
| return; | ||
| } | ||
| const reviewerProposalMap = new Map<number, number[]>(); | ||
| // const reviewerIds = rowData.map((reviewer) => reviewer.user.userId); |
There was a problem hiding this comment.
Can these commented out lines please be removed?
| }); | ||
| const reviewerProposals = Object.fromEntries(reviewerProposalMap); | ||
| reviewerProposalMap.forEach((key, value) => { | ||
| console.log('reviewerid ' + key + `proposals ` + value); |
There was a problem hiding this comment.
This logging can be removed now, I reckon.
mutambaraf
left a comment
mutambaraf
left a comment
There was a problem hiding this comment.
LGTM apart form just a few comments
| const userWithRole = { | ||
| ...res.locals.agent, | ||
| }; |
There was a problem hiding this comment.
We may need to validate we are getting all the information we need from this object.
There was a problem hiding this comment.
Id is the information we need from userWithRole object which will be used further in the process. The middleware effectively validates the user’s id, it reads req.user.user.id, looks up that user with getAgent(id), and returns an unauthorized response if no user is found. It then puts the resulting user data into res.locals.agent. So I believe a second id check in the XLSX handler would usually be redundant.
| const reviewerProposalsParam = req.query.reviewerProposals; | ||
|
|
||
| if (typeof reviewerProposalsParam !== 'string') { | ||
| throw new Error('reviewerProposals is required'); |
There was a problem hiding this comment.
We can say Proposals reviewer is required
This branch has not been deployed
3 participants
Description
This PR adds the export to excel functionality in reviewers and assignments tab in fap page. The export creates multiple sheets if multiple users(reviewers) are selected.
The exported excel will have the columns : Proposal ID, Proposal title, Instrument, Date assigned, Rank, Grade, Comment, Status
Motivation and Context
This functionality has been added to allow the FAP Secretary in STFC to export reviewer reviews to Excel.
How Has This Been Tested
An E2E test has been added to verify the export functionality when the download button is clicked and to validate the contents of the exported Excel file.
Fixes
Closes UserOfficeProject/issue-tracker#1574
Changes
Depends on
Tests included/Docs Updated?