From 7c6681ba727b261de1accf3f0720fa0bd48e1592 Mon Sep 17 00:00:00 2001 From: Ivan Denisov Date: Sun, 2 Sep 2018 18:28:30 +0300 Subject: [PATCH 1/2] use current module's namespace for decorates_association --- lib/draper/decoratable.rb | 14 +++++---- lib/draper/decorated_association.rb | 2 +- lib/draper/decorator.rb | 7 ++++- lib/draper/factory.rb | 12 ++++---- spec/draper/decoratable_spec.rb | 6 ++++ spec/draper/decorated_association_spec.rb | 16 +++++----- spec/draper/decorator_spec.rb | 15 ++++++++++ spec/draper/factory_spec.rb | 36 +++++++++++------------ 8 files changed, 69 insertions(+), 39 deletions(-) diff --git a/lib/draper/decoratable.rb b/lib/draper/decoratable.rb index 5a3aee39..39aff8c8 100644 --- a/lib/draper/decoratable.rb +++ b/lib/draper/decoratable.rb @@ -15,12 +15,12 @@ module Decoratable # @param [Hash] options # see {Decorator#initialize} def decorate(options = {}) - decorator_class.decorate(self, options) + decorator_class(namespace: options.delete(:namespace)).decorate(self, options) end # (see ClassMethods#decorator_class) - def decorator_class - self.class.decorator_class + def decorator_class(namespace: nil) + self.class.decorator_class(namespace: namespace) end def decorator_class? @@ -55,7 +55,7 @@ module ClassMethods # @param [Hash] options # see {Decorator.decorate_collection}. def decorate(options = {}) - decorator_class.decorate_collection(all, options.reverse_merge(with: nil)) + decorator_class(namespace: options[:namespace]).decorate_collection(all, options.reverse_merge(with: nil)) end def decorator_class? @@ -68,9 +68,11 @@ def decorator_class? # `Product` maps to `ProductDecorator`). # # @return [Class] the inferred decorator class. - def decorator_class(called_on = self) + def decorator_class(called_on = self, namespace: nil) prefix = respond_to?(:model_name) ? model_name : name - decorator_name = "#{prefix}Decorator" + namespace = "#{namespace}::" if namespace.present? + + decorator_name = "#{namespace}#{prefix}Decorator" decorator_name_constant = decorator_name.safe_constantize return decorator_name_constant unless decorator_name_constant.nil? diff --git a/lib/draper/decorated_association.rb b/lib/draper/decorated_association.rb index 6bba855d..34b482ac 100644 --- a/lib/draper/decorated_association.rb +++ b/lib/draper/decorated_association.rb @@ -11,7 +11,7 @@ def initialize(owner, association, options) decorator_class = options[:with] context = options.fetch(:context, ->(context){ context }) - @factory = Draper::Factory.new(with: decorator_class, context: context) + @factory = Draper::Factory.new(with: decorator_class, context: context, namespace: owner.namespace) end def call diff --git a/lib/draper/decorator.rb b/lib/draper/decorator.rb index c029bb27..fb3c66b2 100644 --- a/lib/draper/decorator.rb +++ b/lib/draper/decorator.rb @@ -139,7 +139,8 @@ def self.decorates_associations(*associations) # extra data to be stored in the collection decorator. def self.decorate_collection(object, options = {}) options.assert_valid_keys(:with, :context) - collection_decorator_class.new(object, options.reverse_merge(with: self)) + options[:with] ||= self + collection_decorator_class.new(object, options) end # @return [Array] the list of decorators that have been applied to @@ -218,6 +219,10 @@ def attributes object.attributes.select {|attribute, _| respond_to?(attribute) } end + def namespace + self.class.to_s.deconstantize.presence + end + # ActiveModel compatibility delegate :to_param, :to_partial_path diff --git a/lib/draper/factory.rb b/lib/draper/factory.rb index c76422c4..27e1f071 100644 --- a/lib/draper/factory.rb +++ b/lib/draper/factory.rb @@ -10,8 +10,9 @@ class Factory # will be called each time {#decorate} is called and its return value # will be used as the context. def initialize(options = {}) - options.assert_valid_keys(:with, :context) + options.assert_valid_keys(:with, :context, :namespace) @decorator_class = options.delete(:with) + @namespace = options.delete(:namespace) @default_options = options end @@ -28,7 +29,7 @@ def initialize(options = {}) # @return [Decorator, CollectionDecorator] the decorated object. def decorate(object, options = {}) return nil if object.nil? - Worker.new(decorator_class, object).call(options.reverse_merge(default_options)) + Worker.new(decorator_class, object, @namespace).call(options.reverse_merge(default_options)) end private @@ -37,9 +38,10 @@ def decorate(object, options = {}) # @private class Worker - def initialize(decorator_class, object) + def initialize(decorator_class, object, namespace) @decorator_class = decorator_class @object = object + @namespace = namespace end def call(options) @@ -60,9 +62,9 @@ def decorator def object_decorator if collection? - ->(object, options) { object.decorator_class.decorate_collection(object, options.reverse_merge(with: nil))} + ->(object, options) { object.decorator_class(namespace: @namespace).decorate_collection(object, options.reverse_merge(with: nil))} else - ->(object, options) { object.decorate(options) } + ->(object, options) { object.decorate(options.merge(namespace: @namespace)) } end end diff --git a/spec/draper/decoratable_spec.rb b/spec/draper/decoratable_spec.rb index 8e673fa6..956f994c 100644 --- a/spec/draper/decoratable_spec.rb +++ b/spec/draper/decoratable_spec.rb @@ -197,6 +197,12 @@ module Draper end end + context "when namespace is passed explicitly" do + it "returns namespaced decorator class" do + expect(Product.decorator_class(namespace: Namespaced)).to be Namespaced::ProductDecorator + end + end + context "when the decorator contains name error" do it "throws an NameError" do # We imitate ActiveSupport::Autoload behavior here in order to cause lazy NameError exception raising diff --git a/spec/draper/decorated_association_spec.rb b/spec/draper/decorated_association_spec.rb index d68205bb..7d6645ad 100644 --- a/spec/draper/decorated_association_spec.rb +++ b/spec/draper/decorated_association_spec.rb @@ -15,14 +15,14 @@ module Draper it "creates a factory" do options = {with: Decorator, context: {foo: "bar"}} - expect(Factory).to receive(:new).with(options) - DecoratedAssociation.new(double, :association, options) + expect(Factory).to receive(:new).with(options.merge(namespace: nil)) + DecoratedAssociation.new(double(namespace: nil), :association, options) end describe ":with option" do it "defaults to nil" do - expect(Factory).to receive(:new).with(with: nil, context: anything()) - DecoratedAssociation.new(double, :association, {}) + expect(Factory).to receive(:new).with(with: nil, context: anything(), namespace: nil) + DecoratedAssociation.new(double(namespace: nil), :association, {}) end end @@ -31,7 +31,7 @@ module Draper expect(Factory).to receive(:new) do |options| options[:context].call(:anything) == :anything end - DecoratedAssociation.new(double, :association, {}) + DecoratedAssociation.new(double(namespace: nil), :association, {}) end end end @@ -43,7 +43,7 @@ module Draper associated = double owner_context = {foo: "bar"} object = double(association: associated) - owner = double(object: object, context: owner_context) + owner = double(object: object, context: owner_context, namespace: nil) decorated_association = DecoratedAssociation.new(owner, :association, {}) decorated = double @@ -54,7 +54,7 @@ module Draper it "memoizes" do factory = double allow(Factory).to receive_messages(new: factory) - owner = double(object: double(association: double), context: {}) + owner = double(object: double(association: double), context: {}, namespace: nil) decorated_association = DecoratedAssociation.new(owner, :association, {}) decorated = double @@ -69,7 +69,7 @@ module Draper allow(Factory).to receive_messages(new: factory) scoped = double object = double(association: double(applied_scope: scoped)) - owner = double(object: object, context: {}) + owner = double(object: object, context: {}, namespace: nil) decorated_association = DecoratedAssociation.new(owner, :association, scope: :applied_scope) decorated = double diff --git a/spec/draper/decorator_spec.rb b/spec/draper/decorator_spec.rb index 876a3dd1..668b68b4 100644 --- a/spec/draper/decorator_spec.rb +++ b/spec/draper/decorator_spec.rb @@ -471,6 +471,21 @@ module Draper end end + describe "#namespace" do + it "returns own module nesting" do + decorator = Namespaced::ProductDecorator.new(double) + expect(decorator.namespace).to eq("Namespaced") + end + + context "when class has no nesting" do + it "returns nil" do + ::TopLevelDecorator = Class.new(Draper::Decorator) + decorator = TopLevelDecorator.new(double) + expect(decorator.namespace).to eq(nil) + end + end + end + describe ".model_name" do it "delegates to the object class" do allow(Decorator).to receive(:object_class).and_return(double(model_name: :delegated)) diff --git a/spec/draper/factory_spec.rb b/spec/draper/factory_spec.rb index 9a9ce24a..f1c9cc77 100644 --- a/spec/draper/factory_spec.rb +++ b/spec/draper/factory_spec.rb @@ -34,7 +34,7 @@ module Draper factory = Factory.new object = double - expect(Factory::Worker).to receive(:new).with(anything(), object).and_return(->(*){}) + expect(Factory::Worker).to receive(:new).with(anything(), object, anything()).and_return(->(*){}) factory.decorate(object) end @@ -43,7 +43,7 @@ module Draper decorator_class = double factory = Factory.new(with: decorator_class) - expect(Factory::Worker).to receive(:new).with(decorator_class, anything()).and_return(->(*){}) + expect(Factory::Worker).to receive(:new).with(decorator_class, anything(), anything()).and_return(->(*){}) factory.decorate(double) end end @@ -52,7 +52,7 @@ module Draper it "passes nil to the worker" do factory = Factory.new - expect(Factory::Worker).to receive(:new).with(nil, anything()).and_return(->(*){}) + expect(Factory::Worker).to receive(:new).with(nil, anything(), anything()).and_return(->(*){}) factory.decorate(double) end end @@ -94,7 +94,7 @@ module Draper it "calls the decorator method" do object = double options = {foo: "bar"} - worker = Factory::Worker.new(double, object) + worker = Factory::Worker.new(double, object, nil) decorator = ->(*){} allow(worker).to receive(:decorator){ decorator } @@ -104,7 +104,7 @@ module Draper context "when the :context option is callable" do it "calls it" do - worker = Factory::Worker.new(double, double) + worker = Factory::Worker.new(double, double, nil) decorator = ->(*){} allow(worker).to receive_messages decorator: decorator context = {foo: "bar"} @@ -114,7 +114,7 @@ module Draper end it "receives arguments from the :context_args option" do - worker = Factory::Worker.new(double, double) + worker = Factory::Worker.new(double, double, nil) allow(worker).to receive_messages decorator: ->(*){} context = ->{} @@ -123,7 +123,7 @@ module Draper end it "wraps non-arrays passed to :context_args" do - worker = Factory::Worker.new(double, double) + worker = Factory::Worker.new(double, double, nil) allow(worker).to receive_messages decorator: ->(*){} context = ->{} hash = {foo: "bar"} @@ -135,7 +135,7 @@ module Draper context "when the :context option is not callable" do it "doesn't call it" do - worker = Factory::Worker.new(double, double) + worker = Factory::Worker.new(double, double, nil) decorator = ->(*){} allow(worker).to receive_messages decorator: decorator context = {foo: "bar"} @@ -146,7 +146,7 @@ module Draper end it "does not pass the :context_args option to the decorator" do - worker = Factory::Worker.new(double, double) + worker = Factory::Worker.new(double, double, nil) decorator = ->(*){} allow(worker).to receive_messages decorator: decorator @@ -160,7 +160,7 @@ module Draper context "when decorator_class is specified" do it "returns the .decorate method from the decorator" do decorator_class = Class.new(Decorator) - worker = Factory::Worker.new(decorator_class, double) + worker = Factory::Worker.new(decorator_class, double, nil) expect(worker.decorator).to eq decorator_class.method(:decorate) end @@ -171,9 +171,9 @@ module Draper it "returns the object's #decorate method" do object = double options = {foo: "bar"} - worker = Factory::Worker.new(nil, object) + worker = Factory::Worker.new(nil, object, nil) - expect(object).to receive(:decorate).with(options).and_return(:decorated) + expect(object).to receive(:decorate).with(options.merge(namespace: nil)).and_return(:decorated) expect(worker.decorator.call(object, options)).to be :decorated end end @@ -181,7 +181,7 @@ module Draper context "and the object is not decoratable" do it "raises an error" do object = double - worker = Factory::Worker.new(nil, object) + worker = Factory::Worker.new(nil, object, nil) expect{worker.decorator}.to raise_error UninferrableDecoratorError end @@ -193,7 +193,7 @@ module Draper object = Struct.new(:stuff).new("things") decorator_class = Class.new(Decorator) - worker = Factory::Worker.new(decorator_class, object) + worker = Factory::Worker.new(decorator_class, object, nil) expect(worker.decorator).to eq decorator_class.method(:decorate) end @@ -204,7 +204,7 @@ module Draper context "when decorator_class is a CollectionDecorator" do it "returns the .decorate method from the collection decorator" do decorator_class = Class.new(CollectionDecorator) - worker = Factory::Worker.new(decorator_class, []) + worker = Factory::Worker.new(decorator_class, [], nil) expect(worker.decorator).to eq decorator_class.method(:decorate) end @@ -213,7 +213,7 @@ module Draper context "when decorator_class is a Decorator" do it "returns the .decorate_collection method from the decorator" do decorator_class = Class.new(Decorator) - worker = Factory::Worker.new(decorator_class, []) + worker = Factory::Worker.new(decorator_class, [], nil) expect(worker.decorator).to eq decorator_class.method(:decorate_collection) end @@ -226,7 +226,7 @@ module Draper decorator_class = Class.new(Decorator) allow(object).to receive(:decorator_class){ decorator_class } allow(object).to receive(:decorate){ nil } - worker = Factory::Worker.new(nil, object) + worker = Factory::Worker.new(nil, object, nil) expect(decorator_class).to receive(:decorate_collection).with(object, foo: "bar", with: nil).and_return(:decorated) expect(worker.decorator.call(object, foo: "bar")).to be :decorated @@ -235,7 +235,7 @@ module Draper context "and the object is not decoratable" do it "returns the .decorate method from CollectionDecorator" do - worker = Factory::Worker.new(nil, []) + worker = Factory::Worker.new(nil, [], nil) expect(worker.decorator).to eq CollectionDecorator.method(:decorate) end From 226ed0f258b640b145ba6ce6984f7f467891d9fe Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Sun, 14 Oct 2018 02:52:58 +0200 Subject: [PATCH 2/2] Fixes for new :namespace option --- lib/draper/collection_decorator.rb | 9 ++++++++- lib/draper/decorated_association.rb | 5 +++-- lib/draper/decorator.rb | 5 +++-- lib/draper/factory.rb | 10 +++++----- spec/draper/decorator_spec.rb | 4 ++-- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/lib/draper/collection_decorator.rb b/lib/draper/collection_decorator.rb index 35e0b1b4..2ed46647 100644 --- a/lib/draper/collection_decorator.rb +++ b/lib/draper/collection_decorator.rb @@ -16,6 +16,10 @@ class CollectionDecorator # to each item's decorator. attr_accessor :context + # @return [String, nil] decorator namespace to be used to instantiate decorator, and passed + # to each item's decorator. + attr_accessor :namespace + array_methods = Array.instance_methods - Object.instance_methods delegate :==, :as_json, *array_methods, to: :decorated_collection @@ -27,11 +31,14 @@ class CollectionDecorator # @option options [Hash] :context ({}) # extra data to be stored in the collection decorator and used in # user-defined methods, and passed to each item's decorator. + # @option options [String, nil] :namespace (nil) + # decorator namespace def initialize(object, options = {}) - options.assert_valid_keys(:with, :context) + options.assert_valid_keys(:with, :context, :namespace) @object = object @decorator_class = options[:with] @context = options.fetch(:context, {}) + @namespace = options.delete(:namespace) end class << self diff --git a/lib/draper/decorated_association.rb b/lib/draper/decorated_association.rb index 34b482ac..cc1ff5a5 100644 --- a/lib/draper/decorated_association.rb +++ b/lib/draper/decorated_association.rb @@ -2,7 +2,7 @@ module Draper # @private class DecoratedAssociation def initialize(owner, association, options) - options.assert_valid_keys(:with, :scope, :context) + options.assert_valid_keys(:with, :scope, :context, :namespace) @owner = owner @association = association @@ -11,7 +11,8 @@ def initialize(owner, association, options) decorator_class = options[:with] context = options.fetch(:context, ->(context){ context }) - @factory = Draper::Factory.new(with: decorator_class, context: context, namespace: owner.namespace) + namespace = options[:namespace] || owner.namespace + @factory = Draper::Factory.new(with: decorator_class, context: context, namespace: namespace) end def call diff --git a/lib/draper/decorator.rb b/lib/draper/decorator.rb index fb3c66b2..b714a3cb 100644 --- a/lib/draper/decorator.rb +++ b/lib/draper/decorator.rb @@ -104,7 +104,7 @@ def self.decorates_finders # context and should return a new context hash for the association. # @return [void] def self.decorates_association(association, options = {}) - options.assert_valid_keys(:with, :scope, :context) + options.assert_valid_keys(:with, :scope, :context, :namespace) define_method(association) do decorated_associations[association] ||= Draper::DecoratedAssociation.new(self, association, options) decorated_associations[association].call @@ -138,7 +138,7 @@ def self.decorates_associations(*associations) # @option options [Hash] :context # extra data to be stored in the collection decorator. def self.decorate_collection(object, options = {}) - options.assert_valid_keys(:with, :context) + options.assert_valid_keys(:with, :context, :namespace) options[:with] ||= self collection_decorator_class.new(object, options) end @@ -255,6 +255,7 @@ def self.object_class_name def self.inferred_object_class name = object_class_name + name = name.split('::').last unless name.nil? name_constant = name&.safe_constantize return name_constant unless name_constant.nil? diff --git a/lib/draper/factory.rb b/lib/draper/factory.rb index 27e1f071..7ac29578 100644 --- a/lib/draper/factory.rb +++ b/lib/draper/factory.rb @@ -29,12 +29,12 @@ def initialize(options = {}) # @return [Decorator, CollectionDecorator] the decorated object. def decorate(object, options = {}) return nil if object.nil? - Worker.new(decorator_class, object, @namespace).call(options.reverse_merge(default_options)) + Worker.new(decorator_class, object, namespace).call(options.reverse_merge(default_options)) end private - attr_reader :decorator_class, :default_options + attr_reader :decorator_class, :namespace, :default_options # @private class Worker @@ -58,13 +58,13 @@ def decorator private - attr_reader :decorator_class, :object + attr_reader :decorator_class, :object, :namespace def object_decorator if collection? - ->(object, options) { object.decorator_class(namespace: @namespace).decorate_collection(object, options.reverse_merge(with: nil))} + ->(object, options) { object.decorator_class(namespace: namespace).decorate_collection(object, options.reverse_merge(with: nil))} else - ->(object, options) { object.decorate(options.merge(namespace: @namespace)) } + ->(object, options) { object.decorate(options.merge(namespace: namespace)) } end end diff --git a/spec/draper/decorator_spec.rb b/spec/draper/decorator_spec.rb index 668b68b4..ae061eb1 100644 --- a/spec/draper/decorator_spec.rb +++ b/spec/draper/decorator_spec.rb @@ -105,7 +105,7 @@ module Draper before { allow(CollectionDecorator).to receive(:new) } it "does not raise error on valid options" do - valid_options = {with: OtherDecorator, context: {}} + valid_options = {with: OtherDecorator, context: {}, namespace: nil} expect{Decorator.decorate_collection([], valid_options)}.not_to raise_error end @@ -197,7 +197,7 @@ module Draper end it "infers the object class for namespaced decorators" do - expect(Namespaced::ProductDecorator.object_class).to be Namespaced::Product + expect(Namespaced::ProductDecorator.object_class).to be Product end context "when an unrelated NameError is thrown" do