From 340959c73b797e56e4d08d49cfa9eb456f48701a Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Wed, 11 Oct 2017 23:48:41 +0100 Subject: [PATCH] Fix failing tests (likely due to Discourse Rails upgrade) --- README.md | 2 +- .../discourse_donations/charges_controller.rb | 25 +++++++++------ .../charges_controller_spec.rb | 31 ++++++++++--------- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index bd7f386..9e5cd20 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Discourse Donations -[![Build Status](https://travis-ci.org/choiceaustralia/discourse-donations.svg?branch=master)](https://travis-ci.org/choiceaustralia/discourse-donations) +[![Build Status](https://travis-ci.org/chrisbeach/discourse-donations.svg?branch=master)](https://travis-ci.org/choiceaustralia/discourse-donations) Accept donations in Discourse! Integrates with [Stripe](https://stripe.com). diff --git a/app/controllers/discourse_donations/charges_controller.rb b/app/controllers/discourse_donations/charges_controller.rb index 818a522..72be0e0 100644 --- a/app/controllers/discourse_donations/charges_controller.rb +++ b/app/controllers/discourse_donations/charges_controller.rb @@ -4,21 +4,22 @@ module DiscourseDonations class ChargesController < ActionController::Base include CurrentUser - skip_before_filter :verify_authenticity_token, only: [:create] + protect_from_forgery prepend: true + protect_from_forgery with: :exception + + skip_before_action :verify_authenticity_token, only: [:create] def create - params.permit(:name, :username, :email, :password, :stripeToken, :amount, :create_account) - output = { 'messages' => [], 'rewards' => [] } if create_account - if !email.present? || !params[:username].present? + if !email.present? || !user_params[:username].present? output['messages'] << I18n.t('login.missing_user_field') end - if params[:password] && params[:password].length > User.max_password_length + if user_params[:password] && user_params[:password].length > User.max_password_length output['messages'] << I18n.t('login.password_too_long') end - if params[:username] && ::User.reserved_username?(params[:username]) + if user_params[:username] && ::User.reserved_username?(user_params[:username]) output['messages'] << I18n.t('login.reserved_username') end end @@ -30,7 +31,7 @@ module DiscourseDonations payment = DiscourseDonations::Stripe.new(secret_key, stripe_options) begin - charge = payment.charge(email, params) + charge = payment.charge(email, opts: user_params) rescue ::Stripe::CardError => e err = e.json_body[:error] @@ -49,7 +50,7 @@ module DiscourseDonations output['rewards'] << { type: :badge, name: badge_name } if badge_name if create_account && email.present? - args = params.slice(:email, :username, :password, :name).merge(rewards: output['rewards']) + args = user_params.to_h.slice(:email, :username, :password, :name).merge(rewards: output['rewards']) Jobs.enqueue(:donation_user, args) end end @@ -60,7 +61,7 @@ module DiscourseDonations private def create_account - params[:create_account] == 'true' && SiteSetting.discourse_donations_enable_create_accounts + user_params[:create_account] == 'true' && SiteSetting.discourse_donations_enable_create_accounts end def reward?(payment) @@ -86,8 +87,12 @@ module DiscourseDonations } end + def user_params + params.permit(:name, :username, :email, :password, :stripeToken, :amount, :create_account) + end + def email - params[:email] || current_user.try(:email) + user_params[:email] || current_user.try(:email) end end end diff --git a/spec/controllers/discourse_donations/charges_controller_spec.rb b/spec/controllers/discourse_donations/charges_controller_spec.rb index c69b1d6..6f0cc11 100644 --- a/spec/controllers/discourse_donations/charges_controller_spec.rb +++ b/spec/controllers/discourse_donations/charges_controller_spec.rb @@ -23,20 +23,23 @@ module DiscourseDonations SiteSetting.stubs(:discourse_donations_currency).returns('AUD') end + # Workaround for rails-5 issue. See https://github.com/thoughtbot/shoulda-matchers/issues/1018#issuecomment-315876453 + let(:allowed_params) { {create_account: 'true', email: 'email@example.com', password: 'secret', username: 'mr-pink', name: 'kirsten', amount: 100, stripeToken: 'rrurrrurrrrr'} } + it 'whitelists the params' do - params = { create_account: 'true', email: 'email@example.com', password: 'secret', username: 'mr-pink', name: 'kirsten', amount: 100, stripeToken: 'rrurrrurrrrr' } - should permit(:name, :username, :email, :password, :create_account).for(:create, params: params) + should permit(:name, :username, :email, :password, :create_account). + for(:create, params: { params: allowed_params }) end it 'responds ok for anonymous users' do - post :create, { email: 'foobar@example.com' } + post :create, params: { email: 'foobar@example.com' } expect(body['messages']).to include(I18n.t('donations.payment.success')) expect(response).to have_http_status(200) end it 'does not expect a username or email if accounts are not being created' do current_user = log_in(:coding_horror) - post :create, { create_account: 'false' } + post :create, params: { create_account: 'false' } expect(body['messages']).to include(I18n.t('donations.payment.success')) expect(response).to have_http_status(200) end @@ -51,17 +54,17 @@ module DiscourseDonations end it 'does not create user accounts' do - post :create, params + post :create, params: params end it 'does not create user accounts if the user is logged in' do log_in :coding_horror - post :create, params + post :create, params: params end it 'does not create user accounts when settings are disabled and params are not' do log_in :coding_horror - post :create, params.merge(create_account: true, email: 'email@example.com', password: 'secret', username: 'mr-brown', name: 'hacker-guy') + post :create, params: params.merge(create_account: true, email: 'email@example.com', password: 'secret', username: 'mr-brown', name: 'hacker-guy') end end @@ -74,7 +77,7 @@ module DiscourseDonations end it 'enqueues the user account create' do - post :create, params + post :create, params: params end end end @@ -85,19 +88,19 @@ module DiscourseDonations before { SiteSetting.stubs(:discourse_donations_enable_create_accounts).returns(true) } describe 'requires an email' do - before { post :create, params.merge(email: '') } + before { post :create, params: params.merge(email: '') } include_examples 'failure response', 'login.missing_user_field' end describe 'requires a username' do - before { post :create, params.merge(username: '') } + before { post :create, params: params.merge(username: '') } include_examples 'failure response', 'login.missing_user_field' end describe 'reserved usernames' do before do User.expects(:reserved_username?).returns(true) - post :create, params + post :create, params: params end include_examples 'failure response', 'login.reserved_username' @@ -106,7 +109,7 @@ module DiscourseDonations describe 'minimum password length' do before do User.expects(:max_password_length).returns(params[:password].length - 1) - post :create, params + post :create, params: params end include_examples 'failure response', 'login.password_too_long' @@ -119,7 +122,7 @@ module DiscourseDonations shared_examples 'no rewards' do it 'has no rewards' do - post :create, params + post :create, params: params expect(body['rewards']).to be_empty end end @@ -150,7 +153,7 @@ module DiscourseDonations end include_examples 'no rewards' do - let(:params) { nil } + let(:params) { {} } before do stripe.stubs(:create).returns({ 'paid' => true })