I''ve done a little write-up of some confusing behavior around attr_readonly. The upshot is that I think we should make attr_readonly act more like attr_protected. Failing that, I think we should consider change the behavior of update_attribute to make things a bit less confusing. Here''s the gist: http://gist.github.com/70955 Thanks, - Trevor --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Hi - To your options at the bottom, I might add: d) silently ignore update_attribute, in the same way that attr_protected silently ignores mass assignment. (to deal with the issue of the obj in memory appearing to be updated) this seems to be most in keeping with the doc: "Attributes listed as readonly can be set for a new record, but will be ignored in database updates afterwards." So, allowing update_attribute to work on this might be somewhat surprising for people who are expecting to the behavior in the current doc. On the other hand, update_attribute not working might be more surprising... On Feb 26, 12:36 pm, Trevor Turk <trevort...@gmail.com> wrote:> I''ve done a little write-up of some confusing behavior around > attr_readonly. > > The upshot is that I think we should make attr_readonly act more like > attr_protected. Failing that, I think we should consider change the > behavior of update_attribute to make things a bit less confusing. > > Here''s the gist: > > http://gist.github.com/70955 > > Thanks, > - Trevor--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> So, allowing update_attribute to work on this might be somewhat > surprising for people who are expecting to the behavior in the current > doc. On the other hand, update_attribute not working might be more > surprising...I think I like option a. It''s consistent with attr_protected and allows people who ''know what they''re doing'' to override that read only setting if they desperately need to, and avoids the horrible case of an object *appearing* updated but not being persisted with the updates applied. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Feb 27, 12:48 pm, Mischa <f.mis...@gmail.com> wrote:> So, allowing update_attribute to work on this might be somewhat > surprising for people who are expecting to the behavior in the current > doc. On the other hand, update_attribute not working might be more > surprising...On Mar 1, 11:23 pm, Michael Koziarski <mich...@koziarski.com> wrote:> I think I like option a. It''s consistent with attr_protected and > allows people who ''know what they''re doing'' to override that read only > setting if they desperately need to, and avoids the horrible case of > an object *appearing* updated but not being persisted with the updates > applied.Thanks for the replies, guys. I think I like option a, too. I expected update_attribute to work, and I can''t imagine that anyone is counting on it not working. If I don''t hear any objections I''ll try to work up a patch with doc fixes tomorrow and create a Lighthouse ticket. Thanks, - Trevor --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Mar 1, 11:32 pm, Trevor Turk <trevort...@gmail.com> wrote:> If I don''t hear any objections I''ll try to work up a patch with doc > fixes tomorrow and create a Lighthouse ticket.I''ve done some research and added a new file to the original gist: http://gist.github.com/70955 You''re probably best off viewing it here: http://gist.github.com/raw/70955/9ccc058386921fefcc9f9d3e6a9ca0b658619e63/gistfile2.txt The upshot is that I feel changing the behavior of update_attribute may have unexpected consequences, and I believe the short-term solution may be to simply raise an exception if update_attribute is given a readonly attribute. This may be the easiest solution to implement, but it wouldn''t have the 100% desired effect (e.g. allowing update_attribute to work on a readonly attribute). It would be low impact, though, and at least it would notify the user that the action they''re trying to take won''t work. That way, at least the behavior would be clear. The fact that update_attribute appears to work but doesn''t actually work is the "bug" in my mind. I''d prefer to make it actually work, but the options I explored may end up being more problematic than are justified by the issue at hand. Thanks, - Trevor --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> That way, at least the behavior would be clear. The fact that > update_attribute appears to work but doesn''t actually work is the > "bug" in my mind. I''d prefer to make it actually work, but the options > I explored may end up being more problematic than are justified by the > issue at hand.I completely agree with your assesment of the issue, create a new exception subclass along the lines of ReadOnlyAttributeError and make it raise. Just be sure that the exception tells you what the read only attribute was ;). Thanks for spending the time on this. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Mar 2, 3:36 pm, Michael Koziarski <mich...@koziarski.com> wrote:> I completely agree with your assesment of the issue, create a new > exception subclass along the lines of ReadOnlyAttributeError and make > it raise. Just be sure that the exception tells you what the read only > attribute was ;).I worked up a patch and created a ticket on Lighthouse here: http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2132-confusing-behavior-with-attr_readonly I also did a little more research and found some other methods that might be similarly confusing. Everything is detailed in the ticket. Thanks, - Trevor --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---