From 65649eaef08857ae3400cf66446386d7ed373bd2 Mon Sep 17 00:00:00 2001 From: jahan-ggn Date: Mon, 17 Aug 2020 22:07:45 +0530 Subject: [PATCH] User card settings (#10302) * settings implemented * prettier * settings updated * rubocop * prettier * Revert "rubocop" This reverts commit 7805145a7dc1dbccd9a4378bec3626a81ebaa659. * Revert "prettier" This reverts commit 2c53f4fa127a00ee9b3c3ab5b1182036089c286b. * settings updated and changed * rubocop * changes applied * final changes done * Server side feature added * spec changed * changed user_updater and profile file * Fix user card specs * web hook serializer solved * site-setting changed Co-authored-by: Mark VanLandingham --- .../app/controllers/preferences/profile.js | 23 ++++---- .../app/templates/preferences/profile.hbs | 43 ++++++++------- app/serializers/user_serializer.rb | 12 ++++- app/services/user_updater.rb | 4 +- config/locales/server.en.yml | 3 +- config/site_settings.yml | 8 +++ lib/guardian/user_guardian.rb | 9 ++++ .../components/guardian/user_guardian_spec.rb | 42 +++++++++++++++ .../web_hook_user_serializer_spec.rb | 2 +- spec/services/user_updater_spec.rb | 53 +++++++++++-------- 10 files changed, 138 insertions(+), 61 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/profile.js b/app/assets/javascripts/discourse/app/controllers/preferences/profile.js index 88ec0ee4adb..54941de50e8 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/profile.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/profile.js @@ -7,11 +7,11 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; import { cookAsync } from "discourse/lib/text"; import { ajax } from "discourse/lib/ajax"; import showModal from "discourse/lib/show-modal"; +import { readOnly } from "@ember/object/computed"; export default Controller.extend({ init() { this._super(...arguments); - this.saveAttrNames = [ "bio_raw", "website", @@ -44,20 +44,17 @@ export default Controller.extend({ } }, - @discourseComputed("model.can_change_bio") - canChangeBio(canChangeBio) { - return canChangeBio; - }, + canChangeBio: readOnly("model.can_change_bio"), - @discourseComputed("model.can_change_location") - canChangeLocation(canChangeLocation) { - return canChangeLocation; - }, + canChangeLocation: readOnly("model.can_change_location"), - @discourseComputed("model.can_change_website") - canChangeWebsite(canChangeWebsite) { - return canChangeWebsite; - }, + canChangeWebsite: readOnly("model.can_change_website"), + + canUploadProfileHeader: readOnly("model.can_upload_profile_header"), + + canUploadUserCardBackground: readOnly( + "model.can_upload_user_card_background" + ), actions: { showFeaturedTopicModal() { diff --git a/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs b/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs index fb3d780b0e9..66e2a81afe8 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs @@ -43,27 +43,30 @@
{{#if siteSettings.allow_profile_backgrounds}} -
- -
- {{image-uploader imageUrl=model.profile_background_upload_url - type="profile_background"}} + {{#if canUploadProfileHeader}} +
+ +
+ {{image-uploader imageUrl=model.profile_background_upload_url + type="profile_background"}} +
+
+ {{i18n "user.change_profile_background.instructions"}} +
-
- {{i18n "user.change_profile_background.instructions"}} + {{/if}} + {{#if canUploadUserCardBackground}} +
+ +
+ {{image-uploader imageUrl=model.card_background_upload_url + type="card_background"}} +
+
+ {{i18n "user.change_card_background.instructions"}} +
-
- -
- -
- {{image-uploader imageUrl=model.card_background_upload_url - type="card_background"}} -
-
- {{i18n "user.change_card_background.instructions"}} -
-
+ {{/if}} {{/if}} {{#if siteSettings.allow_featured_topic_on_user_profiles}} @@ -101,4 +104,4 @@ {{plugin-outlet name="user-custom-controls" args=(hash model=model)}} -{{save-controls model=model action=(action "save") saved=saved}} +{{save-controls model=model action=(action "save") saved=saved}} \ No newline at end of file diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index f78e9f8ac12..0192d19ceaa 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -16,7 +16,9 @@ class UserSerializer < UserCardSerializer :second_factor_backup_enabled, :second_factor_remaining_backup_codes, :associated_accounts, - :profile_background_upload_url + :profile_background_upload_url, + :can_upload_profile_header, + :can_upload_user_card_background has_one :invited_by, embed: :object, serializer: BasicUserSerializer has_many :groups, embed: :object, serializer: BasicGroupSerializer @@ -170,6 +172,14 @@ class UserSerializer < UserCardSerializer scope.can_edit_name?(object) end + def can_upload_profile_header + scope.can_upload_profile_header?(object) + end + + def can_upload_user_card_background + scope.can_upload_user_card_background?(object) + end + ### ### STAFF ATTRIBUTES ### diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index 8c0149b121e..50e2e007612 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -68,13 +68,13 @@ class UserUpdater user_profile.website = format_url(attributes.fetch(:website) { user_profile.website }) end - if attributes[:profile_background_upload_url] == "" + if attributes[:profile_background_upload_url] == "" || !guardian.can_upload_profile_header?(user) user_profile.profile_background_upload_id = nil elsif upload = Upload.get_from_url(attributes[:profile_background_upload_url]) user_profile.profile_background_upload_id = upload.id end - if attributes[:card_background_upload_url] == "" + if attributes[:card_background_upload_url] == "" || !guardian.can_upload_user_card_background?(user) user_profile.card_background_upload_id = nil elsif upload = Upload.get_from_url(attributes[:card_background_upload_url]) user_profile.card_background_upload_id = upload.id diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ee30f95221f..e2f69d37f42 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1804,7 +1804,8 @@ en: min_trust_to_flag_posts: "The minimum trust level required to flag posts" min_trust_to_post_links: "The minimum trust level required to include links in posts" min_trust_to_post_embedded_media: "The minimum trust level required to embed media items in a post" - + min_trust_level_to_allow_profile_background: "The minimum trust level required to upload a profile background" + min_trust_level_to_allow_user_card_background: "The minimum trust level required to upload a user card background" allowed_link_domains: "Domains that users may link to even if they don't have the appropriate trust level to post links" newuser_max_links: "How many links a new user can add to a post." diff --git a/config/site_settings.yml b/config/site_settings.yml index 4a828f21a2c..17ff664358b 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1354,6 +1354,14 @@ trust: min_trust_to_post_embedded_media: default: 0 enum: "TrustLevelSetting" + min_trust_level_to_allow_profile_background: + default: 0 + client: true + enum: "TrustLevelSetting" + min_trust_level_to_allow_user_card_background: + default: 0 + client: true + enum: "TrustLevelSetting" allow_flagging_staff: true send_tl1_welcome_message: true tl1_requires_topics_entered: 5 diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb index d19bd2af34e..aa917791849 100644 --- a/lib/guardian/user_guardian.rb +++ b/lib/guardian/user_guardian.rb @@ -158,4 +158,13 @@ module UserGuardian def can_see_summary_stats?(target_user) true end + + def can_upload_profile_header?(user) + (is_me?(user) && user.has_trust_level?(SiteSetting.min_trust_level_to_allow_profile_background.to_i)) || is_staff? + end + + def can_upload_user_card_background?(user) + (is_me?(user) && user.has_trust_level?(SiteSetting.min_trust_level_to_allow_user_card_background.to_i)) || is_staff? + end + end diff --git a/spec/components/guardian/user_guardian_spec.rb b/spec/components/guardian/user_guardian_spec.rb index fc2961a1de4..04c34a18b0f 100644 --- a/spec/components/guardian/user_guardian_spec.rb +++ b/spec/components/guardian/user_guardian_spec.rb @@ -38,6 +38,9 @@ describe UserGuardian do Upload.new(user_id: moderator.id, id: 4) end + let(:trust_level_1) { build(:user, trust_level: 1) } + let(:trust_level_2) { build(:user, trust_level: 2) } + describe '#can_pick_avatar?' do let :guardian do @@ -410,4 +413,43 @@ describe UserGuardian do expect(guardian.can_see_review_queue?).to eq(false) end end + + describe 'can_upload_profile_header' do + it 'returns true if it is an admin' do + guardian = Guardian.new(admin) + expect(guardian.can_upload_profile_header?(admin)).to eq(true) + end + + it 'returns true if the trust level of user matches site setting' do + guardian = Guardian.new(trust_level_2) + SiteSetting.min_trust_level_to_allow_profile_background = 2 + expect(guardian.can_upload_profile_header?(trust_level_2)).to eq(true) + end + + it 'returns false if the trust level of user does not matches site setting' do + guardian = Guardian.new(trust_level_1) + SiteSetting.min_trust_level_to_allow_profile_background = 2 + expect(guardian.can_upload_profile_header?(trust_level_1)).to eq(false) + end + end + + describe 'can_upload_user_card_background' do + it 'returns true if it is an admin' do + guardian = Guardian.new(admin) + expect(guardian.can_upload_user_card_background?(admin)).to eq(true) + end + + it 'returns true if the trust level of user matches site setting' do + guardian = Guardian.new(trust_level_2) + SiteSetting.min_trust_level_to_allow_user_card_background = 2 + expect(guardian.can_upload_user_card_background?(trust_level_2)).to eq(true) + end + + it 'returns false if the trust level of user does not matches site setting' do + guardian = Guardian.new(trust_level_1) + SiteSetting.min_trust_level_to_allow_user_card_background = 2 + expect(guardian.can_upload_user_card_background?(trust_level_1)).to eq(false) + end + end + end diff --git a/spec/serializers/web_hook_user_serializer_spec.rb b/spec/serializers/web_hook_user_serializer_spec.rb index 0d32e1a2076..ef6934d0041 100644 --- a/spec/serializers/web_hook_user_serializer_spec.rb +++ b/spec/serializers/web_hook_user_serializer_spec.rb @@ -23,7 +23,7 @@ RSpec.describe WebHookUserSerializer do it 'should only include the required keys' do count = serializer.as_json.keys.count - difference = count - 47 + difference = count - 49 expect(difference).to eq(0), lambda { message = (difference < 0 ? diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index 158c943da6c..e36a44d744b 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -145,8 +145,6 @@ describe UserUpdater do date_of_birth = Time.zone.now theme = Fabricate(:theme, user_selectable: true) - upload1 = Fabricate(:upload) - upload2 = Fabricate(:upload) seq = user.user_option.theme_key_seq @@ -161,9 +159,7 @@ describe UserUpdater do email_in_reply_to: false, date_of_birth: date_of_birth, theme_ids: [theme.id], - allow_private_messages: false, - card_background_upload_url: upload1.url, - profile_background_upload_url: upload2.url + allow_private_messages: false ) expect(val).to be_truthy @@ -182,21 +178,38 @@ describe UserUpdater do expect(user.user_option.theme_key_seq).to eq(seq + 1) expect(user.user_option.allow_private_messages).to eq(false) expect(user.date_of_birth).to eq(date_of_birth.to_date) - expect(user.card_background_upload).to eq(upload1) - expect(user.profile_background_upload).to eq(upload2) - - success = updater.update( - profile_background_upload_url: "", - card_background_upload_url: "" - ) + end + it "allows user to update profile header when the user has required trust level" do + user = Fabricate(:user, trust_level: 2) + updater = UserUpdater.new(user, user) + upload = Fabricate(:upload) + SiteSetting.min_trust_level_to_allow_profile_background = 2 + val = updater.update(profile_background_upload_url: upload.url) + expect(val).to be_truthy user.reload - + expect(user.profile_background_upload).to eq(upload) + success = updater.update(profile_background_upload_url: "") expect(success).to eq(true) - expect(user.card_background_upload).to eq(nil) + user.reload expect(user.profile_background_upload).to eq(nil) end + it "allows user to update user card background when the user has required trust level" do + user = Fabricate(:user, trust_level: 2) + updater = UserUpdater.new(user, user) + upload = Fabricate(:upload) + SiteSetting.min_trust_level_to_allow_user_card_background = 2 + val = updater.update(card_background_upload_url: upload.url) + expect(val).to be_truthy + user.reload + expect(user.card_background_upload).to eq(upload) + success = updater.update(card_background_upload_url: "") + expect(success).to eq(true) + user.reload + expect(user.card_background_upload).to eq(nil) + end + it "disables email_digests when enabling mailing_list_mode" do user = Fabricate(:user) updater = UserUpdater.new(acting_user, user) @@ -349,9 +362,7 @@ describe UserUpdater do context 'with permission to update title' do it 'allows user to change title' do user = Fabricate(:user, title: 'Emperor') - guardian = stub - guardian.stubs(:can_grant_title?).with(user, 'Minion').returns(true) - Guardian.stubs(:new).with(acting_user).returns(guardian) + Guardian.any_instance.stubs(:can_grant_title?).with(user, 'Minion').returns(true) updater = UserUpdater.new(acting_user, user) updater.update(title: 'Minion') @@ -390,9 +401,7 @@ describe UserUpdater do user.update(title: badge.name) user.user_profile.update(badge_granted_title: true) - guardian = stub - guardian.stubs(:can_grant_title?).with(user, 'Dancer').returns(true) - Guardian.stubs(:new).with(user).returns(guardian) + Guardian.any_instance.stubs(:can_grant_title?).with(user, 'Dancer').returns(true) updater = UserUpdater.new(user, user) updater.update(title: 'Dancer') @@ -415,9 +424,7 @@ describe UserUpdater do context 'without permission to update title' do it 'does not allow user to change title' do user = Fabricate(:user, title: 'Emperor') - guardian = stub - guardian.stubs(:can_grant_title?).with(user, 'Minion').returns(false) - Guardian.stubs(:new).with(acting_user).returns(guardian) + Guardian.any_instance.stubs(:can_grant_title?).with(user, 'Minion').returns(false) updater = UserUpdater.new(acting_user, user) updater.update(title: 'Minion')