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

Allow referencing sideloaded include by key. #2136

Merged
merged 3 commits into from
May 18, 2017

Conversation

caomania
Copy link

Purpose

When an association is set to be sideloaded and the association has a key defined we should be able to include this association by referencing the key.

Changes

When checking include_slice for associations to include, check the association's key option first, then fallback to the association name.

Additional helpful information

Added tests to demonstrate that attempting to reference an association by key when including associations currently fails.

This ensures that associations with a key set are still included.
@mention-bot
Copy link

@caomania, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bf4, @beauby and @richmolj to be potential reviewers.

Copy link
Member

@bf4 bf4 left a comment

Choose a reason for hiding this comment

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

Looks good. Please fix the rubocop so that tests pass and add yourself to the CHANGELOG under features

assert_nil(hash[:included])
end


Copy link

@mtwentyman mtwentyman May 17, 2017

Choose a reason for hiding this comment

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

this (extra empty) line is what rubocop doth complain about

@bf4 bf4 merged commit a89e78c into rails-api:0-10-stable May 18, 2017
@bf4
Copy link
Member

bf4 commented May 18, 2017

@caomania merged!

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.

4 participants