From e1bc43aa31feac2aa5ac0ef5c362a15d94e4df99 Mon Sep 17 00:00:00 2001 From: Natalie Tay Date: Mon, 24 Apr 2023 16:14:52 +0800 Subject: [PATCH] Revert "DEV: Improve `add_to_serializer` `include_*` options (#21073)" (#21219) This reverts commit 4895e76ef797127ce45b33ba3a9a2174293ce9d8. --- lib/plugin/instance.rb | 30 ++--------- plugins/chat/plugin.rb | 86 ++++++++++++++------------------ plugins/poll/plugin.rb | 22 ++++---- spec/lib/plugin/instance_spec.rb | 28 ----------- 4 files changed, 53 insertions(+), 113 deletions(-) diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index cad7061ad12..60eeb014e06 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -109,27 +109,7 @@ class Plugin::Instance delegate :name, to: :metadata - def add_to_serializer( - serializer, - attr, - deprecated_respect_plugin_enabled = nil, - respect_plugin_enabled: true, - include_condition: nil, - &block - ) - if !deprecated_respect_plugin_enabled.nil? - Discourse.deprecate( - "add_to_serializer's respect_plugin_enabled argument should be passed as a keyword argument", - ) - respect_plugin_enabled = deprecated_respect_plugin_enabled - end - - if attr.to_s.starts_with?("include_") - Discourse.deprecate( - "add_to_serializer should not be used to directly override include_*? methods. Use the include_condition keyword argument instead", - ) - end - + def add_to_serializer(serializer, attr, define_include_method = true, &block) reloadable_patch do |plugin| base = begin @@ -143,13 +123,9 @@ class Plugin::Instance unless attr.to_s.start_with?("include_") klass.attributes(attr) - if respect_plugin_enabled || include_condition + if define_include_method # Don't include serialized methods if the plugin is disabled - klass.public_send(:define_method, "include_#{attr}?") do - next false if respect_plugin_enabled && !plugin.enabled? - next instance_exec(&include_condition) if include_condition - true - end + klass.public_send(:define_method, "include_#{attr}?") { plugin.enabled? } end end diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index d041adfeb3b..83433ccdf60 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -181,48 +181,31 @@ after_initialize do scope.user.id != object.id && scope.can_chat? && Guardian.new(object).can_chat? end - add_to_serializer( - :current_user, - :can_chat, - include_condition: -> do - return @can_chat if defined?(@can_chat) - @can_chat = SiteSetting.chat_enabled && scope.can_chat? - end, - ) { true } + add_to_serializer(:current_user, :can_chat) { true } - add_to_serializer( - :current_user, - :has_chat_enabled, - include_condition: -> do - return @has_chat_enabled if defined?(@has_chat_enabled) - @has_chat_enabled = include_can_chat? && object.user_option.chat_enabled - end, - ) { true } + add_to_serializer(:current_user, :include_can_chat?) do + return @can_chat if defined?(@can_chat) - add_to_serializer( - :current_user, - :chat_sound, - include_condition: -> { include_has_chat_enabled? && object.user_option.chat_sound }, - ) { object.user_option.chat_sound } + @can_chat = SiteSetting.chat_enabled && scope.can_chat? + end - add_to_serializer( - :current_user, - :needs_channel_retention_reminder, - include_condition: -> do - include_has_chat_enabled? && object.staff? && - !object.user_option.dismissed_channel_retention_reminder && - !SiteSetting.chat_channel_retention_days.zero? - end, - ) { true } + add_to_serializer(:current_user, :has_chat_enabled) { true } - add_to_serializer( - :current_user, - :needs_dm_retention_reminder, - include_condition: -> do - include_has_chat_enabled? && !object.user_option.dismissed_dm_retention_reminder && - !SiteSetting.chat_dm_retention_days.zero? - end, - ) { true } + add_to_serializer(:current_user, :include_has_chat_enabled?) do + return @has_chat_enabled if defined?(@has_chat_enabled) + + @has_chat_enabled = include_can_chat? && object.user_option.chat_enabled + end + + add_to_serializer(:current_user, :chat_sound) { object.user_option.chat_sound } + + add_to_serializer(:current_user, :include_chat_sound?) do + include_has_chat_enabled? && object.user_option.chat_sound + end + + add_to_serializer(:current_user, :needs_channel_retention_reminder) { true } + + add_to_serializer(:current_user, :needs_dm_retention_reminder) { true } add_to_serializer(:current_user, :has_joinable_public_channels) do Chat::ChannelFetcher.secured_public_channel_search( @@ -238,11 +221,18 @@ after_initialize do Chat::ChannelIndexSerializer.new(structured, scope: self.scope, root: false).as_json end - add_to_serializer( - :current_user, - :chat_drafts, - include_condition: -> { include_has_chat_enabled? }, - ) do + add_to_serializer(:current_user, :include_needs_channel_retention_reminder?) do + include_has_chat_enabled? && object.staff? && + !object.user_option.dismissed_channel_retention_reminder && + !SiteSetting.chat_channel_retention_days.zero? + end + + add_to_serializer(:current_user, :include_needs_dm_retention_reminder?) do + include_has_chat_enabled? && !object.user_option.dismissed_dm_retention_reminder && + !SiteSetting.chat_dm_retention_days.zero? + end + + add_to_serializer(:current_user, :chat_drafts) do Chat::Draft .where(user_id: object.id) .order(updated_at: :desc) @@ -251,13 +241,13 @@ after_initialize do .map { |row| { channel_id: row[0], data: row[1] } } end + add_to_serializer(:current_user, :include_chat_drafts?) { include_has_chat_enabled? } + add_to_serializer(:user_option, :chat_enabled) { object.chat_enabled } - add_to_serializer( - :user_option, - :chat_sound, - include_condition: -> { !object.chat_sound.blank? }, - ) { object.chat_sound } + add_to_serializer(:user_option, :chat_sound) { object.chat_sound } + + add_to_serializer(:user_option, :include_chat_sound?) { !object.chat_sound.blank? } add_to_serializer(:user_option, :only_chat_push_notifications) do object.only_chat_push_notifications diff --git a/plugins/poll/plugin.rb b/plugins/poll/plugin.rb index 8ad92eae268..84b337a823b 100644 --- a/plugins/poll/plugin.rb +++ b/plugins/poll/plugin.rb @@ -195,7 +195,7 @@ after_initialize do end end - add_to_class(PostSerializer, :preloaded_polls) do + add_to_serializer(:post, :preloaded_polls, false) do @preloaded_polls ||= if @topic_view.present? @topic_view.polls[object.id] @@ -204,18 +204,15 @@ after_initialize do end end - add_to_serializer(:post, :polls, include_condition: -> { preloaded_polls.present? }) do + add_to_serializer(:post, :include_preloaded_polls?) { false } + + add_to_serializer(:post, :polls, false) do preloaded_polls.map { |p| PollSerializer.new(p, root: false, scope: self.scope) } end - add_to_serializer( - :post, - :polls_votes, - include_condition: -> do - scope.user&.id.present? && preloaded_polls.present? && - preloaded_polls.any? { |p| p.has_voted?(scope.user) } - end, - ) do + add_to_serializer(:post, :include_polls?) { SiteSetting.poll_enabled && preloaded_polls.present? } + + add_to_serializer(:post, :polls_votes, false) do preloaded_polls .map do |poll| user_poll_votes = @@ -230,6 +227,11 @@ after_initialize do .to_h end + add_to_serializer(:post, :include_polls_votes?) do + SiteSetting.poll_enabled && scope.user&.id.present? && preloaded_polls.present? && + preloaded_polls.any? { |p| p.has_voted?(scope.user) } + end + register_search_advanced_filter(/in:polls/) do |posts, match| if SiteSetting.poll_enabled posts.joins(:polls) diff --git a/spec/lib/plugin/instance_spec.rb b/spec/lib/plugin/instance_spec.rb index de983da141e..750226c8acf 100644 --- a/spec/lib/plugin/instance_spec.rb +++ b/spec/lib/plugin/instance_spec.rb @@ -75,14 +75,6 @@ RSpec.describe Plugin::Instance do # Serializer @plugin.add_to_serializer(:trout, :scales) { 1024 } - @plugin.add_to_serializer(:trout, :unconditional_scales, respect_plugin_enabled: false) do - 2048 - end - @plugin.add_to_serializer( - :trout, - :conditional_scales, - include_condition: -> { !!object.data&.[](:has_scales) }, - ) { 4096 } @serializer = TroutSerializer.new(@trout) @child_serializer = TroutJuniorSerializer.new(@trout) @@ -108,7 +100,6 @@ RSpec.describe Plugin::Instance do expect(@hello_count).to eq(1) expect(@serializer.scales).to eq(1024) expect(@serializer.include_scales?).to eq(false) - expect(@serializer.include_unconditional_scales?).to eq(true) expect(@serializer.name).to eq("a trout") expect(@child_serializer.scales).to eq(1024) @@ -116,25 +107,6 @@ RSpec.describe Plugin::Instance do expect(@child_serializer.name).to eq("a trout jr") end - it "can control the include_* implementation" do - @plugin.enabled = true - - expect(@serializer.scales).to eq(1024) - expect(@serializer.include_scales?).to eq(true) - - expect(@serializer.unconditional_scales).to eq(2048) - expect(@serializer.include_unconditional_scales?).to eq(true) - - expect(@serializer.include_conditional_scales?).to eq(false) - @trout.data = { has_scales: true } - expect(@serializer.include_conditional_scales?).to eq(true) - - @plugin.enabled = false - expect(@serializer.include_scales?).to eq(false) - expect(@serializer.include_unconditional_scales?).to eq(true) - expect(@serializer.include_conditional_scales?).to eq(false) - end - it "only returns HTML if enabled" do ctx = Trout.new ctx.data = "hello"