Commit 42678f8b authored by Ash McKenzie's avatar Ash McKenzie

Merge branch '119107-respect-dnt-for-experiments' into 'master'

Use DNT: 1 as an experiment opt-out mechanism

Closes #119107

See merge request gitlab-org/gitlab!22100
parents b2662adb 53c94d92
---
title: 'Use DNT: 1 as an experiment opt-out mechanism'
merge_request: 22100
author:
type: other
...@@ -40,7 +40,7 @@ module Gitlab ...@@ -40,7 +40,7 @@ module Gitlab
extend ActiveSupport::Concern extend ActiveSupport::Concern
included do included do
before_action :set_experimentation_subject_id_cookie before_action :set_experimentation_subject_id_cookie, unless: :dnt_enabled?
helper_method :experiment_enabled? helper_method :experiment_enabled?
end end
...@@ -56,7 +56,12 @@ module Gitlab ...@@ -56,7 +56,12 @@ module Gitlab
end end
def experiment_enabled?(experiment_key) def experiment_enabled?(experiment_key)
Experimentation.enabled_for_user?(experiment_key, experimentation_subject_index) || forced_enabled?(experiment_key) return false if dnt_enabled?
return true if Experimentation.enabled_for_user?(experiment_key, experimentation_subject_index)
return true if forced_enabled?(experiment_key)
false
end end
def track_experiment_event(experiment_key, action, value = nil) def track_experiment_event(experiment_key, action, value = nil)
...@@ -73,6 +78,10 @@ module Gitlab ...@@ -73,6 +78,10 @@ module Gitlab
private private
def dnt_enabled?
Gitlab::Utils.to_boolean(request.headers['DNT'])
end
def experimentation_subject_id def experimentation_subject_id
cookies.signed[:experimentation_subject_id] cookies.signed[:experimentation_subject_id]
end end
......
...@@ -30,7 +30,12 @@ describe Gitlab::Experimentation do ...@@ -30,7 +30,12 @@ describe Gitlab::Experimentation do
end end
describe '#set_experimentation_subject_id_cookie' do describe '#set_experimentation_subject_id_cookie' do
let(:do_not_track) { nil }
let(:cookie) { cookies.permanent.signed[:experimentation_subject_id] }
before do before do
request.headers['DNT'] = do_not_track if do_not_track.present?
get :index get :index
end end
...@@ -46,12 +51,30 @@ describe Gitlab::Experimentation do ...@@ -46,12 +51,30 @@ describe Gitlab::Experimentation do
context 'cookie is not present' do context 'cookie is not present' do
it 'sets a permanent signed cookie' do it 'sets a permanent signed cookie' do
expect(cookies.permanent.signed[:experimentation_subject_id]).to be_present expect(cookie).to be_present
end
context 'DNT: 0' do
let(:do_not_Track) { '0' }
it 'sets a permanent signed cookie' do
expect(cookie).to be_present
end
end
context 'DNT: 1' do
let(:do_not_track) { '1' }
it 'does nothing' do
expect(cookie).not_to be_present
end
end end
end end
end end
describe '#experiment_enabled?' do describe '#experiment_enabled?' do
subject { controller.experiment_enabled?(:test_experiment) }
context 'cookie is not present' do context 'cookie is not present' do
it 'calls Gitlab::Experimentation.enabled_for_user? with the name of the experiment and an experimentation_subject_index of nil' do it 'calls Gitlab::Experimentation.enabled_for_user? with the name of the experiment and an experimentation_subject_index of nil' do
expect(Gitlab::Experimentation).to receive(:enabled_for_user?).with(:test_experiment, nil) expect(Gitlab::Experimentation).to receive(:enabled_for_user?).with(:test_experiment, nil)
...@@ -72,11 +95,25 @@ describe Gitlab::Experimentation do ...@@ -72,11 +95,25 @@ describe Gitlab::Experimentation do
end end
end end
it 'returns true when DNT: 0 is set in the request' do
allow(Gitlab::Experimentation).to receive(:enabled_for_user?) { true }
controller.request.headers['DNT'] = '0'
is_expected.to be_truthy
end
it 'returns false when DNT: 1 is set in the request' do
allow(Gitlab::Experimentation).to receive(:enabled_for_user?) { true }
controller.request.headers['DNT'] = '1'
is_expected.to be_falsy
end
describe 'URL parameter to force enable experiment' do describe 'URL parameter to force enable experiment' do
it 'returns true' do it 'returns true unconditionally' do
get :index, params: { force_experiment: :test_experiment } get :index, params: { force_experiment: :test_experiment }
expect(controller.experiment_enabled?(:test_experiment)).to be_truthy is_expected.to be_truthy
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment