diff --git a/app/models/reaction.rb b/app/models/reaction.rb index 4f3bf7a37..398e4ae38 100644 --- a/app/models/reaction.rb +++ b/app/models/reaction.rb @@ -1,36 +1,17 @@ class Reaction < ApplicationRecord - BASE_POINTS = { - "vomit" => -50.0, - "thumbsup" => 5.0, - "thumbsdown" => -10.0 - }.freeze - - # The union of public and privileged categories - CATEGORIES = %w[like readinglist unicorn thinking hands thumbsup thumbsdown vomit].freeze - - # These are the general category of reactions that anyone can choose - PUBLIC_CATEGORIES = %w[like readinglist unicorn thinking hands].freeze - - # These are categories of reactions that administrators can select - PRIVILEGED_CATEGORIES = %w[thumbsup thumbsdown vomit].freeze - NEGATIVE_PRIVILEGED_CATEGORIES = %w[thumbsdown vomit].freeze - REACTABLE_TYPES = %w[Comment Article User].freeze STATUSES = %w[valid invalid confirmed archived].freeze - # Days to ramp up new user points weight - NEW_USER_RAMPUP_DAYS_COUNT = 10 - belongs_to :reactable, polymorphic: true belongs_to :user counter_culture :reactable, column_name: proc { |model| - PUBLIC_CATEGORIES.include?(model.category) ? "public_reactions_count" : "reactions_count" + ReactionCategory[model.category].visible_to_public? ? "public_reactions_count" : "reactions_count" } counter_culture :user - scope :public_category, -> { where(category: PUBLIC_CATEGORIES) } + scope :public_category, -> { where(category: ReactionCategory.public.map(&:to_s)) } # Be wary, this is all things on the reading list, but for an end # user they might only see readinglist items that are published. @@ -48,10 +29,10 @@ class Reaction < ApplicationRecord .or(comment_vomits.where(reactable_id: user.comment_ids)) .or(user_vomits.where(user_id: user.id)) } - scope :privileged_category, -> { where(category: PRIVILEGED_CATEGORIES) } + scope :privileged_category, -> { where(category: ReactionCategory.privileged.map(&:to_s)) } scope :for_user, ->(user) { where(reactable: user) } - validates :category, inclusion: { in: CATEGORIES } + validates :category, inclusion: { in: ReactionCategory.all_slugs.map(&:to_s) } validates :reactable_type, inclusion: { in: REACTABLE_TYPES } validates :status, inclusion: { in: STATUSES } validates :user_id, uniqueness: { scope: %i[reactable_id reactable_type category] } @@ -128,8 +109,9 @@ class Reaction < ApplicationRecord # @return [Array] Reactions that contain a contradictory category to the category that was passed in, # example, if we pass in a "thumbsup", then we return reactions that have have a thumbsdown or vomit def contradictory_mod_reactions(category:, reactable_id:, reactable_type:, user:) - contradictory_category = NEGATIVE_PRIVILEGED_CATEGORIES if category == "thumbsup" - contradictory_category = "thumbsup" if category.in?(NEGATIVE_PRIVILEGED_CATEGORIES) + negatives = ReactionCategory.negative_privileged.map(&:to_s) + contradictory_category = negatives if category == "thumbsup" + contradictory_category = "thumbsup" if category.in?(negatives) Reaction.where(reactable_id: reactable_id, reactable_type: reactable_type, @@ -153,10 +135,6 @@ class Reaction < ApplicationRecord (status == "invalid") || points.negative? || (user_id == reactor_id) end - def vomit_on_user? - reactable_type == "User" && category == "vomit" - end - def reaction_on_organization_article? reactable_type == "Article" && reactable.organization.present? end @@ -165,8 +143,10 @@ class Reaction < ApplicationRecord reactable_type == "User" ? reactable : reactable.user end - def negative? - NEGATIVE_PRIVILEGED_CATEGORIES.include?(category) + delegate :negative?, :positive?, :visible_to_public?, to: :reaction_category, allow_nil: true + + def reaction_category + ReactionCategory[category.to_sym] end private @@ -200,21 +180,7 @@ class Reaction < ApplicationRecord end def assign_points - base_points = BASE_POINTS.fetch(category, 1.0) - - # Ajust for certain states - base_points = 0 if status == "invalid" - base_points /= 2 if reactable_type == "User" - base_points *= 2 if status == "confirmed" - - unless persisted? # Actions we only want to apply upon initial creation - # Author's comment reaction counts for more weight on to their own posts. (5.0 vs 1.0) - base_points *= 5 if positive_reaction_to_comment_on_own_article? - - # New users will have their reaction weight gradually ramp by 0.1 from 0 to 1.0. - base_points *= new_user_adjusted_points if new_untrusted_user # New users get minimal reaction weight - end - self.points = user ? (base_points * user.reputation_modifier) : -5 + self.points = CalculateReactionPoints.call(self) end def permissions @@ -234,27 +200,13 @@ class Reaction < ApplicationRecord Slack::Messengers::ReactionVomit.call(reaction: self) end - def positive_reaction_to_comment_on_own_article? - BASE_POINTS.fetch(category, 1.0).positive? && - reactable_type == "Comment" && - reactable&.commentable&.user_id == user_id - end - - def new_user_adjusted_points - ((Time.current - user.registered_at).seconds.in_days / NEW_USER_RAMPUP_DAYS_COUNT) - end - - 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 visible_to_public? return unless reactable.is_a?(Article) return unless user_id diff --git a/app/models/reaction_category.rb b/app/models/reaction_category.rb new file mode 100644 index 000000000..87d779adc --- /dev/null +++ b/app/models/reaction_category.rb @@ -0,0 +1,67 @@ +# See also reactions.yml and initializers/load_reaction_category_list.rb +class ReactionCategory + class << self + def [](slug) + hash[slug.to_sym] + end + + def all_slugs + list.map(&:slug) + end + + def negative_privileged + list.filter_map { |category| category.slug if category.privileged? && category.negative? } + end + + def public + list.sort_by(&:position).filter_map { |category| category.slug if category.visible_to_public? } + end + + def privileged + list.filter_map { |category| category.slug if category.privileged? } + end + + def list + @list ||= hash.values + end + + def hash + @hash ||= REACTION_CATEGORY_LIST.each_pair.to_h do |slug, category_or_attributes| + as_category = if category_or_attributes.is_a?(ReactionCategory) + category_or_attributes + else + new(**category_or_attributes.merge(slug: slug)) + end + [slug.to_sym, as_category] + end + end + end + + attr_reader :color, :icon, :name, :position, :privileged, :published, :score, :slug + alias privileged? privileged + alias published? published + + def initialize(attributes = {}) + attributes.symbolize_keys! + @slug = attributes[:slug]&.to_sym + @name = attributes[:name] || slug.to_s.titleize + @icon = attributes[:icon] + @position = attributes[:position] || 99 + @score = attributes[:score] || 1.0 + @privileged = attributes[:privileged] || false + @published = attributes.fetch(:published, true) + @color = attributes[:color] || "000000" + end + + def positive? + score > 0.0 + end + + def negative? + score < 0.0 + end + + def visible_to_public? + !privileged? && published? + end +end diff --git a/app/policies/reaction_policy.rb b/app/policies/reaction_policy.rb index 2f9889a65..f2c811887 100644 --- a/app/policies/reaction_policy.rb +++ b/app/policies/reaction_policy.rb @@ -3,7 +3,7 @@ class ReactionPolicy < ApplicationPolicy # We don't have a robust concept of a Privileged Reaction class, but instead must switch the # reaction permissions based on the given category. def self.policy_query_for(category:) - return :privileged_create? if Reaction::PRIVILEGED_CATEGORIES.include?(category) + return :privileged_create? if ReactionCategory[category.to_s]&.privileged? :create? end diff --git a/app/services/calculate_reaction_points.rb b/app/services/calculate_reaction_points.rb new file mode 100644 index 000000000..92335de4d --- /dev/null +++ b/app/services/calculate_reaction_points.rb @@ -0,0 +1,77 @@ +# Reaction uses this class to calculate points before saving +class CalculateReactionPoints + DEFAULT_SCORE = 1.0 + + POINTS = { + "invalid" => 0, + "User" => 2, + "confirmed" => 2, + "positive" => 5, + "negative" => -5 + }.freeze + + # Days to ramp up new user points weight + NEW_USER_RAMPUP_DAYS_COUNT = 10 + + def self.call(reaction) + new(reaction).calculate_points + end + + def initialize(reaction) + @reaction = reaction + end + + def calculate_points + base_points = reaction_category_score + + # Adjust for certain states + base_points = POINTS["invalid"] if status == "invalid" + base_points /= POINTS["User"] if reactable_type == "User" + base_points *= POINTS["confirmed"] if status == "confirmed" + + unless persisted? # Actions we only want to apply upon initial creation + # Author's comment reaction counts for more weight on to their own posts. (5.0 vs 1.0) + base_points *= POINTS["positive"] if positive_reaction_to_comment_on_own_article? + + # New users will have their reaction weight gradually ramp by 0.1 from 0 to 1.0. + base_points *= new_user_adjusted_points if new_untrusted_user # New users get minimal reaction weight + end + + user ? (base_points * user.reputation_modifier) : POINTS["negative"] + end + + attr_reader :reaction + + delegate :category, :persisted?, :positive?, :reactable, :reactable_type, :status, :user, :user_id, + to: :reaction + + private + + def reaction_category + ReactionCategory[category] + end + + def reaction_category_score + reaction_category&.score || DEFAULT_SCORE + end + + def new_untrusted_user + user.registered_at > NEW_USER_RAMPUP_DAYS_COUNT.days.ago && !user.trusted? && !user.any_admin? + end + + def new_user_adjusted_points + ((Time.current - user.registered_at).seconds.in_days / NEW_USER_RAMPUP_DAYS_COUNT) + end + + def positive_reaction_to_comment_on_own_article? + positive? && reaction_to_comment? && own_article? + end + + def reaction_to_comment? + reactable_type == "Comment" + end + + def own_article? + reactable&.commentable&.user_id == user_id + end +end diff --git a/app/services/reaction_handler.rb b/app/services/reaction_handler.rb index c2c47c89f..99bab548f 100644 --- a/app/services/reaction_handler.rb +++ b/app/services/reaction_handler.rb @@ -53,7 +53,7 @@ class ReactionHandler private def destroy_contradictory_mod_reactions - return unless category.in?(Reaction::PRIVILEGED_CATEGORIES) + return unless ReactionCategory[category].privileged? reactions = Reaction.contradictory_mod_reactions( category: category, @@ -112,7 +112,7 @@ class ReactionHandler category: category } if (current_user&.any_admin? || current_user&.super_moderator?) && - Reaction::NEGATIVE_PRIVILEGED_CATEGORIES.include?(category) + ReactionCategory[category].negative? create_params[:status] = "confirmed" end Reaction.new(create_params) @@ -141,7 +141,7 @@ class ReactionHandler end def sink_articles(reaction) - Moderator::SinkArticles.call(reaction.reactable_id) if reaction.vomit_on_user? + Moderator::SinkArticles.call(reaction.reactable_id) if vomit_on_user? end def send_notifications(reaction) @@ -172,4 +172,8 @@ class ReactionHandler def noop_result Result.new category: category, action: "none", reaction: existing_reaction end + + def vomit_on_user? + reactable_type == "User" && category == "vomit" + end end diff --git a/app/workers/reactions/update_relevant_scores_worker.rb b/app/workers/reactions/update_relevant_scores_worker.rb index c56bb82bc..8b52d3295 100644 --- a/app/workers/reactions/update_relevant_scores_worker.rb +++ b/app/workers/reactions/update_relevant_scores_worker.rb @@ -18,7 +18,7 @@ module Reactions reactable.calculate_score if reaction.reactable_type == "User" - return unless reaction.reactable_type == "Article" && Reaction::PUBLIC_CATEGORIES.include?(reaction.category) + return unless reaction.reactable_type == "Article" && reaction.visible_to_public? Follows::UpdatePointsWorker.perform_async(reaction.reactable_id, reaction.user_id) end diff --git a/config/initializers/load_reaction_category_list.rb b/config/initializers/load_reaction_category_list.rb new file mode 100644 index 000000000..8aed12658 --- /dev/null +++ b/config/initializers/load_reaction_category_list.rb @@ -0,0 +1,3 @@ +require "yaml" + +REACTION_CATEGORY_LIST = YAML.safe_load(Rails.root.join(*%w[config reactions.yml]).read) diff --git a/config/reactions.yml b/config/reactions.yml new file mode 100644 index 000000000..41d1a564b --- /dev/null +++ b/config/reactions.yml @@ -0,0 +1,23 @@ +--- +like: + position: 1 + icon: later +readinglist: + position: 2 + icon: later +unicorn: + position: 3 + icon: later +thumbsup: + privileged: true + score: 5.0 +thumbsdown: + privileged: true + score: -10.0 +vomit: + privileged: true + score: -50.0 +thinking: + published: false +hands: + published: false diff --git a/spec/models/reaction_category_spec.rb b/spec/models/reaction_category_spec.rb new file mode 100644 index 000000000..76c2634a2 --- /dev/null +++ b/spec/models/reaction_category_spec.rb @@ -0,0 +1,110 @@ +require "rails_helper" + +RSpec.describe ReactionCategory, type: :model do + let(:attributes_hash) do + { + "slug" => "lol", + "name" => "Laughing", + "position" => 2, + :published => true + } + end + + it "returns category object via [:slug]" do + expect(described_class[:like]).to be_a(described_class) + expect(described_class[:vomit].slug).to eq(:vomit) + expect(described_class[:thumbsdown].name).to eq("Thumbsdown") + end + + it "lists all category slugs" do + expect(described_class.all_slugs).to contain_exactly(*%i[like unicorn readinglist hands thinking thumbsup + thumbsdown vomit]) + end + + it "lists public categories" do + expect(described_class.public).to contain_exactly(*%i[like readinglist unicorn]) + end + + it "lists privileged categories" do + expect(described_class.privileged).to contain_exactly(*%i[thumbsup thumbsdown vomit]) + end + + it "lists negative_privileged categories" do + expect(described_class.negative_privileged).to contain_exactly(*%i[thumbsdown vomit]) + end + + it "initializes via an attributes hash" do + attributes = attributes_hash + + initialized = described_class.new attributes + expect(initialized.slug).to eq(:lol) + expect(initialized.name).to eq("Laughing") + expect(initialized.position).to eq(2) + expect(initialized).to be_published + expect(initialized).not_to be_privileged + end + + it "name defaults to Slug" do + slugged = described_class.new(slug: "my_name_is") + expect(slugged.name).to eq("My Name Is") + end + + it "score defaults to 1.0" do + default = described_class.new + expect(default.score.to_s).to eq("1.0") + + scored = described_class.new(score: 20.0) + expect(scored.score.to_s).to eq("20.0") + end + + it "privileged defaults to false" do + default = described_class.new + expect(default).not_to be_privileged + + privileged = described_class.new(privileged: true) + expect(privileged).to be_privileged + end + + it "published defaults to true" do + default = described_class.new + expect(default).to be_published + + unpublished = described_class.new(published: false) + expect(unpublished).not_to be_published + end + + it "position defaults to 99" do + default = described_class.new + expect(default.position).to eq(99) + + positioned = described_class.new(position: 4) + expect(positioned.position).to eq(4) + end + + it "is positive when score is above zero" do + positive = described_class.new(score: 15.0) + expect(positive).to be_positive + + negative = described_class.new(score: -1.0) + expect(negative).not_to be_positive + end + + it "is negative when score is below zero" do + positive = described_class.new(score: 15.0) + expect(positive).not_to be_negative + + negative = described_class.new(score: -1.0) + expect(negative).to be_negative + end + + it "is visible_to_public when non-privileged and public" do + privileged = described_class.new(privileged: true) + expect(privileged).not_to be_visible_to_public + + unpublished = described_class.new(published: false) + expect(unpublished).not_to be_visible_to_public + + visible = described_class.new(privileged: false, published: true) + expect(visible).to be_visible_to_public + end +end diff --git a/spec/models/reaction_spec.rb b/spec/models/reaction_spec.rb index d4a05f2f1..075a268e4 100644 --- a/spec/models/reaction_spec.rb +++ b/spec/models/reaction_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Reaction, type: :model do subject { build(:reaction, reactable: article, user: user) } it { is_expected.to belong_to(:user) } - it { is_expected.to validate_inclusion_of(:category).in_array(Reaction::CATEGORIES) } + it { is_expected.to validate_inclusion_of(:category).in_array(ReactionCategory.all_slugs.map(&:to_s)) } it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(%i[reactable_id reactable_type category]) } end @@ -76,59 +76,6 @@ RSpec.describe Reaction, type: :model do expect(reaction).not_to be_valid end - it "assigns 0 points if reaction is invalid" do - reaction.update(status: "invalid") - expect(reaction.points).to eq(0) - end - - it "assigns extra 5 points if reaction is to comment on author's post" do - comment = create(:comment, commentable: article) - comment_reaction = create(:reaction, reactable: comment, user: user) - expect(comment_reaction.points).to eq(5.0) - end - - it "does not extra 5 points if reaction is to comment on author's post" do - second_user = create(:user) - second_article = create(:article, user: second_user) - comment = create(:comment, commentable: second_article) - comment_reaction = create(:reaction, reactable: comment, user: user) - expect(comment_reaction.points).to eq(1) - end - - it "assigns the correct points if reaction is confirmed" do - reaction_points = reaction.points - reaction.update(status: "confirmed") - expect(reaction.points).to eq(reaction_points * 2) - end - - it "assigns fractional points to new users on create" do - newish_user = create(:user, registered_at: 3.days.ago) - reaction = create(:reaction, reactable: article, user: newish_user) - expect(reaction.points).to be_within(0.1).of(0.3) - end - - it "Does not assign new fractional logic on re-save" do - reaction.save - original_points = reaction.points - reaction.user.update_column(:registered_at, 7.days.ago) - reaction.save - expect(reaction.points).to eq(original_points) - end - - it "assigns full points to new users who is also trusted" do - newish_user = create(:user, registered_at: 3.days.ago) - newish_user.add_role(:trusted) - create(:reaction, reactable: article, user: newish_user) - expect(reaction.points).to be_within(0.1).of(1.0) - end - - it "assigns full points to new users who is admin" do - newish_user = create(:user, registered_at: 3.days.ago) - newish_user.add_role(:admin) - create(:reaction, reactable: article, user: newish_user) - expect(reaction.points).to be_within(0.1).of(1.0) - end - context "when user is trusted" do before { reaction.user.add_role(:trusted) } diff --git a/spec/policies/reaction_policy_spec.rb b/spec/policies/reaction_policy_spec.rb index 56cd43b86..d38cfcf24 100644 --- a/spec/policies/reaction_policy_spec.rb +++ b/spec/policies/reaction_policy_spec.rb @@ -10,17 +10,17 @@ RSpec.describe ReactionPolicy do describe ".policy_query_for" do subject { described_class.policy_query_for(category: category) } - Reaction::PRIVILEGED_CATEGORIES.each do |category| + ReactionCategory.privileged.each do |category| context "when #{category} cateogry" do - let(:category) { category } + let(:category) { category.to_s } it { is_expected.to eq(:privileged_create?) } end end - (Reaction::CATEGORIES - Reaction::PRIVILEGED_CATEGORIES).each do |category| - context "when #{category} cateogry" do - let(:category) { category } + (ReactionCategory.all_slugs - ReactionCategory.privileged).each do |category| + context "when #{category} category" do + let(:category) { category.to_s } it { is_expected.to eq(:create?) } end diff --git a/spec/requests/api/v1/docs/reactions_spec.rb b/spec/requests/api/v1/docs/reactions_spec.rb index d630db144..727a300ee 100644 --- a/spec/requests/api/v1/docs/reactions_spec.rb +++ b/spec/requests/api/v1/docs/reactions_spec.rb @@ -34,7 +34,7 @@ RSpec.describe "api/v1/reactions", type: :request do produces "application/json" parameter name: :category, in: :query, required: true, schema: { type: :string, - enum: Reaction::PUBLIC_CATEGORIES + enum: ReactionCategory.public } parameter name: :reactable_id, in: :query, required: true, schema: { type: :integer, @@ -82,7 +82,7 @@ RSpec.describe "api/v1/reactions", type: :request do produces "application/json" parameter name: :category, in: :query, required: true, schema: { type: :string, - enum: Reaction::PUBLIC_CATEGORIES + enum: ReactionCategory.public } parameter name: :reactable_id, in: :query, required: true, schema: { type: :integer, diff --git a/spec/services/calculate_reaction_points_spec.rb b/spec/services/calculate_reaction_points_spec.rb new file mode 100644 index 000000000..cb2b548d9 --- /dev/null +++ b/spec/services/calculate_reaction_points_spec.rb @@ -0,0 +1,63 @@ +require "rails_helper" + +RSpec.describe CalculateReactionPoints, type: :service do + let(:user) { create(:user, registered_at: 20.days.ago) } + let(:article) { create(:article, user: user) } + let(:reaction) { build(:reaction, reactable: article, user: user) } + let(:calculated_points) { described_class.call(reaction) } + + it "assigns 0 points if reaction is invalid" do + reaction.status = "invalid" + expect(calculated_points).to eq(0) + end + + context "when reaction is to comment on author's post" do + let(:comment) { build :comment, commentable: article } + let(:reaction) { build :reaction, reactable: comment, user: user } + + it "assigns extra 5 points" do + expect(calculated_points).to eq(5.0) + end + + it "does not extra 5 points if comment from other author" do + second_user = create(:user) + second_article = create(:article, user: second_user) + comment = create(:comment, commentable: second_article) + comment_reaction = create(:reaction, reactable: comment, user: user) + expect(comment_reaction.points).to eq(1) + end + end + + it "assigns the correct points if reaction is confirmed" do + reaction_points = reaction.points + reaction.status = "confirmed" + expect(calculated_points).to eq(reaction_points * 2) + end + + context "when newish user" do + let(:newish_user) { create(:user, registered_at: 3.days.ago) } + let(:reaction) { build :reaction, reactable: article, user: newish_user } + + it "assigns fractional points to new users on create" do + expect(calculated_points).to be_within(0.1).of(0.3) + end + + it "assigns full points to new user who is also trusted" do + allow(newish_user).to receive(:trusted?).and_return(true) + expect(calculated_points).to be_within(0.1).of(1.0) + end + + it "assigns full points to new users who is admin" do + newish_user.add_role(:admin) + expect(calculated_points).to be_within(0.1).of(1.0) + end + end + + it "Does not assign new fractional logic on re-save" do + reaction.save + original_points = reaction.points + reaction.user.update_column(:registered_at, 7.days.ago) + + expect(calculated_points).to eq(original_points) + end +end