DEV: Convert allow_uploaded_avatars to groups (#24810)

This change converts the allow_uploaded_avatars site setting to uploaded_avatars_allowed_groups.

See: https://meta.discourse.org/t/283408

Hides the old setting
Adds the new site setting
Adds a deprecation warning
Updates to use the new setting
Adds a migration to fill in the new setting if the old setting was changed
Adds an entry to the site_setting.keywords section
Updates tests to account for the new change
After a couple of months, we will remove the allow_uploaded_avatars setting entirely.

Internal ref: /t/117248

app/assets/javascripts/discourse/app/components/modal/avatar-selector.js

@@ -98,20 +98,10 @@ export default class AvatarSelectorModal extends Component {
   }
 
   get siteSettingMatches() {
-    const allowUploadedAvatars = this.siteSettings.allow_uploaded_avatars;
-    switch (allowUploadedAvatars) {
-      case "disabled":
-        return false;
-      case "staff":
-        return this.currentUser.staff;
-      case "admin":
-        return this.currentUser.admin;
-      default:
-        return (
-          this.currentUser.trust_level >= parseInt(allowUploadedAvatars, 10) ||
-          this.currentUser.staff
-        );
-    }
+    return this.siteSettings.userInAnyGroups(
+      "uploaded_avatars_allowed_groups",
+      this.currentUser
+    );
   }
 
   @action

app/controllers/uploads_controller.rb

@@ -30,10 +30,10 @@ class UploadsController < ApplicationController
     type =
       (params[:upload_type].presence || params[:type].presence).parameterize(separator: "_")[0..50]
 
-    if type == "avatar" && !me.admin? &&
+    if type == "avatar" &&
          (
            SiteSetting.discourse_connect_overrides_avatar ||
-             !TrustLevelAndStaffAndDisabledSetting.matches?(SiteSetting.allow_uploaded_avatars, me)
+             !me.in_any_groups?(SiteSetting.uploaded_avatars_allowed_groups_map)
          )
       return render json: failed_json, status: 422
     end

app/controllers/users_controller.rb

@@ -1294,7 +1294,8 @@ class UsersController < ApplicationController
 
     if type.blank? || type == "system"
       upload_id = nil
-    elsif !TrustLevelAndStaffAndDisabledSetting.matches?(SiteSetting.allow_uploaded_avatars, user)
+    elsif !user.in_any_groups?(SiteSetting.uploaded_avatars_allowed_groups_map) &&
+          !user.is_system_user?
       return render json: failed_json, status: 422
     else
       upload_id = params[:upload_id]

config/locales/server.en.yml

@@ -2172,6 +2172,7 @@ en:
     logout_redirect: "Location to redirect browser to after logout (eg: https://example.com/logout)"
 
     allow_uploaded_avatars: "Allow users to upload custom profile pictures."
+    uploaded_avatars_allowed_groups: "Specify groups allowed to upload custom profile pictures."
     default_avatars: "URLs to avatars that will be used by default for new users until they change them."
     automatically_download_gravatars: "Download Gravatars for users upon account creation or email change."
     digest_topics: "The maximum number of popular topics to display in the email summary."
@@ -2553,6 +2554,7 @@ en:
       approve_new_topics_unless_allowed_groups: "approve_new_topics_unless_trust_level"
       email_in_allowed_groups: "email_in_min_trust"
       edit_wiki_post_allowed_groups: "minmin_trust_to_edit_wiki_post"
+      uploaded_avatars_allowed_groups: "allow_uploaded_avatars"
 
     placeholder:
       discourse_connect_provider_secrets:

config/site_settings.yml

@@ -1550,6 +1550,13 @@ files:
     client: true
     default: "0"
     enum: "TrustLevelAndStaffAndDisabledSetting"
+    hidden: true
+  uploaded_avatars_allowed_groups:
+    client: true
+    default: 10
+    type: group_list
+    allow_any: false
+    refresh: true
   default_avatars:
     default: ""
     type: url_list

db/migrate/20231205013029_fill_uploaded_avatars_allowed_groups_based_on_deprecated_settings.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class FillUploadedAvatarsAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0]
+  def up
+    old_setting_trust_level =
+      DB.query_single(
+        "SELECT value FROM site_settings WHERE name = 'allow_uploaded_avatars' LIMIT 1",
+      ).first
+
+    if old_setting_trust_level.present?
+      group_id =
+        case old_setting_trust_level
+        when "disabled"
+          ""
+        when "admin"
+          "1"
+        when "staff"
+          "3"
+        else
+          "1#{old_setting_trust_level}"
+        end
+
+      DB.exec(
+        "INSERT INTO site_settings(name, value, data_type, created_at, updated_at)
+        VALUES('uploaded_avatars_allowed_groups', :setting, '20', NOW(), NOW())",
+        setting: group_id,
+      )
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/composer_messages_finder.rb

