Merge pull request #455 from matestack/20200909_make-properties-inheritable

[FEATURE] make properties inheritable

Author: NdotHenning

Gemfile

@@ -25,12 +25,12 @@ group :development, :test do
   gem 'simplecov', require: false, group: :test
   gem 'byebug'
   gem 'webmock'
-  gem 'pry-rails'
-  gem 'pry-byebug'
   gem 'turbolinks'
 end
 
 group :test do
+  gem 'pry-rails'
+  gem 'pry-byebug'
   gem "generator_spec"
   gem "rspec-retry" # repeating flaky tests
   gem "rspec-wait", "~> 0.0.9"

app/lib/matestack/ui/core/html_attributes.rb

@@ -19,6 +19,7 @@ def self.included(base)
   module ClassMethods
 
     def inherited(subclass)
+      super
       subclass.html_attributes *self.allowed_html_attributes
     end
 

app/lib/matestack/ui/core/properties.rb

@@ -20,18 +20,51 @@ def initialize(model=nil, options={})
       options = model.dup if options.empty? && model.is_a?(Hash)
       required_hooks(options)
       optional_hooks(options)
+      set_values(options)
       super
     end
   end
 
   module ClassMethods
+
+    def inherited(subclass)
+      super
+      subclass.property_keys *self.all_property_keys
+    end
+    
+    # contains all property keys, or property hashes regardless of wheter they are optional or required
+    # used in order to initialize values of properties correctly for subclasses and make properties inheritable
+    def all_property_keys
+      @all_properties.flatten!&.uniq! if defined? @all_properties
+      @all_properties ||= []
+    end
+
+    def created_properties
+      @created_properties ||= []
+    end
+
+    # add property keys to array containing all property keys
+    def property_keys(*attributes)
+      attributes.each do |attribute|
+        if attribute.is_a?(Hash)
+          attribute.each do |temp|
+            all_property_keys.push({ "#{temp.first}": temp.last})
+          end
+        else
+          all_property_keys.push(attribute) 
+        end
+      end
+    end
+
     # define optinoal properties for custom components with `optional :foo, :bar`
     def optional(*properties)
+      property_keys *properties
       add_properties_to_list(optional_properties, properties)
     end
 
     # define required properties for custom components with `requires :title, :foo, :bar`
     def requires(*properties)
+      property_keys *properties
       add_properties_to_list(requires_properties, properties)
     end
 
@@ -47,7 +80,7 @@ def add_properties_to_list(list, properties)
 
     # array of properties created from the component
     def optional_properties
-      @component_properties ||= []
+      @optional_properties ||= []
     end
 
     # array of required properties from the component
@@ -58,30 +91,50 @@ def requires_properties
 
   def optional_hooks(options)
     self.class.optional_properties.compact.each do |prop|
-      if prop.is_a? Array
-        hash = prop.flatten
-        options[hash.last[:as]] = options[hash.first]
-        prop = hash.last[:as]
-      end
-      raise PropertyOverwritingExistingMethodException, "Optional property #{prop} would overwrite already defined instance method for #{self.class}" if self.respond_to? prop
-      send(:define_singleton_method, prop) do 
-        options[prop]
-      end
+      prop = extract_property_key(options, prop)
+    if self.respond_to?(prop) && !self.class.created_properties.include?(prop)
+      raise PropertyOverwritingExistingMethodException, "Optional property \"#{prop}\" would overwrite already defined instance method for #{self.class}" if self.respond_to? prop
+    end
+    define_getter_and_setter_for_property(prop)
     end
   end
 
   def required_hooks(options)
     self.class.requires_properties.compact.each do |prop|
-      if prop.is_a? Array
-        hash = prop.flatten
-        options[hash.last[:as]] = options[hash.first]
-        prop = hash.last[:as]
-      end
+      prop = extract_property_key(options, prop)
       raise PropertyMissingException, "Required property #{prop} is missing for #{self.class}" if options[prop].nil?
