#489 open
Bryan Larsen

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

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.

New-ticket Create new ticket

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

Referenced by

Pages