raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-02-26 01:33:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'master' into add-report-name-and-details

Browse Source
This commit is contained in:
Gareth Rogers
2025-10-01 09:54:51 -04:00
committed by GitHub
parent 0a9eb173ea ef4d3ec138
commit f414e04dea
24 changed files with 319 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
app/controllers/payments_controller.rb
View File

@@ -9,11 +9,7 @@ class PaymentsController < BaseController
@payment = Spree::Payment.find(params[:id])
authorize! :show, @payment.order
if (url = @payment.cvv_response_message)
redirect_to url
else
redirect_to order_url(@payment.order)
end
redirect_to(@payment.redirect_auth_url || order_url(@payment.order))
end
private

4
app/models/spree/payment.rb
View File

@@ -57,7 +57,7 @@ module Spree
scope :failed, -> { with_state('failed') }
scope :valid, -> { where.not(state: %w(failed invalid)) }
scope :void, -> { with_state('void') }
scope :authorization_action_required, -> { where.not(cvv_response_message: nil) }
scope :authorization_action_required, -> { where.not(redirect_auth_url: nil) }
scope :requires_authorization, -> { with_state("requires_authorization") }
scope :with_payment_intent, ->(code) { where(response_code: code) }
@@ -164,7 +164,7 @@ module Spree
end
def clear_authorization_url
update_attribute(:cvv_response_message, nil)
update_attribute(:redirect_auth_url, nil)
end
private

3
app/models/spree/payment/processing.rb
View File

@@ -241,7 +241,8 @@ module Spree
if response.cvv_result
self.cvv_response_code = response.cvv_result['code']
self.cvv_response_message = response.cvv_result['message']
if cvv_response_message.present?
self.redirect_auth_url = response.cvv_result['redirect_auth_url']
if redirect_auth_url.present?
return require_authorization!
end
end

2
app/serializers/api/payment_serializer.rb
View File

@@ -2,7 +2,7 @@
module Api
class PaymentSerializer < ActiveModel::Serializer
attributes :amount, :updated_at, :payment_method, :state, :cvv_response_message
attributes :amount, :updated_at, :payment_method, :state, :redirect_auth_url
def payment_method
object.payment_method.try(:name)

2
app/services/checkout/stripe_redirect.rb
View File

@@ -40,7 +40,7 @@ module Checkout
# Stripe::AuthorizeResponsePatcher patches the Stripe authorization response
# so that this field stores the redirect URL. It also verifies that it is a Stripe URL.
def stripe_payment_url(payment)
payment.cvv_response_message
payment.redirect_auth_url
end
end
end

2
app/services/process_payment_intent.rb
View File

@@ -5,7 +5,7 @@
# /checkout; for admin payments and subscription payemnts it's the order url.
#
# This class confirms that the payment intent matches what's in our database,
# marks the payment as complete, and removes the cvv_response_message field,
# marks the payment as complete, and removes the redirect_auth_url field,
# which we use to indicate that authorization is required. It also completes the
# Order, if appropriate.

2
app/views/spree/users/_transactions.html.haml
View File

@@ -16,7 +16,7 @@
%td
= payment.updated_at.strftime("%Y-%m-%d")
%td
%a{ href: "#{payment.cvv_response_message}" }
%a{ href: "#{payment.redirect_auth_url}" }
%button.x-small
= t(".authorise")
%td.text-right

7
db/migrate/20250820224718_add_redirect_auth_url_in_payment_model.rb Normal file
View File

@@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddRedirectAuthUrlInPaymentModel < ActiveRecord::Migration[7.1]
def change
add_column :spree_payments, :redirect_auth_url, :string
end
end

27
db/migrate/20250827205335_migrate_cvv_message_to_redirect_auth_url.rb Normal file
View File

@@ -0,0 +1,27 @@
# frozen_string_literal: true
class MigrateCvvMessageToRedirectAuthUrl < ActiveRecord::Migration[7.1]
class SpreePayment < ActiveRecord::Base; end
def up
records = SpreePayment.where.not(
cvv_response_message: nil
).where.not(
state: :completed
)
records.update_all(
"redirect_auth_url = cvv_response_message, cvv_response_message = null"
)
end
def down
records = SpreePayment.where.not(
redirect_auth_url: nil
).where.not(
state: :completed
)
records.update_all("cvv_response_message = redirect_auth_url, redirect_auth_url = null")
end
end

3
db/schema.rb
View File

