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

Support a context for update_attributes / update_attributes! methods. #4026

Closed

Conversation

kennyj
Copy link
Contributor

@kennyj kennyj commented Dec 19, 2011

Currently, the update_attributes / update_attributes! method can't specify a validation context.
This patch allow above methods to specify the context.

@@ -209,8 +209,9 @@ def update_attributes(attributes, options = {})
# The following transaction covers any possible database side-effects of the
# attributes assignment. For example, setting the IDs of a child collection.
with_transaction_returning_status do
context = options.has_key?(:context) ? options.delete(:context) : nil
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can rewrite it like this:

context = options.delete(:context)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops... You're right !
I'll fix and squash it.
Thanks!

@drogus
Copy link
Member

drogus commented Dec 19, 2011

Shouldn't it go to validations.rb?

@kennyj
Copy link
Contributor Author

kennyj commented Dec 20, 2011

@drogus

Considers your proposal, I think the easiest way to fix that next.

$ vim lib/active_record/persistence.rb
208     def update_attributes(attributes, options = {})
209       # The following transaction covers any possible database side-effects of the
210       # attributes assignment. For example, setting the IDs of a child collection.
211       with_transaction_returning_status do
212         self.assign_attributes(attributes, options)
213         save(options) # ★ options argument is passed 
214       end
215     end

What do you think?

@kennyj
Copy link
Contributor Author

kennyj commented Dec 20, 2011

I think the below commit is simplest way.

kennyj@5e1875a

I couldn't go to validations.rb, because I must pass options argument to the save(save!) method.
And if I pass options argument to the save in persistence.rb, I have nothing to do in validations.rb.

@kennyj
Copy link
Contributor Author

kennyj commented Feb 27, 2012

My last comment is simplest way to support this feature, but options for assign_attributes and for save are merged ...
I think that this feature should be supported, but it seems that this implement is not good ;-)
I'm closing this PR.

@kennyj kennyj closed this Feb 27, 2012
@andreas
Copy link
Contributor

andreas commented Apr 2, 2012

@kennyj, I think it's a great idea to have context for update_attributes. Any ideas for a smarter implementation? What are the downsides of passing the same options-hash to both assign_attributes and save?

@kennyj
Copy link
Contributor Author

kennyj commented Apr 2, 2012

@andreas I think some unrelated option values were passed to assign_attributes method and we should not merge option values.

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.

None yet

4 participants