From ffc99df3731e823ee9166f2f279c41a00f603228 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 14 Jul 2017 11:40:08 +1000 Subject: [PATCH] Consolidate Stripe routes/actions into StripeAccountsController --- .../controllers/stripe_controller.js.coffee | 2 +- .../admin/enterprises_controller.rb | 18 ---- .../admin/stripe_accounts_controller.rb | 21 ++++- .../form/_stripe_connect.html.haml | 4 +- config/routes.rb | 14 +-- .../admin/enterprises_controller_spec.rb | 68 --------------- .../admin/stripe_accounts_controller_spec.rb | 85 +++++++++++++++++-- 7 files changed, 107 insertions(+), 105 deletions(-) diff --git a/app/assets/javascripts/admin/payment_methods/controllers/stripe_controller.js.coffee b/app/assets/javascripts/admin/payment_methods/controllers/stripe_controller.js.coffee index c8a1b6a019..575a7995a9 100644 --- a/app/assets/javascripts/admin/payment_methods/controllers/stripe_controller.js.coffee +++ b/app/assets/javascripts/admin/payment_methods/controllers/stripe_controller.js.coffee @@ -5,7 +5,7 @@ angular.module("admin.paymentMethods").controller "StripeController", ($scope, $ $scope.$watch "paymentMethod.preferred_enterprise_id", (newID, oldID) -> return unless newID? $scope.stripe_account = {} - $http.get("/admin/enterprises/#{newID}/stripe_account.json").success (data) -> + $http.get("/admin/stripe_accounts/status.json?enterprise_id=#{newID}").success (data) -> angular.extend($scope.stripe_account, data) .error (response) -> $scope.stripe_account.status = "request_failed" diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 0ccb65095b..1e8a4f0968 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -1,6 +1,4 @@ require 'open_food_network/referer_parser' -require 'stripe/account_connector' -require 'stripe/oauth' module Admin class EnterprisesController < ResourceController @@ -115,22 +113,6 @@ module Admin end end - def stripe_connect - redirect_to Stripe::OAuth.authorize_url(params[:enterprise_id]) - end - - def stripe_connect_callback - connector = Stripe::AccountConnector.new(spree_current_user, params) - if connector.create_account - flash[:success] = t('admin.controllers.enterprises.stripe_connect_success') - redirect_to main_app.edit_admin_enterprise_path(connector.enterprise) - else - render text: t('admin.controllers.enterprises.stripe_connect_fail'), status: 500 - end - rescue Stripe::StripeError => e - render text: e.message, status: 500 - end - protected def build_resource_with_address diff --git a/app/controllers/admin/stripe_accounts_controller.rb b/app/controllers/admin/stripe_accounts_controller.rb index 695f415eb9..8bdbeee504 100644 --- a/app/controllers/admin/stripe_accounts_controller.rb +++ b/app/controllers/admin/stripe_accounts_controller.rb @@ -1,7 +1,26 @@ +require 'stripe/account_connector' +require 'stripe/oauth' + module Admin class StripeAccountsController < BaseController protect_from_forgery except: :destroy_from_webhook + def connect + redirect_to Stripe::OAuth.authorize_url(params[:enterprise_id]) + end + + def connect_callback + connector = Stripe::AccountConnector.new(spree_current_user, params) + if connector.create_account + flash[:success] = t('admin.controllers.enterprises.stripe_connect_success') + redirect_to main_app.edit_admin_enterprise_path(connector.enterprise) + else + render text: t('admin.controllers.enterprises.stripe_connect_fail'), status: 500 + end + rescue Stripe::StripeError => e + render text: e.message, status: 500 + end + def destroy stripe_account = StripeAccount.find(params[:id]) authorize! :destroy, stripe_account @@ -18,7 +37,7 @@ module Admin redirect_to spree.admin_path end - def destroy_from_webhook + def deauthorize # 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: 400 unless event.type == "account.application.deauthorized" diff --git a/app/views/admin/enterprises/form/_stripe_connect.html.haml b/app/views/admin/enterprises/form/_stripe_connect.html.haml index 4b13fca017..b1ed3802e6 100644 --- a/app/views/admin/enterprises/form/_stripe_connect.html.haml +++ b/app/views/admin/enterprises/form/_stripe_connect.html.haml @@ -3,7 +3,7 @@ .row = t('.stripe_account_connected') .row - =link_to t('.disconnect'), main_app.admin_enterprise_stripe_account_path(@enterprise,@stripe_account), method: :delete, class: 'button' + =link_to t('.disconnect'), main_app.admin_stripe_account_path(@stripe_account), method: :delete, class: 'button' - else .row.stripe-info .six.columns.alpha @@ -15,5 +15,5 @@ -# What, if any, fees the platform charges. =t('.stripe_connect_info') .five.columns.omega.text-right - %a.stripe-connect{href: 'stripe_connect'} + %a.stripe-connect{ href: main_app.connect_admin_stripe_accounts_path(enterprise_id: @enterprise) } %span= t('.connect_with_stripe') diff --git a/config/routes.rb b/config/routes.rb index bb1422063c..9ed42772e5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -65,9 +65,6 @@ Openfoodnetwork::Application.routes.draw do post 'embedded_shopfront/enable', to: 'application#enable_embedded_styles' post 'embedded_shopfront/disable', to: 'application#disable_embedded_styles' - get '/stripe/callback', :to => 'admin/enterprises#stripe_connect_callback' - post '/stripe/webhook', :to => 'admin/stripe_accounts#destroy_from_webhook' - resources :enterprises do collection do post :search @@ -101,10 +98,6 @@ Openfoodnetwork::Application.routes.draw do post :bulk_update, as: :bulk_update end - get "/stripe_connect", to: "enterprises#stripe_connect" - get "/stripe_account", to: "stripe_accounts#status" - resources :stripe_accounts, only: [:destroy] - member do get :welcome put :register @@ -173,6 +166,13 @@ Openfoodnetwork::Application.routes.draw do resource :invoice_settings, only: [:edit, :update] resource :stripe_connect_settings, only: [:edit, :update] + + resources :stripe_accounts, only: [:destroy] do + get :connect, on: :collection + get :connect_callback, on: :collection + get :status, on: :collection + post :deauthorize, on: :collection + end end namespace :api do diff --git a/spec/controllers/admin/enterprises_controller_spec.rb b/spec/controllers/admin/enterprises_controller_spec.rb index c51fbddd4f..5e2300d122 100644 --- a/spec/controllers/admin/enterprises_controller_spec.rb +++ b/spec/controllers/admin/enterprises_controller_spec.rb @@ -144,74 +144,6 @@ module Admin expect(distributor.users).to_not include user end - describe "#stripe_connect" do - before do - allow(controller).to receive(:spree_current_user) { distributor_manager } - allow(::Stripe::OAuth).to receive(:authorize_url) { "some_url" } - end - - it "redirects to Stripe Authorization url constructed OAuth" do - spree_get :stripe_connect - expect(response).to redirect_to "some_url" - end - end - - context "#stripe_connect_callback" do - let(:params) { { id: distributor.permalink } } - before { allow(controller).to receive(:spree_current_user) { distributor_manager } } - - context "when the connector raises a StripeError" do - before do - allow(Stripe::AccountConnector).to receive(:new).and_raise Stripe::StripeError, "some error" - end - - it "returns a 500 error" do - spree_get :stripe_connect_callback, params - response.status.should be 500 - end - end - - context "when the connector raises an AccessDenied error" do - before do - allow(Stripe::AccountConnector).to receive(:new).and_raise CanCan::AccessDenied, "some error" - end - - it "redirects to unauthorized" do - spree_get :stripe_connect_callback, params - expect(response).to redirect_to spree.unauthorized_path - end - end - - context "when initializing the connector does not raise an error" do - let(:connector) { double(:connector) } - - before do - allow(Stripe::AccountConnector).to receive(:new) { connector } - end - - context "when the connector succeeds in creating a new stripe account record" do - before { allow(connector).to receive(:create_account) { true } } - - it "redirects to the enterprise edit path" do - spree_get :stripe_connect_callback, params - expect(flash[:success]).to eq I18n.t('admin.controllers.enterprises.stripe_connect_success') - expect(response).to redirect_to edit_admin_enterprise_path(distributor) - end - end - - context "when the connector succeeds in creating a new stripe account record" do - before { allow(connector).to receive(:create_account) { false } } - - it "renders a failure message" do - spree_get :stripe_connect_callback, params - expect(response.body).to eq I18n.t('admin.controllers.enterprises.stripe_connect_fail') - expect(response.status).to be 500 - end - end - end - end - - describe "enterprise properties" do let(:producer) { create(:enterprise) } let!(:property) { create(:property, name: "A nice name") } diff --git a/spec/controllers/admin/stripe_accounts_controller_spec.rb b/spec/controllers/admin/stripe_accounts_controller_spec.rb index 01dff11d31..5ee046debf 100644 --- a/spec/controllers/admin/stripe_accounts_controller_spec.rb +++ b/spec/controllers/admin/stripe_accounts_controller_spec.rb @@ -1,7 +1,78 @@ require 'spec_helper' describe Admin::StripeAccountsController, type: :controller do - describe "destroy_from_webhook" do + let(:enterprise) { create(:distributor_enterprise) } + + describe "#connect" do + before do + allow(controller).to receive(:spree_current_user) { enterprise.owner } + allow(Stripe::OAuth).to receive(:authorize_url) { "some_url" } + end + + it "redirects to Stripe Authorization url constructed OAuth" do + spree_get :connect + expect(response).to redirect_to "some_url" + end + end + + context "#connect_callback" do + let(:params) { { id: enterprise.permalink } } + before { allow(controller).to receive(:spree_current_user) { enterprise.owner } } + + context "when the connector raises a StripeError" do + before do + allow(Stripe::AccountConnector).to receive(:new).and_raise Stripe::StripeError, "some error" + end + + it "returns a 500 error" do + spree_get :connect_callback, params + response.status.should be 500 + end + end + + context "when the connector raises an AccessDenied error" do + before do + allow(Stripe::AccountConnector).to receive(:new).and_raise CanCan::AccessDenied, "some error" + end + + it "redirects to unauthorized" do + spree_get :connect_callback, params + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "when initializing the connector does not raise an error" do + let(:connector) { double(:connector) } + + before do + allow(Stripe::AccountConnector).to receive(:new) { connector } + end + + context "when the connector succeeds in creating a new stripe account record" do + before { allow(connector).to receive(:create_account) { true } } + + it "redirects to the enterprise edit path" do + expect(connector).to receive(:enterprise) { enterprise } + spree_get :connect_callback, params + expect(flash[:success]).to eq I18n.t('admin.controllers.enterprises.stripe_connect_success') + expect(response).to redirect_to edit_admin_enterprise_path(enterprise) + end + end + + context "when the connector succeeds in creating a new stripe account record" do + before { allow(connector).to receive(:create_account) { false } } + + it "renders a failure message" do + expect(connector).to_not receive(:enterprise) + spree_get :connect_callback, params + expect(response.body).to eq I18n.t('admin.controllers.enterprises.stripe_connect_fail') + expect(response.status).to be 500 + end + end + end + end + + describe "#deauthorize" do let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") } let(:params) do { @@ -15,7 +86,7 @@ describe Admin::StripeAccountsController, type: :controller do end it "deletes Stripe accounts in response to a webhook" do - post 'destroy_from_webhook', params + post 'deauthorize', params expect(response.status).to eq 200 expect(response.body).to eq "Account webhook_id deauthorized" expect(StripeAccount.all).not_to include stripe_account @@ -27,7 +98,7 @@ describe Admin::StripeAccountsController, type: :controller do end it "does nothing" do - post 'destroy_from_webhook', params + post 'deauthorize', params expect(response.status).to eq 400 expect(StripeAccount.all).to include stripe_account end @@ -39,15 +110,14 @@ describe Admin::StripeAccountsController, type: :controller do end it "does nothing" do - post 'destroy_from_webhook', params + post 'deauthorize', params expect(response.status).to eq 400 expect(StripeAccount.all).to include stripe_account end end end - describe "destroy" do - let(:enterprise) { create(:distributor_enterprise) } + describe "#destroy" do let(:params) { { format: :json, id: "some_id" } } context "when the specified stripe account doesn't exist" do @@ -102,8 +172,7 @@ describe Admin::StripeAccountsController, type: :controller do end end - describe "verifying stripe account status with Stripe" do - let(:enterprise) { create(:distributor_enterprise) } + describe "#status" do let(:params) { { format: :json } } before do