Validate payment info present before attempting to process (#15629)

* Add two guards when purchasing credits

This prevents sending an empty card to stripe, and avoids a
foreseeable error when purchasing 0 credits. Error returned from
stripe is "This value must be greater than or equal to 1".

* Move redirect back into action method

Only use validation methods to check input and set appropriate messages.

* Accept organization id if set

The payment service will use the organization id (and charge the
organization) when that's the purchaser. Don't fail when no stripe
token or card selected if an organization admin is purchasing credits.

* Move validation into the payment processing service

This cleans up the number of details the controller needs to worry
about

Now only if the process payment service had any tests...

* Prevent only organization_id from permitting purchase

and move this check to a method, with an intention revealing name

* Add i18n translation for error message

There weren't any service side error messages - this will be displayed
in a view (credits/purchase) but comes from the payment service -
other messages (from stripe) are passed through as-is there.

If this is the wrong _place_ for the translation I can retool.

* Add unit test for payment processing

Only checking errors are raised as expected and the internal error
message is translated.

* Leave translation alone

Revert test translation
This commit is contained in:
Daniel Uber 2021-12-03 13:09:23 -06:00 committed by GitHub
parent 251e870766
commit 0eb4e690ed
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 69 additions and 2 deletions

View file

@ -25,8 +25,13 @@ module Payments
end
def call
process_purchase
create_credits if success?
if payment_method_provided?
process_purchase
create_credits if success?
else
self.error = I18n.t("services.payments.errors.select_payment_method")
end
self
end
@ -39,6 +44,11 @@ module Payments
attr_accessor :user, :credits_count, :success, :purchase_options
attr_writer :purchaser, :error
def payment_method_provided?
# we expect at least one of :stripe_token or :selected_card to process
purchase_options.slice(:stripe_token, :selected_card).compact_blank.present?
end
def process_purchase
customer = find_or_create_customer
update_user_stripe_info(customer)

View file

@ -0,0 +1,6 @@
---
en:
services:
payments:
errors:
select_payment_method: "Please select a payment method"

View file

@ -0,0 +1,6 @@
---
fr:
services:
payments:
errors:
select_payment_method: "Please choose a payment method"

View file

@ -0,0 +1,45 @@
require "rails_helper"
RSpec.describe Payments::ProcessCreditPurchase, type: :service do
describe ".call" do
let(:purchase_options) { {} }
it "sets error if no payment method" do
purchase = described_class.call(:nouser, :nocredits, purchase_options: purchase_options)
expect(purchase.error).to eq(I18n.t("services.payments.errors.select_payment_method"))
end
context "when a payment error occurs" do
let(:customer) { Stripe::Customer.create }
let(:user) { instance_double(User, id: :id, stripe_id_code: customer.id) }
let(:purchase_options) { { stripe_token: :token } }
before do
StripeMock.start
end
after do
StripeMock.stop
end
it "sets error if less than 1 credit ordered" do
purchase = described_class.call(user, 0, purchase_options: purchase_options)
expect(purchase.error).to eq("Invalid positive integer")
end
it "sets error if payment error is raised" do
# this customer get call is the first thing call does
allow(Payments::Customer)
.to receive(:get)
.with(customer.id)
.and_raise(Payments::PaymentsError, "Message")
purchase = described_class.call(user, 1, purchase_options: purchase_options)
expect(purchase.error).to eq("Message")
end
end
end
end