mirror of
https://github.com/discourse/discourse.git
synced 2025-02-25 18:55:32 -06:00
FIX: add_to_serializer not correctly accounting for inheritance chains
This is a very long standing bug we had, if a plugin attempted to amend a serializer core was not "correcting" the situation for all descendant classes this often only showed up in production cause production eager loads serializers prior to plugins amending them. This is a critical fix for various plugins
This commit is contained in:
parent
a3cd73ef27
commit
bd5fa1737d
@ -94,18 +94,21 @@ class Plugin::Instance
|
|||||||
|
|
||||||
def add_to_serializer(serializer, attr, define_include_method = true, &block)
|
def add_to_serializer(serializer, attr, define_include_method = true, &block)
|
||||||
reloadable_patch do |plugin|
|
reloadable_patch do |plugin|
|
||||||
klass = "#{serializer.to_s.classify}Serializer".constantize rescue "#{serializer.to_s}Serializer".constantize
|
base = "#{serializer.to_s.classify}Serializer".constantize rescue "#{serializer.to_s}Serializer".constantize
|
||||||
|
|
||||||
unless attr.to_s.start_with?("include_")
|
# we have to work through descendants cause serializers may already be baked and cached
|
||||||
klass.attributes(attr)
|
([base] + base.descendants).each do |klass|
|
||||||
|
unless attr.to_s.start_with?("include_")
|
||||||
|
klass.attributes(attr)
|
||||||
|
|
||||||
if define_include_method
|
if define_include_method
|
||||||
# Don't include serialized methods if the plugin is disabled
|
# Don't include serialized methods if the plugin is disabled
|
||||||
klass.public_send(:define_method, "include_#{attr}?") { plugin.enabled? }
|
klass.public_send(:define_method, "include_#{attr}?") { plugin.enabled? }
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
|
||||||
|
|
||||||
klass.public_send(:define_method, attr, &block)
|
klass.public_send(:define_method, attr, &block)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -34,7 +34,25 @@ describe Plugin::Instance do
|
|||||||
context "with a plugin that extends things" do
|
context "with a plugin that extends things" do
|
||||||
|
|
||||||
class Trout; end
|
class Trout; end
|
||||||
class TroutSerializer < ApplicationSerializer; end
|
class TroutSerializer < ApplicationSerializer
|
||||||
|
attribute :name
|
||||||
|
|
||||||
|
def name
|
||||||
|
"a trout"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
class TroutJuniorSerializer < TroutSerializer
|
||||||
|
|
||||||
|
attribute :i_am_child
|
||||||
|
|
||||||
|
def name
|
||||||
|
"a trout jr"
|
||||||
|
end
|
||||||
|
|
||||||
|
def i_am_child
|
||||||
|
true
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
class TroutPlugin < Plugin::Instance
|
class TroutPlugin < Plugin::Instance
|
||||||
attr_accessor :enabled
|
attr_accessor :enabled
|
||||||
@ -47,6 +65,12 @@ describe Plugin::Instance do
|
|||||||
@plugin = TroutPlugin.new
|
@plugin = TroutPlugin.new
|
||||||
@trout = Trout.new
|
@trout = Trout.new
|
||||||
|
|
||||||
|
poison = TroutSerializer.new(@trout)
|
||||||
|
poison.attributes
|
||||||
|
|
||||||
|
poison = TroutJuniorSerializer.new(@trout)
|
||||||
|
poison.attributes
|
||||||
|
|
||||||
# New method
|
# New method
|
||||||
@plugin.add_to_class(:trout, :status?) { "evil" }
|
@plugin.add_to_class(:trout, :status?) { "evil" }
|
||||||
|
|
||||||
@ -57,7 +81,9 @@ describe Plugin::Instance do
|
|||||||
|
|
||||||
# Serializer
|
# Serializer
|
||||||
@plugin.add_to_serializer(:trout, :scales) { 1024 }
|
@plugin.add_to_serializer(:trout, :scales) { 1024 }
|
||||||
|
|
||||||
@serializer = TroutSerializer.new(@trout)
|
@serializer = TroutSerializer.new(@trout)
|
||||||
|
@child_serializer = TroutJuniorSerializer.new(@trout)
|
||||||
end
|
end
|
||||||
|
|
||||||
after do
|
after do
|
||||||
@ -74,6 +100,8 @@ describe Plugin::Instance do
|
|||||||
expect(@serializer.scales).to eq(1024)
|
expect(@serializer.scales).to eq(1024)
|
||||||
expect(@serializer.include_scales?).to eq(true)
|
expect(@serializer.include_scales?).to eq(true)
|
||||||
|
|
||||||
|
expect(@child_serializer.attributes[:scales]).to eq(1024)
|
||||||
|
|
||||||
# When a plugin is disabled
|
# When a plugin is disabled
|
||||||
@plugin.enabled = false
|
@plugin.enabled = false
|
||||||
expect(@trout.status?).to eq(nil)
|
expect(@trout.status?).to eq(nil)
|
||||||
@ -81,7 +109,11 @@ describe Plugin::Instance do
|
|||||||
expect(@hello_count).to eq(1)
|
expect(@hello_count).to eq(1)
|
||||||
expect(@serializer.scales).to eq(1024)
|
expect(@serializer.scales).to eq(1024)
|
||||||
expect(@serializer.include_scales?).to eq(false)
|
expect(@serializer.include_scales?).to eq(false)
|
||||||
|
expect(@serializer.name).to eq("a trout")
|
||||||
|
|
||||||
|
expect(@child_serializer.scales).to eq(1024)
|
||||||
|
expect(@child_serializer.include_scales?).to eq(false)
|
||||||
|
expect(@child_serializer.name).to eq("a trout jr")
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
Loading…
Reference in New Issue
Block a user