From 8e38e76836e16b6b06a48ddd953d20640838ab62 Mon Sep 17 00:00:00 2001 From: nika-piotrowska Date: Wed, 14 Jan 2026 12:24:28 +0100 Subject: [PATCH 1/6] Remove card checks throughout the app --- .../adjustments_updater_decorator.rb | 2 -- app/services/apply_sparta_discount_service.rb | 2 +- app/services/promotion_switcher_service.rb | 18 ++++++++++++------ 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/models/spree/adjustable/adjustments_updater_decorator.rb b/app/models/spree/adjustable/adjustments_updater_decorator.rb index 2be1c17..28de62b 100644 --- a/app/models/spree/adjustable/adjustments_updater_decorator.rb +++ b/app/models/spree/adjustable/adjustments_updater_decorator.rb @@ -7,8 +7,6 @@ module AdjustmentsUpdaterDecorator def set_spree_adjustments adjustable = @adjustable.is_a?(::Spree::Order) ? @adjustable : @adjustable.order - - @adjustable.adjustments.destroy_all if adjustable.public_metadata[:spl_card_active] end def shipment_with_adjustments? diff --git a/app/services/apply_sparta_discount_service.rb b/app/services/apply_sparta_discount_service.rb index e70ef5f..b4ccc51 100644 --- a/app/services/apply_sparta_discount_service.rb +++ b/app/services/apply_sparta_discount_service.rb @@ -19,7 +19,7 @@ def call # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity,Metrics/ next if sparta_item['discounts'].nil? label = "SPARTA_#{sparta_item&.fetch('discounts')&.first&.fetch('name')}_#{line_item.id}" # rubocop:disable Style/SafeNavigationChainLength - amount = -sparta_item&.fetch('discountGross') # Negative value for discount + amount = -BigDecimal(sparta_item["discountGross"].to_s) # Negative value for discount discounts_present?(line_item, label) update_sparta_adjustment(line_item, label, amount) create_sparta_adjustment(order, amount, label, line_item) diff --git a/app/services/promotion_switcher_service.rb b/app/services/promotion_switcher_service.rb index 0396a97..2b6d6db 100644 --- a/app/services/promotion_switcher_service.rb +++ b/app/services/promotion_switcher_service.rb @@ -10,10 +10,7 @@ def initialize(order, check_only) end def call # rubocop:disable Metrics/AbcSize - return unless order.public_metadata.key?(:spl_card_active) - - apply_sparta_discount(order, check_only) if cast_boolean(order.public_metadata[:spl_card_active]) - remove_sparta_discount(order) unless cast_boolean(order.public_metadata[:spl_card_active]) + apply_sparta_discount(order, check_only) ensure order.reload end @@ -23,10 +20,11 @@ def call # rubocop:disable Metrics/AbcSize attr_accessor :check_only, :line_items, :order def apply_sparta_discount(order, check_only) - return unless order.line_items.any? && order.public_metadata['spl_no_card'].present? + return unless order.line_items.any? + card_number = prepare_card_number_if_exist(order.public_metadata) spl_response = Spl::SpartaLoyaltyService.new(order.token, - order.public_metadata['spl_no_card'], + card_number, order.line_items, DateTime.current, order.products, @@ -44,4 +42,12 @@ def create_sparta_adjustments(spl_response, order) def remove_sparta_discount(order) RemoveSpartaDiscountService.destroy_all_sparta_adjustments(order) end + + def prepare_card_number_if_exist(metadata) + if cast_boolean(metadata[:spl_card_active]) + metadata['spl_no_card'] + else + '' + end + end end From 187a2b81ce61fac64f08aa3b05f28b76630ce130 Mon Sep 17 00:00:00 2001 From: nika-piotrowska Date: Thu, 15 Jan 2026 21:40:50 +0100 Subject: [PATCH 2/6] Remove redundant lines and improve test throughout the app --- app/models/order_updater_decorator.rb | 15 - app/models/payment_decorator.rb | 8 +- .../adjustments_updater_decorator.rb | 13 +- .../spree/cart/recalculate_decorator.rb | 15 - config/initializers/spree_spl.rb | 4 - .../adjustments_updater_decorator_spec.rb | 321 +++--------------- spec/models/order_updater_decorator_spec.rb | 67 ---- spec/models/payment_decorator_spec.rb | 197 ++--------- .../promotion_switcher_service_spec.rb | 244 ++++++------- 9 files changed, 193 insertions(+), 691 deletions(-) delete mode 100644 app/services/spree/cart/recalculate_decorator.rb diff --git a/app/models/order_updater_decorator.rb b/app/models/order_updater_decorator.rb index cbe6fd4..7e8fa83 100644 --- a/app/models/order_updater_decorator.rb +++ b/app/models/order_updater_decorator.rb @@ -7,19 +7,4 @@ def preform_update_sparta_state_job # rubocop:disable Metrics/AbcSize UpdateSpartaStateJob.perform_later(order.token, 'D', order.number, order.store) if order.payment_state == 'paid' UpdateSpartaStateJob.perform_later(order.token, 'C', order.number, order.store) if order.state == 'canceled' end - - def check_spl_adjustments # rubocop:disable Metrics/MethodLength - if order.public_metadata['spl_card_active'] == true - updated_any_adjustment = false - order.adjustments.each do |adjustment| - if adjustment.source_type != 'SPL' && adjustment.eligible? - adjustment.update(eligible: false) - updated_any_adjustment = true - end - end - updated_any_adjustment - else - false - end - end end diff --git a/app/models/payment_decorator.rb b/app/models/payment_decorator.rb index 248336c..dc2032a 100644 --- a/app/models/payment_decorator.rb +++ b/app/models/payment_decorator.rb @@ -4,16 +4,10 @@ module PaymentDecorator private def promotion_switcher(order, check_only) - return unless order.public_metadata.key?(:spl_no_card) && order.public_metadata.key?(:spl_card_active) - return unless order.public_metadata['spl_card_active'] - PromotionSwitcherService.new(order, check_only).call end - def update_sparta_state - return unless order.public_metadata.key?(:spl_no_card) && order.public_metadata.key?(:spl_card_active) - return unless order.public_metadata['spl_card_active'] - + def preform_update_sparta_state_job UpdateSpartaStateJob.perform_later(order.token, 'D', order.number, order.store) end end diff --git a/app/models/spree/adjustable/adjustments_updater_decorator.rb b/app/models/spree/adjustable/adjustments_updater_decorator.rb index 28de62b..cbae6ca 100644 --- a/app/models/spree/adjustable/adjustments_updater_decorator.rb +++ b/app/models/spree/adjustable/adjustments_updater_decorator.rb @@ -3,26 +3,19 @@ module Spree module Adjustable module AdjustmentsUpdaterDecorator + SPL_SOURCE_TYPE = 'SPL' private def set_spree_adjustments adjustable = @adjustable.is_a?(::Spree::Order) ? @adjustable : @adjustable.order end - def shipment_with_adjustments? - @adjustable.is_a?(::Spree::Shipment) && @adjustable.order.public_metadata.key?(:spl_card_active) - end - - def order_with_adjustments? - @adjustable.is_a?(::Spree::Order) && @adjustable.public_metadata.key?(:spl_card_active) - end - def line_item_with_spl_adjustments? - @adjustable.is_a?(::Spree::LineItem) && @adjustable.adjustments.any? { |adj| adj.source_type == 'SPL' } + @adjustable.is_a?(::Spree::LineItem) && @adjustable.adjustments.any? { |adj| adj.source_type == SPL_SOURCE_TYPE } end def recalculate_spl_adjustments(attributes, totals) - sparta_adjustments = @adjustable.adjustments.select { |adj| adj.source_type == 'SPL' && adj.eligible? } + sparta_adjustments = @adjustable.adjustments.select { |adj| adj.source_type == SPL_SOURCE_TYPE && adj.eligible? } total_adjustment_amount = sparta_adjustments.sum(&:amount) assign_spl_totals(attributes, total_adjustment_amount, Time.current) @adjustable.update_columns(totals) diff --git a/app/services/spree/cart/recalculate_decorator.rb b/app/services/spree/cart/recalculate_decorator.rb deleted file mode 100644 index a22ffb6..0000000 --- a/app/services/spree/cart/recalculate_decorator.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module Spree - module Cart - module RecalculateDecorator - private - - def spl_cart_active?(order) - return unless order.public_metadata.key?(:spl_card_active) # rubocop:disable Style/ReturnNilInPredicateMethodDefinition - - ActiveModel::Type::Boolean.new.cast(order.public_metadata[:spl_card_active]) - end - end - end -end diff --git a/config/initializers/spree_spl.rb b/config/initializers/spree_spl.rb index ae63b1b..8be0516 100644 --- a/config/initializers/spree_spl.rb +++ b/config/initializers/spree_spl.rb @@ -21,10 +21,6 @@ Spree::Adjustable::AdjustmentsUpdaterDecorator ) - ::Spree::Cart::Recalculate.prepend( - Spree::Cart::RecalculateDecorator - ) - ::Spree::PromotionHandler::Cart.prepend( CartDecorator ) diff --git a/spec/models/adjustments_updater_decorator_spec.rb b/spec/models/adjustments_updater_decorator_spec.rb index 43dbba0..a4bd2a1 100644 --- a/spec/models/adjustments_updater_decorator_spec.rb +++ b/spec/models/adjustments_updater_decorator_spec.rb @@ -1,325 +1,106 @@ # frozen_string_literal: true -require 'rails_helper' +require "rails_helper" RSpec.describe Spree::Adjustable::AdjustmentsUpdater, type: :model do - let(:store) { Spree::Store.default || create(:store, default: true) } - let(:public_metadata) { {} } + # Use the upstream Spree approach to avoid store/product validation issues + let(:order) { create(:order_with_line_items, line_items_count: 1) } + let(:line_item) { order.line_items.first } - let!(:order) do - create( - :order, - store: store, - public_metadata: public_metadata - ) - end - let(:adjustable) { order } subject(:updater) { described_class.new(adjustable) } - describe '#set_spree_adjustments (private)' do - def run_set_spree_adjustments - updater.send(:set_spree_adjustments) - end - before do - order.adjustments = adjustments - shipment.adjustments << shipment_adjustment - order.save - shipment.save - rescue StandardError - # Ignored - end - - context 'when adjustable is an order with spl_card_active: true (symbol key)' do - let(:public_metadata) { { spl_card_active: true } } - let(:adjustments) { [adjustment1, adjustment2] } - let(:adjustment1) { create(:adjustment, order: order) } - let(:adjustment2) { create(:adjustment, order: order) } - - it 'destroys all adjustments on the order' do - expect { run_set_spree_adjustments } - .to change { order.adjustments.reload.count }.from(2).to(0) - end - end - - context 'when adjustable is an order with spl_card_active: true (string key)' do - let(:public_metadata) { { 'spl_card_active' => true } } - let(:adjustments) { [adjustment1, adjustment2] } - let(:adjustment1) { create(:adjustment, order: order) } - let(:adjustment2) { create(:adjustment, order: order) } - - it 'destroys all adjustments on the order' do - expect { run_set_spree_adjustments } - .to change { order.adjustments.reload.count }.from(2).to(0) - end - end - - context 'when adjustable is an order with spl_card_active: false' do - let(:public_metadata) { { spl_card_active: false } } - let(:adjustments) { [adjustment] } - let!(:adjustment) { create(:adjustment, order: order, adjustable: order) } - - it 'does not destroy adjustments' do - expect { run_set_spree_adjustments } - .not_to change { order.adjustments.reload.count }.from(1) - end - end - - context 'when adjustable is a shipment and order has spl_card_active: true' do - let(:public_metadata) { { spl_card_active: true } } - let(:adjustable) { shipment } - let!(:shipment) { create(:shipment, order: order) } - let(:adjustments) { [order_adjustment] } - let!(:shipment_adjustment) { create(:adjustment, order: order, adjustable: shipment) } - let!(:order_adjustment) { create(:adjustment, order: order, adjustable: order) } - - it 'destroys only the shipment adjustments, not order adjustments' do - expect do - run_set_spree_adjustments - end.to change { shipment.adjustments.reload.count }.from(1).to(0) - - expect do - run_set_spree_adjustments - end.not_to(change { order.adjustments.reload.count }) - end - end - - context 'when adjustable is a line item and order has spl_card_active: true' do - let(:public_metadata) { { spl_card_active: true } } - - let(:adjustable) { line_item } - - let(:line_item) { create(:line_item) } - let(:line_item_adjustment) { create(:adjustment, order: order, adjustable: line_item) } - let(:order_adjustment) { create(:adjustment, order: order, adjustable: order) } - - it 'destroys only the line item adjustments, not order adjustments' do - line_item.adjustments << line_item_adjustment - line_item.save - order.adjustments << order_adjustment - order.line_items << line_item - order.save - - expect { run_set_spree_adjustments } - .to change { line_item.adjustments.reload.count }.from(1).to(0) - end - end - - context 'when order has no spl_card_active key' do - let(:public_metadata) { {} } - - let!(:adjustment) { create(:adjustment, order: order, adjustable: order) } - - it 'does nothing' do - expect { run_set_spree_adjustments } - .not_to change { order.adjustments.reload.count }.from(1) - end - end - end - - describe '#shipment_with_adjustments? (private)' do - def shipment_with_adjustments? - updater.send(:shipment_with_adjustments?) - end - - context 'when adjustable is a shipment and order has spl_card_active key as symbol' do - let(:public_metadata) { { spl_card_active: true } } - let(:adjustable) { shipment } - let!(:shipment) { create(:shipment, order: order) } - - it 'returns true' do - expect(shipment_with_adjustments?).to eq(true) - end - end - - context 'when adjustable is a shipment and order has spl_card_active key as string' do - let(:public_metadata) { { 'spl_card_active' => true } } - let(:adjustable) { shipment } - let!(:shipment) { create(:shipment, order: order) } - - it 'returns true' do - expect(shipment_with_adjustments?).to eq(true) - end - end - - context 'when adjustable is a shipment but order has no key' do - let(:public_metadata) { {} } - let(:adjustable) { shipment } - let!(:shipment) { create(:shipment, order: order) } - - it 'returns false' do - expect(shipment_with_adjustments?).to eq(false) - end - end - - context 'when adjustable is not a shipment' do - let(:public_metadata) { { spl_card_active: true } } - let(:adjustable) { order } - - it 'returns false' do - expect(shipment_with_adjustments?).to eq(false) - end - end - end - - describe '#order_with_adjustments? (private)' do - def order_with_adjustments? - updater.send(:order_with_adjustments?) - end - - context 'when adjustable is an order and it has spl_card_active key as symbol' do - let(:public_metadata) { { spl_card_active: true } } - - it 'returns true' do - expect(order_with_adjustments?).to eq(true) - end - end - - context 'when adjustable is an order and it has spl_card_active key as string' do - let(:public_metadata) { { 'spl_card_active' => true } } - - it 'returns true' do - expect(order_with_adjustments?).to eq(true) - end - end - - context 'when adjustable is an order without that key' do - let(:public_metadata) { {} } - - it 'returns false' do - expect(order_with_adjustments?).to eq(false) - end - end - - context 'when adjustable is not an order' do - let(:public_metadata) { { spl_card_active: true } } - let(:adjustable) { create(:shipment, order: order) } - - it 'returns false' do - expect(order_with_adjustments?).to eq(false) - end - end - end - - describe '#line_item_with_spl_adjustments? (private)' do + describe "#line_item_with_spl_adjustments? (private)" do def line_item_with_spl_adjustments? updater.send(:line_item_with_spl_adjustments?) end - before do - line_item.adjustments << line_item_adjustment - line_item.save - order.line_items << line_item - order.save - rescue StandardError - # Ignored - end - - context 'when adjustable is a line item with SPL adjustment' do + context "when adjustable is a line item with an SPL adjustment" do let(:adjustable) { line_item } - let(:line_item) { create(:line_item) } - let(:line_item_adjustment) do - create(:adjustment, - order: order, - adjustable: line_item, - source_type: 'SPL') + + before do + create(:adjustment, order: order, adjustable: line_item, source_type: "SPL", eligible: true, amount: -2.to_d) end - it 'returns true' do + it "returns true" do expect(line_item_with_spl_adjustments?).to eq(true) end end - context 'when adjustable is a line item with non-SPL adjustments only' do + context "when adjustable is a line item without SPL adjustments" do let(:adjustable) { line_item } - let(:line_item) { create(:line_item) } - let(:line_item_adjustment) do - create(:adjustment, - order: order, - adjustable: line_item, - source_type: 'Promo') + before do + create(:adjustment, order: order, adjustable: line_item, source_type: "Promo", eligible: true, amount: -2.to_d) end - it 'returns false' do + it "returns false" do expect(line_item_with_spl_adjustments?).to eq(false) end end - context 'when adjustable is not a line item but has SPL adjustments' do + context "when adjustable is not a line item" do let(:adjustable) { order } - let!(:spl_adj) do - create(:adjustment, - order: order, - adjustable: order, - source_type: 'SPL') + before do + create(:adjustment, order: order, adjustable: order, source_type: "SPL", eligible: true, amount: -2.to_d) end - it 'returns false' do + it "returns false" do expect(line_item_with_spl_adjustments?).to eq(false) end end end - describe '#recalculate_spl_adjustments (private)' do - before do - line_item.adjustments = adjustments - line_item.save - order.line_items << line_item - order.save - rescue StandardError - # Ignored - end - + describe "#recalculate_spl_adjustments (private)" do def recalculate_spl_adjustments(attributes, totals) updater.send(:recalculate_spl_adjustments, attributes, totals) end let(:adjustable) { line_item } - let!(:line_item) { create(:line_item) } - let(:adjustments) do - [spl_adj1, spl_adj2, spl_ineligible, other_adj] - end let!(:spl_adj1) do create(:adjustment, - order: order, - adjustable: line_item, - source_type: 'SPL', - eligible: true, - amount: 10.to_d) + order: order, + adjustable: line_item, + source_type: "SPL", + eligible: true, + amount: 10.to_d + ) end let!(:spl_adj2) do create(:adjustment, - order: order, - adjustable: line_item, - source_type: 'SPL', - eligible: true, - amount: -5.to_d) + order: order, + adjustable: line_item, + source_type: "SPL", + eligible: true, + amount: -5.to_d + ) end let!(:spl_ineligible) do create(:adjustment, - order: order, - adjustable: line_item, - source_type: 'SPL', - eligible: false, - amount: 100.to_d) + order: order, + adjustable: line_item, + source_type: "SPL", + eligible: false, + amount: 100.to_d + ) end let!(:other_adj) do create(:adjustment, - order: order, - adjustable: line_item, - source_type: 'Promo', - eligible: true, - amount: 50.to_d) + order: order, + adjustable: line_item, + source_type: "Promo", + eligible: true, + amount: 50.to_d + ) end let(:attributes) { {} } let(:totals_hash) { { some_total: 123.to_d } } - let(:fixed_time) { Time.zone.parse('2024-01-01 12:00:00') } + let(:fixed_time) { Time.zone.parse("2024-01-01 12:00:00") } let(:expected_total) { spl_adj1.amount + spl_adj2.amount } before do @@ -328,8 +109,9 @@ def recalculate_spl_adjustments(attributes, totals) allow(updater).to receive(:assign_spl_totals).and_call_original end - it 'sums only eligible SPL adjustments and passes them to assign_spl_totals' do + it "sums only eligible SPL adjustments and passes the sum to assign_spl_totals" do recalculate_spl_adjustments(attributes, totals_hash) + expect(updater).to have_received(:assign_spl_totals).with( attributes, expected_total, @@ -337,13 +119,13 @@ def recalculate_spl_adjustments(attributes, totals) ) end - it 'updates the adjustable using update_columns with provided totals' do + it "updates the adjustable using update_columns with provided totals" do recalculate_spl_adjustments(attributes, totals_hash) expect(line_item).to have_received(:update_columns).with(totals_hash) end - it 'mutates attributes to contain adjustment_total, promo_total and updated_at' do + it "mutates attributes with adjustment_total, promo_total and updated_at" do recalculate_spl_adjustments(attributes, totals_hash) expect(attributes[:adjustment_total]).to eq(expected_total) @@ -352,16 +134,17 @@ def recalculate_spl_adjustments(attributes, totals) end end - describe '#assign_spl_totals (private)' do + describe "#assign_spl_totals (private)" do def assign_spl_totals(attributes, total_amount, time) updater.send(:assign_spl_totals, attributes, total_amount, time) end + let(:adjustable) { line_item } let(:attributes) { {} } let(:total_amount) { 42.5.to_d } - let(:time) { Time.zone.parse('2024-02-02 10:00:00') } + let(:time) { Time.zone.parse("2024-02-02 10:00:00") } - it 'sets adjustment_total, promo_total and updated_at' do + it "sets adjustment_total, promo_total and updated_at" do assign_spl_totals(attributes, total_amount, time) expect(attributes[:adjustment_total]).to eq(total_amount) diff --git a/spec/models/order_updater_decorator_spec.rb b/spec/models/order_updater_decorator_spec.rb index ff2c96a..658321b 100644 --- a/spec/models/order_updater_decorator_spec.rb +++ b/spec/models/order_updater_decorator_spec.rb @@ -85,71 +85,4 @@ end end end - - describe '#check_spl_adjustments' do - def run_check - updater.send(:check_spl_adjustments) - end - - context 'when spl_card_active is false' do - let(:public_metadata) { { 'spl_card_active' => false } } - let(:adjustments) { [adjustment] } - - let(:adjustment) do - create(:adjustment, source_type: 'Promo', order:, eligible: true) - end - - it 'returns false and does nothing' do - order.adjustments = adjustments - order.save - expect(run_check).to eq(false) - expect(adjustment.reload.eligible).to eq(true) - end - end - - context 'when spl_card_active is true with eligible non-SPL adjustments' do - let(:public_metadata) { { 'spl_card_active' => true } } - let(:adjustments) { [spl_adj, promo_adj, promo_inelig] } - - let(:spl_adj) { create(:adjustment, order:, source_type: 'SPL', eligible: true) } - let(:promo_adj) { create(:adjustment, order:, source_type: 'Promo', eligible: true) } - let(:promo_inelig) { create(:adjustment, order:, source_type: 'Promo', eligible: false) } - - it 'returns true only if non-SPL eligible adjustments were changed' do - order.adjustments = adjustments - order.save - expect(run_check).to eq(true) - expect(promo_adj.reload.eligible).to eq(false) - expect(spl_adj.reload.eligible).to eq(true) - expect(promo_inelig.reload.eligible).to eq(false) - end - end - - context 'when spl_card_active is true but no eligible non-SPL adjustments' do - let(:public_metadata) { { 'spl_card_active' => true } } - let(:adjustments) { [spl_adj, promo_inelig] } - - let(:spl_adj) { create(:adjustment, order:, source_type: 'SPL', eligible: true) } - let(:promo_inelig) { create(:adjustment, order:, source_type: 'Promo', eligible: false) } - - it 'returns false' do - order.adjustments = adjustments - order.save - expect(run_check).to eq(false) - end - end - - context 'when public_metadata does not contain key' do - let(:public_metadata) { {} } - let(:adjustments) { [promo_adj] } - let(:promo_adj) { create(:adjustment, order:, source_type: 'Promo', eligible: true) } - - it 'returns false and does nothing' do - order.adjustments = adjustments - order.save - expect(run_check).to eq(false) - expect(promo_adj.reload.eligible).to eq(true) - end - end - end end diff --git a/spec/models/payment_decorator_spec.rb b/spec/models/payment_decorator_spec.rb index 1b9ec7d..f6212f4 100644 --- a/spec/models/payment_decorator_spec.rb +++ b/spec/models/payment_decorator_spec.rb @@ -1,194 +1,49 @@ # frozen_string_literal: true -require 'rails_helper' +require "rails_helper" RSpec.describe Spree::Payment, type: :model do let(:store) { Spree::Store.default || create(:store, default: true) } - let(:public_metadata) { {} } + let(:order) { create(:order, store: store) } - let(:order) do - create( - :order, - store: store, - public_metadata: public_metadata - ) - end - - subject(:decorated_payment) { described_class.new(order: order) } - - let(:service_instance) { instance_double(PromotionSwitcherService, call: service_result) } - let(:service_result) { :some_result } - - before do - allow(PromotionSwitcherService).to receive(:new).and_return(service_instance) - allow(UpdateSpartaStateJob).to receive(:perform_later) - end - - describe '#promotion_switcher (private)' do - def run_promotion_switcher(check_only) - decorated_payment.send(:promotion_switcher, order, check_only) - end - - context 'when both keys exist and spl_card_active is true (symbol keys)' do - let(:public_metadata) { { spl_no_card: '1234567890123', spl_card_active: true } } - - it 'initializes PromotionSwitcherService with order and check_only and calls it' do - result = run_promotion_switcher(true) - - expect(PromotionSwitcherService).to have_received(:new).with(order, true) - expect(service_instance).to have_received(:call) - expect(result).to eq(service_result) - end - - it 'passes false correctly as check_only flag' do - run_promotion_switcher(false) - - expect(PromotionSwitcherService).to have_received(:new).with(order, false) - end - end - - context 'when both keys exist but spl_card_active is true (string keys)' do - let(:public_metadata) { { 'spl_no_card' => '1234567890123', 'spl_card_active' => true } } - - it 'initializes PromotionSwitcherService with order and check_only and calls it' do - result = run_promotion_switcher(true) - - expect(result).to eq(service_result) - expect(PromotionSwitcherService).to have_received(:new) - expect(service_instance).to have_received(:call) - end - - it 'passes false correctly as check_only flag' do - run_promotion_switcher(false) - - expect(PromotionSwitcherService).to have_received(:new).with(order, false) - end - end - - context 'when both keys exist but spl_card_active is false (symbol keys)' do - let(:public_metadata) { { spl_no_card: '1234567890123', spl_card_active: false } } - - it 'does not initialize PromotionSwitcherService' do - result = run_promotion_switcher(true) - - expect(result).to be_nil - expect(PromotionSwitcherService).not_to have_received(:new) - expect(service_instance).not_to have_received(:call) - end - end + subject(:payment) { described_class.new(order: order) } - context 'when only one of required keys is present' do - context 'when spl_no_card present but spl_card_active missing' do - let(:public_metadata) { { spl_no_card: '1234567890123' } } + describe "#promotion_switcher (private)" do + let(:service_result) { :some_result } + let(:service_instance) { instance_double(PromotionSwitcherService, call: service_result) } - it 'does nothing and returns nil' do - result = run_promotion_switcher(true) + it "initializes PromotionSwitcherService with order + check_only and calls it" do + allow(PromotionSwitcherService).to receive(:new).and_return(service_instance) - expect(result).to be_nil - expect(PromotionSwitcherService).not_to have_received(:new) - end - end + result = payment.send(:promotion_switcher, order, true) - context 'when spl_card_active present but spl_no_card missing' do - let(:public_metadata) { { spl_card_active: '1234567890123' } } - - it 'does nothing and returns nil' do - result = run_promotion_switcher(true) - - expect(result).to be_nil - expect(PromotionSwitcherService).not_to have_received(:new) - end - end + expect(PromotionSwitcherService).to have_received(:new).with(order, true) + expect(service_instance).to have_received(:call) + expect(result).to eq(service_result) end - context 'when public_metadata is empty' do - let(:public_metadata) { {} } + it "passes false correctly as check_only flag" do + allow(PromotionSwitcherService).to receive(:new).and_return(service_instance) - it 'returns nil and does nothing' do - result = run_promotion_switcher(true) + payment.send(:promotion_switcher, order, false) - expect(result).to be_nil - expect(PromotionSwitcherService).not_to have_received(:new) - end + expect(PromotionSwitcherService).to have_received(:new).with(order, false) end end - describe '#update_sparta_state (private)' do - def run_update_sparta_state - decorated_payment.send(:update_sparta_state) - end - - context 'when both keys exist and spl_card_active is true (symbol keys)' do - let(:public_metadata) { { spl_no_card: '1234567890123', spl_card_active: true } } - - it 'enqueues UpdateSpartaStateJob with D state' do - run_update_sparta_state - - expect(UpdateSpartaStateJob).to have_received(:perform_later).with( - order.token, - 'D', - order.number, - order.store - ) - end - end - - context 'when both keys exist and spl_card_active is true (string keys)' do - let(:public_metadata) { { 'spl_no_card' => '1234567890123', 'spl_card_active' => true } } - - it 'enqueues UpdateSpartaStateJob with D state' do - run_update_sparta_state - - expect(UpdateSpartaStateJob).to have_received(:perform_later).with( - order.token, - 'D', - order.number, - order.store - ) - end - end - - context 'when both keys exist but spl_card_active is false (symbol keys)' do - let(:public_metadata) { { spl_no_card: '1234567890123', spl_card_active: false } } - - it 'does not enqueue UpdateSpartaStateJob' do - run_update_sparta_state - - expect(UpdateSpartaStateJob).not_to have_received(:perform_later) - end - end - - context 'when only one of required keys is present' do - context 'when spl_no_card present but spl_card_active missing' do - let(:public_metadata) { { spl_no_card: '1234567890123' } } - - it 'does not enqueue UpdateSpartaStateJob' do - run_update_sparta_state - - expect(UpdateSpartaStateJob).not_to have_received(:perform_later) - end - end - - context 'when spl_card_active present but spl_no_card missing' do - let(:public_metadata) { { spl_card_active: true } } - - it 'does not enqueue UpdateSpartaStateJob' do - run_update_sparta_state - - expect(UpdateSpartaStateJob).not_to have_received(:perform_later) - end - end - end - - context 'when public_metadata is empty' do - let(:public_metadata) { {} } + describe "#preform_update_sparta_state_job (private)" do + it "enqueues UpdateSpartaStateJob with token, state 'D', number, store" do + allow(UpdateSpartaStateJob).to receive(:perform_later) - it 'does not enqueue UpdateSpartaStateJob' do - run_update_sparta_state + payment.send(:preform_update_sparta_state_job) - expect(UpdateSpartaStateJob).not_to have_received(:perform_later) - end + expect(UpdateSpartaStateJob).to have_received(:perform_later).with( + order.token, + "D", + order.number, + order.store + ) end end end diff --git a/spec/services/promotion_switcher_service_spec.rb b/spec/services/promotion_switcher_service_spec.rb index 5c363df..1ff1c44 100644 --- a/spec/services/promotion_switcher_service_spec.rb +++ b/spec/services/promotion_switcher_service_spec.rb @@ -3,139 +3,94 @@ require 'rails_helper' RSpec.describe PromotionSwitcherService do - let(:country) { create(:country) } - let(:store) { create(:store, default_country: country) } - let(:order) do - create( - :order, - store:, - public_metadata: public_metadata - ) - end - - let(:check_only) { true } + describe '#call' do + let(:country) { create(:country) } + let(:store) { create(:store, default_country: country) } + + let(:public_metadata) do + { + spl_card_active: 'true', # service reads metadata[:spl_card_active] + 'spl_no_card' => '5100179585157' # service reads metadata['spl_no_card'] (STRING key!) + } + end - let(:service) { described_class.new(order, check_only) } + let(:order) { create(:order, store: store, public_metadata: public_metadata) } + let(:check_only) { true } + let(:service) { described_class.new(order, check_only) } + + let(:variant1) { create(:variant, sku: 'TESTPRD1', price: 6.75) } + let(:variant2) { create(:variant, sku: 'TESTPRD4', price: 7.73) } + + let!(:line_item1) { create(:line_item, order: order, variant: variant1, quantity: 1, price: 6.75) } + let!(:line_item2) { create(:line_item, order: order, variant: variant2, quantity: 3, price: 7.73) } + + let(:example_sparta_response) do + { + 'errorCode' => '0', + 'basket' => [ + { 'productCode' => 'TESTPRD1', 'quantity' => 1.0, 'amountGross' => 6.75, 'discountGross' => 0.0, 'pos' => 1 }, + { + 'productCode' => 'TESTPRD4', + 'quantity' => 3.0, + 'amountGross' => 23.2, + 'discountGross' => 0.8, + 'discounts' => [ + { 'source' => 'LP', 'amount' => 0.8, 'percent' => 5.0, 'name' => '5% discount for TESTPRD4' } + ], + 'pos' => 2 + } + ], + 'discountGross' => 0.8 + } + end - let(:variant1) { create(:variant, sku: 'BS49252-BZ020-PSA000-000', price: 6.75) } - let(:variant2) { create(:variant, sku: 'BS49252-BZ020-PSA000-001', price: 7.73) } + it 'calls Sparta loyalty, gets response and applies Sparta discounts' do + sparta_service_double = instance_double(Spl::SpartaLoyaltyService) + apply_service_double = instance_double(ApplySpartaDiscountService) - let!(:line_item1) { create(:line_item, order:, variant: variant1, quantity: 1, price: 6.75) } - let!(:line_item2) { create(:line_item, order:, variant: variant2, quantity: 3, price: 7.73) } + expect(Spl::SpartaLoyaltyService).to receive(:new) do |token, card_no, line_items, date, products, chk, store_arg| + expect(token).to eq(order.token) + expect(card_no).to eq('5100179585157') + expect(line_items).to match_array(order.line_items) + expect(date).to be_a(DateTime) + expect(products).to match_array(order.products) + expect(chk).to eq(check_only) + expect(store_arg).to eq(order.store) - let(:exemple_sparta_response) do - { - 'errorCode' => '0', - 'balanceBurn' => 0.0, - 'balanceEarn' => 0.0, - 'balanceAfter' => 0.12, - 'bookedEarn' => false, - 'processId' => '663c92b05012e0b396ac632b', - 'messages' => [], - 'basket' => [ - { - 'productCode' => 'TESTPRD1', - 'productCode2' => nil, - 'quantity' => 1.0, - 'amountGross' => 6.75, - 'discountGross' => 0.0, - 'discountPercent' => nil, - 'unitPriceGross' => 6.75, - 'discounts' => nil, - 'isAward' => nil, - 'notPromoted' => nil, - 'skipCB' => nil, - 'skipDD' => nil, - 'skipRD' => nil, - 'pos' => 1 - }, - { - 'productCode' => 'TESTPRD4', - 'productCode2' => nil, - 'quantity' => 3.0, - 'amountGross' => 23.2, - 'discountGross' => 0.8, - 'discountPercent' => nil, - 'unitPriceGross' => 7.73, - 'discounts' => [ - { - 'source' => 'LP', - 'amount' => 0.8, - 'percent' => 5.0, - 'code' => '663c926e5012e0b396ac6328', - 'name' => '5% discount for TESTPRD4', - 'order' => 1, - 'quantity' => 2.0, - 'unitPriceGrossDiscounted' => nil - } - ], - 'isAward' => nil, - 'notPromoted' => nil, - 'skipCB' => nil, - 'skipDD' => nil, - 'skipRD' => nil, - 'pos' => 2 - } - ], - 'basketChanged' => true, - 'amountGross' => 29.95, - 'discountGross' => 0.8, - 'coupons' => [], - 'cardType' => { 'code' => 'DV' }, - 'requestId' => '00003_LSHRV' - } - end - - describe '#call' do - context 'when spl_card_active is true and card number is present' do - let(:public_metadata) do - { - spl_card_active: 'true', - spl_no_card: '5100179585157' - } + sparta_service_double end - it 'calls Sparta loyalty, gets response and applies Sparta discounts' do - sparta_service_double = instance_double(Spl::SpartaLoyaltyService) - apply_service_double = instance_double(ApplySpartaDiscountService) + expect(sparta_service_double).to receive(:call).and_return(example_sparta_response) - expect(Spl::SpartaLoyaltyService).to receive(:new) do |token, card_no, line_items, date, products, chk, store_arg| # rubocop:disable Layout/LineLength - expect(token).to eq(order.token) - expect(card_no).to eq('5100179585157') - expect(line_items).to match_array(order.line_items) - expect(date).to be_a(DateTime) - expect(products).to match_array(order.products) - expect(chk).to eq(check_only) - expect(store_arg).to eq(order.store) + expect(ApplySpartaDiscountService).to receive(:new) + .with(example_sparta_response, order) + .and_return(apply_service_double) - sparta_service_double - end + expect(apply_service_double).to receive(:call) - expect(sparta_service_double).to receive(:call).and_return(exemple_sparta_response) - expect(ApplySpartaDiscountService).to receive(:new).with(exemple_sparta_response, order) - .and_return(apply_service_double) - expect(apply_service_double).to receive(:call) - expect(order).to receive(:reload).and_call_original + expect(order).to receive(:reload).and_call_original - service.call - end + service.call end context 'when spl_card_active is true but Sparta returns nil' do let(:public_metadata) do { - spl_card_active: true, - spl_no_card: '5100179585157' + spl_card_active: 'true', + 'spl_no_card' => '5100179585157' } end - it 'does not apply Sparta discounts' do + it 'does not apply Sparta discounts (but still reloads the order)' do sparta_service_double = instance_double(Spl::SpartaLoyaltyService) expect(Spl::SpartaLoyaltyService).to receive(:new).and_return(sparta_service_double) expect(sparta_service_double).to receive(:call).and_return(nil) + expect(ApplySpartaDiscountService).not_to receive(:new) + expect(order).to receive(:reload).and_call_original + service.call end end @@ -144,66 +99,89 @@ let(:public_metadata) do { spl_card_active: 'false', - spl_no_card: '5100179585157' + 'spl_no_card' => '5100179585157' } end - it 'removes Sparta discounts' do - expect(Spl::SpartaLoyaltyService).not_to receive(:new) + it 'calls Sparta with empty card number and does not apply discounts' do + sparta_service_double = instance_double(Spl::SpartaLoyaltyService) + + expect(Spl::SpartaLoyaltyService).to receive(:new) do |_, card_no, *_| + expect(card_no).to eq('') + sparta_service_double + end - expect(RemoveSpartaDiscountService).to receive(:destroy_all_sparta_adjustments).with(order) + expect(sparta_service_double).to receive(:call).and_return(nil) + expect(ApplySpartaDiscountService).not_to receive(:new) + + expect(order).to receive(:reload).and_call_original service.call end end - context 'when spl_card_active key is missing' do + context 'when there are no line items' do let(:public_metadata) do { - spl_no_card: '5100179585157' + spl_card_active: 'true', + 'spl_no_card' => '5100179585157' } end - it 'does nothing' do + before do + order.line_items.destroy_all + end + + it 'does not call Sparta loyalty service or apply discounts' do expect(Spl::SpartaLoyaltyService).not_to receive(:new) expect(ApplySpartaDiscountService).not_to receive(:new) - expect(RemoveSpartaDiscountService).not_to receive(:destroy_all_sparta_adjustments) + + expect(order).to receive(:reload).and_call_original service.call end end - context 'when there are no line items' do + context 'when spl_card_active is true but card number is missing' do let(:public_metadata) do { - spl_card_active: true, - spl_no_card: '5100179585157' + spl_card_active: 'true' } end - before do - order.line_items.destroy_all - end + it 'calls Sparta with nil card number and does not apply discounts' do + sparta_service_double = instance_double(Spl::SpartaLoyaltyService) - it 'does not call Sparta loyalty service' do - expect(Spl::SpartaLoyaltyService).not_to receive(:new) + expect(Spl::SpartaLoyaltyService).to receive(:new) do |_, card_no, *_| + expect(card_no).to be_nil + sparta_service_double + end + + expect(sparta_service_double).to receive(:call).and_return(nil) expect(ApplySpartaDiscountService).not_to receive(:new) + expect(order).to receive(:reload).and_call_original + service.call end end - context 'when card number is missing' do - let(:public_metadata) do - { - spl_card_active: true - } - end + context 'when spl_card_active is missing and card number is missing' do + let(:public_metadata) { {} } - it 'does not call Sparta loyalty service' do - expect(Spl::SpartaLoyaltyService).not_to receive(:new) + it 'calls Sparta with empty card number and does not apply discounts' do + sparta_service_double = instance_double(Spl::SpartaLoyaltyService) + + expect(Spl::SpartaLoyaltyService).to receive(:new) do |_, card_no, *_| + expect(card_no).to eq('') + sparta_service_double + end + + expect(sparta_service_double).to receive(:call).and_return(nil) expect(ApplySpartaDiscountService).not_to receive(:new) + expect(order).to receive(:reload).and_call_original + service.call end end From 0814a1511c39db37e84eeb632280cec34995bd12 Mon Sep 17 00:00:00 2001 From: nika-piotrowska Date: Thu, 15 Jan 2026 21:49:14 +0100 Subject: [PATCH 3/6] Fix rubocop offenses --- .rubocop_todo.yml | 34 ++------ .../adjustments_updater_decorator.rb | 7 +- app/services/apply_sparta_discount_service.rb | 2 +- app/services/promotion_switcher_service.rb | 2 +- app/services/spl/login_account_service.rb | 2 +- .../adjustments_updater_decorator_spec.rb | 82 +++++++++---------- spec/models/payment_decorator_spec.rb | 12 +-- 7 files changed, 58 insertions(+), 83 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 72a56b3..cf2e5f6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,30 +1,22 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2025-12-17 20:33:32 UTC using RuboCop version 1.79.2. +# on 2026-01-15 20:48:43 UTC using RuboCop version 1.81.7. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 5 -# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. -# AllowedMethods: refine -Metrics/BlockLength: - Max: 261 - -# Offense count: 2 +# Offense count: 1 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. -# AllowedMethods: refine -Metrics/BlockLength: - Max: 103 +Metrics/MethodLength: + Max: 11 -# Offense count: 2 +# Offense count: 1 # Configuration parameters: Mode, AllowedMethods, AllowedPatterns, AllowBangMethods, WaywardPredicates. # AllowedMethods: call # WaywardPredicates: nonzero? Naming/PredicateMethod: Exclude: - - 'app/controllers/account_controller_decorator.rb' - 'app/services/assign_sparta_card_number_service.rb' # Offense count: 1 @@ -33,22 +25,6 @@ Rails/ApplicationJob: Exclude: - 'app/jobs/update_sparta_state_job.rb' -# Offense count: 3 -# This cop supports unsafe autocorrection (--autocorrect-all). -# Configuration parameters: NilOrEmpty, NotPresent, UnlessPresent. -Rails/Blank: - Exclude: - - 'app/controllers/account_controller_decorator.rb' - - 'app/controllers/cart_controller_decorator.rb' - -# Offense count: 2 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: EnforcedStyle. -# SupportedStyles: numeric, symbolic -Rails/HttpStatus: - Exclude: - - 'app/controllers/account_controller_decorator.rb' - # Offense count: 1 # This cop supports unsafe autocorrection (--autocorrect-all). Rails/NegateInclude: diff --git a/app/models/spree/adjustable/adjustments_updater_decorator.rb b/app/models/spree/adjustable/adjustments_updater_decorator.rb index cbae6ca..87bb98e 100644 --- a/app/models/spree/adjustable/adjustments_updater_decorator.rb +++ b/app/models/spree/adjustable/adjustments_updater_decorator.rb @@ -4,10 +4,11 @@ module Spree module Adjustable module AdjustmentsUpdaterDecorator SPL_SOURCE_TYPE = 'SPL' + private def set_spree_adjustments - adjustable = @adjustable.is_a?(::Spree::Order) ? @adjustable : @adjustable.order + @adjustable.is_a?(::Spree::Order) ? @adjustable : @adjustable.order end def line_item_with_spl_adjustments? @@ -15,7 +16,9 @@ def line_item_with_spl_adjustments? end def recalculate_spl_adjustments(attributes, totals) - sparta_adjustments = @adjustable.adjustments.select { |adj| adj.source_type == SPL_SOURCE_TYPE && adj.eligible? } + sparta_adjustments = @adjustable.adjustments.select do |adj| + adj.source_type == SPL_SOURCE_TYPE && adj.eligible? + end total_adjustment_amount = sparta_adjustments.sum(&:amount) assign_spl_totals(attributes, total_adjustment_amount, Time.current) @adjustable.update_columns(totals) diff --git a/app/services/apply_sparta_discount_service.rb b/app/services/apply_sparta_discount_service.rb index b4ccc51..3d977f7 100644 --- a/app/services/apply_sparta_discount_service.rb +++ b/app/services/apply_sparta_discount_service.rb @@ -19,7 +19,7 @@ def call # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity,Metrics/ next if sparta_item['discounts'].nil? label = "SPARTA_#{sparta_item&.fetch('discounts')&.first&.fetch('name')}_#{line_item.id}" # rubocop:disable Style/SafeNavigationChainLength - amount = -BigDecimal(sparta_item["discountGross"].to_s) # Negative value for discount + amount = -BigDecimal(sparta_item['discountGross'].to_s) # Negative value for discount discounts_present?(line_item, label) update_sparta_adjustment(line_item, label, amount) create_sparta_adjustment(order, amount, label, line_item) diff --git a/app/services/promotion_switcher_service.rb b/app/services/promotion_switcher_service.rb index 2c2e891..05aeb51 100644 --- a/app/services/promotion_switcher_service.rb +++ b/app/services/promotion_switcher_service.rb @@ -9,7 +9,7 @@ def initialize(order, check_only) @order = order end - def call # rubocop:disable Metrics/AbcSize + def call apply_sparta_discount(order, check_only) ensure order.reload diff --git a/app/services/spl/login_account_service.rb b/app/services/spl/login_account_service.rb index 3f985d9..998e116 100644 --- a/app/services/spl/login_account_service.rb +++ b/app/services/spl/login_account_service.rb @@ -33,7 +33,7 @@ def send_request(url, body) Spl::SendRequestService.new(url, body).call end - def prepare_login_body # rubocop:disable Metrics/MethodLength + def prepare_login_body { context: { prgCode: @store.private_metadata['spl_prg_code'] diff --git a/spec/models/adjustments_updater_decorator_spec.rb b/spec/models/adjustments_updater_decorator_spec.rb index a4bd2a1..d789f67 100644 --- a/spec/models/adjustments_updater_decorator_spec.rb +++ b/spec/models/adjustments_updater_decorator_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "rails_helper" +require 'rails_helper' RSpec.describe Spree::Adjustable::AdjustmentsUpdater, type: :model do # Use the upstream Spree approach to avoid store/product validation issues @@ -9,49 +9,49 @@ subject(:updater) { described_class.new(adjustable) } - describe "#line_item_with_spl_adjustments? (private)" do + describe '#line_item_with_spl_adjustments? (private)' do def line_item_with_spl_adjustments? updater.send(:line_item_with_spl_adjustments?) end - context "when adjustable is a line item with an SPL adjustment" do + context 'when adjustable is a line item with an SPL adjustment' do let(:adjustable) { line_item } before do - create(:adjustment, order: order, adjustable: line_item, source_type: "SPL", eligible: true, amount: -2.to_d) + create(:adjustment, order: order, adjustable: line_item, source_type: 'SPL', eligible: true, amount: -2.to_d) end - it "returns true" do + it 'returns true' do expect(line_item_with_spl_adjustments?).to eq(true) end end - context "when adjustable is a line item without SPL adjustments" do + context 'when adjustable is a line item without SPL adjustments' do let(:adjustable) { line_item } before do - create(:adjustment, order: order, adjustable: line_item, source_type: "Promo", eligible: true, amount: -2.to_d) + create(:adjustment, order: order, adjustable: line_item, source_type: 'Promo', eligible: true, amount: -2.to_d) end - it "returns false" do + it 'returns false' do expect(line_item_with_spl_adjustments?).to eq(false) end end - context "when adjustable is not a line item" do + context 'when adjustable is not a line item' do let(:adjustable) { order } before do - create(:adjustment, order: order, adjustable: order, source_type: "SPL", eligible: true, amount: -2.to_d) + create(:adjustment, order: order, adjustable: order, source_type: 'SPL', eligible: true, amount: -2.to_d) end - it "returns false" do + it 'returns false' do expect(line_item_with_spl_adjustments?).to eq(false) end end end - describe "#recalculate_spl_adjustments (private)" do + describe '#recalculate_spl_adjustments (private)' do def recalculate_spl_adjustments(attributes, totals) updater.send(:recalculate_spl_adjustments, attributes, totals) end @@ -60,47 +60,43 @@ def recalculate_spl_adjustments(attributes, totals) let!(:spl_adj1) do create(:adjustment, - order: order, - adjustable: line_item, - source_type: "SPL", - eligible: true, - amount: 10.to_d - ) + order: order, + adjustable: line_item, + source_type: 'SPL', + eligible: true, + amount: 10.to_d) end let!(:spl_adj2) do create(:adjustment, - order: order, - adjustable: line_item, - source_type: "SPL", - eligible: true, - amount: -5.to_d - ) + order: order, + adjustable: line_item, + source_type: 'SPL', + eligible: true, + amount: -5.to_d) end let!(:spl_ineligible) do create(:adjustment, - order: order, - adjustable: line_item, - source_type: "SPL", - eligible: false, - amount: 100.to_d - ) + order: order, + adjustable: line_item, + source_type: 'SPL', + eligible: false, + amount: 100.to_d) end let!(:other_adj) do create(:adjustment, - order: order, - adjustable: line_item, - source_type: "Promo", - eligible: true, - amount: 50.to_d - ) + order: order, + adjustable: line_item, + source_type: 'Promo', + eligible: true, + amount: 50.to_d) end let(:attributes) { {} } let(:totals_hash) { { some_total: 123.to_d } } - let(:fixed_time) { Time.zone.parse("2024-01-01 12:00:00") } + let(:fixed_time) { Time.zone.parse('2024-01-01 12:00:00') } let(:expected_total) { spl_adj1.amount + spl_adj2.amount } before do @@ -109,7 +105,7 @@ def recalculate_spl_adjustments(attributes, totals) allow(updater).to receive(:assign_spl_totals).and_call_original end - it "sums only eligible SPL adjustments and passes the sum to assign_spl_totals" do + it 'sums only eligible SPL adjustments and passes the sum to assign_spl_totals' do recalculate_spl_adjustments(attributes, totals_hash) expect(updater).to have_received(:assign_spl_totals).with( @@ -119,13 +115,13 @@ def recalculate_spl_adjustments(attributes, totals) ) end - it "updates the adjustable using update_columns with provided totals" do + it 'updates the adjustable using update_columns with provided totals' do recalculate_spl_adjustments(attributes, totals_hash) expect(line_item).to have_received(:update_columns).with(totals_hash) end - it "mutates attributes with adjustment_total, promo_total and updated_at" do + it 'mutates attributes with adjustment_total, promo_total and updated_at' do recalculate_spl_adjustments(attributes, totals_hash) expect(attributes[:adjustment_total]).to eq(expected_total) @@ -134,7 +130,7 @@ def recalculate_spl_adjustments(attributes, totals) end end - describe "#assign_spl_totals (private)" do + describe '#assign_spl_totals (private)' do def assign_spl_totals(attributes, total_amount, time) updater.send(:assign_spl_totals, attributes, total_amount, time) end @@ -142,9 +138,9 @@ def assign_spl_totals(attributes, total_amount, time) let(:adjustable) { line_item } let(:attributes) { {} } let(:total_amount) { 42.5.to_d } - let(:time) { Time.zone.parse("2024-02-02 10:00:00") } + let(:time) { Time.zone.parse('2024-02-02 10:00:00') } - it "sets adjustment_total, promo_total and updated_at" do + it 'sets adjustment_total, promo_total and updated_at' do assign_spl_totals(attributes, total_amount, time) expect(attributes[:adjustment_total]).to eq(total_amount) diff --git a/spec/models/payment_decorator_spec.rb b/spec/models/payment_decorator_spec.rb index f6212f4..e6f6f6c 100644 --- a/spec/models/payment_decorator_spec.rb +++ b/spec/models/payment_decorator_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "rails_helper" +require 'rails_helper' RSpec.describe Spree::Payment, type: :model do let(:store) { Spree::Store.default || create(:store, default: true) } @@ -9,11 +9,11 @@ subject(:payment) { described_class.new(order: order) } - describe "#promotion_switcher (private)" do + describe '#promotion_switcher (private)' do let(:service_result) { :some_result } let(:service_instance) { instance_double(PromotionSwitcherService, call: service_result) } - it "initializes PromotionSwitcherService with order + check_only and calls it" do + it 'initializes PromotionSwitcherService with order + check_only and calls it' do allow(PromotionSwitcherService).to receive(:new).and_return(service_instance) result = payment.send(:promotion_switcher, order, true) @@ -23,7 +23,7 @@ expect(result).to eq(service_result) end - it "passes false correctly as check_only flag" do + it 'passes false correctly as check_only flag' do allow(PromotionSwitcherService).to receive(:new).and_return(service_instance) payment.send(:promotion_switcher, order, false) @@ -32,7 +32,7 @@ end end - describe "#preform_update_sparta_state_job (private)" do + describe '#preform_update_sparta_state_job (private)' do it "enqueues UpdateSpartaStateJob with token, state 'D', number, store" do allow(UpdateSpartaStateJob).to receive(:perform_later) @@ -40,7 +40,7 @@ expect(UpdateSpartaStateJob).to have_received(:perform_later).with( order.token, - "D", + 'D', order.number, order.store ) From 5460a9e3b52fadd71459ef90f9a8eb71936bfcc3 Mon Sep 17 00:00:00 2001 From: nika-piotrowska Date: Mon, 19 Jan 2026 23:26:33 +0100 Subject: [PATCH 4/6] Improve prepare_card_number_if_exist method --- app/services/promotion_switcher_service.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/services/promotion_switcher_service.rb b/app/services/promotion_switcher_service.rb index 05aeb51..44dc9e8 100644 --- a/app/services/promotion_switcher_service.rb +++ b/app/services/promotion_switcher_service.rb @@ -40,10 +40,6 @@ def create_sparta_adjustments(spl_response, order) end def prepare_card_number_if_exist(metadata) - if cast_boolean(metadata[:spl_card_active]) - metadata['spl_no_card'] - else - '' - end + cast_boolean(metadata[:spl_card_active]) ? metadata['spl_no_card'] : '' end end From e2016f3c9957dc72a5fb644964c9fc73aa7881ac Mon Sep 17 00:00:00 2001 From: nika-piotrowska Date: Tue, 20 Jan 2026 13:04:21 +0100 Subject: [PATCH 5/6] Remove redundant line --- app/services/promotion_switcher_service.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/services/promotion_switcher_service.rb b/app/services/promotion_switcher_service.rb index 44dc9e8..9229458 100644 --- a/app/services/promotion_switcher_service.rb +++ b/app/services/promotion_switcher_service.rb @@ -11,7 +11,6 @@ def initialize(order, check_only) def call apply_sparta_discount(order, check_only) - ensure order.reload end From babe1ad558cbc6101fa935e672a214f778fca853 Mon Sep 17 00:00:00 2001 From: nika-piotrowska Date: Tue, 27 Jan 2026 18:16:28 +0100 Subject: [PATCH 6/6] Fix rubocop offense --- app/services/promotion_switcher_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/promotion_switcher_service.rb b/app/services/promotion_switcher_service.rb index d6f9d75..92a9500 100644 --- a/app/services/promotion_switcher_service.rb +++ b/app/services/promotion_switcher_service.rb @@ -9,7 +9,7 @@ def initialize(order, check_only) @order = order end - def call # rubocop:disable Metrics/AbcSize + def call apply_sparta_discount(order, check_only) rescue StandardError => e Rails.logger.error("[PromotionSwitcher] Failed for Order #{order.id}: #{e.message}")