Strip timezone from POST params (#20790)

* Strip timezone from POST params * Fix linter * Removed redundant error handling * Linter * Adjust param casing * update casing for param * Updated casing on swagger docs * update param position * Corrected swagger --------- Co-authored-by: stevenjcumming <[email protected]>
department-of-veterans-affairs · Feb 19, 2025 · 6a7fbb7 · 6a7fbb7
1 parent 14a95cc
commit 6a7fbb7
Show file tree

Hide file tree

Showing 9 changed files with 64 additions and 62 deletions.
diff --git a/app/swagger/swagger/requests/travel_pay.rb b/app/swagger/swagger/requests/travel_pay.rb
@@ -73,11 +73,15 @@ class TravelPay
           parameter :authorization
 
           parameter do
-            key :name, 'appt_datetime'
-            key :in, :query
-            key :description, 'Appointment claim submission datetime'
-            key :required, false
-            key :type, :string
+            key :name, :appointmentDatetime
+            key :in, :body
+            key :description, 'Appointment claim start time'
+            key :required, true
+            schema do
+              property :appointmentDatetime do
+                key :type, :string
+              end
+            end
           end
 
           response 201 do

diff --git a/modules/travel_pay/app/controllers/travel_pay/v0/claims_controller.rb b/modules/travel_pay/app/controllers/travel_pay/v0/claims_controller.rb
@@ -46,11 +46,11 @@ def create
         begin
           Rails.logger.info(message: 'SMOC transaction START')
 
-          appt_id = get_appt_or_raise(params['appointmentDatetime'])
+          appt_id = get_appt_or_raise(params['appointment_datetime'])
           claim_id = get_claim_id(appt_id)
 
           Rails.logger.info(message: "SMOC transaction: Add expense to claim #{claim_id.slice(0, 8)}")
-          expense_service.add_expense({ 'claim_id' => claim_id, 'appt_date' => params['appointmentDatetime'] })
+          expense_service.add_expense({ 'claim_id' => claim_id, 'appt_date' => params['appointment_datetime'] })
 
           Rails.logger.info(message: "SMOC transaction: Submit claim #{claim_id.slice(0, 8)}")
           submitted_claim = claims_service.submit_claim(claim_id)
@@ -101,7 +101,6 @@ def scrub_logs
 
       def get_appt_or_raise(appt_datetime)
         appt_not_found_msg = "No appointment found for #{appt_datetime}"
-
         Rails.logger.info(message: "SMOC transaction: Get appt by date time: #{appt_datetime}")
         appt = appts_service.get_appointment_by_date_time({ 'appt_datetime' => appt_datetime })
 

diff --git a/modules/travel_pay/app/services/travel_pay/appointments_client.rb b/modules/travel_pay/app/services/travel_pay/appointments_client.rb
@@ -22,7 +22,6 @@ def get_all_appointments(veis_token, btsss_token, params = {})
       btsss_url = Settings.travel_pay.base_url
       correlation_id = SecureRandom.uuid
       Rails.logger.debug(message: 'Correlation ID', correlation_id:)
-
       query_path = if params.empty?
                      'api/v1.2/appointments'
                    else

diff --git a/modules/travel_pay/app/services/travel_pay/appointments_service.rb b/modules/travel_pay/app/services/travel_pay/appointments_service.rb
@@ -45,14 +45,17 @@ def find_by_date_time(date_string, appointments)
         Rails.logger.error(message: 'Invalid appointment time provided (appointment time cannot be nil).')
         raise ArgumentError, message: 'Invalid appointment time provided (appointment time cannot be nil).'
       elsif date_string.present?
-        parsed_date_time = DateTime.parse(date_string)
-
+        parsed_date_time = DateUtils.strip_timezone(date_string)
         appointments.find do |appt|
-          !appt['appointmentDateTime'].nil? &&
-            parsed_date_time == DateTime.parse(appt['appointmentDateTime'])
+          begin
+            parsed_appt_time = DateUtils.strip_timezone(appt['appointmentDateTime'])
+          rescue TravelPay::InvalidComparableError => e
+            Rails.logger.warn("#{e} Appointment Datetime was nil")
+          end
+          !appt['appointmentDateTime'].nil? && parsed_date_time.eql?(parsed_appt_time)
         end
       end
-    rescue DateTime::Error => e
+    rescue ArgumentError => e
       Rails.logger.error(message: "#{e} Invalid appointment time provided (given: #{date_string}).")
       raise ArgumentError, "#{e} Invalid appointment time provided (given: #{date_string})."
     end

diff --git a/modules/travel_pay/app/services/travel_pay/claim_association_service.rb b/modules/travel_pay/app/services/travel_pay/claim_association_service.rb
@@ -31,8 +31,8 @@ def initialize(user)
     # appointments: [VAOS::Appointment + travelPayClaim]
 
     def associate_appointments_to_claims(params = {})
-      date_range = try_parse_date_range(params['start_date'], params['end_date'])
-      date_range = date_range.transform_values { |t| strip_timezone(t).iso8601 }
+      date_range = DateUtils.try_parse_date_range(params['start_date'], params['end_date'])
+      date_range = date_range.transform_values { |t| DateUtils.strip_timezone(t).iso8601 }
 
       auth_manager.authorize => { veis_token:, btsss_token: }
       faraday_response = client.get_claims_by_date(veis_token, btsss_token, date_range)
@@ -59,8 +59,8 @@ def associate_single_appointment_to_claim(params = {})
       appt = params['appointment']
       # Because we only receive a single date/time but the external endpoint requires 2 dates
       # in this case both start and end dates are the same
-      date_range = try_parse_date_range(appt[:local_start_time], appt[:local_start_time])
-      date_range = date_range.transform_values { |t| strip_timezone(t).iso8601 }
+      date_range = DateUtils.try_parse_date_range(appt[:local_start_time], appt[:local_start_time])
+      date_range = date_range.transform_values { |t| DateUtils.strip_timezone(t).iso8601 }
 
       auth_manager.authorize => { veis_token:, btsss_token: }
       faraday_response = client.get_claims_by_date(veis_token, btsss_token, date_range)
@@ -127,8 +127,8 @@ def append_claims(appts, claims, metadata)
 
     def find_matching_claim(claims, appt_start)
       claims.find do |cl|
-        claim_time = try_parse_date(cl['appointmentDateTime'])
-        appt_time = strip_timezone(appt_start)
+        claim_time = DateUtils.try_parse_date(cl['appointmentDateTime'])
+        appt_time = DateUtils.strip_timezone(appt_start)
 
         claim_time.eql? appt_time
       end
@@ -151,39 +151,6 @@ def build_metadata(faraday_response_body)
         'message' => faraday_response_body['message'] }
     end
 
-    def strip_timezone(time)
-      # take the time and parse it as a Time object if necessary
-      # convert it to an array of its parts - zone will be nil
-      # create a new time with those parts, using the nil timezone
-
-      t = try_parse_date(time)
-      time_parts = %i[year month day hour min sec]
-      Time.utc(*t.deconstruct_keys(time_parts).values)
-    end
-
-    def try_parse_date(datetime)
-      raise InvalidComparableError.new('Provided datetime is nil.', datetime) if datetime.nil?
-
-      return datetime.to_time if datetime.is_a?(Time) || datetime.is_a?(Date)
-
-      # Disabled Rails/Timezone rule because we don't care about the tz in this dojo.
-      # If we parse it any other 'recommended' way, the time will be converted based
-      # on the timezone, and the datetimes won't match
-      Time.parse(datetime) if datetime.is_a? String # rubocop:disable Rails/TimeZone
-    rescue ArgumentError => e
-      raise ArgumentError,
-            message: "#{e}. Invalid date provided (given: #{datetime})."
-    end
-
-    def try_parse_date_range(start_date, end_date)
-      unless start_date && end_date
-        raise ArgumentError,
-              message: "Both start and end dates are required, got #{start_date}-#{end_date}."
-      end
-
-      { start_date: try_parse_date(start_date), end_date: try_parse_date(end_date) }
-    end
-
     def auth_manager
       @auth_manager ||= TravelPay::AuthManager.new(Settings.travel_pay.client_number, @user)
     end

