diff --git a/frontend/src/components/Navigator/ActivityReportNavigator.js b/frontend/src/components/Navigator/ActivityReportNavigator.js index cb61e4c907..c013d22a5c 100644 --- a/frontend/src/components/Navigator/ActivityReportNavigator.js +++ b/frontend/src/components/Navigator/ActivityReportNavigator.js @@ -16,7 +16,7 @@ import { saveGoalsForReport, saveObjectivesForReport } from '../../fetchers/acti import GoalFormContext from '../../GoalFormContext'; import { validateObjectives } from '../../pages/ActivityReport/Pages/components/objectiveValidator'; import AppLoadingContext from '../../AppLoadingContext'; -import { convertGoalsToFormData } from '../../pages/ActivityReport/formDataHelpers'; +import { convertGoalsToFormData, packageGoals } from '../../pages/ActivityReport/formDataHelpers'; import { objectivesWithValidResourcesOnly, validateListOfResources } from '../GoalForm/constants'; import Navigator from '.'; @@ -66,28 +66,6 @@ export function getPromptErrors(promptTitles, errors) { export const formatEndDate = (formEndDate) => ((formEndDate && formEndDate.toLowerCase() !== 'invalid date') ? formEndDate : ''); -export const packageGoals = (goals, goal, grantIds, prompts) => { - const packagedGoals = [ - // we make sure to mark all the read only goals as "ActivelyEdited: false" - ...goals.map((g) => ({ - ...g, - grantIds, - isActivelyBeingEditing: false, - prompts: grantIds.length < 2 ? g.prompts : [], - })), - ]; - - if (goal && goal.name) { - packagedGoals.push({ - ...goal, - grantIds, - prompts: grantIds.length < 2 ? prompts : [], - }); - } - - return packagedGoals; -}; - /** * @summary checks to see if the tta provided field contains the cursor * if it does, we don't want to update the form data diff --git a/frontend/src/components/Navigator/__tests__/ActivityReportNavigator.js b/frontend/src/components/Navigator/__tests__/ActivityReportNavigator.js index a909b041c0..a84d2b4e49 100644 --- a/frontend/src/components/Navigator/__tests__/ActivityReportNavigator.js +++ b/frontend/src/components/Navigator/__tests__/ActivityReportNavigator.js @@ -12,7 +12,6 @@ import { useFormContext } from 'react-hook-form'; import Navigator, { getPrompts, getPromptErrors, - packageGoals, shouldUpdateFormData, formatEndDate, } from '../ActivityReportNavigator'; @@ -476,133 +475,3 @@ describe('getPromptErrors', () => { expect(getPromptErrors(null, errors)).toBe(false); }); }); - -describe('packageGoals', () => { - it('correctly formats goals with multiple recipients', () => { - const grantIds = [1, 2]; - const packagedGoals = packageGoals( - [ - { - name: 'goal name', - endDate: '09/01/2020', - prompts: [{ fieldName: 'prompt' }], - }, - ], - { - name: 'recipient', - endDate: '09/01/2020', - isActivelyBeingEditing: true, - }, - grantIds, - [{ fieldName: 'prompt2' }], - ); - - expect(packagedGoals).toEqual([ - { - name: 'goal name', - endDate: '09/01/2020', - prompts: [], - grantIds, - isActivelyBeingEditing: false, - }, - { - name: 'recipient', - endDate: '09/01/2020', - isActivelyBeingEditing: true, - grantIds, - prompts: [], - }, - ]); - }); - - it('correctly formats goals for a single recipient', () => { - const grantIds = [1]; - const packagedGoals = packageGoals( - [ - { - name: 'goal name', - endDate: '09/01/2020', - prompts: [{ fieldName: 'prompt' }], - }, - ], - { - name: 'recipient', - endDate: '09/01/2020', - isActivelyBeingEditing: true, - }, - grantIds, - [{ fieldName: 'prompt2' }], - ); - - expect(packagedGoals).toEqual([ - { - name: 'goal name', - endDate: '09/01/2020', - prompts: [{ fieldName: 'prompt' }], - grantIds, - isActivelyBeingEditing: false, - }, - { - name: 'recipient', - endDate: '09/01/2020', - isActivelyBeingEditing: true, - grantIds, - prompts: [{ fieldName: 'prompt2' }], - }, - ]); - }); - it('skips returning edited goal if edited goal is null', () => { - const grantIds = [1]; - const packagedGoals = packageGoals( - [ - { - name: 'goal name', - endDate: '09/01/2020', - prompts: [{ fieldName: 'prompt' }], - }, - ], - null, - grantIds, - [{ fieldName: 'prompt2' }], - ); - - expect(packagedGoals).toEqual([ - { - name: 'goal name', - endDate: '09/01/2020', - prompts: [{ fieldName: 'prompt' }], - grantIds, - isActivelyBeingEditing: false, - }, - ]); - }); - it('skips returning edited goal if edited goal has no name', () => { - const grantIds = [1]; - const packagedGoals = packageGoals( - [ - { - name: 'goal name', - endDate: '09/01/2020', - prompts: [{ fieldName: 'prompt' }], - }, - ], - { - name: '', - endDate: '', - isActivelyBeingEditing: true, - }, - grantIds, - [{ fieldName: 'prompt2' }], - ); - - expect(packagedGoals).toEqual([ - { - name: 'goal name', - endDate: '09/01/2020', - prompts: [{ fieldName: 'prompt' }], - grantIds, - isActivelyBeingEditing: false, - }, - ]); - }); -}); diff --git a/frontend/src/pages/ActivityReport/__tests__/formDataHelpers.js b/frontend/src/pages/ActivityReport/__tests__/formDataHelpers.js index 1f09aa3e4d..c9fd485e30 100644 --- a/frontend/src/pages/ActivityReport/__tests__/formDataHelpers.js +++ b/frontend/src/pages/ActivityReport/__tests__/formDataHelpers.js @@ -1,258 +1,426 @@ import { REPORT_STATUSES } from '@ttahub/common'; -import { convertGoalsToFormData } from '../formDataHelpers'; +import { convertGoalsToFormData, packageGoals } from '../formDataHelpers'; -describe('convertGoalsToFormData', () => { - it('converts', () => { - const { goals, goalForEditing } = convertGoalsToFormData( - [ +describe('FormDataHelpers', () => { + describe('packageGoals', () => { + const baseGoal = { + objectives: [], + goalIds: [1, 2, 3], + onApprovedAR: true, + source: 'Recipient request', + status: 'In progress', + }; + + it('correctly formats goals with multiple recipients', () => { + const grantIds = [1, 2]; + const packagedGoals = packageGoals( + [ + { + ...baseGoal, + name: 'goal name', + endDate: '09/01/2020', + prompts: [{ fieldName: 'prompt' }], + objectives: [], + }, + ], { - id: 1, + ...baseGoal, + name: 'recipient', + endDate: '09/01/2020', + isActivelyBeingEditing: true, objectives: [], - activityReportGoals: [ - { - id: 1, - isActivelyEdited: true, - }, - ], - source: 'Source', }, + grantIds, + [{ fieldName: 'prompt2' }], + ); + + expect(packagedGoals).toEqual([ { - id: 2, + ...baseGoal, + name: 'goal name', + endDate: '09/01/2020', + prompts: [], + grantIds, + isActivelyBeingEditing: false, + objectives: [], + }, + { + ...baseGoal, + name: 'recipient', + endDate: '09/01/2020', + isActivelyBeingEditing: true, + grantIds, + prompts: [], + objectives: [], + }, + ]); + }); + + it('correctly formats goals for a single recipient', () => { + const grantIds = [1]; + const packagedGoals = packageGoals( + [ + { + ...baseGoal, + name: 'goal name', + endDate: '09/01/2020', + prompts: [{ fieldName: 'prompt' }], + objectives: [], + }, + ], + { + ...baseGoal, + name: 'recipient', + objectives: [], + endDate: '09/01/2020', + isActivelyBeingEditing: true, + }, + grantIds, + [{ fieldName: 'prompt2' }], + ); + + expect(packagedGoals).toEqual([ + { + ...baseGoal, + name: 'goal name', + endDate: '09/01/2020', + prompts: [{ fieldName: 'prompt' }], + grantIds, + isActivelyBeingEditing: false, + }, + { + ...baseGoal, + name: 'recipient', + endDate: '09/01/2020', + isActivelyBeingEditing: true, + grantIds, + prompts: [{ fieldName: 'prompt2' }], + }, + ]); + }); + it('skips returning edited goal if edited goal is null', () => { + const grantIds = [1]; + const packagedGoals = packageGoals( + [ + { + ...baseGoal, + name: 'goal name', + endDate: '09/01/2020', + prompts: [{ fieldName: 'prompt' }], + }, + ], + null, + grantIds, + [{ fieldName: 'prompt2' }], + ); + + expect(packagedGoals).toEqual([ + { + ...baseGoal, + name: 'goal name', + endDate: '09/01/2020', + prompts: [{ fieldName: 'prompt' }], + grantIds, + isActivelyBeingEditing: false, + }, + ]); + }); + it('skips returning edited goal if edited goal has no name', () => { + const grantIds = [1]; + const packagedGoals = packageGoals( + [ + { + ...baseGoal, + name: 'goal name', + endDate: '09/01/2020', + prompts: [{ fieldName: 'prompt' }], + objectives: [], + }, + ], + { + ...baseGoal, + name: '', + endDate: '', + isActivelyBeingEditing: true, + objectives: [], + }, + grantIds, + [{ fieldName: 'prompt2' }], + ); + + expect(packagedGoals).toEqual([ + { + ...baseGoal, + name: 'goal name', + endDate: '09/01/2020', + prompts: [{ fieldName: 'prompt' }], + grantIds, + isActivelyBeingEditing: false, objectives: [], + }, + ]); + }); + }); + + describe('convertGoalsToFormData', () => { + it('converts', () => { + const { goals, goalForEditing } = convertGoalsToFormData( + [ + { + id: 1, + objectives: [], + activityReportGoals: [ + { + id: 1, + isActivelyEdited: true, + }, + ], + source: 'Source', + }, + { + id: 2, + objectives: [], + source: '', + activityReportGoals: [ + { + id: 2, + isActivelyEdited: false, + }, + ], + }, + ], + [1, 2, 3], + REPORT_STATUSES.DRAFT, + ); + + expect(goals).toEqual([ + { + id: 2, + grantIds: [1, 2, 3], source: '', + prompts: [], + objectives: [], activityReportGoals: [ - { - id: 2, - isActivelyEdited: false, - }, + { id: 2, isActivelyEdited: false }, ], }, - ], - [1, 2, 3], - REPORT_STATUSES.DRAFT, - ); + ]); - expect(goals).toEqual([ - { - id: 2, - grantIds: [1, 2, 3], + expect(goalForEditing).toEqual({ + id: 1, source: '', prompts: [], + grantIds: [1, 2, 3], objectives: [], activityReportGoals: [ - { id: 2, isActivelyEdited: false }, + { id: 1, isActivelyEdited: true }, ], - }, - ]); - - expect(goalForEditing).toEqual({ - id: 1, - source: '', - prompts: [], - grantIds: [1, 2, 3], - objectives: [], - activityReportGoals: [ - { id: 1, isActivelyEdited: true }, - ], + }); }); - }); - it('only returns one goalForEditing (even if more than one activityreportgoal has isActivelyEditing: true', () => { - const { goals, goalForEditing } = convertGoalsToFormData( - [ - { - id: 1, - activityReportGoals: [ - { - id: 1, - isActivelyEdited: true, - }, - ], - prompts: [], - }, + it('only returns one goalForEditing (even if more than one activityreportgoal has isActivelyEditing: true', () => { + const { goals, goalForEditing } = convertGoalsToFormData( + [ + { + id: 1, + activityReportGoals: [ + { + id: 1, + isActivelyEdited: true, + }, + ], + prompts: [], + }, + { + id: 2, + activityReportGoals: [ + { + id: 3, + isActivelyEdited: true, + }, + ], + prompts: [], + }, + ], + [1, 2, 3], + REPORT_STATUSES.DRAFT, + ); + + expect(goals).toEqual([ { id: 2, + grantIds: [1, 2, 3], activityReportGoals: [ { id: 3, isActivelyEdited: true, }, ], + source: '', prompts: [], }, - ], - [1, 2, 3], - REPORT_STATUSES.DRAFT, - ); + ]); - expect(goals).toEqual([ - { - id: 2, + expect(goalForEditing).toEqual({ + id: 1, grantIds: [1, 2, 3], + source: '', activityReportGoals: [ { - id: 3, + id: 1, isActivelyEdited: true, }, ], - source: '', prompts: [], - }, - ]); - - expect(goalForEditing).toEqual({ - id: 1, - grantIds: [1, 2, 3], - source: '', - activityReportGoals: [ - { - id: 1, - isActivelyEdited: true, - }, - ], - prompts: [], + }); }); - }); - it('returns an empty goalForEditing if no activityreportgoal has isActivelyEditing: true', () => { - const { goals, goalForEditing } = convertGoalsToFormData( - [ + it('returns an empty goalForEditing if no activityreportgoal has isActivelyEditing: true', () => { + const { goals, goalForEditing } = convertGoalsToFormData( + [ + { + id: 1, + + activityReportGoals: [ + { + id: 1, + isActivelyEdited: false, + }, + ], + prompts: [], + }, + { + id: 2, + + activityReportGoals: [ + { + id: 3, + isActivelyEdited: false, + }, + ], + prompts: [], + }, + ], + [1, 2, 3], + REPORT_STATUSES.DRAFT, + ); + + expect(goals).toEqual([ { id: 1, + grantIds: [1, 2, 3], + source: '', + prompts: [], activityReportGoals: [ { id: 1, isActivelyEdited: false, }, ], - prompts: [], }, { id: 2, + source: '', + prompts: [], + grantIds: [1, 2, 3], activityReportGoals: [ { id: 3, isActivelyEdited: false, }, ], - prompts: [], }, - ], - [1, 2, 3], - REPORT_STATUSES.DRAFT, - ); + ]); - expect(goals).toEqual([ - { - id: 1, - grantIds: [1, 2, 3], - source: '', - prompts: [], - activityReportGoals: [ + expect(goalForEditing).toEqual(null); + }); + it('returns an empty goalForEditing if activityreportgoal is not set', () => { + const { goals, goalForEditing } = convertGoalsToFormData( + [ { id: 1, - isActivelyEdited: false, + }, - ], - }, - { - id: 2, - source: '', - prompts: [], - grantIds: [1, 2, 3], - activityReportGoals: [ { - id: 3, - isActivelyEdited: false, + id: 2, + }, ], - }, - ]); + [1, 2, 3], + REPORT_STATUSES.DRAFT, + ); - expect(goalForEditing).toEqual(null); - }); - it('returns an empty goalForEditing if activityreportgoal is not set', () => { - const { goals, goalForEditing } = convertGoalsToFormData( - [ + expect(goals).toEqual([ { id: 1, + grantIds: [1, 2, 3], + source: '', + prompts: [], }, { id: 2, + grantIds: [1, 2, 3], + source: '', + prompts: [], }, - ], - [1, 2, 3], - REPORT_STATUSES.DRAFT, - ); + ]); - expect(goals).toEqual([ - { - id: 1, - grantIds: [1, 2, 3], - source: '', - prompts: [], - }, - { - id: 2, - grantIds: [1, 2, 3], - source: '', - prompts: [], - }, - ]); + expect(goalForEditing).toEqual(null); + }); + it('returns an empty goalForEditing if the goal is submitted', () => { + const { goals, goalForEditing } = convertGoalsToFormData( + [ + { + id: 1, + activityReportGoals: [ + { + id: 1, + isActivelyEdited: true, + }, + ], - expect(goalForEditing).toEqual(null); - }); - it('returns an empty goalForEditing if the goal is submitted', () => { - const { goals, goalForEditing } = convertGoalsToFormData( - [ + }, + { + id: 2, + activityReportGoals: [ + { + id: 3, + isActivelyEdited: true, + }, + ], + + }, + ], + [1, 2, 3], + REPORT_STATUSES.SUBMITTED, + ); + + expect(goals).toEqual([ { id: 1, + grantIds: [1, 2, 3], activityReportGoals: [ { id: 1, isActivelyEdited: true, }, ], + source: '', + prompts: [], }, { id: 2, + grantIds: [1, 2, 3], activityReportGoals: [ { id: 3, isActivelyEdited: true, }, ], + source: '', + prompts: [], }, - ], - [1, 2, 3], - REPORT_STATUSES.SUBMITTED, - ); + ]); - expect(goals).toEqual([ - { - id: 1, - grantIds: [1, 2, 3], - activityReportGoals: [ - { - id: 1, - isActivelyEdited: true, - }, - ], - source: '', - prompts: [], - }, - { - id: 2, - grantIds: [1, 2, 3], - activityReportGoals: [ - { - id: 3, - isActivelyEdited: true, - }, - ], - source: '', - prompts: [], - }, - ]); - - expect(goalForEditing).toEqual(null); + expect(goalForEditing).toEqual(null); + }); }); }); diff --git a/frontend/src/pages/ActivityReport/formDataHelpers.js b/frontend/src/pages/ActivityReport/formDataHelpers.js index 36d46c1119..1d81aa02bc 100644 --- a/frontend/src/pages/ActivityReport/formDataHelpers.js +++ b/frontend/src/pages/ActivityReport/formDataHelpers.js @@ -102,6 +102,68 @@ export const unflattenResourcesUsed = (array) => { return array.map((value) => ({ value })); }; +export const packageGoals = (goals, goal, grantIds, prompts) => { + const packagedGoals = [ + // we make sure to mark all the read only goals as "ActivelyEdited: false" + ...goals.map((g) => ({ + goalIds: g.goalIds, + status: g.status, + endDate: g.endDate, + onApprovedAR: g.onApprovedAR, + source: g.source, + name: g.name, + grantIds, + id: g.id, + isActivelyBeingEditing: false, + prompts: grantIds.length < 2 ? g.prompts : [], + objectives: g.objectives.map((objective) => ({ + id: objective.id, + isNew: objective.isNew, + ttaProvided: objective.ttaProvided, + title: objective.title, + status: objective.status, + resources: objective.resources, + topics: objective.topics, + files: objective.files, + supportType: objective.supportType, + courses: objective.courses, + closeSuspendReason: objective.closeSuspendReason, + closeSuspendContext: objective.closeSuspendContext, + })), + })), + ]; + + if (goal && goal.name) { + packagedGoals.push({ + goalIds: goal.goalIds, + status: goal.status, + endDate: goal.endDate, + onApprovedAR: goal.onApprovedAR, + source: goal.source, + name: goal.name, + isActivelyBeingEditing: goal.isActivelyBeingEditing, + objectives: goal.objectives.map((objective) => ({ + id: objective.id, + isNew: objective.isNew, + ttaProvided: objective.ttaProvided, + title: objective.title, + status: objective.status, + resources: objective.resources, + topics: objective.topics, + files: objective.files, + supportType: objective.supportType, + courses: objective.courses, + closeSuspendReason: objective.closeSuspendReason, + closeSuspendContext: objective.closeSuspendContext, + })), + grantIds, + prompts: grantIds.length < 2 ? prompts : [], + }); + } + + return packagedGoals; +}; + // this function takes goals returned from the API and parses them appropriately, // setting the editable goal (or at least doing its best guess) /** diff --git a/src/goalServices/getGoalsForReport.ts b/src/goalServices/getGoalsForReport.ts index de882c5dc8..f6528d8b24 100644 --- a/src/goalServices/getGoalsForReport.ts +++ b/src/goalServices/getGoalsForReport.ts @@ -32,6 +32,16 @@ const { export default async function getGoalsForReport(reportId: number) { const goals = await Goal.findAll({ attributes: { + exclude: [ + 'timeframe', + 'isFromSmartsheetTtaPlan', + 'isRttapa', + 'mapsToParentGoalId', + 'createdAt', + 'updatedAt', + 'createdVia', + 'deletedAt', + ], include: [ [sequelize.col('grant.regionId'), 'regionId'], [sequelize.col('grant.recipient.id'), 'recipientId'], diff --git a/src/goalServices/goals.js b/src/goalServices/goals.js index 7142516a2e..51cd99a944 100644 --- a/src/goalServices/goals.js +++ b/src/goalServices/goals.js @@ -1152,7 +1152,7 @@ async function createObjectivesForGoal(goal, objectives) { resources, files, courses, - ttaProvided: objective.ttaProvided, + ttaProvided, closeSuspendReason, closeSuspendContext, index, @@ -1229,7 +1229,6 @@ export async function saveGoalsForReport(goals, report) { createdVia: 'activityReport', name: goal.name ? goal.name.trim() : '', grantId, - ...fields, status, }, { individualHooks: true }); } diff --git a/src/goalServices/reduceGoals.ts b/src/goalServices/reduceGoals.ts index b599774989..a07db8b0df 100644 --- a/src/goalServices/reduceGoals.ts +++ b/src/goalServices/reduceGoals.ts @@ -461,22 +461,6 @@ export function reduceGoals( existingGoal.objectives, ); - existingGoal.collaborators = existingGoal.collaborators || []; - - existingGoal.collaborators = uniqBy([ - ...existingGoal.collaborators, - { - goalNumber: currentValue.goalNumber || `G-${currentValue.dataValues.id}`, - ...getGoalCollaboratorDetails('Creator', currentValue.dataValues as IGoal), - ...getGoalCollaboratorDetails('Linker', currentValue.dataValues as IGoal), - } as { - goalNumber: string; - goalCreatorName: string; - goalCreatorRoles: string; - }, - ], 'goalCreatorName'); - - existingGoal.isReopenedGoal = wasGoalPreviouslyClosed(existingGoal); if (forReport) { existingGoal.prompts = existingGoal.prompts || []; existingGoal.prompts = reducePrompts( @@ -507,6 +491,24 @@ export function reduceGoals( [currentValue.grant.numberWithProgramTypes]: currentValue.dataValues.source, }; } + + existingGoal.collaborators = existingGoal.collaborators || []; + + existingGoal.collaborators = uniqBy([ + ...existingGoal.collaborators, + { + goalNumber: currentValue.goalNumber || `G-${currentValue.dataValues.id}`, + ...getGoalCollaboratorDetails('Creator', currentValue.dataValues as IGoal), + ...getGoalCollaboratorDetails('Linker', currentValue.dataValues as IGoal), + } as { + goalNumber: string; + goalCreatorName: string; + goalCreatorRoles: string; + }, + ], 'goalCreatorName'); + + existingGoal.isReopenedGoal = wasGoalPreviouslyClosed(existingGoal); + return previousValues; } @@ -551,7 +553,6 @@ export function reduceGoals( isCurated: currentValue.dataValues.isCurated, goalNumber: currentValue.goalNumber || `G-${currentValue.dataValues.id}`, grantId: currentValue.grant.id, - collaborators: currentValue.collaborators, id: currentValue.dataValues.id, name: currentValue.dataValues.name, endDate, @@ -568,8 +569,6 @@ export function reduceGoals( onAR: currentValue.dataValues.onAR, onApprovedAR: currentValue.dataValues.onApprovedAR, rtrOrder: currentValue.dataValues.rtrOrder, - isReopenedGoal: wasGoalPreviouslyClosed(currentValue), - goalCollaborators: currentValue.goalCollaborators, objectives: objectivesReducer( currentValue.objectives, ), @@ -589,21 +588,23 @@ export function reduceGoals( statusChanges: currentValue.statusChanges, } as IReducedGoal; - goal.collaborators = [ - { - goalNumber: currentValue.goalNumber || `G-${currentValue.dataValues.id}`, - ...getGoalCollaboratorDetails('Creator', currentValue.dataValues), - ...getGoalCollaboratorDetails('Linker', currentValue.dataValues), - } as { - goalNumber: string; - goalCreatorName: string; - goalCreatorRoles: string; - }, - ]; - - goal.collaborators = goal.collaborators.filter( - (c: { goalCreatorName: string }) => c.goalCreatorName !== null, - ); + if (!forReport) { + goal.isReopenedGoal = wasGoalPreviouslyClosed(currentValue); + goal.goalCollaborators = currentValue.goalCollaborators; + goal.collaborators = [ + { + goalNumber: currentValue.goalNumber || `G-${currentValue.dataValues.id}`, + ...getGoalCollaboratorDetails('Creator', currentValue.dataValues), + ...getGoalCollaboratorDetails('Linker', currentValue.dataValues), + } as { + goalNumber: string; + goalCreatorName: string; + goalCreatorRoles: string; + }, + ].filter( + (c: { goalCreatorName: string }) => c.goalCreatorName !== null, + ); + } return [...previousValues, goal]; } catch (err) { diff --git a/src/lib/importSystem/record.ts b/src/lib/importSystem/record.ts index abfab85878..88b9be556d 100644 --- a/src/lib/importSystem/record.ts +++ b/src/lib/importSystem/record.ts @@ -59,6 +59,7 @@ const getPriorFile = async ( // eslint-disable-next-line @typescript-eslint/quotes order: [[Sequelize.literal(`"ftpFileInfo" ->> 'name'`), 'DESC']], raw: true, + lock: true, // Lock the row for update to prevent race conditions }); // Check if a file was found @@ -86,6 +87,7 @@ const importHasMoreToDownload = async ( downloadAttempts: { [Op.lt]: 5 }, status: [IMPORT_STATUSES.IDENTIFIED, IMPORT_STATUSES.COLLECTION_FAILED], }, + lock: true, // Lock the row for update to prevent race conditions }); return (pendingFiles?.length || 0) >= 1; @@ -107,6 +109,7 @@ const importHasMoreToProcess = async ( processAttempts: { [Op.lt]: 5 }, status: [IMPORT_STATUSES.COLLECTED, IMPORT_STATUSES.PROCESSING_FAILED], }, + lock: true, // Lock the row for update to prevent race conditions }); return (pendingFiles?.length || 0) >= 1; @@ -190,6 +193,7 @@ const getNextFileToProcess = async ( literal(`"updatedAt" + INTERVAL '${(avg.seconds || 0) + Math.floor((avg.milliseconds + totalStdDevMilliseconds) / 1000)} seconds ${(avg.milliseconds + totalStdDevMilliseconds) % 1000} milliseconds' > NOW()`), ], }, + lock: true, // Lock the row for update to prevent race conditions }, ); @@ -235,6 +239,7 @@ const getNextFileToProcess = async ( ['createdAt', 'ASC'], ], limit: 1, // Limit the result to 1 record + lock: true, // Lock the row for update to prevent race conditions }); return importFile; @@ -265,12 +270,13 @@ const recordAvailableFiles = async ( importId, }, raw: true, + lock: true, // Lock the row for update to prevent race conditions }); const fileMatches = (currentImportFile, availableFile) => ( importId === currentImportFile.importId - && availableFile.fileInfo.path === currentImportFile.fileInfo.path - && availableFile.fileInfo.name === currentImportFile.fileInfo.name + && availableFile?.fileInfo?.path === currentImportFile?.fileInfo?.path + && availableFile?.fileInfo?.name === currentImportFile?.fileInfo?.name ); // Separate the available files into new, matched, and removed files @@ -298,6 +304,9 @@ const recordAvailableFiles = async ( ftpFileInfo: newFile.fileInfo, status: IMPORT_STATUSES.IDENTIFIED, }, + { + lock: true, // Lock the row for update to prevent race conditions + }, )) : []), // Update matched files in the database if there are any @@ -317,6 +326,7 @@ const recordAvailableFiles = async ( }, }, individualHooks: true, + lock: true, // Lock the row for update to prevent race conditions }, )) : []), @@ -329,6 +339,7 @@ const recordAvailableFiles = async ( status: [IMPORT_STATUSES.IDENTIFIED], }, individualHooks: true, + lock: true, // Lock the row for update to prevent race conditions }) : Promise.resolve()), ]); @@ -519,6 +530,7 @@ const logFileToBeCollected = async ( attributes: ['key'], require: false, }], + lock: true, // Lock the row for update to prevent race conditions }); if (!importFile.fileId) { @@ -552,6 +564,7 @@ const logFileToBeCollected = async ( }, }, }, + lock: true, // Lock the row for update to prevent race conditions }, ); } else { @@ -572,6 +585,7 @@ const logFileToBeCollected = async ( }, }, }, + lock: true, // Lock the row for update to prevent race conditions }, ); } @@ -601,6 +615,7 @@ const setImportFileHash = async ( { where: { id: importFileId }, // Specify the import file to update based on its ID individualHooks: true, // Enable individual hooks for each updated record + lock: true, // Lock the row for update to prevent race conditions }, ); @@ -624,6 +639,7 @@ const setImportFileStatus = async ( { where: { id: importFileId }, // Specify the import file to update based on its ID individualHooks: true, // Enable individual hooks for each updated record + lock: true, // Lock the row for update to prevent race conditions }, ); diff --git a/src/lib/importSystem/tests/record.test.js b/src/lib/importSystem/tests/record.test.js index 1a798b67ca..732bbf6a23 100644 --- a/src/lib/importSystem/tests/record.test.js +++ b/src/lib/importSystem/tests/record.test.js @@ -1,9 +1,6 @@ import { Sequelize, Op, - where, - cast, - col, } from 'sequelize'; import { v4 as uuidv4 } from 'uuid'; import { @@ -68,6 +65,7 @@ describe('record', () => { // Clear all instances and calls to constructor and all methods: jest.clearAllMocks(); }); + describe('getPriorFile', () => { beforeEach(() => { jest.clearAllMocks(); @@ -99,6 +97,7 @@ describe('record', () => { // eslint-disable-next-line @typescript-eslint/quotes order: [[Sequelize.literal(`"ftpFileInfo" ->> 'name'`), 'DESC']], raw: true, + lock: true, }); expect(result).toBe(mockName); }); @@ -128,6 +127,7 @@ describe('record', () => { // eslint-disable-next-line @typescript-eslint/quotes order: [[Sequelize.literal(`"ftpFileInfo" ->> 'name'`), 'DESC']], raw: true, + lock: true, }); expect(result).toBeNull(); }); @@ -167,6 +167,7 @@ describe('record', () => { downloadAttempts: { [Op.lt]: 5 }, status: [IMPORT_STATUSES.IDENTIFIED, IMPORT_STATUSES.COLLECTION_FAILED], }, + lock: true, }); }); @@ -188,6 +189,7 @@ describe('record', () => { downloadAttempts: { [Op.lt]: 5 }, status: [IMPORT_STATUSES.IDENTIFIED, IMPORT_STATUSES.COLLECTION_FAILED], }, + lock: true, }); }); @@ -206,6 +208,7 @@ describe('record', () => { downloadAttempts: { [Op.lt]: 5 }, status: [IMPORT_STATUSES.IDENTIFIED, IMPORT_STATUSES.COLLECTION_FAILED], }, + lock: true, }); }); }); @@ -228,6 +231,7 @@ describe('record', () => { processAttempts: { [Op.lt]: 5 }, status: [IMPORT_STATUSES.COLLECTED, IMPORT_STATUSES.PROCESSING_FAILED], }, + lock: true, }); expect(result).toBe(true); }); @@ -249,6 +253,7 @@ describe('record', () => { processAttempts: { [Op.lt]: 5 }, status: [IMPORT_STATUSES.COLLECTED, IMPORT_STATUSES.PROCESSING_FAILED], }, + lock: true, }); expect(result).toBe(false); }); @@ -309,6 +314,7 @@ describe('record', () => { }, order: [['createdAt', 'ASC']], limit: 1, + lock: true, }); expect(result).toEqual(mockImportFile); @@ -361,6 +367,7 @@ describe('record', () => { ftpFileInfo: expect.any(Object), status: IMPORT_STATUSES.IDENTIFIED, }), + { lock: true }, ); }); @@ -383,6 +390,7 @@ describe('record', () => { }, }, individualHooks: true, + lock: true, }, ); }); @@ -404,6 +412,7 @@ describe('record', () => { status: [IMPORT_STATUSES.IDENTIFIED], }, individualHooks: true, + lock: true, }); }); @@ -514,7 +523,6 @@ describe('record', () => { }); it('should delete removed files from the database when there are removed files', async () => { - // Mock the database response for current import data files ImportDataFile.findAll.mockResolvedValue(currentImportDataFiles); await recordAvailableDataFiles(importFileId, [availableFiles[0]]); @@ -713,7 +721,7 @@ describe('record', () => { expect(ImportFile.update).toHaveBeenCalledWith( { hash: newHash }, - { where: { id: importFileId }, individualHooks: true }, + { where: { id: importFileId }, individualHooks: true, lock: true }, ); }); @@ -722,7 +730,7 @@ describe('record', () => { expect(ImportFile.update).toHaveBeenCalledWith( { hash: newHash, status }, - { where: { id: importFileId }, individualHooks: true }, + { where: { id: importFileId }, individualHooks: true, lock: true }, ); }); @@ -730,8 +738,8 @@ describe('record', () => { await setImportFileHash(importFileId, newHash); expect(ImportFile.update).toHaveBeenCalledWith( - expect.not.objectContaining({ status }), - expect.any(Object), + { hash: newHash }, + { where: { id: importFileId }, individualHooks: true, lock: true }, ); }); @@ -740,7 +748,7 @@ describe('record', () => { expect(ImportFile.update).toHaveBeenCalledWith( { hash: null }, - { where: { id: importFileId }, individualHooks: true }, + { where: { id: importFileId }, individualHooks: true, lock: true }, ); }); @@ -780,6 +788,7 @@ describe('record', () => { expect(ImportFile.update).toHaveBeenCalledWith(expectedUpdateArg, { where: { id: importFileId }, individualHooks: true, + lock: true, }); }); @@ -797,6 +806,7 @@ describe('record', () => { expect(ImportFile.update).toHaveBeenCalledWith(expectedUpdateArg, { where: { id: importFileId }, individualHooks: true, + lock: true, }); }); @@ -814,6 +824,7 @@ describe('record', () => { expect(ImportFile.update).toHaveBeenCalledWith(expectedUpdateArg, { where: { id: importFileId }, individualHooks: true, + lock: true, }); }); @@ -832,6 +843,7 @@ describe('record', () => { expect(ImportFile.update).toHaveBeenCalledWith(expectedUpdateArg, { where: { id: importFileId }, individualHooks: true, + lock: true, }); }); @@ -848,6 +860,7 @@ describe('record', () => { expect(ImportFile.update).toHaveBeenCalledWith(expectedUpdateArg, { where: { id: importFileId }, individualHooks: true, + lock: true, }); }); @@ -873,6 +886,7 @@ describe('record', () => { { where: { id: importDataFileId }, individualHooks: true, + lock: true, }, ); @@ -886,7 +900,7 @@ describe('record', () => { // Check that ImportDataFile.update was called correctly expect(ImportDataFile.update).toHaveBeenCalledWith( { status: newStatus }, - { where: { id: importFileId }, individualHooks: true }, + { where: { id: importFileId }, individualHooks: true, lock: true }, ); }); diff --git a/src/migrations/20240702000000-monitoring-config-correction.js b/src/migrations/20240702000000-monitoring-config-correction.js new file mode 100644 index 0000000000..f672b8c381 --- /dev/null +++ b/src/migrations/20240702000000-monitoring-config-correction.js @@ -0,0 +1,91 @@ +const { + prepMigration, +} = require('../lib/migration'); + +module.exports = { + up: async (queryInterface) => queryInterface.sequelize.transaction( + async (transaction) => { + await prepMigration(queryInterface, transaction, __filename); + // Correct column configuration + await queryInterface.sequelize.query(/* sql */` + WITH + reconfigure AS ( + SELECT + i."name", + jsonb_agg( + CASE + WHEN elem->>'tableName' = 'MonitoringFindings' THEN + jsonb_set( + elem, + '{remapDef}', + (elem->'remapDef')::jsonb - 'ReportDate' || jsonb_build_object('ReportedDate', 'reportedDate') + ) + ELSE + elem + END + ) "definitions" + FROM "Imports" i + CROSS JOIN jsonb_array_elements(i."definitions") as elem + WHERE i."name" = 'ITAMS Monitoring Data' + AND "definitions" @> '[{"tableName": "MonitoringFindings"}]' + GROUP BY 1 + ) + UPDATE "Imports" i + SET "definitions" = r."definitions" + FROM "reconfigure" r + WHERE i."name" = r."name"; + `, { transaction }); + // Remove erroneous duplicate records + await queryInterface.sequelize.query(/* sql */` + WITH + erroneous_records AS ( + SELECT DISTINCT if2.id + FROM "ImportFiles" if1 + JOIN "ImportFiles" if2 + ON if1.id < if2.id + AND if1."ftpFileInfo" -> 'name' = if2."ftpFileInfo" -> 'name' + ) + DELETE FROM "ImportFiles" i + USING erroneous_records er + WHERE i.id = er.id; + `, { transaction }); + // Add a unique constraint to prevent future duplicate entires + await queryInterface.sequelize.query(/* sql */` + CREATE UNIQUE INDEX "ImportFiles_ftpFileInfo_name_unique" ON "ImportFiles" (("ftpFileInfo" -> 'name')); + `, { transaction }); + }, + ), + down: async (queryInterface) => queryInterface.sequelize.transaction( + async (transaction) => { + await prepMigration(queryInterface, transaction, __filename); + await queryInterface.sequelize.query(/* sql */` + WITH + reconfigure AS ( + SELECT + i."name", + jsonb_agg( + CASE + WHEN elem->>'tableName' = 'MonitoringFindings' THEN + jsonb_set( + elem, + '{remapDef}', + (elem->'remapDef')::jsonb - 'ReportedDate' || jsonb_build_object('ReportDate', 'reportDate') + ) + ELSE + elem + END + ) "definitions" + FROM "Imports" i + CROSS JOIN jsonb_array_elements(i."definitions") as elem + WHERE i."name" = 'ITAMS Monitoring Data' + AND "definitions" @> '[{"tableName": "MonitoringFindings"}]' + GROUP BY 1 + ) + UPDATE "Imports" i + SET "definitions" = r."definitions" + FROM "reconfigure" r + WHERE i."name" = r."name"; + `, { transaction }); + }, + ), +}; diff --git a/src/models/hooks/sessionReportPilot.js b/src/models/hooks/sessionReportPilot.js index 401d773f29..66e5548fd2 100644 --- a/src/models/hooks/sessionReportPilot.js +++ b/src/models/hooks/sessionReportPilot.js @@ -133,17 +133,9 @@ const participantsAndNextStepsComplete = async (sequelize, instance, options) => }; const extractEventData = (sessionReport) => { - let event; - let recipients; - - if (sessionReport && sessionReport.data) { - const data = (typeof sessionReport.data.val === 'string') - ? JSON.parse(sessionReport.data.val) - : sessionReport.data; - - event = data.event; - recipients = data.recipients; - } + const data = sessionReport?.data?.val ? JSON.parse(sessionReport.data.val) : sessionReport?.data; + const event = data?.event; + const recipients = Array.isArray(data?.recipients) ? data.recipients : [data?.recipients].filter(Boolean); return { event, recipients }; }; @@ -302,6 +294,7 @@ export const createGoalsForSessionRecipientsIfNecessary = async (sequelize, sess } } catch (error) { auditLogger.error(`Error in createGoalsForSessionRecipientsIfNecessary: ${error}`); + throw error; } }; diff --git a/src/models/hooks/sessionReportPilot.test.js b/src/models/hooks/sessionReportPilot.test.js index eeffad2d54..6ab736c68f 100644 --- a/src/models/hooks/sessionReportPilot.test.js +++ b/src/models/hooks/sessionReportPilot.test.js @@ -53,6 +53,11 @@ describe('sessionReportPilot hooks', () => { update: mockUpdate, })), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -73,6 +78,11 @@ describe('sessionReportPilot hooks', () => { EventReportPilot: { findOne: jest.fn(() => null), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -88,6 +98,11 @@ describe('sessionReportPilot hooks', () => { throw new Error('oops'); }), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -105,6 +120,11 @@ describe('sessionReportPilot hooks', () => { update: mockUpdate, })), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -124,6 +144,11 @@ describe('sessionReportPilot hooks', () => { update: mockUpdate, })), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -152,6 +177,11 @@ describe('sessionReportPilot hooks', () => { update: mockUpdate, })), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -180,6 +210,11 @@ describe('sessionReportPilot hooks', () => { update: mockUpdate, })), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -208,6 +243,11 @@ describe('sessionReportPilot hooks', () => { update: mockUpdate, })), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -236,6 +276,11 @@ describe('sessionReportPilot hooks', () => { update: mockUpdate, })), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -264,6 +309,11 @@ describe('sessionReportPilot hooks', () => { update: mockUpdate, })), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -291,6 +341,11 @@ describe('sessionReportPilot hooks', () => { update: mockUpdate, })), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -316,6 +371,11 @@ describe('sessionReportPilot hooks', () => { update: mockUpdate, })), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -339,6 +399,11 @@ describe('sessionReportPilot hooks', () => { EventReportPilot: { findOne: jest.fn(() => null), }, + SessionReportPilot: { + findByPk: jest.fn(() => ({ + data: { recipients: [] }, + })), + }, }, }; @@ -418,59 +483,40 @@ describe('sessionReportPilot hooks', () => { }); describe('createGoalsForSessionRecipientsIfNecessary hook', () => { - const mockOptions = { - transaction: {}, - }; + const mockOptions = { transaction: {} }; const mockInstance = { id: 1, data: { - event: { - id: '2', - data: { - goal: 'Increase knowledge about X', - }, - }, + event: { id: '2', data: { goal: 'Increase knowledge about X' } }, recipients: [{ value: '3' }], }, }; + const mockModels = (overrides = {}) => ({ + EventReportPilot: { findByPk: jest.fn(() => true) }, + Grant: { findByPk: jest.fn(() => true) }, + EventReportPilotGoal: { create: jest.fn(), findOne: jest.fn(() => null) }, + Goal: { create: jest.fn(() => ({ id: 4 })), findOne: jest.fn(() => null) }, + SessionReportPilot: { findByPk: jest.fn(() => mockInstance), findOne: jest.fn(() => null) }, + CollaboratorType: { findOne: jest.fn(() => ({ id: 1 })) }, + GoalCollaborator: { findAll: jest.fn(() => []), create: jest.fn(), update: jest.fn() }, + User: { findAll: jest.fn(() => []), create: jest.fn(), update: jest.fn() }, + ...overrides, + }); + afterEach(() => { jest.clearAllMocks(); }); it('creates a new goal and event report pilot goal if necessary', async () => { const mockSequelize = { - Sequelize: { - Model: jest.fn(), - }, - models: { - EventReportPilot: { findByPk: jest.fn(() => true) }, - Grant: { findByPk: jest.fn(() => true) }, - EventReportPilotGoal: { create: jest.fn(), findOne: jest.fn(() => null) }, - Goal: { create: jest.fn(() => ({ id: 4 })), findOne: jest.fn(() => null) }, - SessionReportPilot: { findByPk: jest.fn(() => mockInstance), findOne: jest.fn(() => null) }, - CollaboratorType: { findOne: jest.fn(() => ({ id: 1 })) }, - GoalCollaborator: { findAll: jest.fn(() => []), create: jest.fn(), update: jest.fn() }, - }, + Sequelize: { Model: jest.fn() }, + models: mockModels(), }; await createGoalsForSessionRecipientsIfNecessary(mockSequelize, mockInstance, mockOptions); expect(mockSequelize.models.Goal.create).toHaveBeenCalled(); - expect(mockSequelize.models.Goal.create).toHaveBeenCalledWith( - { - createdAt: expect.any(Date), - createdVia: 'tr', - grantId: 3, - name: 'Increase knowledge about X', - onAR: true, - onApprovedAR: false, - source: 'Training event', - status: 'Draft', - updatedAt: expect.any(Date), - }, - { transaction: {} }, - ); expect(mockSequelize.models.EventReportPilotGoal.create).toHaveBeenCalled(); expect(mockSequelize.models.CollaboratorType.findOne).toHaveBeenCalledTimes(2); expect(mockSequelize.models.GoalCollaborator.findAll).toHaveBeenCalledTimes(1); @@ -478,18 +524,33 @@ describe('createGoalsForSessionRecipientsIfNecessary hook', () => { expect(mockSequelize.models.GoalCollaborator.update).toHaveBeenCalledTimes(0); }); + it('doesn\'t throw an error when recipients is null/undefined', async () => { + const noRecips = { + id: 1, + data: { event: { id: '2', data: { goal: 'Increase knowledge about X' } }, recipients: null }, + }; + + const mockSequelize = { + Sequelize: { Model: jest.fn() }, + models: mockModels({ + SessionReportPilot: { findByPk: jest.fn(() => noRecips), findOne: jest.fn(() => null) }, + }), + }; + + await expect(createGoalsForSessionRecipientsIfNecessary( + mockSequelize, + noRecips, + mockOptions, + )).resolves.not.toThrow(); + }); + it('does not create a new goal if one already exists', async () => { const mockSequelize = { - Sequelize: { - Model: jest.fn(), - }, - models: { - EventReportPilot: { findByPk: jest.fn(() => true) }, - Grant: { findByPk: jest.fn(() => true) }, + Sequelize: { Model: jest.fn() }, + models: mockModels({ EventReportPilotGoal: { create: jest.fn(), findOne: jest.fn(() => true) }, Goal: { create: jest.fn(), findOne: jest.fn(() => true) }, - SessionReportPilot: { findByPk: jest.fn(() => mockInstance), findOne: jest.fn(() => null) }, - }, + }), }; await createGoalsForSessionRecipientsIfNecessary(mockSequelize, mockInstance, mockOptions); diff --git a/src/testUtils.js b/src/testUtils.js index c48999f078..1566bfb833 100644 --- a/src/testUtils.js +++ b/src/testUtils.js @@ -68,7 +68,7 @@ export async function createUser(user) { } function defaultRegion() { - const number = faker.datatype.number({ min: 50, max: 1000 }); + const number = faker.datatype.number({ min: 50, max: 2000 }); return { id: faker.unique(() => number, { maxRetries: 20 }), name: `Region ${number}`, diff --git a/yarn.lock b/yarn.lock index ad88a2d527..19e251022c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14049,9 +14049,9 @@ ws@>=8.7.0, ws@^8.14.2, ws@^8.16.0, ws@^8.17.1: integrity sha512-6XQFvXTkbfUOZOKKILFG1PDK2NDQs4azKQl26T0YS5CxqWLgXajbPZ+h4gZekJyRqFU8pvnbAbbs/3TgRPy+GQ== ws@^7.4.6: - version "7.5.9" - resolved "https://registry.yarnpkg.com/ws/-/ws-7.5.9.tgz#54fa7db29f4c7cec68b1ddd3a89de099942bb591" - integrity sha512-F+P9Jil7UiSKSkppIiD94dN07AwvFixvLIj1Og1Rl9GGMuNipJnV9JzjD6XuqmAeiswGvUmNLjr5cFuXwNS77Q== + version "7.5.10" + resolved "https://registry.yarnpkg.com/ws/-/ws-7.5.10.tgz#58b5c20dc281633f6c19113f39b349bd8bd558d9" + integrity sha512-+dbF1tHwZpXcbOJdVOkzLDxZP1ailvSxM6ZweXTegylPny803bFhA+vqBYw4s31NSAk4S2Qz+AKXK9a4wkdjcQ== xdg-basedir@^4.0.0: version "4.0.0"