@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2025_07_09_012346) do
ActiveRecord::Schema[7.1].define(version: 2025_08_27_205335) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_stat_statements"
enable_extension "plpgsql"
@@ -652,6 +652,7 @@ ActiveRecord::Schema[7.0].define(version: 2025_07_09_012346) do
t.string "cvv_response_code", limit: 255
t.text "cvv_response_message"
t.datetime "captured_at", precision: nil
t.string "redirect_auth_url"
t.index ["order_id"], name: "index_spree_payments_on_order_id"
end

2
engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb
View File

@@ -66,7 +66,7 @@ module OrderManagement
allow(PaymentMailer).to receive(:authorization_required) { mail_mock }
allow(payment).to receive(:authorize!) {
payment.state = "requires_authorization"
payment.cvv_response_message = "https://stripe.com/redirect"
payment.redirect_auth_url = "https://stripe.com/redirect"
}
end

2
lib/stripe/authorize_response_patcher.rb
View File

@@ -10,7 +10,7 @@ module Stripe
def call!
if (url = url_for_authorization(@response)) && field_to_patch(@response).present?
field_to_patch(@response)['message'] = url
field_to_patch(@response)['redirect_auth_url'] = url
end
@response

2
lib/tasks/data/anonymize_data.rake
View File

@@ -64,7 +64,7 @@ namespace :ofn do
environment = '#{Rails.env}'")
Spree::Payment.update_all("response_code = null, avs_response = null,
cvv_response_code = null, identifier = null,
cvv_response_message = null")
cvv_response_message = null, redirect_auth_url = null")
Spree::CreditCard.update_all("
month = 12, year = 2020, start_month = 12, start_year = 2000,
cc_type = 'VISA', first_name = 'Dummy', last_name = 'Dummy', last_digits = '2543'")

12
spec/controllers/payment_gateways/stripe_controller_spec.rb
View File

@@ -208,7 +208,7 @@ RSpec.describe PaymentGateways::StripeController do
create(
:payment,
payment_method:,
cvv_response_message: "https://stripe.com/redirect",
redirect_auth_url: "https://stripe.com/redirect",
response_code: "pi_123",
order:,
state: "requires_authorization"
@@ -244,7 +244,7 @@ RSpec.describe PaymentGateways::StripeController do
expect(response).to redirect_to order_path(order)
payment.reload
expect(payment.state).to eq("completed")
expect(payment.cvv_response_message).to be nil
expect(payment.redirect_auth_url).to be nil
end
it "moves the order state to completed" do
@@ -273,7 +273,7 @@ RSpec.describe PaymentGateways::StripeController do
expect(response).to redirect_to order_path(order)
payment.reload
expect(payment.state).to eq("completed")
expect(payment.cvv_response_message).to be nil
expect(payment.redirect_auth_url).to be nil
end
end
@@ -288,7 +288,7 @@ RSpec.describe PaymentGateways::StripeController do
expect(response).to redirect_to order_path(order)
payment.reload
expect(payment.state).to eq("completed")
expect(payment.cvv_response_message).to be nil
expect(payment.redirect_auth_url).to be nil
end
end
end
@@ -307,7 +307,7 @@ RSpec.describe PaymentGateways::StripeController do
expect(response).to redirect_to order_path(order)
expect(flash[:error]).to eq("The payment could not be processed. error message")
payment.reload
expect(payment.cvv_response_message).to be nil
expect(payment.redirect_auth_url).to be nil
expect(payment.state).to eq("failed")
end
end
@@ -330,7 +330,7 @@ RSpec.describe PaymentGateways::StripeController do
expect(response).to redirect_to order_path(order)
expect(flash[:error]).to eq("The payment could not be processed. ")
payment.reload
expect(payment.cvv_response_message).to eq("https://stripe.com/redirect")
expect(payment.redirect_auth_url).to eq("https://stripe.com/redirect")
expect(payment.state).to eq("requires_authorization")
end
end

4
spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb
View File

@@ -78,7 +78,7 @@ RSpec.describe Spree::Admin::PaymentsController do
context "where further action is required" do
before do
allow_any_instance_of(Spree::Payment).to receive(:authorize!) do |payment|
payment.update cvv_response_message: "https://www.stripe.com/authorize"
payment.update redirect_auth_url: "https://www.stripe.com/authorize"
payment.update state: "requires_authorization"
end
end
@@ -234,7 +234,7 @@ RSpec.describe Spree::Admin::PaymentsController do
allow(PaymentMailer).to receive(:authorize_payment) { mail_mock }
request.env["HTTP_REFERER"] = "http://foo.com"
allow(Spree::Payment).to receive(:find).with(payment.id.to_s) { payment }
allow(payment).to receive(:cvv_response_message).and_return("https://www.stripe.com/authorize")
allow(payment).to receive(:redirect_auth_url).and_return("https://www.stripe.com/authorize")
allow(payment).to receive(:requires_authorization?) { true }
end

8
spec/lib/stripe/authorize_response_patcher_spec.rb
View File

@@ -26,9 +26,9 @@ RSpec.describe Stripe::AuthorizeResponsePatcher do
}
}
it "patches response.cvv_result.message with the url in the response" do
it "patches response.cvv_result.redirect_auth_url with the url in the response" do
new_response = patcher.call!
expect(new_response.cvv_result['message']).to eq "https://www.stripe.com/authorize"
expect(new_response.cvv_result['redirect_auth_url']).to eq "https://www.stripe.com/authorize"
end
context "with invalid url containing 'stripe.com'" do
@@ -42,9 +42,9 @@ RSpec.describe Stripe::AuthorizeResponsePatcher do
}
}
it "patches response.cvv_result.message with nil" do
it "patches response.cvv_result.redirect_auth_url with nil" do
new_response = patcher.call!
expect(new_response.cvv_result['message']).to be_nil
expect(new_response.cvv_result['redirect_auth_url']).to eq nil
end
end
end

239
spec/migrations/migrate_cvv_message_to_redirect_auth_url.rb Normal file
View File

@@ -0,0 +1,239 @@
# frozen_string_literal: true
require 'spec_helper'
require_relative '../../db/migrate/20250827205335_migrate_cvv_message_to_redirect_auth_url'
RSpec.describe MigrateCvvMessageToRedirectAuthUrl, type: :migration do
let(:migration) { described_class.new }
describe '#up' do
context 'when payments have cvv_response_message with redirect URLs and are not completed' do
let!(:payment_requires_auth) do
create(:payment,
cvv_response_message: 'https://bank.com/3ds-redirect?token=abc123',
redirect_auth_url: nil,
state: 'requires_authorization')
end
let!(:payment_processing) do
create(:payment,
cvv_response_message: 'https://payment-gateway.com/auth/redirect',
redirect_auth_url: nil,
state: 'processing')
end
let!(:payment_pending) do
create(:payment,
cvv_response_message: 'https://secure.payment.com/authenticate',
redirect_auth_url: nil,
state: 'pending')
end
it 'migrates cvv_response_message to redirect_auth_url' do
migration.up
payment_requires_auth.reload
payment_processing.reload
payment_pending.reload
expect(payment_requires_auth.redirect_auth_url).to eq('https://bank.com/3ds-redirect?token=abc123')
expect(payment_processing.redirect_auth_url).to eq('https://payment-gateway.com/auth/redirect')
expect(payment_pending.redirect_auth_url).to eq('https://secure.payment.com/authenticate')
expect(payment_requires_auth.cvv_response_message).to be_nil
expect(payment_processing.cvv_response_message).to be_nil
expect(payment_pending.cvv_response_message).to be_nil
end
end
context 'when payments are completed' do
let!(:completed_payment) do
create(:payment,
cvv_response_message: nil,
redirect_auth_url: nil,
state: 'completed')
end
it 'does not affect completed payments (they already have nil cvv_response_message)' do
migration.up
completed_payment.reload
expect(completed_payment.cvv_response_message).to be_nil
expect(completed_payment.redirect_auth_url).to be_nil
end
end
context 'when payments have nil cvv_response_message' do
let!(:nil_cvv_payment) do
create(:payment,
cvv_response_message: nil,
redirect_auth_url: nil,
state: 'pending')
end
it 'does not migrate payments with nil cvv_response_message' do
migration.up
nil_cvv_payment.reload
expect(nil_cvv_payment.cvv_response_message).to be_nil
expect(nil_cvv_payment.redirect_auth_url).to be_nil
end
end
context 'mixed payment states' do
let!(:eligible_payments) do
[
create(
:payment,
cvv_response_message: 'https://url1.com',
state: 'requires_authorization'
),
create(:payment, cvv_response_message: 'https://url2.com', state: 'processing'),
create(:payment, cvv_response_message: 'https://url3.com', state: 'pending'),
create(:payment, cvv_response_message: 'https://url4.com', state: 'checkout'),
create(:payment, cvv_response_message: 'https://url5.com', state: 'failed'),
create(:payment, cvv_response_message: 'https://url6.com', state: 'void'),
create(:payment, cvv_response_message: 'https://url7.com', state: 'invalid')
]
end
let!(:ineligible_payments) do
[
create(:payment, cvv_response_message: nil, state: 'completed'),
create(:payment, cvv_response_message: nil, state: 'requires_authorization')
]
end
it 'only migrates non-completed payments with cvv_response_message' do
migration.up
# Check eligible payments were migrated
eligible_payments.each do |payment|
payment.reload
expect(payment.redirect_auth_url).to be_present
expect(payment.cvv_response_message).to be_nil
end
# Check ineligible payments were not migrated
ineligible_payments.each do |payment|
payment.reload
expect(payment.redirect_auth_url).to be_nil
expect(payment.cvv_response_message).to be_nil
end
end
end
end
describe '#down' do
context 'when payments have redirect_auth_url and are not completed' do
let!(:requires_auth_payment) do
create(:payment,
cvv_response_message: nil,
redirect_auth_url: 'https://bank.com/3ds-redirect?token=xyz789',
state: 'requires_authorization')
end
let!(:processing_payment_with_redirect) do
create(:payment,
cvv_response_message: nil,
redirect_auth_url: 'https://gateway.com/authenticate',
state: 'processing')
end
it 'migrates redirect_auth_url back to cvv_response_message' do
migration.down
requires_auth_payment.reload
processing_payment_with_redirect.reload
expect(requires_auth_payment.cvv_response_message).to eq('https://bank.com/3ds-redirect?token=xyz789')
expect(processing_payment_with_redirect.cvv_response_message).to eq('https://gateway.com/authenticate')
expect(requires_auth_payment.redirect_auth_url).to be_nil
expect(processing_payment_with_redirect.redirect_auth_url).to be_nil
end
end
context 'when payments are completed' do
let!(:completed_payment_with_redirect) do
create(:payment,
cvv_response_message: nil,
redirect_auth_url: nil,
state: 'completed')
end
it 'does not affect completed payments (they have nil values)' do
migration.down
completed_payment_with_redirect.reload
expect(completed_payment_with_redirect.redirect_auth_url).to be_nil
expect(completed_payment_with_redirect.cvv_response_message).to be_nil
end
end
context 'when payments have nil redirect_auth_url' do
let!(:nil_redirect_payment) do
create(:payment,
cvv_response_message: nil,
redirect_auth_url: nil,
state: 'pending')
end
it 'does not affect payments with nil redirect_auth_url' do
migration.down
nil_redirect_payment.reload
expect(nil_redirect_payment.redirect_auth_url).to be_nil
expect(nil_redirect_payment.cvv_response_message).to be_nil
end
end
end
describe 'full migration cycle (up then down)' do
let!(:original_payments) do
[
create(
:payment,
cvv_response_message: 'https://original1.com/auth',
state: 'requires_authorization'
),
create(
:payment,
cvv_response_message: 'https://original2.com/redirect',
state: 'processing'
),
create(
:payment,
cvv_response_message: 'https://original3.com/3ds',
state: 'pending'
)
]
end
it 'preserves data integrity through up and down migrations' do
original_urls = original_payments.map(&:cvv_response_message)
# Verify initial state
expect(original_payments.all? { |p| p.redirect_auth_url.nil? }).to be true
# Migrate up
migration.up
original_payments.each(&:reload)
# Verify up migration
expect(original_payments.map(&:redirect_auth_url)).to eq(original_urls)
expect(original_payments.all? { |p| p.cvv_response_message.nil? }).to be true
# Migrate down
migration.down
original_payments.each(&:reload)
# Verify down migration restores original state
expect(original_payments.map(&:cvv_response_message)).to eq(original_urls)
expect(original_payments.all? { |p| p.redirect_auth_url.nil? }).to be true
end
end
end

