[Catalyst] Advice on this controller pattern

[Bill Moseley]

On Mon, Oct  6 16:12:51 2008 Oliver Charles wrote:

Gerda Shank just pointed me to this message as it looks like you are using
Form::Processor.  Is that correct?

> Sorry for the shakey subject title, but that's the best way I can
> think of to sumarise. Anyway, in my project at the moment I often have
> forms where the user is moving something, and needs to first search
> for a destination. An example is changing the track artist: first the
> user searches for a new artist, then they enter this edit with a
> moderation note.

What's the reason for searching first?  I assume that there's a
relationship between track and author so the form would automatically
create is selection list when updating the track.

If the selection list is too huge to manage then maybe an AJAX helper
to search for the new artist instead of a long select list.

[...]

> package Foo::Search;
> 
> sub filter_artist : Private
> {
>     my ($self, $c) = @_;
> 
>     my $form = $c->form(undef, 'Search::Query');
> 
>     return unless $c->form_posted && $form->validate($c->req->params);
> 
>     my $artists = $c->model('Artist')->direct_search($form->value('query'));
>     $c->stash->{artists} = $artists;
> }
> 
> package Foo::Track;
> 
> sub change_artist : Chained('track')
> {
>     my ($self, $c) = @_;
> 
>     $c->forward('/user/login');
>     $c->forward('/search/filter_artist');
> 
> }
> 
> sub confirm_change_artist : Chained('track') Args(1)
> {
>     my ($self, $c, $new_artist) = @_;
> 
>     $c->forward('/user/login');
> 
>     my $track      = $c->stash->{track};
>     my $new_artist = $c->model('Artist')->load($new_artist);
>     $c->stash->{new_artist} = $new_artist;
> 
>     my $form = $c->form($track, 'Track::ChangeArtist');
>     $form->context($c);
> 
>     return unless $c->form_posted && $form->validate($c->req->params);
> 
>     # Database edit here:
>     $form->change_artist($new_artist);
> 
>     my $release = $c->model('Release')->load($track->release);
>     $c->response->redirect($c->entity_url($release, 'show'));
> }


> The user would visit change_artist and be presented with a search
> form. They submit back to change_artist, and selecting an artist
> navigates them to confirm_change_artist, with a new form.

Three step process?  Search form, then the confirm form, then the
change?  I'd rather have a single form that updates and an option to
"undo" after the change.  After all, the majority of the time the
answer is "Yes!" to "Are you sure?".


Also, I find as the application gets larger that it's really handy to
have the form classes map to the actions (as well as templates) so
that I don't have to hard-code the form class in each action. The
Form::Processor plugin does that work for you.  The plugin is suppose
deal with the stuff you would write over-and-over.

So, for example this is how I tend to write something like the above,
which is really almost a single line:


    sub change_track : Local {
        my ( $self, $c, $id ) = @_;

        return unless $c->update_from_form( $id );


        my $track = $c->stash->{form}->item;

        return $c->post_redirect(
            $c->uri_for( 'release', $track->release ),
            'Track updated',
        );
    }

Note that the plugin passes $c to the form object, so you should not
need to set $form->context in each action if you need $c in your form.

The $id has to get validated, of course, both for format and for
access control.  But, if the track can only be updated by, say, the
owner of the object then that check probably should happen in the
model or in the form.

I'd also suggest dealing with login issues higher up -- individual
actions should not need to bother with such administrative detail.


Again, I like the "undo" approach better than the "Are you sure?"
approach.  But, it's not always easy to do. I posted about that a year
or so ago.  I currently have a simple implementation that stores
the previous version of the object for a very short term that can be
used to "undo" the last change.  So, I just set a flag on actions that
can be undone and the page I redirect to will include something like
this with the undo link added:

            Track updated -- undo

But the undo action has to be sure that the item has not changed in
the mean time.


-- 
Bill Moseley
moseley at hank.org
Sent from my iMutt