From 87a8bde3f84b7c31ca8ebec1279165c2dd4aac9e Mon Sep 17 00:00:00 2001 From: Sam Stuckey Date: Mon, 4 Nov 2024 08:09:30 -0700 Subject: [PATCH 1/4] [DBX-94173] fix timestamp string bug in 526 failure email (#19154) * [DBX-94173] Fix timestamp bug in 526 failure email * general save * Adding Zone --------- Co-authored-by: Kyle Soskin <37049625+kylesoskin@users.noreply.github.com> --- app/sidekiq/form526_status_polling_job.rb | 2 +- .../form526_submission_failure_email_job.rb | 8 ++++---- .../form526_backup_submission_process/submit.rb | 2 +- .../submit_spec.rb | 4 ++-- spec/sidekiq/form526_status_polling_job_spec.rb | 8 ++++---- ...form526_submission_failure_email_job_spec.rb | 17 ++++++++++++++--- 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/app/sidekiq/form526_status_polling_job.rb b/app/sidekiq/form526_status_polling_job.rb index b6fb4c918e0..071435190c5 100644 --- a/app/sidekiq/form526_status_polling_job.rb +++ b/app/sidekiq/form526_status_polling_job.rb @@ -76,7 +76,7 @@ def log_result(result) def notify_veteran(submission_id) if Flipper.enabled?(:send_backup_submission_polling_failure_email_notice) - Form526SubmissionFailureEmailJob.perform_async(submission_id, Time.now.utc) + Form526SubmissionFailureEmailJob.perform_async(submission_id, Time.now.utc.to_s) end end end diff --git a/app/sidekiq/form526_submission_failure_email_job.rb b/app/sidekiq/form526_submission_failure_email_job.rb index 3e2df65d493..92f54968a53 100644 --- a/app/sidekiq/form526_submission_failure_email_job.rb +++ b/app/sidekiq/form526_submission_failure_email_job.rb @@ -58,9 +58,9 @@ class Form526SubmissionFailureEmailJob raise e end - def perform(submission_id, date_of_failure = Time.now.utc) + def perform(submission_id, date_of_failure = Time.now.utc.to_s) @submission = Form526Submission.find(submission_id) - @date_of_failure = date_of_failure + @date_of_failure = Time.zone.parse(date_of_failure) send_email track_remedial_action log_success @@ -106,11 +106,11 @@ def personalisation date_submitted: submission.format_creation_time_for_mailers, forms_submitted: list_forms_submitted.presence || 'None', files_submitted: list_files_submitted.presence || 'None', - date_of_failure: + date_of_failure: parsed_date_of_failure } end - def date_of_failure + def parsed_date_of_failure @date_of_failure.strftime('%B %-d, %Y %-l:%M %P %Z').sub(/([ap])m/, '\1.m.') end diff --git a/lib/sidekiq/form526_backup_submission_process/submit.rb b/lib/sidekiq/form526_backup_submission_process/submit.rb index 137d0367876..69988d86ec8 100644 --- a/lib/sidekiq/form526_backup_submission_process/submit.rb +++ b/lib/sidekiq/form526_backup_submission_process/submit.rb @@ -54,7 +54,7 @@ class Submit ) if Flipper.enabled?(:send_backup_submission_exhaustion_email_notice) - ::Form526SubmissionFailureEmailJob.perform_async(form526_submission_id, Time.now.utc) + ::Form526SubmissionFailureEmailJob.perform_async(form526_submission_id, Time.now.utc.to_s) end rescue => e ::Rails.logger.error( diff --git a/spec/lib/sidekiq/form526_backup_submission_process/submit_spec.rb b/spec/lib/sidekiq/form526_backup_submission_process/submit_spec.rb index 8ca6940aceb..9c408d3388f 100644 --- a/spec/lib/sidekiq/form526_backup_submission_process/submit_spec.rb +++ b/spec/lib/sidekiq/form526_backup_submission_process/submit_spec.rb @@ -64,7 +64,7 @@ args = { 'jid' => form526_job_status.job_id, 'args' => [form526_submission.id] } subject.within_sidekiq_retries_exhausted_block(args) do expect(Form526SubmissionFailureEmailJob) - .to receive(:perform_async).with(form526_submission.id, timestamp) + .to receive(:perform_async).with(form526_submission.id, timestamp.to_s) end end end @@ -81,7 +81,7 @@ subject.within_sidekiq_retries_exhausted_block(args) do expect(Form526SubmissionFailureEmailJob) .not_to receive(:perform_async) - .with(form526_submission.id, timestamp) + .with(form526_submission.id, timestamp.to_s) end end end diff --git a/spec/sidekiq/form526_status_polling_job_spec.rb b/spec/sidekiq/form526_status_polling_job_spec.rb index c4434e7be3d..5fe5ccaacdf 100644 --- a/spec/sidekiq/form526_status_polling_job_spec.rb +++ b/spec/sidekiq/form526_status_polling_job_spec.rb @@ -159,14 +159,14 @@ .and_return(response) expect(Form526SubmissionFailureEmailJob) - .not_to receive(:perform_async).with(backup_submission_a.id, timestamp) + .not_to receive(:perform_async).with(backup_submission_a.id, timestamp.to_s) expect(Form526SubmissionFailureEmailJob) - .not_to receive(:perform_async).with(backup_submission_b.id, timestamp) + .not_to receive(:perform_async).with(backup_submission_b.id, timestamp.to_s) expect(Form526SubmissionFailureEmailJob) - .to receive(:perform_async).once.ordered.with(backup_submission_c.id, timestamp) + .to receive(:perform_async).once.ordered.with(backup_submission_c.id, timestamp.to_s) expect(Form526SubmissionFailureEmailJob) - .to receive(:perform_async).once.ordered.with(backup_submission_d.id, timestamp) + .to receive(:perform_async).once.ordered.with(backup_submission_d.id, timestamp.to_s) Form526StatusPollingJob.new.perform end diff --git a/spec/sidekiq/form526_submission_failure_email_job_spec.rb b/spec/sidekiq/form526_submission_failure_email_job_spec.rb index 67417856235..e387975fad0 100644 --- a/spec/sidekiq/form526_submission_failure_email_job_spec.rb +++ b/spec/sidekiq/form526_submission_failure_email_job_spec.rb @@ -40,11 +40,22 @@ } end + context 'when a timestamp is not passed' do + it 'marks the current time as the date_of_failure' do + Timecop.freeze(timestamp) do + expect(email_service).to receive(:send_email).with(expected_params) + + subject.perform_async(form526_submission.id) + subject.drain + end + end + end + it 'dispatches a failure notification email with the expected params' do Timecop.freeze(timestamp) do expect(email_service).to receive(:send_email).with(expected_params) - subject.perform_async(form526_submission.id) + subject.perform_async(form526_submission.id, timestamp.to_s) subject.drain end end @@ -85,7 +96,7 @@ Timecop.freeze(timestamp) do expect(email_service).to receive(:send_email).with(expected_params) - subject.perform_async(form526_submission.id) + subject.perform_async(form526_submission.id, timestamp.to_s) subject.drain end end @@ -117,7 +128,7 @@ Timecop.freeze(timestamp) do expect(email_service).to receive(:send_email).with(expected_params) - subject.perform_async(form526_submission.id) + subject.perform_async(form526_submission.id, timestamp.to_s) subject.drain end end From eac921e32ae333407c82249630a6bee51fc70d33 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Nov 2024 10:35:53 -0500 Subject: [PATCH 2/4] Bump statsd-instrument from 3.9.4 to 3.9.6 (#19233) Bumps [statsd-instrument](https://github.com/Shopify/statsd-instrument) from 3.9.4 to 3.9.6. - [Changelog](https://github.com/Shopify/statsd-instrument/blob/main/CHANGELOG.md) - [Commits](https://github.com/Shopify/statsd-instrument/compare/v3.9.4...v3.9.6) --- updated-dependencies: - dependency-name: statsd-instrument dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 676dcfa1625..cd3980294e4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1031,7 +1031,7 @@ GEM ffi ssrf_filter (1.1.2) staccato (0.5.3) - statsd-instrument (3.9.4) + statsd-instrument (3.9.6) stringio (3.1.1) strong_migrations (2.0.1) activerecord (>= 6.1) From 0a348301b476e94245cc7c806efa4bbc06186c9e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Nov 2024 10:37:52 -0500 Subject: [PATCH 3/4] Bump thollander/actions-comment-pull-request from 3.0.0 to 3.0.1 (#19234) Bumps [thollander/actions-comment-pull-request](https://github.com/thollander/actions-comment-pull-request) from 3.0.0 to 3.0.1. - [Release notes](https://github.com/thollander/actions-comment-pull-request/releases) - [Commits](https://github.com/thollander/actions-comment-pull-request/compare/e2c37e53a7d2227b61585343765f73a9ca57eda9...24bffb9b452ba05a4f3f77933840a6a841d1b32b) --- updated-dependencies: - dependency-name: thollander/actions-comment-pull-request dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/check_codeowners.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check_codeowners.yml b/.github/workflows/check_codeowners.yml index 7e8501ad587..d5bcd26a9e5 100644 --- a/.github/workflows/check_codeowners.yml +++ b/.github/workflows/check_codeowners.yml @@ -49,7 +49,7 @@ jobs: - name: Respond to PR if check CODEOWNERS exists for new files fails if: ${{ failure() }} - uses: thollander/actions-comment-pull-request@e2c37e53a7d2227b61585343765f73a9ca57eda9 # v3.0.0 + uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3.0.1 with: message: 'Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: ${{ env.offending_file }}' GITHUB_TOKEN: ${{ env.VA_VSP_BOT_GITHUB_TOKEN }} @@ -109,7 +109,7 @@ jobs: - name: Respond to PR if check CODEOWNERS exists for deleted files fails if: ${{ failure() }} - uses: thollander/actions-comment-pull-request@e2c37e53a7d2227b61585343765f73a9ca57eda9 # v3.0.0 + uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b # v3.0.1 with: message: 'Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file: ${{ env.offending_file }}' GITHUB_TOKEN: ${{ env.VA_VSP_BOT_GITHUB_TOKEN }} From 846e884beef45c2d0a916e5ae862476d6ea71095 Mon Sep 17 00:00:00 2001 From: Corey Ferris Date: Mon, 4 Nov 2024 10:43:46 -0500 Subject: [PATCH 4/4] clinic sorting error handling and logging hotfix (#19237) --- .../controllers/vaos/v2/clinics_controller.rb | 2 +- .../services/vaos/v2/appointments_service.rb | 9 ++- .../services/v2/appointment_service_spec.rb | 64 +++++++++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/modules/vaos/app/controllers/vaos/v2/clinics_controller.rb b/modules/vaos/app/controllers/vaos/v2/clinics_controller.rb index ce63b218cb2..dab8bf1d0ed 100644 --- a/modules/vaos/app/controllers/vaos/v2/clinics_controller.rb +++ b/modules/vaos/app/controllers/vaos/v2/clinics_controller.rb @@ -45,7 +45,7 @@ def recent_clinics sorted_clinics = [] sorted_appointments = appointments_service.get_recent_sorted_clinic_appointments - if sorted_appointments.nil? + if sorted_appointments.blank? render json: { message: 'No appointments found' }, status: :not_found return end diff --git a/modules/vaos/app/services/vaos/v2/appointments_service.rb b/modules/vaos/app/services/vaos/v2/appointments_service.rb index 8cc4deed9dd..b6f463d5a68 100644 --- a/modules/vaos/app/services/vaos/v2/appointments_service.rb +++ b/modules/vaos/app/services/vaos/v2/appointments_service.rb @@ -179,7 +179,14 @@ def get_recent_sorted_clinic_appointments end def sort_recent_appointments(appointments) - appointments.sort_by { |appointment| DateTime.parse(appointment.start) } + filtered_appts = appointments.reject { |appt| appt&.start.nil? } + removed_appts = appointments - filtered_appts + if removed_appts.length.positive? + removed_appts.each do |rem_appt| + Rails.logger.info("VAOS appointment sorting filtered out id #{rem_appt.id} due to missing start time.") + end + end + filtered_appts.sort_by { |appointment| DateTime.parse(appointment.start) } end # Returns the facility timezone id (eg. 'America/New_York') associated with facility id (location_id) diff --git a/modules/vaos/spec/services/v2/appointment_service_spec.rb b/modules/vaos/spec/services/v2/appointment_service_spec.rb index 6e9c06f4dfb..7c76f1afa7f 100644 --- a/modules/vaos/spec/services/v2/appointment_service_spec.rb +++ b/modules/vaos/spec/services/v2/appointment_service_spec.rb @@ -679,6 +679,70 @@ end end + describe '#get_recent_sorted_clinic_appointments' do + subject { instance_of_class.get_recent_sorted_clinic_appointments } + + let(:instance_of_class) { described_class.new(user) } + let(:mock_appointment_one) { double('Appointment', kind: 'clinic', start: '2022-12-02') } + let(:mock_appointment_two) { double('Appointment', kind: 'telehealth', start: '2022-12-01T21:38:01.476Z') } + let(:mock_appointment_three) { double('Appointment', kind: 'clinic', start: '2022-12-09T21:38:01.476Z') } + + context 'when appointments are available' do + before do + allow(instance_of_class).to receive(:get_appointments).and_return({ data: [mock_appointment_one, + mock_appointment_two, + mock_appointment_three] }) + end + + it 'returns the recent sorted clinic appointments' do + expect(subject).to eq([mock_appointment_two, mock_appointment_one, mock_appointment_three]) + end + end + + context 'when no appointments are available' do + before do + allow(instance_of_class).to receive(:get_appointments).and_return({ data: [] }) + end + + it 'returns nil' do + expect(subject.first).to be_nil + end + end + end + + describe '#sort_recent_appointments' do + subject { instance_of_class } + + let(:instance_of_class) { described_class.new(user) } + let(:mock_appointment_one) { double('Appointment', id: '123', kind: 'clinic', start: '2022-12-02') } + let(:mock_appointment_two) do + double('Appointment', id: '124', kind: 'telehealth', start: '2022-12-01T21:38:01.476Z') + end + let(:mock_appointment_three) { double('Appointment', id: '125', kind: 'clinic', start: '2022-12-09T21:38:01.476Z') } + let(:mock_appointment_four_no_start) { double('Appointment', id: '126', kind: 'clinic', start: nil) } + let(:appointments_input_no_start) do + [mock_appointment_one, mock_appointment_two, mock_appointment_three, mock_appointment_four_no_start] + end + let(:appointments_input) { [mock_appointment_one, mock_appointment_two, mock_appointment_three] } + let(:filtered_sorted_appointments) { [mock_appointment_two, mock_appointment_one, mock_appointment_three] } + + context 'when appointments are available' do + it 'sorts based on start time' do + expect(subject.send(:sort_recent_appointments, appointments_input)).to eq(filtered_sorted_appointments) + expect(Rails.logger).not_to receive(:info) + end + end + + context 'when appointments are available and at least one is missing a start time' do + it 'filters before sorting and logs removed appointments' do + allow(Rails.logger).to receive(:info) + expect(subject.send(:sort_recent_appointments, appointments_input_no_start)).to eq(filtered_sorted_appointments) + expect(Rails.logger).to have_received(:info) + .with('VAOS appointment sorting filtered out id 126 due to missing start time.') + end + end + end + describe '#get_appointment' do context 'using VAOS' do before do