From 0eb4e690ed719e42fc83e15082368c6d5ec266be Mon Sep 17 00:00:00 2001
From: Daniel Uber <djuber@gmail.com>
Date: Fri, 3 Dec 2021 13:09:23 -0600
Subject: [PATCH] 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
---
 .../payments/process_credit_purchase.rb       | 14 +++++-
 config/locales/services/payments/en.yml       |  6 +++
 config/locales/services/payments/fr.yml       |  6 +++
 .../payments/process_credit_purchase_spec.rb  | 45 +++++++++++++++++++
 4 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100644 config/locales/services/payments/en.yml
 create mode 100644 config/locales/services/payments/fr.yml
 create mode 100644 spec/services/payments/process_credit_purchase_spec.rb

diff --git a/app/services/payments/process_credit_purchase.rb b/app/services/payments/process_credit_purchase.rb
index f9cb3aa44..e150f1440 100644
--- a/app/services/payments/process_credit_purchase.rb
+++ b/app/services/payments/process_credit_purchase.rb
@@ -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)
diff --git a/config/locales/services/payments/en.yml b/config/locales/services/payments/en.yml
new file mode 100644
index 000000000..6dc29b408
--- /dev/null
+++ b/config/locales/services/payments/en.yml
@@ -0,0 +1,6 @@
+---
+en:
+  services:
+    payments:
+      errors:
+        select_payment_method: "Please select a payment method"
diff --git a/config/locales/services/payments/fr.yml b/config/locales/services/payments/fr.yml
new file mode 100644
index 000000000..740dabaa6
--- /dev/null
+++ b/config/locales/services/payments/fr.yml
@@ -0,0 +1,6 @@
+---
+fr:
+  services:
+    payments:
+      errors:
+        select_payment_method: "Please choose a payment method"
diff --git a/spec/services/payments/process_credit_purchase_spec.rb b/spec/services/payments/process_credit_purchase_spec.rb
new file mode 100644
index 000000000..debfce6a0
--- /dev/null
+++ b/spec/services/payments/process_credit_purchase_spec.rb
@@ -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