Self-deleting user account (#4480) [deploy]

* Start with self deleting account

* Moved deleting user content and activity out of moderator hierarchy

* Added tests for the users delete services

* Tests for Users::DeleteComments

* User self-deletion (start)

* Some tests for user self-delete

* Specs for user self-deletion

* Test flash settings on users delete

* Added basic specs for the Users::DeleteJob

* Send notification when a user was destroyed

* Rename Users::DeleteJob to SelfDelete

* Change texts about self-deletion

* Fix users delete job spec

* Rescue and log exceptions while self-deleting user

* Added visible flash notices after deleting user

* Remove unneeded css for flash notice

* Fix link to a ghost account

* Remove redundant css

* Added github and twitter oauth apps links

app/assets/stylesheets/shared.scss

@@ -200,4 +200,16 @@
   100% {
     width: 140%;
   }
-}
+}
+
+.global-notice {
+  font-family: $helvetica;
+  background: $green;
+  color: black;
+  padding: 20px 0px 20px;
+  text-align: center;
+  position: relative;
+  top: 0px;
+  left: 0px;
+  right: 0px;
+}

app/controllers/users_controller.rb

@@ -71,7 +71,7 @@ class UsersController < ApplicationController
     if @user.articles_count.zero? && @user.comments_count.zero?
       @user.destroy!
       NotifyMailer.account_deleted_email(@user).deliver
-      flash[:settings_notice] = "Your account has been deleted."
+      flash[:global_notice] = "Your account has been deleted."
       sign_out @user
       redirect_to root_path
     else
@@ -80,6 +80,15 @@ class UsersController < ApplicationController
     end
   end
 
+  def full_delete
+    set_user
+    set_tabs("account")
+    Users::SelfDeleteJob.perform_later(@user.id)
+    sign_out @user
+    flash[:global_notice] = "Your account deletion is scheduled. You'll be notified when it's deleted."
+    redirect_to root_path
+  end
+
   def remove_association
     set_user
     provider = params[:provider]
@@ -249,10 +258,6 @@ class UsersController < ApplicationController
       %0A%0A
       You can keep any comments and discussion posts under the Ghost account.
       %0A
-      ---OR---
-      %0A
-      Please delete all my personal information, including comments and discussion posts.
-      %0A
       %0A
       Regards,
       %0A

app/jobs/users/self_delete_job.rb

@@ -0,0 +1,15 @@
+module Users
+  class SelfDeleteJob < ApplicationJob
+    queue_as :users_self_delete
+
+    def perform(user_id, service = Users::Delete)
+      user = User.find_by(id: user_id)
+      return unless user
+
+      service.call(user)
+      NotifyMailer.account_deleted_email(user).deliver
+    rescue StandardError => e
+      Rails.logger.error("Error while deleting user: #{e}")
+    end
+  end
+end

app/policies/user_policy.rb

@@ -27,6 +27,10 @@ class UserPolicy < ApplicationPolicy
     current_user?
   end
 
+  def full_delete?
+    current_user?
+  end
+
   def join_org?
     !user_is_banned?
   end

app/services/moderator/delete_user.rb

@@ -12,7 +12,7 @@ module Moderator
       if user_params[:ghostify] == "true"
         new(user: user, admin: admin, user_params: user_params).ghostify
       else
-        new(user: user, admin: admin, user_params: user_params).full_delete
+        Users::Delete.call(user)
       end
     end
 
@@ -24,12 +24,6 @@ module Moderator
       CacheBuster.new.bust("/ghost")
     end
 
-    def full_delete
-      delete_comments
-      delete_articles
-      delete_non_content_activity_and_user
-    end
-
     private
 
     def delete_non_content_activity_and_user

app/services/moderator/manage_activity_and_roles.rb

@@ -13,50 +13,15 @@ module Moderator
     end
 
     def delete_comments
-      return unless user.comments.any?
-
-      cachebuster = CacheBuster.new
-      user.comments.find_each do |comment|
-        comment.reactions.delete_all
-        cachebuster.bust_comment(comment.commentable)
-        comment.delete
-        comment.remove_notifications
-      end
-      cachebuster.bust_user(user)
+      Users::DeleteComments.call(user)
     end
 
     def delete_articles
