diff --git a/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.hbs b/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.hbs index 10c485bc4d2..134412666a7 100644 --- a/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.hbs +++ b/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.hbs @@ -3,42 +3,44 @@ {{html-safe (i18n "admin.user.other_matches" - (hash count=this.user.similar_users_count username=this.user.username) + (hash count=@user.similar_users_count username=@user.username) ) }}

- - - - - - - - - - - - - - - {{#each this.user.similar_users as |user|}} + +
{{i18n "username"}}{{i18n "last_seen"}}{{i18n "admin.user.topics_entered"}}{{i18n "admin.user.posts_read_count"}}{{i18n "admin.user.time_read"}}{{i18n "created"}}
+ - - - - - - - + + + + + + + - {{/each}} - -
- - {{avatar user imageSize="small"}} {{user.username}}{{format-duration user.last_seen_age}}{{number user.topics_entered}}{{number user.posts_read_count}}{{format-duration user.time_read}}{{format-duration user.created_at_age}}{{i18n "username"}}{{i18n "last_seen"}}{{i18n "admin.user.topics_entered"}}{{i18n "admin.user.posts_read_count"}}{{i18n "admin.user.time_read"}}{{i18n "created"}}
+ + + + {{#each this.similarUsers as |user|}} + + + + + {{avatar user imageSize="small"}} {{user.username}} + {{format-duration user.last_seen_age}} + {{number user.topics_entered}} + {{number user.posts_read_count}} + {{format-duration user.time_read}} + {{format-duration user.created_at_age}} + + {{/each}} + + + \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.js b/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.js index 4531b83d9d8..adaa615cf0f 100644 --- a/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.js +++ b/app/assets/javascripts/admin/addon/components/admin-penalty-similar-users.js @@ -1,12 +1,21 @@ -import Component from "@ember/component"; +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; import { action } from "@ember/object"; -import { tagName } from "@ember-decorators/component"; -import discourseComputed from "discourse-common/utils/decorators"; +import { ajax } from "discourse/lib/ajax"; -@tagName("") export default class AdminPenaltySimilarUsers extends Component { - @discourseComputed("penaltyType") - penaltyField(penaltyType) { + @tracked isLoading; + @tracked similarUsers = []; + selectedUserIds = []; + + constructor() { + super(...arguments); + + this.loadSimilarUsers(); + } + + get penaltyField() { + const penaltyType = this.args.penaltyType; if (penaltyType === "suspend") { return "can_be_suspended"; } else if (penaltyType === "silence") { @@ -14,16 +23,26 @@ export default class AdminPenaltySimilarUsers extends Component { } } - @action - selectUserId(userId, event) { - if (!this.selectedUserIds) { - return; - } - - if (event.target.checked) { - this.selectedUserIds.pushObject(userId); - } else { - this.selectedUserIds.removeObject(userId); + async loadSimilarUsers() { + this.isLoading = true; + try { + const data = await ajax( + `/admin/users/${this.args.user.id}/similar-users.json` + ); + this.similarUsers = data.users; + } finally { + this.isLoading = false; } } + + @action + selectUserId(userId, event) { + if (event.target.checked) { + this.selectedUserIds.push(userId); + } else { + this.selectedUserIds = this.selectedUserIds.filter((id) => id !== userId); + } + + this.args.onUsersChanged(this.selectedUserIds); + } } diff --git a/app/assets/javascripts/admin/addon/components/modal/penalize-user.hbs b/app/assets/javascripts/admin/addon/components/modal/penalize-user.hbs index 783a20e610b..8372f207098 100644 --- a/app/assets/javascripts/admin/addon/components/modal/penalize-user.hbs +++ b/app/assets/javascripts/admin/addon/components/modal/penalize-user.hbs @@ -46,11 +46,12 @@ @postEdit={{this.postEdit}} /> {{/if}} - {{#if this.user.similar_users}} + {{#if @model.user.similar_users_count}} {{/if}} {{else}} @@ -60,9 +61,9 @@
{{i18n "admin.user.cant_silence"}}
{{/if}} {{/if}} -
{{html-safe this.penaltyHistory}}
<:footer> +
{{html-safe this.penaltyHistory}}
{{i18n "admin.user.silence_explanation"}} {{else}} - - {{i18n "admin.user.silence_explanation"}} + {{#if this.model.canSilence}} + + {{i18n "admin.user.silence_explanation"}} + {{/if}} {{/if}} diff --git a/app/assets/stylesheets/common/admin/penalty.scss b/app/assets/stylesheets/common/admin/penalty.scss index 325f59dcce5..3e5fa827b9e 100644 --- a/app/assets/stylesheets/common/admin/penalty.scss +++ b/app/assets/stylesheets/common/admin/penalty.scss @@ -91,7 +91,7 @@ position: sticky; bottom: 0; background-color: var(--secondary); - padding: 0.5em 0 1em 0; + padding: 0 0 1em 0; } .penalty-history::before { position: absolute; diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index a8e49aaee86..9be0bc0ba26 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -46,18 +46,28 @@ class Admin::UsersController < Admin::StaffController @user = User.find_by(id: params[:id]) raise Discourse::NotFound unless @user - similar_users = - User - .real - .where.not(id: @user.id) - .where(ip_address: @user.ip_address, admin: false, moderator: false) - render_serialized( @user, AdminDetailedUserSerializer, root: false, - similar_users: similar_users.limit(MAX_SIMILAR_USERS), - similar_users_count: similar_users.count, + similar_users_count: @user.similar_users.count, + ) + end + + def similar_users + @user = User.find_by(id: params[:user_id]) + raise Discourse::NotFound if !@user + + render_json_dump( + { + users: + ActiveModel::ArraySerializer.new( + @user.similar_users.limit(MAX_SIMILAR_USERS), + each_serializer: SimilarAdminUserSerializer, + scope: guardian, + root: false, + ), + }, ) end diff --git a/app/models/user.rb b/app/models/user.rb index cf9dc0009ae..520713200d4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1884,6 +1884,13 @@ class User < ActiveRecord::Base update(required_fields_version: UserRequiredFieldsVersion.current) end + def similar_users + User + .real + .where.not(id: self.id) + .where(ip_address: self.ip_address, admin: false, moderator: false) + end + protected def badge_grant diff --git a/app/serializers/admin_detailed_user_serializer.rb b/app/serializers/admin_detailed_user_serializer.rb index e82bf8ac346..a47d3ab66ec 100644 --- a/app/serializers/admin_detailed_user_serializer.rb +++ b/app/serializers/admin_detailed_user_serializer.rb @@ -36,7 +36,6 @@ class AdminDetailedUserSerializer < AdminUserSerializer :can_delete_sso_record, :api_key_count, :external_ids, - :similar_users, :similar_users_count has_one :approved_by, serializer: BasicUserSerializer, embed: :objects @@ -157,25 +156,12 @@ class AdminDetailedUserSerializer < AdminUserSerializer external_ids end - def similar_users - ActiveModel::ArraySerializer.new( - @options[:similar_users], - each_serializer: SimilarAdminUserSerializer, - scope: scope, - root: false, - ).as_json - end - - def include_similar_users? - @options[:similar_users].present? - end - def similar_users_count @options[:similar_users_count] end def include_similar_users_count? - @options[:similar_users].present? + @options[:similar_users_count].present? end def can_delete_sso_record diff --git a/config/routes.rb b/config/routes.rb index 7b0c25b3049..64fbcb0ba24 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -161,6 +161,7 @@ Discourse::Application.routes.draw do post "reset-bounce-score" put "disable_second_factor" delete "sso_record" + get "similar-users.json" => "users#similar_users" end get "users/:id.json" => "users#show", :defaults => { format: "json" } get "users/:id/:username" => "users#show", diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index ea73e76036f..005f4a61140 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -109,7 +109,7 @@ RSpec.describe Admin::UsersController do expect(response.parsed_body["id"]).to eq(user.id) end - it "includes similar users who aren't admin or mods" do + it "includes count of similiar users" do Fabricate(:user, ip_address: "88.88.88.88") Fabricate(:admin, ip_address: user.ip_address) Fabricate(:moderator, ip_address: user.ip_address) @@ -118,11 +118,7 @@ RSpec.describe Admin::UsersController do get "/admin/users/#{user.id}.json" expect(response.status).to eq(200) - expect(response.parsed_body["id"]).to eq(user.id) expect(response.parsed_body["similar_users_count"]).to eq(1) - expect(response.parsed_body["similar_users"].map { |u| u["id"] }).to contain_exactly( - similar_user.id, - ) end end @@ -138,6 +134,22 @@ RSpec.describe Admin::UsersController do end end + describe "#similar_users" do + before { sign_in(admin) } + + it "includes similar users who aren't admin or mods" do + Fabricate(:user, ip_address: "88.88.88.88") + Fabricate(:admin, ip_address: user.ip_address) + Fabricate(:moderator, ip_address: user.ip_address) + similar_user = Fabricate(:user, ip_address: user.ip_address) + + get "/admin/users/#{user.id}/similar-users.json" + + expect(response.status).to eq(200) + expect(response.parsed_body["users"].map { |u| u["id"] }).to contain_exactly(similar_user.id) + end + end + describe "#approve" do let(:evil_trout) { Fabricate(:evil_trout) } diff --git a/spec/requests/api/schemas/json/admin_user_response.json b/spec/requests/api/schemas/json/admin_user_response.json index 6b6d902c1fa..9bd90d2100c 100644 --- a/spec/requests/api/schemas/json/admin_user_response.json +++ b/spec/requests/api/schemas/json/admin_user_response.json @@ -170,6 +170,9 @@ "api_key_count": { "type": "integer" }, + "similar_users_count": { + "type": "integer" + }, "single_sign_on_record": { "type": ["string", "null"] }, diff --git a/spec/system/admin_user_spec.rb b/spec/system/admin_user_spec.rb new file mode 100644 index 00000000000..c24530b44f8 --- /dev/null +++ b/spec/system/admin_user_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +describe "Admin User Page", type: :system do + fab!(:current_user) { Fabricate(:admin) } + + let(:admin_user_page) { PageObjects::Pages::AdminUser.new } + let(:suspend_user_modal) { PageObjects::Modals::PenalizeUser.new("suspend") } + let(:silence_user_modal) { PageObjects::Modals::PenalizeUser.new("silence") } + + before { sign_in(current_user) } + + context "when visiting an admin's page" do + fab!(:admin) + + before { admin_user_page.visit(admin) } + + it "doesn't display the suspend or silence buttons" do + expect(admin_user_page).to have_no_suspend_button + expect(admin_user_page).to have_no_silence_button + end + end + + context "when visiting a moderator's page" do + fab!(:moderator) + + before { admin_user_page.visit(moderator) } + + it "doesn't display the suspend or silence buttons" do + expect(admin_user_page).to have_no_suspend_button + expect(admin_user_page).to have_no_silence_button + end + end + + context "when visting a regular user's page" do + fab!(:user) { Fabricate(:user, ip_address: "93.123.44.90") } + fab!(:similar_user) { Fabricate(:user, ip_address: user.ip_address) } + fab!(:another_mod) { Fabricate(:moderator, ip_address: user.ip_address) } + fab!(:another_admin) { Fabricate(:admin, ip_address: user.ip_address) } + + before { admin_user_page.visit(user) } + + it "displays the suspend and silence buttons" do + expect(admin_user_page).to have_suspend_button + expect(admin_user_page).to have_silence_button + end + + describe "the suspend user modal" do + it "displays the list of users who share the same IP but are not mods or admins" do + admin_user_page.click_suspend_button + + expect(suspend_user_modal.similar_users).to contain_exactly(similar_user.username) + expect(admin_user_page.similar_users_warning).to include( + I18n.t("admin_js.admin.user.other_matches", count: 1, username: user.username), + ) + end + end + + describe "the silence user modal" do + it "displays the list of users who share the same IP but are not mods or admins" do + admin_user_page.click_silence_button + + expect(silence_user_modal.similar_users).to contain_exactly(similar_user.username) + expect(admin_user_page.similar_users_warning).to include( + I18n.t("admin_js.admin.user.other_matches", count: 1, username: user.username), + ) + end + end + end +end diff --git a/spec/system/page_objects/modals/penalize_user.rb b/spec/system/page_objects/modals/penalize_user.rb new file mode 100644 index 00000000000..5ae2fb39ae2 --- /dev/null +++ b/spec/system/page_objects/modals/penalize_user.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module PageObjects + module Modals + class PenalizeUser < PageObjects::Modals::Base + def initialize(penalty_type) + @penalty_type = penalty_type + end + + def similar_users + modal.all("table tbody tr td:nth-child(2)").map(&:text) + end + + def modal + find(".d-modal.#{@penalty_type}-user-modal") + end + end + end +end diff --git a/spec/system/page_objects/pages/admin_user.rb b/spec/system/page_objects/pages/admin_user.rb new file mode 100644 index 00000000000..5e7dc54408f --- /dev/null +++ b/spec/system/page_objects/pages/admin_user.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module PageObjects + module Pages + class AdminUser < PageObjects::Pages::Base + def visit(user) + page.visit("/admin/users/#{user.id}/#{user.username}") + end + + def has_suspend_button? + has_css?(".btn-danger.suspend-user") + end + + def has_no_suspend_button? + has_no_css?(".btn-danger.suspend-user") + end + + def has_silence_button? + has_css?(".btn-danger.silence-user") + end + + def has_no_silence_button? + has_no_css?(".btn-danger.silence-user") + end + + def click_suspend_button + find(".btn-danger.suspend-user").click + end + + def click_silence_button + find(".btn-danger.silence-user").click + end + + def similar_users_warning + find(".penalty-similar-users .alert-warning")["innerHTML"] + end + end + end +end