From 25b8ed740b67ca8910343d6edfcf14b10633a21b Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 13 Oct 2020 16:17:06 +0300 Subject: [PATCH] DEV: Make site setting type uploaded_image_list use upload IDs (#10401) It used to be a list of concatenated upload URLs which was prone to break. --- .../modals/admin-uploaded-image-list.js | 4 +- .../app/controllers/avatar-selector.js | 27 +++++++++- .../app/initializers/avatar-select.js | 36 ------------- .../app/routes/preferences-account.js | 3 +- .../unit/controllers/avatar-selector-test.js | 25 +++++---- .../admin/site_settings_controller.rb | 4 ++ app/controllers/site_controller.rb | 10 ---- app/controllers/users_controller.rb | 2 +- app/jobs/scheduled/clean_up_uploads.rb | 21 ++------ app/models/concerns/has_url.rb | 24 +++++++++ app/models/user.rb | 11 ++-- config/routes.rb | 1 - config/site_settings.yml | 1 + ..._change_selectable_avatars_site_setting.rb | 51 +++++++++++++++++++ lib/site_setting_extension.rb | 23 +++++++-- lib/site_settings/type_supervisor.rb | 2 + .../selectable_avatars_enabled_validator.rb | 2 +- ...lectable_avatars_enabled_validator_spec.rb | 6 +-- spec/jobs/clean_up_uploads_spec.rb | 4 +- spec/models/upload_spec.rb | 50 ++++++++++++++++++ spec/models/user_spec.rb | 2 +- spec/requests/site_controller_spec.rb | 26 ---------- spec/requests/users_controller_spec.rb | 2 +- 23 files changed, 213 insertions(+), 124 deletions(-) delete mode 100644 app/assets/javascripts/discourse/app/initializers/avatar-select.js create mode 100644 db/migrate/20200810194943_change_selectable_avatars_site_setting.rb diff --git a/app/assets/javascripts/admin/addon/controllers/modals/admin-uploaded-image-list.js b/app/assets/javascripts/admin/addon/controllers/modals/admin-uploaded-image-list.js index 2185b7b137f..0aeffaad591 100644 --- a/app/assets/javascripts/admin/addon/controllers/modals/admin-uploaded-image-list.js +++ b/app/assets/javascripts/admin/addon/controllers/modals/admin-uploaded-image-list.js @@ -7,7 +7,7 @@ export default Controller.extend(ModalFunctionality, { @observes("model.value") _setup() { const value = this.get("model.value"); - this.set("images", value && value.length ? value.split("\n") : []); + this.set("images", value && value.length ? value.split("|") : []); }, actions: { @@ -20,7 +20,7 @@ export default Controller.extend(ModalFunctionality, { }, close() { - this.save(this.images.join("\n")); + this.save(this.images.join("|")); this.send("closeModal"); }, }, diff --git a/app/assets/javascripts/discourse/app/controllers/avatar-selector.js b/app/assets/javascripts/discourse/app/controllers/avatar-selector.js index 843a2bb61ba..099919cbce2 100644 --- a/app/assets/javascripts/discourse/app/controllers/avatar-selector.js +++ b/app/assets/javascripts/discourse/app/controllers/avatar-selector.js @@ -11,6 +11,31 @@ export default Controller.extend(ModalFunctionality, { gravatarBaseUrl: setting("gravatar_base_url"), gravatarLoginUrl: setting("gravatar_login_url"), + @discourseComputed( + "siteSettings.selectable_avatars_enabled", + "siteSettings.selectable_avatars" + ) + selectableAvatars(enabled, list) { + if (enabled) { + return list ? list.split("|") : []; + } + }, + + @discourseComputed( + "user.avatar_template", + "user.system_avatar_template", + "user.gravatar_avatar_template" + ) + selected(avatarTemplate, systemAvatarTemplate, gravatarAvatarTemplate) { + if (avatarTemplate === systemAvatarTemplate) { + return "system"; + } else if (avatarTemplate === gravatarAvatarTemplate) { + return "gravatar"; + } else { + return "custom"; + } + }, + @discourseComputed( "selected", "user.system_avatar_upload_id", @@ -55,7 +80,7 @@ export default Controller.extend(ModalFunctionality, { actions: { uploadComplete() { - this.set("selected", "uploaded"); + this.set("selected", "custom"); }, refreshGravatar() { diff --git a/app/assets/javascripts/discourse/app/initializers/avatar-select.js b/app/assets/javascripts/discourse/app/initializers/avatar-select.js deleted file mode 100644 index 2c3d48fc72e..00000000000 --- a/app/assets/javascripts/discourse/app/initializers/avatar-select.js +++ /dev/null @@ -1,36 +0,0 @@ -import showModal from "discourse/lib/show-modal"; -import { ajax } from "discourse/lib/ajax"; - -export default { - name: "avatar-select", - - initialize(container) { - this.selectableAvatarsEnabled = container.lookup( - "site-settings:main" - ).selectable_avatars_enabled; - - container - .lookup("service:app-events") - .on("show-avatar-select", this, "_showAvatarSelect"); - }, - - _showAvatarSelect(user) { - const avatarTemplate = user.avatar_template; - let selected = "uploaded"; - - if (avatarTemplate === user.system_avatar_template) { - selected = "system"; - } else if (avatarTemplate === user.gravatar_avatar_template) { - selected = "gravatar"; - } - - const modal = showModal("avatar-selector"); - modal.setProperties({ user, selected }); - - if (this.selectableAvatarsEnabled) { - ajax("/site/selectable-avatars.json").then((avatars) => - modal.set("selectableAvatars", avatars) - ); - } - }, -}; diff --git a/app/assets/javascripts/discourse/app/routes/preferences-account.js b/app/assets/javascripts/discourse/app/routes/preferences-account.js index bfd5874edb8..7e2fee5b557 100644 --- a/app/assets/javascripts/discourse/app/routes/preferences-account.js +++ b/app/assets/javascripts/discourse/app/routes/preferences-account.js @@ -1,3 +1,4 @@ +import showModal from "discourse/lib/show-modal"; import UserBadge from "discourse/models/user-badge"; import RestrictedUserRoute from "discourse/routes/restricted-user"; @@ -33,7 +34,7 @@ export default RestrictedUserRoute.extend({ actions: { showAvatarSelector(user) { - this.appEvents.trigger("show-avatar-select", user); + showModal("avatar-selector").setProperties({ user }); }, }, }); diff --git a/app/assets/javascripts/discourse/tests/unit/controllers/avatar-selector-test.js b/app/assets/javascripts/discourse/tests/unit/controllers/avatar-selector-test.js index 59880912517..aff8c19d288 100644 --- a/app/assets/javascripts/discourse/tests/unit/controllers/avatar-selector-test.js +++ b/app/assets/javascripts/discourse/tests/unit/controllers/avatar-selector-test.js @@ -1,6 +1,7 @@ +import EmberObject from "@ember/object"; +import { mapRoutes } from "discourse/mapping-router"; import { moduleFor } from "ember-qunit"; import { test } from "qunit"; -import { mapRoutes } from "discourse/mapping-router"; moduleFor("controller:avatar-selector", "controller:avatar-selector", { beforeEach() { @@ -12,29 +13,33 @@ moduleFor("controller:avatar-selector", "controller:avatar-selector", { test("avatarTemplate", function (assert) { const avatarSelectorController = this.subject(); - avatarSelectorController.setProperties({ - selected: "system", - user: { - system_avatar_upload_id: 1, - gravatar_avatar_upload_id: 2, - custom_avatar_upload_id: 3, - }, + const user = EmberObject.create({ + avatar_template: "avatar", + system_avatar_template: "system", + gravatar_avatar_template: "gravatar", + + system_avatar_upload_id: 1, + gravatar_avatar_upload_id: 2, + custom_avatar_upload_id: 3, }); + avatarSelectorController.setProperties({ user }); + + user.set("avatar_template", "system"); assert.equal( avatarSelectorController.get("selectedUploadId"), 1, "we are using system by default" ); - avatarSelectorController.set("selected", "gravatar"); + user.set("avatar_template", "gravatar"); assert.equal( avatarSelectorController.get("selectedUploadId"), 2, "we are using gravatar when set" ); - avatarSelectorController.set("selected", "custom"); + user.set("avatar_template", "avatar"); assert.equal( avatarSelectorController.get("selectedUploadId"), 3, diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index f8695f2f0c8..2455a5b4b64 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -16,6 +16,10 @@ class Admin::SiteSettingsController < Admin::AdminController value.strip! if value.is_a?(String) raise_access_hidden_setting(id) + if SiteSetting.type_supervisor.get_type(id) == :uploaded_image_list + value = Upload.get_from_urls(value.split("|")).to_a + end + if SiteSetting.type_supervisor.get_type(id) == :upload value = Upload.get_from_url(value) || "" end diff --git a/app/controllers/site_controller.rb b/app/controllers/site_controller.rb index f1ea98d34de..c2df1759cea 100644 --- a/app/controllers/site_controller.rb +++ b/app/controllers/site_controller.rb @@ -25,16 +25,6 @@ class SiteController < ApplicationController render json: custom_emoji end - def selectable_avatars - avatars = if SiteSetting.selectable_avatars_enabled? - (SiteSetting.selectable_avatars.presence || "").split("\n") - else - [] - end - - render json: avatars, root: false - end - def basic_info results = { logo_url: UrlHelper.absolute(SiteSetting.site_logo_url), diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index aa82acb8a64..b3fdf66926b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1156,7 +1156,7 @@ class UsersController < ApplicationController return render json: failed_json, status: 422 end - unless SiteSetting.selectable_avatars[upload.sha1] + unless SiteSetting.selectable_avatars.include?(upload) return render json: failed_json, status: 422 end diff --git a/app/jobs/scheduled/clean_up_uploads.rb b/app/jobs/scheduled/clean_up_uploads.rb index 75e91a8df3e..2b735f61a58 100644 --- a/app/jobs/scheduled/clean_up_uploads.rb +++ b/app/jobs/scheduled/clean_up_uploads.rb @@ -25,23 +25,6 @@ module Jobs s3_hostname = URI.parse(base_url).hostname s3_cdn_hostname = URI.parse(SiteSetting.Upload.s3_cdn_url || "").hostname - # Any URLs in site settings are fair game - ignore_urls = [ - *SiteSetting.selectable_avatars.split("\n"), - ].flatten.map do |url| - if url.present? - url = url.dup - - if s3_cdn_hostname.present? && s3_hostname.present? - url.gsub!(s3_cdn_hostname, s3_hostname) - end - - url[base_url] && url[url.index(base_url)..-1] - else - nil - end - end.compact.uniq - result = Upload.by_users .where("uploads.retain_hours IS NULL OR uploads.created_at < current_timestamp - interval '1 hour' * uploads.retain_hours") .where("uploads.created_at < ?", grace_period.hour.ago) @@ -71,7 +54,9 @@ module Jobs .where("g.flair_upload_id IS NULL") .where("ss.value IS NULL") - result = result.where("uploads.url NOT IN (?)", ignore_urls) if ignore_urls.present? + if SiteSetting.selectable_avatars.present? + result = result.where.not(id: SiteSetting.selectable_avatars.map(&:id)) + end result.find_each do |upload| if upload.sha1.present? diff --git a/app/models/concerns/has_url.rb b/app/models/concerns/has_url.rb index 300dafc11ca..dc6c58cee1d 100644 --- a/app/models/concerns/has_url.rb +++ b/app/models/concerns/has_url.rb @@ -44,5 +44,29 @@ module HasUrl result || self.find_by("url LIKE ?", "%#{data[1]}") end + + def get_from_urls(upload_urls) + urls = [] + sha1s = [] + + upload_urls.each do |url| + next if url.blank? + + uri = begin + URI(UrlHelper.unencode(url)) + rescue URI::Error + end + + next if uri&.path.blank? + urls << uri.path + + if data = extract_url(uri.path).presence + urls << data[1] + sha1s << data[2] if self.name == "Upload" + end + end + + self.where(url: urls).or(self.where(sha1: sha1s)) + end end end diff --git a/app/models/user.rb b/app/models/user.rb index c506871e113..72933dfdc7d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1193,13 +1193,10 @@ class User < ActiveRecord::Base end def set_random_avatar - if SiteSetting.selectable_avatars_enabled? && SiteSetting.selectable_avatars.present? - urls = SiteSetting.selectable_avatars.split("\n") - if urls.present? - if upload = Upload.get_from_url(urls.sample) - update_column(:uploaded_avatar_id, upload.id) - UserAvatar.create!(user_id: id, custom_upload_id: upload.id) - end + if SiteSetting.selectable_avatars_enabled? + if upload = SiteSetting.selectable_avatars.sample + update_column(:uploaded_avatar_id, upload.id) + UserAvatar.create!(user_id: id, custom_upload_id: upload.id) end end end diff --git a/config/routes.rb b/config/routes.rb index e549e9389a9..0c5e28b4241 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -66,7 +66,6 @@ Discourse::Application.routes.draw do get "site/basic-info" => 'site#basic_info' get "site/statistics" => 'site#statistics' - get "site/selectable-avatars" => "site#selectable_avatars" get "srv/status" => "forums#status" diff --git a/config/site_settings.yml b/config/site_settings.yml index f15a0577f90..42722d958f1 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1339,6 +1339,7 @@ files: validator: "SelectableAvatarsEnabledValidator" selectable_avatars: default: "" + client: true type: uploaded_image_list allow_all_attachments_for_group_messages: false png_to_jpg_quality: diff --git a/db/migrate/20200810194943_change_selectable_avatars_site_setting.rb b/db/migrate/20200810194943_change_selectable_avatars_site_setting.rb new file mode 100644 index 00000000000..a7566150080 --- /dev/null +++ b/db/migrate/20200810194943_change_selectable_avatars_site_setting.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +class ChangeSelectableAvatarsSiteSetting < ActiveRecord::Migration[6.0] + def up + selectable_avatars = execute("SELECT value FROM site_settings WHERE name = 'selectable_avatars'") + return if selectable_avatars.cmd_tuples == 0 + + # Keep old site setting value as a backup + execute <<~SQL + UPDATE site_settings + SET name = 'selectable_avatars_urls' + WHERE name = 'selectable_avatars' + SQL + + # Extract SHA1s from URLs and then use them for upload ID lookups + urls = [] + sha1s = [] + selectable_avatars.first["value"].split("\n").each do |url| + match = url.match(/(\/original\/\dX[\/\.\w]*\/(\h+)[\.\w]*)/) + if match.present? + urls << match[1] + sha1s << match[2] + else + STDERR.puts "Could not extract a SHA1 from #{url}" + end + end + + # Ensure at least one URL or SHA1 exists so the query below can be valid + return if urls.size == 0 && sha1s.size == 0 + + uploads_query = [] + uploads_query << "url IN (#{urls.map { |url| ActiveRecord::Base.connection.quote(url) }.join(',')})" if urls.size > 0 + uploads_query << "sha1 IN (#{sha1s.map { |sha1| ActiveRecord::Base.connection.quote(sha1) }.join(',')})" if sha1s.size > 0 + uploads_query = "SELECT DISTINCT id FROM uploads WHERE #{uploads_query.join(" OR ")}" + + upload_ids = execute(uploads_query).map { |row| row["id"] } + return if upload_ids.size == 0 + + execute <<~SQL + INSERT INTO site_settings(name, data_type, value, created_at, updated_at) + SELECT 'selectable_avatars', data_type, '#{upload_ids.join("|")}', created_at, NOW() + FROM site_settings + WHERE name = 'selectable_avatars_urls' + SQL + end + + def down + execute("DELETE FROM site_settings WHERE name = 'selectable_avatars'") + execute("UPDATE site_settings SET name = 'selectable_avatars' WHERE name = 'selectable_avatars_urls'") + end +end diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index da3f6cf9814..6b02292c6d3 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -209,6 +209,7 @@ module SiteSettingExtension MultiJson.dump(Hash[*@client_settings.map do |name| value = self.public_send(name) value = value.to_s if type_supervisor.get_type(name) == :upload + value = value.map(&:to_s).join("|") if type_supervisor.get_type(name) == :uploaded_image_list [name, value] end.flatten]) end @@ -233,10 +234,12 @@ module SiteSettingExtension .reject { |s, _| !include_hidden && hidden_settings.include?(s) } .map do |s, v| - value = public_send(s) type_hash = type_supervisor.type_hash(s) default = defaults.get(s, default_locale).to_s + value = public_send(s) + value = value.map(&:to_s).join("|") if type_hash[:type].to_s == "uploaded_image_list" + if type_hash[:type].to_s == "upload" && default.to_i < Upload::SEEDED_ID_THRESHOLD @@ -482,7 +485,21 @@ module SiteSettingExtension def setup_methods(name) clean_name = name.to_s.sub("?", "").to_sym - if type_supervisor.get_type(name) == :upload + if type_supervisor.get_type(name) == :uploaded_image_list + define_singleton_method clean_name do + uploads_list = uploads[name] + return uploads_list if uploads_list + + if (value = current[name]).nil? + refresh! + value = current[name] + end + + value = value.split("|").map(&:to_i) + uploads_list = Upload.where(id: value).to_a + uploads[name] = uploads_list if uploads_list + end + elsif type_supervisor.get_type(name) == :upload define_singleton_method clean_name do upload = uploads[name] return upload if upload @@ -552,7 +569,7 @@ module SiteSettingExtension end def clear_uploads_cache(name) - if type_supervisor.get_type(name) == :upload && uploads.has_key?(name) + if (type_supervisor.get_type(name) == :upload || type_supervisor.get_type(name) == :uploaded_image_list) && uploads.has_key?(name) uploads.delete(name) end end diff --git a/lib/site_settings/type_supervisor.rb b/lib/site_settings/type_supervisor.rb index ae78d5dd1ff..f30be8cfa0d 100644 --- a/lib/site_settings/type_supervisor.rb +++ b/lib/site_settings/type_supervisor.rb @@ -182,6 +182,8 @@ class SiteSettings::TypeSupervisor type = get_data_type(name, val) elsif type == self.class.types[:enum] val = @defaults_provider[name].is_a?(Integer) ? val.to_i : val.to_s + elsif type == self.class.types[:uploaded_image_list] && val.present? + val = val.is_a?(String) ? val : val.map(&:id).join("|") elsif type == self.class.types[:upload] && val.present? val = val.is_a?(Integer) ? val : val.id end diff --git a/lib/validators/selectable_avatars_enabled_validator.rb b/lib/validators/selectable_avatars_enabled_validator.rb index 065c5bcf981..cc54c9ce5d1 100644 --- a/lib/validators/selectable_avatars_enabled_validator.rb +++ b/lib/validators/selectable_avatars_enabled_validator.rb @@ -6,7 +6,7 @@ class SelectableAvatarsEnabledValidator end def valid_value?(value) - value == "f" || SiteSetting.selectable_avatars.split("\n").size > 1 + value == "f" || SiteSetting.selectable_avatars.size > 1 end def error_message diff --git a/spec/components/validators/selectable_avatars_enabled_validator_spec.rb b/spec/components/validators/selectable_avatars_enabled_validator_spec.rb index 2635e2e3740..034f58e4688 100644 --- a/spec/components/validators/selectable_avatars_enabled_validator_spec.rb +++ b/spec/components/validators/selectable_avatars_enabled_validator_spec.rb @@ -10,12 +10,12 @@ describe SelectableAvatarsEnabledValidator do SiteSetting.selectable_avatars = "" expect(validator.valid_value?("f")).to eq(true) - SiteSetting.selectable_avatars = [Fabricate(:image_upload).url, Fabricate(:image_upload).url].join("\n") + SiteSetting.selectable_avatars = [Fabricate(:image_upload), Fabricate(:image_upload)] expect(validator.valid_value?("f")).to eq(true) end it "returns true when there are at least two selectable avatars" do - SiteSetting.selectable_avatars = [Fabricate(:image_upload).url, Fabricate(:image_upload).url].join("\n") + SiteSetting.selectable_avatars = [Fabricate(:image_upload), Fabricate(:image_upload)] expect(validator.valid_value?("t")).to eq(true) end @@ -23,7 +23,7 @@ describe SelectableAvatarsEnabledValidator do SiteSetting.selectable_avatars = "" expect(validator.valid_value?("t")).to eq(false) - SiteSetting.selectable_avatars = Fabricate(:image_upload).url + SiteSetting.selectable_avatars = [Fabricate(:image_upload)] expect(validator.valid_value?("t")).to eq(false) end end diff --git a/spec/jobs/clean_up_uploads_spec.rb b/spec/jobs/clean_up_uploads_spec.rb index 828e1b6c9b7..68a73aa1f2a 100644 --- a/spec/jobs/clean_up_uploads_spec.rb +++ b/spec/jobs/clean_up_uploads_spec.rb @@ -126,11 +126,11 @@ describe Jobs::CleanUpUploads do end end - it "does not clean up uploads with URLs used in site settings" do + it "does not clean up selectable avatars" do avatar1_upload = fabricate_upload avatar2_upload = fabricate_upload - SiteSetting.selectable_avatars = [avatar1_upload.url, avatar2_upload.url].join("\n") + SiteSetting.selectable_avatars = [avatar1_upload, avatar2_upload] Jobs::CleanUpUploads.new.execute(nil) diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index d33ae4c5b1e..bf7024c04e4 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -229,6 +229,56 @@ describe Upload do end end + context ".get_from_urls" do + let(:upload) { Fabricate(:upload, sha1: "10f73034616a796dfd70177dc54b6def44c4ba6f") } + let(:upload2) { Fabricate(:upload, sha1: "2a7081e615f9075befd87a9a6d273935c0262cd5") } + + it "works with multiple uploads" do + expect(Upload.get_from_urls([upload.url, upload2.url])).to contain_exactly(upload, upload2) + end + + it "works for an extensionless URL" do + url = upload.url.sub('.png', '') + upload.update!(url: url) + expect(Upload.get_from_urls([url])).to contain_exactly(upload) + end + + it "works with uploads with mismatched URLs" do + upload.update!(url: "/uploads/default/12345/971308e535305c51.png") + expect(Upload.get_from_urls([upload.url])).to contain_exactly(upload) + expect(Upload.get_from_urls(["/uploads/default/123131/971308e535305c51.png"])).to be_empty + end + + it "works with an upload with a URL containing a deep tree" do + upload.update!(url: Discourse.store.get_path_for("original", 16001, upload.sha1, ".#{upload.extension}")) + expect(Upload.get_from_urls([upload.url])).to contain_exactly(upload) + end + + it "works when using a CDN" do + begin + original_asset_host = Rails.configuration.action_controller.asset_host + Rails.configuration.action_controller.asset_host = 'http://my.cdn.com' + + expect(Upload.get_from_urls([ + URI.join("http://my.cdn.com", upload.url).to_s + ])).to contain_exactly(upload) + ensure + Rails.configuration.action_controller.asset_host = original_asset_host + end + end + + it "works with full URLs" do + expect(Upload.get_from_urls([ + URI.join("http://discourse.some.com:3000/", upload.url).to_s + ])).to contain_exactly(upload) + end + + it "handles invalid URIs" do + urls = ["http://ip:port/index.html", "mailto:admin%40example.com", "mailto:example"] + expect { Upload.get_from_urls(urls) }.not_to raise_error + end + end + describe '.generate_digest' do it "should return the right digest" do expect(Upload.generate_digest(image.path)).to eq('bc975735dfc6409c1c2aa5ebf2239949bcbdbd65') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 36640cd1e8c..c530fc67ee1 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2050,7 +2050,7 @@ describe User do it "sets a random avatar when selectable avatars is enabled" do avatar1 = Fabricate(:upload) avatar2 = Fabricate(:upload) - SiteSetting.selectable_avatars = [avatar1.url, avatar2.url].join("\n") + SiteSetting.selectable_avatars = [avatar1, avatar2] SiteSetting.selectable_avatars_enabled = true user = Fabricate(:user) diff --git a/spec/requests/site_controller_spec.rb b/spec/requests/site_controller_spec.rb index 670a0998a3b..86c9b9e7ff5 100644 --- a/spec/requests/site_controller_spec.rb +++ b/spec/requests/site_controller_spec.rb @@ -64,30 +64,4 @@ describe SiteController do expect(response).to redirect_to '/' end end - - describe '.selectable_avatars' do - before do - SiteSetting.selectable_avatars = "https://www.discourse.org\nhttps://meta.discourse.org" - end - - it 'returns empty array when selectable avatars is disabled' do - SiteSetting.selectable_avatars_enabled = false - - get "/site/selectable-avatars.json" - json = response.parsed_body - - expect(response.status).to eq(200) - expect(json).to eq([]) - end - - it 'returns an array when selectable avatars is enabled' do - SiteSetting.selectable_avatars_enabled = true - - get "/site/selectable-avatars.json" - json = response.parsed_body - - expect(response.status).to eq(200) - expect(json).to contain_exactly("https://www.discourse.org", "https://meta.discourse.org") - end - end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 2909b35fda9..b455b0dc2f9 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -2368,7 +2368,7 @@ describe UsersController do context 'selectable avatars is enabled' do before do - SiteSetting.selectable_avatars = [avatar1.url, avatar2.url].join("\n") + SiteSetting.selectable_avatars = [avatar1, avatar2] SiteSetting.selectable_avatars_enabled = true end