From f2965e6673f9670f5867f8a007289def3041dc70 Mon Sep 17 00:00:00 2001 From: Philipp Rothmann Date: Mon, 5 Dec 2022 10:13:02 +0100 Subject: [PATCH 1/5] Add home controller test Co-authored-by: viehlieb Co-authored-by: Tobias Kneuker --- Gemfile | 1 + Gemfile.lock | 5 + app/controllers/home_controller.rb | 6 +- spec/controllers/home_controller_spec.rb | 199 +++++++++++++++++++++++ spec/spec_helper.rb | 6 +- spec/support/spec_test_helper.rb | 23 +++ 6 files changed, 236 insertions(+), 4 deletions(-) create mode 100644 spec/controllers/home_controller_spec.rb create mode 100644 spec/support/spec_test_helper.rb diff --git a/Gemfile b/Gemfile index a6e27fae..56f320cf 100644 --- a/Gemfile +++ b/Gemfile @@ -112,6 +112,7 @@ group :test do gem 'rspec-core' gem 'rspec-rerun' gem 'i18n-spec' + gem 'rails-controller-testing' # code coverage gem 'simplecov', require: false gem 'simplecov-lcov', require: false diff --git a/Gemfile.lock b/Gemfile.lock index c53687fb..97f90f3e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -350,6 +350,10 @@ GEM sprockets-rails (>= 2.0.0) rails-assets-listjs (0.2.0.beta.4) railties (>= 3.1) + rails-controller-testing (1.0.5) + actionpack (>= 5.0.1.rc1) + actionview (>= 5.0.1.rc1) + activesupport (>= 5.0.1.rc1) rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) @@ -606,6 +610,7 @@ DEPENDENCIES rack-cors rails (~> 5.2) rails-assets-listjs (= 0.2.0.beta.4) + rails-controller-testing rails-i18n rails-settings-cached (= 0.4.3) rails_tokeninput diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 86f9e2eb..d01a78ca 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -18,7 +18,7 @@ class HomeController < ApplicationController @bank_accounts = @types.includes(:bank_account).map(&:bank_account).uniq.compact @bank_accounts = [BankAccount.last] if @bank_accounts.empty? else - redirect_to root_url, alert: I18n.t('group_orders.errors.no_member') + redirect_to root_path, alert: I18n.t('group_orders.errors.no_member') end end @@ -26,7 +26,7 @@ class HomeController < ApplicationController if @current_user.update(user_params) @current_user.ordergroup.update(ordergroup_params) if ordergroup_params session[:locale] = @current_user.locale - redirect_to my_profile_url, notice: I18n.t('home.changes_saved') + redirect_to my_profile_path, notice: I18n.t('home.changes_saved') else render :profile end @@ -64,7 +64,7 @@ class HomeController < ApplicationController # cancel personal memberships direct from the myProfile-page def cancel_membership if params[:membership_id] - membership = @current_user.memberships.find!(params[:membership_id]) + membership = @current_user.memberships.find(params[:membership_id]) else membership = @current_user.memberships.find_by_group_id!(params[:group_id]) end diff --git a/spec/controllers/home_controller_spec.rb b/spec/controllers/home_controller_spec.rb new file mode 100644 index 00000000..f3616cd4 --- /dev/null +++ b/spec/controllers/home_controller_spec.rb @@ -0,0 +1,199 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe HomeController, type: :controller do + let(:user) { create :user } + + describe 'GET index' do + describe 'NOT logged in' do + it 'redirects' do + get_with_defaults :profile + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to(login_path) + end + end + + describe 'logegd in' do + before { login user } + + it 'assigns tasks' do + get_with_defaults :index + + expect(assigns(:unaccepted_tasks)).not_to be_nil + expect(assigns(:next_tasks)).not_to be_nil + expect(assigns(:unassigned_tasks)).not_to be_nil + expect(response).to render_template('home/index') + end + end + end + + describe 'GET profile' do + before { login user } + + it 'renders dashboard' do + get_with_defaults :profile + expect(response).to have_http_status(:success) + expect(response).to render_template('home/profile') + end + end + + describe 'GET reference_calculator' do + describe 'with simple user' do + before { login user } + + it 'redirects to home' do + get_with_defaults :reference_calculator + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to(root_path) + end + end + + describe 'with ordergroup user' do + let(:og_user) { create :user, :ordergroup } + + before { login og_user } + + it 'renders reference calculator' do + get_with_defaults :reference_calculator + expect(response).to have_http_status(:success) + expect(response).to render_template('home/reference_calculator') + end + end + end + + describe 'GET update_profile' do + describe 'with simple user' do + let(:unchanged_attributes) { user.attributes.slice('first_name', 'last_name', 'email') } + let(:changed_attributes) { attributes_for :user } + let(:invalid_attributes) { { email: 'e.mail.com' } } + + before { login user } + + it 'renders profile after update with invalid attributes' do + get_with_defaults :update_profile, params: { user: invalid_attributes } + expect(response).to have_http_status(:success) + expect(response).to render_template('home/profile') + expect(assigns(:current_user).errors.present?).to be true + end + + it 'redirects to profile after update with unchanged attributes' do + get_with_defaults :update_profile, params: { user: unchanged_attributes } + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to(my_profile_path) + end + + it 'redirects to profile after update' do + patch :update_profile, params: { foodcoop: FoodsoftConfig[:default_scope], user: changed_attributes } + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to(my_profile_path) + expect(flash[:notice]).to match(/#{I18n.t('home.changes_saved')}/) + expect(user.reload.attributes.slice(:first_name, :last_name, :email)).to eq(changed_attributes.slice('first_name', 'last_name', 'email')) + end + end + + describe 'with ordergroup user' do + let(:og_user) { create :user, :ordergroup } + let(:unchanged_attributes) { og_user.attributes.slice('first_name', 'last_name', 'email') } + let(:changed_attributes) { unchanged_attributes.merge({ ordergroup: { contact_address: 'new Adress 7' } }) } + + before { login og_user } + + it 'redirects to home after update' do + get_with_defaults :update_profile, params: { user: changed_attributes } + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to(my_profile_path) + expect(og_user.reload.ordergroup.contact_address).to eq('new Adress 7') + end + end + end + + describe 'GET ordergroup' do + describe 'with simple user' do + before { login user } + + it 'redirects to home' do + get_with_defaults :ordergroup + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to(root_path) + end + end + + describe 'with ordergroup user' do + let(:og_user) { create :user, :ordergroup } + + before { login og_user } + + it 'renders ordergroup' do + get_with_defaults :ordergroup + expect(response).to have_http_status(:success) + expect(response).to render_template('home/ordergroup') + end + + describe 'assigns sortings' do + let(:fin_trans1) { create :financial_transaction, user: og_user, ordergroup: og_user.ordergroup, note: 'A', amount: 200, created_on: Time.now } + let(:fin_trans2) { create :financial_transaction, user: og_user, ordergroup: og_user.ordergroup, note: 'B', amount: 100, created_on: Time.now + 2.minutes } + let(:fin_trans3) { create :financial_transaction, user: og_user, ordergroup: og_user.ordergroup, note: 'C', amount: 50, created_on: Time.now + 1.minute } + + before do + fin_trans1 + fin_trans2 + fin_trans3 + end + + it 'by criteria' do + sortings = [ + ['date', [fin_trans1, fin_trans3, fin_trans2]], + ['note', [fin_trans1, fin_trans2, fin_trans3]], + ['amount', [fin_trans3, fin_trans2, fin_trans1]], + ['date_reverse', [fin_trans2, fin_trans3, fin_trans1]], + ['note_reverse', [fin_trans3, fin_trans2, fin_trans1]], + ['amount_reverse', [fin_trans1, fin_trans2, fin_trans3]] + ] + sortings.each do |sorting| + get_with_defaults :ordergroup, params: { sort: sorting[0] } + expect(response).to have_http_status(:success) + expect(assigns(:financial_transactions).to_a).to eq(sorting[1]) + end + end + end + end + end + + describe 'GET cancel_membership' do + describe 'with simple user without group' do + before { login user } + + it 'fails' do + expect do + get_with_defaults :cancel_membership + end.to raise_error(ActiveRecord::RecordNotFound) + expect do + get_with_defaults :cancel_membership, params: { membership_id: 424242 } + end.to raise_error(ActiveRecord::RecordNotFound) + end + end + + describe 'with ordergroup user' do + let(:fin_user) { create :user, :role_finance } + + before { login fin_user } + + it 'removes user from group' do + membership = fin_user.memberships.first + get_with_defaults :cancel_membership, params: { group_id: fin_user.groups.first.id } + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to(my_profile_path) + expect(flash[:notice]).to match(/#{I18n.t('home.ordergroup_cancelled', group: membership.group.name)}/) + end + + it 'removes user membership' do + membership = fin_user.memberships.first + get_with_defaults :cancel_membership, params: { membership_id: membership.id } + expect(response).to have_http_status(:redirect) + expect(response).to redirect_to(my_profile_path) + expect(flash[:notice]).to match(/#{I18n.t('home.ordergroup_cancelled', group: membership.group.name)}/) + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 88dea423..41894406 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -21,6 +21,10 @@ Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } RSpec.configure do |config| # We use capybara with webkit, and need database_cleaner + config.before(:suite) do + DatabaseCleaner.clean_with(:truncation) + end + config.before(:each) do DatabaseCleaner.strategy = (RSpec.current_example.metadata[:js] ? :truncation : :transaction) DatabaseCleaner.start @@ -51,8 +55,8 @@ RSpec.configure do |config| # --seed 1234 config.order = "random" + config.include SpecTestHelper, type: :controller config.include SessionHelper, type: :feature - # Automatically determine spec from directory structure, see: # https://www.relishapp.com/rspec/rspec-rails/v/3-0/docs/directory-structure config.infer_spec_type_from_file_location! diff --git a/spec/support/spec_test_helper.rb b/spec/support/spec_test_helper.rb new file mode 100644 index 00000000..f3737c15 --- /dev/null +++ b/spec/support/spec_test_helper.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module SpecTestHelper + def login(user) + user = User.find_by_nick(user.nick) + session[:user_id] = user.id + session[:scope] = FoodsoftConfig[:default_scope] # Save scope in session to not allow switching between foodcoops with one account + session[:locale] = user.locale + end + + def current_user + User.find(session[:user_id]) + end + + def get_with_defaults(action, params: {}, xhr: false, format: nil) + params['foodcoop'] = FoodsoftConfig[:default_scope] + get action, params: params, xhr: xhr, format: format + end +end + +RSpec.configure do |config| + config.include SpecTestHelper, type: :controller +end From 33eec7aa25658aa6472f8a20b66b2c435b3691b6 Mon Sep 17 00:00:00 2001 From: Philipp Rothmann Date: Mon, 6 Mar 2023 12:23:46 +0100 Subject: [PATCH 2/5] remove assigns, render_template to respect controller test boundaries --- Gemfile | 1 - Gemfile.lock | 7 +--- spec/controllers/home_controller_spec.rb | 51 ++++-------------------- 3 files changed, 8 insertions(+), 51 deletions(-) diff --git a/Gemfile b/Gemfile index 56f320cf..a6e27fae 100644 --- a/Gemfile +++ b/Gemfile @@ -112,7 +112,6 @@ group :test do gem 'rspec-core' gem 'rspec-rerun' gem 'i18n-spec' - gem 'rails-controller-testing' # code coverage gem 'simplecov', require: false gem 'simplecov-lcov', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 97f90f3e..0fc4df57 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -350,10 +350,6 @@ GEM sprockets-rails (>= 2.0.0) rails-assets-listjs (0.2.0.beta.4) railties (>= 3.1) - rails-controller-testing (1.0.5) - actionpack (>= 5.0.1.rc1) - actionview (>= 5.0.1.rc1) - activesupport (>= 5.0.1.rc1) rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) @@ -610,7 +606,6 @@ DEPENDENCIES rack-cors rails (~> 5.2) rails-assets-listjs (= 0.2.0.beta.4) - rails-controller-testing rails-i18n rails-settings-cached (= 0.4.3) rails_tokeninput @@ -647,4 +642,4 @@ DEPENDENCIES whenever BUNDLED WITH - 1.17.3 + 2.4.5 diff --git a/spec/controllers/home_controller_spec.rb b/spec/controllers/home_controller_spec.rb index f3616cd4..c5732bd9 100644 --- a/spec/controllers/home_controller_spec.rb +++ b/spec/controllers/home_controller_spec.rb @@ -14,16 +14,12 @@ describe HomeController, type: :controller do end end - describe 'logegd in' do + describe 'logged in' do before { login user } - it 'assigns tasks' do + it 'succeeds' do get_with_defaults :index - - expect(assigns(:unaccepted_tasks)).not_to be_nil - expect(assigns(:next_tasks)).not_to be_nil - expect(assigns(:unassigned_tasks)).not_to be_nil - expect(response).to render_template('home/index') + expect(response).to have_http_status(:success) end end end @@ -31,10 +27,9 @@ describe HomeController, type: :controller do describe 'GET profile' do before { login user } - it 'renders dashboard' do + it 'succeeds' do get_with_defaults :profile expect(response).to have_http_status(:success) - expect(response).to render_template('home/profile') end end @@ -54,10 +49,9 @@ describe HomeController, type: :controller do before { login og_user } - it 'renders reference calculator' do + it 'succeeds' do get_with_defaults :reference_calculator expect(response).to have_http_status(:success) - expect(response).to render_template('home/reference_calculator') end end end @@ -70,11 +64,9 @@ describe HomeController, type: :controller do before { login user } - it 'renders profile after update with invalid attributes' do + it 'stays on profile after update with invalid attributes' do get_with_defaults :update_profile, params: { user: invalid_attributes } expect(response).to have_http_status(:success) - expect(response).to render_template('home/profile') - expect(assigns(:current_user).errors.present?).to be true end it 'redirects to profile after update with unchanged attributes' do @@ -124,38 +116,9 @@ describe HomeController, type: :controller do before { login og_user } - it 'renders ordergroup' do + it 'succeeds' do get_with_defaults :ordergroup expect(response).to have_http_status(:success) - expect(response).to render_template('home/ordergroup') - end - - describe 'assigns sortings' do - let(:fin_trans1) { create :financial_transaction, user: og_user, ordergroup: og_user.ordergroup, note: 'A', amount: 200, created_on: Time.now } - let(:fin_trans2) { create :financial_transaction, user: og_user, ordergroup: og_user.ordergroup, note: 'B', amount: 100, created_on: Time.now + 2.minutes } - let(:fin_trans3) { create :financial_transaction, user: og_user, ordergroup: og_user.ordergroup, note: 'C', amount: 50, created_on: Time.now + 1.minute } - - before do - fin_trans1 - fin_trans2 - fin_trans3 - end - - it 'by criteria' do - sortings = [ - ['date', [fin_trans1, fin_trans3, fin_trans2]], - ['note', [fin_trans1, fin_trans2, fin_trans3]], - ['amount', [fin_trans3, fin_trans2, fin_trans1]], - ['date_reverse', [fin_trans2, fin_trans3, fin_trans1]], - ['note_reverse', [fin_trans3, fin_trans2, fin_trans1]], - ['amount_reverse', [fin_trans1, fin_trans2, fin_trans3]] - ] - sortings.each do |sorting| - get_with_defaults :ordergroup, params: { sort: sorting[0] } - expect(response).to have_http_status(:success) - expect(assigns(:financial_transactions).to_a).to eq(sorting[1]) - end - end end end end From afd82a4901f1293404fe2fbdaf9586f38e71a9ee Mon Sep 17 00:00:00 2001 From: Philipp Rothmann Date: Mon, 6 Mar 2023 12:38:25 +0100 Subject: [PATCH 3/5] remove unused truncation database cleaner strategy --- spec/spec_helper.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 41894406..8b1c6ace 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -21,10 +21,6 @@ Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } RSpec.configure do |config| # We use capybara with webkit, and need database_cleaner - config.before(:suite) do - DatabaseCleaner.clean_with(:truncation) - end - config.before(:each) do DatabaseCleaner.strategy = (RSpec.current_example.metadata[:js] ? :truncation : :transaction) DatabaseCleaner.start From 54e11dc838a74ea25f0ac509550ac05a90110574 Mon Sep 17 00:00:00 2001 From: Philipp Rothmann Date: Mon, 6 Mar 2023 12:49:14 +0100 Subject: [PATCH 4/5] revert changes on home controller --- app/controllers/home_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index d01a78ca..a3d9cd53 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -18,7 +18,7 @@ class HomeController < ApplicationController @bank_accounts = @types.includes(:bank_account).map(&:bank_account).uniq.compact @bank_accounts = [BankAccount.last] if @bank_accounts.empty? else - redirect_to root_path, alert: I18n.t('group_orders.errors.no_member') + redirect_to root_url, alert: I18n.t('group_orders.errors.no_member') end end @@ -26,7 +26,7 @@ class HomeController < ApplicationController if @current_user.update(user_params) @current_user.ordergroup.update(ordergroup_params) if ordergroup_params session[:locale] = @current_user.locale - redirect_to my_profile_path, notice: I18n.t('home.changes_saved') + redirect_to my_profile_url, notice: I18n.t('home.changes_saved') else render :profile end From b78e6a93e2590a382a5b75a0aea1d5ccfaa06dc7 Mon Sep 17 00:00:00 2001 From: Philipp Rothmann Date: Sat, 25 Mar 2023 18:08:43 +0100 Subject: [PATCH 5/5] reset Gemfile.lock --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 0fc4df57..c53687fb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -642,4 +642,4 @@ DEPENDENCIES whenever BUNDLED WITH - 2.4.5 + 1.17.3