Allow a user to accept/decline terms
When a user accepts, we store this in the agreements to keep track of which terms they accepted. We also update the flag on the user.
This commit is contained in:
parent
65bea3f7d0
commit
10aa55a770
|
@ -33,5 +33,13 @@
|
|||
.panel-content {
|
||||
padding: 0 $gl-padding;
|
||||
}
|
||||
|
||||
.footer-block {
|
||||
margin: 0;
|
||||
|
||||
.btn {
|
||||
margin-left: 5px;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -2,18 +2,55 @@ module Users
|
|||
class TermsController < ApplicationController
|
||||
before_action :terms
|
||||
|
||||
|
||||
layout 'terms'
|
||||
|
||||
def index
|
||||
@redirect = redirect_path
|
||||
end
|
||||
|
||||
def accept
|
||||
agreement = Users::RespondToTermsService.new(current_user, viewed_term)
|
||||
.execute(accepted: true)
|
||||
|
||||
if agreement.persisted?
|
||||
redirect_to redirect_path
|
||||
else
|
||||
flash[:alert] = agreement.errors.full_messages.join(', ')
|
||||
redirect_to terms_path, redirect: redirect_path
|
||||
end
|
||||
end
|
||||
|
||||
def decline
|
||||
agreement = Users::RespondToTermsService.new(current_user, viewed_term)
|
||||
.execute(accepted: false)
|
||||
|
||||
if agreement.persisted?
|
||||
sign_out(current_user)
|
||||
redirect_to root_path
|
||||
else
|
||||
flash[:alert] = agreement.errors.full_messages.join(', ')
|
||||
redirect_to terms_path, redirect: redirect_path
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def viewed_term
|
||||
@viewed_term ||= ApplicationSetting::Term.find(params[:id])
|
||||
end
|
||||
|
||||
def terms
|
||||
unless @terms = Gitlab::CurrentSettings.current_application_settings.latest_terms
|
||||
redirect_to request.referer || root_path
|
||||
unless @term = Gitlab::CurrentSettings.current_application_settings.latest_terms
|
||||
redirect_to redirect_path
|
||||
end
|
||||
end
|
||||
|
||||
def redirect_path
|
||||
referer = if request.referer && !request.referer.include?(terms_path)
|
||||
URI(request.referer).path
|
||||
end
|
||||
|
||||
params[:redirect] || referer || root_path
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -38,13 +38,24 @@ module UsersHelper
|
|||
end
|
||||
|
||||
def get_current_user_menu_items
|
||||
items = [:help, :sign_out]
|
||||
items = []
|
||||
|
||||
if can?(current_user, :read_user, current_user)
|
||||
items << :sign_out if current_user
|
||||
|
||||
# TODO: Remove these conditions when the permissions are prevented in
|
||||
# https://gitlab.com/gitlab-org/gitlab-ce/issues/45849
|
||||
terms_not_enforced = !Gitlab::CurrentSettings
|
||||
.current_application_settings
|
||||
.enforce_terms?
|
||||
required_terms_accepted = terms_not_enforced || current_user.terms_accepted?
|
||||
|
||||
items << :help if required_terms_accepted
|
||||
|
||||
if can?(current_user, :read_user, current_user) && required_terms_accepted
|
||||
items << :profile
|
||||
end
|
||||
|
||||
if can?(current_user, :update_user, current_user)
|
||||
if can?(current_user, :update_user, current_user) && required_terms_accepted
|
||||
items << :settings
|
||||
end
|
||||
|
||||
|
|
|
@ -138,6 +138,8 @@ class User < ActiveRecord::Base
|
|||
has_many :custom_attributes, class_name: 'UserCustomAttribute'
|
||||
has_many :callouts, class_name: 'UserCallout'
|
||||
has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
|
||||
has_many :term_agreements
|
||||
belongs_to :accepted_term, class_name: 'ApplicationSetting::Term'
|
||||
|
||||
#
|
||||
# Validations
|
||||
|
@ -1187,6 +1189,10 @@ class User < ActiveRecord::Base
|
|||
max_member_access_for_group_ids([group_id])[group_id]
|
||||
end
|
||||
|
||||
def terms_accepted?
|
||||
accepted_term_id.present?
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
# override, from Devise::Validatable
|
||||
|
|
|
@ -0,0 +1,30 @@
|
|||
class ApplicationSetting
|
||||
class TermPolicy < BasePolicy
|
||||
include Gitlab::Utils::StrongMemoize
|
||||
|
||||
condition(:logged_in, scope: :user) { @user }
|
||||
|
||||
condition(:current_terms, scope: :subject) do
|
||||
Gitlab::CurrentSettings.current_application_settings.latest_terms == @subject
|
||||
end
|
||||
|
||||
condition(:terms_accepted, scope: :user, score: 1) do
|
||||
agreement&.accepted
|
||||
end
|
||||
|
||||
rule { logged_in & current_terms }.policy do
|
||||
enable :accept_terms
|
||||
enable :decline_terms
|
||||
end
|
||||
|
||||
rule { terms_accepted }.prevent :accept_terms
|
||||
|
||||
def agreement
|
||||
strong_memoize(:agreement) do
|
||||
next nil if @user.nil? || @subject.nil?
|
||||
|
||||
@user.term_agreements.find_by(term: @subject)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,24 @@
|
|||
module Users
|
||||
class RespondToTermsService
|
||||
def initialize(user, term)
|
||||
@user, @term = user, term
|
||||
end
|
||||
|
||||
def execute(accepted:)
|
||||
agreement = @user.term_agreements.find_or_initialize_by(term: @term)
|
||||
agreement.accepted = accepted
|
||||
|
||||
if agreement.save
|
||||
store_accepted_term(accepted)
|
||||
end
|
||||
|
||||
agreement
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def store_accepted_term(accepted)
|
||||
@user.update_column(:accepted_term_id, accepted ? @term.id : nil)
|
||||
end
|
||||
end
|
||||
end
|
|
@ -3,11 +3,18 @@
|
|||
%html{ lang: I18n.locale, class: page_class }
|
||||
= render "layouts/head"
|
||||
|
||||
%body{ class: "#{user_application_theme} #{@body_class}", data: { page: body_data_page } }
|
||||
%body{ data: { page: body_data_page } }
|
||||
= render 'peek/bar'
|
||||
.layout-page.terms
|
||||
.content-wrapper
|
||||
%div{ class: "#{(container_class unless @no_container)} #{@content_class}" }
|
||||
.mobile-overlay
|
||||
.alert-wrapper
|
||||
= render "layouts/broadcast"
|
||||
= render 'layouts/header/read_only_banner'
|
||||
= yield :flash_message
|
||||
= render "layouts/flash"
|
||||
|
||||
%div{ class: "#{container_class}" }
|
||||
.content{ id: "content-body" }
|
||||
.panel.panel-default
|
||||
.panel-heading
|
||||
|
|
|
@ -1,2 +1,12 @@
|
|||
- redirect_params = { redirect: @redirect } if @redirect
|
||||
.panel-content.rendered-terms
|
||||
= markdown_field(@terms, :terms)
|
||||
= markdown_field(@term, :terms)
|
||||
.row-content-block.footer-block.clearfix
|
||||
- if can?(current_user, :accept_terms, @term)
|
||||
.pull-right
|
||||
= button_to accept_term_path(@term, redirect_params), class: 'btn btn-success prepend-left-8' do
|
||||
= _('Accept terms')
|
||||
- if can?(current_user, :decline_terms, @term)
|
||||
.pull-right
|
||||
= button_to decline_term_path(@term, redirect_params), class: 'btn btn-default prepend-left-8' do
|
||||
= _('Decline and sign out')
|
||||
|
|
|
@ -2,6 +2,7 @@ require 'spec_helper'
|
|||
|
||||
describe Users::TermsController do
|
||||
let(:user) { create(:user) }
|
||||
let(:term) { create(:term) }
|
||||
|
||||
before do
|
||||
sign_in user
|
||||
|
@ -15,9 +16,42 @@ describe Users::TermsController do
|
|||
end
|
||||
|
||||
it 'shows terms when they exist' do
|
||||
create(:term)
|
||||
term
|
||||
|
||||
expect(response).to have_gitlab_http_status(:success)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'POST #accept' do
|
||||
it 'saves that the user accepted the terms' do
|
||||
post :accept, id: term.id
|
||||
|
||||
agreement = user.term_agreements.find_by(term: term)
|
||||
|
||||
expect(agreement.accepted).to eq(true)
|
||||
end
|
||||
|
||||
it 'redirects to a path when specified' do
|
||||
post :accept, id: term.id, redirect: groups_path
|
||||
|
||||
expect(response).to redirect_to(groups_path)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'POST #decline' do
|
||||
it 'stores that the user declined the terms' do
|
||||
post :decline, id: term.id
|
||||
|
||||
agreement = user.term_agreements.find_by(term: term)
|
||||
|
||||
expect(agreement.accepted).to eq(false)
|
||||
end
|
||||
|
||||
it 'signs out the user' do
|
||||
post :decline, id: term.id
|
||||
|
||||
expect(response).to redirect_to(root_path)
|
||||
expect(assigns(:current_user)).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,6 @@
|
|||
FactoryBot.define do
|
||||
factory :term_agreement do
|
||||
term
|
||||
user
|
||||
end
|
||||
end
|
|
@ -5,12 +5,35 @@ describe 'Users > Terms' do
|
|||
let!(:term) { create(:term, terms: 'By accepting, you promise to be nice!') }
|
||||
|
||||
before do
|
||||
stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
|
||||
sign_in(user)
|
||||
|
||||
visit terms_path
|
||||
end
|
||||
|
||||
it 'shows the terms' do
|
||||
visit terms_path
|
||||
|
||||
expect(page).to have_content('By accepting, you promise to be nice!')
|
||||
end
|
||||
|
||||
context 'declining the terms' do
|
||||
it 'returns the user to the app' do
|
||||
visit terms_path
|
||||
|
||||
click_button 'Decline and sign out'
|
||||
|
||||
expect(page).not_to have_content(term.terms)
|
||||
expect(user.reload.terms_accepted?).to be(false)
|
||||
end
|
||||
end
|
||||
|
||||
context 'accepting the terms' do
|
||||
it 'returns the user to the app' do
|
||||
visit terms_path
|
||||
|
||||
click_button 'Accept terms'
|
||||
|
||||
expect(page).not_to have_content(term.terms)
|
||||
expect(user.reload.terms_accepted?).to be(true)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,6 +1,8 @@
|
|||
require 'rails_helper'
|
||||
|
||||
describe UsersHelper do
|
||||
include TermsHelper
|
||||
|
||||
let(:user) { create(:user) }
|
||||
|
||||
describe '#user_link' do
|
||||
|
@ -51,5 +53,15 @@ describe UsersHelper do
|
|||
|
||||
expect(items).to include(:profile)
|
||||
end
|
||||
|
||||
context 'when terms are enforced' do
|
||||
before do
|
||||
enforce_terms
|
||||
end
|
||||
|
||||
it 'hides the profile and the settings tab' do
|
||||
expect(items).not_to include(:settings, :profile, :help)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,50 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe ApplicationSetting::TermPolicy do
|
||||
include TermsHelper
|
||||
|
||||
set(:term) { create(:term) }
|
||||
let(:user) { create(:user) }
|
||||
|
||||
subject(:policy) { described_class.new(user, term) }
|
||||
|
||||
before do
|
||||
stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
|
||||
end
|
||||
|
||||
it 'has the correct permissions', :aggregate_failures do
|
||||
is_expected.to be_allowed(:accept_terms)
|
||||
is_expected.to be_allowed(:decline_terms)
|
||||
end
|
||||
|
||||
context 'for anonymous users' do
|
||||
let(:user) { nil }
|
||||
|
||||
it 'has the correct permissions', :aggregate_failures do
|
||||
is_expected.to be_disallowed(:accept_terms)
|
||||
is_expected.to be_disallowed(:decline_terms)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the terms are not current' do
|
||||
before do
|
||||
create(:term)
|
||||
end
|
||||
|
||||
it 'has the correct permissions', :aggregate_failures do
|
||||
is_expected.to be_disallowed(:accept_terms)
|
||||
is_expected.to be_disallowed(:decline_terms)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the user already accepted the terms' do
|
||||
before do
|
||||
accept_terms(user)
|
||||
end
|
||||
|
||||
it 'has the correct permissions', :aggregate_failures do
|
||||
is_expected.to be_disallowed(:accept_terms)
|
||||
is_expected.to be_allowed(:decline_terms)
|
||||
end
|
||||
end
|
||||
end
|
|
@ -0,0 +1,37 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Users::RespondToTermsService do
|
||||
let(:user) { create(:user) }
|
||||
let(:term) { create(:term) }
|
||||
|
||||
subject(:service) { described_class.new(user, term) }
|
||||
|
||||
describe '#execute' do
|
||||
it 'creates a new agreement if it did not exist' do
|
||||
expect { service.execute(accepted: true) }
|
||||
.to change { user.term_agreements.size }.by(1)
|
||||
end
|
||||
|
||||
it 'updates an agreement if it existed' do
|
||||
agreement = create(:term_agreement, user: user, term: term, accepted: true)
|
||||
|
||||
service.execute(accepted: true)
|
||||
|
||||
expect(agreement.reload.accepted).to be_truthy
|
||||
end
|
||||
|
||||
it 'adds the accepted terms to the user' do
|
||||
service.execute(accepted: true)
|
||||
|
||||
expect(user.reload.accepted_term).to eq(term)
|
||||
end
|
||||
|
||||
it 'removes accepted terms when declining' do
|
||||
user.update!(accepted_term: term)
|
||||
|
||||
service.execute(accepted: false)
|
||||
|
||||
expect(user.reload.accepted_term).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue