Fix Vine::VoucherValidatorService to handle exceptions

This commit is contained in:
Gaetan Craig-Riou
2024-11-18 11:25:39 +11:00
committed by Rachel Arnould
parent 1b50217242
commit d102652c03
4 changed files with 28 additions and 52 deletions

View File

@@ -91,9 +91,9 @@ class VoucherAdjustmentsController < BaseController
)
voucher = vine_voucher_validator.validate
if vine_voucher_validator.errors.present?
return nil if ignored_vine_api_error(vine_voucher_validator)
return nil if vine_voucher_validator.errors[:not_found_voucher].present?
if vine_voucher_validator.errors.present?
@order.errors.add(:voucher_code, I18n.t('checkout.errors.add_voucher_error'))
return nil
end
@@ -101,11 +101,6 @@ class VoucherAdjustmentsController < BaseController
voucher
end
def ignored_vine_api_error(validator)
# Ignore distributor not set up for VINE or not found error
validator.errors[:vine_settings].present? || validator.errors[:not_found_voucher].present?
end
def update_payment_section
render cable_ready: cable_car.replace(
selector: "#checkout-payment-methods",

View File

@@ -11,19 +11,14 @@ module Vine
end
def validate
if vine_settings.nil?
errors[:vine_settings] = I18n.t("vine_voucher_validator_service.errors.vine_settings")
return nil
end
return nil if vine_settings.nil?
response = call_vine_api
if !response.success?
handle_errors(response)
return nil
end
save_voucher(response)
rescue Faraday::ClientError => e
handle_errors(e.response)
nil
rescue Faraday::Error => e
Rails.logger.error e.inspect
Bugsnag.notify(e)
@@ -47,9 +42,9 @@ module Vine
end
def handle_errors(response)
if response.status == 400
if response[:status] == 400
errors[:invalid_voucher] = I18n.t("vine_voucher_validator_service.errors.invalid_voucher")
elsif response.status == 404
elsif response[:status] == 404
errors[:not_found_voucher] =
I18n.t("vine_voucher_validator_service.errors.not_found_voucher")
else

View File

@@ -118,10 +118,7 @@ RSpec.describe VoucherAdjustmentsController, type: :request do
context "when coordinator is not connected to VINE" do
it "returns 422 and an error message" do
mock_vine_voucher_validator(
voucher: nil,
errors: { vine_settings: "No Vine api settings for the given enterprise" }
)
mock_vine_voucher_validator(voucher: nil)
post("/voucher_adjustments", params:)

View File

@@ -42,15 +42,15 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
let(:vine_voucher_set_id) { "9d24349c-1fe8-4090-988b-d7355ed32559" }
it "verifies the voucher with VINE API" do
expect(vine_api_service).to receive(:voucher_validation)
.and_return(mock_api_response(success: true, data:))
expect(vine_api_service).to receive(:voucher_validation).and_return(
mock_api_response(data:)
)
validate_voucher_service.validate
end
it "creates a new VINE voucher" do
allow(vine_api_service).to receive(:voucher_validation)
.and_return(mock_api_response(success: true, data:))
allow(vine_api_service).to receive(:voucher_validation).and_return(mock_api_response(data:))
vine_voucher = validate_voucher_service.validate
@@ -88,7 +88,7 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
external_voucher_id: vine_voucher_id,
external_voucher_set_id: vine_voucher_set_id)
allow(vine_api_service).to receive(:voucher_validation)
.and_return(mock_api_response(success: true, data:))
.and_return(mock_api_response(data:))
vine_voucher = validate_voucher_service.validate
@@ -129,7 +129,7 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
external_voucher_id: vine_voucher_id,
external_voucher_set_id: vine_voucher_set_id)
allow(vine_api_service).to receive(:voucher_validation)
.and_return(mock_api_response(success: true, data:))
.and_return(mock_api_response(data:))
vine_voucher = validate_voucher_service.validate
@@ -155,14 +155,6 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
validate_voucher_service.validate
end
it "adds an error message" do
validate_voucher_service.validate
expect(validate_voucher_service.errors).to include(
{ vine_settings: "This shop is not enabled for VINE Vouchers." }
)
end
it "doesn't creates a new VINE voucher" do
expect_voucher_count_not_to_change
end
@@ -176,7 +168,7 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
}
before do
allow(vine_api_service).to receive(:voucher_validation).and_raise(Faraday::ConnectionFailed)
mock_api_exception(type: Faraday::ConnectionFailed)
end
it "returns nil" do
@@ -218,9 +210,7 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
}
before do
allow(vine_api_service).to receive(:voucher_validation).and_return(
mock_api_response(success: false, status: 401, data: )
)
mock_api_exception(type: Faraday::UnauthorizedError, status: 401, body: data)
end
it "returns nil" do
@@ -254,9 +244,7 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
}
before do
allow(vine_api_service).to receive(:voucher_validation).and_return(
mock_api_response(success: false, status: 404, data: )
)
mock_api_exception(type: Faraday::ResourceNotFound, status: 404, body: data)
end
it "returns nil" do
@@ -290,9 +278,7 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
}
before do
allow(vine_api_service).to receive(:voucher_validation).and_return(
mock_api_response(success: false, status: 400, data: )
)
mock_api_exception(type: Faraday::BadRequestError, status: 400, body: data)
end
it "returns nil" do
@@ -339,7 +325,7 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
before do
allow(vine_api_service).to receive(:voucher_validation).and_return(
mock_api_response(success: true, status: 200, data: )
mock_api_response(data: )
)
end
@@ -384,13 +370,13 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
before do
allow(vine_api_service).to receive(:voucher_validation).and_return(
mock_api_response(success: true, status: 200, data: )
mock_api_response(data: )
)
end
it "verify the voucher with VINE API" do
expect(vine_api_service).to receive(:voucher_validation).and_return(
mock_api_response(success: true, status: 200, data: )
mock_api_response(data: )
)
validate_voucher_service.validate
@@ -445,13 +431,16 @@ RSpec.describe Vine::VoucherValidatorService, feature: :connected_apps do
expect { validate_voucher_service.validate }.not_to change { Voucher.count }
end
def mock_api_response(success:, data: nil, status: 200)
def mock_api_response(data: nil)
mock_response = instance_double(Faraday::Response)
allow(mock_response).to receive(:success?).and_return(success)
allow(mock_response).to receive(:status).and_return(status)
if data.present?
allow(mock_response).to receive(:body).and_return(data)
end
mock_response
end
def mock_api_exception(type: Faraday::Error, status: 503, body: nil)
allow(vine_api_service).to receive(:voucher_validation).and_raise(type.new(nil,
{ status:, body: }) )
end
end