FEATURE: Add bulk destroy to admin users list

app/assets/javascripts/admin/addon/controllers/admin-users-list-show.js

@@ -1,7 +1,11 @@
+import { tracked } from "@glimmer/tracking";
 import Controller from "@ember/controller";
 import { action } from "@ember/object";
+import { service } from "@ember/service";
 import { observes } from "@ember-decorators/object";
 import { computedI18n } from "discourse/lib/computed";
+import { ajax } from "discourse/lib/ajax";
+import { extractError } from "discourse/lib/ajax-error";
 import CanCheckEmails from "discourse/mixins/can-check-emails";
 import { INPUT_DELAY } from "discourse-common/config/environment";
 import discourseDebounce from "discourse-common/lib/debounce";
@@ -12,14 +16,19 @@ import AdminUser from "admin/models/admin-user";
 export default class AdminUsersListShowController extends Controller.extend(
   CanCheckEmails
 ) {
-  model = null;
+  @service dialog;
+
+  @tracked bulkSelect = false;
+  @tracked displayBulkActions = false;
+  @tracked bulkSelectedUsers = null;
+
   query = null;
   order = null;
   asc = null;
+  users = null;
   showEmails = false;
   refreshing = false;
   listFilter = null;
-  selectAll = false;
 
   @computedI18n("search_hint") searchHint;
 
@@ -76,7 +85,7 @@ export default class AdminUsersListShowController extends Controller.extend(
     })
       .then((result) => {
         this._results[page] = result;
-        this.set("model", this._results.flat());
+        this.set("users", this._results.flat());
 
         if (result.length === 0) {
           this._canLoadMore = false;
@@ -106,4 +115,54 @@ export default class AdminUsersListShowController extends Controller.extend(
       asc,
     });
   }
+
+  @action
+  toggleBulkSelect() {
+    this.bulkSelect = !this.bulkSelect;
+    this.displayBulkActions = false;
+    this.bulkSelectedUsers = null;
+  }
+
+  @action
+  bulkSelectItemToggle(userId, event) {
+    if (!this.bulkSelectedUsers) {
+      this.bulkSelectedUsers = {};
+    }
+
+    if (event.target.checked) {
+      this.bulkSelectedUsers[userId] = 1;
+    } else {
+      delete this.bulkSelectedUsers[userId];
+    }
+    this.displayBulkActions = Object.keys(this.bulkSelectedUsers).length > 0;
+  }
+
+  @action
+  performBulkDelete() {
+    const userIds = Object.keys(this.bulkSelectedUsers);
+    const count = userIds.length;
+    this.dialog.deleteConfirm({
+      title: I18n.t("admin.users.bulk_actions.confirm_delete_title", {
+        count,
+      }),
+      message: I18n.t("admin.users.bulk_actions.confirm_delete_body", {
+        count,
+      }),
+      confirmButtonClass: "btn-danger",
+      confirmButtonIcon: "trash-can",
+      didConfirm: async () => {
+        try {
+          await ajax("/admin/users/destroy-bulk.json", {
+            type: "DELETE",
+            data: { user_ids: userIds },
+          });
+          this.bulkSelectedUsers = null;
+          this.displayBulkActions = false;
+          this.resetFilters();
+        } catch (err) {
+          this.dialog.alert(extractError(err));
+        }
+      },
+    });
+  }
 }

app/assets/javascripts/admin/addon/routes/admin-users-list-show.js

@@ -24,6 +24,8 @@ export default class AdminUsersListShowRoute extends DiscourseRoute {
           listFilter: transition.to.queryParams.username,
           query: params.filter,
           refreshing: false,
+          bulkSelectedUsers: null,
+          displayBulkActions: false,
         });
 
         controller.resetFilters();

app/assets/javascripts/admin/addon/templates/users-list-show.hbs

@@ -19,19 +19,46 @@
   {{/if}}
 </div>
 
