From 5cc09767c5df8489b36d33e4bdc9330e253c885c Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Tue, 5 Oct 2021 10:11:41 -0400 Subject: [PATCH] Add conditional autovomits to user name and improve user score recalc (#14858) * Add conditional autovomits to user name and improve user score recalc * Remove puts * Add missing || * Fix tests * Update app/views/users/show.html.erb Co-authored-by: Michael Kohl * Update app/models/user.rb Co-authored-by: Michael Kohl * Add in-line score calc principles Co-authored-by: Michael Kohl --- app/models/organization.rb | 6 --- app/models/user.rb | 29 +++++++++++++- app/views/users/_meta.html.erb | 4 -- app/views/users/show.html.erb | 4 ++ .../update_relevant_scores_worker.rb | 11 ++++-- spec/models/user_spec.rb | 38 ++++++++++++++++++- .../update_relevant_scores_worker_spec.rb | 7 ++++ 7 files changed, 82 insertions(+), 17 deletions(-) diff --git a/app/models/organization.rb b/app/models/organization.rb index 3e087aa82..e0fe93fa5 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -110,12 +110,6 @@ class Organization < ApplicationRecord credits.unspent.size >= num_credits_needed end - def suspended? - # Hacky, yuck! - # TODO: [@jacobherrington] Remove this method - false - end - def destroyable? organization_memberships.count == 1 && articles.count.zero? && credits.count.zero? end diff --git a/app/models/user.rb b/app/models/user.rb index 906dfba78..18a9dce0c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -220,6 +220,7 @@ class User < ApplicationRecord after_create_commit :send_welcome_notification + after_save :create_conditional_autovomits after_commit :subscribe_to_mailchimp_newsletter after_commit :bust_cache @@ -249,8 +250,19 @@ class User < ApplicationRecord end def calculate_score - score = (articles.where(featured: true).size * 100) + comments.sum(:score) - update_column(:score, score) + # User score is used to mitigate spam by reducing visibility of flagged users + # It can generally be used as a baseline for affecting certain functionality which + # relies on trust gray area. + + # Current main use: If score is below zero, the user's profile page will render noindex + # meta tag. This is a subtle anti-spam mechanism. + + # It can be changed as frequently as needed to do a better job reflecting its purpose + # Changes should generally keep the score within the same order of magnitude so that + # mass re-calculation is needed. + user_reaction_points = Reaction.user_vomits.where(reactable_id: id).sum(:points) + calculated_score = (badge_achievements_count * 10) + user_reaction_points + update_column(:score, calculated_score) end def path @@ -600,6 +612,19 @@ class User < ApplicationRecord Users::BustCacheWorker.perform_async(id) end + def create_conditional_autovomits + return unless Settings::RateLimit.spam_trigger_terms.any? do |term| + name.match?(/#{term}/i) + end + + Reaction.create!( + user_id: Settings::General.mascot_user_id, + reactable_id: id, + reactable_type: "User", + category: "vomit", + ) + end + # TODO: @citizen428 I don't want to completely remove this method yet, as we # have similar methods in other models. But the previous implementation used # three profile fields that we can't guarantee to exist across all Forems. So diff --git a/app/views/users/_meta.html.erb b/app/views/users/_meta.html.erb index ce4b698fc..58dbaece5 100644 --- a/app/views/users/_meta.html.erb +++ b/app/views/users/_meta.html.erb @@ -18,7 +18,3 @@ <%= auto_discovery_link_tag(:rss, app_url("/feed/#{@user.username}"), title: "#{community_name} RSS Feed") %> <% end %> -<% if @user.suspended? %> - - -<% end %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index ddd066439..e3b7b81cf 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -5,6 +5,10 @@ <%= content_for :page_meta do %> <%= render "users/meta" %> + <% if @user.score.negative? || @user.suspended? %> + + + <% end %> <% end %> <% unless internal_navigation? || user_signed_in? %> diff --git a/app/workers/reactions/update_relevant_scores_worker.rb b/app/workers/reactions/update_relevant_scores_worker.rb index a03b8ec43..7deb385d7 100644 --- a/app/workers/reactions/update_relevant_scores_worker.rb +++ b/app/workers/reactions/update_relevant_scores_worker.rb @@ -6,14 +6,19 @@ module Reactions def perform(reaction_id) reaction = Reaction.find_by(id: reaction_id) - return unless reaction&.reactable + reactable = reaction&.reactable + return unless reactable - reaction.reactable.touch_by_reaction if reaction.reactable.respond_to?(:touch_by_reaction) + + reactable.touch_by_reaction if reactable.respond_to?(:touch_by_reaction) if reaction.reactable.respond_to?(:sync_reactions_count) ThrottledCall.perform(:sync_reactions_count, throttle_for: 15.minutes) do - reaction.reactable.sync_reactions_count + reactable.sync_reactions_count end end + + reactable.calculate_score if reaction.reactable_type == "User" + return unless reaction.reactable_type == "Article" && Reaction::PUBLIC_CATEGORIES.include?(reaction.category) Follows::UpdatePointsWorker.perform_async(reaction.reactable_id, reaction.user_id) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2b24dc1a8..ef26cd428 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -541,6 +541,40 @@ RSpec.describe User, type: :model do end end + describe "spam" do + before do + allow(Settings::General).to receive(:mascot_user_id).and_return(user.id) + allow(Settings::RateLimit).to receive(:spam_trigger_terms).and_return( + ["yahoomagoo gogo", "testtestetest", "magoo.+magee"], + ) + end + + it "creates vomit reaction if possible spam" do + user.name = "Hi my name is Yahoomagoo gogo" + user.save + expect(Reaction.last.category).to eq("vomit") + expect(Reaction.last.reactable_id).to eq(user.id) + end + + it "creates vomit reaction if possible spam based on pattern" do + user.name = "Hi my name is magoo to the magee" + user.save + expect(Reaction.last.category).to eq("vomit") + expect(Reaction.last.reactable_id).to eq(user.id) + end + + it "does not create vomit reaction if does not have matching title" do + user.save + expect(Reaction.last).to be nil + end + + it "does not create vomit reaction if does not have pattern match" do + user.name = "Hi my name is magoo to" + user.save + expect(Reaction.last).to be nil + end + end + describe "#suspended?" do subject { user.suspended? } @@ -659,10 +693,10 @@ RSpec.describe User, type: :model do describe "#calculate_score" do it "calculates a score" do - create(:article, featured: true, user: user) + user.update_column(:badge_achievements_count, 3) user.calculate_score - expect(user.score).to be_positive + expect(user.score).to eq(30) end end diff --git a/spec/workers/reactions/update_relevant_scores_worker_spec.rb b/spec/workers/reactions/update_relevant_scores_worker_spec.rb index 0717c809b..3c5a266af 100644 --- a/spec/workers/reactions/update_relevant_scores_worker_spec.rb +++ b/spec/workers/reactions/update_relevant_scores_worker_spec.rb @@ -33,6 +33,13 @@ RSpec.describe Reactions::UpdateRelevantScoresWorker, type: :worker, throttled_c end end + it "recalculates score if reactable is User" do + user = create(:user) + reaction.update_columns(category: "vomit", reactable_id: user.id, reactable_type: "User", points: -50) + worker.perform(reaction.id) + expect(user.reload.score).to be < -1 + end + it "updates the reactable Comment" do updated_at = 1.day.ago comment.update_columns(updated_at: updated_at)