Skip to content

Commit

Permalink
BE | Ask Va Api: Update Inquiries::Creator `Correspondences::Creato…
Browse files Browse the repository at this point in the history
…r` Error Handling (#16529)

* Updated `Creators`

- `Inquiries::Creator` now handle errors correctly
- `Correspondences::Creator` now handle errors correctly

* Updated `CacheData`

- `CacheData` now handle errors correctly

* Updated `Optionset`

- `Optionset` now handle errors correctly

* Updated `Controllers` spec

- updated `Controllers` spec to reflect changes in error handling

---------

Co-authored-by: khoa-v-nguyen <khoa.nguyen@oddball.io>
  • Loading branch information
Khoa-V-Nguyen and khoa-v-nguyen committed Apr 26, 2024
1 parent d4c5192 commit a1d3c91
Show file tree
Hide file tree
Showing 10 changed files with 113 additions and 97 deletions.
Expand Up @@ -34,11 +34,12 @@ def post_data(payload: {})
end

def handle_response_data(response)
if response[:Data].nil?
error = JSON.parse(response[:body], symbolize_names: true)
raise CorrespondencesCreatorError, error[:Message]
else
case response
when Hash
response[:Data]
else
error = JSON.parse(response.body, symbolize_names: true)
raise(CorrespondencesCreatorError, error[:Message])
end
end
end
Expand Down
9 changes: 5 additions & 4 deletions modules/ask_va_api/app/lib/ask_va_api/inquiries/creator.rb
Expand Up @@ -31,11 +31,12 @@ def post_data(payload: {})
end

def handle_response_data(response)
if response[:Data].nil?
error = JSON.parse(response[:body], symbolize_names: true)
raise InquiriesCreatorError, error[:Message]
else
case response
when Hash
response[:Data]
else
error = JSON.parse(response.body, symbolize_names: true)
raise(InquiriesCreatorError, error[:Message])
end
end
end
Expand Down
6 changes: 1 addition & 5 deletions modules/ask_va_api/app/lib/ask_va_api/optionset/retriever.rb
Expand Up @@ -30,11 +30,7 @@ def fetch_data
data = File.read("modules/ask_va_api/config/locales/get_#{name}_mock_data.json")
JSON.parse(data, symbolize_names: true)[:Data]
else
Crm::CacheData.new.call(
endpoint: 'OptionSet',
cache_key: name,
payload: { name: "iris_#{name}" }
)[:Data]
Crm::CacheData.new.call(endpoint: 'optionset', cache_key: name, payload: { name: "iris_#{name}" })[:Data]
end
end
end
Expand Down
27 changes: 18 additions & 9 deletions modules/ask_va_api/app/services/crm/cache_data.rb
Expand Up @@ -12,23 +12,32 @@ def initialize(service: Service.new(icn: nil), cache_client: AskVAApi::RedisClie
def call(endpoint:, cache_key:, payload: {})
data = cache_client.fetch(cache_key)

if data.nil?
fetch_api_data(endpoint:, cache_key:, payload:)
else
data
end
data = fetch_api_data(endpoint:, cache_key:, payload:) if data.nil?

data
rescue => e
ErrorHandler.handle(endpoint, e)
::ErrorHandler.handle_service_error(e)
end

def fetch_api_data(endpoint:, cache_key:, payload: {})
data = service.call(endpoint:, payload:)
response = service.call(endpoint:, payload:)
data = handle_response_data(response)

cache_client.store_data(key: cache_key, data:, ttl: 86_400)

data
rescue => e
ErrorHandler.handle(endpoint, e)
end

private

def handle_response_data(response)
case response
when Hash
response
else
error = JSON.parse(response.body, symbolize_names: true)
raise(StandardError, error[:Message])
end
end
end
end
Expand Up @@ -27,14 +27,7 @@ module Correspondences
',"ExceptionOccurred":true,"ExceptionMessage":"Data Validation: ' \
'Missing Reply","MessageId":"e2cbe041-df91-41f4-8bd2-8b6d9dbb2e38"}'
end
let(:failure) do
{
status: 400,
body:,
response_headers: nil,
url: nil
}
end
let(:failure) { Faraday::Response.new(response_body: body, status: 400) }

before do
allow_any_instance_of(Crm::CrmToken).to receive(:call).and_return('Token')
Expand Down
Expand Up @@ -41,14 +41,7 @@
',"ExceptionOccurred":true,"ExceptionMessage":"Data Validation: missing' \
'InquiryCategory","MessageId":"cb0dd954-ef25-4e56-b0d9-41925e5a190c"}'
end
let(:failure) do
{
status: 400,
body:,
response_headers: nil,
url: nil
}
end
let(:failure) { Faraday::Response.new(response_body: body, status: 400) }

before do
allow(service).to receive(:call).and_return(failure)
Expand Down
Expand Up @@ -32,6 +32,32 @@ module Optionset
expect(retriever.call).to all(be_a(entity_class))
end
end

context 'when an error occur' do
let(:retriever) { described_class.new(name: 'branchofservic', user_mock_data: false, entity_class:) }
let(:body) do
'{"Data":null,"Message":"Data Validation: Invalid OptionSet Name iris_branchofservic, valid' \
' values are iris_inquiryabout, iris_inquirysource, iris_inquirytype, iris_levelofauthentication,' \
' iris_suffix, iris_veteranrelationship, iris_branchofservice, iris_country, iris_province,' \
' iris_responsetype, iris_dependentrelationship, statuscode, iris_messagetype","ExceptionOccurred":' \
'true,"ExceptionMessage":"Data Validation: Invalid OptionSet Name iris_branchofservic, valid' \
' values are iris_inquiryabout, iris_inquirysource, iris_inquirytype, iris_levelofauthentication,' \
' iris_suffix, iris_veteranrelationship, iris_branchofservice, iris_country, iris_province,' \
' iris_responsetype, iris_dependentrelationship, statuscode, iris_messagetype","MessageId":' \
'"6dfa81bd-f04a-4f39-88c5-1422d88ed3ff"}'
end
let(:failure) { Faraday::Response.new(response_body: body, status: 400) }

before do
allow_any_instance_of(Crm::CrmToken).to receive(:call).and_return('token')
allow_any_instance_of(Crm::Service).to receive(:call)
.with(endpoint: 'optionset', payload: { name: 'iris_branchofservic' }).and_return(failure)
end

it 'raise the error' do
expect { retriever.call }.to raise_error(ErrorHandler::ServiceError)
end
end
end
end
end
Expand Down
27 changes: 3 additions & 24 deletions modules/ask_va_api/spec/requests/v0/inquiries_spec.rb
Expand Up @@ -353,14 +353,7 @@
',"ExceptionOccurred":true,"ExceptionMessage":"Data Validation: missing' \
'InquiryCategory","MessageId":"cb0dd954-ef25-4e56-b0d9-41925e5a190c"}'
end
let(:failure) do
{
status: 400,
body:,
response_headers: nil,
url: nil
}
end
let(:failure) { Faraday::Response.new(response_body: body, status: 400) }

before do
allow_any_instance_of(Crm::Service).to receive(:call)
Expand Down Expand Up @@ -410,14 +403,7 @@
',"ExceptionOccurred":true,"ExceptionMessage":"Data Validation: missing' \
'InquiryCategory","MessageId":"cb0dd954-ef25-4e56-b0d9-41925e5a190c"}'
end
let(:failure) do
{
status: 400,
body:,
response_headers: nil,
url: nil
}
end
let(:failure) { Faraday::Response.new(response_body: body, status: 400) }

before do
allow_any_instance_of(Crm::Service).to receive(:call)
Expand Down Expand Up @@ -517,14 +503,7 @@ def json_response
',"ExceptionOccurred":true,"ExceptionMessage":"Data Validation: ' \
'Missing Reply","MessageId":"e2cbe041-df91-41f4-8bd2-8b6d9dbb2e38"}'
end
let(:failure) do
{
status: 400,
body:,
response_headers: nil,
url: nil
}
end
let(:failure) { Faraday::Response.new(response_body: body, status: 400) }

before do
sign_in(authorized_user)
Expand Down
29 changes: 22 additions & 7 deletions modules/ask_va_api/spec/requests/v0/static_data_spec.rb
Expand Up @@ -249,17 +249,32 @@
end

context 'when an error occurs' do
let(:error_message) { 'service error' }
let(:body) do
'{"Data":null,"Message":"Data Validation: Invalid OptionSet Name iris_branchofservic, valid' \
' values are iris_inquiryabout, iris_inquirysource, iris_inquirytype, iris_levelofauthentication,' \
' iris_suffix, iris_veteranrelationship, iris_branchofservice, iris_country, iris_province,' \
' iris_responsetype, iris_dependentrelationship, statuscode, iris_messagetype","ExceptionOccurred":' \
'true,"ExceptionMessage":"Data Validation: Invalid OptionSet Name iris_branchofservic, valid' \
' values are iris_inquiryabout, iris_inquirysource, iris_inquirytype, iris_levelofauthentication,' \
' iris_suffix, iris_veteranrelationship, iris_branchofservice, iris_country, iris_province,' \
' iris_responsetype, iris_dependentrelationship, statuscode, iris_messagetype","MessageId":' \
'"6dfa81bd-f04a-4f39-88c5-1422d88ed3ff"}'
end
let(:failure) { Faraday::Response.new(response_body: body, status: 400) }

before do
allow_any_instance_of(AskVAApi::Optionset::Retriever)
.to receive(:call)
.and_raise(StandardError, 'standard error')
get optionset_path, params: { user_mock_data: true, mame: 'branchofservice' }
allow_any_instance_of(Crm::Service).to receive(:call)
.with(endpoint: 'optionset', payload: { name: 'iris_branchofservic' }).and_return(failure)
get optionset_path, params: { user_mock_data: nil, name: 'branchofservic' }
end

it_behaves_like 'common error handling', :internal_server_error, 'unexpected_error',
'standard error'
it_behaves_like 'common error handling', :unprocessable_entity, 'service_error',
'ErrorHandler::ServiceError: StandardError: Data Validation: ' \
'Invalid OptionSet Name iris_branchofservic, valid values are ' \
'iris_inquiryabout, iris_inquirysource, iris_inquirytype, ' \
'iris_levelofauthentication, iris_suffix, iris_veteranrelationship, ' \
'iris_branchofservice, iris_country, iris_province, iris_responsetype,' \
' iris_dependentrelationship, statuscode, iris_messagetype'
end
end

Expand Down
59 changes: 31 additions & 28 deletions modules/ask_va_api/spec/services/crm/cache_data_spec.rb
Expand Up @@ -3,13 +3,13 @@
require 'rails_helper'

RSpec.describe Crm::CacheData do
let(:service) { double('Crm::Service') }
let(:cache_client) { double('AskVAApi::RedisClient') }
let(:service) { Crm::Service.new(icn: nil) }
let(:cache_client) { AskVAApi::RedisClient.new }
let(:cache_data_instance) { Crm::CacheData.new(service:, cache_client:) }
let(:cache_data) { { topics: [{ id: 1, name: 'Topic 1' }] } }

def mock_response(status:, body:)
instance_double(Faraday::Response, status:, body: body.to_json)
before do
allow_any_instance_of(Crm::CrmToken).to receive(:call).and_return('token')
end

describe '#call' do
Expand All @@ -20,44 +20,47 @@ def mock_response(status:, body:)
expect(cache_data_instance.call(endpoint: 'topics', cache_key: 'categories_topics_subtopics')).to eq(cache_data)
end
end
end

describe '#fetch_api_data' do
context 'when the cache is empty' do
it 'fetches data from the service and stores it in the cache' do
expect(service).to receive(:call).with(endpoint: 'topics', payload: {}).and_return(cache_data)
expect(cache_client).to receive(:store_data).with(key: 'categories_topics_subtopics', data: cache_data,
expect(cache_client).to receive(:store_data).with(key: 'categories_topics_subtopics',
data: cache_data,
ttl: 86_400)
expect(cache_data_instance.fetch_api_data(endpoint: 'topics',
cache_key: 'categories_topics_subtopics')).to eq(cache_data)
expect(cache_data_instance.call(endpoint: 'topics',
cache_key: 'categories_topics_subtopics')).to eq(cache_data)
end
end

context 'when an error occurs' do
let(:resp) { mock_response(body: { error: 'invalid_client' }, status: 401) }
let(:exception) { Common::Exceptions::BackendServiceException.new(nil, {}, resp.status, resp.body) }
let(:body) do
'{"Data":null,"Message":"Data Validation: Invalid OptionSet Name iris_branchofservic, valid' \
' values are iris_inquiryabout, iris_inquirysource, iris_inquirytype, iris_levelofauthentication,' \
' iris_suffix, iris_veteranrelationship, iris_branchofservice, iris_country, iris_province,' \
' iris_responsetype, iris_dependentrelationship, statuscode, iris_messagetype","ExceptionOccurred":' \
'true,"ExceptionMessage":"Data Validation: Invalid OptionSet Name iris_branchofservic, valid' \
' values are iris_inquiryabout, iris_inquirysource, iris_inquirytype, iris_levelofauthentication,' \
' iris_suffix, iris_veteranrelationship, iris_branchofservice, iris_country, iris_province,' \
' iris_responsetype, iris_dependentrelationship, statuscode, iris_messagetype","MessageId":' \
'"6dfa81bd-f04a-4f39-88c5-1422d88ed3ff"}'
end
let(:failure) { Faraday::Response.new(response_body: body, status: 400) }
let(:response) do
cache_data_instance.call(
endpoint: 'optionset',
cache_key: 'branchofservic',
payload: { name: 'iris_branchofservic' }
)
end

before do
allow_any_instance_of(Faraday::Connection).to receive(:get).with(anything).and_raise(exception)
allow_any_instance_of(Crm::Service).to receive(:call)
.with(endpoint: 'optionset', payload: { name: 'iris_branchofservic' })
.and_return(failure)
end

it 'handles the service error through the ErrorHandler' do
expect(service).to receive(:call).with(endpoint: 'topics', payload: {}).and_raise('Service error')
expect(Crm::ErrorHandler).to receive(:handle).with('topics', instance_of(RuntimeError))
expect do
cache_data_instance.fetch_api_data(endpoint: 'topics', cache_key: 'categories_topics_subtopics')
end.not_to raise_error
end
end

# Edge case where cache returns empty array which should be treated as no data
context 'when the cache returns an empty array' do
it 'fetches data from the service as if the cache was empty' do
expect(service).to receive(:call).with(endpoint: 'topics', payload: {}).and_return(cache_data)
expect(cache_client).to receive(:store_data).with(key: 'categories_topics_subtopics', data: cache_data,
ttl: 86_400)
expect(cache_data_instance.fetch_api_data(endpoint: 'topics',
cache_key: 'categories_topics_subtopics')).to eq(cache_data)
expect { response }.to raise_error(ErrorHandler::ServiceError)
end
end
end
Expand Down

0 comments on commit a1d3c91

Please sign in to comment.