diff --git a/app/services/articles/feeds/article_score_calculator_for_user.rb b/app/services/articles/feeds/article_score_calculator_for_user.rb new file mode 100644 index 000000000..c5a898007 --- /dev/null +++ b/app/services/articles/feeds/article_score_calculator_for_user.rb @@ -0,0 +1,89 @@ +module Articles + module Feeds + # @api private + # + # The purpose of this class is to encapsulate how we apply scores + # to an article. + # + # @see Articles::Feeds::Basic + # @see Articles::Feeds::LargeForemExperimental + class ArticleScoreCalculatorForUser + # This constant defines some of the levers that we use to help + # calculate an article's score. + DEFAULT_CONFIGURATION = { + comment_weight: 0.2, + xp_level_weight: 1, + default_user_xp_level: 5, + followed_user_score: 1, + not_followed_user_score: 0, + nil_user_tag_score: 0, + followed_tag_weight: 1, + not_followed_tag_score: 0, + followed_org_score: 1, + not_followed_org_score: 0 + }.freeze + + # @param user [User] the user for whom we're calculating the article score. + # @param config [Hash] exposes the means + # for overriding the default configuration values + # + # @see DEFAULT_CONFIGURATION + def initialize(user:, config: {}) + @user = user + DEFAULT_CONFIGURATION.each_pair do |key, value| + instance_variable_set("@#{key}", config.fetch(key, value)) + end + end + + # @api private + def score_followed_user(article) + user_following_users_ids.include?(article.user_id) ? @followed_user_score : @not_followed_user_score + end + + # @api private + def score_followed_tags(article) + return @nil_user_tag_score unless @user + + article_tags = article.decorate.cached_tag_list_array + user_followed_tags.sum do |tag| + article_tags.include?(tag.name) ? tag.points * @followed_tag_weight : @not_followed_tag_score + end + end + + # @api private + def score_followed_organization(article) + return @not_followed_org_score unless article.organization_id? + + user_following_org_ids.include?(article.organization_id) ? @followed_org_score : @not_followed_org_score + end + + # @api private + def score_experience_level(article) + user_experience_level = @user&.setting&.experience_level || @default_user_xp_level + + # Calculate the distance between the user's experience level + # and that of the article's experience level. + - (((article.experience_level_rating - user_experience_level).abs / 2) * @xp_level_weight) + end + + # @api private + def score_comments(article) + article.comments_count * @comment_weight + end + + private + + def user_followed_tags + @user_followed_tags ||= (@user&.decorate&.cached_followed_tags || []) + end + + def user_following_org_ids + @user_following_org_ids ||= (@user&.cached_following_organizations_ids || []) + end + + def user_following_users_ids + @user_following_users_ids ||= (@user&.cached_following_users_ids || []) + end + end + end +end diff --git a/app/services/articles/feeds/basic.rb b/app/services/articles/feeds/basic.rb index f1c73d695..51a9e7241 100644 --- a/app/services/articles/feeds/basic.rb +++ b/app/services/articles/feeds/basic.rb @@ -6,6 +6,7 @@ module Articles @number_of_articles = number_of_articles @page = page @tag = tag + @article_score_applicator = Articles::Feeds::ArticleScoreCalculatorForUser.new(user: @user) end def feed @@ -18,29 +19,21 @@ module Articles articles = articles.where.not(user_id: UserBlock.cached_blocked_ids_for_blocker(@user.id)) articles.sort_by.with_index do |article, index| - article_tags = article.decorate.cached_tag_list_array - tag_score = user_followed_tags.sum do |tag| - article_tags.include?(tag.name) ? tag.points : 0 - end - user_score = user_following_users_ids.include?(article.user_id) ? 1 : 0 - org_score = user_following_org_ids.include?(article.organization_id) ? 1 : 0 + tag_score = score_followed_tags(article) + user_score = score_followed_user(article) + org_score = score_followed_organization(article) + + # NOTE: Not quite understanding the purpose of the `- + # index`. My guess is that it helps reduce the impact of the + # hotness score on the sort order. tag_score + org_score + user_score - index end.reverse! end - private - - def user_followed_tags - @user_followed_tags ||= (@user&.decorate&.cached_followed_tags || []) - end - - def user_following_org_ids - @user_following_org_ids ||= (@user&.cached_following_organizations_ids || []) - end - - def user_following_users_ids - @user_following_users_ids ||= (@user&.cached_following_users_ids || []) - end + delegate(:score_followed_tags, + :score_followed_user, + :score_followed_organization, + to: :@article_score_applicator) end end end diff --git a/app/services/articles/feeds/large_forem_experimental.rb b/app/services/articles/feeds/large_forem_experimental.rb index 671c7301c..05d5cf148 100644 --- a/app/services/articles/feeds/large_forem_experimental.rb +++ b/app/services/articles/feeds/large_forem_experimental.rb @@ -1,15 +1,12 @@ module Articles module Feeds class LargeForemExperimental - DEFAULT_USER_XP_LEVEL = 5 def initialize(user: nil, number_of_articles: 50, page: 1, tag: nil) @user = user @number_of_articles = number_of_articles @page = page @tag = tag - @tag_weight = 1 # default weight tags play in rankings - @comment_weight = 0.2 # default weight comments play in rankings - @xp_level_weight = 1 # default weight for user experience level + @article_score_applicator = Articles::Feeds::ArticleScoreCalculatorForUser.new(user: user) end def default_home_feed(user_signed_in: false) @@ -58,37 +55,12 @@ module Articles article_points end - # @api private - def score_followed_user(article, follow_user_score: 1, not_followed_user_score: 0) - user_following_users_ids.include?(article.user_id) ? follow_user_score : not_followed_user_score - end - - # @api private - def score_followed_tags(article, nil_user_tag_score: 0, followed_tag_weight: @tag_weight, unfollowed_tag_score: 0) - return nil_user_tag_score unless @user - - article_tags = article.decorate.cached_tag_list_array - user_followed_tags.sum do |tag| - article_tags.include?(tag.name) ? tag.points * followed_tag_weight : unfollowed_tag_score - end - end - - # @api private - def score_followed_organization(article, followed_org_score: 1, not_followed_org_score: 0) - return not_followed_org_score unless article.organization_id? - - user_following_org_ids.include?(article.organization_id) ? followed_org_score : not_followed_org_score - end - - # @api private - def score_experience_level(article, xp_level_weight: @xp_level_weight) - - (((article.experience_level_rating - user_experience_level).abs / 2) * xp_level_weight) - end - - # @api private - def score_comments(article, comment_weight: @comment_weight) - article.comments_count * comment_weight - end + delegate(:score_followed_user, + :score_followed_tags, + :score_followed_organization, + :score_experience_level, + :score_comments, + to: :@article_score_applicator) # @api private def globally_hot_articles(user_signed_in, article_score_threshold: -15, min_rand_limit: 15, max_rand_limit: 80) @@ -113,10 +85,6 @@ module Articles private - def user_experience_level - @user_experience_level ||= @user&.setting&.experience_level || DEFAULT_USER_XP_LEVEL - end - def experimental_hot_story_grab start_time = [(@user.page_views.second_to_last&.created_at || 7.days.ago) - 18.hours, 7.days.ago].max Article.published.limited_column_select.includes(top_comments: :user) @@ -125,18 +93,6 @@ module Articles .order(score: :desc) end - def user_followed_tags - @user_followed_tags ||= (@user&.decorate&.cached_followed_tags || []) - end - - def user_following_org_ids - @user_following_org_ids ||= (@user&.cached_following_organizations_ids || []) - end - - def user_following_users_ids - @user_following_users_ids ||= (@user&.cached_following_users_ids || []) - end - def first_quarter(array) array[0...(array.length / 4)] end diff --git a/spec/services/articles/feeds/article_score_calculator_for_user_spec.rb b/spec/services/articles/feeds/article_score_calculator_for_user_spec.rb new file mode 100644 index 000000000..d900d1223 --- /dev/null +++ b/spec/services/articles/feeds/article_score_calculator_for_user_spec.rb @@ -0,0 +1,201 @@ +require "rails_helper" + +RSpec.describe Articles::Feeds::ArticleScoreCalculatorForUser, type: :service do + let(:user) { create(:user) } + let!(:article) { create(:article) } + let(:applicator) { described_class.new(user: user) } + + describe "#score_followed_user" do + context "when article is written by a followed user" do + before { user.follow(article.user) } + + it "returns a score of 1" do + expect(applicator.score_followed_user(article)).to eq 1 + end + end + + context "when article is not written by a followed user" do + it "returns a score of 0" do + expect(applicator.score_followed_user(article)).to eq 0 + end + end + end + + describe "#score_followed_organization" do + let(:organization) { create(:organization) } + let(:article) { create(:article, organization: organization) } + + context "when article is from a followed organization" do + before { user.follow(organization) } + + it "returns a score of 1" do + expect(applicator.score_followed_organization(article)).to eq 1 + end + end + + context "when article is not from a followed organization" do + it "returns a score of 0" do + expect(applicator.score_followed_organization(article)).to eq 0 + end + end + + context "when article has no organization" do + let(:article) { create(:article) } + + it "returns a score of 0" do + expect(applicator.score_followed_organization(article)).to eq 0 + end + end + end + + describe "#score_followed_tags" do + let(:tag) { create(:tag) } + let(:unfollowed_tag) { create(:tag) } + + context "when article includes a followed tag" do + let(:article) { create(:article, tags: tag.name) } + + before do + user.follow(tag) + user.save + user.follows.last.update(explicit_points: 2) + end + + it "returns the followed tag point value" do + expect(applicator.score_followed_tags(article)).to eq 2 + end + end + + context "when article includes multiple followed tags" do + let(:tag2) { create(:tag) } + let(:article) { create(:article, tags: "#{tag.name}, #{tag2.name}") } + + before do + user.follow(tag) + user.follow(tag2) + user.save + user.follows.each { |follow| follow.update(explicit_points: 2) } + end + + it "returns the sum of followed tag point values" do + expect(applicator.score_followed_tags(article)).to eq 4 + end + end + + context "when article includes an unfollowed tag" do + let(:article) { create(:article, tags: "#{tag.name}, #{unfollowed_tag.name}") } + + before do + user.follow(tag) + user.save + end + + it "doesn't score the unfollowed tag" do + expect(applicator.score_followed_tags(article)).to eq 1 + end + end + + context "when article doesn't include any followed tags" do + let(:article) { create(:article, tags: unfollowed_tag.name) } + + it "returns 0" do + expect(applicator.score_followed_tags(article)).to eq 0 + end + end + + context "when user doesn't follow any tags" do + it "returns 0" do + expect(user.cached_followed_tag_names).to be_empty + expect(applicator.score_followed_tags(article)).to eq 0 + end + end + end + + describe "#score_experience_level" do + let(:article) { create(:article, experience_level_rating: 7) } + + context "when user has a further experience level" do + let(:user) { create(:user) } + + before do + user.setting.update(experience_level: 1) + end + + it "returns negative of (absolute value of the difference between article and user experience) divided by 2" do + expect(applicator.score_experience_level(article)).to eq(-3) + end + + it "returns proper negative when fractional" do + article.experience_level_rating = 8 + expect(applicator.score_experience_level(article)).to eq(-3.5) + end + end + + context "when user has a closer experience level" do + let(:user) { create(:user) } + + before do + user.setting.update(experience_level: 9) + end + + it "returns negative of (absolute value of the difference between article and user experience) divided by 2" do + expect(applicator.score_experience_level(article)).to eq(-1) + end + end + + context "when the user does not have an experience level set" do + let(:user) { create(:user) } + + before do + user.setting.update(experience_level: nil) + end + + it "uses a value of 5 for user experience level" do + expect(applicator.score_experience_level(article)).to eq(-1) + end + end + end + + describe "#score_comments" do + let(:applicator) { described_class.new(user: user, config: { comment_weight: 1 }) } + let(:article_with_one_comment) { create(:article) } + let(:article_with_five_comments) { create(:article) } + + before do + create(:comment, user: user, commentable: article_with_one_comment) + create_list(:comment, 5, user: user, commentable: article_with_five_comments) + article_with_one_comment.update_score + article_with_five_comments.update_score + article_with_one_comment.reload + article_with_five_comments.reload + end + + context "when comment_weight is default of 0" do + it "returns 0 for uncommented articles" do + expect(applicator.score_comments(article)).to eq(0) + end + + it "returns a multiple of the parameterized weight for articles with comments" do + expect(article_with_five_comments.comments_count).to eq(5) + expect(applicator.score_comments(article_with_five_comments)).to eq(5) + end + end + + context "when comment_weight is higher than 0" do + before { applicator.instance_variable_set(:@comment_weight, 2) } + + it "returns 0 for uncommented articles" do + expect(applicator.score_comments(article)).to eq(0) + end + + it "returns a non-zero score for commented upon articles" do + expect(applicator.score_comments(article_with_one_comment)).to be > 0 + end + + it "scores article with more comments high than others" do + expect(applicator.score_comments(article_with_five_comments)) + .to be > applicator.score_comments(article_with_one_comment) + end + end + end +end diff --git a/spec/services/articles/feeds/basic_spec.rb b/spec/services/articles/feeds/basic_spec.rb index 624ef9a36..faf65fb86 100644 --- a/spec/services/articles/feeds/basic_spec.rb +++ b/spec/services/articles/feeds/basic_spec.rb @@ -28,10 +28,10 @@ RSpec.describe Articles::Feeds::Basic, type: :service do let(:feed) { described_class.new(user: user, number_of_articles: 100, page: 1) } it "returns articles with score above 0 sorted by user preference scores" do - allow(feed).to receive(:user_following_users_ids).and_return([old_story.user_id]) + user.follow(old_story.user) old_story_tag = Tag.find_by(name: unique_tag_name) old_story_tag.update(points: 10) - allow(feed).to receive(:user_followed_tags).and_return([old_story_tag]) + user.follow(old_story_tag) result = feed.feed expect(result.first).to eq old_story diff --git a/spec/services/articles/feeds/large_forem_experimental_spec.rb b/spec/services/articles/feeds/large_forem_experimental_spec.rb index ebe8edcb6..13966db03 100644 --- a/spec/services/articles/feeds/large_forem_experimental_spec.rb +++ b/spec/services/articles/feeds/large_forem_experimental_spec.rb @@ -132,198 +132,6 @@ RSpec.describe Articles::Feeds::LargeForemExperimental, type: :service do end end - describe "#score_followed_user" do - context "when article is written by a followed user" do - before { user.follow(article.user) } - - it "returns a score of 1" do - expect(feed.score_followed_user(article)).to eq 1 - end - end - - context "when article is not written by a followed user" do - it "returns a score of 0" do - expect(feed.score_followed_user(article)).to eq 0 - end - end - end - - describe "#score_followed_organization" do - let(:organization) { create(:organization) } - let(:article) { create(:article, organization: organization) } - - context "when article is from a followed organization" do - before { user.follow(organization) } - - it "returns a score of 1" do - expect(feed.score_followed_organization(article)).to eq 1 - end - end - - context "when article is not from a followed organization" do - it "returns a score of 0" do - expect(feed.score_followed_organization(article)).to eq 0 - end - end - - context "when article has no organization" do - let(:article) { create(:article) } - - it "returns a score of 0" do - expect(feed.score_followed_organization(article)).to eq 0 - end - end - end - - describe "#score_followed_tags" do - let(:tag) { create(:tag) } - let(:unfollowed_tag) { create(:tag) } - - context "when article includes a followed tag" do - let(:article) { create(:article, tags: tag.name) } - - before do - user.follow(tag) - user.save - user.follows.last.update(explicit_points: 2) - end - - it "returns the followed tag point value" do - expect(feed.score_followed_tags(article)).to eq 2 - end - end - - context "when article includes multiple followed tags" do - let(:tag2) { create(:tag) } - let(:article) { create(:article, tags: "#{tag.name}, #{tag2.name}") } - - before do - user.follow(tag) - user.follow(tag2) - user.save - user.follows.each { |follow| follow.update(explicit_points: 2) } - end - - it "returns the sum of followed tag point values" do - expect(feed.score_followed_tags(article)).to eq 4 - end - end - - context "when article includes an unfollowed tag" do - let(:article) { create(:article, tags: "#{tag.name}, #{unfollowed_tag.name}") } - - before do - user.follow(tag) - user.save - end - - it "doesn't score the unfollowed tag" do - expect(feed.score_followed_tags(article)).to eq 1 - end - end - - context "when article doesn't include any followed tags" do - let(:article) { create(:article, tags: unfollowed_tag.name) } - - it "returns 0" do - expect(feed.score_followed_tags(article)).to eq 0 - end - end - - context "when user doesn't follow any tags" do - it "returns 0" do - expect(user.cached_followed_tag_names).to be_empty - expect(feed.score_followed_tags(article)).to eq 0 - end - end - end - - describe "#score_experience_level" do - let(:article) { create(:article, experience_level_rating: 7) } - - context "when user has a further experience level" do - let(:user) { create(:user) } - - before do - user.setting.update(experience_level: 1) - end - - it "returns negative of (absolute value of the difference between article and user experience) divided by 2" do - expect(feed.score_experience_level(article)).to eq(-3) - end - - it "returns proper negative when fractional" do - article.experience_level_rating = 8 - expect(feed.score_experience_level(article)).to eq(-3.5) - end - end - - context "when user has a closer experience level" do - let(:user) { create(:user) } - - before do - user.setting.update(experience_level: 9) - end - - it "returns negative of (absolute value of the difference between article and user experience) divided by 2" do - expect(feed.score_experience_level(article)).to eq(-1) - end - end - - context "when the user does not have an experience level set" do - let(:user) { create(:user) } - - before do - user.setting.update(experience_level: nil) - end - - it "uses a value of 5 for user experience level" do - expect(feed.score_experience_level(article)).to eq(-1) - end - end - end - - describe "#score_comments" do - let(:article_with_one_comment) { create(:article) } - let(:article_with_five_comments) { create(:article) } - - before do - create(:comment, user: user, commentable: article_with_one_comment) - create_list(:comment, 5, user: user, commentable: article_with_five_comments) - article_with_one_comment.update_score - article_with_five_comments.update_score - article_with_one_comment.reload - article_with_five_comments.reload - end - - context "when comment_weight is default of 0" do - it "returns 0 for uncommented articles" do - expect(feed.score_comments(article, comment_weight: 1)).to eq(0) - end - - it "returns a multiple of the parameterized weight for articles with comments" do - expect(article_with_five_comments.comments_count).to eq(5) - expect(feed.score_comments(article_with_five_comments, comment_weight: 1)).to eq(5) - end - end - - context "when comment_weight is higher than 0" do - before { feed.instance_variable_set(:@comment_weight, 2) } - - it "returns 0 for uncommented articles" do - expect(feed.score_comments(article)).to eq(0) - end - - it "returns a non-zero score for commented upon articles" do - expect(feed.score_comments(article_with_one_comment)).to be > 0 - end - - it "scores article with more comments high than others" do - expect(feed.score_comments(article_with_five_comments)).to be > feed.score_comments(article_with_one_comment) - end - end - end - describe "#rank_and_sort_articles" do let(:article1) { create(:article) } let(:article2) { create(:article) }