diff --git a/app/assets/javascripts/admin/addon/components/bulk-user-delete-confirmation.gjs b/app/assets/javascripts/admin/addon/components/bulk-user-delete-confirmation.gjs new file mode 100644 index 00000000000..36c246a0d75 --- /dev/null +++ b/app/assets/javascripts/admin/addon/components/bulk-user-delete-confirmation.gjs @@ -0,0 +1,190 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { on } from "@ember/modifier"; +import { action } from "@ember/object"; +import { service } from "@ember/service"; +import { TrackedArray } from "@ember-compat/tracked-built-ins"; +import DButton from "discourse/components/d-button"; +import DModal from "discourse/components/d-modal"; +import { ajax } from "discourse/lib/ajax"; +import { extractError } from "discourse/lib/ajax-error"; +import { bind } from "discourse-common/utils/decorators"; +import { i18n } from "discourse-i18n"; + +const BULK_DELETE_CHANNEL = "/bulk-user-delete"; + +export default class BulkUserDeleteConfirmation extends Component { + @service messageBus; + + @tracked confirmButtonDisabled = true; + @tracked deleteStarted = false; + @tracked logs = new TrackedArray(); + failedUsernames = []; + + callAfterBulkDelete = false; + + constructor() { + super(...arguments); + this.messageBus.subscribe(BULK_DELETE_CHANNEL, this.onDeleteProgress); + } + + willDestroy() { + super.willDestroy(...arguments); + this.messageBus.unsubscribe(BULK_DELETE_CHANNEL, this.onDeleteProgress); + } + + get confirmDeletePhrase() { + return i18n( + "admin.users.bulk_actions.delete.confirmation_modal.confirmation_phrase", + { count: this.args.model.userIds.length } + ); + } + + #logError(line) { + this.#log(line, "error"); + } + + #logSuccess(line) { + this.#log(line, "success"); + } + + #logNeutral(line) { + this.#log(line, "neutral"); + } + + #log(line, type) { + this.logs.push({ + line, + type, + }); + } + + @bind + onDeleteProgress(data) { + if (data.success) { + this.#logSuccess( + i18n( + "admin.users.bulk_actions.delete.confirmation_modal.user_delete_succeeded", + data + ) + ); + } else if (data.failed) { + this.failedUsernames.push(data.username); + this.#logError( + i18n( + "admin.users.bulk_actions.delete.confirmation_modal.user_delete_failed", + data + ) + ); + } + + if (data.position === data.total) { + this.callAfterBulkDelete = true; + this.#logNeutral( + i18n( + "admin.users.bulk_actions.delete.confirmation_modal.bulk_delete_finished" + ) + ); + if (this.failedUsernames.length > 0) { + this.#logNeutral( + i18n( + "admin.users.bulk_actions.delete.confirmation_modal.failed_to_delete_users" + ) + ); + for (const username of this.failedUsernames) { + this.#logNeutral(`* ${username}`); + } + } + } + } + + @action + onPromptInput(event) { + this.confirmButtonDisabled = + event.target.value.toLowerCase() !== this.confirmDeletePhrase; + } + + @action + async startDelete() { + this.deleteStarted = true; + this.confirmButtonDisabled = true; + this.#logNeutral( + i18n( + "admin.users.bulk_actions.delete.confirmation_modal.bulk_delete_starting" + ) + ); + + try { + await ajax("/admin/users/destroy-bulk.json", { + type: "DELETE", + data: { user_ids: this.args.model.userIds }, + }); + this.callAfterBulkDelete = true; + } catch (err) { + this.#logError(extractError(err)); + this.confirmButtonDisabled = false; + } + } + + @action + closeModal() { + this.args.closeModal(); + if (this.callAfterBulkDelete) { + this.args.model?.afterBulkDelete(); + } + } + + +} diff --git a/app/assets/javascripts/admin/addon/controllers/admin-users-list-show.js b/app/assets/javascripts/admin/addon/controllers/admin-users-list-show.js index 394473aa008..887c132739d 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-users-list-show.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-users-list-show.js @@ -1,25 +1,38 @@ +import { tracked } from "@glimmer/tracking"; import Controller from "@ember/controller"; import { action } from "@ember/object"; -import { observes } from "@ember-decorators/object"; +import { service } from "@ember/service"; import { computedI18n } from "discourse/lib/computed"; import CanCheckEmails from "discourse/mixins/can-check-emails"; import { INPUT_DELAY } from "discourse-common/config/environment"; import discourseDebounce from "discourse-common/lib/debounce"; -import discourseComputed from "discourse-common/utils/decorators"; +import discourseComputed, { bind } from "discourse-common/utils/decorators"; import { i18n } from "discourse-i18n"; +import BulkUserDeleteConfirmation from "admin/components/bulk-user-delete-confirmation"; import AdminUser from "admin/models/admin-user"; +const MAX_BULK_SELECT_LIMIT = 100; + export default class AdminUsersListShowController extends Controller.extend( CanCheckEmails ) { - model = null; + @service dialog; + @service modal; + @service toasts; + + @tracked bulkSelect = false; + @tracked displayBulkActions = false; + @tracked bulkSelectedUserIdsSet = new Set(); + @tracked bulkSelectedUsersMap = {}; + query = null; order = null; asc = null; + users = null; showEmails = false; refreshing = false; listFilter = null; - selectAll = false; + lastSelected = null; @computedI18n("search_hint") searchHint; @@ -47,16 +60,11 @@ export default class AdminUsersListShowController extends Controller.extend( return colCount; } - @observes("listFilter") - _filterUsers() { - discourseDebounce(this, this.resetFilters, INPUT_DELAY); - } - resetFilters() { this._page = 1; this._results = []; this._canLoadMore = true; - this._refreshUsers(); + return this._refreshUsers(); } _refreshUsers() { @@ -67,7 +75,7 @@ export default class AdminUsersListShowController extends Controller.extend( const page = this._page; this.set("refreshing", true); - AdminUser.findAll(this.query, { + return AdminUser.findAll(this.query, { filter: this.listFilter, show_emails: this.showEmails, order: this.order, @@ -76,7 +84,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; @@ -87,6 +95,12 @@ export default class AdminUsersListShowController extends Controller.extend( }); } + @action + onListFilterChange(event) { + this.set("listFilter", event.target.value); + discourseDebounce(this, this.resetFilters, INPUT_DELAY); + } + @action loadMore() { this._page += 1; @@ -106,4 +120,92 @@ export default class AdminUsersListShowController extends Controller.extend( asc, }); } + + @action + toggleBulkSelect() { + this.bulkSelect = !this.bulkSelect; + this.displayBulkActions = false; + this.bulkSelectedUsersMap = {}; + this.bulkSelectedUserIdsSet = new Set(); + } + + @action + bulkSelectItemToggle(userId, event) { + if (event.target.checked) { + if (!this.#canBulkSelectMoreUsers(1)) { + this.#showBulkSelectionLimitToast(event); + return; + } + + if (event.shiftKey && this.lastSelected) { + const list = Array.from( + document.querySelectorAll( + "input.directory-table__cell-bulk-select:not([disabled])" + ) + ); + const lastSelectedIndex = list.indexOf(this.lastSelected); + if (lastSelectedIndex !== -1) { + const newSelectedIndex = list.indexOf(event.target); + const start = Math.min(lastSelectedIndex, newSelectedIndex); + const end = Math.max(lastSelectedIndex, newSelectedIndex); + + if (!this.#canBulkSelectMoreUsers(end - start)) { + this.#showBulkSelectionLimitToast(event); + return; + } + + list.slice(start, end).forEach((input) => { + input.checked = true; + this.#addUserToBulkSelection(parseInt(input.dataset.userId, 10)); + }); + } + } + this.#addUserToBulkSelection(userId); + this.lastSelected = event.target; + } else { + this.bulkSelectedUserIdsSet.delete(userId); + delete this.bulkSelectedUsersMap[userId]; + } + + this.displayBulkActions = this.bulkSelectedUserIdsSet.size > 0; + } + + @bind + async afterBulkDelete() { + await this.resetFilters(); + this.bulkSelectedUsersMap = {}; + this.bulkSelectedUserIdsSet = new Set(); + this.displayBulkActions = false; + } + + @action + openBulkDeleteConfirmation() { + this.modal.show(BulkUserDeleteConfirmation, { + model: { + userIds: Array.from(this.bulkSelectedUserIdsSet), + afterBulkDelete: this.afterBulkDelete, + }, + }); + } + + #addUserToBulkSelection(userId) { + this.bulkSelectedUserIdsSet.add(userId); + this.bulkSelectedUsersMap[userId] = 1; + } + + #canBulkSelectMoreUsers(count) { + return this.bulkSelectedUserIdsSet.size + count <= MAX_BULK_SELECT_LIMIT; + } + + #showBulkSelectionLimitToast(event) { + this.toasts.error({ + duration: 3000, + data: { + message: i18n("admin.users.bulk_actions.too_many_selected_users", { + count: MAX_BULK_SELECT_LIMIT, + }), + }, + }); + event.preventDefault(); + } } diff --git a/app/assets/javascripts/admin/addon/routes/admin-users-list-show.js b/app/assets/javascripts/admin/addon/routes/admin-users-list-show.js index e17c17ae4ac..81970884e28 100644 --- a/app/assets/javascripts/admin/addon/routes/admin-users-list-show.js +++ b/app/assets/javascripts/admin/addon/routes/admin-users-list-show.js @@ -24,6 +24,9 @@ export default class AdminUsersListShowRoute extends DiscourseRoute { listFilter: transition.to.queryParams.username, query: params.filter, refreshing: false, + bulkSelectedUsersMap: {}, + bulkSelectedUserIdsSet: new Set(), + displayBulkActions: false, }); controller.resetFilters(); diff --git a/app/assets/javascripts/admin/addon/templates/users-list-show.hbs b/app/assets/javascripts/admin/addon/templates/users-list-show.hbs index 2c692fca1b3..91e7d161d3f 100644 --- a/app/assets/javascripts/admin/addon/templates/users-list-show.hbs +++ b/app/assets/javascripts/admin/addon/templates/users-list-show.hbs @@ -19,19 +19,50 @@ {{/if}} -
- +
+
+ +
+ {{#if this.displayBulkActions}} +
+ + <:trigger> + + {{i18n "admin.users.bulk_actions.title"}} + + {{dIcon "angle-down"}} + + + <:content> + + + + + + + +
+ {{/if}}
- {{#if this.model}} + {{#if this.users}} <:header> - +
+ + +
<:body> - {{#each this.model as |user|}} + {{#each this.users as |user|}}
+ {{#if this.bulkSelect}} + {{#if user.can_be_deleted}} + + {{else}} + + <:trigger> + + + <:content> + {{#if user.admin}} + {{i18n + "admin.users.bulk_actions.admin_cant_be_deleted" + }} + {{else}} + {{i18n + "admin.users.bulk_actions.too_many_or_old_posts" + }} + {{/if}} + + + {{/if}} + {{/if}} - - - {{else}} + {{else if (not this.refreshing)}}

{{i18n "search.no_results"}}

{{/if}} + \ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/components/responsive-table.hbs b/app/assets/javascripts/discourse/app/components/responsive-table.hbs index 1cbd920dcf8..8cfc2185e42 100644 --- a/app/assets/javascripts/discourse/app/components/responsive-table.hbs +++ b/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}} > diff --git a/app/assets/javascripts/discourse/tests/acceptance/admin-users-list-test.js b/app/assets/javascripts/discourse/tests/acceptance/admin-users-list-test.js index d41cca1c6dd..0ea03ab3e02 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/admin-users-list-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/admin-users-list-test.js @@ -18,7 +18,7 @@ acceptance("Admin - Users List", function (needs) { test("searching users with no matches", async function (assert) { await visit("/admin/users/list/active"); - await fillIn(".controls.username input", "doesntexist"); + await fillIn(".admin-users-list__controls .username input", "doesntexist"); assert.dom(".users-list-container").hasText(i18n("search.no_results")); }); @@ -28,7 +28,9 @@ acceptance("Admin - Users List", function (needs) { assert.dom(".users-list .user").exists(); - await click(".users-list .sortable:nth-child(1)"); + await click( + ".users-list .directory-table__column-header--username.sortable" + ); assert.ok( query(".users-list .user:nth-child(1) .username").innerText.includes( @@ -37,7 +39,9 @@ acceptance("Admin - Users List", function (needs) { "list should be sorted by username" ); - await click(".users-list .sortable:nth-child(1)"); + await click( + ".users-list .directory-table__column-header--username.sortable" + ); assert.ok( query(".users-list .user:nth-child(1) .username").innerText.includes( diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index cf92963f4f9..bc4965c48db 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/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; @@ -1106,3 +1106,4 @@ a.inline-editable-field { @import "common/admin/mini_profiler"; @import "common/admin/schema_theme_setting_editor"; @import "common/admin/customize_themes_show_schema"; +@import "common/admin/admin_bulk_users_delete_modal"; diff --git a/app/assets/stylesheets/common/admin/admin_bulk_users_delete_modal.scss b/app/assets/stylesheets/common/admin/admin_bulk_users_delete_modal.scss new file mode 100644 index 00000000000..4789926f4ba --- /dev/null +++ b/app/assets/stylesheets/common/admin/admin_bulk_users_delete_modal.scss @@ -0,0 +1,23 @@ +.bulk-user-delete-confirmation { + &__progress { + font-family: var(--d-font-family--monospace); + max-height: 400px; + background: var(--blend-primary-secondary-5); + padding: 1em; + overflow-y: auto; + } + &__progress-line { + overflow-anchor: none; + + &.-success { + color: var(--success); + } + &.-error { + color: var(--danger); + } + } + &__progress-anchor { + overflow-anchor: auto; + height: 1px; + } +} diff --git a/app/assets/stylesheets/common/admin/users.scss b/app/assets/stylesheets/common/admin/users.scss index e824dbf9a9f..3e794859c42 100644 --- a/app/assets/stylesheets/common/admin/users.scss +++ b/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 diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index f291c36cee5..073b373451b 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -32,7 +32,7 @@ class Admin::UsersController < Admin::StaffController def index users = ::AdminUserIndexQuery.new(params).find_users - opts = {} + opts = { include_can_be_deleted: true } if params[:show_emails] == "true" StaffActionLogger.new(current_user).log_show_emails(users, context: request.path) opts[:emails_desired] = true @@ -402,6 +402,24 @@ 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 + + on_model_not_found(:users) { render json: failed_json, status: 404 } + end + end + end + def badges end diff --git a/app/serializers/admin_user_list_serializer.rb b/app/serializers/admin_user_list_serializer.rb index 4ce713fb145..c4de578e7d8 100644 --- a/app/serializers/admin_user_list_serializer.rb +++ b/app/serializers/admin_user_list_serializer.rb @@ -24,7 +24,8 @@ class AdminUserListSerializer < BasicUserSerializer :silenced_till, :time_read, :staged, - :second_factor_enabled + :second_factor_enabled, + :can_be_deleted %i[days_visited posts_read_count topics_entered post_count].each do |sym| attributes sym @@ -111,4 +112,12 @@ class AdminUserListSerializer < BasicUserSerializer def second_factor_enabled true end + + def can_be_deleted + scope.can_delete_user?(object) + end + + def include_can_be_deleted? + @options[:include_can_be_deleted] + end end diff --git a/app/serializers/admin_user_serializer.rb b/app/serializers/admin_user_serializer.rb index fc396fc4bdd..ea69fc872a8 100644 --- a/app/serializers/admin_user_serializer.rb +++ b/app/serializers/admin_user_serializer.rb @@ -39,4 +39,8 @@ class AdminUserSerializer < AdminUserListSerializer def registration_ip_address object.registration_ip_address.try(:to_s) end + + def include_can_be_deleted? + true + end end diff --git a/app/services/user/bulk_destroy.rb b/app/services/user/bulk_destroy.rb new file mode 100644 index 00000000000..cf1f119a546 --- /dev/null +++ b/app/services/user/bulk_destroy.rb @@ -0,0 +1,76 @@ +# 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:) + ids = params.user_ids + # this order cluase ensures we retrieve the users in the same order as the + # IDs in the param. we do this to ensure the users are deleted in the same + # order as they're selected in the UI + User.where(id: ids).order(DB.sql_fragment("array_position(ARRAY[?], users.id)", ids)) + end + + def can_delete_users(guardian:, users:) + users.all? { |u| guardian.can_delete_user?(u) } + end + + def delete(users:, guardian:) + users + .find_each + .with_index(1) do |user, position| + success = + UserDestroyer.new(guardian.user).destroy( + user, + delete_posts: true, + prepare_for_destroy: true, + context: I18n.t("staff_action_logs.bulk_user_delete"), + ) + + if success + publish_progress( + guardian.user, + { position:, username: user.username, total: users.size, success: true }, + ) + else + publish_progress( + guardian.user, + { + position:, + username: user.username, + total: users.size, + failed: true, + error: user.errors.full_messages.join(", "), + }, + ) + end + rescue => err + publish_progress( + guardian.user, + { + position:, + username: user.username, + total: users.size, + failed: true, + error: err.message, + }, + ) + end + end + + def publish_progress(actor, data) + ::MessageBus.publish("/bulk-user-delete", data, user_ids: [actor.id]) + end +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b76fece785b..a562a74bfd9 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -6649,6 +6649,32 @@ en: status: "Status" show_emails: "Show Emails" hide_emails: "Hide Emails" + bulk_actions: + title: "Bulk actions" + admin_cant_be_deleted: "This user can't be deleted because they're an admin" + too_many_or_old_posts: "This user can't be deleted they have too many posts or a very old post" + too_many_selected_users: + one: "You've reached the %{count} user limit for bulk deletion" + other: "You've reached the %{count} users limit for bulk deletion" + delete: + label: "Delete users…" + confirmation_modal: + prompt_text: + one: 'You''re about to delete %{count} user permanently. Type "%{confirmation_phrase}" below to proceed:' + other: 'You''re about to delete %{count} users permanently. Type "%{confirmation_phrase}" below to proceed:' + confirmation_phrase: + one: "delete %{count} user" + other: "delete %{count} users" + close: "Close" + confirm: "Delete" + title: + one: "Delete %{count} user" + other: "Delete %{count} users" + bulk_delete_starting: "Starting bulk delete…" + user_delete_succeeded: "[%{position}/%{total}] Successfully deleted @%{username}" + user_delete_failed: "[%{position}/%{total}] Failed to delete @%{username} - %{error}" + bulk_delete_finished: "Bulk delete operation completed." + failed_to_delete_users: "The following users failed to be deleted:" nav: new: "New" active: "Active" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index a5806b39000..67c54e4e01f 100644 --- a/config/locales/server.en.yml +++ b/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: "deleted in a bulk delete operation" reviewables: already_handled: "Thanks, but we've already reviewed that post and determined it does not need to be flagged again." diff --git a/config/routes.rb b/config/routes.rb index df71ef73285..a8bfcf266bc 100644 --- a/config/routes.rb +++ b/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" diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 6c643fa7171..6803524db48 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -1435,6 +1435,73 @@ 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 "responds with 404 when sending an empty user_ids list" do + delete "/admin/users/destroy-bulk.json", params: { user_ids: [] } + + expect(response.status).to eq(404) + 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) } diff --git a/spec/serializers/admin_user_list_serializer_spec.rb b/spec/serializers/admin_user_list_serializer_spec.rb index a22c5612cb6..9b9b96823cc 100644 --- a/spec/serializers/admin_user_list_serializer_spec.rb +++ b/spec/serializers/admin_user_list_serializer_spec.rb @@ -89,4 +89,24 @@ RSpec.describe AdminUserListSerializer do expect(json[:secondary_emails]).to contain_exactly("first@email.com", "second@email.com") end end + + describe "#can_be_deleted" do + it "is not included if the include_can_be_deleted option is not present" do + json = AdminUserListSerializer.new(user, scope: guardian, root: false).as_json + + expect(json.key?(:can_be_deleted)).to eq(false) + end + + it "is included if the include_can_be_deleted option is true" do + json = + AdminUserListSerializer.new( + user, + scope: guardian, + root: false, + include_can_be_deleted: true, + ).as_json + + expect(json[:can_be_deleted]).to eq(true) + end + end end diff --git a/spec/system/admin_users_list_spec.rb b/spec/system/admin_users_list_spec.rb new file mode 100644 index 00000000000..e92634a597a --- /dev/null +++ b/spec/system/admin_users_list_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +describe "Admin Users Page", type: :system do + fab!(:current_user) { Fabricate(:admin) } + fab!(:another_admin) { Fabricate(:admin) } + fab!(:users) { Fabricate.times(3, :user) } + + let(:admin_users_page) { PageObjects::Pages::AdminUsers.new } + + before { sign_in(current_user) } + + describe "bulk user delete" do + let(:confirmation_modal) { PageObjects::Modals::BulkUserDeleteConfirmation.new } + + it "disables checkboxes for users that can't be deleted" do + admin_users_page.visit + + admin_users_page.bulk_select_button.click + + expect(admin_users_page.user_row(current_user.id).bulk_select_checkbox.disabled?).to eq(true) + expect(admin_users_page.user_row(another_admin.id).bulk_select_checkbox.disabled?).to eq(true) + expect(admin_users_page.user_row(users[0].id).bulk_select_checkbox.disabled?).to eq(false) + + admin_users_page.user_row(another_admin.id).bulk_select_checkbox.hover + expect(PageObjects::Components::Tooltips.new("bulk-delete-unavailable-reason")).to be_present( + text: I18n.t("admin_js.admin.users.bulk_actions.admin_cant_be_deleted"), + ) + end + + 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(confirmation_modal).to be_open + expect(confirmation_modal).to have_confirm_button_disabled + + confirmation_modal.fill_in_confirmation_phase(user_count: 3) + expect(confirmation_modal).to have_confirm_button_disabled + + confirmation_modal.fill_in_confirmation_phase(user_count: 2) + expect(confirmation_modal).to have_confirm_button_enabled + + confirmation_modal.confirm_button.click + + expect(confirmation_modal).to have_successful_log_entry_for_user( + user: users[0], + position: 1, + total: 2, + ) + expect(confirmation_modal).to have_successful_log_entry_for_user( + user: users[1], + position: 2, + total: 2, + ) + expect(confirmation_modal).to have_no_error_log_entries + + confirmation_modal.close + 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(confirmation_modal).to be_open + confirmation_modal.fill_in_confirmation_phase(user_count: 2) + confirmation_modal.confirm_button.click + expect(confirmation_modal).to have_successful_log_entry_for_user( + user: users[0], + position: 1, + total: 2, + ) + expect(confirmation_modal).to have_successful_log_entry_for_user( + user: users[1], + position: 2, + total: 2, + ) + confirmation_modal.close + + 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 "displays an error message if bulk delete fails" do + admin_users_page.visit + admin_users_page.bulk_select_button.click + + admin_users_page.user_row(users[0].id).bulk_select_checkbox.click + admin_users_page.bulk_actions_dropdown.expand + admin_users_page.bulk_actions_dropdown.option(".bulk-delete").click + confirmation_modal.fill_in_confirmation_phase(user_count: 1) + + users[0].update!(admin: true) + + confirmation_modal.confirm_button.click + expect(confirmation_modal).to have_error_log_entry( + I18n.t("js.generic_error_with_reason", error: I18n.t("user.cannot_bulk_delete")), + ) + confirmation_modal.close + expect(admin_users_page).to have_users([users[0].id]) + end + end +end diff --git a/spec/system/page_objects/components/d_menu.rb b/spec/system/page_objects/components/d_menu.rb new file mode 100644 index 00000000000..4d90131d0ff --- /dev/null +++ b/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 diff --git a/spec/system/page_objects/modals/bulk_user_delete_confirmation.rb b/spec/system/page_objects/modals/bulk_user_delete_confirmation.rb new file mode 100644 index 00000000000..f70b93b1a3f --- /dev/null +++ b/spec/system/page_objects/modals/bulk_user_delete_confirmation.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module PageObjects + module Modals + class BulkUserDeleteConfirmation < Base + MODAL_SELECTOR = ".bulk-user-delete-confirmation" + + def confirm_button + within(modal) { find(".btn.confirm-delete") } + end + + def has_confirm_button_disabled? + within(modal) { has_css?(".btn.confirm-delete[disabled]") } + end + + def has_confirm_button_enabled? + within(modal) do + has_no_css?(".btn.confirm-delete[disabled]") && has_css?(".btn.confirm-delete") + end + end + + def fill_in_confirmation_phase(user_count:) + within(modal) do + find("input.confirmation-phrase").fill_in( + with: + I18n.t( + "admin_js.admin.users.bulk_actions.delete.confirmation_modal.confirmation_phrase", + count: user_count, + ), + ) + end + end + + def has_successful_log_entry_for_user?(user:, position:, total:) + within(modal) do + has_css?( + ".bulk-user-delete-confirmation__progress-line.-success", + text: + I18n.t( + "admin_js.admin.users.bulk_actions.delete.confirmation_modal.user_delete_succeeded", + position:, + total:, + username: user.username, + ), + ) + end + end + + def has_no_error_log_entries? + within(modal) { has_no_css?(".bulk-user-delete-confirmation__progress-line.-error") } + end + + def has_error_log_entry?(message) + within(modal) do + has_css?(".bulk-user-delete-confirmation__progress-line.-error", text: message) + end + end + + private + + def modal + find(MODAL_SELECTOR) + end + end + end +end diff --git a/spec/system/page_objects/pages/admin_users.rb b/spec/system/page_objects/pages/admin_users.rb new file mode 100644 index 00000000000..9603d2dc5c9 --- /dev/null +++ b/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