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

Strip timezone from POST params #20790

Merged
merged 14 commits into from
Feb 19, 2025
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 })

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
34 changes: 34 additions & 0 deletions modules/travel_pay/app/services/travel_pay/date_utils.rb
Original file line number Diff line number Diff line change
@@ -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
10 changes: 4 additions & 6 deletions modules/travel_pay/spec/requests/travel_pay/claims_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Loading