-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TTAHUB-3757] Close monitoring goals in CRON when there are no active citations or un-approved reports #2589
Conversation
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 was asked not to merge in these changes, but post them into the PR as a review. The tests will need to be updated on src/tools/createMonitoringGoals.test.js
@@ -10,6 +10,39 @@ interface GoalStatusChangeParams { | |||
transaction?: Sequelize.Transaction; | |||
} | |||
|
|||
export async function changeGoalStatusWithSystemUser({ |
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.
do you want to DRY this up and combine the bottom half of the two functions?
I.E. both call basically this exact thing, maybe they could call an "updateGoalStatus" function or something:
// Change goal status.
await db.GoalStatusChange.create({
goalId: goal.id,
userId: null, // For now we will use null to prevent FK constraint violation.
userName: 'system',
userRoles: null,
oldStatus: goal.status,
newStatus,
reason,
context,
});
// Reload goal.
await goal.reload();
// Return goal.
return goal;
I'll approve
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 thought about this but just decided to keep the separate.
68edc6f
into
al-ttahub-3603-activity-report-objective-citations
Description of change
This change adds SQL to the create monitoring job to set goals to closed if they have no active citations and they have no un-approved reports.
How to test
Issue(s)
Checklists
Every PR
Before merge to main
Production Deploy
After merge/deploy