-
Notifications
You must be signed in to change notification settings - Fork 66
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
96247 add organization address validation to trexler file process job #20085
Open
opticbob
wants to merge
28
commits into
master
Choose a base branch
from
96247-add-organization-address-validation-to-trexler-file-process-job
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,561
−0
Open
Changes from 23 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
dfa372d
[WIP]
opticbob 5e28438
[WIP] comments to move forward
opticbob f2ffd38
Restore original job config
opticbob 08a863b
[WIP] update for orgs
opticbob e845234
[WIP] Continue removing non-org code
opticbob 42157a4
Queue updates for orgs
opticbob f512d14
[WIP] VSOs processing
opticbob d6956b5
[WIP] Allow processing of test file
opticbob ddc5fd6
[WIP] Add diff checking
opticbob b7d57b4
Org not rep
opticbob dc036da
Queue updates spec
opticbob 97c7caf
Test fixes
opticbob d21b612
[WIP] Update spec progress
opticbob d078048
[WIP] file processor spec
opticbob bd584dd
Test fixes
opticbob 85a771d
Removing unused code
opticbob cce3d14
Fix remaining tests
opticbob e2595b3
Cleanup
opticbob a9212a3
Merge branch 'master' into 96247-add-organization-address-validation-…
opticbob aab565b
Merge branch 'master' into 96247-add-organization-address-validation-…
opticbob ac02173
Merge branch 'master' into 96247-add-organization-address-validation-…
opticbob 2e30c32
Merge branch 'master' into 96247-add-organization-address-validation-…
opticbob e8071e4
Simplify organization diff
opticbob e8ba6d3
poa -> id
opticbob bf531fd
More poa -> id and string conversion
opticbob a96cc1c
Queue update test fix
opticbob 76f07fd
Merge branch 'master' into 96247-add-organization-address-validation-…
opticbob c46c027
States hash -> array
opticbob File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
67 changes: 67 additions & 0 deletions
67
modules/veteran/app/sidekiq/organizations/queue_updates.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'sidekiq' | ||
require 'sentry_logging' | ||
|
||
module Organizations | ||
class QueueUpdates | ||
include Sidekiq::Job | ||
include SentryLogging | ||
|
||
SLICE_SIZE = 30 | ||
|
||
def perform | ||
file_content = fetch_file_content | ||
return unless file_content | ||
|
||
processed_data = Organizations::XlsxFileProcessor.new(file_content).process | ||
queue_address_updates(processed_data) | ||
rescue => e | ||
log_error("Error in file fetching process: #{e.message}") | ||
end | ||
|
||
private | ||
|
||
def fetch_file_content | ||
Representatives::XlsxFileFetcher.new.fetch | ||
end | ||
|
||
def queue_address_updates(data) | ||
delay = 0 | ||
|
||
Organizations::XlsxFileProcessor::SHEETS_TO_PROCESS.each do |sheet| | ||
next if data[sheet].blank? | ||
|
||
batch = Sidekiq::Batch.new | ||
batch.description = "Batching #{sheet} sheet data" | ||
|
||
begin | ||
batch.jobs do | ||
rows_to_process(data[sheet]).each_slice(SLICE_SIZE) do |rows| | ||
json_rows = rows.to_json | ||
Organizations::Update.perform_in(delay.minutes, json_rows) | ||
delay += 1 | ||
end | ||
end | ||
rescue => e | ||
log_error("Error queuing address updates: #{e.message}") | ||
end | ||
end | ||
end | ||
|
||
def rows_to_process(rows) | ||
rows.map do |row| | ||
org = Veteran::Service::Organization.find_by(poa: row[:poa]) | ||
diff = org.diff(row) | ||
row.merge(diff.merge({ address_exists: org.location.present? })) if diff.values.any? | ||
rescue ActiveRecord::RecordNotFound => e | ||
log_error("Error: Organization not found #{e.message}") | ||
nil | ||
end.compact | ||
end | ||
|
||
def log_error(message) | ||
log_message_to_sentry("QueueUpdates error: #{message}", :error) | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,283 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'sidekiq' | ||
require 'sentry_logging' | ||
require 'va_profile/models/validation_address' | ||
require 'va_profile/address_validation/service' | ||
require 'va_profile/models/v3/validation_address' | ||
require 'va_profile/v3/address_validation/service' | ||
|
||
module Organizations | ||
# Processes updates for organization records based on provided JSON data. | ||
# This class is designed to parse organization data, validate addresses using an external service, | ||
# and update records in the database accordingly. | ||
class Update | ||
include Sidekiq::Job | ||
include SentryLogging | ||
|
||
# Processes each organization's data provided in JSON format. | ||
# This method parses the JSON, validates each organization's address, and updates the database records. | ||
# @param orgs_json [String] JSON string containing an array of organization data. | ||
def perform(orgs_json) | ||
orgs_data = JSON.parse(orgs_json) | ||
orgs_data.each { |org_data| process_org_data(org_data) } | ||
rescue => e | ||
log_error("Error processing job: #{e.message}") | ||
end | ||
|
||
private | ||
|
||
# Processes individual organization data, validates the address, and updates the record. | ||
# If the address validation fails or an error occurs during the update, the error is logged and the process | ||
# is halted for the current organization. | ||
# @param org_data [Hash] The organization data including id and address. | ||
def process_org_data(org_data) # rubocop:disable Metrics/MethodLength | ||
return unless record_can_be_updated?(org_data) | ||
|
||
address_validation_api_response = nil | ||
|
||
if org_data['address_changed'] | ||
|
||
api_response = if Flipper.enabled?(:va_v3_contact_information_service) | ||
get_best_address_candidate(org_data) | ||
else | ||
get_best_address_candidate(org_data['address']) | ||
end | ||
|
||
# don't update the record if there is not a valid address with non-zero lat and long at this point | ||
if api_response.nil? | ||
return | ||
else | ||
address_validation_api_response = api_response | ||
end | ||
end | ||
|
||
begin | ||
update_org_record(org_data, address_validation_api_response) | ||
rescue Common::Exceptions::BackendServiceException => e | ||
log_error("Address validation failed for Org id: #{org_data['id']}: #{e.message}") | ||
nil | ||
rescue => e | ||
log_error("Update failed for Org id: #{org_data['id']}: #{e.message}") | ||
nil | ||
end | ||
end | ||
|
||
def record_can_be_updated?(org_data) | ||
org_data['address_exists'] || org_data['address_changed'] | ||
end | ||
|
||
# Constructs a validation address object from the provided address data. | ||
# @param address [Hash] A hash containing the details of the organization's address. | ||
# @return [VAProfile::Models::ValidationAddress] A validation address object ready for address validation service. | ||
def build_validation_address(address) | ||
if Flipper.enabled?(:va_v3_contact_information_service) | ||
validation_model = VAProfile::Models::V3::ValidationAddress | ||
state_code = address['state']['state_code'] | ||
city = address['city_name'] | ||
else | ||
validation_model = VAProfile::Models::ValidationAddress | ||
state_code = address['state_province']['code'] | ||
city = address['city'] | ||
end | ||
|
||
validation_model.new( | ||
address_pou: address['address_pou'], | ||
address_line1: address['address_line1'], | ||
address_line2: address['address_line2'], | ||
address_line3: address['address_line3'], | ||
city: city, | ||
state_code: state_code, | ||
zip_code: address['zip_code5'], | ||
zip_code_suffix: address['zip_code4'], | ||
country_code_iso3: address['country_code_iso3'] | ||
) | ||
end | ||
|
||
# Validates the given address using the VAProfile address validation service. | ||
# @param candidate_address [VAProfile::Models::ValidationAddress] The address to be validated. | ||
# @return [Hash] The response from the address validation service. | ||
def validate_address(candidate_address) | ||
validation_service = if Flipper.enabled?(:va_v3_contact_information_service) | ||
VAProfile::V3::AddressValidation::Service.new | ||
else | ||
VAProfile::AddressValidation::Service.new | ||
end | ||
validation_service.candidate(candidate_address) | ||
end | ||
|
||
# Checks if the address validation response is valid. | ||
# @param response [Hash] The response from the address validation service. | ||
# @return [Boolean] True if the address is valid, false otherwise. | ||
def address_valid?(response) | ||
response.key?('candidate_addresses') && !response['candidate_addresses'].empty? | ||
end | ||
|
||
# Updates the address record based on the org_data and validation response. | ||
# If the record cannot be found, logs an error to Sentry. | ||
# @param org_data [Hash] Original org_data containing the address and other details. | ||
# @param api_response [Hash] The response from the address validation service. | ||
def update_org_record(org_data, api_response) | ||
record = | ||
Veteran::Service::Organization.find_by(poa: org_data['id']) | ||
if record.nil? | ||
raise StandardError, 'Organization not found.' | ||
else | ||
address_attributes = org_data['address_changed'] ? build_address_attributes(org_data, api_response) : {} | ||
record.update(address_attributes) | ||
end | ||
end | ||
|
||
# Updates the given record with the new address and other relevant attributes. | ||
# @param org_data [Hash] Original org_data containing the address and other details. | ||
# @param api_response [Hash] The response from the address validation service. | ||
def build_address_attributes(org_data, api_response) | ||
if Flipper.enabled?(:va_v3_contact_information_service) | ||
build_v3_address(api_response['candidate_addresses'].first) | ||
else | ||
address = api_response['candidate_addresses'].first['address'] | ||
geocode = api_response['candidate_addresses'].first['geocode'] | ||
meta = api_response['candidate_addresses'].first['address_meta_data'] | ||
build_address(address, geocode, meta).merge({ raw_address: org_data['address'].to_json }) | ||
end | ||
end | ||
|
||
# Builds the attributes for the record update from the address, geocode, and metadata. | ||
# @param address [Hash] Address details from the validation response. | ||
# @param geocode [Hash] Geocode details from the validation response. | ||
# @param meta [Hash] Metadata about the address from the validation response. | ||
# @return [Hash] The attributes to update the record with. | ||
def build_address(address, geocode, meta) | ||
{ | ||
address_type: meta['address_type'], | ||
address_line1: address['address_line1'], | ||
address_line2: address['address_line2'], | ||
address_line3: address['address_line3'], | ||
city: address['city'], | ||
province: address['state_province']['name'], | ||
state_code: address['state_province']['code'], | ||
zip_code: address['zip_code5'], | ||
zip_suffix: address['zip_code4'], | ||
country_code_iso3: address['country']['iso3_code'], | ||
country_name: address['country']['name'], | ||
county_name: address.dig('county', 'name'), | ||
county_code: address.dig('county', 'county_fips_code'), | ||
lat: geocode['latitude'], | ||
long: geocode['longitude'], | ||
location: "POINT(#{geocode['longitude']} #{geocode['latitude']})" | ||
} | ||
end | ||
|
||
def build_v3_address(address) | ||
{ | ||
address_type: address['address_type'], | ||
address_line1: address['address_line1'], | ||
address_line2: address['address_line2'], | ||
address_line3: address['address_line3'], | ||
city: address['city_name'], | ||
province: address['state']['state_name'], | ||
state_code: address['state']['state_code'], | ||
zip_code: address['zip_code5'], | ||
zip_suffix: address['zip_code4'], | ||
country_code_iso3: address['country']['iso3_code'], | ||
country_name: address['country']['country_name'], | ||
county_name: address.dig('county', 'county_name'), | ||
county_code: address.dig('county', 'county_code'), | ||
lat: address['geocode']['latitude'], | ||
long: address['geocode']['longitude'], | ||
location: "POINT(#{address['geocode']['longitude']} #{address['geocode']['latitude']})" | ||
} | ||
end | ||
|
||
# Logs an error to Sentry. | ||
# @param error [Exception] The error string to be logged. | ||
def log_error(error) | ||
log_message_to_sentry("Organizations::Update: #{error}", :error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here |
||
end | ||
|
||
# Checks if the latitude and longitude of an address are both set to zero, which are the default values | ||
# for DualAddressError warnings we see with some P.O. Box addresses the validator struggles with | ||
# @param candidate_address [Hash] an address hash object returned by [VAProfile::AddressValidation::Service] | ||
# @return [Boolean] | ||
def lat_long_zero?(candidate_address) | ||
address = candidate_address['candidate_addresses']&.first | ||
return false if address.blank? | ||
|
||
geocode = address['geocode'] | ||
return false if geocode.blank? | ||
|
||
geocode['latitude']&.zero? && geocode['longitude']&.zero? | ||
end | ||
|
||
# Attempt to get valid address with non-zero coordinates by modifying the OGC address data | ||
# @param address [Hash] the OGC address object | ||
# @param retry_count [Integer] the current retry attempt which determines how the address object should be modified | ||
# @return [Hash] the response from the address validation service | ||
def modified_validation(address, retry_count) | ||
address_attempt = address.dup | ||
case retry_count | ||
when 1 # only use the original address_line1 | ||
when 2 # set address_line1 to the original address_line2 | ||
address_attempt['address_line1'] = address['address_line2'] | ||
else # set address_line1 to the original address_line3 | ||
address_attempt['address_line1'] = address['address_line3'] | ||
end | ||
|
||
address_attempt['address_line2'] = nil | ||
address_attempt['address_line3'] = nil | ||
|
||
validate_address(build_validation_address(address_attempt)) | ||
end | ||
|
||
# An address validation attempt is retriable if the address is invalid OR the coordinates are zero | ||
# @param response [Hash, Nil] the response from the address validation service | ||
# @return [Boolean] | ||
def retriable?(response) | ||
return true if response.blank? | ||
|
||
!address_valid?(response) || lat_long_zero?(response) | ||
end | ||
|
||
# Retry address validation | ||
# @param org_address [Hash] the address provided by OGC | ||
# @return [Hash, Nil] the response from the address validation service | ||
def retry_validation(org_address) | ||
# the address validation service requires at least one of address_line1, address_line2, and address_line3 to | ||
# exist. No need to run the retry if we know it will fail before attempting the api call. | ||
api_response = modified_validation(org_address, 1) if org_address['address_line1'].present? | ||
|
||
if retriable?(api_response) && org_address['address_line2'].present? | ||
api_response = modified_validation(org_address, 2) | ||
end | ||
|
||
if retriable?(api_response) && org_address['address_line3'].present? | ||
api_response = modified_validation(org_address, 3) | ||
end | ||
|
||
api_response | ||
end | ||
|
||
# Get the best address that the address validation api can provide with some retry logic added in | ||
# @param org_address [Hash] the address provided by OGC | ||
# @return [Hash, Nil] the response from the address validation service | ||
def get_best_address_candidate(org_address) | ||
candidate_address = build_validation_address(org_address) | ||
original_response = validate_address(candidate_address) | ||
return nil unless address_valid?(original_response) | ||
|
||
# retry validation if we get zero as the coordinates - this should indicate some warning with validation that | ||
# is typically seen with addresses that mix street addresses with P.O. Boxes | ||
if lat_long_zero?(original_response) | ||
retry_response = retry_validation(org_address) | ||
|
||
if retriable?(retry_response) | ||
nil | ||
else | ||
retry_response | ||
end | ||
else | ||
original_response | ||
end | ||
end | ||
end | ||
end |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to log this to datadog instead? Then you could put a monitor in place to look for this log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LindseySaari Do you have a place in vets-api where you could point me to a good example of logging to datadog? I see some code referencing statsd and some for
Datadog::Tracing
, but I'm not sure what you're looking for.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LindseySaari would it work to investigate datadog integration in a future PR? We are waiting for this to merge before we expand the percentage of users exposed to our product launch.