-      return unless user.articles.any?
-
-      cachebuster = CacheBuster.new
-      virtual_articles = user.articles.map { |article| Article.new(article.attributes) }
-      user.articles.find_each do |article|
-        article.reactions.delete_all
-        article.comments.includes(:user).find_each do |comment|
-          comment.reactions.delete_all
-          cachebuster.bust_comment(comment.commentable)
-          cachebuster.bust_user(comment.user)
-          comment.delete
-        end
-        article.remove_algolia_index
-        article.delete
-        article.purge
-      end
-      virtual_articles.each do |article|
-        cachebuster.bust_article(article)
-      end
+      Users::DeleteArticles.call(user)
     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.messages.delete_all
-      user.chat_channel_memberships.delete_all
-      user.mentions.delete_all
-      user.badge_achievements.delete_all
-      user.github_repos.delete_all
+      Users::DeleteActivity.call(user)
     end
 
     def remove_privileges

app/services/users/delete.rb

@@ -0,0 +1,36 @@
+module Users
+  class Delete
+    def initialize(user)
+      @user = user
+    end
+
+    def call
+      delete_comments
+      delete_articles
+      delete_user_activity
+      user.unsubscribe_from_newsletters
+      CacheBuster.new.bust("/#{user.username}")
+      user.delete
+    end
+
+    def self.call(*args)
+      new(*args).call
+    end
+
+    private
+
+    attr_reader :user
+
+    def delete_user_activity
+      DeleteActivity.call(user)
+    end
+
+    def delete_comments
+      DeleteComments.call(user)
+    end
+
+    def delete_articles
+      DeleteArticles.call(user)
+    end
+  end
+end

app/services/users/delete_activity.rb

@@ -0,0 +1,17 @@
+module Users
+  module DeleteActivity
+    module_function
+
+    def call(user)
+      user.notifications.delete_all
+      user.reactions.delete_all
+      user.follows.delete_all
+      Follow.where(followable_id: user.id, followable_type: "User").delete_all
+      user.messages.delete_all
+      user.chat_channel_memberships.delete_all
+      user.mentions.delete_all
+      user.badge_achievements.delete_all
+      user.github_repos.delete_all
+    end
+  end
+end

app/services/users/delete_articles.rb

@@ -0,0 +1,26 @@
+module Users
+  module DeleteArticles
+    module_function
+
+    def call(user, cache_buster = CacheBuster.new)
+      return unless user.articles.any?
+
+      virtual_articles = user.articles.map { |article| Article.new(article.attributes) }
+      user.articles.find_each do |article|
+        article.reactions.delete_all
+        article.comments.includes(:user).find_each do |comment|
+          comment.reactions.delete_all
+          cache_buster.bust_comment(comment.commentable)
+          cache_buster.bust_user(comment.user)
+          comment.delete
+        end
+        article.remove_algolia_index
+        article.delete
+        article.purge
+      end
+      virtual_articles.each do |article|
+        cache_buster.bust_article(article)
+      end
+    end
+  end
+end

app/services/users/delete_comments.rb

@@ -0,0 +1,17 @@
+module Users
+  module DeleteComments
+    module_function
+
+    def call(user, cache_buster = CacheBuster.new)
+      return unless user.comments.any?
+
+      user.comments.find_each do |comment|
+        comment.reactions.delete_all
+        cache_buster.bust_comment(comment.commentable)
+        comment.delete
+        comment.remove_notifications
+      end
+      cache_buster.bust_user(user)
+    end
+  end
+end

app/views/layouts/application.html.erb

@@ -88,6 +88,11 @@
     <% end %>
     <div id="message-notice"></div>
     <div id="page-content" class="universal-page-content-wrapper <%= view_class %>" data-current-page="<%= current_page %>">
+      <% if flash[:global_notice] %>
+        <div class="global-notice">
+          <%= flash[:global_notice] %>
+        </div>
+      <% end %>
       <div id="page-content-inner">
         <%= yield %>
       </div>

app/views/users/_account.html.erb

@@ -66,7 +66,7 @@
     </ul>
     <strong>
       Note that this does not revoke our OAuth app access; you will have to do so in your
-      <a href="https://twitter.com/settings/sessions">Twitter profile settings</a> or your
+      <a href="https://twitter.com/settings/applications">Twitter profile settings</a> or your
       <a href="https://github.com/settings/applications">GitHub profile settings</a>.
     </strong>
     <br>
@@ -87,71 +87,72 @@
   </h4>
 <% end %>
 
