[html-formfu] Re: (ping Andreas + zby) Re: DBIC: setting accessor for a Select element triggers options_from_model()

[Carl Franks]

2008/6/5 Carl Franks <fireartist at gmail.com>:
> 2008/4/3 Carl Franks <fireartist at gmail.com>:
>> On 28/02/2008, Zbigniew Lukasiak <zzbbyy at gmail.com> wrote:
>>> On Wed, Feb 27, 2008 at 11:01 PM, Carl Franks <fireartist at gmail.com> wrote:
>>>  > On 22/02/2008, Andreas Marienborg <omega at palle.net> wrote:
>>>  >  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?
>>>
>>>
>>> Here is my proposal:
>>>
>>>  sub post_process {
>>>     my $self = shift;
>>>
>>>     $self->next::method(@_);
>>>
>>>     my $args = $self->model_config;
>>>
>>>     # only call options_from_model if there's no options already
>>>     # and {options_from_model} isn't 0
>>>
>>>     my $option_count = scalar @{ $self->options };
>>>
>>>     my $option_flag = exists $args->{options_from_model}
>>>         ? $args->{options_from_model}
>>>         : 1;
>>>
>>>     if ( $option_count == 0 && $option_flag  != 0 ) {
>>>         $self->options(
>>>             [ $self->form->model( $args->{model_name}
>>>  )->options_from_model( $self, $args ) ] );
>>>     }
>>>  }
>>
>> Okay, I can dig that - lets do it!
>> Do you want to make the necessary changes?
>> (I'm gonna be off-line for the next week, so won't be able to do it
>> until after then).
>
> Hmm, thinking about it further, if this happens during post_process(),
> we won't be able to use the AutoSet constraint, as that requires the
> options are available earlier on during process().
> I do consider this a big deal, as I find the AutoSet constraint very helpful.
>
> This keeps causing problems, and I'm really thinking we're going to
> have to make it less magic.
> I still like my idea of implementing this as a plugin:
>
>   type: Select
>   name: foo
>   plugins: [ 'OptionsFromModel' ]
>   model_config:
>     model: Foo
>
> (better plugin-name suggestions welcome)
>
> I suppose we could keep the current post_process behaviour for people
> [ andreas ;) ] that want it, and it just won't run if the plugin has
> already populated the options?

Or, we could change it so it runs during process() instead of post_process()
Which is, in fact, what I've just done.

The original reason it was being run during post_process() was for
efficiency, so it wasn't doing a SELECT unnecessarily after the user
submitted the form.
However, 'working' beats 'fast'.

I've also added some new tests to HTML-FormFu-Model-DBIC to test the
AutoSet constraint works, and also to test is Does The Right Thing
when you set options_from_model to false.

Carl