maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Adding more goals to feed experiments (#17673)

Browse source 
This commit adds three new feed experiment goals:

* user publishes four posts within a week
* user reacts with a "heart", "unicorn", or "reading list" to an article
* user reacts with a "heart", "unicorn", or "reading list" to an article
  four times in a week

In addition it adds it to the existing experiment.  Adding it to the
existing experiment is acceptable because:

1. We can ignore the results
2. The experiments are structured such that these new goals could be met
   with prior data.
3. Both variants are playing by the same rules, so the results relative
   to each other are valid.

**There is a nuanced assumption in how we handle reactions:**

This implementation does not count by unique article reactions. However,
most folks will ❤️ , 🦄 , and 🔖 in one swoop; thus
those 3 reactions are all grouped into happening on one day.

Further, it's a reflection of the person taking an action after reading
a post, not how much action on that particular post.

**Rollback considerations:**

This also includes a feature flag that we can explicitly disable if we
overload the application with workers handling reaction goals.  Using
`FeatureFlag.accessible?(:field_test_event_for_reactions)` returns
`true` unless we explicitly disable this flag.

Closes forem/forem#17669
This commit is contained in:
Jeremy Friesen 2022-05-13 13:22:06 -04:00 • committed by GitHub
parent ba3e619c55
commit 8a4e1ebeba
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 162 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
app/models/ab_experiment/goal_conversion_handler.rb
View file

@ -12,6 +12,7 @@ class AbExperiment
USER_PUBLISHES_POST_GOAL = "user_publishes_post".freeze
USER_CREATES_PAGEVIEW_GOAL = "user_creates_pageview".freeze
USER_CREATES_COMMENT_GOAL = "user_creates_comment".freeze
USER_CREATES_ARTICLE_REACTION_GOAL = "user_creates_article_reaction".freeze
def self.call(...)
new(...).call
@ -49,6 +50,8 @@ class AbExperiment
convert_comment_goal(experiment: experiment, experiment_start_date: experiment_start_date)
when USER_PUBLISHES_POST_GOAL
convert_post_goal(experiment: experiment, experiment_start_date: experiment_start_date)
when USER_CREATES_ARTICLE_REACTION_GOAL
convert_reaction_goal(experiment: experiment, experiment_start_date: experiment_start_date)
else
field_test_converted(experiment, participant: user, goal: goal) # base single comment goal.
end
@ -88,6 +91,11 @@ class AbExperiment
def convert_post_goal(experiment:, experiment_start_date:)
field_test_converted(experiment, participant: user, goal: goal) # base is we created a post
post_goal_with_group(experiment,
[7.days.ago, experiment_start_date].max,
"DATE(published_at)",
4,
"user_publishes_post_on_four_different_days_within_a_week")
post_goal(experiment,
[7.days.ago, experiment_start_date].max,
2,
@ -98,6 +106,15 @@ class AbExperiment
"user_publishes_post_at_least_two_times_within_two_weeks")
end
def convert_reaction_goal(experiment:, experiment_start_date:)
field_test_converted(experiment, participant: user, goal: goal) # base is we created a post
reaction_goal(experiment,
[7.days.ago, experiment_start_date].max,
"DATE(created_at)",
4,
"user_creates_article_reaction_on_four_different_days_within_a_week")
end
def pageview_goal(experiment, time_start, group_value, min_count, goal)
page_view_counts = user.page_views.where("created_at > ?", time_start)
.group(group_value).count.values
@ -113,6 +130,14 @@ class AbExperiment
field_test_converted(experiment, participant: user, goal: goal)
end
def post_goal_with_group(experiment, time_start, group_value, min_count, goal)
post_publication_counts = user.articles.published.where("published_at > ?", time_start)
.group(group_value).count.values
return unless post_publication_counts.size >= min_count
field_test_converted(experiment, participant: user, goal: goal)
end
def comment_goal(experiment, time_start, group_value, min_count, goal)
comment_counts = user.comments.where("created_at > ?", time_start)
.group(group_value).count.values
@ -121,5 +146,16 @@ class AbExperiment
field_test_converted(experiment, participant: user, goal: goal)
end
def reaction_goal(experiment, time_start, group_value, min_count, goal)
reaction_counts = user.reactions
.only_articles # as per `Reaction#record_field_test_event` we only record reactions to articles
.public_category # as per `Reaction#record_field_test_event` we only record public category reactions
.where("created_at > ?", time_start)
.group(group_value).count.values
return unless reaction_counts.size >= min_count
field_test_converted(experiment, participant: user, goal: goal)
end
end
end

18
app/models/reaction.rb
View file

@ -36,7 +36,8 @@ class Reaction < ApplicationRecord
# user they might only see readinglist items that are published.
# See https://github.com/forem/forem/issues/14796
scope :readinglist, -> { where(category: "readinglist") }
scope :for_articles, ->(ids) { where(reactable_type: "Article", reactable_id: ids) }
scope :for_articles, ->(ids) { only_articles.where(reactable_id: ids) }
scope :only_articles, -> { where(reactable_type: "Article") }
scope :eager_load_serialized_data, -> { includes(:reactable, :user) }
scope :article_vomits, -> { where(category: "vomit", reactable_type: "Article") }
scope :comment_vomits, -> { where(category: "vomit", reactable_type: "Comment") }
@ -61,6 +62,7 @@ class Reaction < ApplicationRecord
before_destroy :update_reactable_without_delay, unless: :destroyed_by_association
after_commit :async_bust
after_commit :bust_reactable_cache, :update_reactable, on: %i[create update]
after_commit :record_field_test_event, on: %i[create]
class << self
def count_for_article(id)
@ -209,4 +211,18 @@ class Reaction < ApplicationRecord
def new_untrusted_user
user.registered_at > NEW_USER_RAMPUP_DAYS_COUNT.days.ago && !user.trusted? && !user.any_admin?
end
# @see AbExperiment::GoalConversionHandler
def record_field_test_event
# TODO: Remove once we know that this test is not over-heating the application. That would be a
# few days after the deploy to DEV of this change.
return unless FeatureFlag.accessible?(:field_test_event_for_reactions)
return if FieldTest.config["experiments"].nil?
return unless PUBLIC_CATEGORIES.include?(category)
return unless reactable.is_a?(Article)
return unless user_id
Users::RecordFieldTestEventWorker
.perform_async(user_id, AbExperiment::GoalConversionHandler::USER_CREATES_ARTICLE_REACTION_GOAL)
end
end

3
config/field_test.yml
View file

@ -21,6 +21,9 @@ experiments:
- user_publishes_post
- user_publishes_post_at_least_two_times_within_week
- user_publishes_post_at_least_two_times_within_two_weeks
- user_publishes_post_on_four_different_days_within_a_week
- user_creates_article_reaction
- user_creates_article_reaction_on_four_different_days_within_a_week
exclude:
bots: true

76
spec/models/ab_experiment/goal_conversion_handler_spec.rb
View file

@ -73,6 +73,24 @@ RSpec.describe AbExperiment::GoalConversionHandler do
"user_publishes_post_at_least_two_times_within_week",
].sort)
end
it "records a conversion when they post 4 within a week" do
create(:article, published_at: 25.hours.ago, user_id: user.id)
create(:article, published_at: 49.hours.ago, user_id: user.id)
create(:article, published_at: 73.hours.ago, user_id: user.id)
create(:article, published_at: 97.hours.ago, user_id: user.id)
handler.call
expect(FieldTest::Event.last.field_test_membership.participant_id).to eq(user.id.to_s)
expect(FieldTest::Event.all.pluck(:name).sort)
.to match_array([
goal,
"user_publishes_post_at_least_two_times_within_two_weeks",
"user_publishes_post_at_least_two_times_within_week",
"user_publishes_post_on_four_different_days_within_a_week",
].sort)
end
end
context "with user who is part of field test and user_creates_comment goal" do
@ -169,6 +187,64 @@ RSpec.describe AbExperiment::GoalConversionHandler do
end
end
context "with user who is part of field test and user_creates_article_reaction goal" do
let(:goal) { described_class::USER_CREATES_ARTICLE_REACTION_GOAL }
let(:user) { create(:user, :trusted) } # Because we want to test handling of privileged reactions
before do
field_test(AbExperiment::CURRENT_FEED_STRATEGY_EXPERIMENT, participant: user)
end
it "records a conversion", :aggregate_failures do
create(:reaction, user_id: user.id)
handler.call
expect(FieldTest::Event.last.field_test_membership.participant_id).to eq(user.id.to_s)
expect(FieldTest::Event.last.name).to eq(goal)
end
it "records a conversion when they react 4 times within a week", :aggregate_failures do
create(:reaction, user_id: user.id, created_at: 25.hours.ago)
create(:reaction, user_id: user.id, created_at: 49.hours.ago)
create(:reaction, user_id: user.id, created_at: 73.hours.ago)
create(:reaction, user_id: user.id, created_at: 97.hours.ago)
handler.call
expect(FieldTest::Event.last.field_test_membership.participant_id).to eq(user.id.to_s)
expect(FieldTest::Event.all.pluck(:name).sort)
.to match_array([
goal,
"user_creates_article_reaction_on_four_different_days_within_a_week",
].sort)
end
it "does not record the conversion when they react to non-articles or non-public reactions" do
create(:reaction, user_id: user.id, created_at: 25.hours.ago)
create(:reaction, user_id: user.id, created_at: 49.hours.ago)
create(:reaction, user_id: user.id, created_at: 73.hours.ago, category: "vomit")
create(:reaction, user_id: user.id, created_at: 97.hours.ago, reactable: create(:comment))
create(:reaction, user_id: user.id, created_at: 121.hours.ago)
handler.call
expect(FieldTest::Event.last.field_test_membership.participant_id).to eq(user.id.to_s)
expect(FieldTest::Event.all.pluck(:name))
.to match_array([goal])
end
it "does not record the conversion when their 4 reactions are not within a week" do
create(:reaction, user_id: user.id, created_at: 25.hours.ago)
create(:reaction, user_id: user.id, created_at: 49.hours.ago)
create(:reaction, user_id: user.id, created_at: 73.hours.ago)
create(:reaction, user_id: user.id, created_at: 217.hours.ago) # 9 days ago
handler.call
expect(FieldTest::Event.last.field_test_membership.participant_id).to eq(user.id.to_s)
expect(FieldTest::Event.all.pluck(:name))
.to match_array([goal])
end
end
context "with user who is not part of field test" do
let(:goal) { described_class::USER_CREATES_COMMENT_GOAL }

