From d0cac663537e44a3e7f1725c853ac92d84193d91 Mon Sep 17 00:00:00 2001 From: Ridhwana Date: Mon, 29 Aug 2022 18:00:47 +0200 Subject: [PATCH] Further refactoring of the Reactions Service (#18378) --- app/models/reaction.rb | 17 ++++++++++ app/services/reaction_toggle.rb | 60 ++++++++++++++++----------------- spec/models/reaction_spec.rb | 16 +++++++++ 3 files changed, 63 insertions(+), 30 deletions(-) diff --git a/app/models/reaction.rb b/app/models/reaction.rb index aa706e380..d0702dced 100644 --- a/app/models/reaction.rb +++ b/app/models/reaction.rb @@ -114,6 +114,23 @@ class Reaction < ApplicationRecord def user_has_spammy_profile_reaction?(user:) user_vomits.exists?(reactable_id: user.id) end + + # @param category [String] the reaction category type, see the CATEGORIES var + # @param reactable_id [Boolean] the ID of the item that was reacted on + # @param reactable_type [String] the type of the item, see the REACTABLE_TYPES var + # @param user [User] a moderator user + + # @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) + + Reaction.where(reactable_id: reactable_id, + reactable_type: reactable_type, + user: user, + category: contradictory_category) + end end # no need to send notification if: diff --git a/app/services/reaction_toggle.rb b/app/services/reaction_toggle.rb index 62a0f016f..a8f45ecb3 100644 --- a/app/services/reaction_toggle.rb +++ b/app/services/reaction_toggle.rb @@ -20,28 +20,26 @@ class ReactionToggle end def initialize(params, current_user:) - @params = params @current_user = current_user + + @params = params + @reactable_id = params[:reactable_id] + @reactable_type = params[:reactable_type] @category = params[:category] || "like" end - attr_reader :category, :params, :current_user + attr_reader :reactable_id, :reactable_type, :category, :params, :current_user delegate :rate_limiter, to: :current_user def toggle - if params[:reactable_type] == "Article" && params[:category].in?(Reaction::PRIVILEGED_CATEGORIES) - destroy_contradictory_mod_reactions( - params[:reactable_id], - params[:reactable_type], - current_user, - params[:category], - ) + if reactable_type == "Article" && category.in?(Reaction::PRIVILEGED_CATEGORIES) + destroy_contradictory_mod_reactions end - existing_reaction = get_existing_reaction - if existing_reaction - handle_existing_reaction(existing_reaction) + @existing_reaction ||= existing_reaction + if @existing_reaction + handle_existing_reaction else create_new_reaction end @@ -49,13 +47,13 @@ class ReactionToggle private - def destroy_contradictory_mod_reactions(id, type, mod, category) - reactions = if category == "thumbsup" - Reaction.where(reactable_id: id, reactable_type: type, user: mod, - category: Reaction::NEGATIVE_PRIVILEGED_CATEGORIES) - elsif category.in?(Reaction::NEGATIVE_PRIVILEGED_CATEGORIES) - Reaction.where(reactable_id: id, reactable_type: type, user: mod, category: "thumbsup") - end + def destroy_contradictory_mod_reactions + reactions = Reaction.contradictory_mod_reactions( + category: category, + reactable_id: reactable_id, + reactable_type: reactable_type, + user: current_user, + ) return if reactions.blank? reactions.find_each { |reaction| destroy_reaction(reaction) } @@ -67,24 +65,26 @@ class ReactionToggle send_notifications_without_delay(reaction) end - def get_existing_reaction + def existing_reaction Reaction.where( user_id: current_user.id, - reactable_id: params[:reactable_id], - reactable_type: params[:reactable_type], + reactable_id: reactable_id, + reactable_type: reactable_type, category: category, ).first end - def handle_existing_reaction(reaction) - destroy_reaction(reaction) - log_audit(reaction) - create_result(reaction, "destroy") if reaction + def handle_existing_reaction + return unless @existing_reaction + + destroy_reaction(@existing_reaction) + log_audit(@existing_reaction) + result(@existing_reaction, "destroy") end def create_new_reaction reaction = build_reaction(category) - result = create_result(reaction, nil) + result = result(reaction, nil) if reaction.save rate_limit_reaction_creation @@ -108,8 +108,8 @@ class ReactionToggle def build_reaction(category) create_params = { user_id: current_user.id, - reactable_id: params[:reactable_id], - reactable_type: params[:reactable_type], + reactable_id: reactable_id, + reactable_type: reactable_type, category: category } if (current_user&.any_admin? || current_user&.super_moderator?) && @@ -127,7 +127,7 @@ class ReactionToggle Audit::Logger.log(:moderator, current_user, updated_params) end - def create_result(reaction, action) + def result(reaction, action) if action Result.new category: category, reaction: reaction, action: action else diff --git a/spec/models/reaction_spec.rb b/spec/models/reaction_spec.rb index a1f5bcd54..d4a05f2f1 100644 --- a/spec/models/reaction_spec.rb +++ b/spec/models/reaction_spec.rb @@ -331,4 +331,20 @@ RSpec.describe Reaction, type: :model do expect(described_class.related_negative_reactions_for_user(moderator).first.id).to eq(reaction.id) end end + + describe ".contradictory_mod_reactions" do + let(:moderator) { create(:user, :trusted) } + + it "returns the contradictary reactions related to the category passed in" do + article = create(:article, user: moderator) + reaction = create(:vomit_reaction, user: moderator, reactable: article) + + expect(described_class.contradictory_mod_reactions( + category: "thumbsup", + reactable_id: article.id, + reactable_type: "Article", + user: moderator, + ).first).to eq(reaction) + end + end end