-<div class="username controls">
-  <TextField
-    @value={{this.listFilter}}
-    @placeholder={{this.searchHint}}
-    @title={{this.searchHint}}
-  />
+<div class="admin-users-list__controls">
+  <div class="username">
+    <TextField
+      @value={{this.listFilter}}
+      @placeholder={{this.searchHint}}
+      @title={{this.searchHint}}
+    />
+  </div>
+  {{#if this.displayBulkActions}}
+    <div class="bulk-actions-dropdown">
+      <DMenu @autofocus={{true}} @identifier="bulk-select-admin-users-dropdown">
+        <:trigger>
+          <span class="d-button-label">
+            {{i18n "admin.users.bulk_actions.title"}}
+          </span>
+          {{dIcon "angle-down"}}
+        </:trigger>
+
+        <:content>
+          <DropdownMenu as |dropdown|>
+            <dropdown.item>
+              <DButton
+                @translatedLabel={{i18n "admin.users.bulk_actions.delete"}}
+                @icon="trash-can"
+                @action={{this.performBulkDelete}}
+                class="bulk-delete btn-danger"
+              />
+            </dropdown.item>
+          </DropdownMenu>
+        </:content>
+      </DMenu>
+    </div>
+  {{/if}}
 </div>
 <LoadMore
   @selector=".directory-table .directory-table__cell"
   @action={{action "loadMore"}}
   class="users-list-container"
 >
-  {{#if this.model}}
+  {{#if this.users}}
     <ResponsiveTable
       @className="users-list"
       @aria-label={{this.title}}
@@ -42,18 +69,24 @@
           ", minmax(min-content, 1fr))"
         )
       }}
-      @updates={{this.model.email}}
     >
       <:header>
-        <TableHeaderToggle
-          @onToggle={{this.updateOrder}}
-          @field="username"
-          @labelKey="username"
-          @order={{this.order}}
-          @asc={{this.asc}}
-          @automatic={{true}}
-          class="directory-table__column-header--username"
-        />
+        <div class="directory-table__column-header-wrapper">
+          <DButton
+            class="btn-flat bulk-select"
+            @icon="list-check"
+            @action={{this.toggleBulkSelect}}
+          />
+          <TableHeaderToggle
+            @onToggle={{this.updateOrder}}
+            @field="username"
+            @labelKey="username"
+            @order={{this.order}}
+            @asc={{this.asc}}
+            @automatic={{true}}
+            class="directory-table__column-header--username"
+          />
+        </div>
         <TableHeaderToggle
           @onToggle={{this.updateOrder}}
           @field="email"
@@ -130,14 +163,23 @@
       </:header>
 
       <:body>
-        {{#each this.model as |user|}}
+        {{#each this.users as |user|}}
           <div
             class="user
               {{user.selected}}
               {{unless user.active 'not-activated'}}
               directory-table__row"
+            data-user-id={{user.id}}
           >
             <div class="directory-table__cell username">
+              {{#if this.bulkSelect}}
+                <input
+                  type="checkbox"
+                  class="directory-table__cell-bulk-select"
+                  checked={{eq (get this.bulkSelectedUsers user.id) 1}}
+                  {{on "click" (fn this.bulkSelectItemToggle user.id)}}
+                />
+              {{/if}}
               <a
                 class="avatar"
                 href={{user.path}}

app/assets/javascripts/discourse/app/components/responsive-table.hbs

@@ -8,7 +8,7 @@
     aria-label={{@ariaLabel}}
     style={{@style}}
     {{did-insert this.checkScroll}}
-    {{did-update this.checkScroll @updates}}
+    {{did-update this.checkScroll}}
     {{on-resize this.checkScroll}}
     {{on "scroll" this.onBottomScroll}}
   >

app/assets/stylesheets/common/admin/admin_base.scss

@@ -473,7 +473,7 @@ $mobile-breakpoint: 700px;
   }
 
   .username {
-    input {
+    input[type="text"] {
       min-width: 15em;
       @media screen and (max-width: 500px) {
         box-sizing: border-box;

app/assets/stylesheets/common/admin/users.scss

@@ -115,11 +115,25 @@
 
   .directory-table {
     margin-top: 1em;
-    &__column-header--username,
-    &__column-header--email {
-      .header-contents {
-        text-align: left;
+    &__column-header {
+      &--username,
+      &---email {
+        .header-contents {
+          text-align: left;
+        }
       }
+
+      &--username {
+        flex-grow: 1;
+      }
+    }
+
+    &__column-header-wrapper {
+      display: flex;
+    }
+
+    &__cell-bulk-select {
+      margin-right: 1em;
     }
 
     &__cell.username {
@@ -148,6 +162,11 @@
   .avatar {
     margin-right: 0.25em;
   }
+
+  &__controls {
+    display: flex;
+    gap: 1em;
+  }
 }
 
 // mobile styles

app/controllers/admin/users_controller.rb

@@ -402,6 +402,22 @@ class Admin::UsersController < Admin::StaffController
     end
   end
 
+  def destroy_bulk
+    hijack do
+      User::BulkDestroy.call(service_params) do
+        on_success { render json: { deleted: true } }
+
+        on_failed_contract do |contract|
+          render json: failed_json.merge(errors: contract.errors.full_messages), status: 400
+        end
+
+        on_failed_policy(:can_delete_users) do
+          render json: failed_json.merge(errors: [I18n.t("user.cannot_bulk_delete")]), status: 403
+        end
+      end
+    end
+  end
+
   def badges
   end
 

app/services/user/bulk_destroy.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+class User::BulkDestroy
+  include Service::Base
+
+  params do
+    attribute :user_ids, :array
+
+    validates :user_ids, length: { maximum: 100 }
+  end
+
+  model :users
+  policy :can_delete_users
+  step :delete
+
+  private
+
+  def fetch_users(params:)
+    User.where(id: params.user_ids.to_a)
+  end
+
+  def can_delete_users(guardian:, users:)
+    users.all? { |u| guardian.can_delete_user?(u) }
+  end
+
+  def delete(users:, guardian:)
+    users.each do |u|
+      UserDestroyer.new(guardian.user).destroy(
+        u,
+        delete_posts: true,
+        context: I18n.t("staff_action_logs.bulk_user_delete", users: users.map(&:id).inspect),
+      )
+    end
+  end
+end

config/locales/client.en.yml

@@ -6639,6 +6639,15 @@ en:
         status: "Status"
         show_emails: "Show Emails"
         hide_emails: "Hide Emails"
+        bulk_actions:
+          title: "Bulk actions"
+          delete: "Delete users…"
+          confirm_delete_title:
+            one: "Delete %{count} user?"
+            other: "Delete %{count} users?"
+          confirm_delete_body:
+            one: "Are you sure you want to delete %{count} user? This action is permanent and cannot be reversed."
+            other: "Are you sure you want to delete %{count} users? This action is permanent and cannot be reversed."
         nav:
           new: "New"
           active: "Active"

config/locales/server.en.yml

@@ -3062,6 +3062,7 @@ en:
     cannot_delete_has_posts:
       one: "User %{username} has %{count} post in a public topic or personal message, so they can't be deleted."
       other: "User %{username} has %{count} posts in public topics or personal messages, so they can't be deleted."
+    cannot_bulk_delete: "One or more users cannot be deleted either because they're an admin, have too many posts or have a very old post."
 
   unsubscribe_mailer:
     title: "Unsubscribe Mailer"
@@ -5534,6 +5535,7 @@ en:
         other: "Automatically revoked, created at more than %{count} days ago"
       revoked: Revoked
       restored: Restored
+    bulk_user_delete: "bulk deletion of users: %{users}"
 
   reviewables:
     already_handled: "Thanks, but we've already reviewed that post and determined it does not need to be flagged again."

config/routes.rb

@@ -133,6 +133,7 @@ Discourse::Application.routes.draw do
           delete "delete-others-with-same-ip" => "users#delete_other_accounts_with_same_ip"
           get "total-others-with-same-ip" => "users#total_other_accounts_with_same_ip"
           put "approve-bulk" => "users#approve_bulk"
+          delete "destroy-bulk" => "users#destroy_bulk"
         end
         delete "penalty_history", constraints: AdminConstraint.new
         put "suspend"

spec/requests/admin/users_controller_spec.rb

@@ -1435,6 +1435,67 @@ RSpec.describe Admin::UsersController do
     end
   end
 
+  describe "#destroy_bulk" do
+    fab!(:deleted_users) { Fabricate.times(3, :user) }
+
+    shared_examples "bulk user deletion possible" do
+      it "can delete multiple users" do
+        delete "/admin/users/destroy-bulk.json", params: { user_ids: deleted_users.map(&:id) }
+        expect(response.status).to eq(200)
+        expect(User.where(id: deleted_users.map(&:id)).count).to eq(0)
+      end
+
+      it "doesn't allow deleting a user that can't be deleted" do
+        deleted_users[0].update!(admin: true)
+
+        delete "/admin/users/destroy-bulk.json", params: { user_ids: deleted_users.map(&:id) }
+        expect(response.status).to eq(403)
+        expect(User.where(id: deleted_users.map(&:id)).count).to eq(3)
+      end
+
+      it "doesn't accept more than 100 user ids" do
+        delete "/admin/users/destroy-bulk.json",
+               params: {
+                 user_ids: deleted_users.map(&:id) + (1..101).to_a,
+               }
+        expect(response.status).to eq(400)
+        expect(User.where(id: deleted_users.map(&:id)).count).to eq(3)
+      end
+
+      it "doesn't fail when a user id doesn't exist" do
+        user_id = (User.unscoped.maximum(:id) || 0) + 1
+        delete "/admin/users/destroy-bulk.json",
+               params: {
+                 user_ids: deleted_users.map(&:id).push(user_id),
+               }
+        expect(response.status).to eq(200)
+        expect(User.where(id: deleted_users.map(&:id)).count).to eq(0)
+      end
+    end
+
+    context "when logged in as an admin" do
+      before { sign_in(admin) }
+
+      include_examples "bulk user deletion possible"
+    end
+
+    context "when logged in as a moderator" do
+      before { sign_in(moderator) }
+
+      include_examples "bulk user deletion possible"
+    end
+
+    context "when logged in as a non-staff user" do
+      before { sign_in(user) }
+
+      it "responds with a 404 and doesn't delete users" do
+        delete "/admin/users/destroy-bulk.json", params: { user_ids: deleted_users.map(&:id) }
+        expect(response.status).to eq(404)
+        expect(User.where(id: deleted_users.map(&:id)).count).to eq(3)
+      end
+    end
+  end
+
   describe "#activate" do
     fab!(:reg_user) { Fabricate(:inactive_user) }
 

spec/system/admin_users_list_spec.rb

@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+describe "Admin Users Page", type: :system do
+  fab!(:current_user) { Fabricate(:admin) }
+  fab!(:users) { Fabricate.times(3, :user) }
+
+  let(:admin_users_page) { PageObjects::Pages::AdminUsers.new }
+  let(:dialog) { PageObjects::Components::Dialog.new }
+
+  before { sign_in(current_user) }
+
+  it "has a button that toggles the bulk select checkboxes" do
+    admin_users_page.visit
+
+    expect(admin_users_page).to have_users(users.map(&:id))
+
+    expect(admin_users_page.user_row(users[0].id)).to have_no_bulk_select_checkbox
+    expect(admin_users_page.user_row(users[1].id)).to have_no_bulk_select_checkbox
+    expect(admin_users_page.user_row(users[2].id)).to have_no_bulk_select_checkbox
+
+    admin_users_page.bulk_select_button.click
+
+    expect(admin_users_page.user_row(users[0].id)).to have_bulk_select_checkbox
+    expect(admin_users_page.user_row(users[1].id)).to have_bulk_select_checkbox
+    expect(admin_users_page.user_row(users[2].id)).to have_bulk_select_checkbox
+
+    expect(admin_users_page).to have_no_bulk_actions_dropdown
+
+    admin_users_page.user_row(users[0].id).bulk_select_checkbox.click
+
+    expect(admin_users_page).to have_bulk_actions_dropdown
+
+    admin_users_page.user_row(users[1].id).bulk_select_checkbox.click
+    admin_users_page.bulk_actions_dropdown.expand
+    admin_users_page.bulk_actions_dropdown.option(".bulk-delete").click
+
+    expect(dialog).to be_open
+    dialog.click_danger
+
+    deleted_ids = users[0..1].map(&:id)
+    expect(admin_users_page).to have_no_users(deleted_ids)
+    expect(User.where(id: deleted_ids).count).to eq(0)
+  end
+
+  it "remembers selected users when the user list refreshes due to search" do
+    admin_users_page.visit
+    admin_users_page.bulk_select_button.click
+    admin_users_page.search_input.fill_in(with: users[0].username)
+    admin_users_page.user_row(users[0].id).bulk_select_checkbox.click
+
+    admin_users_page.search_input.fill_in(with: users[1].username)
+    admin_users_page.user_row(users[1].id).bulk_select_checkbox.click
+
+    admin_users_page.search_input.fill_in(with: "")
+
+    expect(admin_users_page).to have_users(users.map(&:id))
+    expect(admin_users_page.user_row(users[0].id).bulk_select_checkbox).to be_checked
+    expect(admin_users_page.user_row(users[1].id).bulk_select_checkbox).to be_checked
+    expect(admin_users_page.user_row(users[2].id).bulk_select_checkbox).not_to be_checked
+
+    admin_users_page.bulk_actions_dropdown.expand
+    admin_users_page.bulk_actions_dropdown.option(".bulk-delete").click
+
+    expect(dialog).to be_open
+    dialog.click_danger
+
+    deleted_ids = users[0..1].map(&:id)
+    expect(admin_users_page).to have_no_users(deleted_ids)
+    expect(User.where(id: deleted_ids).count).to eq(0)
+  end
+end

spec/system/page_objects/components/d_menu.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+module PageObjects
+  module Components
+    class DMenu < PageObjects::Components::Base
+      attr_reader :component
+
+      def initialize(input)
+        if input.is_a?(Capybara::Node::Element)
+          @component = input
+        else
+          @component = find(input)
+        end
+      end
+
+      def expand
+        raise "DMenu is already expanded" if is_expanded?
+        component.click
+      end
+
+      def collapse
+        raise "DMenu is already collapsed" if is_collapsed?
+        component.click
+      end
+
+      def is_expanded?
+        component["aria-expanded"] == "true"
+      end
+
+      def is_collapsed?
+        !is_expanded?
+      end
+
+      def option(selector)
+        within("#d-menu-portals") { find(selector) }
+      end
+    end
+  end
+end

spec/system/page_objects/pages/admin_users.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+module PageObjects
+  module Pages
+    class AdminUsers < PageObjects::Pages::Base
+      class UserRow
+        attr_reader :element
+
+        def initialize(element)
+          @element = element
+        end
+
+        def bulk_select_checkbox
+          element.find(".directory-table__cell-bulk-select")
+        end
+
+        def has_bulk_select_checkbox?
+          element.has_css?(".directory-table__cell-bulk-select")
+        end
+
+        def has_no_bulk_select_checkbox?
+          element.has_no_css?(".directory-table__cell-bulk-select")
+        end
+      end
+
+      def visit
+        page.visit("/admin/users/list/active")
+      end
+
+      def bulk_select_button
+        find(".btn.bulk-select")
+      end
+
+      def search_input
+        find(".admin-users-list__controls .username input")
+      end
+
+      def user_row(id)
+        UserRow.new(find(".directory-table__row[data-user-id=\"#{id}\"]"))
+      end
+
+      def users_count
+        all(".directory-table__row").size
+      end
+
+      def has_users?(user_ids)
+        user_ids.all? { |id| has_css?(".directory-table__row[data-user-id=\"#{id}\"]") }
+      end
+
+      def has_no_users?(user_ids)
+        user_ids.all? { |id| has_no_css?(".directory-table__row[data-user-id=\"#{id}\"]") }
+      end
+
+      def bulk_actions_dropdown
+        PageObjects::Components::DMenu.new(find(".bulk-select-admin-users-dropdown-trigger"))
+      end
+
+      def has_bulk_actions_dropdown?
+        has_css?(".bulk-select-admin-users-dropdown-trigger")
+      end
+
+      def has_no_bulk_actions_dropdown?
+        has_no_css?(".bulk-select-admin-users-dropdown-trigger")
+      end
+    end
+  end
+end