Revert "DEV: Improve `add_to_serializer` `include_*` options (#21073)" (#21219)

This reverts commit 4895e76ef7.

lib/plugin/instance.rb

@@ -109,27 +109,7 @@ class Plugin::Instance
 
   delegate :name, to: :metadata
 
-  def add_to_serializer(
+  def add_to_serializer(serializer, attr, define_include_method = true, &block)
-    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
-
     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
+            klass.public_send(:define_method, "include_#{attr}?") { plugin.enabled? }
-              next false if respect_plugin_enabled && !plugin.enabled?
-              next instance_exec(&include_condition) if include_condition
-              true
-            end
           end
         end
 

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(
+  add_to_serializer(:current_user, :can_chat) { true }
-    :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(
+  add_to_serializer(:current_user, :include_can_chat?) do
-    :current_user,
+    return @can_chat if defined?(@can_chat)
-    :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(
+    @can_chat = SiteSetting.chat_enabled && scope.can_chat?
-    :current_user,
+  end
-    :chat_sound,
-    include_condition: -> { include_has_chat_enabled? && object.user_option.chat_sound },
-  ) { object.user_option.chat_sound }
 
-  add_to_serializer(
+  add_to_serializer(:current_user, :has_chat_enabled) { true }
-    :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(
+  add_to_serializer(:current_user, :include_has_chat_enabled?) do
-    :current_user,
+    return @has_chat_enabled if defined?(@has_chat_enabled)
-    :needs_dm_retention_reminder,
+
-    include_condition: -> do
+    @has_chat_enabled = include_can_chat? && object.user_option.chat_enabled
-      include_has_chat_enabled? && !object.user_option.dismissed_dm_retention_reminder &&
+  end
-        !SiteSetting.chat_dm_retention_days.zero?
+
-    end,
+  add_to_serializer(:current_user, :chat_sound) { object.user_option.chat_sound }
-  ) { true }
+
+  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(
+  add_to_serializer(:current_user, :include_needs_channel_retention_reminder?) do
-    :current_user,
+    include_has_chat_enabled? && object.staff? &&
-    :chat_drafts,
+      !object.user_option.dismissed_channel_retention_reminder &&
-    include_condition: -> { include_has_chat_enabled? },
+      !SiteSetting.chat_channel_retention_days.zero?
-  ) do
+  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(
+  add_to_serializer(:user_option, :chat_sound) { object.chat_sound }
-    :user_option,
+
-    :chat_sound,
+  add_to_serializer(:user_option, :include_chat_sound?) { !object.chat_sound.blank? }
-    include_condition: -> { !object.chat_sound.blank? },
-  ) { object.chat_sound }
 
   add_to_serializer(:user_option, :only_chat_push_notifications) do
     object.only_chat_push_notifications

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(
+  add_to_serializer(:post, :include_polls?) { SiteSetting.poll_enabled && preloaded_polls.present? }
-    :post,
+
-    :polls_votes,
+  add_to_serializer(:post, :polls_votes, false) do
-    include_condition: -> do
-      scope.user&.id.present? && preloaded_polls.present? &&
-        preloaded_polls.any? { |p| p.has_voted?(scope.user) }
-    end,
-  ) 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)

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"