@@ -90,7 +90,7 @@ class ComposerMessagesFinder
     # - "allow uploaded avatars" is disabled
     if SiteSetting.disable_avatar_education_message ||
          SiteSetting.discourse_connect_overrides_avatar ||
-         !TrustLevelAndStaffAndDisabledSetting.matches?(SiteSetting.allow_uploaded_avatars, @user)
+         !@user.in_any_groups?(SiteSetting.uploaded_avatars_allowed_groups_map)
       return
     end
 

lib/site_settings/deprecated_settings.rb

@@ -20,6 +20,7 @@ module SiteSettings::DeprecatedSettings
     ],
     ["email_in_min_trust", "email_in_allowed_groups", false, "3.3"],
     ["min_trust_to_edit_wiki_post", "edit_wiki_post_allowed_groups", false, "3.3"],
+    ["allow_uploaded_avatars", "uploaded_avatars_allowed_groups", false, "3.3"],
   ]
 
   def setup_deprecated_methods

spec/lib/composer_messages_finder_spec.rb

@@ -100,7 +100,7 @@ RSpec.describe ComposerMessagesFinder do
 
   describe ".check_avatar_notification" do
     let(:finder) { ComposerMessagesFinder.new(user, composer_action: "createTopic") }
-    fab!(:user)
+    fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
 
     context "with success" do
       let!(:message) { finder.check_avatar_notification }
@@ -143,8 +143,15 @@ RSpec.describe ComposerMessagesFinder do
     end
 
     it "doesn't notify users if 'allow_uploaded_avatars' setting is disabled" do
-      SiteSetting.allow_uploaded_avatars = "disabled"
+      user.update!(trust_level: 3)
+      Group.refresh_automatic_groups!
+      user.reload
+
+      SiteSetting.uploaded_avatars_allowed_groups = ""
       expect(finder.check_avatar_notification).to be_blank
+
+      SiteSetting.uploaded_avatars_allowed_groups = "13"
+      expect(finder.check_avatar_notification).to be_present
     end
   end
 

spec/requests/api/users_spec.rb

@@ -185,7 +185,7 @@ RSpec.describe "users" do
       response "200", "avatar updated" do
         expected_response_schema = load_spec_schema("success_ok_response")
 
-        let(:user) { Fabricate(:user) }
+        let(:user) { Fabricate(:user, refresh_auto_groups: true) }
         let(:username) { user.username }
         let(:upload) { Fabricate(:upload, user: user) }
         let(:params) { { "upload_id" => upload.id, "type" => "uploaded" } }

spec/requests/uploads_controller_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 RSpec.describe UploadsController do
-  fab!(:user)
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
 
   describe "#create" do
     it "requires you to be logged in" do
@@ -132,10 +132,15 @@ RSpec.describe UploadsController do
         )
       end
 
-      it "ensures allow_uploaded_avatars is enabled when uploading an avatar" do
-        SiteSetting.allow_uploaded_avatars = "disabled"
+      it "ensures user belongs to uploaded_avatars_allowed_groups when uploading an avatar" do
+        SiteSetting.uploaded_avatars_allowed_groups = "13"
         post "/uploads.json", params: { file: logo, type: "avatar" }
         expect(response.status).to eq(422)
+
+        user.update!(trust_level: 3)
+        Group.refresh_automatic_groups!
+        post "/uploads.json", params: { file: logo, type: "avatar" }
+        expect(response.status).to eq(200)
       end
 
       it "ensures discourse_connect_overrides_avatar is not enabled when uploading an avatar" do
@@ -144,14 +149,6 @@ RSpec.describe UploadsController do
         expect(response.status).to eq(422)
       end
 
-      it "always allows admins to upload avatars" do
-        sign_in(Fabricate(:admin))
-        SiteSetting.allow_uploaded_avatars = "disabled"
-
-        post "/uploads.json", params: { file: logo, type: "avatar" }
-        expect(response.status).to eq(200)
-      end
-
       it "allows staff to upload any file in PM" do
         SiteSetting.authorized_extensions = "jpg"
         SiteSetting.allow_staff_to_upload_any_file_in_pm = true