11
spec/models/spree/payment_spec.rb
View File

@@ -204,8 +204,9 @@ RSpec.describe Spree::Payment do
context "authorization is required" do
before do
allow(success_response).to receive(:cvv_result) {
{ 'code' => "123",
'message' => "https://stripe.com/redirect" }
{ 'code' => nil,
'message' => nil,
'redirect_auth_url' => "https://stripe.com/redirect" }
}
expect(payment.payment_method).to receive(:authorize).with(
amount_in_cents, card, anything
@@ -1047,11 +1048,11 @@ RSpec.describe Spree::Payment do
end
describe "#clear_authorization_url" do
let(:payment) { create(:payment, cvv_response_message: "message") }
let(:payment) { create(:payment, redirect_auth_url: "auth_url") }
it "removes the cvv_response_message" do
it "removes the redirect_auth_url" do
payment.clear_authorization_url
expect(payment.cvv_response_message).to eq(nil)
expect(payment.redirect_auth_url).to eq(nil)
end
end

8
spec/queries/payments_requiring_action_query_spec.rb
View File

@@ -9,11 +9,11 @@ RSpec.describe PaymentsRequiringActionQuery do
let(:order) { create(:order, user:) }
describe '#call' do
context "payment has a cvv_response_message" do
context "payment has a redirect_auth_url" do
let(:payment) do
create(:payment,
order:,
cvv_response_message: "https://stripe.com/redirect",
redirect_auth_url: "https://stripe.com/redirect",
state: "requires_authorization")
end
@@ -22,9 +22,9 @@ RSpec.describe PaymentsRequiringActionQuery do
end
end
context "payment has no cvv_response_message" do
context "payment has no redirect_auth_url" do
let(:payment) do
create(:payment, order:, cvv_response_message: nil)
create(:payment, order:, redirect_auth_url: nil)
end
it "does not find the payment" do

8
spec/requests/payments_controller_spec.rb
View File

@@ -18,18 +18,18 @@ RSpec.describe "/payments/:id/authorize" do
describe "when user is logged in" do
before { sign_in user }
context "has cvv response message" do
context "has redirect auth url" do
before do
allow_any_instance_of(Spree::Payment).to receive(:cvv_response_message).and_return('http://example.com')
allow_any_instance_of(Spree::Payment).to receive(:redirect_auth_url).and_return('http://example.com')
end
it "redirects to the CVV response URL" do
it "redirects to the 3D-Auth url" do
get authorize_payment_path(payment)
expect(response).to redirect_to('http://example.com')
end
end
context "doesn't have cvv response message" do
context "doesn't have redirect auth url" do
it "redirect to order URL" do
get authorize_payment_path(payment)
expect(response).to redirect_to(order_url(order))

2
spec/serializers/api/admin/order_serializer_spec.rb
View File

@@ -44,7 +44,7 @@ RSpec.describe Api::Admin::OrderSerializer do
order:,
state: 'requires_authorization',
amount: 123.45,
cvv_response_message: "https://stripe.com/redirect"
redirect_auth_url: "https://stripe.com/redirect"
)
end

