Skip to content
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

AdamAdHocTeam
Copy link
Collaborator

@AdamAdHocTeam AdamAdHocTeam commented Jan 9, 2025

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

  • Review the code.
  • Make sure all tests pass.

Issue(s)

Checklists

Every PR

  • Meets issue criteria
  • JIRA ticket status updated
  • Code is meaningfully tested
  • Meets accessibility standards (WCAG 2.1 Levels A, AA)
  • API Documentation updated
  • Boundary diagram updated
  • Logical Data Model updated
  • Architectural Decision Records written for major infrastructure decisions
  • UI review complete
  • QA review complete

Before merge to main

  • OHS demo complete
  • Ready to create production PR

Production Deploy

  • Staging smoke test completed

After merge/deploy

  • Update JIRA ticket status

@AdamAdHocTeam AdamAdHocTeam changed the base branch from main to al-ttahub-3603-activity-report-objective-citations January 9, 2025 20:56
@AdamAdHocTeam AdamAdHocTeam marked this pull request as ready for review January 9, 2025 21:15
Copy link
Collaborator

@GarrettEHill GarrettEHill left a 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

src/tools/createMonitoringGoals.js Show resolved Hide resolved
src/tools/createMonitoringGoals.js Outdated Show resolved Hide resolved
src/tools/createMonitoringGoals.js Outdated Show resolved Hide resolved
src/tools/createMonitoringGoals.js Show resolved Hide resolved
@@ -10,6 +10,39 @@ interface GoalStatusChangeParams {
transaction?: Sequelize.Transaction;
}

export async function changeGoalStatusWithSystemUser({
Copy link
Collaborator

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

Copy link
Collaborator Author

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.

@AdamAdHocTeam AdamAdHocTeam merged commit 68edc6f into al-ttahub-3603-activity-report-objective-citations Jan 15, 2025
10 checks passed
@AdamAdHocTeam AdamAdHocTeam deleted the al-ttahub-3757-close-monitoring-goals branch January 15, 2025 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants