From d1bdb6c65d1d1de8f610d1e39bf3788c0c241ef6 Mon Sep 17 00:00:00 2001 From: Jeff Wong Date: Thu, 24 Feb 2022 10:57:39 -1000 Subject: [PATCH] FEATURE: upload an avatar option for uploading avatars with selectable avatars (#15878) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * FEATURE: upload an avatar option for uploading avatars with selectable avatars Allow staff or users at or above a trust level to upload avatars even when the site has selectable avatars enabled. Everyone can still pick from the list of avatars. The option to upload is shown below the selectable avatar list. refactored boolean site setting into an enum with the following values: disabled: No selectable avatars enabled (default) everyone: Show selectable avatars, and allow everyone to upload custom avatars tl1: Show selectable avatars, but require tl1+ and staff to upload custom avatars tl2: Show selectable avatars, but require tl2+ and staff to upload custom avatars tl3: Show selectable avatars, but require tl3+ and staff to upload custom avatars tl4: Show selectable avatars, but require tl4 and staff to upload custom avatars staff: Show selectable avatars, but only allow staff to upload custom avatars no_one: Show selectable avatars. No users can upload custom avatars Co-authored-by: Régis Hanol --- .../app/controllers/avatar-selector.js | 34 ++++- .../app/templates/modal/avatar-selector.hbs | 12 +- .../tests/acceptance/preferences-test.js | 130 +++++++++++++++++- app/controllers/users_controller.rb | 2 +- app/models/user.rb | 2 +- config/locales/client.en.yml | 1 + config/locales/server.en.yml | 2 +- config/site_settings.yml | 16 ++- ...3955_migrate_selectable_avatars_enabled.rb | 31 +++++ ...b => selectable_avatars_mode_validator.rb} | 4 +- .../selectable_avatars_mode_validator_spec.rb | 30 ++++ spec/models/user_spec.rb | 2 +- spec/requests/users_controller_spec.rb | 2 +- 13 files changed, 250 insertions(+), 18 deletions(-) create mode 100644 db/post_migrate/20220202223955_migrate_selectable_avatars_enabled.rb rename lib/validators/{selectable_avatars_enabled_validator.rb => selectable_avatars_mode_validator.rb} (66%) create mode 100644 spec/components/validators/selectable_avatars_mode_validator_spec.rb diff --git a/app/assets/javascripts/discourse/app/controllers/avatar-selector.js b/app/assets/javascripts/discourse/app/controllers/avatar-selector.js index 83121e0468e..8373b7a106d 100644 --- a/app/assets/javascripts/discourse/app/controllers/avatar-selector.js +++ b/app/assets/javascripts/discourse/app/controllers/avatar-selector.js @@ -17,15 +17,43 @@ export default Controller.extend(ModalFunctionality, { }, @discourseComputed( - "siteSettings.selectable_avatars_enabled", + "siteSettings.selectable_avatars_mode", "siteSettings.selectable_avatars" ) - selectableAvatars(enabled, list) { - if (enabled) { + selectableAvatars(mode, list) { + if (mode !== "disabled") { return list ? list.split("|") : []; } }, + @discourseComputed("siteSettings.selectable_avatars_mode") + showSelectableAvatars(mode) { + return mode !== "disabled"; + }, + + @discourseComputed("siteSettings.selectable_avatars_mode") + showAvatarUploader(mode) { + switch (mode) { + case "no_one": + return false; + case "tl1": + case "tl2": + case "tl3": + case "tl4": + const allowedTl = parseInt(mode.replace("tl", ""), 10); + return ( + this.user.admin || + this.user.moderator || + this.user.trust_level >= allowedTl + ); + case "staff": + return this.user.admin || this.user.moderator; + case "everyone": + default: + return true; + } + }, + @discourseComputed( "user.use_logo_small_as_avatar", "user.avatar_template", diff --git a/app/assets/javascripts/discourse/app/templates/modal/avatar-selector.hbs b/app/assets/javascripts/discourse/app/templates/modal/avatar-selector.hbs index 18d845b9a3c..e7dd9107d19 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/avatar-selector.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/avatar-selector.hbs @@ -1,5 +1,5 @@ {{#d-modal-body title="user.change_avatar.title" class="avatar-selector"}} - {{#if siteSettings.selectable_avatars_enabled}} + {{#if showSelectableAvatars}}
{{#each selectableAvatars as |avatar|}} @@ -7,7 +7,11 @@ {{/each}}
- {{else}} + {{#if showAvatarUploader}} +

{{html-safe (i18n "user.change_avatar.use_custom")}}

+ {{/if}} + {{/if}} + {{#if showAvatarUploader}} {{#if user.use_logo_small_as_avatar}}
{{radio-button id="logo-small" name="logo" value="logo" selection=selected}} @@ -55,9 +59,9 @@ {{/if}} {{/d-modal-body}} -{{#unless siteSettings.selectable_avatars_enabled}} +{{#if showAvatarUploader}}
{{d-button action=(action "saveAvatarSelection") class="btn-primary" disabled=submitDisabled label="save"}} {{d-modal-cancel close=(route-action "closeModal")}}
-{{/unless}} +{{/if}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/preferences-test.js b/app/assets/javascripts/discourse/tests/acceptance/preferences-test.js index ea04050eb11..023ab689217 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/preferences-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/preferences-test.js @@ -299,7 +299,7 @@ acceptance( "Avatar selector when selectable avatars is enabled", function (needs) { needs.user(); - needs.settings({ selectable_avatars_enabled: true }); + needs.settings({ selectable_avatars_mode: "no_one" }); needs.pretender((server, helper) => { server.get("/site/selectable-avatars.json", () => helper.response([ @@ -315,6 +315,134 @@ acceptance( assert.ok( exists(".selectable-avatars", "opens the avatar selection modal") ); + assert.notOk( + exists( + "#uploaded-avatar", + "avatar selection modal does not include option to upload" + ) + ); + }); + } +); +acceptance( + "Avatar selector when selectable avatars allows staff to upload", + function (needs) { + needs.user(); + needs.settings({ selectable_avatars_mode: "staff" }); + needs.pretender((server, helper) => { + server.get("/site/selectable-avatars.json", () => + helper.response([ + "https://www.discourse.org", + "https://meta.discourse.org", + ]) + ); + }); + + test("allows staff to upload", async function (assert) { + await updateCurrentUser({ + trust_level: 3, + moderator: true, + admin: false, + }); + await visit("/u/eviltrout/preferences"); + await click(".pref-avatar .btn"); + assert.ok( + exists(".selectable-avatars", "opens the avatar selection modal") + ); + assert.ok( + exists( + "#uploaded-avatar", + "avatar selection modal includes option to upload" + ) + ); + }); + test("disallow nonstaff", async function (assert) { + await visit("/u/eviltrout/preferences"); + await updateCurrentUser({ + trust_level: 3, + moderator: false, + admin: false, + }); + await click(".pref-avatar .btn"); + assert.ok( + exists(".selectable-avatars", "opens the avatar selection modal") + ); + assert.notOk( + exists( + "#uploaded-avatar", + "avatar selection modal does not include option to upload" + ) + ); + }); + } +); +acceptance( + "Avatar selector when selectable avatars allows trust level 3+ to upload", + function (needs) { + needs.user(); + needs.settings({ selectable_avatars_mode: "tl3" }); + needs.pretender((server, helper) => { + server.get("/site/selectable-avatars.json", () => + helper.response([ + "https://www.discourse.org", + "https://meta.discourse.org", + ]) + ); + }); + + test("with a tl3 user", async function (assert) { + await visit("/u/eviltrout/preferences"); + await updateCurrentUser({ + trust_level: 3, + moderator: false, + admin: false, + }); + await click(".pref-avatar .btn"); + assert.ok( + exists(".selectable-avatars", "opens the avatar selection modal") + ); + assert.ok( + exists( + "#uploaded-avatar", + "avatar selection modal does includes option to upload" + ) + ); + }); + test("with a tl2 user", async function (assert) { + await visit("/u/eviltrout/preferences"); + await updateCurrentUser({ + trust_level: 2, + moderator: false, + admin: false, + }); + await click(".pref-avatar .btn"); + assert.ok( + exists(".selectable-avatars", "opens the avatar selection modal") + ); + assert.notOk( + exists( + "#uploaded-avatar", + "avatar selection modal does not include option to upload" + ) + ); + }); + test("always allow staff to upload", async function (assert) { + await visit("/u/eviltrout/preferences"); + await updateCurrentUser({ + trust_level: 2, + moderator: true, + admin: false, + }); + await click(".pref-avatar .btn"); + assert.ok( + exists(".selectable-avatars", "opens the avatar selection modal") + ); + assert.ok( + exists( + "#uploaded-avatar", + "avatar selection modal includes option to upload" + ) + ); }); } ); diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 493cbd112b4..b329dd53cf2 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1244,7 +1244,7 @@ class UsersController < ApplicationController return render json: failed_json, status: 422 end - unless SiteSetting.selectable_avatars_enabled + if SiteSetting.selectable_avatars_mode == "disabled" return render json: failed_json, status: 422 end diff --git a/app/models/user.rb b/app/models/user.rb index f4f80e70e7b..0ab48be074d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1261,7 +1261,7 @@ class User < ActiveRecord::Base end def set_random_avatar - if SiteSetting.selectable_avatars_enabled? + if SiteSetting.selectable_avatars_mode != "disabled" if upload = SiteSetting.selectable_avatars.sample update_column(:uploaded_avatar_id, upload.id) UserAvatar.create!(user_id: id, custom_upload_id: upload.id) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 5ef7ed8fd6b..e7c0e941fb7 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1381,6 +1381,7 @@ en: upload_title: "Upload your picture" image_is_not_a_square: "Warning: we've cropped your image; width and height were not equal." logo_small: "Site's small logo. Used by default." + use_custom: "Or upload a custom avatar:" change_profile_background: title: "Profile Header" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index d6b0f60d385..70f46f48cf6 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1828,7 +1828,7 @@ en: use_site_small_logo_as_system_avatar: "Use the site's small logo instead of the system user's avatar. Requires the logo to be present." restrict_letter_avatar_colors: "A list of 6-digit hexadecimal color values to be used for letter avatar background." enable_listing_suspended_users_on_search: "Enable regular users to find suspended users." - selectable_avatars_enabled: "Force users to choose an avatar from the list." + selectable_avatars_mode: "Allow users to select an avatar from the selectable_avatars list and limit custom avatar uploads to the selected trust level." selectable_avatars: "List of avatars users can choose from." allow_all_attachments_for_group_messages: "Allow all email attachments for group messages." diff --git a/config/site_settings.yml b/config/site_settings.yml index 36af41ffdcc..922a5d28fbe 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1397,10 +1397,20 @@ files: type: list list_type: compact validator: "ColorListValidator" - selectable_avatars_enabled: - default: false + selectable_avatars_mode: + default: disabled client: true - validator: "SelectableAvatarsEnabledValidator" + type: enum + choices: + - disabled + - everyone + - tl1 + - tl2 + - tl3 + - tl4 + - staff + - no_one + validator: "SelectableAvatarsModeValidator" selectable_avatars: default: "" client: true diff --git a/db/post_migrate/20220202223955_migrate_selectable_avatars_enabled.rb b/db/post_migrate/20220202223955_migrate_selectable_avatars_enabled.rb new file mode 100644 index 00000000000..5b67f9041ce --- /dev/null +++ b/db/post_migrate/20220202223955_migrate_selectable_avatars_enabled.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class MigrateSelectableAvatarsEnabled < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + UPDATE site_settings AS s + SET value = + CASE WHEN t.value = 't' THEN 'no_one' + ELSE 'disabled' + END, + data_type = #{SiteSettings::TypeSupervisor.types[:enum]}, + name = 'selectable_avatars_mode' + FROM site_settings t + WHERE s.id = t.id AND s.name = 'selectable_avatars_enabled' + SQL + end + + def down + execute <<~SQL + UPDATE site_settings AS s + SET value = + CASE WHEN t.value IN ('everyone', 'no_one', 'staff', 'tl1','tl2', 'tl3', 'tl4') THEN 't' + ELSE 'f' + END, + data_type = #{SiteSettings::TypeSupervisor.types[:bool]}, + name = 'selectable_avatars_enabled' + FROM site_settings t + WHERE s.id = t.id AND s.name = 'selectable_avatars_mode' + SQL + end +end diff --git a/lib/validators/selectable_avatars_enabled_validator.rb b/lib/validators/selectable_avatars_mode_validator.rb similarity index 66% rename from lib/validators/selectable_avatars_enabled_validator.rb rename to lib/validators/selectable_avatars_mode_validator.rb index cc54c9ce5d1..dcf46e7d21a 100644 --- a/lib/validators/selectable_avatars_enabled_validator.rb +++ b/lib/validators/selectable_avatars_mode_validator.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true -class SelectableAvatarsEnabledValidator +class SelectableAvatarsModeValidator def initialize(opts = {}) @opts = opts end def valid_value?(value) - value == "f" || SiteSetting.selectable_avatars.size > 1 + value == "disabled" || SiteSetting.selectable_avatars.size > 1 end def error_message diff --git a/spec/components/validators/selectable_avatars_mode_validator_spec.rb b/spec/components/validators/selectable_avatars_mode_validator_spec.rb new file mode 100644 index 00000000000..cfdb0ed2955 --- /dev/null +++ b/spec/components/validators/selectable_avatars_mode_validator_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe SelectableAvatarsModeValidator do + describe '#valid_value?' do + subject(:validator) { described_class.new } + + it "returns true when disabling" do + SiteSetting.selectable_avatars = "" + expect(validator.valid_value?("disabled")).to eq(true) + + SiteSetting.selectable_avatars = [Fabricate(:image_upload), Fabricate(:image_upload)] + expect(validator.valid_value?("disabled")).to eq(true) + end + + it "returns true when there are at least two selectable avatars" do + SiteSetting.selectable_avatars = [Fabricate(:image_upload), Fabricate(:image_upload)] + expect(validator.valid_value?("no_one")).to eq(true) + end + + it "returns false when selectable avatars is blank or has one avatar" do + SiteSetting.selectable_avatars = "" + expect(validator.valid_value?("no_one")).to eq(false) + + SiteSetting.selectable_avatars = [Fabricate(:image_upload)] + expect(validator.valid_value?("no_one")).to eq(false) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9b2c2e1f3a5..3e67f029abc 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2141,7 +2141,7 @@ describe User do avatar1 = Fabricate(:upload) avatar2 = Fabricate(:upload) SiteSetting.selectable_avatars = [avatar1, avatar2] - SiteSetting.selectable_avatars_enabled = true + SiteSetting.selectable_avatars_mode = "no_one" user = Fabricate(:user) expect(user.uploaded_avatar_id).not_to be(nil) diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 8027d38520c..2ca1107f5d6 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -2622,7 +2622,7 @@ describe UsersController do before do SiteSetting.selectable_avatars = [avatar1, avatar2] - SiteSetting.selectable_avatars_enabled = true + SiteSetting.selectable_avatars_mode = "no_one" end it 'raises an error when selectable avatars is empty' do