From ce96b5880029bcb3151be48fd482f1283e1422fd Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 17 Feb 2026 13:38:41 +1100 Subject: [PATCH 01/10] Remove useless setting of payment amount It gets overridden later anyway. --- app/services/checkout/params.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/services/checkout/params.rb b/app/services/checkout/params.rb index a14a04656f..3175e76a6a 100644 --- a/app/services/checkout/params.rb +++ b/app/services/checkout/params.rb @@ -14,7 +14,6 @@ module Checkout apply_strong_parameters set_pickup_address set_address_details - set_payment_amount set_existing_card @order_params @@ -58,12 +57,6 @@ module Checkout end end - def set_payment_amount - return unless @order_params[:payments_attributes] - - @order_params[:payments_attributes].first[:amount] = order.outstanding_balance.amount - end - def set_existing_card return unless existing_card_selected? From 303b91af5e3cb77509ec9d886c895626a4902cdf Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 17 Feb 2026 14:07:11 +1100 Subject: [PATCH 02/10] Move list of payment actions from card to gateway We currently ask the credit card first which payment actions like "void" it supports. But all the logic is not card specifc. It depends on the payment method which actions it supports. And instead of having two different classes potentially being the source of truth for actions, I prefer leaving that responsibility with exactly one class, the payment method. I'll move the `can_?` methods next. --- app/models/spree/credit_card.rb | 4 ---- app/models/spree/gateway.rb | 4 ++++ app/models/spree/payment.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index 47fa1b8fca..19fc47bff3 100644 --- a/app/models/spree/credit_card.rb +++ b/app/models/spree/credit_card.rb @@ -63,10 +63,6 @@ module Spree "XXXX-XXXX-XXXX-#{last_digits}" end - def actions - %w{capture_and_complete_order void credit resend_authorization_email} - end - def can_resend_authorization_email?(payment) payment.requires_authorization? end diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb index 08f6d20e27..1ab4e8808f 100644 --- a/app/models/spree/gateway.rb +++ b/app/models/spree/gateway.rb @@ -13,6 +13,10 @@ module Spree preference :server, :string, default: 'live' preference :test_mode, :boolean, default: false + def actions + %w{capture_and_complete_order void credit resend_authorization_email} + end + def payment_source_class CreditCard end diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index ae224f44d0..ef5c4d67a5 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -152,9 +152,9 @@ module Spree end def actions - return [] unless payment_source.respond_to?(:actions) + return [] unless payment_method.respond_to?(:actions) - payment_source.actions.select do |action| + payment_method.actions.select do |action| !payment_source.respond_to?("can_#{action}?") || payment_source.__send__("can_#{action}?", self) end From 2b32f6b909cfbf9ac1c34d49bcac73c20b2ea6d8 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 17 Feb 2026 14:41:11 +1100 Subject: [PATCH 03/10] Make payment method source of truth of supported actions --- .../spree/admin/payments_controller.rb | 2 +- app/models/spree/credit_card.rb | 25 ---------- app/models/spree/gateway.rb | 25 ++++++++++ app/models/spree/payment.rb | 9 +--- spec/models/spree/credit_card_spec.rb | 47 ----------------- spec/models/spree/gateway_spec.rb | 50 ++++++++++++++++++- spec/requests/spree/admin/payments_spec.rb | 2 - 7 files changed, 76 insertions(+), 84 deletions(-) diff --git a/app/controllers/spree/admin/payments_controller.rb b/app/controllers/spree/admin/payments_controller.rb index b95070ccf2..7649a5af41 100644 --- a/app/controllers/spree/admin/payments_controller.rb +++ b/app/controllers/spree/admin/payments_controller.rb @@ -52,7 +52,7 @@ module Spree # (we can't use respond_override because Spree no longer uses respond_with) def fire event = params[:e] - return unless event && @payment.payment_source + return unless event # capture_and_complete_order will complete the order, so we want to try to redeem VINE # voucher first and exit if it fails diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index 19fc47bff3..5a6d8dfddc 100644 --- a/app/models/spree/credit_card.rb +++ b/app/models/spree/credit_card.rb @@ -63,31 +63,6 @@ module Spree "XXXX-XXXX-XXXX-#{last_digits}" end - def can_resend_authorization_email?(payment) - payment.requires_authorization? - end - - # Indicates whether its possible to capture the payment - def can_capture_and_complete_order?(payment) - return false if payment.requires_authorization? - - payment.pending? || payment.checkout? - end - - # Indicates whether its possible to void the payment. - def can_void?(payment) - !payment.void? - end - - # Indicates whether its possible to credit the payment. Note that most gateways require that the - # payment be settled first which generally happens within 12-24 hours of the transaction. - def can_credit?(payment) - return false unless payment.completed? - return false unless payment.order.payment_state == 'credit_owed' - - payment.credit_allowed.positive? - end - # Allows us to use a gateway_payment_profile_id to store Stripe Tokens def has_payment_profile? gateway_customer_profile_id.present? || gateway_payment_profile_id.present? diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb index 1ab4e8808f..e08a4e0ca0 100644 --- a/app/models/spree/gateway.rb +++ b/app/models/spree/gateway.rb @@ -17,6 +17,31 @@ module Spree %w{capture_and_complete_order void credit resend_authorization_email} end + # Indicates whether its possible to capture the payment + def can_capture_and_complete_order?(payment) + return false if payment.requires_authorization? + + payment.pending? || payment.checkout? + end + + # Indicates whether its possible to void the payment. + def can_void?(payment) + !payment.void? + end + + # Indicates whether its possible to credit the payment. Note that most gateways require that the + # payment be settled first which generally happens within 12-24 hours of the transaction. + def can_credit?(payment) + return false unless payment.completed? + return false unless payment.order.payment_state == 'credit_owed' + + payment.credit_allowed.positive? + end + + def can_resend_authorization_email?(payment) + payment.requires_authorization? + end + def payment_source_class CreditCard end diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index ef5c4d67a5..f397c43106 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -155,8 +155,8 @@ module Spree return [] unless payment_method.respond_to?(:actions) payment_method.actions.select do |action| - !payment_source.respond_to?("can_#{action}?") || - payment_source.__send__("can_#{action}?", self) + !payment_method.respond_to?("can_#{action}?") || + payment_method.__send__("can_#{action}?", self) end end @@ -166,11 +166,6 @@ module Spree PaymentMailer.authorize_payment(self).deliver_later end - def payment_source - res = source.is_a?(Payment) ? source.source : source - res || payment_method - end - def ensure_correct_adjustment revoke_adjustment_eligibility if ['failed', 'invalid', 'void'].include?(state) return if adjustment.try(:finalized?) diff --git a/spec/models/spree/credit_card_spec.rb b/spec/models/spree/credit_card_spec.rb index 29f1fdf753..7a7db93886 100644 --- a/spec/models/spree/credit_card_spec.rb +++ b/spec/models/spree/credit_card_spec.rb @@ -21,53 +21,6 @@ RSpec.describe Spree::CreditCard do let(:credit_card) { described_class.new } - context "#can_capture?" do - it "should be true if payment is pending" do - payment = build_stubbed(:payment, created_at: Time.zone.now) - allow(payment).to receive(:pending?) { true } - expect(credit_card.can_capture_and_complete_order?(payment)).to be_truthy - end - - it "should be true if payment is checkout" do - payment = build_stubbed(:payment, created_at: Time.zone.now) - allow(payment).to receive_messages pending?: false, - checkout?: true - expect(credit_card.can_capture_and_complete_order?(payment)).to be_truthy - end - end - - context "#can_void?" do - it "should be true if payment is not void" do - payment = build_stubbed(:payment) - allow(payment).to receive(:void?) { false } - expect(credit_card.can_void?(payment)).to be_truthy - end - end - - context "#can_credit?" do - it "should be false if payment is not completed" do - payment = build_stubbed(:payment) - allow(payment).to receive(:completed?) { false } - expect(credit_card.can_credit?(payment)).to be_falsy - end - - it "should be false when order payment_state is not 'credit_owed'" do - payment = build_stubbed(:payment, - order: create(:order, payment_state: 'paid')) - allow(payment).to receive(:completed?) { true } - expect(credit_card.can_credit?(payment)).to be_falsy - end - - it "should be false when credit_allowed is zero" do - payment = build_stubbed(:payment, - order: create(:order, payment_state: 'credit_owed')) - allow(payment).to receive_messages completed?: true, - credit_allowed: 0 - - expect(credit_card.can_credit?(payment)).to be_falsy - end - end - context "#valid?" do it "should validate presence of number" do credit_card.attributes = valid_credit_card_attributes.except(:number) diff --git a/spec/models/spree/gateway_spec.rb b/spec/models/spree/gateway_spec.rb index f6e975ba38..3ff9dc0741 100644 --- a/spec/models/spree/gateway_spec.rb +++ b/spec/models/spree/gateway_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true RSpec.describe Spree::Gateway do + subject(:gateway) { test_gateway.new } let(:test_gateway) do Class.new(Spree::Gateway) do def provider_class @@ -15,13 +16,58 @@ RSpec.describe Spree::Gateway do it "passes through all arguments on a method_missing call" do expect(Rails.env).to receive(:local?).and_return(false) - gateway = test_gateway.new expect(gateway.provider).to receive(:imaginary_method).with('foo') gateway.imaginary_method('foo') end it "raises an error in test env" do - gateway = test_gateway.new expect { gateway.imaginary_method('foo') }.to raise_error StandardError end + + describe "#can_capture?" do + it "should be true if payment is pending" do + payment = build_stubbed(:payment, created_at: Time.zone.now) + allow(payment).to receive(:pending?) { true } + expect(gateway.can_capture_and_complete_order?(payment)).to be_truthy + end + + it "should be true if payment is checkout" do + payment = build_stubbed(:payment, created_at: Time.zone.now) + allow(payment).to receive_messages pending?: false, + checkout?: true + expect(gateway.can_capture_and_complete_order?(payment)).to be_truthy + end + end + + describe "#can_void?" do + it "should be true if payment is not void" do + payment = build_stubbed(:payment) + allow(payment).to receive(:void?) { false } + expect(gateway.can_void?(payment)).to be_truthy + end + end + + describe "#can_credit?" do + it "should be false if payment is not completed" do + payment = build_stubbed(:payment) + allow(payment).to receive(:completed?) { false } + expect(gateway.can_credit?(payment)).to be_falsy + end + + it "should be false when order payment_state is not 'credit_owed'" do + payment = build_stubbed(:payment, + order: create(:order, payment_state: 'paid')) + allow(payment).to receive(:completed?) { true } + expect(gateway.can_credit?(payment)).to be_falsy + end + + it "should be false when credit_allowed is zero" do + payment = build_stubbed(:payment, + order: create(:order, payment_state: 'credit_owed')) + allow(payment).to receive_messages completed?: true, + credit_allowed: 0 + + expect(gateway.can_credit?(payment)).to be_falsy + end + end end diff --git a/spec/requests/spree/admin/payments_spec.rb b/spec/requests/spree/admin/payments_spec.rb index fc4abdaf0f..283daf16e1 100644 --- a/spec/requests/spree/admin/payments_spec.rb +++ b/spec/requests/spree/admin/payments_spec.rb @@ -157,8 +157,6 @@ RSpec.describe Spree::Admin::PaymentsController do context "with no payment source" do it "redirect to payments page" do - allow(payment).to receive(:payment_source).and_return(nil) - put( "/admin/orders/#{order.number}/payments/#{order.payments.first.id}/fire?e=void", params: {}, From 956c4a27c221e7ba07287f73715f48261debff37 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 17 Feb 2026 14:43:18 +1100 Subject: [PATCH 04/10] Remove unnecessary guard clause Previously, payment actions that were listed without an associated `can_?` method were interpreted as supported. All payment methods are implementing all `can_?` methods for listed actions though. And I think that new payment methods should explicitely implement all `can_?` methods instead of relying on this hidden logic. --- app/models/spree/payment.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index f397c43106..7953d71a3c 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -155,8 +155,7 @@ module Spree return [] unless payment_method.respond_to?(:actions) payment_method.actions.select do |action| - !payment_method.respond_to?("can_#{action}?") || - payment_method.__send__("can_#{action}?", self) + payment_method.__send__("can_#{action}?", self) end end From fdd22bc09731ad47400fbdc2cccfedbc47f525ed Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 17 Feb 2026 16:16:31 +1100 Subject: [PATCH 05/10] Move payment action logic to payment At closer inspection, almost all logic around which payment actions to display involves only the state of the payment. So I moved the logic there. We now have one list of all possible actions supported by the UX. Then payment methods can declare a list of supported actions. If that's conditional, they can implement the conditions themselves. The payment model itself then still filters the actions based on its state. --- app/models/spree/gateway.rb | 25 ------------ app/models/spree/payment.rb | 37 ++++++++++++++++-- app/models/spree/payment_method/check.rb | 10 ----- app/models/spree/payment_method/taler.rb | 4 -- spec/models/spree/gateway_spec.rb | 47 ---------------------- spec/models/spree/payment_spec.rb | 50 +++++++++++++++++++++++- 6 files changed, 82 insertions(+), 91 deletions(-) diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb index e08a4e0ca0..1ab4e8808f 100644 --- a/app/models/spree/gateway.rb +++ b/app/models/spree/gateway.rb @@ -17,31 +17,6 @@ module Spree %w{capture_and_complete_order void credit resend_authorization_email} end - # Indicates whether its possible to capture the payment - def can_capture_and_complete_order?(payment) - return false if payment.requires_authorization? - - payment.pending? || payment.checkout? - end - - # Indicates whether its possible to void the payment. - def can_void?(payment) - !payment.void? - end - - # Indicates whether its possible to credit the payment. Note that most gateways require that the - # payment be settled first which generally happens within 12-24 hours of the transaction. - def can_credit?(payment) - return false unless payment.completed? - return false unless payment.order.payment_state == 'credit_owed' - - payment.credit_allowed.positive? - end - - def can_resend_authorization_email?(payment) - payment.requires_authorization? - end - def payment_source_class CreditCard end diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 7953d71a3c..f8aef25daa 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -11,6 +11,13 @@ module Spree localize_number :amount + # All possible actions offered to shop owners. + ACTIONS = %w[ + capture_and_complete_order + void + credit + resend_authorization_email + ].freeze IDENTIFIER_CHARS = (('A'..'Z').to_a + ('0'..'9').to_a - %w(0 1 I O)).freeze delegate :line_items, to: :order @@ -152,11 +159,33 @@ module Spree end def actions - return [] unless payment_method.respond_to?(:actions) + supported = ACTIONS & payment_method.actions.to_a + supported.select { public_send("can_#{it}?") } + end - payment_method.actions.select do |action| - payment_method.__send__("can_#{action}?", self) - end + # Indicates whether its possible to capture the payment + def can_capture_and_complete_order? + return false if requires_authorization? + + pending? || checkout? + end + + # Indicates whether its possible to void the payment. + def can_void? + !void? + end + + # Indicates whether its possible to credit the payment. Note that most gateways require that the + # payment be settled first which generally happens within 12-24 hours of the transaction. + def can_credit? + return false unless completed? + return false unless order.payment_state == "credit_owed" + + credit_allowed.positive? + end + + def can_resend_authorization_email? + requires_authorization? end def resend_authorization_email! diff --git a/app/models/spree/payment_method/check.rb b/app/models/spree/payment_method/check.rb index 839698bebc..0a931f9f17 100644 --- a/app/models/spree/payment_method/check.rb +++ b/app/models/spree/payment_method/check.rb @@ -7,16 +7,6 @@ module Spree %w{capture_and_complete_order void} end - # Indicates whether its possible to capture the payment - def can_capture_and_complete_order?(payment) - ['checkout', 'pending'].include?(payment.state) - end - - # Indicates whether its possible to void the payment. - def can_void?(payment) - payment.state != 'void' - end - def capture(*_args) ActiveMerchant::Billing::Response.new(true, "", {}, {}) end diff --git a/app/models/spree/payment_method/taler.rb b/app/models/spree/payment_method/taler.rb index 479f06811e..9fa8d00caf 100644 --- a/app/models/spree/payment_method/taler.rb +++ b/app/models/spree/payment_method/taler.rb @@ -25,10 +25,6 @@ module Spree %w{void} end - def can_void?(payment) - payment.state == "completed" - end - # Name of the view to display during checkout def method_type "check" # empty view diff --git a/spec/models/spree/gateway_spec.rb b/spec/models/spree/gateway_spec.rb index 3ff9dc0741..41f81bdd84 100644 --- a/spec/models/spree/gateway_spec.rb +++ b/spec/models/spree/gateway_spec.rb @@ -23,51 +23,4 @@ RSpec.describe Spree::Gateway do it "raises an error in test env" do expect { gateway.imaginary_method('foo') }.to raise_error StandardError end - - describe "#can_capture?" do - it "should be true if payment is pending" do - payment = build_stubbed(:payment, created_at: Time.zone.now) - allow(payment).to receive(:pending?) { true } - expect(gateway.can_capture_and_complete_order?(payment)).to be_truthy - end - - it "should be true if payment is checkout" do - payment = build_stubbed(:payment, created_at: Time.zone.now) - allow(payment).to receive_messages pending?: false, - checkout?: true - expect(gateway.can_capture_and_complete_order?(payment)).to be_truthy - end - end - - describe "#can_void?" do - it "should be true if payment is not void" do - payment = build_stubbed(:payment) - allow(payment).to receive(:void?) { false } - expect(gateway.can_void?(payment)).to be_truthy - end - end - - describe "#can_credit?" do - it "should be false if payment is not completed" do - payment = build_stubbed(:payment) - allow(payment).to receive(:completed?) { false } - expect(gateway.can_credit?(payment)).to be_falsy - end - - it "should be false when order payment_state is not 'credit_owed'" do - payment = build_stubbed(:payment, - order: create(:order, payment_state: 'paid')) - allow(payment).to receive(:completed?) { true } - expect(gateway.can_credit?(payment)).to be_falsy - end - - it "should be false when credit_allowed is zero" do - payment = build_stubbed(:payment, - order: create(:order, payment_state: 'credit_owed')) - allow(payment).to receive_messages completed?: true, - credit_allowed: 0 - - expect(gateway.can_credit?(payment)).to be_falsy - end - end end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index cc6e8b029c..48dd75df95 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -855,7 +855,8 @@ RSpec.describe Spree::Payment do describe "available actions" do context "for most gateways" do - let(:payment) { build_stubbed(:payment, source: build_stubbed(:credit_card)) } + let(:payment) { build_stubbed(:payment, payment_method:) } + let(:payment_method) { Spree::Gateway::StripeSCA.new } it "can capture and void" do expect(payment.actions).to match_array %w(capture_and_complete_order void) @@ -872,6 +873,53 @@ RSpec.describe Spree::Payment do end end end + + describe "#can_capture_and_complete_order?" do + it "should be true if payment is pending" do + payment = build_stubbed(:payment, created_at: Time.zone.now) + allow(payment).to receive(:pending?) { true } + expect(payment.can_capture_and_complete_order?).to be_truthy + end + + it "should be true if payment is checkout" do + payment = build_stubbed(:payment, created_at: Time.zone.now) + allow(payment).to receive_messages pending?: false, + checkout?: true + expect(payment.can_capture_and_complete_order?).to be_truthy + end + end + + describe "#can_void?" do + it "should be true if payment is not void" do + payment = build_stubbed(:payment) + allow(payment).to receive(:void?) { false } + expect(payment.can_void?).to be_truthy + end + end + + describe "#can_credit?" do + it "should be false if payment is not completed" do + payment = build_stubbed(:payment) + allow(payment).to receive(:completed?) { false } + expect(payment.can_credit?).to be_falsy + end + + it "should be false when order payment_state is not 'credit_owed'" do + payment = build_stubbed(:payment, + order: create(:order, payment_state: 'paid')) + allow(payment).to receive(:completed?) { true } + expect(payment.can_credit?).to be_falsy + end + + it "should be false when credit_allowed is zero" do + payment = build_stubbed(:payment, + order: create(:order, payment_state: 'credit_owed')) + allow(payment).to receive_messages completed?: true, + credit_allowed: 0 + + expect(payment.can_credit?).to be_falsy + end + end end describe "refund!" do From cf53ac199021e3b821039c2cf7ca6bd8aa6e3b37 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 17 Feb 2026 16:39:48 +1100 Subject: [PATCH 06/10] Add credit action to Taler --- app/models/spree/payment_method/taler.rb | 17 +++++++- .../models/spree/payment_method/taler_spec.rb | 40 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/app/models/spree/payment_method/taler.rb b/app/models/spree/payment_method/taler.rb index 9fa8d00caf..777b3f3090 100644 --- a/app/models/spree/payment_method/taler.rb +++ b/app/models/spree/payment_method/taler.rb @@ -22,7 +22,7 @@ module Spree preference :api_key, :password def actions - %w{void} + %w[credit void] end # Name of the view to display during checkout @@ -64,6 +64,21 @@ module Spree ActiveMerchant::Billing::Response.new(success, message) end + def credit(money, gateway_options) + payment = gateway_options[:payment] + taler_order = taler_order(id: payment.response_code) + status = taler_order.fetch("order_status") + + raise "Unsupported action" if status != "paid" + + amount = "KUDOS:#{money}" + taler_order.refund(refund: amount, reason: "credit") + + PaymentMailer.refund_available(payment, taler_order.status_url).deliver_later + + ActiveMerchant::Billing::Response.new(true, "Refund initiated") + end + def void(response_code, gateway_options) payment = gateway_options[:payment] taler_order = taler_order(id: response_code) diff --git a/spec/models/spree/payment_method/taler_spec.rb b/spec/models/spree/payment_method/taler_spec.rb index fc52541693..3b44f82a8d 100644 --- a/spec/models/spree/payment_method/taler_spec.rb +++ b/spec/models/spree/payment_method/taler_spec.rb @@ -50,6 +50,46 @@ RSpec.describe Spree::PaymentMethod::Taler do end end + describe "#credit" do + let(:order_endpoint) { "#{backend_url}/private/orders/taler-order-8" } + let(:refund_endpoint) { "#{order_endpoint}/refund" } + let(:taler_refund_uri) { + "taler://refund/backend.demo.taler.net/instances/sandbox/taler-order-8/" + } + + it "starts the refund process" do + order_status = { order_status: "paid" } + stub_request(:get, order_endpoint).to_return(body: order_status.to_json) + stub_request(:post, refund_endpoint).to_return(body: { taler_refund_uri: }.to_json) + + order = create(:completed_order_with_totals) + order.payments.create( + amount: order.total, state: :completed, + payment_method: taler, + response_code: "taler-order-8", + ) + expect { + response = taler.credit(100, { payment: order.payments[0] }) + expect(response.success?).to eq true + }.to enqueue_mail(PaymentMailer, :refund_available) + end + + it "raises an error if payment hasn't been taken yet" do + order_status = { order_status: "claimed" } + stub_request(:get, order_endpoint).to_return(body: order_status.to_json) + + order = create(:completed_order_with_totals) + order.payments.create( + amount: order.total, state: :completed, + payment_method: taler, + response_code: "taler-order-8", + ) + expect { + taler.credit(100, { payment: order.payments[0] }) + }.to raise_error StandardError, "Unsupported action" + end + end + describe "#void" do let(:order_endpoint) { "#{backend_url}/private/orders/taler-order-8" } let(:refund_endpoint) { "#{order_endpoint}/refund" } From fb2dfed6bf0f9a7bcde39255362fb67407369ef9 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 25 Feb 2026 14:01:00 +1100 Subject: [PATCH 07/10] Pass amount to payment mailer We now include the refunded amount in the email to the customer. When voiding a payment, it's the full amount of the payment. When giving back credit then it's only the money that has been paid too much. --- app/mailers/payment_mailer.rb | 4 ++-- app/models/spree/payment_method/taler.rb | 11 +++++++---- spec/mailers/payment_mailer_spec.rb | 2 +- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/mailers/payment_mailer.rb b/app/mailers/payment_mailer.rb index b90d4c2191..0968224341 100644 --- a/app/mailers/payment_mailer.rb +++ b/app/mailers/payment_mailer.rb @@ -22,10 +22,10 @@ class PaymentMailer < ApplicationMailer end end - def refund_available(payment, taler_order_status_url) + def refund_available(amount, payment, taler_order_status_url) @order = payment.order @shop = @order.distributor.name - @amount = payment.display_amount + @amount = amount @taler_order_status_url = taler_order_status_url I18n.with_locale valid_locale(@order.user) do diff --git a/app/models/spree/payment_method/taler.rb b/app/models/spree/payment_method/taler.rb index 777b3f3090..2ee18757e6 100644 --- a/app/models/spree/payment_method/taler.rb +++ b/app/models/spree/payment_method/taler.rb @@ -65,16 +65,18 @@ module Spree end def credit(money, gateway_options) + amount = money / 100 # called with cents payment = gateway_options[:payment] taler_order = taler_order(id: payment.response_code) status = taler_order.fetch("order_status") raise "Unsupported action" if status != "paid" - amount = "KUDOS:#{money}" - taler_order.refund(refund: amount, reason: "credit") + taler_amount = "KUDOS:#{amount}" + taler_order.refund(refund: taler_amount, reason: "credit") - PaymentMailer.refund_available(payment, taler_order.status_url).deliver_later + spree_money = Spree::Money.new(amount, currency: payment.currency).to_s + PaymentMailer.refund_available(spree_money, payment, taler_order.status_url).deliver_later ActiveMerchant::Billing::Response.new(true, "Refund initiated") end @@ -93,7 +95,8 @@ module Spree amount = taler_order.fetch("contract_terms")["amount"] taler_order.refund(refund: amount, reason: "void") - PaymentMailer.refund_available(payment, taler_order.status_url).deliver_later + spree_money = payment.money.to_s + PaymentMailer.refund_available(spree_money, payment, taler_order.status_url).deliver_later ActiveMerchant::Billing::Response.new(true, "Refund initiated") end diff --git a/spec/mailers/payment_mailer_spec.rb b/spec/mailers/payment_mailer_spec.rb index dcce998d6e..f5cdae6c21 100644 --- a/spec/mailers/payment_mailer_spec.rb +++ b/spec/mailers/payment_mailer_spec.rb @@ -56,7 +56,7 @@ RSpec.describe PaymentMailer do payment = build(:payment) payment.order.distributor = build(:enterprise, name: "Carrot Castle") link = "https://taler.example.com/order/1" - mail = PaymentMailer.refund_available(payment, link) + mail = PaymentMailer.refund_available(payment.money.to_s, payment, link) expect(mail.subject).to eq "Refund from Carrot Castle" expect(mail.body).to include "Your payment of $45.75 to Carrot Castle is being refunded." From 7619062ad2b86b395059bb9d826817073d4b8cde Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 11 Mar 2026 11:09:35 +1100 Subject: [PATCH 08/10] Revert "Move payment action logic to payment" This reverts commit fdd22bc09731ad47400fbdc2cccfedbc47f525ed. And it adds the now needed `can_credit?` method to Taler. It's just a duplicate. --- app/models/spree/gateway.rb | 25 +++++++++++++ app/models/spree/payment.rb | 37 ++----------------- app/models/spree/payment_method/check.rb | 10 +++++ app/models/spree/payment_method/taler.rb | 11 ++++++ spec/models/spree/gateway_spec.rb | 47 ++++++++++++++++++++++++ spec/models/spree/payment_spec.rb | 47 ------------------------ 6 files changed, 97 insertions(+), 80 deletions(-) diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb index 1ab4e8808f..e08a4e0ca0 100644 --- a/app/models/spree/gateway.rb +++ b/app/models/spree/gateway.rb @@ -17,6 +17,31 @@ module Spree %w{capture_and_complete_order void credit resend_authorization_email} end + # Indicates whether its possible to capture the payment + def can_capture_and_complete_order?(payment) + return false if payment.requires_authorization? + + payment.pending? || payment.checkout? + end + + # Indicates whether its possible to void the payment. + def can_void?(payment) + !payment.void? + end + + # Indicates whether its possible to credit the payment. Note that most gateways require that the + # payment be settled first which generally happens within 12-24 hours of the transaction. + def can_credit?(payment) + return false unless payment.completed? + return false unless payment.order.payment_state == 'credit_owed' + + payment.credit_allowed.positive? + end + + def can_resend_authorization_email?(payment) + payment.requires_authorization? + end + def payment_source_class CreditCard end diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index f8aef25daa..7953d71a3c 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -11,13 +11,6 @@ module Spree localize_number :amount - # All possible actions offered to shop owners. - ACTIONS = %w[ - capture_and_complete_order - void - credit - resend_authorization_email - ].freeze IDENTIFIER_CHARS = (('A'..'Z').to_a + ('0'..'9').to_a - %w(0 1 I O)).freeze delegate :line_items, to: :order @@ -159,33 +152,11 @@ module Spree end def actions - supported = ACTIONS & payment_method.actions.to_a - supported.select { public_send("can_#{it}?") } - end + return [] unless payment_method.respond_to?(:actions) - # Indicates whether its possible to capture the payment - def can_capture_and_complete_order? - return false if requires_authorization? - - pending? || checkout? - end - - # Indicates whether its possible to void the payment. - def can_void? - !void? - end - - # Indicates whether its possible to credit the payment. Note that most gateways require that the - # payment be settled first which generally happens within 12-24 hours of the transaction. - def can_credit? - return false unless completed? - return false unless order.payment_state == "credit_owed" - - credit_allowed.positive? - end - - def can_resend_authorization_email? - requires_authorization? + payment_method.actions.select do |action| + payment_method.__send__("can_#{action}?", self) + end end def resend_authorization_email! diff --git a/app/models/spree/payment_method/check.rb b/app/models/spree/payment_method/check.rb index 0a931f9f17..839698bebc 100644 --- a/app/models/spree/payment_method/check.rb +++ b/app/models/spree/payment_method/check.rb @@ -7,6 +7,16 @@ module Spree %w{capture_and_complete_order void} end + # Indicates whether its possible to capture the payment + def can_capture_and_complete_order?(payment) + ['checkout', 'pending'].include?(payment.state) + end + + # Indicates whether its possible to void the payment. + def can_void?(payment) + payment.state != 'void' + end + def capture(*_args) ActiveMerchant::Billing::Response.new(true, "", {}, {}) end diff --git a/app/models/spree/payment_method/taler.rb b/app/models/spree/payment_method/taler.rb index 2ee18757e6..fb89e42a1d 100644 --- a/app/models/spree/payment_method/taler.rb +++ b/app/models/spree/payment_method/taler.rb @@ -25,6 +25,17 @@ module Spree %w[credit void] end + def can_void?(payment) + payment.state == "completed" + end + + def can_credit?(payment) + return false unless payment.completed? + return false unless payment.order.payment_state == 'credit_owed' + + payment.credit_allowed.positive? + end + # Name of the view to display during checkout def method_type "check" # empty view diff --git a/spec/models/spree/gateway_spec.rb b/spec/models/spree/gateway_spec.rb index 41f81bdd84..3ff9dc0741 100644 --- a/spec/models/spree/gateway_spec.rb +++ b/spec/models/spree/gateway_spec.rb @@ -23,4 +23,51 @@ RSpec.describe Spree::Gateway do it "raises an error in test env" do expect { gateway.imaginary_method('foo') }.to raise_error StandardError end + + describe "#can_capture?" do + it "should be true if payment is pending" do + payment = build_stubbed(:payment, created_at: Time.zone.now) + allow(payment).to receive(:pending?) { true } + expect(gateway.can_capture_and_complete_order?(payment)).to be_truthy + end + + it "should be true if payment is checkout" do + payment = build_stubbed(:payment, created_at: Time.zone.now) + allow(payment).to receive_messages pending?: false, + checkout?: true + expect(gateway.can_capture_and_complete_order?(payment)).to be_truthy + end + end + + describe "#can_void?" do + it "should be true if payment is not void" do + payment = build_stubbed(:payment) + allow(payment).to receive(:void?) { false } + expect(gateway.can_void?(payment)).to be_truthy + end + end + + describe "#can_credit?" do + it "should be false if payment is not completed" do + payment = build_stubbed(:payment) + allow(payment).to receive(:completed?) { false } + expect(gateway.can_credit?(payment)).to be_falsy + end + + it "should be false when order payment_state is not 'credit_owed'" do + payment = build_stubbed(:payment, + order: create(:order, payment_state: 'paid')) + allow(payment).to receive(:completed?) { true } + expect(gateway.can_credit?(payment)).to be_falsy + end + + it "should be false when credit_allowed is zero" do + payment = build_stubbed(:payment, + order: create(:order, payment_state: 'credit_owed')) + allow(payment).to receive_messages completed?: true, + credit_allowed: 0 + + expect(gateway.can_credit?(payment)).to be_falsy + end + end end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 48dd75df95..a59243096a 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -873,53 +873,6 @@ RSpec.describe Spree::Payment do end end end - - describe "#can_capture_and_complete_order?" do - it "should be true if payment is pending" do - payment = build_stubbed(:payment, created_at: Time.zone.now) - allow(payment).to receive(:pending?) { true } - expect(payment.can_capture_and_complete_order?).to be_truthy - end - - it "should be true if payment is checkout" do - payment = build_stubbed(:payment, created_at: Time.zone.now) - allow(payment).to receive_messages pending?: false, - checkout?: true - expect(payment.can_capture_and_complete_order?).to be_truthy - end - end - - describe "#can_void?" do - it "should be true if payment is not void" do - payment = build_stubbed(:payment) - allow(payment).to receive(:void?) { false } - expect(payment.can_void?).to be_truthy - end - end - - describe "#can_credit?" do - it "should be false if payment is not completed" do - payment = build_stubbed(:payment) - allow(payment).to receive(:completed?) { false } - expect(payment.can_credit?).to be_falsy - end - - it "should be false when order payment_state is not 'credit_owed'" do - payment = build_stubbed(:payment, - order: create(:order, payment_state: 'paid')) - allow(payment).to receive(:completed?) { true } - expect(payment.can_credit?).to be_falsy - end - - it "should be false when credit_allowed is zero" do - payment = build_stubbed(:payment, - order: create(:order, payment_state: 'credit_owed')) - allow(payment).to receive_messages completed?: true, - credit_allowed: 0 - - expect(payment.can_credit?).to be_falsy - end - end end describe "refund!" do From 53c2ef53d55e36d955cd65559cb2457db3995cf3 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 13 Mar 2026 16:24:41 +1100 Subject: [PATCH 09/10] Call #credit with right arguments --- app/models/spree/payment_method/taler.rb | 4 +-- .../models/spree/payment_method/taler_spec.rb | 4 +-- spec/system/admin/payments_taler_spec.rb | 33 ++++++++++++++++++- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/app/models/spree/payment_method/taler.rb b/app/models/spree/payment_method/taler.rb index fb89e42a1d..7c4953eac6 100644 --- a/app/models/spree/payment_method/taler.rb +++ b/app/models/spree/payment_method/taler.rb @@ -75,10 +75,10 @@ module Spree ActiveMerchant::Billing::Response.new(success, message) end - def credit(money, gateway_options) + def credit(money, response_code, gateway_options) amount = money / 100 # called with cents payment = gateway_options[:payment] - taler_order = taler_order(id: payment.response_code) + taler_order = taler_order(id: response_code) status = taler_order.fetch("order_status") raise "Unsupported action" if status != "paid" diff --git a/spec/models/spree/payment_method/taler_spec.rb b/spec/models/spree/payment_method/taler_spec.rb index 3b44f82a8d..a9e87bb0b6 100644 --- a/spec/models/spree/payment_method/taler_spec.rb +++ b/spec/models/spree/payment_method/taler_spec.rb @@ -69,7 +69,7 @@ RSpec.describe Spree::PaymentMethod::Taler do response_code: "taler-order-8", ) expect { - response = taler.credit(100, { payment: order.payments[0] }) + response = taler.credit(100, "taler-order-8", { payment: order.payments[0] }) expect(response.success?).to eq true }.to enqueue_mail(PaymentMailer, :refund_available) end @@ -85,7 +85,7 @@ RSpec.describe Spree::PaymentMethod::Taler do response_code: "taler-order-8", ) expect { - taler.credit(100, { payment: order.payments[0] }) + taler.credit(100, "taler-order-8", { payment: order.payments[0] }) }.to raise_error StandardError, "Unsupported action" end end diff --git a/spec/system/admin/payments_taler_spec.rb b/spec/system/admin/payments_taler_spec.rb index b6ab367ba0..bafabe2c89 100644 --- a/spec/system/admin/payments_taler_spec.rb +++ b/spec/system/admin/payments_taler_spec.rb @@ -25,7 +25,7 @@ RSpec.describe "Admin -> Order -> Payments" do login_as distributor.owner end - it "allows to refund a Taler payment" do + it "allows to void a Taler payment" do order_status = { order_status: "paid", contract_terms: { @@ -49,4 +49,35 @@ RSpec.describe "Admin -> Order -> Payments" do expect(page).not_to have_link "Void" end end + + it "allows to credit a Taler payment" do + order_status = { + order_status: "paid", + contract_terms: { + amount: "KUDOS:2", + } + } + order_endpoint = "https://taler.example.com/private/orders/taler-id-1" + refund_endpoint = "https://taler.example.com/private/orders/taler-id-1/refund" + stub_request(:get, order_endpoint).to_return(body: order_status.to_json) + stub_request(:post, refund_endpoint).to_return(body: "{}") + + visit spree.admin_order_payments_path(order.number) + + within row_containing("Taler") do + expect(page).to have_text "COMPLETED" + expect(page).to have_link "Credit" + + click_link class: "icon-credit" + + expect(page).to have_text "COMPLETED" + expect(page).not_to have_link "Credit" + end + + # Our payment system creates a new payment to show the credit. + within row_containing("$-9.75") do + # TODO: don't offer to void a credited amount. + expect(page).to have_link "Void" + end + end end From 9961578fc12cddc7f219f224bec4ed4c318655e5 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 16 Mar 2026 12:37:52 +1100 Subject: [PATCH 10/10] Don't offer to void a refund --- app/models/spree/payment_method/taler.rb | 4 +++- spec/system/admin/payments_taler_spec.rb | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/spree/payment_method/taler.rb b/app/models/spree/payment_method/taler.rb index 7c4953eac6..f991f289ef 100644 --- a/app/models/spree/payment_method/taler.rb +++ b/app/models/spree/payment_method/taler.rb @@ -26,7 +26,9 @@ module Spree end def can_void?(payment) - payment.state == "completed" + # The source can be another payment. Then this is an offset payment + # like a credit record. We can't void a refund. + payment.source == self && payment.state == "completed" end def can_credit?(payment) diff --git a/spec/system/admin/payments_taler_spec.rb b/spec/system/admin/payments_taler_spec.rb index bafabe2c89..d5db87393f 100644 --- a/spec/system/admin/payments_taler_spec.rb +++ b/spec/system/admin/payments_taler_spec.rb @@ -76,8 +76,7 @@ RSpec.describe "Admin -> Order -> Payments" do # Our payment system creates a new payment to show the credit. within row_containing("$-9.75") do - # TODO: don't offer to void a credited amount. - expect(page).to have_link "Void" + expect(page).not_to have_link "Void" end end end