30
spec/models/reaction_spec.rb
View file

@ -193,6 +193,36 @@ RSpec.describe Reaction, type: :model do
end
context "when callbacks are called after create" do
describe "field tests" do
let!(:user) { create(:user, :trusted) }
before do
# making sure there are no other enqueued jobs from other tests
sidekiq_perform_enqueued_jobs(only: Users::RecordFieldTestEventWorker)
end
it "enqueues a Users::RecordFieldTestEventWorker for giving a like to an article" do
article = create(:article, user: user)
sidekiq_assert_enqueued_jobs(1, only: Users::RecordFieldTestEventWorker) do
create(:reaction, reactable: article, user: user, category: "like")
end
end
it "does not enqueue a Users::RecordFieldTestEventWorker for giving a privileged reaction to an article" do
article = create(:article, user: user)
sidekiq_assert_enqueued_jobs(0, only: Users::RecordFieldTestEventWorker) do
create(:reaction, reactable: article, user: user, category: "thumbsdown")
end
end
it "does not enqueue a Users::RecordFieldTestEventWorker for giving a like to a comment" do
comment = create(:comment, user: user)
sidekiq_assert_enqueued_jobs(0, only: Users::RecordFieldTestEventWorker) do
create(:reaction, reactable: comment, user: user, category: "like")
end
end
end
describe "slack messages" do
let!(:user) { create(:user, :trusted) }
let!(:article) { create(:article, user: user) }