From a3322bcb2b4725715e3116d35d687db601f4e49b Mon Sep 17 00:00:00 2001 From: GarrettEHill Date: Fri, 20 Sep 2024 00:15:57 -0700 Subject: [PATCH 1/8] Add a new hook to update the flags as part of the merge process when the originalGoalId is updated --- src/models/hooks/activityReportGoal.js | 42 +++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/models/hooks/activityReportGoal.js b/src/models/hooks/activityReportGoal.js index 5b22f0fe62..2ef2018006 100644 --- a/src/models/hooks/activityReportGoal.js +++ b/src/models/hooks/activityReportGoal.js @@ -152,6 +152,44 @@ const destroyLinkedSimilarityGroups = async (sequelize, instance, options) => { }); }; +const updateOnARAndOnApprovedARForMergedGoals = async (sequelize, instance) => { + const changed = instance.changed(); + + // Check if both originalGoalId and goalId have been changed and originalGoalId is not null + if (Array.isArray(changed) + && changed.includes('originalGoalId') + && changed.includes('goalId') + && instance.originalGoalId !== null) { + const goalId = instance.goalId; + + // Check if the ActivityReport linked to this ActivityReportGoal has a calculatedStatus of 'approved' + const approvedActivityReports = await sequelize.models.ActivityReport.count({ + where: { + calculatedStatus: 'approved', + id: instance.activityReportId, // Use the activityReportId from the instance + }, + }); + + const onApprovedAR = approvedActivityReports > 0; + + // Update only if the current values differ + await sequelize.models.Goal.update( + { onAR: true, onApprovedAR }, + { + where: { + id: goalId, + [sequelize.Op.or]: [ + { onAR: { [sequelize.Op.ne]: true } }, // Update if onAR is not already true + { onApprovedAR: { [sequelize.Op.ne]: onApprovedAR } }, // Update if onApprovedAR differs + ], + }, + individualHooks: true, // Ensure individual hooks are triggered + } + ); + } +}; + + const afterCreate = async (sequelize, instance, options) => { await processForEmbeddedResources(sequelize, instance, options); await autoPopulateLinker(sequelize, instance, options); @@ -176,7 +214,8 @@ const afterDestroy = async (sequelize, instance, options) => { const afterUpdate = async (sequelize, instance, options) => { await processForEmbeddedResources(sequelize, instance, options); - await destroyLinkedSimilarityGroups(sequelize, instance, options); + await destroyLinkedSimilarityGroups(sequelize, instance, options); + await updateOnARAndOnApprovedARForMergedGoals(sequelize, instance); }; export { @@ -185,6 +224,7 @@ export { recalculateOnAR, propagateDestroyToMetadata, destroyLinkedSimilarityGroups, + updateOnARAndOnApprovedARForMergedGoals, afterCreate, beforeDestroy, afterDestroy, From 5d091827ca868498f32cd90ee28f14291cd2c5b4 Mon Sep 17 00:00:00 2001 From: GarrettEHill Date: Fri, 20 Sep 2024 00:16:12 -0700 Subject: [PATCH 2/8] Tests to cover the new hook --- src/models/hooks/activityReportGoal.test.js | 165 +++++++++++++++++++- 1 file changed, 164 insertions(+), 1 deletion(-) diff --git a/src/models/hooks/activityReportGoal.test.js b/src/models/hooks/activityReportGoal.test.js index 882bf46506..ffa9a7c548 100644 --- a/src/models/hooks/activityReportGoal.test.js +++ b/src/models/hooks/activityReportGoal.test.js @@ -1,5 +1,8 @@ const { REPORT_STATUSES } = require('@ttahub/common'); -const { destroyLinkedSimilarityGroups } = require('./activityReportGoal'); +const { + destroyLinkedSimilarityGroups, + updateOnARAndOnApprovedARForMergedGoals, +} = require('./activityReportGoal'); describe('destroyLinkedSimilarityGroups', () => { afterEach(() => { @@ -205,3 +208,163 @@ describe('destroyLinkedSimilarityGroups', () => { expect(sequelize.models.GoalSimilarityGroup.destroy).not.toHaveBeenCalled(); }); }); + +describe('updateOnARAndOnApprovedARForMergedGoals', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + const sequelize = { + models: { + Goal: { + update: jest.fn(), + }, + ActivityReport: { + count: jest.fn(), + }, + }, + }; + + it('should update onAR and onApprovedAR for merged goals when originalGoalId and goalId are changed', async () => { + const instance = { + goalId: 1, + originalGoalId: 2, + activityReportId: 1, + changed: () => ['originalGoalId', 'goalId'], // Simulate that both columns have changed + }; + + const options = { + transaction: 'mockTransaction', + }; + + // Mock the necessary Sequelize methods + sequelize.models.ActivityReport.count.mockResolvedValue(1); // Simulate approved ActivityReport exists + + await updateOnARAndOnApprovedARForMergedGoals(sequelize, instance, options); + + expect(sequelize.models.ActivityReport.count).toHaveBeenCalledWith({ + where: { + calculatedStatus: 'approved', + id: instance.activityReportId, + }, + }); + + expect(sequelize.models.Goal.update).toHaveBeenCalledWith( + { onAR: true, onApprovedAR: true }, + { + where: { + id: instance.goalId, + [sequelize.Op.or]: [ + { onAR: { [sequelize.Op.ne]: true } }, // Ensure onAR condition is in the where clause + { onApprovedAR: { [sequelize.Op.ne]: true } }, // Ensure onApprovedAR condition is in the where clause + ], + }, + individualHooks: true, + } + ); + }); + + + it('should update onAR and onApprovedAR with false when there are no approved activity reports', async () => { + const instance = { + goalId: 1, + originalGoalId: 2, + activityReportId: 1, + changed: () => ['originalGoalId', 'goalId'], // Simulate that both columns have changed + }; + + const options = { + transaction: 'mockTransaction', + }; + + // Mock the necessary Sequelize methods + sequelize.models.ActivityReport.count.mockResolvedValue(0); // No approved ActivityReports + + await updateOnARAndOnApprovedARForMergedGoals(sequelize, instance, options); + + expect(sequelize.models.ActivityReport.count).toHaveBeenCalledWith({ + where: { + calculatedStatus: 'approved', + id: instance.activityReportId, + }, + }); + + expect(sequelize.models.Goal.update).toHaveBeenCalledWith( + { onAR: true, onApprovedAR: false }, // onApprovedAR is false since no approved reports + { + where: { id: instance.goalId }, + individualHooks: true, + } + ); + }); + + it('should not update if originalGoalId or goalId is not changed', async () => { + const instance = { + goalId: 1, + originalGoalId: 2, + activityReportId: 1, + changed: () => [], // Simulate no changes + }; + + const options = { + transaction: 'mockTransaction', + }; + + await updateOnARAndOnApprovedARForMergedGoals(sequelize, instance, options); + + expect(sequelize.models.ActivityReport.count).not.toHaveBeenCalled(); + expect(sequelize.models.Goal.update).not.toHaveBeenCalled(); + }); + + it('should not update if originalGoalId is null', async () => { + const instance = { + goalId: 1, + originalGoalId: null, + activityReportId: 1, + changed: () => ['originalGoalId', 'goalId'], // Simulate that both columns have changed + }; + + const options = { + transaction: 'mockTransaction', + }; + + await updateOnARAndOnApprovedARForMergedGoals(sequelize, instance, options); + + expect(sequelize.models.ActivityReport.count).not.toHaveBeenCalled(); + expect(sequelize.models.Goal.update).not.toHaveBeenCalled(); + }); + + it('should not update if onAR and onApprovedAR are already set to the correct values', async () => { + const instance = { + goalId: 1, + originalGoalId: 2, + activityReportId: 1, + changed: () => ['originalGoalId', 'goalId'], // Simulate that both columns have changed + }; + + const options = { + transaction: 'mockTransaction', + }; + + // Mock the necessary Sequelize methods + sequelize.models.ActivityReport.count.mockResolvedValue(1); // Simulate approved ActivityReport exists + sequelize.models.Goal.update.mockResolvedValue(0); // Simulate that the update doesn't happen + + await updateOnARAndOnApprovedARForMergedGoals(sequelize, instance, options); + + expect(sequelize.models.Goal.update).toHaveBeenCalledWith( + { onAR: true, onApprovedAR: true }, + { + where: { + id: instance.goalId, + [sequelize.Op.or]: [ + { onAR: { [sequelize.Op.ne]: true } }, // Check if onAR is already true + { onApprovedAR: { [sequelize.Op.ne]: true } }, // Check if onApprovedAR is already true + ], + }, + individualHooks: true, + } + ); + }); + +}); From 719d59e69808dedb83d3dd46a1c31bbf43b6ac71 Mon Sep 17 00:00:00 2001 From: GarrettEHill Date: Fri, 20 Sep 2024 00:33:32 -0700 Subject: [PATCH 3/8] linter fixes --- src/models/hooks/activityReportGoal.js | 20 +++++----- src/models/hooks/activityReportGoal.test.js | 43 +++++++++++---------- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/models/hooks/activityReportGoal.js b/src/models/hooks/activityReportGoal.js index 2ef2018006..e39b986ce8 100644 --- a/src/models/hooks/activityReportGoal.js +++ b/src/models/hooks/activityReportGoal.js @@ -160,13 +160,14 @@ const updateOnARAndOnApprovedARForMergedGoals = async (sequelize, instance) => { && changed.includes('originalGoalId') && changed.includes('goalId') && instance.originalGoalId !== null) { - const goalId = instance.goalId; + const {goalId} = instance; - // Check if the ActivityReport linked to this ActivityReportGoal has a calculatedStatus of 'approved' + // Check if the ActivityReport linked to this ActivityReportGoal has a + // calculatedStatus of 'approved' const approvedActivityReports = await sequelize.models.ActivityReport.count({ where: { calculatedStatus: 'approved', - id: instance.activityReportId, // Use the activityReportId from the instance + id: instance.activityReportId, // Use the activityReportId from the instance }, }); @@ -179,17 +180,18 @@ const updateOnARAndOnApprovedARForMergedGoals = async (sequelize, instance) => { where: { id: goalId, [sequelize.Op.or]: [ - { onAR: { [sequelize.Op.ne]: true } }, // Update if onAR is not already true - { onApprovedAR: { [sequelize.Op.ne]: onApprovedAR } }, // Update if onApprovedAR differs + // Update if onAR is not already true + { onAR: { [sequelize.Op.ne]: true } }, + // Update if onApprovedAR differs + { onApprovedAR: { [sequelize.Op.ne]: onApprovedAR } }, ], }, - individualHooks: true, // Ensure individual hooks are triggered - } + individualHooks: true, // Ensure individual hooks are triggered + }, ); } }; - const afterCreate = async (sequelize, instance, options) => { await processForEmbeddedResources(sequelize, instance, options); await autoPopulateLinker(sequelize, instance, options); @@ -214,7 +216,7 @@ const afterDestroy = async (sequelize, instance, options) => { const afterUpdate = async (sequelize, instance, options) => { await processForEmbeddedResources(sequelize, instance, options); - await destroyLinkedSimilarityGroups(sequelize, instance, options); + await destroyLinkedSimilarityGroups(sequelize, instance, options); await updateOnARAndOnApprovedARForMergedGoals(sequelize, instance); }; diff --git a/src/models/hooks/activityReportGoal.test.js b/src/models/hooks/activityReportGoal.test.js index ffa9a7c548..0553cc4839 100644 --- a/src/models/hooks/activityReportGoal.test.js +++ b/src/models/hooks/activityReportGoal.test.js @@ -232,38 +232,40 @@ describe('updateOnARAndOnApprovedARForMergedGoals', () => { activityReportId: 1, changed: () => ['originalGoalId', 'goalId'], // Simulate that both columns have changed }; - + const options = { transaction: 'mockTransaction', }; - + // Mock the necessary Sequelize methods - sequelize.models.ActivityReport.count.mockResolvedValue(1); // Simulate approved ActivityReport exists - + // Simulate approved ActivityReport exists + sequelize.models.ActivityReport.count.mockResolvedValue(1); + await updateOnARAndOnApprovedARForMergedGoals(sequelize, instance, options); - + expect(sequelize.models.ActivityReport.count).toHaveBeenCalledWith({ where: { calculatedStatus: 'approved', id: instance.activityReportId, }, }); - + expect(sequelize.models.Goal.update).toHaveBeenCalledWith( { onAR: true, onApprovedAR: true }, { where: { id: instance.goalId, [sequelize.Op.or]: [ - { onAR: { [sequelize.Op.ne]: true } }, // Ensure onAR condition is in the where clause - { onApprovedAR: { [sequelize.Op.ne]: true } }, // Ensure onApprovedAR condition is in the where clause + // Ensure onAR condition is in the where clause + { onAR: { [sequelize.Op.ne]: true } }, + // Ensure onApprovedAR condition is in the where clause + { onApprovedAR: { [sequelize.Op.ne]: true } }, ], }, individualHooks: true, - } + }, ); }); - it('should update onAR and onApprovedAR with false when there are no approved activity reports', async () => { const instance = { @@ -294,7 +296,7 @@ describe('updateOnARAndOnApprovedARForMergedGoals', () => { { where: { id: instance.goalId }, individualHooks: true, - } + }, ); }); @@ -341,17 +343,19 @@ describe('updateOnARAndOnApprovedARForMergedGoals', () => { activityReportId: 1, changed: () => ['originalGoalId', 'goalId'], // Simulate that both columns have changed }; - + const options = { transaction: 'mockTransaction', }; - + // Mock the necessary Sequelize methods - sequelize.models.ActivityReport.count.mockResolvedValue(1); // Simulate approved ActivityReport exists - sequelize.models.Goal.update.mockResolvedValue(0); // Simulate that the update doesn't happen - + // Simulate approved ActivityReport exists + sequelize.models.ActivityReport.count.mockResolvedValue(1); + // Simulate that the update doesn't happen + sequelize.models.Goal.update.mockResolvedValue(0); + await updateOnARAndOnApprovedARForMergedGoals(sequelize, instance, options); - + expect(sequelize.models.Goal.update).toHaveBeenCalledWith( { onAR: true, onApprovedAR: true }, { @@ -363,8 +367,7 @@ describe('updateOnARAndOnApprovedARForMergedGoals', () => { ], }, individualHooks: true, - } + }, ); - }); - + }); }); From 3954d9ee68e9d6ebcf63880c2ce055172f06c755 Mon Sep 17 00:00:00 2001 From: GarrettEHill Date: Fri, 20 Sep 2024 00:38:25 -0700 Subject: [PATCH 4/8] more lint --- src/models/hooks/activityReportGoal.js | 2 +- src/models/hooks/activityReportGoal.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/models/hooks/activityReportGoal.js b/src/models/hooks/activityReportGoal.js index e39b986ce8..ea40ec80ed 100644 --- a/src/models/hooks/activityReportGoal.js +++ b/src/models/hooks/activityReportGoal.js @@ -160,7 +160,7 @@ const updateOnARAndOnApprovedARForMergedGoals = async (sequelize, instance) => { && changed.includes('originalGoalId') && changed.includes('goalId') && instance.originalGoalId !== null) { - const {goalId} = instance; + const { goalId } = instance; // Check if the ActivityReport linked to this ActivityReportGoal has a // calculatedStatus of 'approved' diff --git a/src/models/hooks/activityReportGoal.test.js b/src/models/hooks/activityReportGoal.test.js index 0553cc4839..1027fc84d7 100644 --- a/src/models/hooks/activityReportGoal.test.js +++ b/src/models/hooks/activityReportGoal.test.js @@ -369,5 +369,5 @@ describe('updateOnARAndOnApprovedARForMergedGoals', () => { individualHooks: true, }, ); - }); + }); }); From 66e268d4f60397741981e9b45862c06c3028309d Mon Sep 17 00:00:00 2001 From: GarrettEHill Date: Fri, 20 Sep 2024 01:17:53 -0700 Subject: [PATCH 5/8] Update activityReportGoal.test.js --- src/models/hooks/activityReportGoal.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/models/hooks/activityReportGoal.test.js b/src/models/hooks/activityReportGoal.test.js index 1027fc84d7..a72ae48330 100644 --- a/src/models/hooks/activityReportGoal.test.js +++ b/src/models/hooks/activityReportGoal.test.js @@ -1,3 +1,4 @@ +const { Op } = require('sequelize'); // Import Sequelize operators const { REPORT_STATUSES } = require('@ttahub/common'); const { destroyLinkedSimilarityGroups, From 4da4e9f047eb5b777f29f2e250e79cc7646637a4 Mon Sep 17 00:00:00 2001 From: GarrettEHill Date: Fri, 20 Sep 2024 08:57:22 -0700 Subject: [PATCH 6/8] Fix implementation error --- src/models/hooks/activityReportGoal.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/models/hooks/activityReportGoal.js b/src/models/hooks/activityReportGoal.js index ea40ec80ed..1552e1402f 100644 --- a/src/models/hooks/activityReportGoal.js +++ b/src/models/hooks/activityReportGoal.js @@ -1,3 +1,4 @@ +const { Op } = require('sequelize'); const { REPORT_STATUSES } = require('@ttahub/common'); const { GOAL_COLLABORATORS } = require('../../constants'); const { @@ -179,11 +180,11 @@ const updateOnARAndOnApprovedARForMergedGoals = async (sequelize, instance) => { { where: { id: goalId, - [sequelize.Op.or]: [ + [Op.or]: [ // Update if onAR is not already true - { onAR: { [sequelize.Op.ne]: true } }, + { onAR: { [Op.ne]: true } }, // Update if onApprovedAR differs - { onApprovedAR: { [sequelize.Op.ne]: onApprovedAR } }, + { onApprovedAR: { [Op.ne]: onApprovedAR } }, ], }, individualHooks: true, // Ensure individual hooks are triggered From df0f71dec16f96f3e0a1e35dcd3cff9663f3a63e Mon Sep 17 00:00:00 2001 From: GarrettEHill Date: Fri, 20 Sep 2024 09:07:24 -0700 Subject: [PATCH 7/8] Update activityReportGoal.test.js --- src/models/hooks/activityReportGoal.test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/models/hooks/activityReportGoal.test.js b/src/models/hooks/activityReportGoal.test.js index a72ae48330..1d418205b6 100644 --- a/src/models/hooks/activityReportGoal.test.js +++ b/src/models/hooks/activityReportGoal.test.js @@ -256,11 +256,11 @@ describe('updateOnARAndOnApprovedARForMergedGoals', () => { { where: { id: instance.goalId, - [sequelize.Op.or]: [ + [Op.or]: [ // Ensure onAR condition is in the where clause - { onAR: { [sequelize.Op.ne]: true } }, + { onAR: { [Op.ne]: true } }, // Ensure onApprovedAR condition is in the where clause - { onApprovedAR: { [sequelize.Op.ne]: true } }, + { onApprovedAR: { [Op.ne]: true } }, ], }, individualHooks: true, @@ -362,9 +362,9 @@ describe('updateOnARAndOnApprovedARForMergedGoals', () => { { where: { id: instance.goalId, - [sequelize.Op.or]: [ - { onAR: { [sequelize.Op.ne]: true } }, // Check if onAR is already true - { onApprovedAR: { [sequelize.Op.ne]: true } }, // Check if onApprovedAR is already true + [Op.or]: [ + { onAR: { [Op.ne]: true } }, // Check if onAR is already true + { onApprovedAR: { [Op.ne]: true } }, // Check if onApprovedAR is already true ], }, individualHooks: true, From 996a64c8c865f7cc04050f7bdf3fa896dcbd9cab Mon Sep 17 00:00:00 2001 From: GarrettEHill Date: Fri, 20 Sep 2024 10:15:43 -0700 Subject: [PATCH 8/8] Update activityReportGoal.test.js --- src/models/hooks/activityReportGoal.test.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/models/hooks/activityReportGoal.test.js b/src/models/hooks/activityReportGoal.test.js index 1d418205b6..f4db5f3778 100644 --- a/src/models/hooks/activityReportGoal.test.js +++ b/src/models/hooks/activityReportGoal.test.js @@ -295,7 +295,15 @@ describe('updateOnARAndOnApprovedARForMergedGoals', () => { expect(sequelize.models.Goal.update).toHaveBeenCalledWith( { onAR: true, onApprovedAR: false }, // onApprovedAR is false since no approved reports { - where: { id: instance.goalId }, + where: { + id: instance.goalId, + [Op.or]: [ + // Ensure onAR condition is in the where clause + { onAR: { [Op.ne]: true } }, + // Ensure onApprovedAR condition is in the where clause + { onApprovedAR: { [Op.ne]: false } }, + ], + }, individualHooks: true, }, );