Skip to content

Conversation

@jorroll
Copy link
Contributor

@jorroll jorroll commented Sep 24, 2017

I'm not sure if this is a bug or intended behavior, but currently: if an ActiveNode class A is inherited by B, and then an ActiveNode property is subsequently declared on A, B will not have that property (i.e. properties are only be transferred at inheritance).

This PR makes it so that every property declared on a parent is also present on the children (unless undef_property was explicitly called on a child).

In achieving this, I plucked ActiveSupport/core_ext/class/subclasses and added the subclasses method to Class.

This pull introduces/changes:

  • properties declared on parent classes after inheritance has taken place are properly inherited by subclasses
  • all classes now have subclasses method

Example

class Person
  include Neo4j::ActiveNode

  class Superhero < Person
  end

  property :name
end

hero = Person::Superhero.new
hero.name = 'John'
hero.name
#=> 'John'

Pings:
@cheerfulstoic
@subvertallchris

@jorroll
Copy link
Contributor Author

jorroll commented Sep 24, 2017

This PR has a minor conflict with #1419

@jorroll jorroll changed the title Fix / upd: properties declared on parent after inheritance takes place are now passed to child Fix / upd: properties declared on parent after inheritance takes place are now passed to children Sep 24, 2017
@codecov-io
Copy link

codecov-io commented Sep 24, 2017

Codecov Report

Merging #1428 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1428      +/-   ##
==========================================
+ Coverage   96.81%   96.81%   +<.01%     
==========================================
  Files         205      205              
  Lines       12511    12539      +28     
==========================================
+ Hits        12112    12140      +28     
  Misses        399      399
Impacted Files Coverage Δ
lib/neo4j/shared/enum.rb 98.66% <100%> (+0.03%) ⬆️
lib/neo4j/shared/property.rb 97.54% <100%> (+0.06%) ⬆️
spec/e2e/inheritance_spec.rb 100% <100%> (ø) ⬆️
lib/neo4j.rb 97.95% <100%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9409732...273e2e3. Read the comment docs.

@jorroll
Copy link
Contributor Author

jorroll commented Sep 24, 2017

I suppose this update has the capacity to break things... If it is something you're interested in accepting, might be a good idea to release it along with #1419

@cheerfulstoic
Copy link
Contributor

Thanks so much for this! I would say this falls into the bugfix camp, but I'll release it in 9.0.0 just to make sure. I resolved the conflict and now waiting for Travis to finish before merging (though I might not get back to it this evening)

@jorroll
Copy link
Contributor Author

jorroll commented Sep 25, 2017

awesome

@cheerfulstoic cheerfulstoic merged commit a81410b into neo4jrb:master Sep 25, 2017
@jorroll jorroll deleted the property_inheritence_fix branch September 25, 2017 23:27
@cheerfulstoic
Copy link
Contributor

9.0.0 released

@jorroll
Copy link
Contributor Author

jorroll commented Sep 26, 2017

Whoo hoo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants