diff --git a/app/models/articles/feeds.rb b/app/models/articles/feeds.rb index 4d1ee464d..882cc3d81 100644 --- a/app/models/articles/feeds.rb +++ b/app/models/articles/feeds.rb @@ -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, " \ diff --git a/app/models/feed_event.rb b/app/models/feed_event.rb index 1de9ebe5e..47b5b8290 100644 --- a/app/models/feed_event.rb +++ b/app/models/feed_event.rb @@ -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 diff --git a/app/services/feed_events/bulk_upsert.rb b/app/services/feed_events/bulk_upsert.rb index dd9eaf07b..47980ce68 100644 --- a/app/services/feed_events/bulk_upsert.rb +++ b/app/services/feed_events/bulk_upsert.rb @@ -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 diff --git a/db/migrate/20230912220412_add_feed_success_score_to_articles.rb b/db/migrate/20230912220412_add_feed_success_score_to_articles.rb new file mode 100644 index 000000000..b4802c69d --- /dev/null +++ b/db/migrate/20230912220412_add_feed_success_score_to_articles.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index 877e91f0a..418ba1a4f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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" diff --git a/spec/models/feed_event_spec.rb b/spec/models/feed_event_spec.rb index 66f1f7008..a5841fd4f 100644 --- a/spec/models/feed_event_spec.rb +++ b/spec/models/feed_event_spec.rb @@ -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 diff --git a/spec/services/feed_events/bulk_upsert_spec.rb b/spec/services/feed_events/bulk_upsert_spec.rb index 2cfabccc3..24918ede1 100644 --- a/spec/services/feed_events/bulk_upsert_spec.rb +++ b/spec/services/feed_events/bulk_upsert_spec.rb @@ -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