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

[WIP] Fix extra db hit with belongs_to association #1118

Closed
wants to merge 1 commit into from

Conversation

beauby
Copy link
Contributor

@beauby beauby commented Sep 6, 2015

As mentioned in #1100, belongs_to associations currently load the record instead of using the association_name_id attribute when available (i.e. with ActiveRecord).
The explicit calls have been fixed, but the way things are, the records are actually loaded when building the associations anyways. A satisfying fix would be to lazify that part.

This PR is built on top of #1103 (which was initially intended to provide an easy way to fix #1100).

@NullVoxPopuli
Copy link
Contributor

rebase needed.

@beauby, resource_identifier_type_for is a useful method - I should see how the json adapter works / doesn't under the same scenario of belongs_to :author, class_name: User.name

@beauby
Copy link
Contributor Author

beauby commented Sep 14, 2015

I'll rebase this eventually but currently it requires more work during the phase when the serializer actually gets built in order to fix the issue.

@sheldonbaker
Copy link

Has there been any recent activity on this or #1100? I'm currently getting bit pretty hard by this issue. :)

@NullVoxPopuli
Copy link
Contributor

@beauby can you rebase this? we recently had another issue open about this. #1552

@beauby
Copy link
Contributor Author

beauby commented Mar 17, 2016

This doesn't fix it as the problem lies upstream + needs discussion because the real fix implies tighter coupling with AR. Closing this for now.

@beauby beauby closed this Mar 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants