Merge branch '204723-nomethoderror-undefined-method-term_agreements-for-nil-nilclass' into 'master'

Resolve "NoMethodError: undefined method `term_agreements' for nil:NilClass"

Closes #204723

See merge request gitlab-org/gitlab!24771

app/controllers/users/terms_controller.rb

@@ -4,7 +4,7 @@ module Users
   class TermsController < ApplicationController
     include InternalRedirect
 
-    skip_before_action :authenticate_user!
+    skip_before_action :authenticate_user!, only: [:index]
     skip_before_action :enforce_terms!
     skip_before_action :check_password_expiration
     skip_before_action :check_two_factor_requirement

changelogs/unreleased/204723-nomethoderror-undefined-method-term_agreements-for-nil-nilclass.yml

+---
+title: Require a logged in user to accept or decline a term
+merge_request: 24771
+author:
+type: fixed

spec/controllers/users/terms_controller_spec.rb

@@ -4,7 +4,8 @@ require 'spec_helper'
 
 describe Users::TermsController do
   include TermsHelper
-  let(:user) { create(:user) }
+
+  let_it_be(:user) { create(:user) }
   let(:term) { create(:term) }
 
   before do
@@ -12,88 +13,145 @@ describe Users::TermsController do
   end
 
   describe 'GET #index' do
-    it 'redirects when no terms exist' do
-      get :index
+    context 'when a user is signed in' do
+      it 'redirects when no terms exist' do
+        get :index
+
+        expect(response).to redirect_to(root_path)
+      end
+
+      context 'when terms exist' do
+        before do
+          stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+          term
+        end
+
+        it 'shows terms when they exist' do
+          get :index
+
+          expect(response).to have_gitlab_http_status(:success)
+        end
+
+        it 'shows a message when the user already accepted the terms' do
+          accept_terms(user)
+
+          get :index
 
-      expect(response).to have_gitlab_http_status(:redirect)
+          expect(controller).to set_flash.now[:notice].to(/already accepted/)
+        end
+      end
     end
 
-    context 'when terms exist' do
+    context 'when a user is not signed in' do
       before do
-        stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
-        term
+        sign_out user
       end
 
-      it 'shows terms when they exist' do
-        get :index
+      context 'when terms exist' do
+        before do
+          stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+          term
+        end
 
-        expect(response).to have_gitlab_http_status(:success)
-      end
+        it 'returns success response' do
+          get :index
 
-      it 'shows a message when the user already accepted the terms' do
-        accept_terms(user)
+          expect(response).to have_gitlab_http_status(:success)
+        end
+      end
 
-        get :index
+      context 'when no terms exist' do
+        it 'redirects' do
+          get :index
 
-        expect(controller).to set_flash.now[:notice].to(/already accepted/)
+          expect(response).to redirect_to(root_path)
+        end
       end
     end
   end
 
   describe 'POST #accept' do
-    it 'saves that the user accepted the terms' do
-      post :accept, params: { id: term.id }
+    context 'when a user is signed in' do
+      it 'saves that the user accepted the terms' do
+        post :accept, params: { id: term.id }
 
-      agreement = user.term_agreements.find_by(term: term)
+        agreement = user.term_agreements.find_by(term: term)
 
-      expect(agreement.accepted).to eq(true)
-    end
+        expect(agreement.accepted).to eq(true)
+      end
 
-    it 'redirects to a path when specified' do
-      post :accept, params: { id: term.id, redirect: groups_path }
+      it 'redirects to a path when specified' do
+        post :accept, params: { id: term.id, redirect: groups_path }
 
-      expect(response).to redirect_to(groups_path)
-    end
+        expect(response).to redirect_to(groups_path)
+      end
 
-    it 'redirects to the referer when no redirect specified' do
-      request.env["HTTP_REFERER"] = groups_url
+      it 'redirects to the referer when no redirect specified' do
+        request.env["HTTP_REFERER"] = groups_url
 
-      post :accept, params: { id: term.id }
+        post :accept, params: { id: term.id }
 
-      expect(response).to redirect_to(groups_path)
-    end
+        expect(response).to redirect_to(groups_path)
+      end
 
-    context 'redirecting to another domain' do
-      it 'is prevented when passing a redirect param' do
-        post :accept, params: { id: term.id, redirect: '//example.com/random/path' }
+      context 'redirecting to another domain' do
+        it 'is prevented when passing a redirect param' do
+          post :accept, params: { id: term.id, redirect: '//example.com/random/path' }
 
-        expect(response).to redirect_to(root_path)
+          expect(response).to redirect_to(root_path)
+        end
+
+        it 'is prevented when redirecting to the referer' do
+          request.env["HTTP_REFERER"] = 'http://example.com/and/a/path'
+
+          post :accept, params: { id: term.id }
+
+          expect(response).to redirect_to(root_path)
+        end
       end
+    end
 
-      it 'is prevented when redirecting to the referer' do
-        request.env["HTTP_REFERER"] = 'http://example.com/and/a/path'
+    context 'when a user is not signed in' do
+      before do
+        sign_out user
+      end
 
+      it 'redirects to login page' do
         post :accept, params: { id: term.id }
 
-        expect(response).to redirect_to(root_path)
+        expect(response).to redirect_to(new_user_session_path)
       end
     end
   end
 
   describe 'POST #decline' do
-    it 'stores that the user declined the terms' do
-      post :decline, params: { id: term.id }
+    context 'when a user is signed in' do
+      it 'stores that the user declined the terms' do
+        post :decline, params: { id: term.id }
+
+        agreement = user.term_agreements.find_by(term: term)
 
-      agreement = user.term_agreements.find_by(term: term)
+        expect(agreement.accepted).to eq(false)
+      end
 
-      expect(agreement.accepted).to eq(false)
+      it 'signs out the user' do
+        post :decline, params: { id: term.id }
+
+        expect(response).to redirect_to(root_path)
+        expect(assigns(:current_user)).to be_nil
+      end
     end
 
-    it 'signs out the user' do
-      post :decline, params: { id: term.id }
+    context 'when a user is not signed in' do
+      before do
+        sign_out user
+      end
 
-      expect(response).to redirect_to(root_path)
-      expect(assigns(:current_user)).to be_nil
+      it 'redirects to login page' do
+        post :decline, params: { id: term.id }
+
+        expect(response).to redirect_to(new_user_session_path)
+      end
     end
   end
 end