From 78022e7a5f0808e4c1febdc500daa635a296347d Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Mon, 15 May 2023 20:45:26 +0200 Subject: [PATCH] FEATURE: Show user cards for inactive users (#21387) It used to return 404 which made the user card render and then quickly disappear. --- .../app/components/user-card-contents.hbs | 12 +++- .../app/components/user-card-contents.js | 5 ++ .../tests/acceptance/user-card-test.js | 66 +++++++++++++++++++ .../common/components/user-card.scss | 3 +- app/controllers/users_controller.rb | 11 ++-- app/serializers/inactive_user_serializer.rb | 9 +++ config/locales/client.en.yml | 1 + spec/requests/users_controller_spec.rb | 22 +++++-- 8 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 app/serializers/inactive_user_serializer.rb diff --git a/app/assets/javascripts/discourse/app/components/user-card-contents.hbs b/app/assets/javascripts/discourse/app/components/user-card-contents.hbs index 159224f9537..dda453250c2 100644 --- a/app/assets/javascripts/discourse/app/components/user-card-contents.hbs +++ b/app/assets/javascripts/discourse/app/components/user-card-contents.hbs @@ -28,7 +28,7 @@ {{else}}
- {{#if this.user.profile_hidden}} + {{#if this.contentHidden}} {{bound-avatar this.user "huge" @@ -57,7 +57,7 @@ {{this.newUser}} {{if this.nameFirst 'full-name' 'username'}}" > - {{#if this.user.profile_hidden}} + {{#if this.contentHidden}} {{i18n "user.profile_hidden"}}
+ {{else if this.user.inactive}} +
+
+ {{i18n "user.inactive_user"}} +
+
{{/if}} {{#if this.isSuspendedOrHasBio}} @@ -292,7 +298,7 @@ {{/if}}
- {{#unless this.user.profile_hidden}} + {{#unless this.contentHidden}}
{{#if this.user.last_posted_at}}

{{i18n "last_post"}} diff --git a/app/assets/javascripts/discourse/app/components/user-card-contents.js b/app/assets/javascripts/discourse/app/components/user-card-contents.js index 884fcf35d37..72a4926f1a8 100644 --- a/app/assets/javascripts/discourse/app/components/user-card-contents.js +++ b/app/assets/javascripts/discourse/app/components/user-card-contents.js @@ -178,6 +178,11 @@ export default Component.extend(CardContentsBase, CanCheckEmails, CleansUp, { return `group-${primaryGroup}`; }, + @discourseComputed("user.profile_hidden", "user.inactive") + contentHidden(profileHidden, inactive) { + return profileHidden || inactive; + }, + _showCallback(username, $target) { this._positionCard($target); this.setProperties({ visible: true, loading: true }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-card-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-card-test.js index d7c08f30b00..c912ff97467 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-card-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-card-test.js @@ -1,3 +1,4 @@ +import I18n from "I18n"; import { acceptance, exists, @@ -98,3 +99,68 @@ acceptance("User Card - User Status", function (needs) { assert.notOk(exists(".user-card h3.user-status")); }); }); + +acceptance("User Card - Hidden Profile", function (needs) { + needs.user(); + needs.pretender((server, helper) => { + server.get("/u/eviltrout/card.json", () => + helper.response({ + user: { + id: 6, + username: "eviltrout", + name: null, + avatar_template: "/letter_avatar_proxy/v4/letter/f/8edcca/{size}.png", + profile_hidden: true, + title: null, + primary_group_name: null, + }, + }) + ); + }); + + test("it shows less information", async function (assert) { + await visit("/t/this-is-a-test-topic/9"); + await click('a[data-user-card="eviltrout"]'); + + assert.equal( + query(".user-card .name-username-wrapper").innerText, + "eviltrout" + ); + assert.equal( + query(".user-card .profile-hidden").innerText, + I18n.t("user.profile_hidden") + ); + }); +}); + +acceptance("User Card - Inactive user", function (needs) { + needs.user(); + needs.pretender((server, helper) => { + server.get("/u/eviltrout/card.json", () => + helper.response({ + user: { + id: 6, + username: "eviltrout", + name: null, + avatar_template: "/letter_avatar_proxy/v4/letter/f/8edcca/{size}.png", + inactive: true, + }, + }) + ); + }); + + test("it shows less information", async function (assert) { + await visit("/t/this-is-a-test-topic/9"); + await click('a[data-user-card="eviltrout"]'); + + assert.equal( + query(".user-card .name-username-wrapper").innerText, + "eviltrout" + ); + + assert.equal( + query(".user-card .inactive-user").innerText, + I18n.t("user.inactive_user") + ); + }); +}); diff --git a/app/assets/stylesheets/common/components/user-card.scss b/app/assets/stylesheets/common/components/user-card.scss index b0224c7cedb..495d12d5959 100644 --- a/app/assets/stylesheets/common/components/user-card.scss +++ b/app/assets/stylesheets/common/components/user-card.scss @@ -171,7 +171,8 @@ $avatar_margin: -50px; // negative margin makes avatars extend above cards font-weight: bold; } } - .profile-hidden { + .profile-hidden, + .inactive-user { font-size: var(--font-up-1); margin-top: 0.5em; } diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f24aaaefa94..c52ee650247 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -110,12 +110,15 @@ class UsersController < ApplicationController @user = fetch_user_from_params( - include_inactive: - current_user.try(:staff?) || (current_user && SiteSetting.show_inactive_accounts), + include_inactive: current_user&.staff? || for_card || SiteSetting.show_inactive_accounts, ) user_serializer = nil - if guardian.can_see_profile?(@user) + if !current_user&.staff? && !@user.active? + user_serializer = InactiveUserSerializer.new(@user, scope: guardian, root: "user") + elsif !guardian.can_see_profile?(@user) + user_serializer = HiddenProfileSerializer.new(@user, scope: guardian, root: "user") + else serializer_class = for_card ? UserCardSerializer : UserSerializer user_serializer = serializer_class.new(@user, scope: guardian, root: "user") @@ -125,8 +128,6 @@ class UsersController < ApplicationController topic_id => Post.secured(guardian).where(topic_id: topic_id, user_id: @user.id).count, } end - else - user_serializer = HiddenProfileSerializer.new(@user, scope: guardian, root: "user") end track_visit_to_user_profile if !params[:skip_track_visit] && (@user != current_user) diff --git a/app/serializers/inactive_user_serializer.rb b/app/serializers/inactive_user_serializer.rb new file mode 100644 index 00000000000..f76a02e6cf7 --- /dev/null +++ b/app/serializers/inactive_user_serializer.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class InactiveUserSerializer < BasicUserSerializer + attributes :inactive + + def inactive + !object.active? + end +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index a108f5f787b..352dabc631c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1124,6 +1124,7 @@ en: warning: "Are you sure you want to clear your featured topic?" use_current_timezone: "Use Current Timezone" profile_hidden: "This user's public profile is hidden." + inactive_user: "This user is no longer active." expand_profile: "Expand" sr_expand_profile: "Expand profile details" collapse_profile: "Collapse" diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 6af48bec505..6e4d17be872 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -4592,13 +4592,13 @@ RSpec.describe UsersController do before { sign_in(user1) } it "works correctly" do - get "/u/#{user1.username}/card.json" + get "/u/#{user.username}/card.json" expect(response.status).to eq(200) json = response.parsed_body expect(json["user"]["associated_accounts"]).to eq(nil) # Not serialized in card - expect(json["user"]["username"]).to eq(user1.username) + expect(json["user"]["username"]).to eq(user.username) end it "returns not found when the username doesn't exist" do @@ -4606,9 +4606,23 @@ RSpec.describe UsersController do expect(response).not_to be_successful end + it "returns partial response when inactive user" do + user.update!(active: false) + get "/u/#{user.username}/card.json" + expect(response).to be_successful + expect(response.parsed_body["user"]["inactive"]).to eq(true) + end + + it "returns partial response when hidden users" do + user.user_option.update!(hide_profile_and_presence: true) + get "/u/#{user.username}/card.json" + expect(response).to be_successful + expect(response.parsed_body["user"]["profile_hidden"]).to eq(true) + end + it "raises an error on invalid access" do - Guardian.any_instance.expects(:can_see?).with(user1).returns(false) - get "/u/#{user1.username}/card.json" + Guardian.any_instance.expects(:can_see?).with(user).returns(false) + get "/u/#{user.username}/card.json" expect(response).to be_forbidden end end