2
spec/services/checkout/stripe_redirect_spec.rb
View File

@@ -36,7 +36,7 @@ RSpec.describe Checkout::StripeRedirect do
true
end
expect(stripe_payment).to receive(:cvv_response_message).and_return(test_redirect_url)
expect(stripe_payment).to receive(:redirect_auth_url).and_return(test_redirect_url)
expect(service.path).to eq test_redirect_url
end

4
spec/services/process_payment_intent_spec.rb
View File

@@ -17,7 +17,7 @@ RSpec.describe ProcessPaymentIntent do
create(
:payment,
payment_method:,
cvv_response_message: "https://stripe.com/redirect",
redirect_auth_url: "https://stripe.com/redirect",
response_code: "pi_123",
order:,
state: "requires_authorization"
@@ -104,7 +104,7 @@ RSpec.describe ProcessPaymentIntent do
service.call!
payment.reload
expect(payment.state).to eq("completed")
expect(payment.cvv_response_message).to be nil
expect(payment.redirect_auth_url).to be nil
end
it "completes the order" do

4
spec/system/consumer/account/payments_spec.rb
View File

@@ -17,7 +17,7 @@ RSpec.describe "Payments requiring action" do
let!(:payment) do
create(:payment,
order:,
cvv_response_message: "https://stripe.com/redirect",
redirect_auth_url: "https://stripe.com/redirect",
state: "requires_authorization")
end
@@ -31,7 +31,7 @@ RSpec.describe "Payments requiring action" do
context "there are no payments requiring authorization" do
let!(:payment) do
create(:payment, order:, cvv_response_message: nil)
create(:payment, order:, redirect_auth_url: nil)
end
it "does not show the table of payments requiring authorization" do