From 6bdfb117b00874fb665c8b77aeb043fb8b9063af Mon Sep 17 00:00:00 2001 From: Adam Levin Date: Fri, 14 Feb 2025 16:19:25 -0500 Subject: [PATCH 1/3] ensure all old grants have the same region and recipient --- src/lib/updateGrantsRecipients.js | 7 ++++++- src/lib/updateGrantsRecipients.test.js | 28 ++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/lib/updateGrantsRecipients.js b/src/lib/updateGrantsRecipients.js index 5a6ad5019e..c1108f2633 100644 --- a/src/lib/updateGrantsRecipients.js +++ b/src/lib/updateGrantsRecipients.js @@ -199,10 +199,15 @@ export const updateCDIGrantsWithOldGrantData = async (grantsToUpdate) => { const [regionId] = uniq(validOldGrants.map((g) => g.regionId)); const [recipientId] = uniq(validOldGrants.map((g) => g.recipientId)); - if (!regionId || !recipientId || validOldGrants.length !== replacedGrants.length) { + if (!regionId || !recipientId) { throw new Error(`Expected one region and recipient for grant ${grant.id}, got ${validOldGrants.length} valid grants`); } + // Ensure allValidOldGrants have the same recipient and region. + if (!validOldGrants.every((g) => g.regionId === regionId && g.recipientId === recipientId)) { + throw new Error(`Expected all valid replaced grants to have the same recipient and region for CDI grant ${grant.id}`); + } + // eslint-disable-next-line consistent-return return grant.update({ recipientId, regionId }); }); diff --git a/src/lib/updateGrantsRecipients.test.js b/src/lib/updateGrantsRecipients.test.js index 92605dbb8a..ca4faf46b1 100644 --- a/src/lib/updateGrantsRecipients.test.js +++ b/src/lib/updateGrantsRecipients.test.js @@ -1165,9 +1165,9 @@ describe('Update grants, program personnel, and recipients', () => { // spy on logger.error. jest.spyOn(logger, 'error').mockImplementation(() => {}); jest.spyOn(logger, 'info').mockImplementation(() => {}); - const newGrant = await Grant.create({ + const newGrant = { id: 8546, cdi: true, number: 'X5', recipientId: 628, regionId: 13, - }); + }; await updateCDIGrantsWithOldGrantData([newGrant]); @@ -1177,6 +1177,30 @@ describe('Update grants, program personnel, and recipients', () => { // Expect logger.info to display the message that no replacements were found. expect(logger.info).toHaveBeenCalledWith('updateCDIGrantsWithOldGrantData: No grant replacements found for CDI grant: 8546, skipping'); }); + + it('logs an error if all oldGrants dont have the same recipient and region id', async () => { + const newGrant = { + id: 8546, cdi: true, number: 'X5', recipientId: 628, regionId: 13, + }; + + // spy on logger.error. + jest.spyOn(logger, 'error').mockImplementation(() => {}); + + // Mock return replacements. + jest.spyOn(GrantReplacements, 'findAll').mockResolvedValueOnce([{ replacedGrantId: 1 }, { replacedGrantId: 2 }]); + + // Mock return old grants. + jest.spyOn(Grant, 'findByPk').mockResolvedValueOnce({ recipientId: 1, regionId: 1 }); + jest.spyOn(Grant, 'findByPk').mockResolvedValueOnce({ recipientId: 2, regionId: 2 }); + + await updateCDIGrantsWithOldGrantData([newGrant]); + + // Ensure logger.error was called. + expect(logger.error).toHaveBeenCalledWith( + 'updateGrantsRecipients: Error updating grants:', + new Error('Expected all valid replaced grants to have the same recipient and region for CDI grant 8546'), + ); + }); }); }); From f1ea132378d694bd6db40ae2038c8a58406a8ddb Mon Sep 17 00:00:00 2001 From: Adam Levin Date: Tue, 18 Feb 2025 09:23:30 -0500 Subject: [PATCH 2/3] log an error and continue processing if we have a grant replacement mis match --- src/lib/updateGrantsRecipients.js | 51 ++++++++++++++++++------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/lib/updateGrantsRecipients.js b/src/lib/updateGrantsRecipients.js index c1108f2633..d1788c4017 100644 --- a/src/lib/updateGrantsRecipients.js +++ b/src/lib/updateGrantsRecipients.js @@ -183,38 +183,45 @@ async function getProgramPersonnel(grantId, programId, program) { export const updateCDIGrantsWithOldGrantData = async (grantsToUpdate) => { try { const updates = grantsToUpdate.map(async (grant) => { - // eslint-disable-next-line max-len - const replacedGrants = await GrantReplacements.findAll({ where: { replacingGrantId: grant.id } }); + try { + // eslint-disable-next-line max-len + const replacedGrants = await GrantReplacements.findAll({ where: { replacingGrantId: grant.id } }); + + // If we don't have any replaced grants replacements we have nothing to do for this grant. + // Prevent confusion of throwing exception below. + if (!replacedGrants.length) { + logger.info(`updateCDIGrantsWithOldGrantData: No grant replacements found for CDI grant: ${grant.id}, skipping`); + return await Promise.resolve(); + } - // If we don't have any replaced grants replacements we have nothing to do for this grant. - // Prevent confusion of throwing exception below. - if (!replacedGrants.length) { - logger.info(`updateCDIGrantsWithOldGrantData: No grant replacements found for CDI grant: ${grant.id}, skipping`); - return Promise.resolve(); - } + // eslint-disable-next-line max-len + const validOldGrants = (await Promise.all(replacedGrants.map((rg) => Grant.findByPk(rg.replacedGrantId)))).filter(Boolean); - // eslint-disable-next-line max-len - const validOldGrants = (await Promise.all(replacedGrants.map((rg) => Grant.findByPk(rg.replacedGrantId)))).filter(Boolean); + const [regionId] = uniq(validOldGrants.map((g) => g.regionId)); + const [recipientId] = uniq(validOldGrants.map((g) => g.recipientId)); - const [regionId] = uniq(validOldGrants.map((g) => g.regionId)); - const [recipientId] = uniq(validOldGrants.map((g) => g.recipientId)); + if (!regionId || !recipientId) { + throw new Error(`Expected one region and recipient for grant ${grant.id}, got ${validOldGrants.length} valid grants`); + } - if (!regionId || !recipientId) { - throw new Error(`Expected one region and recipient for grant ${grant.id}, got ${validOldGrants.length} valid grants`); - } + // Ensure allValidOldGrants have the same recipient and region. + if (!validOldGrants.every( + (g) => g.regionId === regionId && g.recipientId === recipientId, + )) { + throw new Error(`Expected all valid replaced grants to have the same recipient and region for CDI grant ${grant.id}`); + } - // Ensure allValidOldGrants have the same recipient and region. - if (!validOldGrants.every((g) => g.regionId === regionId && g.recipientId === recipientId)) { - throw new Error(`Expected all valid replaced grants to have the same recipient and region for CDI grant ${grant.id}`); + // eslint-disable-next-line consistent-return + return grant.update({ recipientId, regionId }); + } catch (error) { + logger.error(`updateCDIGrantsWithOldGrantData: Error processing grant ${grant.id}, skipping:`, error); + return await Promise.resolve(); } - - // eslint-disable-next-line consistent-return - return grant.update({ recipientId, regionId }); }); await Promise.all(updates); } catch (error) { - logger.error('updateGrantsRecipients: Error updating grants:', error); + logger.error('updateGrantsRecipients: Fatal Error updating grants:', error); } }; From 526db29ed30fc783fe9047b31abe2512c9e69015 Mon Sep 17 00:00:00 2001 From: Adam Levin Date: Tue, 18 Feb 2025 11:02:54 -0500 Subject: [PATCH 3/3] change to error log instead of throw --- src/lib/updateGrantsRecipients.js | 57 +++++++++++++------------- src/lib/updateGrantsRecipients.test.js | 2 +- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/lib/updateGrantsRecipients.js b/src/lib/updateGrantsRecipients.js index d1788c4017..ec0f707c3d 100644 --- a/src/lib/updateGrantsRecipients.js +++ b/src/lib/updateGrantsRecipients.js @@ -183,45 +183,44 @@ async function getProgramPersonnel(grantId, programId, program) { export const updateCDIGrantsWithOldGrantData = async (grantsToUpdate) => { try { const updates = grantsToUpdate.map(async (grant) => { - try { - // eslint-disable-next-line max-len - const replacedGrants = await GrantReplacements.findAll({ where: { replacingGrantId: grant.id } }); - - // If we don't have any replaced grants replacements we have nothing to do for this grant. - // Prevent confusion of throwing exception below. - if (!replacedGrants.length) { - logger.info(`updateCDIGrantsWithOldGrantData: No grant replacements found for CDI grant: ${grant.id}, skipping`); - return await Promise.resolve(); - } + // eslint-disable-next-line max-len + const replacedGrants = await GrantReplacements.findAll({ where: { replacingGrantId: grant.id } }); - // eslint-disable-next-line max-len - const validOldGrants = (await Promise.all(replacedGrants.map((rg) => Grant.findByPk(rg.replacedGrantId)))).filter(Boolean); + // If we don't have any replaced grants replacements we have nothing to do for this grant. + // Prevent confusion of throwing exception below. + if (!replacedGrants.length) { + logger.info(`updateCDIGrantsWithOldGrantData: No grant replacements found for CDI grant: ${grant.id}, skipping`); + return Promise.resolve(); + } - const [regionId] = uniq(validOldGrants.map((g) => g.regionId)); - const [recipientId] = uniq(validOldGrants.map((g) => g.recipientId)); + // eslint-disable-next-line max-len + const validOldGrants = (await Promise.all(replacedGrants.map((rg) => Grant.findByPk(rg.replacedGrantId)))).filter(Boolean); - if (!regionId || !recipientId) { - throw new Error(`Expected one region and recipient for grant ${grant.id}, got ${validOldGrants.length} valid grants`); - } + const [regionId] = uniq(validOldGrants.map((g) => g.regionId)); + const [recipientId] = uniq(validOldGrants.map((g) => g.recipientId)); - // Ensure allValidOldGrants have the same recipient and region. - if (!validOldGrants.every( - (g) => g.regionId === regionId && g.recipientId === recipientId, - )) { - throw new Error(`Expected all valid replaced grants to have the same recipient and region for CDI grant ${grant.id}`); - } + if (!regionId || !recipientId) { + throw new Error(`Expected one region and recipient for grant ${grant.id}, got ${validOldGrants.length} valid grants`); + } - // eslint-disable-next-line consistent-return - return grant.update({ recipientId, regionId }); - } catch (error) { - logger.error(`updateCDIGrantsWithOldGrantData: Error processing grant ${grant.id}, skipping:`, error); - return await Promise.resolve(); + // Ensure allValidOldGrants have the same recipient and region. + if (!validOldGrants.every( + (g) => g.regionId === regionId && g.recipientId === recipientId, + )) { + logger.error( + 'updateGrantsRecipients: Error updating grants:', + `Expected all valid replaced grants to have the same recipient and region for CDI grant ${grant.id}, skipping`, + ); + return Promise.resolve(); } + + // eslint-disable-next-line consistent-return + return grant.update({ recipientId, regionId }); }); await Promise.all(updates); } catch (error) { - logger.error('updateGrantsRecipients: Fatal Error updating grants:', error); + logger.error('updateGrantsRecipients: Error updating grants:', error); } }; diff --git a/src/lib/updateGrantsRecipients.test.js b/src/lib/updateGrantsRecipients.test.js index ca4faf46b1..8a8f40fec7 100644 --- a/src/lib/updateGrantsRecipients.test.js +++ b/src/lib/updateGrantsRecipients.test.js @@ -1198,7 +1198,7 @@ describe('Update grants, program personnel, and recipients', () => { // Ensure logger.error was called. expect(logger.error).toHaveBeenCalledWith( 'updateGrantsRecipients: Error updating grants:', - new Error('Expected all valid replaced grants to have the same recipient and region for CDI grant 8546'), + 'Expected all valid replaced grants to have the same recipient and region for CDI grant 8546, skipping', ); }); });