Merge branch 'master' into 83846-disability-benefits-migrate-0781A-up…

…loads-to-lighthouse

.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 }}

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)

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

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
 

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(

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

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)

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

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

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

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