-<% if @user.articles_count.zero? && @user.comments_count.zero? %>
-  <h3>Delete Account</h3>
-  <h4 style="font-weight:400;">
-    <%= form_tag "/users/destroy", method: :delete, autocomplete: "off" do %>
-      Deleting your account will:
-      <ul class="delete__account">
-        <li>delete your profile, along with your Twitter and/or GitHub associations.
-          This does not include app permissions, which you will have to remove yourself on Twitter/GitHub.
-        </li>
-        <%# TODO: expand the delete messaging later %>
-        <li>delete any and all content you have, such as your reading list or chat messages.</li>
-        <li>allow your username to become available to anyone.</li>
-      </ul>
-      <div class="field">
-        <label for="delete__account__username__field">
-          <b>To delete your account, type in your username:</b>
-        </label>
-        <input type="text" name="delete__account__username__field" id="delete__account__username__field">
-      </div>
-      <div class="field">
-        <label for="delete__account__verification__field">
-          <b>For verification, type the words:</b><i> delete my account</i>
-        </label>
-        <input type="text" name="delete__account__verification__field" id="delete__account__verification__field">
-      </div>
-      <button class="big-action danger-button small-button" id="delete__account__btn" type="submit" disabled="true">
-        DELETE ACCOUNT
-      </button>
-    <% end %>
-  </h4>
-  <br>
-  <script>
-    var deleteAccountBtn = document.getElementById('delete__account__btn')
-    var deleteAccountUsernameInput = document.getElementById('delete__account__username__field')
-    var deleteAcccountVerificationInput = document.getElementById('delete__account__verification__field')
+<h3>Delete Account</h3>
+<h4 style="font-weight:400;">
+  <% delete_path = @user.articles_count.zero? && @user.comments_count.zero? ? user_destroy_path : user_full_delete_path %>
+  <%= form_tag delete_path, method: :delete, autocomplete: "off", id: "delete__account" do %>
+    Deleting your account will:
+    <ul class="delete__account">
+      <li>delete your profile, along with your Twitter and/or GitHub associations.
+        This does not include app permissions. You will have to remove them yourself on
+        <a href="https://twitter.com/settings/applications">Twitter profile settings</a> and
+        <a href="https://github.com/settings/applications">GitHub profile settings</a>.
+      </li>
+      <%# TODO: expand the delete messaging later %>
+      <li>delete any and all content you have, such as articles, comments, your reading list or chat messages.</li>
+      <li>allow your username to become available to anyone.</li>
+    </ul>
+    <div class="field">
+      <label for="delete__account__username__field">
+        <b>To delete your account, type in your username:</b>
+      </label>
+      <input type="text" name="delete__account__username__field" id="delete__account__username__field">
+    </div>
+    <div class="field">
+      <label for="delete__account__verification__field">
+        <b>For verification, type the words:</b><i> delete my account</i>
+      </label>
+      <input type="text" name="delete__account__verification__field" id="delete__account__verification__field">
+    </div>
+    <button class="big-action danger-button small-button" id="delete__account__btn" type="submit" disabled="true">
+      DELETE ACCOUNT
+    </button>
+  <% end %>
+</h4>
+<br>
+<script>
+  var deleteAccountBtn = document.getElementById('delete__account__btn')
+  var deleteAccountUsernameInput = document.getElementById('delete__account__username__field')
+  var deleteAcccountVerificationInput = document.getElementById('delete__account__verification__field')
 
