Hobo Hobo → Hobo 1.4

Comments and changes to this ticket

• 

  Bryan Larsen July 5th, 2011 @ 02:27 AM

  • no changes were found...

  • 0001-957-marshal-context-when-o.patch 4.1 KB
• 

  Bryan Larsen July 5th, 2011 @ 02:33 AM

  • Assigned user set to “Domizio Demichelis”
  • Milestone set to “Hobo 1.3 (Rails 3)”
  • Milestone order changed from “197977” to “0”

  The big question is whether to include this before 1.3.0. I vote yes, because this creates many more situations where AJAX works "out of the box" without fiddling or stupid workarounds.

  It has the following drawbacks:

  1) typed_id can now be very large and can contain carriage returns, so any usage of typed_id in class names or ids have to be updated.

  2) If this is a collection, you can end up with a VERY large HTML page.

  3) private data can be leaked.

  benefits:

  1) usually is the right thing to do

  2) working around the lack of this patch is much harder than disabling it to avoid leakage etc:

   <do part="blah" with="&nil">...</do>
  

• 

  Bryan Larsen July 5th, 2011 @ 02:42 AM

  drawback 4: we're actually double marshalling the context, which has a significant size impact. Not marshalling twice would have resulted in a lot more code churn.

• 

  Matt Jones July 5th, 2011 @ 07:14 PM

  Couple thoughts on this:

  • I'm a little blurry on what problem the Marshal stuff is intended to solve. For instance, if I've got a part where this is, say, all the Foos for some Bar, then getting back the array exactly as it was originally is actually wrong for a common use case (adding another Foo). Even if the behavior just described was correct for a particular case, wouldn't it be equally valid to serialize this as an array of typed_ids? At best, the Marshaled data is excessive (could have been retrieved from the DB) and at worst it's actually wrong (AR objects will contain loaded values in instance variables, which may have just been updated in the DB).

  • ultimately, would it make more sense to extend the existing handling for type:id:field to somehow include arbitrary expressions for generating the context?

• 

  Bryan Larsen July 5th, 2011 @ 07:29 PM

  Thanks for your comments Matt.

  • remember this code doesn't trigger whenever the current context is the same as page_this, so it's not going to bloat standard index pages. Hopefully people aren't creating large collections in their view code.

  • the most common scenario where I run into a problem solved by this solution is when the context is a single new object

  • you're right about the array of typed_id's. I thought that would be a slipper slope - the next step is what about belongs_to objects, et cetera? But it might not actually be too bad -- marshal_dump and marshal_load exist for that reason...

  • I'm not sure if I completely understand your last comment. I have a guess, but rather than guessing, I'll ask you to clarify.

• 

  Matt Jones July 5th, 2011 @ 08:33 PM

  In the first case, I'm thinking of the typical "child collection with inline new form" scenario where the part mechanism currently trips people up - although on thinking further, is that case even going to go down this path? I know it used to cause problems, but it's been many months since I fiddled with the part stuff.

  On the "parts on new record page" issue, the tricky part is that many users seem to expect this in that context to pick up the as-yet-unsubmitted values from elsewhere on the page, exactly the opposite of what's actually going to happen.

  As for my last point, imagine a variant of the "child collection with inline new" scenario where the collection is only displaying a scoped subset of the records (for instance, only "active" tasks attached to a story). The form in that case is likely to be intended only to create "active" tasks (kind of silly otherwise), so submitting the form is likely to need to refresh the collection. It'd be great for that to be (for instance) represented by a context of story:story_id:taska:active, but getting that to be generated automatically seems pretty difficult...

  We may also want to tap Henry Baragar on this one, as he's done quite a bit of stuff with the part mechanism and was pretty interested in improving it at Railsconf.

• 

  Matt Jones September 21st, 2011 @ 04:18 PM

  I suspect #954 is linked to this.

• 

  Bryan Larsen October 25th, 2011 @ 04:20 PM

  • Milestone changed from “Hobo 1.3 (Rails 3)” to “Hobo 1.4”
  • Milestone order changed from “65” to “0”