This project is archived and is in readonly mode.
Nested attribute accessors should ignore new records with truthy _delete key
-
Eloy Duran
I would suggest to change the test test_should_remove_delete_key_from_arguments_hash_of_new_records to use '0' for the 'delete' key. Right now the record is rejected because delete has a truthy value, if you use '0' it will build and actually test what you intended to.
Other then that I agree with the patch.
-
Eloy Duran
Arg, don't mind the formatting you get the point :)
-
DHH
- Milestone cleared.
-
Pascal Ehlert
Haha yes, I've been pretty tired when I did the patch. Thanks for the review and please find attached the updated version.
-
Eloy Duran
I have added some documentation and fixed a few whitespace errors. I'm a whitespace fanatic :)
+1 The patch indeed tests the expected behaviour.
-
Pascal Ehlert
Excellent, thanks a lot. I thought it would be obvious that a _delete key on new records rejects them, but it's certainly good to clearly state it in the documentation.
Where have you found a whitespace error? I'm using TextMate with two spaces indentation, so there shouldn't be any. I suppose we just have a different idea of how it should be done here, because I'm usually careful with whitespaces as well. ;-)
Thanks.
-
Eloy Duran
The Rails source in general uses no spaces at all for empty lines. Textmate, however, adds spaces up-to the current indentation level. I myself prefer Textmate's style, but we should follow the general Rails convention.
The “whitespace errors” I mentioned were around your test method :-)
-
Pascal Ehlert
Totally agree and having no meaningless characters in the code does actually make more sense to me.
-
Repository
- State changed from new to committed
(from [455a7633dbdb295de828eb2657433d47d85eb0bc]) Nested attribute accessors should ignore new records with truthy _delete key.
Signed-off-by: Michael Koziarski michael@koziarski.com [#1861 Nested attribute accessors should ignore new records with truthy _delete key state:committed] http://github.com/rails/rails/co...
