[html-formfu] DBIC: setting accessor for a Select element triggers options_from_model()

[Carl Franks]

On 22/02/2008, Andreas Marienborg <omega at palle.net> wrote:
>
>  On Feb 22, 2008, at 12:08 PM, Carl Franks wrote:
>
>  > On 21/02/2008, Steve Caldwell <info-formfu at caldwellhb.com> wrote:
>  >> I'm building a form that has a Select element that I'm mapping to a
>  >> non-column accessor on my DBIC object, as described in "non-column
>  >> accessors" in the HTML::FormFu::Model::DBIC POD, like this:
>  >>
>  >> ---
>  >> action: /foo
>  >> elements:
>  >>  - type: Select
>  >>    name: bar
>  >>    label: Your Bar
>  >>    db:
>  >>      accessor: bar
>  >>    options:
>  >>      - [ 01, January ]
>  >>      - [ 02, February ]
>  >>
>  >> When I then try and populate it from my DBIC object:
>  >>
>  >> $form->defaults_from_model( $myobj );
>  >>
>  >> HTML::FormFu::Model::DBIC::options_from_model gets called (thanks to
>  >> line 44 in HTML::FormFu::Element::_Group), which then overwrites the
>  >> options I've set from my select list with a bunch of stuff from the
>  >> database.  This is obviously not what I want here.
>  >>
>  >> Anyone else come across this problem?  The work-around isn't that
>  >> much
>  >> work (manually populate from and save to my object instead of using
>  >> defaults_from_model() and save_to_model()), but it would be cooler
>  >> if it
>  >> all worked correctly.
>  >
>  > I agree that this is broken.
>  > I think Select automatically calling options_from_model() is a case of
>  > *too much magic*
>  >
>  > (sidenote) - I know you've only just subscribed to the list, so a
>  > quick explanation, so that you understand my following proposal:
>  > FormFu in svn has been changed so that you set Model/DBIC options
>  > with:
>  >    $field->{model_config}{DBIC}
>  > instead of:
>  >    $field->{db}
>  >
>  > This will be in the next cpan release. However, setting {db} will
>  > still work, and Do The Right Thing, it'll just print a warning that
>  > it's deprecated and will be removed at some point in the future.
>  >
>  > The reason for this, is the model was implemented as a bit of a
>  > rush-job and we need to get it working properly to support multiple
>  > models - much like how Catalyst handles models.
>  > (/sidenote)
>  >
>  > I suggest we require that a new option be set to run
>  > options_from_model(), such as:
>  >
>  >    ---
>  >    element:
>  >      type: Select
>  >      name: foo
>  >        model_config:
>  >          DBIC:
>  >            options_from_model: 1
>  >
>  > (or, in the old, deprecated, style):
>  >
>  >    ---
>  >    element:
>  >      type: Select
>  >      name: foo
>  >        db:
>  >          options_from_model: 1
>  >
>  > Any opinions on this?
>
>
> Could the default be to run it if:
>
>  - no options_from_model: 0 (false) exists
>  AND
>  - no options already set on the select?

okay, we need to revisit this :(

I've moved HTML/FormFu/Model/DBIC.pm out into a separate distribution.
$form->model_class() has been renamed to default_model() and no longer
defaults to 'DBIC'.
(more on that to come in another mail!)

However - there's still DBIC-specific code in Element/_Group.pm to
handle options_from_model()
- this is bad, and needs dealt with.

I can see 2 ways to deal with this:

1) Create a new system to provide hooks that model classes can attach
to, so the user doesn't need to do anything.

2) Use the hooks provided by the current plugin system, and require
the user to specifically add it to the Select field.
e.g.
    ---
    type: Select
    name: foo
    plugins: [ 'DBIC::LoadOptions' ]
    model_config:
      DBIC:
        model: Foo

I'm inclined to prefer solution 2, because I foresee the following
problems with solution 1:
- because we currently don't require the user to load the models in
the form config, there's no guarantee that the model will be available
during process(), so the hooks won't get run anyway.
- adding yet-more hooks will make the formfu internals more complex

Any thoughts?

Carl