Refactor reaction categories as abstract class collection (#18540)

* Try a ReactionCategory abstract 'model'

* Use ReactionCategory

* Lookup category, pre-reaction

* Lookup category > constant

* Category can sometimes be nil?

* Try something friendlier for code reloading

* Refactor: extract point calculation out of Reaction

* Reaction Category can have color

* Rename reaction points service to follow convention

* method signature changed as well, calculate -> call

* Fix typo, leading comment, and rename data structures for clarity

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
 

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

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

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

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

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

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)

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

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

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) }
 

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

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,

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