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 :default option to belongs_to #28453

Merged
merged 1 commit into from Mar 17, 2017
Merged

Conversation

@georgeclaghorn
Copy link
Member

@georgeclaghorn georgeclaghorn commented Mar 16, 2017

In Basecamp, we often use before_validation callbacks to initialize belongs_to associations like so:

belongs_to :person
before_validation -> { self.person ||= Current.person }

belongs_to :account
before_validation -> { self.account = person.account }

This PR adds a :default option to the belongs_to macro to formalize and simplify that pattern. The :default option adds a before_validation callback that initializes the association with the given lambda’s return value. It’s used like this:

belongs_to :person, default: -> { Current.person }
belongs_to :account, default: -> { person.account }

/cc @dhh

@georgeclaghorn georgeclaghorn force-pushed the georgeclaghorn:belongs-to-default branch 6 times, most recently Mar 16, 2017
@dhh
Copy link
Member

@dhh dhh commented Mar 17, 2017

Looking good! Need to document the new option in the belongs_to documentation. Then I think it's a wrap.

Use it to specify that an association should be initialized with a
particular record before validation. For example:

    # Before
    belongs_to :account
    before_validation -> { self.account ||= Current.account }

    # After
    belongs_to :account, default: -> { Current.account }
@georgeclaghorn georgeclaghorn force-pushed the georgeclaghorn:belongs-to-default branch to 2762aa0 Mar 17, 2017
@georgeclaghorn
Copy link
Member Author

@georgeclaghorn georgeclaghorn commented Mar 17, 2017

Added to the belongs_to docs.

@dhh dhh merged commit 73b86ac into rails:master Mar 17, 2017
0 of 2 checks passed
0 of 2 checks passed
codeclimate 1 new issue
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@georgeclaghorn georgeclaghorn deleted the georgeclaghorn:belongs-to-default branch Mar 17, 2017
@bogdan
Copy link
Contributor

@bogdan bogdan commented Mar 17, 2017

@geoffharcourt What should I do when I need to cache column value but not association? Should I still do it in the before_validation callback? This feature seems incomplete to me.

assert_equal jamis, ship.developer

ship.update!(developer: nil)
assert_equal david, ship.developer

This comment has been minimized.

@simi

simi Mar 17, 2017
Contributor

Updating to nil is not mentioned in docs and I think it can lead to confusions.

Let's say I have code where I pass relation by variable to update. Some problem can occur and that variable can be nil. I will expect to get error instead of using default value when I pass explicitly wrong value (nil).

@dhh
Copy link
Member

@dhh dhh commented Mar 17, 2017

@bogdan Are you talking about how can we set default values for general attributes, unrelated to belongs_to? If so, yeah, this doesn't tackle that. I don't think those things go together, though.

kamipo added a commit to kamipo/rails that referenced this pull request Mar 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.