fix: proposal pdf download fails when instrument picker question is used in proposal question template #663
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| DO | ||
| $$ | ||
| BEGIN | ||
| IF register_patch('0158_UpdateOldInstrumentPickerAnswers.sql', 'Bhaswati Dey', 'Update old instrument picker answers', '2024-07-23') THEN | ||
| BEGIN | ||
| --QUERY TO UPDATE ANSWERS WITH SINGLE INSTRUMENT ID | ||
| UPDATE answers | ||
| SET answer = jsonb_set(answer, '{value}', jsonb_build_object('instrumentId', answer->>'value','timeRequested',null)) | ||
| WHERE jsonb_typeof(answer->'value')='number' and question_id IN (SELECT question_id FROM questions WHERE data_type='INSTRUMENT_PICKER' AND | ||
| (default_config->'isMultipleSelect' IS NULL OR default_config->>'isMultipleSelect' = 'false')); | ||
|
|
||
| --SQL BLOCK TO UPDATE ANSWERS WITH MULTIPLE INSTRUMENT IDS | ||
| DECLARE rec record; | ||
| BEGIN | ||
| FOR rec IN | ||
| WITH item AS ( | ||
| SELECT ('{value,' || pos - 1 || '}')::text[] AS path, answer->'value'->((pos - 1)::int) AS instrumentId, answer_id FROM answers, | ||
| jsonb_array_elements(answer->'value') WITH ordinality arr(item, pos) WHERE question_id IN ( | ||
| SELECT question_id FROM questions WHERE data_type='INSTRUMENT_PICKER' AND default_config->>'isMultipleSelect' = 'true') | ||
| )SELECT * FROM item | ||
|
|
||
| LOOP | ||
| IF jsonb_typeof(rec.instrumentId)='number' THEN | ||
| UPDATE answers a | ||
| SET answer = jsonb_set(answer, rec.path, jsonb_build_object('instrumentId',rec.instrumentId::text,'timeRequested',null)) | ||
| WHERE a.answer_id = rec.answer_id; | ||
| END IF; | ||
| END LOOP; | ||
| END; | ||
| END; | ||
| END IF; | ||
| END; | ||
| $$ | ||
| LANGUAGE plpgsql; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import { | |
| FeatureId, | ||
| } from '@user-office-software-libs/shared-types'; | ||
| import { DateTime } from 'luxon'; | ||
| import PdfParse from 'pdf-parse'; | ||
|
|
||
| import featureFlags from '../support/featureFlags'; | ||
| import initialDBData from '../support/initialDBData'; | ||
|
|
@@ -41,6 +42,8 @@ context('Proposal tests', () => { | |
| let createdWorkflowId: number; | ||
| let createdProposalPk: number; | ||
| let createdProposalId: string; | ||
| let createdCallId: number; | ||
| let createdTemplateId: number; | ||
| const textQuestion = faker.random.words(2); | ||
|
|
||
| const currentDayStart = DateTime.now().startOf('day'); | ||
|
|
@@ -1650,4 +1653,157 @@ context('Proposal tests', () => { | |
| ).should('exist'); | ||
| }); | ||
| }); | ||
| describe('Proposal PDF generation with instrument picker question', () => { | ||
| const instrumentPickerQuestion = 'Select your Instrument'; | ||
| const instrument = { | ||
| name: 'Instrument 1', | ||
| shortCode: 'Instrument 1', | ||
| description: 'Instrument 1', | ||
| managerUserId: initialDBData.users.user1.id, | ||
| }; | ||
| const instrument2 = { | ||
| name: 'Instrument 2', | ||
| shortCode: 'Instrument 2', | ||
| description: 'Instrument 2', | ||
| managerUserId: initialDBData.users.user1.id, | ||
| }; | ||
| let topicId: number; | ||
| let instrumentPickerQuestionId: string; | ||
|
|
||
| beforeEach(() => { | ||
| // NOTE: Stop the web application and clearly separate the end-to-end tests by visiting the blank about page after each test. | ||
| // This prevents flaky tests with some long-running network requests from one test to finish in the next and unexpectedly update the app. | ||
| cy.window().then((win) => { | ||
| win.location.href = 'about:blank'; | ||
| }); | ||
| cy.resetDB(); | ||
| cy.getAndStoreFeaturesEnabled(); | ||
| cy.createTemplate({ | ||
| name: 'Proposal Template with Instrument Picker', | ||
| groupId: TemplateGroupId.PROPOSAL, | ||
| }).then((result) => { | ||
| if (result.createTemplate) { | ||
| createdTemplateId = result.createTemplate.templateId; | ||
|
|
||
| cy.createTopic({ | ||
| templateId: createdTemplateId, | ||
| sortOrder: 1, | ||
| }).then((topicResult) => { | ||
| if (!topicResult.createTopic) { | ||
| throw new Error('Can not create topic'); | ||
| } | ||
|
|
||
| topicId = | ||
| topicResult.createTopic.steps[ | ||
| topicResult.createTopic.steps.length - 1 | ||
| ].topic.id; | ||
| cy.createQuestion({ | ||
| categoryId: TemplateCategoryId.PROPOSAL_QUESTIONARY, | ||
| dataType: DataType.INSTRUMENT_PICKER, | ||
| }).then((result) => { | ||
| instrumentPickerQuestionId = result.createQuestion.id; | ||
| cy.updateQuestion({ | ||
| id: instrumentPickerQuestionId, | ||
| question: instrumentPickerQuestion, | ||
| config: `{"variant":"dropdown","isMultipleSelect":false,"required":true,"requestTime":true}`, | ||
| }); | ||
| cy.createQuestionTemplateRelation({ | ||
| questionId: instrumentPickerQuestionId, | ||
| templateId: createdTemplateId, | ||
| sortOrder: 0, | ||
| topicId: topicId, | ||
| }); | ||
| }); | ||
| }); | ||
| } | ||
| }); | ||
| cy.createCall({ | ||
| ...newCall, | ||
| allocationTimeUnit: AllocationTimeUnits.DAY, | ||
| proposalWorkflowId: initialDBData.workflows.defaultWorkflow.id, | ||
| templateId: createdTemplateId, | ||
| }).then((response) => { | ||
| if (response.createCall) { | ||
| createdCallId = response.createCall.id; | ||
| cy.createInstrument(instrument).then((result) => { | ||
| cy.assignInstrumentToCall({ | ||
| callId: createdCallId, | ||
| instrumentFapIds: [{ instrumentId: result.createInstrument.id }], | ||
| }); | ||
| }); | ||
| cy.createInstrument(instrument2).then((result) => { | ||
| cy.assignInstrumentToCall({ | ||
| callId: createdCallId, | ||
| instrumentFapIds: [{ instrumentId: result.createInstrument.id }], | ||
| }); | ||
| }); | ||
| } | ||
| }); | ||
| cy.login('officer'); | ||
| cy.visit('/'); | ||
| }); | ||
| it('Should be able to download proposal pdf for a proposal which contains instrument picker question', function () { | ||
| if (!featureFlags.getEnabledFeatures().get(FeatureId.SCHEDULER)) { | ||
|
There was a problem hiding this comment. is there any reason we are not running these in e2e mode and only stfc mode? There was a problem hiding this comment. The test case was failing without this check and now I don't remember the message shown for the failed test case. There was a problem hiding this comment. This flag is often used to check which config we are running in (e2e vs stfc) so we are skipping the test in e2e mode (which aligns more closly with ess config) since this is not something that is stfc specific (correct me if im wrong) I think we should have it working in both modes |
||
| /*temporarily skipping, there is some issue on github actions which fails | ||
| when downloading pdfs specifically in stfc enviroment | ||
| (error message - Bad status code : 500) | ||
| This test passes in local environment. | ||
| */ | ||
| this.skip(); | ||
| } | ||
| cy.createProposal({ callId: createdCallId }).then((result) => { | ||
| if (result.createProposal) { | ||
| createdProposalPk = result.createProposal.primaryKey; | ||
| createdProposalId = result.createProposal.proposalId; | ||
| const proposal = result.createProposal; | ||
| cy.updateProposal({ | ||
| proposalPk: result.createProposal.primaryKey, | ||
| proposerId: proposer.id, | ||
| title: title, | ||
| abstract: abstract, | ||
| }); | ||
| cy.answerTopic({ | ||
| isPartialSave: false, | ||
| questionaryId: proposal.questionaryId, | ||
| topicId: topicId, | ||
| answers: [ | ||
| { | ||
| questionId: instrumentPickerQuestionId, | ||
| value: `{"value":{"instrumentId": "1", "timeRequested": "1"}}`, | ||
| }, | ||
| ], | ||
| }); | ||
| cy.submitProposal({ | ||
| proposalPk: result.createProposal.primaryKey, | ||
| }); | ||
|
|
||
| const token = window.localStorage.getItem('token'); | ||
|
|
||
| if (!token) { | ||
| throw new Error('Token not provided'); | ||
| } | ||
| const currentYear = new Date().getFullYear(); | ||
| const downloadedFileName = `${createdProposalId}_${initialDBData.users.officer.lastName}_${currentYear}.pdf`; | ||
| const downloadsFolder = Cypress.config('downloadsFolder'); | ||
| const downloadFilePath = `${downloadsFolder}/${downloadedFileName}`; | ||
|
|
||
| cy.task('downloadFile', { | ||
| url: `${Cypress.config( | ||
| 'baseUrl' | ||
| )}/download/pdf/proposal/${createdProposalPk}`, | ||
| token: token, | ||
| filename: downloadedFileName, | ||
| downloadsFolder: downloadsFolder, | ||
| }); | ||
|
|
||
| cy.task('readPdf', downloadFilePath).then((args) => { | ||
| const { text } = args as PdfParse.Result; | ||
| const instrumentPickerAnswer = 'Instrument 1 (1 day)'; | ||
| expect(text).to.include(title); | ||
| expect(text).to.include(instrumentPickerAnswer); | ||
| }); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try to run this against our develop database and get back with approval if everything works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Bhaswati1148 after running this migration locally I noticed that it works well when we had value just as number for example:
this answer:
was converted to this:
but it seems to have a problem when converting some other cases like for example:
this one:
was converted to this:
which I think it is wrong. It should be something like:
And then another example is where we have:
converted to:
which I also think it is wrong and it should be: