Ensure multiple extra_attrs without blocks work correctly

Two pieces of context needed:

First, there's a bunch of logic around serializer attributes:
subclassing resources, explicit serializers, defaults, overrides, etc.
And all of this gets dicey considering it all wraps and hacks an
external library, `jsonapi-serializable`.  So we're pretty dumb about
saying "any time we're doing something that can affect serialization,
re-configure everything".

Second, we recently accommodated the scenario where a parent resource
class guards an attribute, and a subclass removes that guard. The logic
added says "if this was previously guarded, remove the guard - as we're
adding the property again, the guard will get added again if it needs to
be".

These two scenarios conflicted. If you had two `extra_attribute` calls,
the second would cause the first to be re-applied as part of the
"reconfigure everything" approach. And when re-applied, we remove the
prior guard...expecting it to be added again as part of the idempotent
nature of the call.

This worked as expected if you passed a block, because you'd end up back
[here](https://github.com/graphiti-api/graphiti/blob/master/lib/graphiti/util/serializer_attributes.rb#L20).
But if you didn't pass a block, you'd end up [here](https://github.com/graphiti-api/graphiti/blob/master/lib/graphiti/util/serializer_attributes.rb#L24),
with the logic being "the only thing we need to do here is add the guard
back.

This was dumb! If you're going to do the "reconfigure everything"
approach, you need to ensure you're following the same codepath - not
this special codepath that was added. This had the adverse effect that
no guard was re-added (for extra attributes, the conditional [is added
in a different way](https://github.com/graphiti-api/graphiti/blob/master/lib/graphiti/extensions/extra_attribute.rb#L43) than a normal guard).

So the fix is to ensure we always follow the same codepath, which will
now correctly re-apply the guard. And added tests for the scenario where
multiple `extra_attributes` are added with and without blocks.

Finally, I'm not so sure about the wisdom of "reconfigure everything".
This was added 3 years ago as part of the major refactor that replaced
`jsonapi_suite` and became the `graphiti` we know and love today. It's
possible that the scenarios we were worried about are no longer
relevant, and this is more of a hindrance than a help. For now, simplest
fix.

Author: Lee Richmond

lib/graphiti/util/serializer_attributes.rb

@@ -16,13 +16,11 @@ def apply
 
         if @name == :id
           @serializer.id(&proc)
-        elsif @attr[:proc]
+        elsif @attr[:proc] ||
+            !previously_applied? ||
+            previously_applied_via_resource?
           @serializer.send(_method, @name, serializer_options, &proc)
-        elsif @serializer.attribute_blocks[@name].nil?
-          @serializer.send(_method, @name, serializer_options, &proc)
-        elsif @serializer.send(applied_method).include?(@name)
-          @serializer.field_condition_blocks[@name] = guard if guard?
-        else
+        else # Previously applied via explicit serializer, so wrap it
           inner = @serializer.attribute_blocks.delete(@name)
           wrapped = wrap_proc(inner)
           @serializer.send(_method, @name, serializer_options, &wrapped)
@@ -34,6 +32,14 @@ def apply
 
       private
 
+      def previously_applied?
+        @serializer.attribute_blocks[@name].present?
+      end
+
+      def previously_applied_via_resource?
+        @serializer.send(applied_method).include?(@name)
+      end
+
       def previously_guarded?
         @serializer.field_condition_blocks[@name]
       end

lib/graphiti/version.rb

@@ -1,3 +1,3 @@
 module Graphiti
-  VERSION = "1.2.37"
+  VERSION = "1.2.38"
 end

spec/extra_fields_spec.rb

@@ -29,6 +29,64 @@ def attributes
       .to match_array(%w[first_name last_name age stack_ranking])
   end
 
+  context "when multiple extra attributes" do
+    context "added with blocks" do
+      before do
+        resource.extra_attribute :a, :string do
+          "a"
+        end
+        resource.extra_attribute :b, :string do
+          "b"
+        end
+      end
+
+      context "when not requested" do
+        it "does not render them" do
+          render
+          expect(attributes.keys).to match_array(%w[first_name last_name age])
+        end
+      end
+
+      context "when one is requested" do
+        before do
+          params[:extra_fields] = {employees: "b"}
+        end
+
+        it "is rendered" do
+          render
+          expect(attributes.keys).to match_array(%w[first_name last_name age b])
+        end
+      end
+    end
+
+    context "added without blocks" do
+      before do
+        allow_any_instance_of(PORO::Employee).to receive(:a) { "a" }
+        allow_any_instance_of(PORO::Employee).to receive(:b) { "b" }
+        resource.extra_attribute :a, :string
+        resource.extra_attribute :b, :string
+      end
+
+      context "when not requested" do
+        it "does not render them" do
+          render
+          expect(attributes.keys).to match_array(%w[first_name last_name age])
+        end
+      end
+
+      context "when one is requested" do
+        before do
+          params[:extra_fields] = {employees: "b"}
+        end
+
+        it "is rendered" do
+          render
+          expect(attributes.keys).to match_array(%w[first_name last_name age b])
+        end
+      end
+    end
+  end
+
   context "when altering scope based on extra attrs" do
     context "when the extra attr exists" do
       before do

spec/serialization_spec.rb

@@ -871,6 +871,7 @@ def admin?(object, attribute)
 
         context "without a guard" do
           before do
+            allow_any_instance_of(PORO::Employee).to receive(:foo) { "bar" }
             resource.attribute :foo, :string
           end
 
@@ -895,6 +896,7 @@ def admin?(object, attribute)
 
         context "with a guard" do
           before do
+            allow_any_instance_of(PORO::Employee).to receive(:foo) { "bar" }
             resource.class_eval do
               attribute :foo, :string, readable: :overriden?