Skip to content
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

Fixes duplicated entries when using nested fields #58

Closed
wants to merge 1 commit into from

Conversation

Nerian
Copy link
Contributor

@Nerian Nerian commented Feb 17, 2014

#57

@@ -1,7 +1,7 @@
Next Release
============

* Your contribution here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put back the * Your contribution here, please, for the next contributor.

@dblock
Copy link
Member

dblock commented Feb 17, 2014

Thanks a lot for fixing this. The build failed on Rubocop, use 1.9.x Ruby syntax. Please try to use --amend when updating.

@Nerian
Copy link
Contributor Author

Nerian commented Feb 17, 2014

Done :)

@dblock
Copy link
Member

dblock commented Feb 17, 2014

Waait... a second :) The check for _nested is an attribute name check, or am I missing something? If I rename every _nested to _x in the test everything breaks.

@Nerian
Copy link
Contributor Author

Nerian commented Feb 17, 2014

Oh true. For some reason I thought that internally we were using __nested to refer to nested fields. Now I see it's just a coincidence.

@Nerian
Copy link
Contributor Author

Nerian commented Feb 17, 2014

https://github.com/intridea/grape-entity/blob/master/lib/grape_entity/entity.rb#L141

As I understand it, we can say that if there is a '' in the name, it is a nested attribute. I changed the code to check for '' instead of '_nested'

@dblock
Copy link
Member

dblock commented Feb 17, 2014

I don't think this is correct or reliable. Why can't I have an attribute with these characters that's not nested? We should be able to add something to options or check against @nested_exposures. I can help you find a better way to figure out whether the attribute is nested if you can't figure it out. I also think you should write a spec for this with more than one level of nesting.

@Nerian
Copy link
Contributor Author

Nerian commented Feb 17, 2014

I agree. So I nuked the branch and started fresh with a test:

Nerian@386f50a#diff-2f09e9dc67c5575f0172b3d0defe0e1bR110

I am not sure how to implement it though.

@dblock
Copy link
Member

dblock commented Mar 3, 2014

Closing this in favor of #60. @Nerian please take a look, LMK if that fixed all your known issues.

@dblock dblock closed this Mar 3, 2014
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.

2 participants