From f22dd7513d6e2d62b9a30c093ba29abbf1329943 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Wed, 11 Oct 2017 17:50:46 +1100
Subject: [PATCH] Add a service object for handling Stripe webhooks

---
 app/controllers/stripe_controller.rb       | 13 ++--
 lib/stripe/webhook_handler.rb              | 34 +++++++++
 spec/controllers/stripe_controller_spec.rb |  1 -
 spec/lib/stripe/webhook_handler_spec.rb    | 86 ++++++++++++++++++++++
 4 files changed, 125 insertions(+), 9 deletions(-)
 create mode 100644 lib/stripe/webhook_handler.rb
 create mode 100644 spec/lib/stripe/webhook_handler_spec.rb

diff --git a/app/controllers/stripe_controller.rb b/app/controllers/stripe_controller.rb
index e81e951fb9..374f116cef 100644
--- a/app/controllers/stripe_controller.rb
+++ b/app/controllers/stripe_controller.rb
@@ -1,14 +1,11 @@
+require 'stripe/webhook_handler'
+
 class StripeController < BaseController
   def webhook
     # TODO is there a sensible way to confirm this webhook call is actually from Stripe?
-    event = Stripe::Event.construct_from(params)
-    return render nothing: true, status: 204 unless event.type == "account.application.deauthorized"
+    handler = Stripe::WebhookHandler.new(params)
+    status = handler.handle ? 200 : 204
 
-    destroyed = StripeAccount.where(stripe_user_id: event.account).destroy_all
-    if destroyed.any?
-      render text: "Account #{event.account} deauthorized", status: 200
-    else
-      render nothing: true, status: 204
-    end
+    render nothing: true, status: status
   end
 end
diff --git a/lib/stripe/webhook_handler.rb b/lib/stripe/webhook_handler.rb
new file mode 100644
index 0000000000..b40fa7a0a3
--- /dev/null
+++ b/lib/stripe/webhook_handler.rb
@@ -0,0 +1,34 @@
+module Stripe
+  class WebhookHandler
+    def initialize(params)
+      @event = Event.construct_from(params)
+    end
+
+    def handle
+      return false unless known_event?
+      send(event_mappings[@event.type])
+    end
+
+    private
+
+    def event_mappings
+      {
+        "account.application.deauthorized" => :deauthorize
+      }
+    end
+
+    def known_event?
+      event_mappings.keys.include? @event.type
+    end
+
+    def deauthorize
+      return false unless @event.respond_to?(:account)
+      destroyed = destroy_stripe_accounts_linked_to(@event.account)
+      destroyed.any?
+    end
+
+    def destroy_stripe_accounts_linked_to(account)
+      StripeAccount.where(stripe_user_id: account).destroy_all
+    end
+  end
+end
diff --git a/spec/controllers/stripe_controller_spec.rb b/spec/controllers/stripe_controller_spec.rb
index ac10d08432..7896cf1230 100644
--- a/spec/controllers/stripe_controller_spec.rb
+++ b/spec/controllers/stripe_controller_spec.rb
@@ -17,7 +17,6 @@ describe StripeController do
     it "deletes Stripe accounts in response to a webhook" do
       post 'webhook', params
       expect(response.status).to eq 200
-      expect(response.body).to eq "Account webhook_id deauthorized"
       expect(StripeAccount.all).not_to include stripe_account
     end
 
diff --git a/spec/lib/stripe/webhook_handler_spec.rb b/spec/lib/stripe/webhook_handler_spec.rb
new file mode 100644
index 0000000000..2847efdab9
--- /dev/null
+++ b/spec/lib/stripe/webhook_handler_spec.rb
@@ -0,0 +1,86 @@
+require 'spec_helper'
+require 'stripe/webhook_handler'
+
+module Stripe
+  describe WebhookHandler do
+    let(:params) { { type: 'some.event' } }
+    let(:handler) { WebhookHandler.new(params) }
+
+    describe "event_mappings" do
+      it { expect(handler.send(:event_mappings)).to be_a Hash }
+    end
+
+    describe "known_event?" do
+      context "when event mappings know about the event type" do
+        before do
+          allow(handler).to receive(:event_mappings) { { 'some.event' => :something } }
+        end
+
+        it { expect(handler.send(:known_event?)).to be true }
+      end
+
+      context "when event mappings do not know about the event type" do
+        before do
+          allow(handler).to receive(:event_mappings) { { 'some.other.event' => :something } }
+        end
+
+        it { expect(handler.send(:known_event?)).to be false }
+      end
+    end
+
+    describe "handle" do
+      context "when the event is known" do
+        before do
+          allow(handler).to receive(:event_mappings) { { 'some.event' => :some_method } }
+        end
+
+        it "calls the handler method, and returns the result" do
+          expect(handler).to receive(:some_method) { 'result' }
+          expect(handler.handle).to eq 'result'
+        end
+      end
+
+      context "when the event is unknown" do
+        before do
+          allow(handler).to receive(:event_mappings) { { 'some.other.event' => :some_method } }
+        end
+
+        it "does not call the handler method, and returns false" do
+          expect(handler).to_not receive(:some_method)
+          expect(handler.handle).to be false
+        end
+      end
+    end
+
+    describe "deauthorize" do
+      context "when the event has no 'account' attribute" do
+        it "does destroy stripe accounts, returns false" do
+          expect(handler).to_not receive(:destroy_stripe_accounts_linked_to)
+          expect(handler.send(:deauthorize)).to be false
+        end
+      end
+
+      context "when the event has an 'account' attribute" do
+        before do
+          params[:account] = 'some.account'
+        end
+
+        context "when some stripe accounts are destroyed" do
+          before do
+            allow(handler).to receive(:destroy_stripe_accounts_linked_to).with('some.account') { [double(:destroyed_stripe_account)] }
+          end
+
+          it { expect(handler.send(:deauthorize)).to be true }
+        end
+
+        context "when no stripe accounts are destroyed" do
+          before do
+            allow(handler).to receive(:destroy_stripe_accounts_linked_to).with('some.account') { [] }
+          end
+
+          it { expect(handler.send(:deauthorize)).to be false }
+        end
+      end
+    end
+  end
+end