update_attribute returns unexpected nil when new value is the same as current

[donapieppo]

update_attribute(name, value) returns nil if the submitted value is the same as the current value.
In rails 4 it returned true (as aspected, imo).

p = Post.create(title: 'foo')
p.update_attribute(:title, 'foo') # returns nil
p.update_attribute(:title, 'bar') # return true

I think in rails 4 the db update query was always done.
In rails 5 the unecessary update is skipped but I think the return value should be true.

With update_column the update is done and the return value is always true.

Expected behavior

   p = Post.create(title: 'foo')
   p.update_attribute(:title, 'foo') # should return true

System configuration

Rails: 5.0.0.1
Ruby: 2.3.1p112

[rafaelfranca]

If there is no change in the database why true is expected?
On Fri, 23 Sep 2016 at 11:15 Donapieppo notifications@github.com wrote:

update_attribute(name, value) returns nil if the submitted value is the
same as the current value.
In rails 4 it returned true (as aspected, imo).

p = Post.create(title: 'foo')
p.update_attribute(:title, 'foo') # returns nil
p.update_attribute(:title, 'bar') # return true

I think in rails 4 the db update query was always done.
In rails 5 the unecessary update is skipped but I think the return value
should be true.

With update_column the update is done and the return value is always true.
Expected behavior

p = Post.create(title: 'foo')
p.update_attribute(:title, 'foo') # should return true

System configuration

Rails: 5.0.0.1
Ruby: 2.3.1p112

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#26593, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC66GXg0fqqie-mAJKQd3CiZF48kao-ks5qs4qxgaJpZM4KEuDo
.

[donapieppo]

Because I don't care what happens behind (in db) or what was the previous value.
With update_attribute(:title, 'foo') I need to know that now the current value of title is 'foo', that everything went fine. And I expect true in order to feel good an go on :-)
A nil return value is like a false return value.

[rafaelfranca]

In fact the return of this method should not care for you. If you are
expecting always true, so why are you checking the return of the method?

And yes nil is false in that case and it means exactly what it means, no
attribute were updated so it return false.
On Fri, 23 Sep 2016 at 11:29 Donapieppo notifications@github.com wrote:

Because I don't care what happens behind (in db) or what was the previous
value.
With update_attribute(:title, 'foo') I need to know that now the current
value of title is 'foo', that everything went fine. And I expect true in
order to feel good an go on :-)
A nil return value is like a false return value.

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#26593 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAC66PBuz6qEsGcHaRxly2AzMqGtqgy4ks5qs43kgaJpZM4KEuDo
.

[donapieppo]

I am not expecting always true. I expect true if everthing went fine and the value of title is foo.

You mean that what with update_attribute nothing can go wrong and I can never get a false result?
If that is the case maybe the doc should say that. Should say that there is no interesting return value.

If instead there can be a false (and I need to check it), than a nil return value is unexpected for an update where everything went fine.

[donapieppo]

Of course if the nil return value means exactly what it means, no attribute were updated so it return false is the wanted behaviour than you can close this.

I still think that update_attribute should be idempotent (if it is the correct word) meaning that it sould return the same value when executed multiple time. It should not depend on the previous value of the attribute. As was in rails 4.

Maybe I just need to replace update_attribute with update_column.

[prathamesh-sonpatki]

@rafaelfranca @donapieppo I had changed this behavior to save a SQL call in 0fcd4cf5.

I think we can return true if changed? is false to restore the old behavior. I can make a PR if this sounds like acceptable solution.

[donapieppo]

a true return value with changed? false would be great for me.

Otherwise the doc should say that update_attribute returns nil if the new value is the same as the value in the database.

[rafaelfranca]

👍
On Fri, 23 Sep 2016 at 12:18 Donapieppo notifications@github.com wrote:

a true return value with changed? false would be great for me.

Otherwise the doc should say that update_attribute returns nil if the new
value is the same as the value in the database.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#26593 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAC66GaalP-tzPngr6Rf70yfxq1iOYAaks5qs5lagaJpZM4KEuDo
.

[prathamesh-sonpatki]

Great, I will send a PR in some time.

[prathamesh-sonpatki]

r? @prathamesh-sonpatki