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

Submission Status Job: Assign lighthouse_updated_at before running callbacks upon state change #20658

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

Thrillberg
Copy link
Contributor

Summary

This PR fixes a small bug where we would update state on a FormSubmissionAttempt (triggering an email to send) before populating the lighthouse_updated_at field. The emails, without this field set, would fail. Currently this whole Job is behind a feature flag (:benefits_intake_submission_status_job) which is OFF. Once we fix this, I believe we are safe to turn the feature flag back on.

Copy link
Contributor

@wayne-weibel wayne-weibel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new job is behind a flipper and runs once daily. Take the time to implement a handler to move the email logic out of the model.

@@ -152,7 +153,7 @@ def update_attempt_record(uuid, status, submission)
form_submission_attempt.vbms!
end

form_submission_attempt.update(lighthouse_updated_at:, error_message:)
form_submission_attempt.update(error_message:) if error_message
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop the conditional, there should only be an error_message if the attempt is in the failure state

@va-vfs-bot va-vfs-bot temporarily deployed to benefits_intake_submission_status_job_bugfix/main/main February 6, 2025 17:45 Inactive
Copy link
Contributor

@wayne-weibel wayne-weibel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a temporary fix, so the old job can be removed, and not disrupt email notifications.

@wayne-weibel wayne-weibel merged commit 67c685a into master Feb 11, 2025
36 of 38 checks passed
@wayne-weibel wayne-weibel deleted the benefits_intake_submission_status_job_bugfix branch February 11, 2025 15:05
holdenhinkle pushed a commit that referenced this pull request Feb 12, 2025
patrick-brown-oddball pushed a commit that referenced this pull request Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants