#4169 ActiveResource::Base.save (PUT) doesn't clear out errors - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4169 ✓stale

ActiveResource::Base.save (PUT) doesn't clear out errors

Reported by Matthew Parker | March 13th, 2010 @ 10:51 PM | in 2.3.10

i've confirmed this bug in rails 2.3.5. here are the steps to reproduce:

first, create a standard rails app with a scaffolded "Author":

  # rails library
  # cd library
  # script/generate scaffold Author name:string
  # rake db:migrate

next, require that the "name" attribute on an author be present:

  # vim library/app/models/author.rb
  class Author < ActiveRecord::Base
    validates_presence_of :name
  end

start the app:

  
  # script/server

now, go into irb and create an ActiveResource::Base class that points to our library app:

  # irb
  >> require 'active_resource'
  >> class Author < ActiveResource::Base; self.site = 'http://localhost:3000'; end
  >> a = Author.new
  >> a.name = "moonmaster9000"
  >> a.save 
    ==> true
  >> a.name = nil
  >> a.save
    ==> false
  >> a.errors
    ==> #["can't be blank"]} ....
  >> a.name = "Matt Parker"
  >> a.save
    ==> true

In our library app, we've successfully updated the author record with the name "Matt Parker". At this point, I would expect a.errors to be empty again, and a.valid? to be true. Unfortunately, this is not the case:

  >> a.errors
    ==> #["can't be blank"]} ....
  >> a.valid? 
    ==> false

I'd be happy to look into fixing this bug; I do a lot of work with ActiveResource and have several gems that build on and extend ActiveResource (http://github.com/moonmaster9000) - however, I don't want to fix this bug if ARes is already undergoing a lot of changes for Rails 3, rendering any bug fix to a 2.3 branch obsolete.

Comments and changes to this ticket

Yehuda Katz (wycats) April 13th, 2010 @ 05:33 AM
- Milestone set to “2.3.6”
Rizwan Reza May 16th, 2010 @ 02:41 AM
- Tag changed from “activeresource, validation” to “activeresource, bugmash, validation”
[bulk edit]
Josh Kalderimis May 16th, 2010 @ 11:28 AM
- Tag changed from “activeresource, bugmash, validation” to “activeresource, bugmash, patch, validation”
Here are failing tests and a patch tested against 2-3-stable
Josh Kalderimis May 16th, 2010 @ 11:35 AM
Sorry, was fighting LH there for a sec, here is the patch
- ar_save_clear_errors.diff 3.5 KB
You flagged this item as spam.
Anil Wadghule May 16th, 2010 @ 02:03 PM
-1

Looks like it still have some issues. See pastie http://pastie.org/962508

after setting

a.name = nil
a.valid? should return false. But it's returning true.
Josh Kalderimis May 16th, 2010 @ 06:09 PM
Hi Anil,

valid? on AResource works differently than ARecrod. In ARecord you can check against the validations, but in AResource you need to post again as validations are on the server. save is the only way to run the validations again, which this patch corrects (clearing errors before save).

What you are asking for is not supported in AResource.

Josh
Josh Kalderimis May 16th, 2010 @ 06:49 PM
Sorry, I didn't add before that the original bug is verified
Josh Kalderimis May 16th, 2010 @ 07:46 PM
- Tag changed from “activeresource, bugmash, patch, validation” to “activeresource, bugmash, bugmash-review, patch, validation”

Jeroen van Dijk May 16th, 2010 @ 09:42 PM

applies cleanly and tests pass for 1.8.7, but tests do not pass for 1.8.6 so I can't give a plus one.

I don't think it has to do with this pathch though. This is the error I get:

  1) Error:
test_exists_without_http_mock(BaseTest):
NoMethodError: undefined method `any_instance' for ActiveResource::Connection:Class
    ./test/base_test.rb:989:in `test_exists_without_http_mock'

  2) Error:
test_accept_http_header(ConnectionTest):
NoMethodError: undefined method `mock' for #<ConnectionTest:0xb78ae2a8>
    ./test/connection_test.rb:203:in `test_accept_http_header'

  3) Error:
test_ssl_error(ConnectionTest):
NoMethodError: undefined method `expects' for #<ActiveResource::Connection:0xb7bb7a1c>
    ./test/connection_test.rb:223:in `test_ssl_error'

  4) Error:
test_timeout(ConnectionTest):
NoMethodError: undefined method `mock' for #<ConnectionTest:0xb78adf74>
    ./test/connection_test.rb:185:in `test_timeout'

162 tests, 501 assertions, 0 failures, 4 errors

Code looks good too. One thing though, i think the test should be as follows


[ :json, :xml ].each do |format|
  def test_should_clear_errors_when_error_is_corrected
    set_mocks_to_invalid
    invalid_user_using_format(format) do
      assert !@person.valid?

      @person.name = "Joe Shmoe"
      @person.age  = 56

      set_mocks_to_valid

      assert @person.save
      assert @person.valid?
    end
  end
end

This way the tests for xml and json fail seperately if ever.

You flagged this item as spam.
Jeroen van Dijk May 16th, 2010 @ 09:44 PM
sorry, i get those errors without this patch too. Anyone else?
You flagged this item as spam.
Jeroen van Dijk May 16th, 2010 @ 09:50 PM
Here is the improved example http://gist.github.com/403156
Josh Kalderimis May 18th, 2010 @ 07:58 PM
Is anyone able to verify and comment on this patch?
Jeremy Kemper May 23rd, 2010 @ 05:54 PM
- Milestone changed from “2.3.6” to “2.3.7”
[bulk edit]
Jeremy Kemper May 24th, 2010 @ 09:40 AM
- Milestone changed from “2.3.7” to “2.3.8”
[bulk edit]
Jeremy Kemper May 25th, 2010 @ 11:45 PM
- Milestone changed from “2.3.8” to “2.3.9”
[bulk edit]
Jeremy Kemper August 30th, 2010 @ 02:28 AM
- Milestone changed from “2.3.9” to “2.3.10”
- Importance changed from “” to “Low”
[bulk edit]
You flagged this item as spam.
Jacques Crocker September 19th, 2010 @ 03:17 AM
+1

I can still repro this issue in Rails3
Jeff Kreeftmeijer October 15th, 2010 @ 07:15 AM
- Tag cleared.
Automatic cleanup of spam.
Ryan Bigg October 19th, 2010 @ 08:32 AM
Automatic cleanup of spam.
Ryan Bigg November 8th, 2010 @ 01:49 AM
Automatic cleanup of spam.
Santiago Pastorino February 9th, 2011 @ 12:32 AM
- State changed from “new” to “open”
This issue has been automatically marked as stale because it has not been commented on for at least three months.

The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
Santiago Pastorino February 9th, 2011 @ 12:32 AM
- State changed from “open” to “stale”
You flagged this item as spam.
bingbing March 26th, 2011 @ 02:15 AM
louis vuitton watches