-
Notifications
You must be signed in to change notification settings - Fork 274
Add: has_one(chainable: true) option #1422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -463,13 +463,14 @@ def define_has_one_id_methods(name) | |
| end | ||
| end | ||
|
|
||
| def define_has_one_getter(name) | ||
| def define_has_one_getter(name) # rubocop:disable Metrics/PerceivedComplexity | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rubocop was failing this with PerceivedComplexity as too high. I didn't know what to do with that, so I suppressed the warning to make it pass 😕 ...
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could probably just do with an extraction of a method. I'll fix it and link to the commit here
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Meh, not the best: 1122ee7 |
||
| define_method(name) do |node = nil, rel = nil, options = {}| | ||
| options, node = node, nil if node.is_a?(Hash) | ||
|
|
||
| # Return all results if a variable-length relationship length was given | ||
| association_proxy = association_proxy(name, {node: node, rel: rel}.merge!(options)) | ||
| if options[:rel_length] && !options[:rel_length].is_a?(Integer) | ||
|
|
||
| # Return all results if options[:chainable] == true or a variable-length relationship length was given | ||
| if options[:chainable] || (options[:rel_length] && !options[:rel_length].is_a?(Integer)) | ||
| association_proxy | ||
| else | ||
| target_class = self.class.send(:association_target_class, name) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,12 @@ | |
| it 'returns a nil object' do | ||
| expect(unsaved_node.parent).to eq nil | ||
| end | ||
|
|
||
| describe 'with chainable: true option' do | ||
| it 'returns an empty association proxy object' do | ||
| expect(unsaved_node.parent(chainable: true).inspect).to eq '#<AssociationProxy HasOneB#parent []>' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this would be a bit better as: I can make that change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| end | ||
| end | ||
| end | ||
|
|
||
| it 'find the nodes via the has_one accessor' do | ||
|
|
@@ -38,6 +44,22 @@ | |
| expect(a.children.to_a).to match_array([b, c]) | ||
| end | ||
|
|
||
| describe 'with chainable: true option' do | ||
| it 'find the nodes via the has_one accessor' do | ||
| a = HasOneA.create(name: 'a') | ||
| b = HasOneB.create(name: 'b') | ||
| c = HasOneB.create(name: 'c') | ||
| a.children << b | ||
| a.children << c | ||
|
|
||
| expect(c.parent(chainable: true).inspect).to include('#<AssociationProxy HasOneB#parent') | ||
| expect(c.parent(chainable: true).first).to eq(a) | ||
| expect(b.parent(chainable: true).inspect).to include('#<AssociationProxy HasOneB#parent') | ||
| expect(b.parent(chainable: true).first).to eq(a) | ||
| expect(a.children.to_a).to match_array([b, c]) | ||
| end | ||
| end | ||
|
|
||
| it 'caches the result of has_one accessor' do | ||
| a = HasOneA.create(name: 'a') | ||
| b = HasOneB.create(name: 'b') | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, in the docs this
see alsoblock links to nothing. I've updated it with appropriate links to the API on rubydoc.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I had actually written some code a while back to turn the
YARDdocumentation intorstfiles for readthedocs (you can see therstformatting on rubydoc.info), but re-implementing rubydoc.info was probably largely a waste of my time ;) . I think these were links to that internal API documentation, but glad to have these fixed now