From bd5fa1737d42a958bb5cda35ae5bab2464d4e084 Mon Sep 17 00:00:00 2001
From: Sam Saffron <sam.saffron@gmail.com>
Date: Tue, 27 Aug 2019 18:21:53 +1000
Subject: [PATCH] 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
---
 lib/plugin/instance.rb                  | 19 ++++++++------
 spec/components/plugin/instance_spec.rb | 34 ++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb
index 3c45aa7a893..94037ff7e35 100644
--- a/lib/plugin/instance.rb
+++ b/lib/plugin/instance.rb
@@ -94,18 +94,21 @@ class Plugin::Instance
 
   def add_to_serializer(serializer, attr, define_include_method = true, &block)
     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_")
-        klass.attributes(attr)
+      # we have to work through descendants cause serializers may already be baked and cached
+      ([base] + base.descendants).each do |klass|
+        unless attr.to_s.start_with?("include_")
+          klass.attributes(attr)
 
-        if define_include_method
-          # Don't include serialized methods if the plugin is disabled
-          klass.public_send(:define_method, "include_#{attr}?") { plugin.enabled? }
+          if define_include_method
+            # Don't include serialized methods if the plugin is disabled
+            klass.public_send(:define_method, "include_#{attr}?") { plugin.enabled? }
+          end
         end
-      end
 
-      klass.public_send(:define_method, attr, &block)
+        klass.public_send(:define_method, attr, &block)
+      end
     end
   end
 
diff --git a/spec/components/plugin/instance_spec.rb b/spec/components/plugin/instance_spec.rb
index 916fef5e8ba..76ecc02cacf 100644
--- a/spec/components/plugin/instance_spec.rb
+++ b/spec/components/plugin/instance_spec.rb
@@ -34,7 +34,25 @@ describe Plugin::Instance do
     context "with a plugin that extends things" do
 
       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
         attr_accessor :enabled
@@ -47,6 +65,12 @@ describe Plugin::Instance do
         @plugin = TroutPlugin.new
         @trout = Trout.new
 
+        poison = TroutSerializer.new(@trout)
+        poison.attributes
+
+        poison = TroutJuniorSerializer.new(@trout)
+        poison.attributes
+
         # New method
         @plugin.add_to_class(:trout, :status?) { "evil" }
 
@@ -57,7 +81,9 @@ describe Plugin::Instance do
 
         # Serializer
         @plugin.add_to_serializer(:trout, :scales) { 1024 }
+
         @serializer = TroutSerializer.new(@trout)
+        @child_serializer = TroutJuniorSerializer.new(@trout)
       end
 
       after do
@@ -74,6 +100,8 @@ describe Plugin::Instance do
         expect(@serializer.scales).to eq(1024)
         expect(@serializer.include_scales?).to eq(true)
 
+        expect(@child_serializer.attributes[:scales]).to eq(1024)
+
         # When a plugin is disabled
         @plugin.enabled = false
         expect(@trout.status?).to eq(nil)
@@ -81,7 +109,11 @@ 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.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