diff --git a/app/assets/javascripts/discourse/app/lib/utilities.js b/app/assets/javascripts/discourse/app/lib/utilities.js index 75ba7f02364..4b6c57aa573 100644 --- a/app/assets/javascripts/discourse/app/lib/utilities.js +++ b/app/assets/javascripts/discourse/app/lib/utilities.js @@ -20,17 +20,17 @@ export function splitString(str, separator = ",") { export function translateSize(size) { switch (size) { case "tiny": - return 20; + return 24; case "small": - return 25; + return 24; case "medium": - return 32; + return 48; case "large": - return 45; + return 48; case "extra_large": - return 60; + return 96; case "huge": - return 120; + return 144; } return size; } @@ -62,11 +62,31 @@ export function avatarUrl(template, size, { customGetURL } = {}) { if (!template) { return ""; } - const rawSize = getRawSize(translateSize(size)); + const rawSize = getRawAvatarSize(translateSize(size)); const templatedPath = template.replace(/\{size\}/g, rawSize); return (customGetURL || getURLWithCDN)(templatedPath); } +let allowedSizes = null; + +export function getRawAvatarSize(size) { + allowedSizes ??= + helperContext() + .siteSettings["avatar_sizes"].split("|") + .map((s) => parseInt(s, 10)) + .sort((a, b) => a - b); + + size = getRawSize(size); + + for (let i = 0; i < allowedSizes.length; i++) { + if (allowedSizes[i] >= size) { + return allowedSizes[i]; + } + } + + return allowedSizes[allowedSizes.length - 1]; +} + export function getRawSize(size) { const pixelRatio = window.devicePixelRatio || 1; let rawSize = 1; diff --git a/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js b/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js index 281ae9b189f..9da560adb2a 100644 --- a/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js +++ b/app/assets/javascripts/discourse/tests/unit/lib/utilities-test.js @@ -8,7 +8,7 @@ import { escapeExpression, extractDomainFromUrl, fillMissingDates, - getRawSize, + getRawAvatarSize, inCodeBlock, initializeDefaultHomepage, mergeSortedLists, @@ -86,17 +86,26 @@ module("Unit | Utilities", function (hooks) { ); }); + test("getRawAvatarSize avoids redirects", function (assert) { + assert.strictEqual( + getRawAvatarSize(1), + 24, + "returns the first size larger on the menu" + ); + + assert.strictEqual(getRawAvatarSize(2000), 288, "caps at highest"); + }); + test("avatarUrl", function (assert) { - let rawSize = getRawSize; assert.blank(avatarUrl("", "tiny"), "no template returns blank"); assert.strictEqual( avatarUrl("/fake/template/{size}.png", "tiny"), - "/fake/template/" + rawSize(20) + ".png", + "/fake/template/" + getRawAvatarSize(24) + ".png", "simple avatar url" ); assert.strictEqual( avatarUrl("/fake/template/{size}.png", "large"), - "/fake/template/" + rawSize(45) + ".png", + "/fake/template/" + getRawAvatarSize(48) + ".png", "different size" ); @@ -104,7 +113,9 @@ module("Unit | Utilities", function (hooks) { assert.strictEqual( avatarUrl("/fake/template/{size}.png", "large"), - "https://app-cdn.example.com/fake/template/" + rawSize(45) + ".png", + "https://app-cdn.example.com/fake/template/" + + getRawAvatarSize(48) + + ".png", "uses CDN if present" ); }); @@ -124,7 +135,7 @@ module("Unit | Utilities", function (hooks) { let avatarTemplate = "/path/to/avatar/{size}.png"; assert.strictEqual( avatarImg({ avatarTemplate, size: "tiny" }), - "", + "", "it returns the avatar html" ); @@ -134,7 +145,7 @@ module("Unit | Utilities", function (hooks) { size: "tiny", title: "evilest trout", }), - "", + "", "it adds a title if supplied" ); @@ -144,7 +155,7 @@ module("Unit | Utilities", function (hooks) { size: "tiny", extraClasses: "evil fish", }), - "", + "", "it adds extra classes if supplied" ); diff --git a/app/assets/javascripts/discourse/tests/unit/models/report-test.js b/app/assets/javascripts/discourse/tests/unit/models/report-test.js index c626a5a50b1..8e01d05762d 100644 --- a/app/assets/javascripts/discourse/tests/unit/models/report-test.js +++ b/app/assets/javascripts/discourse/tests/unit/models/report-test.js @@ -370,7 +370,7 @@ module("Unit | Model | report", function (hooks) { const computedUsernameLabel = usernameLabel.compute(row); assert.strictEqual( computedUsernameLabel.formattedValue, - "joffrey" + "joffrey" ); assert.strictEqual(computedUsernameLabel.value, "joffrey"); @@ -459,7 +459,7 @@ module("Unit | Model | report", function (hooks) { const userLink = computedLabels[0].compute(row).formattedValue; assert.strictEqual( userLink, - "joffrey" + "joffrey" ); }); }); diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb index 838b5cd182e..fa830b3fa46 100644 --- a/app/models/user_avatar.rb +++ b/app/models/user_avatar.rb @@ -150,7 +150,7 @@ class UserAvatar < ActiveRecord::Base tempfile.close! if tempfile && tempfile.respond_to?(:close!) end - def self.ensure_consistency! + def self.ensure_consistency!(max_optimized_avatars_to_remove: 20_000) DB.exec <<~SQL UPDATE user_avatars SET gravatar_upload_id = NULL @@ -174,6 +174,29 @@ class UserAvatar < ActiveRecord::Base up.id IS NULL ) SQL + + ids = + DB.query_single(<<~SQL, sizes: Discourse.avatar_sizes, limit: max_optimized_avatars_to_remove) + SELECT oi.id FROM user_avatars a + JOIN optimized_images oi ON oi.upload_id = a.custom_upload_id + LEFT JOIN upload_references ur ON ur.upload_id = a.custom_upload_id and ur.target_type <> 'UserAvatar' + WHERE oi.width not in (:sizes) AND oi.height not in (:sizes) AND ur.upload_id IS NULL + LIMIT :limit + SQL + + warnings_reported = 0 + + ids.each do |id| + begin + OptimizedImage.find(id).destroy! + rescue ActiveRecord::RecordNotFound + rescue => e + if warnings_reported < 10 + Discourse.warn_exception(e, message: "Failed to remove optimized image") + warnings_reported += 1 + end + end + end end end diff --git a/config/site_settings.yml b/config/site_settings.yml index 98ff694b03c..5cc215c0e27 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1467,9 +1467,10 @@ files: type: url_list client: true avatar_sizes: - default: "20|25|32|45|60|120" + default: "24|48|72|96|144|288" type: list list_type: compact + client: true external_system_avatars_enabled: default: true client: true diff --git a/lib/discourse.rb b/lib/discourse.rb index 156812a31b2..97c1954e1b8 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -327,19 +327,12 @@ module Discourse @anonymous_top_menu_items ||= Discourse.anonymous_filters + %i[categories top] end + # list of pixel ratios Discourse tries to optimize for PIXEL_RATIOS ||= [1, 1.5, 2, 3] def self.avatar_sizes # TODO: should cache these when we get a notification system for site settings - set = Set.new - - SiteSetting - .avatar_sizes - .split("|") - .map(&:to_i) - .each { |size| PIXEL_RATIOS.each { |pixel_ratio| set << (size * pixel_ratio).to_i } } - - set + Set.new(SiteSetting.avatar_sizes.split("|").map(&:to_i)) end def self.activate_plugins! diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 91d9b304791..4a668a30bfc 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -208,6 +208,7 @@ module PrettyText __optInput.watchedWordsReplace = #{WordWatcher.word_matcher_regexps(:replace, engine: :js).to_json}; __optInput.watchedWordsLink = #{WordWatcher.word_matcher_regexps(:link, engine: :js).to_json}; __optInput.additionalOptions = #{Site.markdown_additional_options.to_json}; + __optInput.avatar_sizes = #{SiteSetting.avatar_sizes.to_json}; JS buffer << "__optInput.topicId = #{opts[:topic_id].to_i};\n" if opts[:topic_id] diff --git a/lib/pretty_text/shims.js b/lib/pretty_text/shims.js index 793a3992eb5..37f3e492382 100644 --- a/lib/pretty_text/shims.js +++ b/lib/pretty_text/shims.js @@ -16,10 +16,11 @@ define("I18n", ["exports"], function (exports) { exports.default = I18n; }); -// Formatting doesn't currently need any helper context define("discourse-common/lib/helpers", ["exports"], function (exports) { exports.helperContext = function () { - return {}; + return { + siteSettings: { avatar_sizes: __optInput.avatar_sizes }, + }; }; }); diff --git a/plugins/chat/spec/integration/post_chat_quote_spec.rb b/plugins/chat/spec/integration/post_chat_quote_spec.rb index 2ffe97df0b5..c3343d1c199 100644 --- a/plugins/chat/spec/integration/post_chat_quote_spec.rb +++ b/plugins/chat/spec/integration/post_chat_quote_spec.rb @@ -237,7 +237,7 @@ martin
- +
#{message1.user.username}
@@ -251,7 +251,7 @@ martin
- +
#{message2.user.username}
diff --git a/plugins/chat/spec/models/chat/message_spec.rb b/plugins/chat/spec/models/chat/message_spec.rb index 561e9256dea..5ddfb92adaa 100644 --- a/plugins/chat/spec/models/chat/message_spec.rb +++ b/plugins/chat/spec/models/chat/message_spec.rb @@ -75,7 +75,7 @@ describe Chat::Message do post = Fabricate(:post, topic: topic) SiteSetting.external_system_avatars_enabled = false avatar_src = - "//test.localhost#{User.system_avatar_template(post.user.username).gsub("{size}", "40")}" + "//test.localhost#{User.system_avatar_template(post.user.username).gsub("{size}", "48")}" cooked = described_class.cook(<<~RAW) [quote="#{post.user.username}, post:#{post.post_number}, topic:#{topic.id}"] @@ -87,7 +87,7 @@ describe Chat::Message do
- +
chatbbcodeuser
@@ -150,7 +150,7 @@ describe Chat::Message do
- +
otherbbcodeuser
diff --git a/spec/lib/discourse_spec.rb b/spec/lib/discourse_spec.rb index f07eaee1071..2504a12169a 100644 --- a/spec/lib/discourse_spec.rb +++ b/spec/lib/discourse_spec.rb @@ -13,28 +13,8 @@ RSpec.describe Discourse do describe "avatar_sizes" do it "returns a list of integers" do - expect(Discourse.avatar_sizes).to contain_exactly( - 20, - 25, - 30, - 32, - 37, - 40, - 45, - 48, - 50, - 60, - 64, - 67, - 75, - 90, - 96, - 120, - 135, - 180, - 240, - 360, - ) + SiteSetting.avatar_sizes = "10|20|30" + expect(Discourse.avatar_sizes).to contain_exactly(10, 20, 30) end end diff --git a/spec/lib/oneboxer_spec.rb b/spec/lib/oneboxer_spec.rb index ed7cf50c2e9..4cc6472a6f5 100644 --- a/spec/lib/oneboxer_spec.rb +++ b/spec/lib/oneboxer_spec.rb @@ -94,7 +94,7 @@ RSpec.describe Oneboxer do onebox = preview(public_reply.url, user, public_category, public_topic) expect(onebox).not_to include(public_topic.title) - expect(onebox).to include(replier.avatar_template_url.sub("{size}", "40")) + expect(onebox).to include(replier.avatar_template_url.sub("{size}", "48")) expect(preview(public_hidden.url, user, public_category)).to match_html( link(public_hidden.url), diff --git a/spec/lib/pretty_text_spec.rb b/spec/lib/pretty_text_spec.rb index 3dc294aca2d..2848fca3a82 100644 --- a/spec/lib/pretty_text_spec.rb +++ b/spec/lib/pretty_text_spec.rb @@ -251,7 +251,7 @@ RSpec.describe PrettyText do

ddd

@@ -273,7 +273,7 @@ RSpec.describe PrettyText do

ddd

@@ -294,7 +294,7 @@ RSpec.describe PrettyText do

ddd

@@ -320,7 +320,7 @@ RSpec.describe PrettyText do

quoted post

@@ -477,7 +477,7 @@ RSpec.describe UsernameChanger do

quoted post

@@ -485,7 +485,7 @@ RSpec.describe UsernameChanger do