From db5503dd807937a4696a6cd27350dca3c2bd313c Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 22 Sep 2017 16:47:22 +1000
Subject: [PATCH] Allow destruction of StripeAccounts even if deauthorise
 request fails

Log deauthorise failures to Bugsnag
---
 app/models/stripe_account.rb       | 15 +++++++++------
 spec/models/stripe_account_spec.rb |  8 +++++---
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/app/models/stripe_account.rb b/app/models/stripe_account.rb
index dfbcc560ea..1b2840ffe6 100644
--- a/app/models/stripe_account.rb
+++ b/app/models/stripe_account.rb
@@ -7,12 +7,15 @@ class StripeAccount < ActiveRecord::Base
     accounts = StripeAccount.where(stripe_user_id: stripe_user_id)
 
     # Only deauthorize the user if it is not linked to multiple accounts
-    if accounts.count > 1 || Stripe::OAuth.deauthorize(stripe_user_id: stripe_user_id)
-      destroy
-    else
-      false
-    end
+    return destroy if accounts.count > 1
+
+    destroy && Stripe::OAuth.deauthorize(stripe_user_id: stripe_user_id)
   rescue Stripe::OAuth::OAuthError
-    false
+    Bugsnag.notify(
+      RuntimeError.new("StripeDeauthorizeFailure"),
+      stripe_account: stripe_user_id,
+      enterprise_id: enterprise_id
+    )
+    true
   end
 end
diff --git a/spec/models/stripe_account_spec.rb b/spec/models/stripe_account_spec.rb
index a3afa1685a..b4a1cbfb00 100644
--- a/spec/models/stripe_account_spec.rb
+++ b/spec/models/stripe_account_spec.rb
@@ -21,9 +21,10 @@ describe StripeAccount do
           to_return(status: 400, body: JSON.generate(error: 'invalid_grant', error_description: "Some Message"))
       end
 
-      it "doesn't destroy the record" do
+      it "destroys the record and notifies Bugsnag" do
+        expect(Bugsnag).to receive(:notify)
         stripe_account.deauthorize_and_destroy
-        expect(StripeAccount.all).to include(stripe_account)
+        expect(StripeAccount.all).to_not include(stripe_account)
       end
     end
 
@@ -41,11 +42,12 @@ describe StripeAccount do
     end
 
     context "if the account is also associated with another Enterprise" do
-      let!(:another_stripe_account) { create(:stripe_account, enterprise: enterprise2, stripe_user_id: stripe_account.stripe_user_id) }
+      let!(:another_stripe_account) { create(:stripe_account, enterprise: enterprise2, stripe_user_id: stripe_user_id) }
 
       it "Doesn't make a Stripe API disconnection request " do
         expect(Stripe::OAuth).to_not receive(:deauthorize)
         stripe_account.deauthorize_and_destroy
+        expect(StripeAccount.all).not_to include(stripe_account)
       end
     end
   end