Messaging on terms page when user already accepted

We show a blue flash banner if the user already accepted, and show a
button allowing them to continue to the application.

5 files changed, 89 insertions, 3 deletions

diff --git a/spec/controllers/users/terms_controller_spec.rb b/spec/controllers/users/terms_controller_spec.rb
index a744463413c..0d77e91a67d 100644
--- a/spec/controllers/users/terms_controller_spec.rb
+++ b/spec/controllers/users/terms_controller_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe Users::TermsController do
+  include TermsHelper
   let(:user) { create(:user) }
   let(:term) { create(:term) }
 
@@ -15,10 +16,25 @@ describe Users::TermsController do
       expect(response).to have_gitlab_http_status(:redirect)
     end
 
-    it 'shows terms when they exist' do
-      term
+    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
 
-      expect(response).to have_gitlab_http_status(:success)
+      it 'shows a message when the user already accepted the terms' do
+        accept_terms(user)
+
+        get :index
+
+        expect(controller).to set_flash.now[:notice].to(/already accepted/)
+      end
     end
   end
 
diff --git a/spec/factories/term_agreements.rb b/spec/factories/term_agreements.rb
index 557599e663d..3c4eebd0196 100644
--- a/spec/factories/term_agreements.rb
+++ b/spec/factories/term_agreements.rb
@@ -3,4 +3,12 @@ FactoryBot.define do
     term
     user
   end
+
+  trait :declined do
+    accepted false
+  end
+
+  trait :accepted do
+    accepted true
+  end
 end
diff --git a/spec/features/users/terms_spec.rb b/spec/features/users/terms_spec.rb
index 1efa5cd5490..af407c52917 100644
--- a/spec/features/users/terms_spec.rb
+++ b/spec/features/users/terms_spec.rb
@@ -39,6 +39,22 @@ describe 'Users > Terms' do
     end
   end
 
+  context 'when the user has already accepted the terms' do
+    before do
+      accept_terms(user)
+    end
+
+    it 'allows the user to continue to the app' do
+      visit terms_path
+
+      expect(page).to have_content "You have already accepted the Terms of Service as #{user.to_reference}"
+
+      click_link 'Continue'
+
+      expect(current_path).to eq(root_path)
+    end
+  end
+
   context 'terms were enforced while session is active', :js do
     let(:project) { create(:project) }
 
diff --git a/spec/models/application_setting/term_spec.rb b/spec/models/application_setting/term_spec.rb
index 1eddf3c56ff..aa49594f4d1 100644
--- a/spec/models/application_setting/term_spec.rb
+++ b/spec/models/application_setting/term_spec.rb
@@ -12,4 +12,41 @@ describe ApplicationSetting::Term do
       expect(described_class.latest).to eq(terms)
     end
   end
+
+  describe '#accepted_by_user?' do
+    let(:user) { create(:user) }
+    let(:term) { create(:term) }
+
+    it 'is true when the user accepted the terms' do
+      accept_terms(term, user)
+
+      expect(term.accepted_by_user?(user)).to be(true)
+    end
+
+    it 'is false when the user declined the terms' do
+      decline_terms(term, user)
+
+      expect(term.accepted_by_user?(user)).to be(false)
+    end
+
+    it 'does not cause a query when the user accepted the current terms' do
+      accept_terms(term, user)
+
+      expect { term.accepted_by_user?(user) }.not_to exceed_query_limit(0)
+    end
+
+    it 'returns false if the currently accepted terms are different' do
+      accept_terms(create(:term), user)
+
+      expect(term.accepted_by_user?(user)).to be(false)
+    end
+
+    def accept_terms(term, user)
+      Users::RespondToTermsService.new(user, term).execute(accepted: true)
+    end
+
+    def decline_terms(term, user)
+      Users::RespondToTermsService.new(user, term).execute(accepted: false)
+    end
+  end
 end
diff --git a/spec/models/term_agreement_spec.rb b/spec/models/term_agreement_spec.rb
index a59bf119692..950dfa09a6a 100644
--- a/spec/models/term_agreement_spec.rb
+++ b/spec/models/term_agreement_spec.rb
@@ -5,4 +5,13 @@ describe TermAgreement do
     it { is_expected.to validate_presence_of(:term) }
     it { is_expected.to validate_presence_of(:user) }
   end
+
+  describe '.accepted' do
+    it 'only includes accepted terms' do
+      accepted = create(:term_agreement, :accepted)
+      create(:term_agreement, :declined)
+
+      expect(described_class.accepted).to contain_exactly(accepted)
+    end
+  end
 end