-      raise PropertyOverwritingExistingMethodException, "Required property #{prop} would overwrite already defined instance method for #{self.class}" if self.respond_to? prop
-      send(:define_singleton_method, prop) do
-        options[prop]
+      if self.respond_to?(prop) && !self.class.created_properties.include?(prop)
+        raise PropertyOverwritingExistingMethodException, "Required property \"#{prop}\" would overwrite already defined instance method for #{self.class}" if self.respond_to? prop
       end
+      define_getter_and_setter_for_property(prop)
+    end
+  end
+
+  def set_values(options)
+    self.class.all_property_keys.compact.each do |prop|
+      value_key = prop
+      prop = extract_property_key(options, prop)
+      send(:"#{prop}=", options[prop])
+    end
+  end
+
+  # returns property key and sets alias if hash with as option is given
+  def extract_property_key(options, prop)
+    if prop.is_a?(Array) || prop.is_a?(Hash)
+      hash = prop.flatten
+      options[hash.last[:as]] = options[hash.first]
+      prop = hash.last[:as]
+    end
+    prop
+  end
+
+  def define_getter_and_setter_for_property(prop)
+    self.class.created_properties.push(prop)
+    self.class.send(:define_method, prop) do 
+      self.instance_variable_get(:"@#{prop}")
+    end
+    self.class.send(:define_method, :"#{prop}=") do |value|
+      self.instance_variable_set(:"@#{prop}", value)
     end
   end
 

spec/dummy/app/matestack/components/fancy/card.rb

@@ -1,4 +1,5 @@
 class Components::Fancy::Card < Matestack::Ui::StaticComponent
 
+  optional :foobar
 
 end

spec/test/components/base/properties_spec.rb

@@ -94,7 +94,7 @@ def response
 
     visit '/example'
     expect(page).to have_content(Matestack::Ui::Core::Properties::PropertyOverwritingExistingMethodException.to_s)
-    expect(page).to have_content('Required property response would overwrite already defined instance method for TempPropertyComponent')
+    expect(page).to have_content('Required property "response" would overwrite already defined instance method for TempPropertyComponent')
   end
 
   it 'should raise exception if optional property overwrites existing method' do
@@ -113,7 +113,7 @@ def response
 
     visit '/example'
     expect(page).to have_content(Matestack::Ui::Core::Properties::PropertyOverwritingExistingMethodException.to_s)
-    expect(page).to have_content('Optional property response would overwrite already defined instance method for TempOptionalPropertyComponent')
+    expect(page).to have_content('Optional property "response" would overwrite already defined instance method for TempOptionalPropertyComponent')
   end
 
   it 'should create instance method with given alias name for required properties' do
@@ -170,6 +170,7 @@ def prepare
         @bar = desc
       end
       def response
+        plain @foo
         paragraph text: @foo
         paragraph text: @bar
       end
@@ -232,4 +233,25 @@ def other_slot
     expect(stripped(static_output)).to include(stripped(expected_static_output))
   end
 
+  it 'should be inheritable' do
+    class Component < Matestack::Ui::Component
+      optional :foobar, response: { as: :test }
+      def response
+      end
+    end
+    class AnotherComponent < Component
+      optional :custom
+    end
+    component = Component.new(foobar: 'Foobar', response: 'Response')
+    another_component = AnotherComponent.new(custom: 'hi', foobar: 'foobar', response: 'response')
+    expect(component.respond_to? :foobar).to be(true)
+    expect(component.respond_to? :test).to be(true)
+    expect(another_component.respond_to? :custom).to be(true)
+    expect(another_component.custom).to eq('hi')
+    expect(another_component.respond_to? :foobar).to be(true)
+    expect(another_component.foobar).to eq('foobar')
+    expect(another_component.respond_to? :test).to be(true)
+    expect(another_component.test).to eq('response')
+  end
+
 end