diff --git a/app/controllers/projects/ci/pipeline_editor_controller.rb b/app/controllers/projects/ci/pipeline_editor_controller.rb index 392a6afc6368650f029f1f43d02eaec10fac4aa8..6f12e3940dd59c3517dadad2dfeeb3b393d2787d 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 d5675618da5691f25ccd6212bcf7739de59ae60a..177533b89c87110ed730e2b35bc744e689adeb26 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 d49539c56fb8c5a5b10ee3d5ddb5dee746b2f37d..79935300fb6f50ecc79e964620590219221ebdb7 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 22c632a08aa2fd3aa710ea047673a0d1b9d5c55b..783e92c64603644ac0f0d320be07b12cbf6f5a88 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 d9f0fb3b93eb92ee9c9b036e1b541cb160c29979..1de7632268dcc742175797a5343d1fb8707ddcf9 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 45975d1953a88d409690e2d152de3a138a8a2317..78edc20599034034d1e0dbab6f7812cdd286fabc 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 d81ac5372e38c1d5a3ffef06da9702c0421dd6bc..288823bb41fcc1416dff96a8211cd663a5b016d8 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 d692c3104333d813991955efa555ef5a5f3ef1b7..dacb91f32af52cf869ad996b53d39ff0c69ebc55 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 d8348a14f7ae7b0ff18e8d4ed36defc46a904212..6c2b292c0ea85d1349313c5ff11f728d80c78dae 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 d9ea6af095b392ed0ef2e1a57e8e7b2cecf6dea0..c0c017469625d3b1404133783fa70a33d9e4e88f 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 de5328b31e48b8510c7049166b2271ae999d5f0b..65e060f329264df4a686987ada8d414daedebd80 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 76b418adcead32eba7c4f6d367aae31a36a32ead..ca772680ff633a1213332c7f2c78a2067f8ae4c9 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'