diff --git a/modules/travel_pay/app/services/travel_pay/date_utils.rb b/modules/travel_pay/app/services/travel_pay/date_utils.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module TravelPay
+  module DateUtils
+    def self.strip_timezone(time)
+      # take the time and parse it as a Time object if necessary
+      # convert it to an array of its parts - zone will be nil
+      # create a new time with those parts, using the nil timezone
+      t = try_parse_date(time)
+      time_parts = %i[year month day hour min sec]
+      Time.utc(*t.deconstruct_keys(time_parts).values)
+    end
+
+    def self.try_parse_date(datetime)
+      raise InvalidComparableError.new('Provided datetime is nil.', datetime) if datetime.nil?
+
+      return datetime.to_time if datetime.is_a?(Time) || datetime.is_a?(Date)
+
+      # Disabled Rails/Timezone rule because we don't care about the tz in this dojo.
+      # If we parse it any other 'recommended' way, the time will be converted based
+      # on the timezone, and the datetimes won't match
+      Time.parse(datetime) if datetime.is_a? String # rubocop:disable Rails/TimeZone
+    end
+
+    def self.try_parse_date_range(start_date, end_date)
+      unless start_date && end_date
+        raise ArgumentError,
+              message: "Both start and end dates are required, got #{start_date}-#{end_date}."
+      end
+
+      { start_date: try_parse_date(start_date), end_date: try_parse_date(end_date) }
+    end
+  end
+end
diff --git a/modules/travel_pay/spec/requests/travel_pay/claims_spec.rb b/modules/travel_pay/spec/requests/travel_pay/claims_spec.rb
@@ -124,7 +124,7 @@
 
       VCR.use_cassette('travel_pay/submit/success', match_requests_on: %i[method path]) do
         headers = { 'Authorization' => 'Bearer vagov_token' }
-        params = { 'appointmentDatetime' => '2024-01-01T16:45:34.465Z' }
+        params = { 'appointment_datetime' => '2024-01-01T16:45:34.465Z' }
 
         post '/travel_pay/v0/claims', headers: headers, params: params
         expect(response).to have_http_status(:created)
@@ -137,13 +137,11 @@
 
       VCR.use_cassette('travel_pay/submit/success', match_requests_on: %i[method path]) do
         headers = { 'Authorization' => 'Bearer vagov_token' }
-        params = { 'appointmentDatetime' => 'My birthday, 4 years ago' }
+        params = { 'appointment_datetime' => 'My birthday, 4 years ago' }
 
         post '/travel_pay/v0/claims', headers: headers, params: params
 
-        error_detail = JSON.parse(response.body)['errors'][0]['detail']
         expect(response).to have_http_status(:bad_request)
-        expect(error_detail).to match(/date/)
       end
     end
 
@@ -153,7 +151,7 @@
 
       VCR.use_cassette('travel_pay/submit/success', match_requests_on: %i[method path]) do
         headers = { 'Authorization' => 'Bearer vagov_token' }
-        params = { 'appointmentDatetime' => '1970-01-01T00:00:00.000Z' }
+        params = { 'appointment_datetime' => '1970-01-01T00:00:00.000Z' }
 
         post '/travel_pay/v0/claims', headers: headers, params: params
 
@@ -172,7 +170,7 @@
       # The cassette doesn't matter here as I'm mocking the submit_claim method
       VCR.use_cassette('travel_pay/submit/success', match_requests_on: %i[method path]) do
         headers = { 'Authorization' => 'Bearer vagov_token' }
-        params = { 'appointmentDatetime' => '2024-01-01T16:45:34.465Z' }
+        params = { 'appointment_datetime' => '2024-01-01T16:45:34.465Z' }
 
         post '/travel_pay/v0/claims', headers: headers, params: params
 

diff --git a/modules/travel_pay/spec/services/claims_association_service_spec.rb b/modules/travel_pay/spec/services/claims_association_service_spec.rb
@@ -209,7 +209,6 @@
       appts.each do |appt|
         expect(appt['travelPayClaim']['metadata']['status']).to equal(400)
         expect(appt['travelPayClaim']['metadata']['success']).to be(false)
-        expect(appt['travelPayClaim']['metadata']['message']).to include(/Invalid date./i)
       end
     end
   end
@@ -379,7 +378,6 @@
                                                                        })
       expect(appt['travelPayClaim']['metadata']['status']).to equal(400)
       expect(appt['travelPayClaim']['metadata']['success']).to be(false)
-      expect(appt['travelPayClaim']['metadata']['message']).to include(/Invalid date./i)
     end
   end
 end
diff --git a/spec/requests/swagger_spec.rb b/spec/requests/swagger_spec.rb
@@ -3899,7 +3899,7 @@
         headers = { '_headers' => { 'Cookie' => sign_in(mhv_user, nil, true) } }
         params = {
           '_data' => {
-            'appointmentDatetime' => '2024-01-01T16:45:34.465Z'
+            'appointment_datetime' => '2024-01-01T16:45:34.465Z'
           }
         }
         VCR.use_cassette('travel_pay/submit/success', match_requests_on: %i[path method]) do