Commit 49b40e97 authored by Matthias Käppler's avatar Matthias Käppler

Merge branch 'jejacks0n/gitlab-experiment-cleanup-pass' into 'master'

Removes `record!` experiment method

See merge request gitlab-org/gitlab!74648
parents 3bbf1bb1 0d9c8128
...@@ -23,7 +23,7 @@ class Projects::Ci::PipelineEditorController < Projects::ApplicationController ...@@ -23,7 +23,7 @@ class Projects::Ci::PipelineEditorController < Projects::ApplicationController
def setup_walkthrough_experiment def setup_walkthrough_experiment
experiment(:pipeline_editor_walkthrough, namespace: @project.namespace, sticky_to: current_user) do |e| experiment(:pipeline_editor_walkthrough, namespace: @project.namespace, sticky_to: current_user) do |e|
e.candidate {} e.candidate {}
e.record! e.publish_to_database
end end
end end
end end
...@@ -21,7 +21,7 @@ class Projects::LearnGitlabController < Projects::ApplicationController ...@@ -21,7 +21,7 @@ class Projects::LearnGitlabController < Projects::ApplicationController
experiment(:invite_for_help_continuous_onboarding, namespace: project.namespace) do |e| experiment(:invite_for_help_continuous_onboarding, namespace: project.namespace) do |e|
e.candidate {} e.candidate {}
e.record! e.publish_to_database
end end
end end
end end
...@@ -312,7 +312,7 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -312,7 +312,7 @@ class Projects::PipelinesController < Projects::ApplicationController
e.control {} e.control {}
e.candidate {} e.candidate {}
e.record! e.publish_to_database
end end
end end
...@@ -325,7 +325,7 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -325,7 +325,7 @@ class Projects::PipelinesController < Projects::ApplicationController
e.control {} e.control {}
e.candidate {} e.candidate {}
e.record! e.publish_to_database
end end
end end
......
...@@ -13,7 +13,6 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp ...@@ -13,7 +13,6 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp
super super
publish_to_client publish_to_client
publish_to_database if @record
end end
def publish_to_client def publish_to_client
...@@ -25,6 +24,8 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp ...@@ -25,6 +24,8 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp
end end
def publish_to_database def publish_to_database
ActiveSupport::Deprecation.warn('publish_to_database is deprecated and should not be used for reporting anymore')
return unless should_track? return unless should_track?
# if the context contains a namespace, group, project, user, or actor # if the context contains a namespace, group, project, user, or actor
...@@ -36,10 +37,6 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp ...@@ -36,10 +37,6 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp
Experiment.add_subject(name, variant: variant_name || :control, subject: subject) Experiment.add_subject(name, variant: variant_name || :control, subject: subject)
end end
def record!
@record = true
end
def control_behavior def control_behavior
# define a default nil control behavior so we can omit it when not needed # define a default nil control behavior so we can omit it when not needed
end end
......
...@@ -6,7 +6,7 @@ class NewProjectReadmeContentExperiment < ApplicationExperiment # rubocop:disabl ...@@ -6,7 +6,7 @@ class NewProjectReadmeContentExperiment < ApplicationExperiment # rubocop:disabl
def run_with(project, variant: nil) def run_with(project, variant: nil)
@project = project @project = project
record! publish_to_database
run(variant) run(variant)
end end
......
...@@ -29,13 +29,12 @@ module Namespaces ...@@ -29,13 +29,12 @@ module Namespaces
return if email_for_track_sent_to_user? return if email_for_track_sent_to_user?
experiment(:invite_team_email, group: group) do |e| experiment(:invite_team_email, group: group) do |e|
e.publish_to_database
e.candidate do e.candidate do
send_email(user, group) send_email(user, group)
sent_email_records.add(user, track, series) sent_email_records.add(user, track, series)
sent_email_records.save! sent_email_records.save!
end end
e.record!
end end
end end
......
...@@ -394,26 +394,6 @@ You may be asked from time to time to track a specific record ID in experiments. ...@@ -394,26 +394,6 @@ You may be asked from time to time to track a specific record ID in experiments.
The approach is largely up to the PM and engineer creating the implementation. The approach is largely up to the PM and engineer creating the implementation.
No recommendations are provided here at this time. No recommendations are provided here at this time.
### Record experiment subjects
Snowplow tracking of identifiable users or groups is prohibited, but you can still
determine if an experiment is successful or not. We're allowed to record the ID of
a namespace, project or user in our database. Therefore, we can tell the experiment
to record their ID together with the assigned experiment variant in the
`experiment_subjects` database table for later analysis.
For the recording to work, the experiment's context must include a `namespace`,
`group`, `project`, `user`, or `actor`.
To record the experiment subject when you first assign a variant, call `record!` in
the experiment's block:
```ruby
experiment(:pill_color, actor: current_user) do |e|
e.record!
end
```
## Test with RSpec ## Test with RSpec
This gem provides some RSpec helpers and custom matchers. These are in flux as of GitLab 13.10. This gem provides some RSpec helpers and custom matchers. These are in flux as of GitLab 13.10.
......
...@@ -225,9 +225,9 @@ class TrialsController < ApplicationController ...@@ -225,9 +225,9 @@ class TrialsController < ApplicationController
def redirect_trial_user_to_feature_experiment_flow def redirect_trial_user_to_feature_experiment_flow
experiment(:redirect_trial_user_to_feature, namespace: @namespace) do |e| experiment(:redirect_trial_user_to_feature, namespace: @namespace) do |e|
e.record!
e.use { redirect_to group_url(@namespace, { trial: true }) } e.use { redirect_to group_url(@namespace, { trial: true }) }
e.try { redirect_to group_security_dashboard_url(@namespace, { trial: true }) } e.try { redirect_to group_security_dashboard_url(@namespace, { trial: true }) }
e.publish_to_database
end end
end end
......
...@@ -10,7 +10,7 @@ module PaidFeatureCalloutHelper ...@@ -10,7 +10,7 @@ module PaidFeatureCalloutHelper
e.exclude! unless group && eligible_for_trial_upgrade_callout?(group) e.exclude! unless group && eligible_for_trial_upgrade_callout?(group)
e.use { nil } # control gets nothing new added to the existing UI e.use { nil } # control gets nothing new added to the existing UI
e.try(&block) e.try(&block)
e.record! e.publish_to_database
end end
end end
......
...@@ -52,7 +52,7 @@ module TrialStatusWidgetHelper ...@@ -52,7 +52,7 @@ module TrialStatusWidgetHelper
current_user_callout_feature_id(group.trial_days_remaining) current_user_callout_feature_id(group.trial_days_remaining)
) )
end end
e.record! e.publish_to_database
end end
force_popover force_popover
......
...@@ -79,14 +79,6 @@ RSpec.describe ApplicationExperiment, :experiment do ...@@ -79,14 +79,6 @@ RSpec.describe ApplicationExperiment, :experiment do
application_experiment.publish application_experiment.publish
end end
it "publishes to the database if we've opted for that" do
application_experiment.record!
expect(application_experiment).to receive(:publish_to_database)
application_experiment.publish
end
context 'when we should not track' do context 'when we should not track' do
let(:should_track) { false } let(:should_track) { false }
......
...@@ -31,9 +31,10 @@ RSpec.describe "Gitlab::Experiment", :js do ...@@ -31,9 +31,10 @@ RSpec.describe "Gitlab::Experiment", :js do
expect(page).to have_content('Abuse Reports') expect(page).to have_content('Abuse Reports')
published_experiments = page.evaluate_script('window.gon.experiment') published_experiments = page.evaluate_script('window.gl.experiments')
expect(published_experiments).to include({ expect(published_experiments).to include({
'null_hypothesis' => { 'null_hypothesis' => {
'excluded' => false,
'experiment' => 'null_hypothesis', 'experiment' => 'null_hypothesis',
'key' => anything, 'key' => anything,
'variant' => 'candidate' 'variant' => 'candidate'
......
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