proper fix for bug 477
Reported by Bryan Larsen | August 18th, 2009 @ 03:46 PM | in Beyond Hobo 1.0
We ended implementing a hack fix for Bug 477 so that we wouldn't break our API. Post-1.0 we should look at a better fix.
Email discussion:
Tom Locke wrote:
Yeah I'm very familiar with this problem. The basic structure of the way Rails handles form validation errors doesn't lend itself at all well to generically handling the case where the form is somewhere other than the default place.
I've done similar things to your solution in the past, so I think its along the right lines, or at least is a good compromise without figuring out a deeper solution (and that might cause us to deviate too much from 'normal' Rails anyway).
I don't think calling hobo_index is correct though - what if the controller is doing custom stuff in #index ? Also the problem is not really limited to #index (although this is the only place it shows up in a default app I think?)
Would this work? (in re_render_form):
- Always assign to @errored_object (better name @invalid_record ?) i.e. skip the view=="index" test
OK, that makes sense. I like your name better.
- send(view)
At first glance, that seemed better than my first thought, but:
-
That will break everything except index, because they'll have a clean object in this, with errors only in @invalid_record. Most of our controller actions now do self.this ||= ..., but any custom ones generally do not.
-
Anybody who does a redirect will cause problems. Granted this is highly unlikely because we're coming from a page that rendered initially.
If we were still in 0.7 days, I think this would be the right change to make, but for pre-1.0?
Bryan
Hmm but if that action does not render explicitly, will the default template be the wrong one? Can we do
params[:action] = view
to fix that?
Tom
On 17 Aug 2009, at 22:41, Bryan Larsen wrote:
I attached a hacky fix there. The fix should explain what the problem is. Any better ideas?
Bryan
Comments and changes to this ticket
-
Bryan Larsen August 28th, 2009 @ 03:03 PM
- State changed from new to open
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป
People watching this ticket
Tags
Referenced by
- 912 auto_actions_for ignores failed validation However, there is still the #477 and #489 bug that bites ...