I was leaning toward Approach 1 & 4 because of the much cleaner interface. However, it just hit me that this feature will not be enabled by default (for security), so that kind of puts a damper on using it directly in Ruby code (such as in test cases to create records). I'm convinced that 99% of the time this will only be used in multi-model forms so we should cater to that.
On that note, I've added Josh's approach (from his patch) to the gist. See Approach 5. http://gist.github.com/10793 I like this because the implementation is very simple. It does not override the "tasks=" method or add any extra magic there. New records are created with a hash so they can be sorted and have a unique identifier when passing through the form. Regards, Ryan On Sep 17, 8:50 am, Ryan Bates <[EMAIL PROTECTED]> wrote: > On Sep 17, 8:39 am, Eloy Duran <[EMAIL PROTECTED]> wrote: > > > > > > I think he's referring to Approach 2 (ofhttp://gist.github.com/gists/10793 > > > ) which has both a new_task_attributes and existing_task_attributes > > > accessor. I'm not very fond of this either, especially when you're > > > setting it directly through Ruby and not using form params. > > > Thanks for clarifying. > > > > BTW Eloy, I saw your idea of using the timestamp when adding new task > > > records (through javascript). Genius! > > > Thanks :) > > > > What if we support both Approach 1 and Approach 4? Here's the logic in > > > pseudo code. > > > > -- > > > def tasks=(new_tasks) > > > if new_tasks is a hash > > > grab values (as array) and sort by keys > > > call self.tasks= with new values array > > > else > > > for each task in new_tasks > > > if task is hash > > > if task has id key > > > update existing task matching id > > > else > > > add new task with hash attributes > > > end > > > else > > > add task > > > end > > > end > > > end > > > end > > > -- > > > > Supporting both hashes and arrays opens up a lot of flexibility. > > > Generally you'd use an array when dealing directly in Ruby, but a hash > > > when going through a form. > > > I agree. I think I already support what you mean. > > But maybe it would be an idea if you could send me a test which tests > > the difference > > in behaviour between a Hash and a Array so I can verify? > > Sure thing, I'll work on getting some real code together. I think the > primary difference between this approach and yours is that it uses > the :id key to distinguish existing records. I like this because it > removes the need for the magic "new_" hash key and is almost identical > to passing an array. The only thing the keys are used for is sorting > the values. > > Regarding deleting. It depends on whether or not we use the "tasks=" > accessor method. Rails already has this implemented, along with logic > on how deleting happens. AFAIK, it currently "resets" the tasks > association. That is, tasks which aren't mentioned there are removed > from the association automatically. Whether they are deleted entirely > or not depends on the :dependent option in has_many (I'm assuming). I > think we should follow that same logic. > > Look at it this way, when you're setting "tasks=", the values could be > represented as either hashes or actual Task instances (which are > currently supported). It makes sense that you can mix and match them > and not get strange behavior in how deleting happens. --~--~---------~--~----~------------~-------~--~----~ 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 [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