spec/requests/users_controller_spec.rb

@@ -3342,8 +3342,8 @@ RSpec.describe UsersController do
         expect(response.status).to eq(422)
       end
 
-      it "raises an error when selecting the custom/uploaded avatar and allow_uploaded_avatars is disabled" do
-        SiteSetting.allow_uploaded_avatars = "disabled"
+      it "raises an error when selecting the custom/uploaded avatar and uploaded_avatars_allowed_groups is disabled" do
+        SiteSetting.uploaded_avatars_allowed_groups = ""
         put "/u/#{user1.username}/preferences/avatar/pick.json",
             params: {
               upload_id: upload.id,
@@ -3353,8 +3353,8 @@ RSpec.describe UsersController do
         expect(response.status).to eq(422)
       end
 
-      it "raises an error when selecting the custom/uploaded avatar and allow_uploaded_avatars is admin" do
-        SiteSetting.allow_uploaded_avatars = "admin"
+      it "raises an error when selecting the custom/uploaded avatar and uploaded_avatars_allowed_groups is admin" do
+        SiteSetting.uploaded_avatars_allowed_groups = "1"
         put "/u/#{user1.username}/preferences/avatar/pick.json",
             params: {
               upload_id: upload.id,
@@ -3363,6 +3363,7 @@ RSpec.describe UsersController do
         expect(response.status).to eq(422)
 
         user1.update!(admin: true)
+        Group.refresh_automatic_groups!
         put "/u/#{user1.username}/preferences/avatar/pick.json",
             params: {
               upload_id: upload.id,
@@ -3371,8 +3372,8 @@ RSpec.describe UsersController do
         expect(response.status).to eq(200)
       end
 
-      it "raises an error when selecting the custom/uploaded avatar and allow_uploaded_avatars is staff" do
-        SiteSetting.allow_uploaded_avatars = "staff"
+      it "raises an error when selecting the custom/uploaded avatar and uploaded_avatars_allowed_groups is staff" do
+        SiteSetting.uploaded_avatars_allowed_groups = "3"
         put "/u/#{user1.username}/preferences/avatar/pick.json",
             params: {
               upload_id: upload.id,
@@ -3381,6 +3382,7 @@ RSpec.describe UsersController do
         expect(response.status).to eq(422)
 
         user1.update!(moderator: true)
+        Group.refresh_automatic_groups!
         put "/u/#{user1.username}/preferences/avatar/pick.json",
             params: {
               upload_id: upload.id,
@@ -3389,8 +3391,8 @@ RSpec.describe UsersController do
         expect(response.status).to eq(200)
       end
 
-      it "raises an error when selecting the custom/uploaded avatar and allow_uploaded_avatars is a trust level" do
-        SiteSetting.allow_uploaded_avatars = "3"
+      it "raises an error when selecting the custom/uploaded avatar and uploaded_avatars_allowed_groups is a trust level" do
+        SiteSetting.uploaded_avatars_allowed_groups = "13"
         put "/u/#{user1.username}/preferences/avatar/pick.json",
             params: {
               upload_id: upload.id,
@@ -3399,6 +3401,7 @@ RSpec.describe UsersController do
         expect(response.status).to eq(422)
 
         user1.update!(trust_level: 3)
+        Group.refresh_automatic_groups!
         put "/u/#{user1.username}/preferences/avatar/pick.json",
             params: {
               upload_id: upload.id,
@@ -3408,7 +3411,7 @@ RSpec.describe UsersController do
       end
 
       it "ignores the upload if picking a system avatar" do
-        SiteSetting.allow_uploaded_avatars = "disabled"
+        SiteSetting.uploaded_avatars_allowed_groups = ""
         another_upload = Fabricate(:upload)
 
         put "/u/#{user1.username}/preferences/avatar/pick.json",
@@ -3422,7 +3425,7 @@ RSpec.describe UsersController do
       end
 
       it "raises an error if the type is invalid" do
-        SiteSetting.allow_uploaded_avatars = "disabled"
+        SiteSetting.uploaded_avatars_allowed_groups = ""
         another_upload = Fabricate(:upload)
 
         put "/u/#{user1.username}/preferences/avatar/pick.json",

spec/system/user_page/user_preferences_account_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 describe "User preferences for Account", type: :system do
-  fab!(:user)
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
   let(:user_account_preferences_page) { PageObjects::Pages::UserPreferencesAccount.new }
   let(:avatar_selector_modal) { PageObjects::Modals::AvatarSelector.new }
   before { sign_in(user) }