From 0d9c8128d49123980943e302e1b78dca7c79ec4c Mon Sep 17 00:00:00 2001
From: jejacks0n <jjackson@gitlab.com>
Date: Tue, 16 Nov 2021 14:23:23 -0700
Subject: [PATCH] Removes `record!` experiment method
- Deprecates the usage of `publish_to_database` as well
---
.../projects/ci/pipeline_editor_controller.rb | 2 +-
.../projects/learn_gitlab_controller.rb | 2 +-
.../projects/pipelines_controller.rb | 4 ++--
app/experiments/application_experiment.rb | 7 ++-----
.../new_project_readme_content_experiment.rb | 2 +-
.../namespaces/invite_team_email_service.rb | 3 +--
.../experiment_guide/gitlab_experiment.md | 20 -------------------
ee/app/controllers/trials_controller.rb | 2 +-
ee/app/helpers/paid_feature_callout_helper.rb | 2 +-
ee/app/helpers/trial_status_widget_helper.rb | 2 +-
.../application_experiment_spec.rb | 8 --------
spec/features/gitlab_experiments_spec.rb | 3 ++-
12 files changed, 13 insertions(+), 44 deletions(-)
diff --git a/app/controllers/projects/ci/pipeline_editor_controller.rb b/app/controllers/projects/ci/pipeline_editor_controller.rb
index 392a6afc63686..6f12e3940dd59 100644
--- a/app/controllers/projects/ci/pipeline_editor_controller.rb
+++ b/app/controllers/projects/ci/pipeline_editor_controller.rb
@@ -23,7 +23,7 @@ def check_can_collaborate!
def setup_walkthrough_experiment
experiment(:pipeline_editor_walkthrough, namespace: @project.namespace, sticky_to: current_user) do |e|
e.candidate {}
- e.record!
+ e.publish_to_database
end
end
end
diff --git a/app/controllers/projects/learn_gitlab_controller.rb b/app/controllers/projects/learn_gitlab_controller.rb
index d5675618da569..177533b89c871 100644
--- a/app/controllers/projects/learn_gitlab_controller.rb
+++ b/app/controllers/projects/learn_gitlab_controller.rb
@@ -21,7 +21,7 @@ def enable_invite_for_help_continuous_onboarding_experiment
experiment(:invite_for_help_continuous_onboarding, namespace: project.namespace) do |e|
e.candidate {}
- e.record!
+ e.publish_to_database
end
end
end
diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index d49539c56fb8c..79935300fb6f5 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -312,7 +312,7 @@ def enable_code_quality_walkthrough_experiment
e.control {}
e.candidate {}
- e.record!
+ e.publish_to_database
end
end
@@ -325,7 +325,7 @@ def enable_ci_runner_templates_experiment
e.control {}
e.candidate {}
- e.record!
+ e.publish_to_database
end
end
diff --git a/app/experiments/application_experiment.rb b/app/experiments/application_experiment.rb
index 22c632a08aa2f..783e92c646036 100644
--- a/app/experiments/application_experiment.rb
+++ b/app/experiments/application_experiment.rb
@@ -13,7 +13,6 @@ def publish(_result = nil)
super
publish_to_client
- publish_to_database if @record
end
def publish_to_client
@@ -25,6 +24,8 @@ def publish_to_client
end
def publish_to_database
+ ActiveSupport::Deprecation.warn('publish_to_database is deprecated and should not be used for reporting anymore')
+
return unless should_track?
# if the context contains a namespace, group, project, user, or actor
@@ -36,10 +37,6 @@ def publish_to_database
Experiment.add_subject(name, variant: variant_name || :control, subject: subject)
end
- def record!
- @record = true
- end
-
def control_behavior
# define a default nil control behavior so we can omit it when not needed
end
diff --git a/app/experiments/new_project_readme_content_experiment.rb b/app/experiments/new_project_readme_content_experiment.rb
index d9f0fb3b93eb9..1de7632268dcc 100644
--- a/app/experiments/new_project_readme_content_experiment.rb
+++ b/app/experiments/new_project_readme_content_experiment.rb
@@ -6,7 +6,7 @@ class NewProjectReadmeContentExperiment < ApplicationExperiment # rubocop:disabl
def run_with(project, variant: nil)
@project = project
- record!
+ publish_to_database
run(variant)
end
diff --git a/app/services/namespaces/invite_team_email_service.rb b/app/services/namespaces/invite_team_email_service.rb
index 45975d1953a88..78edc20599034 100644
--- a/app/services/namespaces/invite_team_email_service.rb
+++ b/app/services/namespaces/invite_team_email_service.rb
@@ -29,13 +29,12 @@ def execute
return if email_for_track_sent_to_user?
experiment(:invite_team_email, group: group) do |e|
+ e.publish_to_database
e.candidate do
send_email(user, group)
sent_email_records.add(user, track, series)
sent_email_records.save!
end
-
- e.record!
end
end
diff --git a/doc/development/experiment_guide/gitlab_experiment.md b/doc/development/experiment_guide/gitlab_experiment.md
index d81ac5372e38c..288823bb41fcc 100644
--- a/doc/development/experiment_guide/gitlab_experiment.md
+++ b/doc/development/experiment_guide/gitlab_experiment.md
@@ -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.
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
This gem provides some RSpec helpers and custom matchers. These are in flux as of GitLab 13.10.
diff --git a/ee/app/controllers/trials_controller.rb b/ee/app/controllers/trials_controller.rb
index d692c3104333d..dacb91f32af52 100644
--- a/ee/app/controllers/trials_controller.rb
+++ b/ee/app/controllers/trials_controller.rb
@@ -225,9 +225,9 @@ def remove_known_trial_form_fields_context
def redirect_trial_user_to_feature_experiment_flow
experiment(:redirect_trial_user_to_feature, namespace: @namespace) do |e|
- e.record!
e.use { redirect_to group_url(@namespace, { trial: true }) }
e.try { redirect_to group_security_dashboard_url(@namespace, { trial: true }) }
+ e.publish_to_database
end
end
diff --git a/ee/app/helpers/paid_feature_callout_helper.rb b/ee/app/helpers/paid_feature_callout_helper.rb
index d8348a14f7ae7..6c2b292c0ea85 100644
--- a/ee/app/helpers/paid_feature_callout_helper.rb
+++ b/ee/app/helpers/paid_feature_callout_helper.rb
@@ -10,7 +10,7 @@ def run_highlight_paid_features_during_active_trial_experiment(group, &block)
e.exclude! unless group && eligible_for_trial_upgrade_callout?(group)
e.use { nil } # control gets nothing new added to the existing UI
e.try(&block)
- e.record!
+ e.publish_to_database
end
end
diff --git a/ee/app/helpers/trial_status_widget_helper.rb b/ee/app/helpers/trial_status_widget_helper.rb
index d9ea6af095b39..c0c017469625d 100644
--- a/ee/app/helpers/trial_status_widget_helper.rb
+++ b/ee/app/helpers/trial_status_widget_helper.rb
@@ -52,7 +52,7 @@ def force_popover_to_be_shown?(group)
current_user_callout_feature_id(group.trial_days_remaining)
)
end
- e.record!
+ e.publish_to_database
end
force_popover
diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb
index de5328b31e48b..65e060f329264 100644
--- a/spec/experiments/application_experiment_spec.rb
+++ b/spec/experiments/application_experiment_spec.rb
@@ -79,14 +79,6 @@
application_experiment.publish
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
let(:should_track) { false }
diff --git a/spec/features/gitlab_experiments_spec.rb b/spec/features/gitlab_experiments_spec.rb
index 76b418adcead3..ca772680ff633 100644
--- a/spec/features/gitlab_experiments_spec.rb
+++ b/spec/features/gitlab_experiments_spec.rb
@@ -31,9 +31,10 @@
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({
'null_hypothesis' => {
+ 'excluded' => false,
'experiment' => 'null_hypothesis',
'key' => anything,
'variant' => 'candidate'
--
GitLab