Thanks, Jeremy. I'm going to just use :unmatched_key for now, and if anyone comes up with a better alternative it's a simple find-and-replace.
Chris On Tuesday, May 29, 2012 2:23:55 PM UTC-7, Jeremy Evans wrote: > > On Tuesday, May 29, 2012 11:16:21 AM UTC-7, Chris Hanks wrote: >> >> I've been working on extending the nested_attributes plugin to handle >> child models with composite primary keys, and I've been able to get them to >> work just like models with single primary keys with minimal changes to the >> plugin ( >> https://github.com/chanks/sequel/commit/466391e901f47cba40127b017711724db8ecdf2c), >> >> but there's another change I'd like to make that I need some community >> guidance on. >> > > This patch looks good overall. I added a few line notes, but they are > just style related. As long as the specs pass, there shouldn't be a > problem applying this. > > >> >> Currently, if a child attributes hash being sent to the parent model >> contains a primary key, but no child with that primary key exists for the >> attributes hash to be applied to, an error will be raised, unless :strict >> => false is set, in which case nothing will happen. This works fine if your >> primary key is autoincrementing, or is set in a model callback, but I think >> it's insufficient for composite primary keys. When I've been hacking around >> nested_attributes in the past one of my requirements has usually been to be >> able to create a new model using the primary key fields from the form, and >> I'd really like to be able to support this, but it requires an addition to >> the nested_attributes API. >> >> I'm thinking about a new option to the nested_attributes declaration >> that's something like :new_pk, and has the possible arguments of :create, >> :raise and :ignore. :create means that we create a new child when a primary >> key is supplied that currently doesn't exist, :raise would mean that an >> error is thrown (this would be the default and would match the current >> behavior) and ignore would mean that nothing happens (this is the same as >> the current :strict => false, and in fact I'd just make :strict => false >> set :new_pk => :ignore internally to maintain compatibility). >> >> Here's a gist showing the type of usage I'd like to be able to support: >> https://gist.github.com/2829829 >> >> But I'd like to hear from other people who use this plugin. Suggestions? >> Comments? Criticisms? >> > > I think you are correct that this needs an additional option. I'm don't > really like the :new_pk option name, but I'm not sure I can think up a > better one. Maybe :unmatched_pk? That's longer but I think a bit more > descriptive. Maybe someone else here can recommend a better one. > > Thanks for working on this. Just let me know when you think it is ready > and I'll work on merging and testing it. > > Thanks, > Jeremy > -- You received this message because you are subscribed to the Google Groups "sequel-talk" group. To view this discussion on the web visit https://groups.google.com/d/msg/sequel-talk/-/DJODjI7i22QJ. 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/sequel-talk?hl=en.
