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

Add symbol support to the collection option from the association builder #1705

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

caioertai
Copy link

@caioertai caioertai commented Jun 4, 2020

Purpose

This change allows the association builder to use symbols as arguments for its collection option. Which will then send them to the class implied from the reflection.

E.g.:

f.association :company, collection: :active

Which would essentially result in the same as:

f.association :company, collection: Company.active

Changes

Previously, passing a symbol to the collection key would result in an undefined method #to_a for the given symbol. This includes a check for the value being a symbol, which then sends the given symbol to the reflection klass, which, returning an object that responds to #to_a fits in easily with the current code.

Reasoning

Passing the collection argument for the association builder in the view requires a reference to the class. The proposed change allows for a more loose relation between the view and your class names, allowing for more reusability.

Known issues

  • The current implementation doesn't allow for class methods with arguments. But should work nicely for simple scopes.

Notes

This is my first contribution and I have but passing knowledge of the code base. I'd appreciate an honest review and feedback and would love to dig into it more if needed. Thanks!

@caioertai caioertai changed the title Add symbol support to association collection option Add symbol support to the collection option from the association builder Jun 4, 2020
@caioertai caioertai closed this Jun 4, 2020
@caioertai caioertai reopened this Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant