From 0eb5402926680eeddbba66fe26415aaec7f152bb Mon Sep 17 00:00:00 2001 From: Jess Lee Date: Wed, 23 Jan 2019 16:57:32 -0500 Subject: [PATCH] Delete User Functionality (#1615) * add full delete route and update banisher * add user deletion spec * enhance styling * remove availability if banned from mentorship * add specs * do not show banned mentors * delete addiional associations * include more associations for deletion * simplify activity deletion wtih delete_all --- app/controllers/internal/users_controller.rb | 48 +++++-- app/services/moderator/banisher.rb | 49 ++++--- app/views/internal/users/edit.html.erb | 56 +++++--- app/views/internal/users/index.html.erb | 5 + app/views/internal/users/show.html.erb | 85 +++++++----- config/routes.rb | 1 + .../internal_users_controller_spec.rb | 124 ++++++++++++------ 7 files changed, 248 insertions(+), 120 deletions(-) diff --git a/app/controllers/internal/users_controller.rb b/app/controllers/internal/users_controller.rb index 3e3b4d2d7..d3d99602a 100644 --- a/app/controllers/internal/users_controller.rb +++ b/app/controllers/internal/users_controller.rb @@ -32,7 +32,7 @@ class Internal::UsersController < Internal::ApplicationController @new_mentor = user_params[:add_mentor] ban_from_mentorship make_matches - warn_or_ban_user + update_role add_note @user.update!(user_params) if user_params[:quick_match] @@ -42,17 +42,26 @@ class Internal::UsersController < Internal::ApplicationController end end - def warn_or_ban_user - if user_params[:ban_user] == "1" - @user.add_role :banned - create_note("banned", user_params[:note_for_current_role]) - elsif user_params[:warn_user] == "1" - @user.add_role :warned - create_note("warned", user_params[:note_for_current_role]) - elsif user_params[:good_standing_user] == "1" - @user.remove_role :warned - create_note("good_standing", user_params[:note_for_current_role]) - end + def update_role + ban_user if user_params[:ban_user] == "1" + warn_user if user_params[:warn_user] == "1" + return_to_good_standing if user_params[:good_standing_user] == "1" + end + + def return_to_good_standing + @user.remove_role :banned if @user.banned + @user.remove_role :warned if @user.warned + create_note("good_standing", user_params[:note_for_current_role]) + end + + def ban_user + @user.add_role :banned + create_note("banned", user_params[:note_for_current_role]) + end + + def warn_user + @user.add_role :warned + create_note("warned", user_params[:note_for_current_role]) end def add_note @@ -97,6 +106,7 @@ class Internal::UsersController < Internal::ApplicationController mentor_relationships = MentorRelationship.where(mentee_id: @user.id) deactivate_mentorship(mentee_relationships) deactivate_mentorship(mentor_relationships) + @user.update(offering_mentorship: false, seeking_mentorship: false) create_note("banned_from_mentorship", user_params[:note_for_mentorship_ban]) end @@ -109,13 +119,25 @@ class Internal::UsersController < Internal::ApplicationController def banish @user = User.find(params[:id]) begin - Moderator::Banisher.call(admin: current_user, offender: @user) + Moderator::Banisher.call_banish(admin: current_user, offender: @user) rescue StandardError => e flash[:error] = e.message end redirect_to "/internal/users/#{@user.id}/edit" end + def full_delete + @user = User.find(params[:id]) + username = @user.username + begin + Moderator::Banisher.call_delete_activity(admin: current_user, offender: @user) + flash[:notice] = "@" + username + " has been fully deleted. If this is a GDPR delete, remember to delete them from Mailchimp and Google Analytics." + rescue StandardError => e + flash[:error] = e.message + end + redirect_to "/internal/users" + end + private def user_params diff --git a/app/services/moderator/banisher.rb b/app/services/moderator/banisher.rb index 4e37b20ea..57f34dd6c 100644 --- a/app/services/moderator/banisher.rb +++ b/app/services/moderator/banisher.rb @@ -2,10 +2,14 @@ module Moderator class Banisher attr_reader :user, :admin - def self.call(admin:, offender:) + def self.call_banish(admin:, offender:) new(offender: offender, admin: admin).banish end + def self.call_delete_activity(admin:, offender:) + new(offender: offender, admin: admin).full_delete + end + def initialize(admin:, offender:) @user = offender @admin = admin @@ -39,29 +43,23 @@ module Moderator end end - def delete_reactions - user.reactions.find_each(&:delete) - end - def delete_comments - return unless user.comments.count.positive? + return unless user.comments.any? user.comments.find_each do |comment| - comment.reactions.find_each(&:delete) + comment.reactions.delete_all CacheBuster.new.bust_comment(comment.commentable, user.username) comment.delete end end - def delete_follows - user.follows.find_each(&:delete) - end - def delete_articles + return unless user.articles.any? + user.articles.find_each do |article| - article.reactions.find_each(&:delete) + article.reactions.delete_all article.comments.find_each do |comment| - comment.reactions.find_each(&:delete) + comment.reactions.delete_all CacheBuster.new.bust_comment(comment.commentable, comment.user.username) comment.delete end @@ -71,15 +69,30 @@ module Moderator end end + def delete_user_activity + user.notifications.delete_all + user.reactions.delete_all + user.follows.delete_all + Follow.where(followable_id: user.id, followable_type: "User").delete_all + user.chat_channel_memberships.delete_all + user.mentions.delete_all + user.badge_achievements.delete_all + user.github_repos.delete_all + delete_comments + delete_articles + end + + def full_delete + delete_user_activity + CacheBuster.new.bust("/#{user.old_username}") + user.delete + end + def banish - # return unless user.comments.where("created_at < ?", 150.days.ago).empty? reassign_and_bust_username remove_profile_info add_banned_role - delete_reactions - delete_comments - delete_articles - delete_follows + delete_user_activity user.remove_from_algolia_index user.save! end diff --git a/app/views/internal/users/edit.html.erb b/app/views/internal/users/edit.html.erb index ade251500..ace377e6e 100644 --- a/app/views/internal/users/edit.html.erb +++ b/app/views/internal/users/edit.html.erb @@ -1,23 +1,45 @@ -

<%= @user.name %>

-

@<%= @user.username %>

-

<%= @user.comments.size %> comments

-

<%= @user.articles.size %> articles

-

<%= @user.reactions.size %> reactions

-

Joined <%= @user.created_at.to_formatted_s(:long_ordinal) %>

-<% if @user.banned %> -

User is banned

- <% @user.notes.each do |note| %> - <%= note.reason %> | <%= note.content %> - <% end %> -<% else %> +

+ <%= @user.name%> (@<%= @user.username %>) +

+

click here view user details & take other actions

+

Member since <%= @user.created_at.strftime("%b %e '%y") %>

+

Activity:

+
- <% if @user.comments.where("created_at < ?", 100.days.ago).empty? %> - <%= form_for(@user, :url => banish_internal_user_path(@user), :html => {:method => :post, :onsubmit => "confirm('Are you sure? This is extremely destructive and will change their username to @spam_###. Do not do it lightly.')"}) do %> +

Banish User

+ <% if @user.comments.where("created_at < ?", 100.days.ago).empty? && current_user.has_role?(:super_admin) %> +

This is not a new user. You are only allowed to take this action because you are a super admin.

+

This is extremely destructive. Banishing will delete all the user's existing content and change their username to @spam_###.

+

Do not do this lightly.

+ <%= form_for(@user, :url => banish_internal_user_path(@user), :html => {:method => :post, :onsubmit => "confirm('Are you sure? This is extremely destructive. Banishing will delete all the user's existing content and change their username to @spam_###. Do not do it lightly.')"}) do %> <% end %> - This is extremely destructive and will change their username to @spam_###. Do not do it lightly. + <% elsif @user.comments.where("created_at < ?", 100.days.ago).empty? %> +

This is extremely destructive. Banishing will delete all the user's existing content and change their username to @spam_###.

+

Do not do this lightly.

+ <%= form_for(@user, :url => banish_internal_user_path(@user), :html => {:method => :post, :onsubmit => "confirm('Are you sure? This is extremely destructive. Banishing will delete all the user's existing content and change their username to @spam_###. Do not do it lightly.')"}) do %> + + <% end %> <% else %> - You cannot take admin action from this view because this is not a newish user. For safety. +

This is not a new user so you'll need to contact a super admin to banish this user.

<% end %>
-<% end %> + +
+

Fully Delete User

+ <% if current_user.has_role?(:super_admin) %> +

This will completely destroy the user and all of their activity from our database. This action is irreversible.

+

Do not do this lightly.

+ <%= form_for(@user, :url => full_delete_internal_user_path(@user), :html => {:method => :post, :onsubmit => "confirm('Are you sure? This is extremely destructive and irreversible.')"}) do %> + + <% end %> + <% else %> +

Only super admins can fully delete users.

+ <% end %> +
+ + diff --git a/app/views/internal/users/index.html.erb b/app/views/internal/users/index.html.erb index 1bdcccbb1..ec81053fc 100644 --- a/app/views/internal/users/index.html.erb +++ b/app/views/internal/users/index.html.erb @@ -1,3 +1,8 @@ +

All Users || Mentors Only || diff --git a/app/views/internal/users/show.html.erb b/app/views/internal/users/show.html.erb index 6bc4c155d..6d9e0d7e8 100644 --- a/app/views/internal/users/show.html.erb +++ b/app/views/internal/users/show.html.erb @@ -2,8 +2,18 @@

<%= @user.name%> (@<%= @user.username %>)

+

click here to banish or delete user

Member since <%= @user.created_at.strftime("%b %e '%y") %>

Email: <%= @user.email %>

+

Activity:

+ + + +

Current Roles

<%= form_for ([:internal, @user]) do |f| %> - <% if @user.banned %> + <% if @user.banned && current_user.has_role?(:super_admin) %>

🚨 Banned User 🚨

-

You can only unban this user through the admin panel.

+ <%= f.label "Un-ban User (return to good standing)" %> + <%= f.check_box :good_standing_user %> +
+ +
+ <%= f.submit "Update User Status" %> + <% elsif @user.banned %> +

🚨 Banned User 🚨

+

Only super-admins can unban users.

<% elsif @user.warned %>

Warned User

<%= f.label :ban_user %> @@ -107,7 +125,7 @@ <% end %>
-<% if @user.seeking_mentorship && @user.mentee_description.present? %> +<% if @user.seeking_mentorship && @user.mentee_description.present? && !@user.banned_from_mentorship %>

Mentee Description

@@ -139,39 +157,40 @@ order("mentor_form_updated_at DESC").limit(175). sort_by { |possible_mentor| UserSimilarity.new(@user, possible_mentor).score }.reverse. first(40).each do |possible_mentor| %> -
-
- -
- @<%= possible_mentor.username %> -
- Internal(<%= possible_mentor.id %>) + + <% if !possible_mentor.banned_from_mentorship %> +
+
+ +
+ @<%= possible_mentor.username %> +
+ Internal Profile (id #<%= possible_mentor.id %>) +
+
+

Tag Intersection

+ <%= UserSimilarity.new(@user, possible_mentor).tag_intersection.join(", ") %> +
+
+

Mentor description

+ <%= possible_mentor.mentor_description %> +
+
+

Skills/Languages

+ <%= possible_mentor.mostly_work_with %> +
+
+

Action

+ <%= form_for ([:internal, @user]) do |f| %> + <%= f.hidden_field :quick_match, value: true %> + <%= f.hidden_field :add_mentor, value: possible_mentor.id %> + <%= f.submit "Match!" %> + <% end %> +
-
-

Tag Intersection

- <%= UserSimilarity.new(@user, possible_mentor).tag_intersection.join(", ") %> -
-
-

Mentor description

- <%= possible_mentor.mentor_description %> -
-
-

Skills/Languages

- <%= possible_mentor.mostly_work_with %> -
-
-

Action

- <%= form_for ([:internal, @user]) do |f| %> - <%= f.hidden_field :quick_match, value: true %> - <%= f.hidden_field :add_mentor, value: possible_mentor.id %> - <%= f.submit "Match!" %> - <% end %> -
-
+ <% end %> <% end %> <% end %> - -

Recent Emails

    <% @user.email_messages.order("sent_at DESC").limit(50).each do |email| %> diff --git a/config/routes.rb b/config/routes.rb index 64490e695..2a8fc2720 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -31,6 +31,7 @@ Rails.application.routes.draw do resources :users do member do post "banish" + post "full_delete" end end resources :events diff --git a/spec/controllers/internal_users_controller_spec.rb b/spec/controllers/internal_users_controller_spec.rb index 06d8c08c3..ac2cff38d 100644 --- a/spec/controllers/internal_users_controller_spec.rb +++ b/spec/controllers/internal_users_controller_spec.rb @@ -1,57 +1,103 @@ require "rails_helper" RSpec.describe "internal/users", type: :request do - context "when banishing user" do - let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:user3) { create(:user) } - let(:super_admin) { create(:user, :super_admin) } - let(:article) { create(:article, user: user) } - let(:article2) { create(:article, user: user2) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:user3) { create(:user) } + let(:super_admin) { create(:user, :super_admin) } + let(:article) { create(:article, user: user) } + let(:article2) { create(:article, user: user2) } + + before do + sign_in super_admin + user + user2 + Delayed::Worker.delay_jobs = true + dependents_for_offending_user_article + offender_activity_on_other_content + end + + after do + Delayed::Worker.delay_jobs = false + end + + def dependents_for_offending_user_article + # create user2 comment on offending user article + comment = create(:comment, commentable_type: "Article", commentable: article, user: user2) + # create user3 reaction to user2 comment + create(:reaction, reactable: comment, reactable_type: "Comment", user: user3) + # create user3 comment response to user2 comment + comment2 = create(:comment, commentable_type: "Article", commentable: article, user: user3, ancestry: comment.id) + # create user2 reaction to user3 comment response + create(:reaction, reactable: comment2, reactable_type: "Comment", user: user2) + # create user3 reaction to offending article + create(:reaction, reactable: article, reactable_type: "Article", user: user3, category: "like") + Delayed::Worker.new(quiet: true).work_off + end + + def offender_activity_on_other_content + # offender reacts to user2 article + create(:reaction, reactable: article2, reactable_type: "Article", user: user) + # offender comments on user2 article + comment = create(:comment, commentable_type: "Article", commentable: article2, user: user) + # user3 reacts to offender comment + create(:reaction, reactable: comment, reactable_type: "Comment", user: user3) + Delayed::Worker.new(quiet: true).work_off + end + + context "when deleting user" do + def create_mention + comment = create( + :comment, + body_markdown: "Hello @#{user.username}, you are cool.", + user_id: user2.id, + commentable_id: article2.id, + ) + Mention.create_all_without_delay(comment) + end + + def create_mutual_follows + user.follow(user3) + follow = user3.follow(user) + Notification.send_new_follower_notification_without_delay(follow) + end before do - sign_in super_admin - user - user2 - Delayed::Worker.delay_jobs = true - dependents_for_offending_user_article - offender_activity_on_other_content + create_mutual_follows + create_mention + create(:badge_achievement, rewarder_id: 1, rewarding_context_message: "yay", user_id: user.id) + Delayed::Worker.new(quiet: true).work_off end - after do - Delayed::Worker.delay_jobs = false + it "raises a 'record not found' error after deletion" do + post "/internal/users/#{user.id}/full_delete" + expect { User.find(user.id) }.to raise_exception(ActiveRecord::RecordNotFound) + end + end + + context "when banning from mentorship" do + before do + user.update(offering_mentorship: true, mentor_description: "I want to be a mentor") end + it "adds banned from mentorship role" do + put "/internal/users/#{user.id}", params: { user: { ban_from_mentorship: "1", note_for_mentorship_ban: "banned" } } + expect(user.roles.first.name).to eq("banned_from_mentorship") + end + + it "returns user to good standing if unbanned" do + put "/internal/users/#{user.id}", params: { user: { good_standing_user: "1" } } + expect(user.roles.count).to eq(0) + end + end + + context "when banishing user" do def banish_user post "/internal/users/#{user.id}/banish" Delayed::Worker.new(quiet: true).work_off user.reload end - def dependents_for_offending_user_article - # create user2 comment on offending user article - comment = create(:comment, commentable_type: "Article", commentable: article, user: user2) - # create user3 reaction to user2 comment - create(:reaction, reactable: comment, reactable_type: "Comment", user: user3) - # create user3 comment response to user2 comment - comment2 = create(:comment, commentable_type: "Article", commentable: article, user: user3, ancestry: comment.id) - # create user2 reaction to user3 comment response - create(:reaction, reactable: comment2, reactable_type: "Comment", user: user2) - # create user3 reaction to offending article - create(:reaction, reactable: article, reactable_type: "Article", user: user3, category: "like") - Delayed::Worker.new(quiet: true).work_off - end - - def offender_activity_on_other_content - # offender reacts to user2 article - create(:reaction, reactable: article2, reactable_type: "Article", user: user) - # offender comments on user2 article - comment = create(:comment, commentable_type: "Article", commentable: article2, user: user) - # user3 reacts to offender comment - create(:reaction, reactable: comment, reactable_type: "Comment", user: user3) - Delayed::Worker.new(quiet: true).work_off - end - it "reassigns username and removes profile info" do user.currently_hacking_on = "currently hackin on !!!!!!!!!!!!" user.save