Remove concept of "ongoing" field tests and clarify a/b test instructions/expectations. (#11734)

* Remove concept of 'ongoing' field tests

* Fix specs

* Feeds spec

* Fix spec

* Fix style issues

* More details on field tests

app/controllers/stories/feeds_controller.rb

@@ -50,10 +50,6 @@ module Stories
 
     def optimized_signed_in_feed
       feed = Articles::Feeds::LargeForemExperimental.new(user: current_user, page: @page, tag: params[:tag])
-      # continue to track conversions even in the absence of an experiment so we
-      # can develop a baseline to compare to
-      field_test(:user_home_feed, participant: current_user)
-
       feed.more_comments_minimal_weight_randomized_at_end
     end
   end

app/models/comment.rb

@@ -304,7 +304,8 @@ class Comment < ApplicationRecord
   end
 
   def record_field_test_event
-    Users::RecordFieldTestEventWorker.perform_async(user_id, :user_home_feed, "user_creates_comment")
+    Users::RecordFieldTestEventWorker
+      .perform_async(user_id, :follow_implicit_points, "user_creates_comment")
   end
 
   def notify_slack_channel_about_warned_users

app/models/page_view.rb

@@ -30,7 +30,9 @@ class PageView < ApplicationRecord
   def record_field_test_event
     return unless user_id
 
-    Users::RecordFieldTestEventWorker.perform_async(user_id, :user_home_feed, "user_views_article_four_days_in_week")
-    Users::RecordFieldTestEventWorker.perform_async(user_id, :user_home_feed, "user_views_article_four_hours_in_day")
+    Users::RecordFieldTestEventWorker
+      .perform_async(user_id, :follow_implicit_points, "user_views_article_four_days_in_week")
+    Users::RecordFieldTestEventWorker
+      .perform_async(user_id, :follow_implicit_points, "user_views_article_four_hours_in_day")
   end
 end

app/models/reaction.rb

@@ -139,7 +139,8 @@ class Reaction < ApplicationRecord
   end
 
   def record_field_test_event
-    Users::RecordFieldTestEventWorker.perform_async(user_id, :user_home_feed, "user_creates_reaction")
+    Users::RecordFieldTestEventWorker
+      .perform_async(user_id, :follow_implicit_points, "user_creates_reaction")
   end
 
   def notify_slack_channel_about_vomit_reaction

app/workers/field_tests/prune_old_experiments_worker.rb

@@ -1,13 +0,0 @@
-module FieldTests
-  class PruneOldExperimentsWorker
-    include Sidekiq::Worker
-    sidekiq_options queue: :low_priority, retry: 10
-
-    def perform
-      five_precent_membership_count = FieldTest::Membership.count / 20
-      memberships = FieldTest::Membership.first(five_precent_membership_count)
-      FieldTest::Event.where(field_test_membership_id: memberships.map(&:id)).delete_all
-      memberships.map(&:delete)
-    end
-  end
-end

config/field_test.yml

@@ -1,14 +1,4 @@
 experiments:
-  user_home_feed: # Home feed collection for logged in user
-    variants:
-      - more_comments_minimal_weight_randomized_at_end
-    weights:
-      - 100
-    goals:
-      - user_creates_comment
-      - user_creates_reaction
-      - user_views_article_four_days_in_week
-      - user_views_article_four_hours_in_day
   follow_implicit_points: # Points given for implicit tag weights
     # Implemented here: app/workers/follows/update_points_worker.rb
     # Hypotheses: Weighting tags based on implied preference will lead

config/schedule.yml

@@ -4,9 +4,6 @@ fetch_all_rss:
 log_worker_queue_stats:
   cron: "*/10 * * * *" # every 10 minutes
   class: "Metrics::RecordBackgroundQueueStatsWorker"
-prune_old_field_tests:
-  cron: "0 13 * * *" # daily at 1 pm UTC
-  class: "FieldTests::PruneOldExperimentsWorker"
 record_daily_usage:
   cron: "0 11 * * *" # daily at 11:00 UTC
   class: "Metrics::RecordDailyUsageWorker"

docs/technical-overview/ab_testing.md

@@ -2,6 +2,54 @@
 
 We use the [Field Test](https://github.com/ankane/field_test) gem for conducting simple A/B tests.
 
-If you want to propose an A/B test of a feature, you may make a pull request which defines the hypotheses and what admins should look for to declare a winner. As A/B tests are going to have results that may differ from Forem to Forem, the process is relatively immature. In the future we may have more studies that can return anonymous ecosystem-wide results.
+If you want to propose an A/B test of a feature, you may make a pull request
+which defines the hypotheses and what admins should look for to declare a winner.
+As A/B tests are going to have results that may differ from Forem to Forem, the
+process is relatively immature. In the future we may have more studies that can return anonymous ecosystem-wide results.
 
-A/B tests are inherently the most useful in _large_ Forems, where qualitative feedback may be more useful on small Forems. As such, [DEV](https://dev.to) is our largest Forem and therefore can provide us with the most feedback for our existing A/B tests. However, we must keep in mind that DEV results may not apply well to future large Forems. We should seek to re-run useful experiments within the ecosystem after time has passed.
+A/B tests are inherently the most useful in _large_ Forems, where qualitative
+feedback may be more useful on small Forems. As such, [DEV](https://dev.to)
+is our largest Forem and therefore can provide us with the most feedback for
+our existing A/B tests. However, we must keep in mind that DEV results may
+not apply well to future large Forems. We should seek to re-run useful experiments within the ecosystem after time has passed.
+
+## Creating a new A/B test
+
+Follow the guidelines of the field test gem and add the test info to [config/field_test.yml](https://github.com/forem/forem/blob/master/config/field_test.yml).
+
+Then where you want to trigger the variant, you'll add some code like this:
+
+```ruby
+  test_variant = field_test(:follow_implicit_points, participant: user)
+  case test_variant
+  when "no_implicit_score"
+    0
+  when "half_weight_after_log"
+    Math.log(occurrences + bonus + 1) * 0.5
+  when "double_weight_after_log"
+    Math.log(occurrences + bonus + 1) * 2.0
+  when "double_bonus_before_log"
+    Math.log(occurrences + (bonus * 2) + 1)
+  when "without_weighting_bonus"
+    Math.log(occurrences + 1)
+  else # base - Our current "default" implementation
+    Math.log(occurrences + bonus + 1) # + 1 in all cases is to avoid log(0) => -infinity
+  end
+```
+
+Which would find or create the test variant for that user in particular. If
+this code is not called in the controller or view, you'll need to first
+include the gem helpers at the top of the file...
+
+```
+include FieldTest::Helpers
+```
+
+To record a successful field test outcome, you should call something like this
+
+```ruby
+    Users::RecordFieldTestEventWorker
+      .perform_async(user_id, :follow_implicit_points, "user_creates_reaction")
+```
+
+And modify that class as needed to determine whether to record the successful trial.

spec/factories/field_test_memberships.rb

@@ -1,7 +1,7 @@
 FactoryBot.define do
   factory :field_test_membership, class: "FieldTest::Membership" do
     converted         { false }
-    experiment        { :user_home_feed }
+    experiment        { :follow_implicit_points }
     participant_type  { "User" }
     variant           { "base" }
   end

spec/requests/comments_spec.rb

@@ -318,7 +318,7 @@ RSpec.describe "Comments", type: :request do
       it "converts field test" do
         post "/comments", params: base_comment_params
 
-        expected_args = [user.id, :user_home_feed, "user_creates_comment"]
+        expected_args = [user.id, :follow_implicit_points, "user_creates_comment"]
         expect(Users::RecordFieldTestEventWorker).to have_received(:perform_async).with(*expected_args)
       end
     end

spec/requests/page_views_spec.rb

@@ -56,7 +56,7 @@ RSpec.describe "PageViews", type: :request do
           referrer: "test"
         }
         expect(Users::RecordFieldTestEventWorker).to have_received(:perform_async)
-          .with(user.id, :user_home_feed, "user_views_article_four_days_in_week")
+          .with(user.id, :follow_implicit_points, "user_views_article_four_days_in_week")
       end
     end
 

spec/requests/reactions_spec.rb

@@ -324,7 +324,7 @@ RSpec.describe "Reactions", type: :request do
 
       it "converts field test" do
         post "/reactions", params: article_params
-        expect(Users::RecordFieldTestEventWorker).to have_received(:perform_async).with(user.id, :user_home_feed,
+        expect(Users::RecordFieldTestEventWorker).to have_received(:perform_async).with(user.id, :follow_implicit_points,
                                                                                         "user_creates_reaction")
       end
     end

spec/requests/stories/feeds_spec.rb

@@ -127,23 +127,6 @@ RSpec.describe "Stories::Feeds", type: :request do
         sign_in user
       end
 
-      it "sets a field test when feed_strategy is optimized" do
-        allow(SiteConfig).to receive(:feed_strategy).and_return("optimized")
-        expect do
-          get "/stories/feed"
-        end.to change(user.field_test_memberships, :count).by(1)
-
-        ftm = user.field_test_memberships.last
-        expect(ftm.experiment).to eq("user_home_feed")
-      end
-
-      it "does not set a field test when feed_strategy is basic" do
-        allow(SiteConfig).to receive(:feed_strategy).and_return("basic")
-        expect do
-          get "/stories/feed"
-        end.not_to change(user.field_test_memberships, :count)
-      end
-
       it "returns feed when feed_strategy is basic" do
         allow(SiteConfig).to receive(:feed_strategy).and_return("basic")
         get "/stories/feed"
@@ -192,19 +175,5 @@ RSpec.describe "Stories::Feeds", type: :request do
         expect(response_article["top_comments"].first["username"]).not_to be_nil
       end
     end
-
-    context "when user is signed in but there's no field_test" do
-      before do
-        sign_in user
-      end
-
-      it "does not set a field test" do
-        allow_any_instance_of(Stories::FeedsController).to receive(:field_test) # rubocop:disable RSpec/AnyInstance
-
-        expect do
-          get "/stories/feed"
-        end.not_to change(user.field_test_memberships, :count)
-      end
-    end
   end
 end

spec/workers/field_tests/prune_old_experiments_worker_spec.rb

@@ -1,25 +0,0 @@
-require "rails_helper"
-
-RSpec.describe FieldTests::PruneOldExperimentsWorker, type: :worker do
-  include_examples "#enqueues_on_correct_queue", "low_priority", 1
-  include FieldTest::Helpers
-
-  describe "#perform" do
-    let(:worker) { subject }
-
-    it "prunes first 5% of memberships and events" do
-      users = create_list(:user, 40)
-
-      users.each do |user|
-        create(:field_test_membership, participant_id: user.id.to_s)
-        field_test_converted(:user_home_feed, participant: user, goal: "user_creates_comment")
-      end
-
-      worker.perform
-
-      expect(FieldTest::Membership.count).to eq(38)
-      expect(FieldTest::Event.count).to eq(38)
-      expect(FieldTest::Event.pluck(:field_test_membership_id).sort).to eq(FieldTest::Membership.ids.sort)
-    end
-  end
-end

spec/workers/users/record_field_test_event_worker_spec.rb

@@ -11,17 +11,17 @@ RSpec.describe Users::RecordFieldTestEventWorker, type: :worker do
 
     context "with user who is part of field test" do
       before do
-        field_test(:user_home_feed, participant: user)
+        field_test(:follow_implicit_points, participant: user)
       end
 
       it "records user_creates_reaction field test conversion" do
-        worker.perform(user.id, "user_home_feed", "user_creates_reaction")
+        worker.perform(user.id, "follow_implicit_points", "user_creates_reaction")
         expect(FieldTest::Event.last.field_test_membership.participant_id).to eq(user.id.to_s)
         expect(FieldTest::Event.last.name).to eq("user_creates_reaction")
       end
 
       it "records user_creates_comment field test conversion" do
-        worker.perform(user.id, "user_home_feed", "user_creates_comment")
+        worker.perform(user.id, "follow_implicit_points", "user_creates_comment")
         expect(FieldTest::Event.last.field_test_membership.participant_id).to eq(user.id.to_s)
         expect(FieldTest::Event.last.name).to eq("user_creates_comment")
       end
@@ -30,7 +30,7 @@ RSpec.describe Users::RecordFieldTestEventWorker, type: :worker do
         7.times do |n|
           create(:page_view, user_id: user.id, created_at: n.days.ago)
         end
-        worker.perform(user.id, "user_home_feed", "user_views_article_four_days_in_week")
+        worker.perform(user.id, "follow_implicit_points", "user_views_article_four_days_in_week")
         expect(FieldTest::Event.last.field_test_membership.participant_id).to eq(user.id.to_s)
         expect(FieldTest::Event.last.name).to eq("user_views_article_four_days_in_week")
       end
@@ -39,7 +39,7 @@ RSpec.describe Users::RecordFieldTestEventWorker, type: :worker do
         2.times do |n|
           create(:page_view, user_id: user.id, created_at: n.days.ago)
         end
-        worker.perform(user.id, "user_home_feed", "user_views_article_four_days_in_week")
+        worker.perform(user.id, "follow_implicit_points", "user_views_article_four_days_in_week")
         expect(FieldTest::Event.all.size).to be(0)
       end
 
@@ -47,7 +47,7 @@ RSpec.describe Users::RecordFieldTestEventWorker, type: :worker do
         7.times do |n|
           create(:page_view, user_id: user.id, created_at: n.hours.ago)
         end
-        worker.perform(user.id, "user_home_feed", "user_views_article_four_hours_in_day")
+        worker.perform(user.id, "follow_implicit_points", "user_views_article_four_hours_in_day")
         expect(FieldTest::Event.last.field_test_membership.participant_id).to eq(user.id.to_s)
         expect(FieldTest::Event.last.name).to eq("user_views_article_four_hours_in_day")
       end
@@ -56,19 +56,19 @@ RSpec.describe Users::RecordFieldTestEventWorker, type: :worker do
         2.times do |n|
           create(:page_view, user_id: user.id, created_at: n.hours.ago)
         end
-        worker.perform(user.id, "user_home_feed", "user_views_article_four_hours_in_day")
+        worker.perform(user.id, "follow_implicit_points", "user_views_article_four_hours_in_day")
         expect(FieldTest::Event.all.size).to be(0)
       end
     end
 
     context "with user who is not part of field test" do
       it "records user_creates_reaction field test conversion" do
-        worker.perform(user.id, "user_home_feed", "user_creates_reaction")
+        worker.perform(user.id, "follow_implicit_points", "user_creates_reaction")
         expect(FieldTest::Event.all.size).to be(0)
       end
 
       it "records user_creates_comment field test conversion" do
-        worker.perform(user.id, "user_home_feed", "user_creates_comment")
+        worker.perform(user.id, "follow_implicit_points", "user_creates_comment")
         expect(FieldTest::Event.all.size).to be(0)
       end
 
@@ -83,7 +83,7 @@ RSpec.describe Users::RecordFieldTestEventWorker, type: :worker do
     context "without a user" do
       it "does not raise an error" do
         expect do
-          worker.perform(user.id + 1000, "user_home_feed", "user_creates_reaction")
+          worker.perform(user.id + 1000, "follow_implicit_points", "user_creates_reaction")
         end.not_to raise_error
       end
     end