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

Send reject resolution message on 4/13 and 4/22 for clients that have been notified of rejection and not yet been accepted #4479

Merged
merged 5 commits into from
Apr 12, 2024

Conversation

embarnard
Copy link
Contributor

No description provided.

Copy link

Heroku app: https://gyr-review-app-4479-5c47e86d0c2c.herokuapp.com/
View logs: heroku logs --app gyr-review-app-4479 (optionally add --tail)

def perform(intake)
return unless notified_of_rejected_and_not_accepted(intake)

puts "gets here"
Copy link
Contributor

Choose a reason for hiding this comment

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

stray puts

submission.efile_submission_transitions.map(&:to_state)
end.uniq

last_state = intake.efile_submissions.last.efile_submission_transitions.last.to_state
Copy link
Contributor

Choose a reason for hiding this comment

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

i think you can do intake.efile_submissions.last.current_state

Copy link
Member

@mpidcock mpidcock left a comment

Choose a reason for hiding this comment

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

Non-blocking suggestion

Comment on lines +11 to +25
ny_intakes = StateFileNyIntake.joins(efile_submissions: :efile_submission_transitions)
.where(efile_submission_transitions: { to_state: 'notified_of_rejection' })
.where.not(id: StateFileNyIntake.joins(efile_submissions: :efile_submission_transitions)
.where(efile_submission_transitions: { to_state: 'accepted' })
.select(:id))
.distinct

az_intakes = StateFileAzIntake.joins(efile_submissions: :efile_submission_transitions)
.where(efile_submission_transitions: { to_state: 'notified_of_rejection' })
.where.not(id: StateFileAzIntake.joins(efile_submissions: :efile_submission_transitions)
.where(efile_submission_transitions: { to_state: 'accepted' })
.select(:id))
.distinct

ny_intakes.to_a + az_intakes.to_a
Copy link
Member

Choose a reason for hiding this comment

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

Instead of iterating over the state intake classes separates, some other new message implementations have been doing both at once using ApplicationRecord::STATE_INTAKE_CLASS_NAMES.each do |base_class| so that there aren't multiple blocks to maintain going forward. Something to consider doing here as well, but not blocking.

https://github.com/codeforamerica/vita-min/pull/4469/files#diff-a989d8d483d2f5596544f4765e35d8cd663fc6710402932443c8a3737565ce14R10

crontab Outdated
@@ -8,3 +8,5 @@
*/5 * * * * bundle exec rake worker_heartbeat:perform
*/10 * * * * bundle exec rake efile:poll_and_get_acknowledgments
0 17 * * * bundle exec rake state_file:reminder_to_finish_state_return
0 14 13 4 * bundle exec rake send_reject_resolution_reminder_notifications:send
0 14 22 4 * bundle exec rake send_reject_resolution_reminder_notifications:send
Copy link
Contributor

Choose a reason for hiding this comment

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

UTC?

crontab Outdated
@@ -8,3 +8,5 @@
*/5 * * * * bundle exec rake worker_heartbeat:perform
*/10 * * * * bundle exec rake efile:poll_and_get_acknowledgments
0 17 * * * bundle exec rake state_file:reminder_to_finish_state_return
0 14 13 4 * bundle exec rake send_reject_resolution_reminder_notifications:send
Copy link
Contributor Author

Choose a reason for hiding this comment

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

change 14 to 21

namespace :send_reject_resolution_reminder_notifications do
desc 'Send reject resolution reminder notifications'
task 'send' => :environment do
BATCH_SIZE = 50
Copy link
Contributor

Choose a reason for hiding this comment

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

use batch size

Copy link
Contributor

Choose a reason for hiding this comment

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

or remove

Copy link
Contributor

@jenny-heath jenny-heath left a comment

Choose a reason for hiding this comment

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

pair reviewed, looks good (some minor changes, see comments)

namespace :send_reject_resolution_reminder_notifications do
desc 'Send reject resolution reminder notifications'
task 'send' => :environment do
BATCH_SIZE = 50
Copy link
Contributor

Choose a reason for hiding this comment

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

or remove

@embarnard embarnard force-pushed the reject-resolution-msg-187355965 branch from 87db214 to c99ea3d Compare April 12, 2024 18:25
… been notified of rejection and not yet been accepted
@embarnard embarnard force-pushed the reject-resolution-msg-187355965 branch from c99ea3d to ba8acb2 Compare April 12, 2024 18:28
@embarnard embarnard merged commit be6f62e into main Apr 12, 2024
2 of 6 checks passed
@embarnard embarnard deleted the reject-resolution-msg-187355965 branch April 12, 2024 19:17
jenny-heath added a commit that referenced this pull request Apr 12, 2024
2024-04-12 Last release of tax season 2024

* Send reject resolution message on 4/13 and 4/22 for clients that have been notified of rejection and not yet been accepted (#4479)
* Update GYR banner for closing season (#4473)
* Always showing download button after April 25 #187355990  (#4481)
* Post-Deadline notification (#4475)
* Pre-Deadline notification (#4469)
* truncate 3rd party designee name (#4462)
* Change logic for Reminder notification (#4447)
* Close fyst #187355990 (#4472)
* Fix submission and skip flaky tests #187278919 (#4468)
* Refine AZ excise credit #186927846 (#4418)
* pre-populate w2 info (#4457)
* Center content on desktop #186632607 (#4444)
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.

None yet

4 participants