From c08c40dc230cdd58a7d7458a4a7c45df78752f4c Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 18 Oct 2024 01:16:16 +1100 Subject: [PATCH] FEATURE: theme_modifiers can depend on theme settings (plus serialize_post_user_badges) (#29227) Theme modifiers can now be defined as theme settings, this allows for site operators to override behavior of theme modifiers. New syntax is: ``` { ... "modifiers": { "modifier_name": { "type": "setting", "value": "setting_name" } } } ``` This also introduces a new theme modifier for serialize_post_user_badges. Name of badge must match the name of the badge in the badges table. The client-side is updated to load this new data from the post-stream serializer. Co-authored-by: David Taylor --- .../discourse/app/models/post-stream.js | 4 ++ .../javascripts/discourse/app/models/post.js | 10 ++++ app/models/remote_theme.rb | 12 ++-- app/models/theme.rb | 2 - app/models/theme_modifier_set.rb | 55 ++++++++++++++++--- app/models/theme_setting.rb | 7 +++ .../post_stream_serializer_mixin.rb | 17 ++++++ ...3602_add_post_badges_to_theme_modifiers.rb | 7 +++ lib/topic_view.rb | 50 +++++++++++++++++ spec/models/remote_theme_spec.rb | 34 ++++++++++-- spec/requests/topics_controller_spec.rb | 47 ++++++++++++++++ 11 files changed, 226 insertions(+), 19 deletions(-) create mode 100644 db/migrate/20241011033602_add_post_badges_to_theme_modifiers.rb diff --git a/app/assets/javascripts/discourse/app/models/post-stream.js b/app/assets/javascripts/discourse/app/models/post-stream.js index 6315fd1e88c..a30e1f51afc 100644 --- a/app/assets/javascripts/discourse/app/models/post-stream.js +++ b/app/assets/javascripts/discourse/app/models/post-stream.js @@ -1152,6 +1152,10 @@ export default class PostStream extends RestModel { headers, }).then((result) => { this._setSuggestedTopics(result); + if (result.user_badges) { + this.topic.user_badges ??= {}; + Object.assign(this.topic.user_badges, result.user_badges); + } const posts = get(result, "post_stream.posts"); diff --git a/app/assets/javascripts/discourse/app/models/post.js b/app/assets/javascripts/discourse/app/models/post.js index 51c2470e041..96737deaeda 100644 --- a/app/assets/javascripts/discourse/app/models/post.js +++ b/app/assets/javascripts/discourse/app/models/post.js @@ -478,4 +478,14 @@ export default class Post extends RestModel { get topicNotificationLevel() { return this.topic.details.notification_level; } + + get userBadges() { + if (!this.topic?.user_badges) { + return; + } + const badgeIds = this.topic.user_badges.users[this.user_id]?.badge_ids; + if (badgeIds) { + return badgeIds.map((badgeId) => this.topic.user_badges.badges[badgeId]); + } + } } diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb index 8b8452fd3d8..7179934f7b6 100644 --- a/app/models/remote_theme.rb +++ b/app/models/remote_theme.rb @@ -297,10 +297,12 @@ class RemoteTheme < ActiveRecord::Base end ThemeModifierSet.modifiers.keys.each do |modifier_name| - theme.theme_modifier_set.public_send( - :"#{modifier_name}=", - theme_info.dig("modifiers", modifier_name.to_s), - ) + value = theme_info.dig("modifiers", modifier_name.to_s) + if Hash === value && value["type"] == "setting" + theme.theme_modifier_set.add_theme_setting_modifier(modifier_name, value["value"]) + else + theme.theme_modifier_set.public_send(:"#{modifier_name}=", value) + end end if !theme.theme_modifier_set.valid? @@ -383,6 +385,8 @@ class RemoteTheme < ActiveRecord::Base self.transaction(&transaction_block) end + theme.theme_modifier_set.save! if theme.theme_modifier_set.refresh_theme_setting_modifiers + self ensure begin diff --git a/app/models/theme.rb b/app/models/theme.rb index fbfb36c7eb8..dd0b032fbe9 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -756,9 +756,7 @@ class Theme < ActiveRecord::Base def update_setting(setting_name, new_value) target_setting = settings[setting_name.to_sym] raise Discourse::NotFound unless target_setting - target_setting.value = new_value - self.theme_setting_requests_refresh = true if target_setting.requests_refresh? end diff --git a/app/models/theme_modifier_set.rb b/app/models/theme_modifier_set.rb index f7fe5a5a5b7..89eda882595 100644 --- a/app/models/theme_modifier_set.rb +++ b/app/models/theme_modifier_set.rb @@ -47,7 +47,7 @@ class ThemeModifierSet < ActiveRecord::Base when :string_array all_values.flatten(1) else - raise ThemeModifierSetError "Invalid theme modifier combine_mode" + raise ThemeModifierSetError, "Invalid theme modifier combine_mode" end end @@ -75,14 +75,49 @@ class ThemeModifierSet < ActiveRecord::Base super(val.map { |dim| "#{dim[0]}x#{dim[1]}" }) end + def add_theme_setting_modifier(modifier_name, setting_name) + self.theme_setting_modifiers ||= {} + self.theme_setting_modifiers[modifier_name] = setting_name + end + + def refresh_theme_setting_modifiers(target_setting_name: nil, target_setting_value: nil) + changed = false + if self.theme_setting_modifiers.present? + self.theme_setting_modifiers.each do |modifier_name, setting_name| + modifier_name = modifier_name.to_sym + setting_name = setting_name.to_sym + + next if target_setting_name.present? && target_setting_name.to_sym != setting_name + + value = + target_setting_name.present? ? target_setting_value : theme.settings[setting_name]&.value + value = coerce_setting_value(modifier_name, value) + if read_attribute(modifier_name) != value + write_attribute(modifier_name, value) + changed = true + end + end + end + changed + end + private + def coerce_setting_value(modifier_name, value) + type = ThemeModifierSet.modifiers.dig(modifier_name, :type) + if type == :boolean + value.to_s != "false" + elsif type == :string_array + value.is_a?(Array) ? value : value.to_s.split("|") + end + end + # Build the list of modifiers from the DB schema. # This allows plugins to introduce new modifiers by adding columns to the table def self.load_modifiers hash = {} columns_hash.each do |column_name, info| - next if %w[id theme_id].include?(column_name) + next if %w[id theme_id theme_setting_modifiers].include?(column_name) type = nil if info.type == :string && info.array? @@ -105,13 +140,15 @@ end # # Table name: theme_modifier_sets # -# id :bigint not null, primary key -# theme_id :bigint not null -# serialize_topic_excerpts :boolean -# csp_extensions :string is an Array -# svg_icons :string is an Array -# topic_thumbnail_sizes :string is an Array -# custom_homepage :boolean +# id :bigint not null, primary key +# theme_id :bigint not null +# serialize_topic_excerpts :boolean +# csp_extensions :string is an Array +# svg_icons :string is an Array +# topic_thumbnail_sizes :string is an Array +# custom_homepage :boolean +# serialize_post_user_badges :string is an Array +# theme_setting_modifiers :jsonb # # Indexes # diff --git a/app/models/theme_setting.rb b/app/models/theme_setting.rb index 58221b548d9..cabfbc54f89 100644 --- a/app/models/theme_setting.rb +++ b/app/models/theme_setting.rb @@ -22,6 +22,13 @@ class ThemeSetting < ActiveRecord::Base if self.data_type == ThemeSetting.types[:upload] && saved_change_to_value? UploadReference.ensure_exist!(upload_ids: [self.value], target: self) end + + if theme.theme_modifier_set.refresh_theme_setting_modifiers( + target_setting_name: self.name, + target_setting_value: self.value, + ) + theme.theme_modifier_set.save! + end end def clear_settings_cache diff --git a/app/serializers/post_stream_serializer_mixin.rb b/app/serializers/post_stream_serializer_mixin.rb index e3e8612aab5..e3cb95554ee 100644 --- a/app/serializers/post_stream_serializer_mixin.rb +++ b/app/serializers/post_stream_serializer_mixin.rb @@ -4,6 +4,7 @@ module PostStreamSerializerMixin def self.included(klass) klass.attributes :post_stream klass.attributes :timeline_lookup + klass.attributes :user_badges end def include_stream? @@ -14,6 +15,18 @@ module PostStreamSerializerMixin true end + def include_user_badges? + badges_to_include.present? + end + + def user_badges + object.user_badges(badges_to_include) + end + + def badges_to_include + @badges_to_include ||= theme_modifier_helper.serialize_post_user_badges + end + def post_stream result = { posts: posts } @@ -55,4 +68,8 @@ module PostStreamSerializerMixin end end end + + def theme_modifier_helper + @theme_modifier_helper ||= ThemeModifierHelper.new(request: scope.request) + end end diff --git a/db/migrate/20241011033602_add_post_badges_to_theme_modifiers.rb b/db/migrate/20241011033602_add_post_badges_to_theme_modifiers.rb new file mode 100644 index 00000000000..fd54996385d --- /dev/null +++ b/db/migrate/20241011033602_add_post_badges_to_theme_modifiers.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true +class AddPostBadgesToThemeModifiers < ActiveRecord::Migration[7.1] + def change + add_column :theme_modifier_sets, :serialize_post_user_badges, :string, array: true + add_column :theme_modifier_sets, :theme_setting_modifiers, :jsonb + end +end diff --git a/lib/topic_view.rb b/lib/topic_view.rb index d89612d12c7..16aed2f74f7 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -155,6 +155,56 @@ class TopicView @personal_message = @topic.private_message? end + def user_badges(badge_names) + return if !badge_names.present? + + user_ids = Set.new + posts.each { |post| user_ids << post.user_id if post.user_id } + + return if !user_ids.present? + + badges = + Badge.where("LOWER(name) IN (?)", badge_names.map(&:downcase)).where(enabled: true).to_a + + sql = <<~SQL + SELECT user_id, badge_id + FROM user_badges + WHERE user_id IN (:user_ids) AND badge_id IN (:badge_ids) + GROUP BY user_id, badge_id + ORDER BY user_id, badge_id + SQL + + user_badges = DB.query(sql, user_ids: user_ids, badge_ids: badges.map(&:id)) + + user_badge_mapping = {} + user_badges.each do |user_badge| + user_badge_mapping[user_badge.user_id] ||= [] + user_badge_mapping[user_badge.user_id] << user_badge.badge_id + end + + indexed_badges = {} + + badges.each do |badge| + indexed_badges[badge.id] = { + id: badge.id, + name: badge.name, + slug: badge.slug, + description: badge.description, + icon: badge.icon, + image_url: badge.image_url, + badge_grouping_id: badge.badge_grouping_id, + badge_type_id: badge.badge_type_id, + } + end + + user_badge_mapping = + user_badge_mapping + .map { |user_id, badge_ids| [user_id, { id: user_id, badge_ids: badge_ids }] } + .to_h + + { users: user_badge_mapping, badges: indexed_badges } + end + def show_read_indicator? return false if !@user || !topic.private_message? diff --git a/spec/models/remote_theme_spec.rb b/spec/models/remote_theme_spec.rb index a4d70b1ca87..51f1396d761 100644 --- a/spec/models/remote_theme_spec.rb +++ b/spec/models/remote_theme_spec.rb @@ -25,7 +25,15 @@ RSpec.describe RemoteTheme do } }, "modifiers": { - "serialize_topic_excerpts": true + "serialize_topic_excerpts": true, + "custom_homepage": { + "type": "setting", + "value": "boolean_setting" + }, + "serialize_post_user_badges": { + "type": "setting", + "value": "list_setting" + } } } JSON @@ -42,6 +50,12 @@ RSpec.describe RemoteTheme do JS let :initial_repo do + settings = <<~YAML + boolean_setting: true + list_setting: + type: list + default: "" + YAML setup_git_repo( "about.json" => about_json, "desktop/desktop.scss" => scss_data, @@ -55,7 +69,7 @@ RSpec.describe RemoteTheme do "common/embedded.scss" => "EMBED", "common/color_definitions.scss" => ":root{--color-var: red}", "assets/font.woff2" => "FAKE FONT", - "settings.yaml" => "boolean_setting: true", + "settings.yaml" => settings, "locales/en.yml" => "sometranslations", "migrations/settings/0001-some-migration.js" => migration_js, ) @@ -175,6 +189,7 @@ RSpec.describe RemoteTheme do expect(remote.minimum_discourse_version).to eq("1.0.0") expect(theme.theme_modifier_set.serialize_topic_excerpts).to eq(true) + expect(theme.theme_modifier_set.custom_homepage).to eq(true) expect(theme.theme_fields.length).to eq(12) @@ -187,7 +202,9 @@ RSpec.describe RemoteTheme do expect(mapped["0-font"]).to eq("") - expect(mapped["3-yaml"]).to eq("boolean_setting: true") + expect(mapped["3-yaml"]).to eq( + "boolean_setting: true\nlist_setting:\n type: list\n default: \"\"\n", + ) expect(mapped["4-en"]).to eq("sometranslations") expect(mapped["7-acceptance/theme-test.js"]).to eq("assert.ok(true);") @@ -197,9 +214,18 @@ RSpec.describe RemoteTheme do expect(mapped.length).to eq(12) - expect(theme.settings.length).to eq(1) + expect(theme.settings.length).to eq(2) expect(theme.settings[:boolean_setting].value).to eq(true) + expect(theme.settings[:list_setting].value).to eq("") + # lets change the setting to see modifier reflects + theme.update_setting(:boolean_setting, false) + theme.update_setting(:list_setting, "badge1|badge2") + theme.save! + theme.reload + + expect(theme.theme_modifier_set.custom_homepage).to eq(false) + expect(theme.theme_modifier_set.serialize_post_user_badges).to eq(%w[badge1 badge2]) expect(remote.remote_updated_at).to eq_time(time) scheme = ColorScheme.find_by(theme_id: theme.id) diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 6470eec2868..c36cff13727 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -2455,6 +2455,53 @@ RSpec.describe TopicsController do expect(user_options_queries.size).to eq(1) # for all mentioned users end + context "with serialize_post_user_badges" do + fab!(:badge) + before do + theme = Fabricate(:theme) + theme.theme_modifier_set.update!(serialize_post_user_badges: [badge.name]) + SiteSetting.default_theme_id = theme.id + end + + it "correctly returns user badges that are registered" do + first_post = topic.posts.order(:post_number).first + first_post.user.user_badges.create!( + badge_id: badge.id, + granted_at: Time.zone.now, + granted_by: Discourse.system_user, + ) + + expected_payload = { + "users" => { + first_post.user_id.to_s => { + "id" => first_post.user.id, + "badge_ids" => [badge.id], + }, + }, + "badges" => { + badge.id.to_s => { + "id" => badge.id, + "name" => badge.name, + "slug" => badge.slug, + "description" => badge.description, + "icon" => badge.icon, + "image_url" => badge.image_url, + "badge_grouping_id" => badge.badge_grouping_id, + "badge_type_id" => badge.badge_type_id, + }, + }, + } + + get "/t/#{topic.slug}/#{topic.id}.json" + user_badges = response.parsed_body["user_badges"] + expect(user_badges).to eq(expected_payload) + + get "/t/#{topic.id}/posts.json?post_ids[]=#{first_post.id}" + user_badges = response.parsed_body["user_badges"] + expect(user_badges).to eq(expected_payload) + end + end + context "with registered redirect_to_correct_topic_additional_query_parameters" do let(:modifier_block) { Proc.new { |allowed_params| allowed_params << :silly_param } }