From 780a3b688fdc15e6c21855329bebcee345a07eea Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Mon, 15 Mar 2021 15:30:50 -0500 Subject: [PATCH 1/6] Fix resources issues * Pressing enter in a resource textbox now adds that resource and does not open the file delete dialog * Resources are only updated on the server when passed to the API, previously if they were not included in the API call they would be set to empty * The frontend properly sets the report in the format needed by react-hook-form when setting formData, previously this was only done on the initial load. This solves an issue resetting drafts to normal and having resources disappear --- frontend/src/components/FileUploader.js | 2 +- .../Pages/components/ResourceSelector.js | 23 ++++++++++++------- .../pages/ActivityReport/Pages/nextSteps.js | 2 +- frontend/src/pages/ActivityReport/index.js | 19 ++++++++++----- src/services/activityReports.js | 18 +++++++++------ 5 files changed, 41 insertions(+), 23 deletions(-) diff --git a/frontend/src/components/FileUploader.js b/frontend/src/components/FileUploader.js index 228a651f9a..de813f447c 100644 --- a/frontend/src/components/FileUploader.js +++ b/frontend/src/components/FileUploader.js @@ -199,7 +199,7 @@ const FileTable = ({ onFileRemoved, files }) => { diff --git a/frontend/src/pages/ActivityReport/index.js b/frontend/src/pages/ActivityReport/index.js index 50366d011b..98f1e213f9 100644 --- a/frontend/src/pages/ActivityReport/index.js +++ b/frontend/src/pages/ActivityReport/index.js @@ -95,6 +95,12 @@ function ActivityReport({ return array.map((value) => ({ value })); }; + const convertReportToFormData = (fetchedReport) => { + const ECLKCResourcesUsed = unflattenResourcesUsed(fetchedReport.ECLKCResourcesUsed); + const nonECLKCResourcesUsed = unflattenResourcesUsed(fetchedReport.nonECLKCResourcesUsed); + return { ...fetchedReport, ECLKCResourcesUsed, nonECLKCResourcesUsed }; + }; + useDeepCompareEffect(() => { const fetch = async () => { let report; @@ -103,9 +109,7 @@ function ActivityReport({ updateLoading(true); if (activityReportId !== 'new') { const fetchedReport = await getReport(activityReportId); - const ECLKCResourcesUsed = unflattenResourcesUsed(fetchedReport.ECLKCResourcesUsed); - const nonECLKCResourcesUsed = unflattenResourcesUsed(fetchedReport.nonECLKCResourcesUsed); - report = { ...fetchedReport, ECLKCResourcesUsed, nonECLKCResourcesUsed }; + report = convertReportToFormData(fetchedReport); } else { report = { ...defaultValues, @@ -213,18 +217,21 @@ function ActivityReport({ }; const onFormSubmit = async (data) => { - const report = await submitReport(reportId.current, data); + const fetchedReport = await submitReport(reportId.current, data); + const report = convertReportToFormData(fetchedReport); updateFormData(report); updateEditable(false); }; const onReview = async (data) => { - const report = await reviewReport(reportId.current, data); + const fetchedReport = await reviewReport(reportId.current, data); + const report = convertReportToFormData(fetchedReport); updateFormData(report); }; const onResetToDraft = async () => { - const report = await resetToDraft(reportId.current); + const fetchedReport = await resetToDraft(reportId.current); + const report = convertReportToFormData(fetchedReport); updateFormData(report); updateEditable(true); }; diff --git a/src/services/activityReports.js b/src/services/activityReports.js index 492cbd5bac..c1b6774e0f 100644 --- a/src/services/activityReports.js +++ b/src/services/activityReports.js @@ -488,18 +488,22 @@ export async function createOrUpdate(newActivityReport, report) { author, granteeNextSteps, specialistNextSteps, + ECLKCResourcesUsed, + nonECLKCResourcesUsed, ...allFields } = newActivityReport; - const ECLKCResourcesUsed = allFields.ECLKCResourcesUsed - ? allFields.ECLKCResourcesUsed.map((item) => item.value) - : []; + const resources = {}; - const nonECLKCResourcesUsed = allFields.nonECLKCResourcesUsed - ? allFields.nonECLKCResourcesUsed.map((item) => item.value) - : []; + if (ECLKCResourcesUsed) { + resources.ECLKCResourcesUsed = ECLKCResourcesUsed.map((item) => item.value); + } + + if (nonECLKCResourcesUsed) { + resources.nonECLKCResourcesUsed = nonECLKCResourcesUsed.map((item) => item.value); + } - const updatedFields = { ...allFields, ECLKCResourcesUsed, nonECLKCResourcesUsed }; + const updatedFields = { ...allFields, ...resources }; await sequelize.transaction(async (transaction) => { if (report) { savedReport = await update(updatedFields, report, transaction); From d0fbaba9dd822faf9c8617d14a09d2577abb3840 Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Mon, 15 Mar 2021 16:10:27 -0400 Subject: [PATCH 2/6] Fixes for production data irregularities --- .../20210315181828-goal-timeframe-text.js | 11 ++++ src/tools/importPlanGoals.js | 54 +++++++++++-------- 2 files changed, 44 insertions(+), 21 deletions(-) create mode 100644 src/migrations/20210315181828-goal-timeframe-text.js diff --git a/src/migrations/20210315181828-goal-timeframe-text.js b/src/migrations/20210315181828-goal-timeframe-text.js new file mode 100644 index 0000000000..0c89fa55a9 --- /dev/null +++ b/src/migrations/20210315181828-goal-timeframe-text.js @@ -0,0 +1,11 @@ +'use strict'; + +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.changeColumn('Goals', 'timeframe', { type: Sequelize.TEXT }); + }, + + down: async (queryInterface, Sequelize) => { + await queryInterface.changeColumn('Goals', 'timeframe', { type: Sequelize.STRING }); + }, +}; diff --git a/src/tools/importPlanGoals.js b/src/tools/importPlanGoals.js index f912f257de..5c3c765043 100644 --- a/src/tools/importPlanGoals.js +++ b/src/tools/importPlanGoals.js @@ -41,6 +41,19 @@ async function prePopulateRoles() { updateOnDuplicate: ['updatedAt'], }); } + +const grantNumRE = /\s(?[0-9]{2}[A-Z]{2}[0-9]+)(?:[,\s]|$)/g; +const parseGrantNumbers = (value) => { + const matchIter = value.matchAll(grantNumRE); + const results = []; + for (const { groups: { grantNumber } } of matchIter) { + if (grantNumber) { + results.push(grantNumber); + } + } + return results; +}; + /** * Processes data from .csv inserting the data during the processing as well as * creating data arrays for associations and then inserting them to the database @@ -66,32 +79,31 @@ export default async function importGoals(fileKey, region) { const cleanRoleTopics = []; const cleanGrantGoals = []; const cleanTopicGoals = []; - const currentGoals = []; await prePopulateRoles(); for await (const el of grantees) { - let currentGranteeId; - let grants; let currentGrants = []; - let currentGoalName; - let currentGoal = {}; + let currentGoals = []; + let currentGoalName = ''; + let currentGoalNum = 0; for await (const key of Object.keys(el)) { if (key && (key.trim().startsWith('Grantee (distinct') || key.trim().startsWith('Grantee Name'))) { - grants = el[key] ? el[key].split('|')[1].trim() : 'Unknown Grant'; - currentGrants = grants.split(','); + currentGrants = parseGrantNumbers(el[key]); } else if (key && key.startsWith('Goal')) { const goalColumn = key.split(' '); let column; if (goalColumn.length === 2) { // Column name is "Goal X" representing goal's name currentGoalName = el[key].trim(); + if (currentGoalName.match(/(no goals?|none)( identified)? at this time\.?/i)) { + currentGoalName = ''; + } if (currentGoalName !== '') { // Ignore empty goals - currentGoal = { name: currentGoalName }; // change to dbGoal - const goalNum = goalColumn[1]; - currentGoals[goalNum] = { ...currentGoals[goalNum], ...currentGoal }; + currentGoalNum = goalColumn[1]; + currentGoals[currentGoalNum] = { ...currentGoals[currentGoalNum], name: currentGoalName }; } - } else { + } else if (currentGoalName !== '') { // column will be either "topics", "timeframe" or "status" column = goalColumn[2].toLowerCase(); if (column === 'topics') { @@ -124,16 +136,14 @@ export default async function importGoals(fileKey, region) { } // Add topic to junction with goal cleanTopicGoals.push( - { topicId, goalName: currentGoal.name }, + { topicId, goalName: currentGoalName }, ); // we don't have goal's id at this point yet } } - } else // it's either "timeframe" or "status" - // both "timeframe" and "status" column names will be reused as goal's object keys - if (currentGoalName !== '') { - currentGoal[column] = el[key].trim(); - const goalNum = goalColumn[1].slice(0, 1); // represents a goal number from 1 to 5 - currentGoals[goalNum] = { ...currentGoals[goalNum], ...currentGoal }; + } else { + // it's either "timeframe" or "status" + // both "timeframe" and "status" column names will be reused as goal's object keys + currentGoals[currentGoalNum] = { ...currentGoals[currentGoalNum], [column]: el[key].trim() }; } } } @@ -142,12 +152,14 @@ export default async function importGoals(fileKey, region) { // after each row let goalId; let grantId; + let currentGranteeId; for await (const goal of currentGoals) { if (goal) { // ignore the dummy element at index 0 - const [dbGoal] = await Goal.findOrCreate( - { where: { ...goal, isFromSmartsheetTtaPlan: true } }, - ); + const [dbGoal] = await Goal.findOrCreate({ + where: { name: goal.name, isFromSmartsheetTtaPlan: true }, + defaults: goal, + }); goalId = dbGoal.id; // add goal id to cleanTopicGoals cleanTopicGoals.forEach((tp) => { From ed6700b31f6623b85bd95deb91d4065f11b72092 Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Mon, 15 Mar 2021 16:54:54 -0400 Subject: [PATCH 3/6] Linter fixes --- .../20210315181828-goal-timeframe-text.js | 2 -- src/tools/importPlanGoals.js | 13 ++++++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/migrations/20210315181828-goal-timeframe-text.js b/src/migrations/20210315181828-goal-timeframe-text.js index 0c89fa55a9..7eb8938838 100644 --- a/src/migrations/20210315181828-goal-timeframe-text.js +++ b/src/migrations/20210315181828-goal-timeframe-text.js @@ -1,5 +1,3 @@ -'use strict'; - module.exports = { up: async (queryInterface, Sequelize) => { await queryInterface.changeColumn('Goals', 'timeframe', { type: Sequelize.TEXT }); diff --git a/src/tools/importPlanGoals.js b/src/tools/importPlanGoals.js index 5c3c765043..9af97ebf07 100644 --- a/src/tools/importPlanGoals.js +++ b/src/tools/importPlanGoals.js @@ -84,7 +84,7 @@ export default async function importGoals(fileKey, region) { for await (const el of grantees) { let currentGrants = []; - let currentGoals = []; + const currentGoals = []; let currentGoalName = ''; let currentGoalNum = 0; @@ -100,8 +100,12 @@ export default async function importGoals(fileKey, region) { currentGoalName = ''; } if (currentGoalName !== '') { // Ignore empty goals + // eslint-disable-next-line prefer-destructuring currentGoalNum = goalColumn[1]; - currentGoals[currentGoalNum] = { ...currentGoals[currentGoalNum], name: currentGoalName }; + currentGoals[currentGoalNum] = { + ...currentGoals[currentGoalNum], + name: currentGoalName, + }; } } else if (currentGoalName !== '') { // column will be either "topics", "timeframe" or "status" @@ -143,7 +147,10 @@ export default async function importGoals(fileKey, region) { } else { // it's either "timeframe" or "status" // both "timeframe" and "status" column names will be reused as goal's object keys - currentGoals[currentGoalNum] = { ...currentGoals[currentGoalNum], [column]: el[key].trim() }; + currentGoals[currentGoalNum] = { + ...currentGoals[currentGoalNum], + [column]: el[key].trim(), + }; } } } From 7c0ea5a3ad75fa05ad89a3ba8d7d8445f8513718 Mon Sep 17 00:00:00 2001 From: Josh Salisbury Date: Mon, 15 Mar 2021 16:07:43 -0500 Subject: [PATCH 4/6] Add test --- .../Pages/components/__tests__/ResourceSelector.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/frontend/src/pages/ActivityReport/Pages/components/__tests__/ResourceSelector.js b/frontend/src/pages/ActivityReport/Pages/components/__tests__/ResourceSelector.js index 9ae4fef254..5cca992626 100644 --- a/frontend/src/pages/ActivityReport/Pages/components/__tests__/ResourceSelector.js +++ b/frontend/src/pages/ActivityReport/Pages/components/__tests__/ResourceSelector.js @@ -49,6 +49,16 @@ describe('ResourceSelector', () => { }); }); + describe('when enter is pressed', () => { + it('adds a new resource', async () => { + render(); + const textBox = await screen.findByTestId('textInput'); + userEvent.type(textBox, 'test{enter}'); + const text = await screen.findAllByRole('textbox'); + expect(text.length).toBe(2); + }); + }); + describe('with multiple entries', () => { it('allows removal of an item', async () => { render(); From f8f210d84c24188daff6a190b50d157040f4c28d Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Mon, 15 Mar 2021 17:42:59 -0400 Subject: [PATCH 5/6] Ensure the report finished saving before trying to access the id --- src/tools/importActivityReports.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/tools/importActivityReports.js b/src/tools/importActivityReports.js index 813e3e06da..e3b091459e 100644 --- a/src/tools/importActivityReports.js +++ b/src/tools/importActivityReports.js @@ -274,10 +274,12 @@ export default async function importActivityReports(fileKey, region) { // Imported ARs won't pass `checkRequiredForSubmission`, // because `approvingManagerId`, `requester`, etc. may be null // so we build, then save without validating; - const [ar] = await ActivityReport.findOrBuild( + const [ar, built] = await ActivityReport.findOrBuild( { where: { legacyId }, defaults: arRecord }, ); - ar.save({ validate: false }); + if (built) { + await ar.save({ validate: false }); + } // ActivityRecipients: connect Grants to ActivityReports const grantNumbers = parseGrantNumbers(getValue(data, 'granteeName')); From 6ed638cf5a64f521a54858a2369fd1db7561926d Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Mon, 15 Mar 2021 17:53:25 -0400 Subject: [PATCH 6/6] Pick up a lot more single grant AR recipient associations --- src/tools/importActivityReports.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/importActivityReports.js b/src/tools/importActivityReports.js index e3b091459e..7efadff326 100644 --- a/src/tools/importActivityReports.js +++ b/src/tools/importActivityReports.js @@ -87,7 +87,7 @@ import { REPORT_STATUSES } from '../constants'; const columnCleanupRE = /(\s?\(.*\)|:|\.|\/|&|')+/g; const decimalRE = /^\d+(\.\d*)?$/; const invalidRegionRE = /R14/; -const grantNumRE = /\|\s+(?[0-9A-Z]+)\n/g; +const grantNumRE = /\|\s+(?[0-9A-Z]+)(\n|$)/g; const mdyDateRE = /^\d{1,2}\/\d{1,2}\/(\d{2}|\d{4})$/; const mdyFormat = 'MM/DD/YYYY';