From 3a367ceb6e403cda9ffe5cff4d19456f2c7c7f80 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 16 Oct 2024 13:08:48 +1100 Subject: [PATCH] Handle adding a VINE voucher to an order Plus specs --- .../voucher_adjustments_controller.rb | 64 +++++++- app/models/voucher.rb | 3 + config/locales/en.yml | 1 + spec/requests/voucher_adjustments_spec.rb | 152 +++++++++++++++++- 4 files changed, 208 insertions(+), 12 deletions(-) diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb index c4f5fdf5fb..b8f3a3f1e4 100644 --- a/app/controllers/voucher_adjustments_controller.rb +++ b/app/controllers/voucher_adjustments_controller.rb @@ -4,7 +4,16 @@ class VoucherAdjustmentsController < BaseController before_action :set_order def create - if add_voucher + if voucher_params[:voucher_code].blank? + @order.errors.add(:voucher_code, I18n.t('checkout.errors.voucher_not_found')) + return render_error + end + + voucher = load_voucher + + return render_error unless valid_voucher?(voucher) + + if add_voucher_to_order(voucher) update_payment_section elsif @order.errors.present? render_error @@ -30,19 +39,28 @@ class VoucherAdjustmentsController < BaseController @order = current_order end - def add_voucher - if voucher_params[:voucher_code].blank? - @order.errors.add(:voucher_code, I18n.t('checkout.errors.voucher_not_found')) - return false - end - - voucher = Voucher.find_by(code: voucher_params[:voucher_code], enterprise: @order.distributor) + def valid_voucher?(voucher) + return false if @order.errors.present? if voucher.nil? @order.errors.add(:voucher_code, I18n.t('checkout.errors.voucher_not_found')) return false end + if !voucher.valid? + @order.errors.add( + :voucher_code, + I18n.t( + 'checkout.errors.create_voucher_error', error: voucher.errors.full_messages.to_sentence + ) + ) + return false + end + + true + end + + def add_voucher_to_order(voucher) adjustment = voucher.create_adjustment(voucher.code, @order) unless adjustment.persisted? @@ -51,6 +69,7 @@ class VoucherAdjustmentsController < BaseController return false end + # calculate_voucher_adjustment clear_payments VoucherAdjustmentsService.new(@order).update @@ -59,6 +78,35 @@ class VoucherAdjustmentsController < BaseController true end + def load_voucher + voucher = Voucher.not_vine.find_by(code: voucher_params[:voucher_code], + enterprise: @order.distributor) + return voucher unless voucher.nil? + + vine_voucher + end + + def vine_voucher + vine_voucher_validator = VineVoucherValidatorService.new( + voucher_code: voucher_params[:voucher_code], enterprise: @order.distributor + ) + voucher = vine_voucher_validator.validate + + if vine_voucher_validator.errors.present? + return nil if ignored_vine_api_error(vine_voucher_validator) + + @order.errors.add(:voucher_code, I18n.t('checkout.errors.add_voucher_error')) + return nil + end + + 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", diff --git a/app/models/voucher.rb b/app/models/voucher.rb index 12ec455edf..eaa1d2273b 100644 --- a/app/models/voucher.rb +++ b/app/models/voucher.rb @@ -18,6 +18,9 @@ class Voucher < ApplicationRecord TYPES = ["Vouchers::FlatRate", "Vouchers::PercentageRate"].freeze + scope :vine, -> { where(voucher_type: "VINE") } + scope :not_vine, -> { where.not(voucher_type: "VINE").or(where(voucher_type: nil)) } + def code=(value) super(value.to_s.strip) end diff --git a/config/locales/en.yml b/config/locales/en.yml index e8e87f0303..cbb449f039 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2128,6 +2128,7 @@ en: no_shipping_methods_available: Checkout is not possible due to absence of shipping options. Please contact the shop owner. voucher_not_found: Not found add_voucher_error: There was an error while adding the voucher + create_voucher_error: "There was an error while creating the voucher: %{error}" shops: hubs: show_closed_shops: "Show closed shops" diff --git a/spec/requests/voucher_adjustments_spec.rb b/spec/requests/voucher_adjustments_spec.rb index 78ae9c29d5..158911dd68 100644 --- a/spec/requests/voucher_adjustments_spec.rb +++ b/spec/requests/voucher_adjustments_spec.rb @@ -34,10 +34,10 @@ RSpec.describe VoucherAdjustmentsController, type: :request do let(:params) { { order: { voucher_code: voucher.code } } } it "adds a voucher to the user's current order" do - post("/voucher_adjustments", params:) - + expect { + post("/voucher_adjustments", params:) + }.to change { order.reload.voucher_adjustments.count }.by(1) expect(response).to be_successful - expect(order.reload.voucher_adjustments.length).to eq(1) end context "when voucher doesn't exist" do @@ -56,7 +56,7 @@ RSpec.describe VoucherAdjustmentsController, type: :request do # Create a non valid adjustment bad_adjustment = build(:adjustment, label: nil) allow(voucher).to receive(:create_adjustment).and_return(bad_adjustment) - allow(Voucher).to receive(:find_by).and_return(voucher) + allow(Voucher).to receive_message_chain(:not_vine, :find_by).and_return(voucher) post("/voucher_adjustments", params:) @@ -85,6 +85,145 @@ RSpec.describe VoucherAdjustmentsController, type: :request do end.to change { order.reload.all_adjustments.payment_fee.count }.from(1).to(0) end end + + context "with a VINE voucher", feature: :connected_apps do + let(:vine_voucher_validator) { instance_double(VineVoucherValidatorService) } + + before do + allow(VineVoucherValidatorService).to receive(:new).and_return(vine_voucher_validator) + end + + context "with a new voucher" do + let(:params) { { order: { voucher_code: vine_voucher_code } } } + let(:vine_voucher_code) { "PQ3187" } + + context "with a valid voucher" do + it "verifies the voucher with VINE API" do + expect(vine_voucher_validator).to receive(:validate) + allow(vine_voucher_validator).to receive(:errors).and_return({}) + + post "/voucher_adjustments", params: + end + + it "adds a voucher to the user's current order" do + vine_voucher = create(:voucher_flat_rate, code: vine_voucher_code) + mock_vine_voucher_validator(voucher: vine_voucher) + + post("/voucher_adjustments", params:) + + expect(response).to be_successful + expect(order.reload.voucher_adjustments.length).to eq(1) + end + end + + 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" } + ) + + post("/voucher_adjustments", params:) + + expect(response).to be_unprocessable + expect(flash[:error]).to match "Voucher code Not found" + end + end + + context "when there is an API error" do + it "returns 422 and an error message" do + mock_vine_voucher_validator( + voucher: nil, + errors: { vine_api: "There was an error communicating with the API" } + ) + + post("/voucher_adjustments", params:) + + expect(response).to be_unprocessable + expect(flash[:error]).to match "There was an error while adding the voucher" + end + end + + context "when the voucher doesn't exist" do + it "returns 422 and an error message" do + mock_vine_voucher_validator(voucher: nil, + errors: { not_found_voucher: "The voucher doesn't exist" }) + + post("/voucher_adjustments", params:) + + expect(response).to be_unprocessable + expect(flash[:error]).to match "Voucher code Not found" + end + end + + context "when the voucher is invalid voucher" do + it "returns 422 and an error message" do + mock_vine_voucher_validator(voucher: nil, + errors: { invalid_voucher: "The voucher is not valid" }) + + post("/voucher_adjustments", params:) + + expect(response).to be_unprocessable + expect(flash[:error]).to match "There was an error while adding the voucher" + end + end + + context "when creating a new voucher fails" do + it "returns 422 and an error message" do + vine_voucher = build(:voucher_flat_rate, code: vine_voucher_code, + enterprise: distributor, amount: "") + mock_vine_voucher_validator(voucher: vine_voucher) + + post("/voucher_adjustments", params:) + + expect(response).to be_unprocessable + expect(flash[:error]).to match( + "There was an error while creating the voucher: Amount can't be blank and " \ + "Amount is not a number" + ) + end + end + end + + context "with an existing voucher" do + let(:params) { { order: { voucher_code: vine_voucher_code } } } + let(:vine_voucher_code) { "PQ3187" } + + it "verify the voucher with VINE API" do + expect(vine_voucher_validator).to receive(:validate) + allow(vine_voucher_validator).to receive(:errors).and_return({}) + + post "/voucher_adjustments", params: + end + + it "adds a voucher to the user's current order" do + vine_voucher = create(:voucher_flat_rate, code: vine_voucher_code, + enterprise: distributor) + mock_vine_voucher_validator(voucher: vine_voucher) + + expect { + post("/voucher_adjustments", params:) + }.to change { order.reload.voucher_adjustments.count }.by(1) + expect(response).to be_successful + end + + context "when updating the voucher fails" do + it "returns 422 and an error message" do + vine_voucher = build(:voucher_flat_rate, code: vine_voucher_code, + enterprise: distributor, amount: "") + mock_vine_voucher_validator(voucher: vine_voucher) + + post("/voucher_adjustments", params:) + + expect(response).to be_unprocessable + expect(flash[:error]).to match( + "There was an error while creating the voucher: Amount can't be blank and " \ + "Amount is not a number" + ) + end + end + end + end end describe "DELETE voucher_adjustments/:id" do @@ -137,4 +276,9 @@ RSpec.describe VoucherAdjustmentsController, type: :request do end end end + + def mock_vine_voucher_validator(voucher:, errors: {}) + allow(vine_voucher_validator).to receive(:validate).and_return(voucher) + allow(vine_voucher_validator).to receive(:errors).and_return(errors) + end end