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

@@ -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'

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/

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

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

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

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)