diff --git a/modules/ask_va_api/app/lib/ask_va_api/correspondences/creator.rb b/modules/ask_va_api/app/lib/ask_va_api/correspondences/creator.rb index 3e57c1fd81a..4ba228f501b 100644 --- a/modules/ask_va_api/app/lib/ask_va_api/correspondences/creator.rb +++ b/modules/ask_va_api/app/lib/ask_va_api/correspondences/creator.rb @@ -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 diff --git a/modules/ask_va_api/app/lib/ask_va_api/inquiries/creator.rb b/modules/ask_va_api/app/lib/ask_va_api/inquiries/creator.rb index 918601ea690..5bf16d53698 100644 --- a/modules/ask_va_api/app/lib/ask_va_api/inquiries/creator.rb +++ b/modules/ask_va_api/app/lib/ask_va_api/inquiries/creator.rb @@ -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 diff --git a/modules/ask_va_api/app/lib/ask_va_api/optionset/retriever.rb b/modules/ask_va_api/app/lib/ask_va_api/optionset/retriever.rb index a3497d97ba7..497171f56fa 100644 --- a/modules/ask_va_api/app/lib/ask_va_api/optionset/retriever.rb +++ b/modules/ask_va_api/app/lib/ask_va_api/optionset/retriever.rb @@ -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 diff --git a/modules/ask_va_api/app/services/crm/cache_data.rb b/modules/ask_va_api/app/services/crm/cache_data.rb index 9b53c16c82e..482137988fd 100644 --- a/modules/ask_va_api/app/services/crm/cache_data.rb +++ b/modules/ask_va_api/app/services/crm/cache_data.rb @@ -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 diff --git a/modules/ask_va_api/spec/app/lib/ask_va_api/correspondences/creator_spec.rb b/modules/ask_va_api/spec/app/lib/ask_va_api/correspondences/creator_spec.rb index bea0c25bd50..fb554d9f43e 100644 --- a/modules/ask_va_api/spec/app/lib/ask_va_api/correspondences/creator_spec.rb +++ b/modules/ask_va_api/spec/app/lib/ask_va_api/correspondences/creator_spec.rb @@ -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') diff --git a/modules/ask_va_api/spec/app/lib/ask_va_api/inquiries/creator_spec.rb b/modules/ask_va_api/spec/app/lib/ask_va_api/inquiries/creator_spec.rb index 793ebba7057..ae862588e41 100644 --- a/modules/ask_va_api/spec/app/lib/ask_va_api/inquiries/creator_spec.rb +++ b/modules/ask_va_api/spec/app/lib/ask_va_api/inquiries/creator_spec.rb @@ -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) diff --git a/modules/ask_va_api/spec/app/lib/ask_va_api/optionset/retriever_spec.rb b/modules/ask_va_api/spec/app/lib/ask_va_api/optionset/retriever_spec.rb index 397bc122d97..dad080a76b4 100644 --- a/modules/ask_va_api/spec/app/lib/ask_va_api/optionset/retriever_spec.rb +++ b/modules/ask_va_api/spec/app/lib/ask_va_api/optionset/retriever_spec.rb @@ -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 diff --git a/modules/ask_va_api/spec/requests/v0/inquiries_spec.rb b/modules/ask_va_api/spec/requests/v0/inquiries_spec.rb index b9acb4575f8..b94d93d6044 100644 --- a/modules/ask_va_api/spec/requests/v0/inquiries_spec.rb +++ b/modules/ask_va_api/spec/requests/v0/inquiries_spec.rb @@ -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) @@ -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) @@ -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) diff --git a/modules/ask_va_api/spec/requests/v0/static_data_spec.rb b/modules/ask_va_api/spec/requests/v0/static_data_spec.rb index d9dddf16948..19647366682 100644 --- a/modules/ask_va_api/spec/requests/v0/static_data_spec.rb +++ b/modules/ask_va_api/spec/requests/v0/static_data_spec.rb @@ -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 diff --git a/modules/ask_va_api/spec/services/crm/cache_data_spec.rb b/modules/ask_va_api/spec/services/crm/cache_data_spec.rb index 1e341f6469d..323b7f8167b 100644 --- a/modules/ask_va_api/spec/services/crm/cache_data_spec.rb +++ b/modules/ask_va_api/spec/services/crm/cache_data_spec.rb @@ -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 @@ -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