Merge branch 'jejacks0n/maintenance/remove-publish-overrides' into 'master'

Removes the publish override methods

See merge request gitlab-org/gitlab!79893

app/experiments/application_experiment.rb

@@ -3,20 +3,6 @@
 class ApplicationExperiment < Gitlab::Experiment
   control { nil } # provide a default control for anonymous experiments
 
-  def publish(_result = nil)
-    super
-
-    publish_to_client
-  end
-
-  def publish_to_client
-    return unless should_track?
-
-    Gon.push({ experiment: { name => signature } }, true)
-  rescue NoMethodError
-    # means we're not in the request cycle, and can't add to Gon. Log a warning maybe?
-  end
-
   def publish_to_database
     ActiveSupport::Deprecation.warn('publish_to_database is deprecated and should not be used for reporting anymore')
 

doc/development/experiment_guide/gitlab_experiment.md

@@ -317,7 +317,7 @@ of tracking an event in Ruby would be:
 experiment(:pill_color, actor: current_user).track(:clicked)
 ```
 
-When you run an experiment with any of the examples so far, an `:assigned` event
+When you run an experiment with any of the examples so far, an `:assignment` event
 is tracked automatically by default. All events that are tracked from an
 experiment have a special
 [experiment context](https://gitlab.com/gitlab-org/iglu/-/blob/master/public/schemas/com.gitlab/gitlab_experiment/jsonschema/1-0-3)

ee/app/controllers/ee/registrations/welcome_controller.rb

@@ -85,7 +85,7 @@ module EE
       end
 
       def publish_combined_registration_experiment
-        combined_registration_experiment.publish_to_client if show_signup_onboarding?
+        combined_registration_experiment.publish if show_signup_onboarding?
       end
 
       def combined_registration_experiment

ee/app/controllers/registrations/verification_controller.rb

@@ -15,7 +15,7 @@ module Registrations
     private
 
     def publish_experiment
-      experiment(:registration_verification, user: current_user).publish_to_client
+      experiment(:registration_verification, user: current_user).publish
     end
   end
 end

ee/spec/controllers/registrations/verification_controller_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe Registrations::VerificationController do
 
         it 'publishes the experiment' do
           expect_next_instance_of(ApplicationExperiment) do |instance|
-            expect(instance).to receive(:publish_to_client)
+            expect(instance).to receive(:publish)
           end
 
           subject

ee/spec/controllers/registrations/welcome_controller_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe Registrations::WelcomeController do
       allow(controller.helpers).to receive(:signup_onboarding_enabled?).and_return(true)
 
       wrapped_experiment(experiment(:combined_registration)) do |e|
-        expect(e).to receive(:publish_to_client)
+        expect(e).to receive(:publish)
       end
 
       get :show

spec/experiments/application_experiment_spec.rb

@@ -28,61 +28,23 @@ RSpec.describe ApplicationExperiment, :experiment do
   end
 
   describe "#publish" do
-    let(:should_track) { true }
-
-    before do
-      allow(application_experiment).to receive(:should_track?).and_return(should_track)
-    end
-
     it "tracks the assignment", :snowplow do
-      application_experiment.publish
-
-      expect_snowplow_event(
-        category: 'namespaced/stub',
-        action: 'assignment',
-        context: [{ schema: anything, data: anything }]
-      )
-    end
-
-    it "publishes to the client" do
-      expect(application_experiment).to receive(:publish_to_client)
+      expect(application_experiment).to receive(:track).with(:assignment)
 
       application_experiment.publish
     end
 
-    context 'when we should not track' do
-      let(:should_track) { false }
-
-      it 'does not track an event to Snowplow', :snowplow do
+    it "adds to the published experiments" do
+      # These are surfaced in the client layer by rendering them in the
+      # _published_experiments.html.haml partial.
       application_experiment.publish
 
-        expect_no_snowplow_event
-      end
-    end
-
-    describe "#publish_to_client" do
-      it "adds the data into Gon" do
-        signature = { key: '86208ac54ca798e11f127e8b23ec396a', variant: 'control' }
-        expect(Gon).to receive(:push).with({ experiment: { 'namespaced/stub' => hash_including(signature) } }, true)
-
-        application_experiment.publish_to_client
-      end
-
-      it "handles when Gon raises exceptions (like when it can't be pushed into)" do
-        expect(Gon).to receive(:push).and_raise(NoMethodError)
-
-        expect { application_experiment.publish_to_client }.not_to raise_error
-      end
-
-      context 'when we should not track' do
-        let(:should_track) { false }
-
-        it 'returns early' do
-          expect(Gon).not_to receive(:push)
-
-          application_experiment.publish_to_client
-        end
-      end
+      expect(ApplicationExperiment.published_experiments['namespaced/stub']).to include(
+        experiment: 'namespaced/stub',
+        excluded: false,
+        key: anything,
+        variant: 'control'
+      )
     end
 
     describe '#publish_to_database' do

spec/views/layouts/_published_experiments.html.haml_spec.rb

@@ -4,22 +4,20 @@ require 'spec_helper'
 
 RSpec.describe 'layouts/_published_experiments', :experiment do
   before do
-    stub_const('TestControlExperiment', ApplicationExperiment)
-    stub_const('TestCandidateExperiment', ApplicationExperiment)
-    stub_const('TestExcludedExperiment', ApplicationExperiment)
+    # Stub each experiment to be enabled, otherwise tracking does not happen.
+    stub_experiments(
+      test_control: :control,
+      test_excluded: true,
+      test_published_only: :control,
+      test_candidate: :candidate,
+      test_variant: :variant_name
+    )
 
-    TestControlExperiment.new('test_control').tap do |e|
-      e.variant(:control)
-      e.publish
-    end
-    TestCandidateExperiment.new('test_candidate').tap do |e|
-      e.variant(:candidate)
-      e.publish
-    end
-    TestExcludedExperiment.new('test_excluded').tap do |e|
-      e.exclude!
-      e.publish
-    end
+    experiment(:test_control) { }
+    experiment(:test_excluded) { |e| e.exclude! }
+    experiment(:test_candidate) { |e| e.candidate { } }
+    experiment(:test_variant) { |e| e.variant(:variant_name) { } }
+    experiment(:test_published_only).publish
 
     render
   end
@@ -29,7 +27,9 @@ RSpec.describe 'layouts/_published_experiments', :experiment do
 
     expect(output).to include('gl.experiments = {')
     expect(output).to match(/"test_control":\{[^}]*"variant":"control"/)
-    expect(output).to match(/"test_candidate":\{[^}]*"variant":"candidate"/)
     expect(output).not_to include('"test_excluded"')
+    expect(output).to match(/"test_candidate":\{[^}]*"variant":"candidate"/)
+    expect(output).to match(/"test_variant":\{[^}]*"variant":"variant_name"/)
+    expect(output).to match(/"test_published_only":\{[^}]*"variant":"control"/)
   end
 end