This project is archived and is in readonly mode.
AssociationCollection's destroy method is not compatible with old version
Reported by quake wang | March 21st, 2009 @ 06:20 AM | in 2.x
class Article < ActiveRecord::Base
has_many :reviews
end
in rails 2.2.2, we can use fixnum parameter to destroy one collection object:
article = Article.find(params[:article_id])
article.reviews.destroy(params[:id])
the second line code generates sql which looks like this: SELECT * FROM reviews WHERE (article.id = 1) AND (reviews.id = 10) ORDER BY id desc DELETE FROM reviews WHERE id = 10
in rails 2.3, it will raise an exception: ActiveRecord::AssociationTypeMismatch: Review(#-615260898) expected, got Fixnum(#-604984008)
I thought this commit breaks the compatibility http://github.com/rails/rails/co...
Comments and changes to this ticket
-
Anthony Eden April 6th, 2009 @ 04:22 PM
I'm seeing the same issue. Is there documentation somewhere about the deprecation of collection.destroy(id) because I see that it is not in the latest API docs (maybe it was never there in the first place?) Alternatively is there a fix?
-
CancelProfileIsBroken August 6th, 2009 @ 01:10 PM
- Tag set to bugmash
-
Josh Nichols August 9th, 2009 @ 05:48 PM
- Tag changed from bugmash to bugmash, verified
Verified this occurring 2-3-stable. I'd +1 adding back support for destroy(id), and if appropriate, adding a deprecation warning.
-
Dan Pickett August 9th, 2009 @ 06:48 PM
+1 verified
latest patch applies cleanly to master and tests behavior adequately
-
Elad Meidar August 9th, 2009 @ 06:58 PM
+1 Verified, +1 on the catch, i am usually using delete since i almost never REALLY destroy data in my applications.
+1 patch applies to 2-3-stable and master, all tests pass.
Tested also on a mini app, included some extra #delete manual tests to assure that method is working well, and it does.
-
Dan Croak August 9th, 2009 @ 06:59 PM
+1 verified Josh's patch applies to 2-3-stable and tests run green.
I have attached a patch that alters his implementation to check ActiveRecord::Base instead of String and Integer. This better reveals the intent, in my opinion, of the conditional.
It should apply cleanly after applying his 2306-destroy_association_by_id.patch
-
Repository August 9th, 2009 @ 09:47 PM
(from [29a5549b3408597f3d3eb17350737631368c6cc9]) Added back support for destroying an association's object by id. [#2306 status:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/29a5549b3408597f3d3eb173507376... -
Repository August 9th, 2009 @ 09:47 PM
(from [0ec64bea9293dd21588359da80bb43b82886cf2c]) Added back support for destroying an association's object by id. [#2306 status:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/0ec64bea9293dd21588359da80bb43... -
José Valim August 9th, 2009 @ 10:02 PM
- State changed from new to resolved
- Tag changed from bugmash, verified to verified
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Tags
Referenced by
- 2306 AssociationCollection's destroy method is not compatible with old version (from [29a5549b3408597f3d3eb17350737631368c6cc9]) Added b...
- 2306 AssociationCollection's destroy method is not compatible with old version (from [0ec64bea9293dd21588359da80bb43b82886cf2c]) Added b...