From acd3b92b320c738dbf9914b0fad8e913d19b940a Mon Sep 17 00:00:00 2001 From: Andy Zhao <17884966+Zhao-Andy@users.noreply.github.com> Date: Mon, 17 Feb 2020 10:29:50 -0500 Subject: [PATCH] Add sink user service and specs for mass vomits (#6006) * Add sink user service and specs for mass vomits * Use new score calculation * Lower articles' scores instead and make async * Add lower score functionality to controllers * Remove unused variable * Use correct var and add missing arg * Remove unused status * Add composite index for reactable_type and ID * Use alias instead of additional boolean * Use medium priority for sink worker * Log to honeybadger and not logs * Log the error and not a string --- .../internal/reactions_controller.rb | 7 +++++ app/controllers/reactions_controller.rb | 6 ++++ app/models/article.rb | 3 +- app/models/reaction.rb | 1 + app/models/user.rb | 6 ++-- app/services/moderator/sink_articles.rb | 7 +++++ app/workers/moderator/sink_articles_worker.rb | 20 ++++++++++++ ...le_type_reactable_id_index_to_reactions.rb | 7 +++++ db/schema.rb | 1 + spec/services/moderator/sink_articles_spec.rb | 31 +++++++++++++++++++ 10 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 app/services/moderator/sink_articles.rb create mode 100644 app/workers/moderator/sink_articles_worker.rb create mode 100644 db/migrate/20200212164359_add_reactable_type_reactable_id_index_to_reactions.rb create mode 100644 spec/services/moderator/sink_articles_spec.rb diff --git a/app/controllers/internal/reactions_controller.rb b/app/controllers/internal/reactions_controller.rb index b9e9a26fa..9882ecab6 100644 --- a/app/controllers/internal/reactions_controller.rb +++ b/app/controllers/internal/reactions_controller.rb @@ -2,6 +2,13 @@ class Internal::ReactionsController < Internal::ApplicationController def update @reaction = Reaction.find(params[:id]) @reaction.update(status: params[:reaction][:status]) + Moderator::SinkArticles.call(@reaction.user_id) if confirmed_vomit_reaction? redirect_to "/internal/reports" end + + private + + def confirmed_vomit_reaction? + @reaction.reactable_type == "User" && @reaction.status == "confirmed" && @reaction.category == "vomit" + end end diff --git a/app/controllers/reactions_controller.rb b/app/controllers/reactions_controller.rb index 902357d62..bf3ed62ec 100644 --- a/app/controllers/reactions_controller.rb +++ b/app/controllers/reactions_controller.rb @@ -51,6 +51,7 @@ class ReactionsController < ApplicationController if reaction current_user.touch reaction.destroy + Moderator::SinkArticles.call(reaction.user_id) if vomit_reaction_on_user?(reaction) Notification.send_reaction_notification_without_delay(reaction, reaction.reactable.user) Notification.send_reaction_notification_without_delay(reaction, reaction.reactable.organization) if organization_article?(reaction) @result = "destroy" @@ -62,6 +63,7 @@ class ReactionsController < ApplicationController category: category, ) @result = "create" + Moderator::SinkArticles.call(reaction.user_id) if vomit_reaction_on_user?(reaction) Notification.send_reaction_notification(reaction, reaction.reactable.user) Notification.send_reaction_notification(reaction, reaction.reactable.organization) if organization_article?(reaction) end @@ -80,4 +82,8 @@ class ReactionsController < ApplicationController def organization_article?(reaction) reaction.reactable_type == "Article" && reaction.reactable.organization.present? end + + def vomit_reaction_on_user?(reaction) + reaction.reactable_type == "User" && reaction.category == "vomit" + end end diff --git a/app/models/article.rb b/app/models/article.rb index 7030b2f8f..e579251b6 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -394,7 +394,8 @@ class Article < ApplicationRecord end def update_score - update_columns(score: reactions.sum(:points), + new_score = reactions.sum(:points) + Reaction.where(reactable_id: user_id, reactable_type: "User").sum(:points) + update_columns(score: new_score, comment_score: comments.sum(:score), hotness_score: BlackBox.article_hotness_score(self), spaminess_rating: BlackBox.calculate_spaminess(self)) diff --git a/app/models/reaction.rb b/app/models/reaction.rb index 8926f25e7..3f32630e0 100644 --- a/app/models/reaction.rb +++ b/app/models/reaction.rb @@ -153,6 +153,7 @@ class Reaction < ApplicationRecord def assign_points base_points = BASE_POINTS.fetch(category, 1.0) base_points = 0 if status == "invalid" + base_points /= 2 if reactable_type == "User" base_points *= 2 if status == "confirmed" self.points = user ? (base_points * user.reputation_modifier) : -5 end diff --git a/app/models/user.rb b/app/models/user.rb index 03b2eb1c6..1508e5ae4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -154,6 +154,8 @@ class User < ApplicationRecord validate :non_banished_username, :username_changed? validate :unique_including_orgs_and_podcasts, if: :username_changed? + alias_attribute :positive_reactions_count, :reactions_count + scope :dev_account, -> { find_by(id: SiteConfig.staff_user_id) } scope :welcoming_account, -> { find_by(id: ApplicationConfig["WELCOMING_USER_ID"]) } @@ -628,10 +630,6 @@ class User < ApplicationRecord def featured_number; end - def positive_reactions_count - reactions_count - end - def user self end diff --git a/app/services/moderator/sink_articles.rb b/app/services/moderator/sink_articles.rb new file mode 100644 index 000000000..feeee2d89 --- /dev/null +++ b/app/services/moderator/sink_articles.rb @@ -0,0 +1,7 @@ +module Moderator + class SinkArticles + def self.call(user_id) + Moderator::SinkArticlesWorker.perform_async(user_id) + end + end +end diff --git a/app/workers/moderator/sink_articles_worker.rb b/app/workers/moderator/sink_articles_worker.rb new file mode 100644 index 000000000..370ab14c8 --- /dev/null +++ b/app/workers/moderator/sink_articles_worker.rb @@ -0,0 +1,20 @@ +module Moderator + class SinkArticlesWorker + include Sidekiq::Worker + + sidekiq_options queue: :medium_priority, retry: 10 + + def perform(user_id) + user = User.find_by(id: user_id) + return unless user + + articles = Article.where(user: user) + reactions = Reaction.where(reactable: articles) + new_score = reactions.sum(:points) + Reaction.where(reactable: user).sum(:points) + articles.update_all(score: new_score) + rescue StandardError => e + DataDogStatsClient.count("moderators.sink", 1, tags: ["action:failed", "user_id:#{user.id}"]) + Honeybadger.notify(e) + end + end +end diff --git a/db/migrate/20200212164359_add_reactable_type_reactable_id_index_to_reactions.rb b/db/migrate/20200212164359_add_reactable_type_reactable_id_index_to_reactions.rb new file mode 100644 index 000000000..107f43333 --- /dev/null +++ b/db/migrate/20200212164359_add_reactable_type_reactable_id_index_to_reactions.rb @@ -0,0 +1,7 @@ +class AddReactableTypeReactableIdIndexToReactions < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_index :reactions, %i[reactable_id reactable_type], algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 272961a5c..5840bd949 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -867,6 +867,7 @@ ActiveRecord::Schema.define(version: 2020_02_13_182938) do t.index ["category"], name: "index_reactions_on_category" t.index ["created_at"], name: "index_reactions_on_created_at" t.index ["points"], name: "index_reactions_on_points" + t.index ["reactable_id", "reactable_type"], name: "index_reactions_on_reactable_id_and_reactable_type" t.index ["reactable_id"], name: "index_reactions_on_reactable_id" t.index ["reactable_type"], name: "index_reactions_on_reactable_type" t.index ["user_id"], name: "index_reactions_on_user_id" diff --git a/spec/services/moderator/sink_articles_spec.rb b/spec/services/moderator/sink_articles_spec.rb new file mode 100644 index 000000000..a93ee2edf --- /dev/null +++ b/spec/services/moderator/sink_articles_spec.rb @@ -0,0 +1,31 @@ +require "rails_helper" + +RSpec.describe Moderator::SinkArticles, type: :service do + let(:moderator) { create(:user, :trusted) } + let(:spam_user) do + user = create(:user) + create_list(:article, 3, user: user) + user + end + let(:vomit_reaction) { create(:reaction, reactable: spam_user, user: moderator, category: "vomit") } + + describe "#call" do + it "lowers all of a user's articles' scores by 25 each if not confirmed" do + vomit_reaction + expect do + sidekiq_perform_enqueued_jobs do + described_class.call(spam_user.id) + end + end.to change { spam_user.articles.sum(:score) }.from(0).to(-75) + end + + it "lowers all of the user's articles' scores by 50 each if confirmed" do + vomit_reaction.update(status: "confirmed") + expect do + sidekiq_perform_enqueued_jobs do + described_class.call(spam_user.id) + end + end.to change { spam_user.articles.sum(:score) }.from(0).to(-150) + end + end +end