-    function bothInputsVerified() {
-      return deleteAccountUsernameInput.value === '<%= @user.username %>' && deleteAcccountVerificationInput.value === "delete my account"
+  function bothInputsVerified() {
+    return deleteAccountUsernameInput.value === '<%= @user.username %>' && deleteAcccountVerificationInput.value === "delete my account"
+  }
+
+  deleteAccountUsernameInput.addEventListener('input', function () {
+    if (bothInputsVerified()) {
+      deleteAccountBtn.disabled = false;
+      // enable the button via css or removing disabled class
+    } else {
+      deleteAccountBtn.disabled = true;
     }
+  })
 
-    deleteAccountUsernameInput.addEventListener('input', function () {
-      if (bothInputsVerified()) {
-        deleteAccountBtn.disabled = false;
-        // enable the button via css or removing disabled class
-      } else {
-        deleteAccountBtn.disabled = true;
-      }
-    })
-
-    deleteAcccountVerificationInput.addEventListener('input', function () {
-      if (bothInputsVerified()) {
-        deleteAccountBtn.disabled = false;
-      } else {
-        deleteAccountBtn.disabled = true;
-      }
-    })
-  </script>
-<% end %>
+  deleteAcccountVerificationInput.addEventListener('input', function () {
+    if (bothInputsVerified()) {
+      deleteAccountBtn.disabled = false;
+    } else {
+      deleteAccountBtn.disabled = true;
+    }
+  })
+</script>
+If you would like to keep your content under the <%= link_to "@ghost", "/ghost" %> account, please:
 <h3>Request Account Deletion</h3>
 <h4 style="font-weight: 400;">
   <br>
   <a href="mailto:<%= ApplicationConfig["DEFAULT_SITE_EMAIL"] %>?subject=Request Account Deletion&body=<%= @email_body %>">
     Click this link to request account deletion via email.
-  </a> This includes all articles, comments, reactions, etc. as well as any personal information you have.
-  <br>
+  </a>
   <br>
   Be sure to change the email template to fit your needs.
 </h4>

config/routes.rb

@@ -227,7 +227,8 @@ Rails.application.routes.draw do
   post "users/remove_org_admin" => "users#remove_org_admin"
   post "users/remove_from_org" => "users#remove_from_org"
   delete "users/remove_association", to: "users#remove_association"
-  delete "users/destroy", to: "users#destroy"
+  delete "users/destroy", to: "users#destroy", as: :user_destroy
+  delete "users/full_delete", to: "users#full_delete", as: :user_full_delete
   post "organizations/generate_new_secret" => "organizations#generate_new_secret"
   post "users/api_secrets" => "api_secrets#create", :as => :users_api_secrets
   delete "users/api_secrets/:id" => "api_secrets#destroy", :as => :users_api_secret

spec/jobs/users/self_delete_job_spec.rb

@@ -0,0 +1,45 @@
+require "rails_helper"
+
+RSpec.describe Users::SelfDeleteJob, type: :job do
+  include_examples "#enqueues_job", "users_self_delete", 1
+
+  describe "#perform_now" do
+    let(:user) { create(:user) }
+    let(:delete) { double }
+
+    before do
+      allow(delete).to receive(:call)
+    end
+
+    context "when user is found" do
+      it "calls the service when a user is found" do
+        described_class.perform_now(user.id, delete)
+        expect(delete).to have_received(:call).with(user)
+      end
+
+      it "sends the notification" do
+        expect do
+          described_class.perform_now(user.id, delete)
+        end.to change(ActionMailer::Base.deliveries, :count).by(1)
+      end
+
+      it "sends the correct notification" do
+        allow(NotifyMailer).to receive(:account_deleted_email).and_call_original
+        described_class.perform_now(user.id, delete)
+        expect(NotifyMailer).to have_received(:account_deleted_email).with(user)
+      end
+    end
+
+    context "when user is not found" do
+      it "doesn't fail" do
+        described_class.perform_now(-1, delete)
+      end
+
+      it "doesn't send the notification" do
+        expect do
+          described_class.perform_now(-1, delete)
+        end.not_to change(ActionMailer::Base.deliveries, :count)
+      end
+    end
+  end
+end

spec/requests/user/user_settings_spec.rb

@@ -270,6 +270,10 @@ RSpec.describe "UserSettings", type: :request do
       it "redirects successfully to the home page" do
         expect(response).to redirect_to "/"
       end
+
+      it "sets flash settings" do
+        expect(flash[:global_notice]).to include("has been deleted")
+      end
     end
 
     context "when users are not allowed to destroy" do
@@ -303,4 +307,27 @@ RSpec.describe "UserSettings", type: :request do
       end
     end
   end
+
+  describe "DELETE /users/full_delete" do
+    before do
+      sign_in user
+    end
+
+    it "schedules a user delete job" do
+      expect do
+        delete "/users/full_delete"
+      end.to have_enqueued_job(Users::SelfDeleteJob).with(user.id)
+    end
+
+    it "signs out" do
+      delete "/users/full_delete"
+      expect(controller.current_user).to eq nil
+    end
+
+    it "redirects to root" do
+      delete "/users/full_delete"
+      expect(response).to redirect_to "/"
+      expect(flash[:global_notice]).to include("Your account deletion is scheduled")
+    end
+  end
 end

spec/services/moderator/delete_user_spec.rb

@@ -0,0 +1,28 @@
+require "rails_helper"
+
+RSpec.describe Moderator::DeleteUser, type: :service do
+  let(:user) { create(:user) }
+  let(:admin) { create(:user, :super_admin) }
+
+  describe "delete_user" do
+    it "deletes user" do
+      described_class.call_deletion(user: user, admin: admin, user_params: {})
+      expect(User.find_by(id: user.id)).to be_nil
+    end
+
+    it "deletes user's follows" do
+      create(:follow, follower: user)
+      create(:follow, followable: user)
+
+      expect do
+        described_class.call_deletion(user: user, admin: admin, user_params: {})
+      end.to change(Follow, :count).by(-2)
+    end
+
+    it "deletes user's articles" do
+      article = create(:article, user: user)
+      described_class.call_deletion(user: user, admin: admin, user_params: {})
+      expect(Article.find_by(id: article.id)).to be_nil
+    end
+  end
+end

spec/services/users/delete_articles_spec.rb

@@ -0,0 +1,40 @@
+require "rails_helper"
+
+RSpec.describe Users::DeleteArticles, type: :service do
+  let(:user) { create(:user) }
+  let(:user2) { create(:user) }
+  let!(:article) { create(:article, user: user) }
+  let!(:article2) { create(:article, user: user) }
+  let!(:article3) { create(:article, user: user2) }
+
+  it "deletes articles" do
+    described_class.call(user)
+    expect(Article.find_by(id: article.id)).to be_nil
+    expect(Article.find_by(id: article2.id)).to be_nil
+    expect(Article.find(article3.id)).to be_present
+  end
+
+  context "with comments" do
+    let(:buster) { double }
+
+    before do
+      allow(buster).to receive(:bust_comment)
+      allow(buster).to receive(:bust_article)
+      allow(buster).to receive(:bust_user)
+
+      create_list(:comment, 2, commentable: article, user: user2)
+    end
+
+    it "deletes articles' comments" do
+      described_class.call(user)
+      expect(Comment.where(commentable_id: article, commentable_type: "Article").any?).to be false
+    end
+
+    it "busts cache" do
+      described_class.call(user, buster)
+      expect(buster).to have_received(:bust_comment).with(article).twice
+      expect(buster).to have_received(:bust_user).with(user2).at_least(:once)
+      expect(buster).to have_received(:bust_article).with(article)
+    end
+  end
+end

spec/services/users/delete_comments_spec.rb

@@ -0,0 +1,25 @@
+require "rails_helper"
+
+RSpec.describe Users::DeleteComments, type: :service do
+  let(:user) { create(:user) }
+  let(:article) { create(:article) }
+  let(:buster) { double }
+
+  before do
+    create_list(:comment, 2, commentable: article, user: user)
+
+    allow(buster).to receive(:bust_comment)
+    allow(buster).to receive(:bust_user)
+  end
+
+  it "destroys user comments" do
+    described_class.call(user, buster)
+    expect(Comment.where(user_id: user.id).any?).to be false
+  end
+
+  it "busts cache" do
+    described_class.call(user, buster)
+    expect(buster).to have_received(:bust_comment).with(article).at_least(:once)
+    expect(buster).to have_received(:bust_user).with(user)
+  end
+end

spec/services/users/delete_spec.rb

@@ -0,0 +1,33 @@
+require "rails_helper"
+
+RSpec.describe Users::Delete, type: :service do
+  let(:user) { create(:user) }
+
+  it "deletes user" do
+    described_class.call(user)
+    expect(User.find_by(id: user.id)).to be_nil
+  end
+
+  it "busts user profile page" do
+    buster = double
+    allow(buster).to receive(:bust)
+    allow(CacheBuster).to receive(:new).and_return(buster)
+    described_class.new(user).call
+    expect(buster).to have_received(:bust).with("/#{user.username}")
+  end
+
+  it "deletes user's follows" do
+    create(:follow, follower: user)
+    create(:follow, followable: user)
+
+    expect do
+      described_class.call(user)
+    end.to change(Follow, :count).by(-2)
+  end
+
+  it "deletes user's articles" do
+    article = create(:article, user: user)
+    described_class.call(user)
+    expect(Article.find_by(id: article.id)).to be_nil
+  end
+end

spec/system/user/user_self_destroy_spec.rb

@@ -0,0 +1,28 @@
+require "rails_helper"
+
+RSpec.describe "User destroys their profile", type: :system, js: true do
+  let(:user) { create(:user, saw_onboarding: true) }
+
+  before do
+    sign_in user
+  end
+
+  it "destroys a user without content" do
+    visit "/settings/account"
+    fill_in "delete__account__username__field", with: user.username
+    fill_in "delete__account__verification__field", with: "delete my account"
+    click_button "DELETE ACCOUNT"
+    expect(User.find_by(id: user.id).present?).to be false
+  end
+
+  it "destroys a user with content" do
+    create(:article, user: user)
+    user.update_attribute(:articles_count, 1)
+    visit "/settings/account"
+    fill_in "delete__account__username__field", with: user.username
+    fill_in "delete__account__verification__field", with: "delete my account"
+    expect do
+      click_button "DELETE ACCOUNT"
+    end.to have_enqueued_job(Users::SelfDeleteJob)
+  end
+end