From d86fb61c9b9ec7568c91b0ecc69123c74d4709cc Mon Sep 17 00:00:00 2001 From: Robert Fletcher Date: Sat, 27 Mar 2021 18:13:29 -0700 Subject: [PATCH] Specs: add tests for redirects in app.rb Also made it so that auth check applies in test mode as well. Needed to make a couple of other changes to tests to get them working. --- .rubocop_todo.yml | 24 ++++++------ app/helpers/authentication_helpers.rb | 1 - spec/app_spec.rb | 39 +++++++++++++++++++ spec/controllers/first_run_controller_spec.rb | 14 +++---- spec/factories/users.rb | 12 ++++-- spec/helpers/authentications_helper_spec.rb | 9 ----- 6 files changed, 68 insertions(+), 31 deletions(-) create mode 100644 spec/app_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 59c231955..e521faa0d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2021-03-13 20:00:37 UTC using RuboCop version 1.10.0. +# on 2021-03-28 01:11:19 UTC using RuboCop version 1.10.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -44,7 +44,7 @@ Layout/MultilineMethodArgumentLineBreaks: - 'spec/controllers/sessions_controller_spec.rb' - 'spec/javascript/test_controller.rb' -# Offense count: 725 +# Offense count: 744 # Configuration parameters: Only, Ignore. Lint/ConstantResolution: Enabled: false @@ -132,7 +132,7 @@ RSpec/DescribeClass: - 'spec/integration/feed_importing_spec.rb' - 'spec/utils/i18n_support_spec.rb' -# Offense count: 115 +# Offense count: 132 # Cop supports --auto-correct. # Configuration parameters: SkipBlocks, EnforcedStyle. # SupportedStyles: described_class, explicit @@ -244,7 +244,7 @@ RSpec/MessageExpectation: RSpec/MessageSpies: Enabled: false -# Offense count: 94 +# Offense count: 96 RSpec/MultipleExpectations: Max: 8 @@ -331,12 +331,13 @@ Rails/HasManyOrHasOneDependent: Exclude: - 'app/models/group.rb' -# Offense count: 24 +# Offense count: 28 # Cop supports --auto-correct. # Configuration parameters: Include. # Include: spec/**/*, test/**/* Rails/HttpPositionalArguments: Exclude: + - 'spec/app_spec.rb' - 'spec/controllers/debug_controller_spec.rb' - 'spec/controllers/feeds_controller_spec.rb' - 'spec/controllers/first_run_controller_spec.rb' @@ -432,11 +433,10 @@ Rails/WhereEquals: - 'app/models/feed.rb' - 'app/repositories/story_repository.rb' -# Offense count: 2 +# Offense count: 1 # Cop supports --auto-correct. Rails/WhereNot: Exclude: - - 'app/repositories/feed_repository.rb' - 'spec/commands/feeds/import_from_opml_spec.rb' # Offense count: 2 @@ -465,7 +465,7 @@ Style/DisableCopsWithinSourceCodeDirective: Style/DocumentationMethod: Enabled: false -# Offense count: 151 +# Offense count: 152 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. # SupportedStyles: always, always_true, never @@ -477,7 +477,7 @@ Style/InlineComment: Exclude: - 'app/utils/opml_parser.rb' -# Offense count: 670 +# Offense count: 690 # Cop supports --auto-correct. # Configuration parameters: IgnoreMacros, IgnoredMethods, IgnoredPatterns, IncludedMacros, AllowParenthesesInMultilineCall, AllowParenthesesInChaining, AllowParenthesesInCamelCaseMethod, EnforcedStyle. # SupportedStyles: require_parentheses, omit_parentheses @@ -506,7 +506,7 @@ Style/NumericPredicate: Exclude: - 'app/commands/stories/mark_group_as_read.rb' -# Offense count: 30 +# Offense count: 28 # Configuration parameters: SuspiciousParamNames. # SuspiciousParamNames: options, opts, args, params, parameters Style/OptionHash: @@ -563,13 +563,15 @@ Style/StaticClass: - 'app/utils/api_key.rb' - 'app/utils/content_sanitizer.rb' -# Offense count: 18 +# Offense count: 19 # Cop supports --auto-correct. Style/StringHashKeys: Exclude: - 'fever_api.rb' + - 'spec/app_spec.rb' - 'spec/controllers/debug_controller_spec.rb' - 'spec/controllers/feeds_controller_spec.rb' + - 'spec/controllers/first_run_controller_spec.rb' - 'spec/controllers/sessions_controller_spec.rb' - 'spec/fever_api/read_favicons_spec.rb' - 'spec/fever_api/read_feeds_groups_spec.rb' diff --git a/app/helpers/authentication_helpers.rb b/app/helpers/authentication_helpers.rb index d9f564aae..cb70a94f5 100644 --- a/app/helpers/authentication_helpers.rb +++ b/app/helpers/authentication_helpers.rb @@ -9,7 +9,6 @@ def authenticated? end def needs_authentication?(path) - return false if ENV["RACK_ENV"] == "test" return false unless UserRepository.setup_complete? return false if %w(/login /logout /heroku).include?(path) return false if path =~ /css|js|img/ diff --git a/spec/app_spec.rb b/spec/app_spec.rb new file mode 100644 index 000000000..b8faeebf9 --- /dev/null +++ b/spec/app_spec.rb @@ -0,0 +1,39 @@ +require "spec_helper" +require "support/active_record" + +describe "App" do + context "when user is not authenticated and page requires authentication" do + it "sets the session redirect_to" do + create_user(:setup_complete) + + get("/news") + + expect(session[:redirect_to]).to eq("/news") + end + + it "redirects to /login" do + create_user(:setup_complete) + + get("/news") + + expect(last_response).to be_redirect + expect(last_response.headers["Location"]).to end_with("/login") + end + end + + it "does not redirect when page needs no authentication" do + create_user(:setup_complete) + + get("/login") + + expect(last_response).not_to be_redirect + end + + it "does not redirect when user is authenticated" do + user = create_user(:setup_complete) + + get("/news", {}, "rack.session" => { user_id: user.id }) + + expect(last_response).not_to be_redirect + end +end diff --git a/spec/controllers/first_run_controller_spec.rb b/spec/controllers/first_run_controller_spec.rb index 22155e133..878cc3037 100644 --- a/spec/controllers/first_run_controller_spec.rb +++ b/spec/controllers/first_run_controller_spec.rb @@ -1,4 +1,5 @@ require "spec_helper" +require "support/active_record" app_require "controllers/first_run_controller" @@ -72,20 +73,19 @@ end context "when a user has been setup" do - before do - allow(UserRepository).to receive(:setup_complete?).and_return(true) - end - it "should redirect any requests to first run stuff" do - get "/" + user = create_user(:setup_complete) + session = { "rack.session" => { user_id: user.id } } + + get "/", {}, session expect(last_response.status).to be 302 expect(URI.parse(last_response.location).path).to eq "/news" - get "/setup/password" + get "/setup/password", {}, session expect(last_response.status).to be 302 expect(URI.parse(last_response.location).path).to eq "/news" - get "/setup/tutorial" + get "/setup/tutorial", {}, session expect(last_response.status).to be 302 expect(URI.parse(last_response.location).path).to eq "/news" end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 923a74d7f..4a76ebe07 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -1,9 +1,15 @@ module Factories - def create_user(params = {}) - build_user(params).tap(&:save!) + USER_TRAITS = { + setup_complete: -> { { setup_complete: true } } + }.freeze + + def create_user(*traits, **params) + build_user(*traits, **params).tap(&:save!) end - def build_user(params = {}) + def build_user(*traits, **params) + traits.each { |trait| params.merge!(USER_TRAITS.fetch(trait).call) } + User.new(password: "super-secret", **params) end end diff --git a/spec/helpers/authentications_helper_spec.rb b/spec/helpers/authentications_helper_spec.rb index 5407bb13f..eb95f0555 100644 --- a/spec/helpers/authentications_helper_spec.rb +++ b/spec/helpers/authentications_helper_spec.rb @@ -13,18 +13,9 @@ class Helper # rubocop:disable Lint/ConstantDefinitionInBlock let(:authenticated_path) { "/news" } before do - stub_const("ENV", "RACK_ENV" => "not-test") allow(UserRepository).to receive(:setup_complete?).and_return(true) end - context "when `RACK_ENV` is 'test'" do - it "returns false" do - stub_const("ENV", "RACK_ENV" => "test") - - expect(helper.needs_authentication?(authenticated_path)).to eq(false) - end - end - context "when setup in not complete" do it "returns false" do allow(UserRepository).to receive(:setup_complete?).and_return(false)