Add feed_success_score and other count tallies (#20106)

* Add initial feed events score

* Add feed events score

* Clean up code styles

* Adjust how tests are run

* Fix conflicts

* Update app/javascript/articles/Feed.jsx

* Add additional feed events tabulation queries for more reliable eventual consistency

* Add additional feed events tabulation queries for more reliable eventual consistency

* Fix test logic

* Updates from feedback
This commit is contained in:
Ben Halpern 2023-09-15 12:02:26 -04:00 committed by GitHub
parent 84a65f0a26
commit cadebde66f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 231 additions and 2 deletions

View file

@ -89,6 +89,13 @@ module Articles
label: "Order by conflating a random number and the score (see forem/forem#16128)",
order_by_fragment: "article_relevancies.randomized_value " \
"^ (1.0 / greatest(articles.score, 0.1)) DESC")
order_by_lever(:final_order_by_feed_success_score,
label: "Order by feed success score",
order_by_fragment: "articles.feed_success_score DESC")
order_by_lever(:final_order_by_feed_success_score_and_primary_score,
label: "Order by feed success score and primary score",
order_by_fragment: "((articles.feed_success_score + 0.1) * (articles.score / 10)) DESC")
order_by_lever(:published_at_with_randomization_favoring_public_reactions,
label: "Favor recent articles with more reactions, " \

View file

@ -6,6 +6,8 @@ class FeedEvent < ApplicationRecord
belongs_to :article, optional: true
belongs_to :user, optional: true
after_save :update_article_counters_and_scores
enum category: {
impression: 0,
click: 1,
@ -23,6 +25,9 @@ class FeedEvent < ApplicationRecord
].freeze
DEFAULT_TIMEBOX = 5.minutes.freeze
REACTION_SCORE_MULTIPLIER = 5
COMMENT_SCORE_MULTIPLIER = 10
validates :article_position, numericality: { only_integer: true, greater_than: 0 }
validates :context_type, inclusion: { in: VALID_CONTEXT_TYPES }, presence: true
# Since we have disabled association validation, this is handy to filter basic bad data
@ -42,4 +47,54 @@ class FeedEvent < ApplicationRecord
article: article,
)
end
def self.bulk_update_counters_by_article_id(article_ids)
unique_article_ids = article_ids.uniq
update_counters_for_articles(unique_article_ids)
end
def self.update_counters_for_articles(article_ids)
article_ids.each do |article_id|
update_single_article_counters(article_id)
end
end
def self.update_single_article_counters(article_id)
ThrottledCall.perform("article_feed_success_score_#{article_id}", throttle_for: 15.minutes) do
impressions = FeedEvent.where(article_id: article_id, category: "impression")
return if impressions.empty?
clicks = FeedEvent.where(article_id: article_id, category: "click")
reactions = FeedEvent.where(article_id: article_id, category: "reaction")
comments = FeedEvent.where(article_id: article_id, category: "comment")
# Count the distinct users for impressions and each event type
distinct_impressions_users = impressions.distinct.pluck(:user_id)
distinct_clicks_users = clicks.distinct.pluck(:user_id)
distinct_reactions_users = reactions.distinct.pluck(:user_id)
distinct_comments_users = comments.distinct.pluck(:user_id)
# Calculate score based on distinct users
reactions_score = distinct_reactions_users.size * REACTION_SCORE_MULTIPLIER
clicks_score = distinct_clicks_users.size # 1x multiplier for clicks
comments_score = distinct_comments_users.size * COMMENT_SCORE_MULTIPLIER
score = (clicks_score + reactions_score + comments_score).to_f / distinct_impressions_users.size
# Update the article counters
Article.where(id: article_id).update_all(
feed_success_score: score,
feed_clicks_count: clicks.size,
feed_impressions_count: impressions.size,
)
end
end
private
def update_article_counters_and_scores
return unless article
self.class.update_single_article_counters(article_id)
end
end

View file

@ -56,7 +56,10 @@ module FeedEvents
end
end
FeedEvent.insert_all(records_to_insert) if records_to_insert.present?
return if records_to_insert.blank?
FeedEvent.insert_all(records_to_insert)
FeedEvent.bulk_update_counters_by_article_id(records_to_insert.pluck(:article_id).sample(5))
end
private

View file

@ -0,0 +1,5 @@
class AddFeedSuccessScoreToArticles < ActiveRecord::Migration[7.0]
def change
add_column :articles, :feed_success_score, :float, default: 0.0, null: false
end
end

View file

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2023_09_10_135738) do
ActiveRecord::Schema[7.0].define(version: 2023_09_12_220412) do
# These are extensions that must be enabled in order to support this database
enable_extension "citext"
enable_extension "ltree"
@ -107,6 +107,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_09_10_135738) do
t.integer "feed_clicks_count", default: 0
t.integer "feed_impressions_count", default: 0
t.string "feed_source_url"
t.float "feed_success_score", default: 0.0, null: false
t.integer "hotness_score", default: 0
t.string "language"
t.datetime "last_comment_at", precision: nil, default: "2017-01-01 05:00:00"

View file

@ -1,6 +1,9 @@
require "rails_helper"
RSpec.describe FeedEvent do
let(:reaction_multiplier) { FeedEvent::REACTION_SCORE_MULTIPLIER }
let(:comment_multiplier) { FeedEvent::COMMENT_SCORE_MULTIPLIER }
describe "validations" do
it { is_expected.to belong_to(:article).optional }
it { is_expected.to validate_numericality_of(:article_id).only_integer }
@ -63,4 +66,128 @@ RSpec.describe FeedEvent do
end
end
end
describe "after_save .update_article_counters_and_scores" do
let!(:article) { create(:article) }
let!(:user1) { create(:user) }
let!(:user2) { create(:user) }
it "updates article counters and scores when a FeedEvent is saved" do
# Create some feed events to simulate behavior
create(:feed_event, category: "impression", article: article, user: user1)
create(:feed_event, category: "click", article: article, user: user1)
create(:feed_event, category: "reaction", article: article, user: user1)
create(:feed_event, category: "comment", article: article, user: user2)
# Trigger the after_save
create(:feed_event, category: "impression", article: article, user: user2)
# Reload the article to get the updated counters and scores
article.reload
expect(article.feed_success_score).to eq((1 + reaction_multiplier + comment_multiplier) / 2.0) # Calculated score
expect(article.feed_impressions_count).to eq(2) # Two impressions
expect(article.feed_clicks_count).to eq(1) # One click
end
it "returns early if article is nil" do
feed_event_without_article = build(:feed_event, category: "impression")
allow(feed_event_without_article).to receive(:update_article_counters_and_scores)
feed_event_without_article.save
expect(feed_event_without_article).not_to have_received(:update_article_counters_and_scores)
end
it "initializes counters and scores to zero when no events have occurred" do
expect do
create(:feed_event, category: "impression", article: article, user: user1)
end.to not_change { article.reload.feed_success_score }
.and not_change { article.reload.feed_clicks_count }
.and change { article.reload.feed_impressions_count }.from(0).to(1)
end
it "correctly updates when only one type of event occurs" do
create(:feed_event, category: "reaction", article: article, user: user1)
# Trigger the after_save
create(:feed_event, category: "impression", article: article, user: user1)
article.reload
expect(article.feed_success_score).to eq(5.0) # One reaction by one distinct user
expect(article.feed_impressions_count).to eq(1) # One impression
expect(article.feed_clicks_count).to eq(0) # Zero clicks
end
it "considers only distinct users for each type of event for score, but not count" do
create_list(:feed_event, 2, category: "impression", article: article, user: user1)
create_list(:feed_event, 4, category: "click", article: article, user: user1)
create_list(:feed_event, 3, category: "reaction", article: article, user: user2)
create_list(:feed_event, 2, category: "comment", article: article, user: user2)
# Trigger the after_save
create(:feed_event, category: "impression", article: article, user: user2)
article.reload
expect(article.feed_success_score).to eq((1 + reaction_multiplier + comment_multiplier) / 2.0) # Calculated score
expect(article.feed_clicks_count).to eq(4)
expect(article.feed_impressions_count).to eq(3)
end
end
describe ".bulk_update_counters_by_article_id" do
let!(:article1) { create(:article) }
let!(:article2) { create(:article) }
let!(:user1) { create(:user) }
let!(:user2) { create(:user) }
it "updates counters and scores for multiple articles" do
# Create some feed events for article1
create(:feed_event, category: "impression", article: article1, user: user1)
create(:feed_event, category: "impression", article: article1, user: user2)
create(:feed_event, category: "impression", article: article1, user: user2) # Duplicate
create(:feed_event, category: "click", article: article1, user: user1)
create(:feed_event, category: "reaction", article: article1, user: user1)
create(:feed_event, category: "comment", article: article1, user: user2)
# Create some feed events for article2
create(:feed_event, category: "impression", article: article2, user: user2)
create(:feed_event, category: "click", article: article2, user: user1)
create(:feed_event, category: "reaction", article: article2, user: user2)
described_class.bulk_update_counters_by_article_id([article1.id, article2.id])
article1.reload
article2.reload
expect(article1.feed_success_score).to eq((1 + reaction_multiplier + comment_multiplier) / 2.0) # Calculated score
expect(article1.feed_impressions_count).to eq(3)
expect(article1.feed_clicks_count).to eq(1)
expect(article2.feed_success_score).to eq((1 + 5) / 1.0) # Calculated score
expect(article2.feed_impressions_count).to eq(1)
expect(article2.feed_clicks_count).to eq(1)
end
it "skips articles with no impressions" do
# Create some feed events for article2, but none for article1
create(:feed_event, category: "click", article: article2, user: user1)
create(:feed_event, category: "reaction", article: article2, user: user2)
create(:feed_event, category: "comment", article: article2, user: user2)
described_class.bulk_update_counters_by_article_id([article1.id, article2.id])
article1.reload
article2.reload
expect(article1.feed_success_score).to eq(0.0) # Should remain uninitialized
expect(article1.feed_impressions_count).to eq(0)
expect(article1.feed_clicks_count).to eq(0)
# Scores and counters for article2 should remain uninitialized
expect(article2.feed_success_score).to eq(0.0)
expect(article2.feed_impressions_count).to eq(0)
expect(article2.feed_clicks_count).to eq(0)
end
end
end

View file

@ -30,6 +30,37 @@ RSpec.describe FeedEvents::BulkUpsert, type: :service do
articles.map { |article| [article.id, user.id, "impression"] },
)
end
it "calls bulk_update_counters_by_article_id for article ids" do
allow(FeedEvent).to receive(:bulk_update_counters_by_article_id)
described_class.call(feed_events_data)
expect(FeedEvent).to have_received(:bulk_update_counters_by_article_id).with(match_array(articles.map(&:id)))
end
end
context "when there are more than 5 events" do
let(:articles) { create_list(:article, 8) }
let(:feed_events_data) do
articles
.map
.with_index do |article, index|
{
article: article,
user: user,
article_position: index + 1,
category: :impression,
context_type: FeedEvent::CONTEXT_TYPE_HOME
}
end
end
it "calls bulk_update_counters_by_article_id for article ids" do
allow(FeedEvent).to receive(:bulk_update_counters_by_article_id)
described_class.call(feed_events_data)
expect(FeedEvent).to have_received(:bulk_update_counters_by_article_id).with(satisfy do |arg|
arg.is_a?(Array) && arg.length == 5
end)
end
end
context "when there are duplicate events within the list" do