Need some advice on the correct way to "stop" a rendering (?)

[fluca1978]

I need some advice: I've a controller method that is going to display details about a resource.
What to do if the resource is not found or the user does not have permissions to see the details?

So far, I've implemented this:

sub detail( $self ) {
    my $pk = $self->param( 'pk' );
    my $user  = $self->current_user;  # helper method, get user from session
    $self->stash( issue => undef );

    if ( ! $pk || ! $user ) {
         # stop here!
	return;
    }

    my $issue = $self->db_schema->resultset( 'Issue' )->find( { pk => $pk } );

    # the issue must be visible for this user!
    if ( $issue && $issue->is_visible_by( $user ) ) {
	$self->stash( issue => $issue );
    }
    else {
	$self->log->debug( sprintf 'Issue [%s] not found or not visible by [%s]',
			   $pk, $user->username );
        # stop here
       return;
    }


    $self->render;
}

Questions:

• is the return the correct way to stop the controller method?
• should I perform e redirect to an error page instead of a return?

[ghandmann]

Technically it is totally fine to return early from a controller. But this leads to bad user experience, since the user does not get any clues what went wrong and is just left behind on a blank screen.

Better options here would be either a redirect - as you already mentioned. Or simply render a different template/text message to the client.

if($not_allowed) {
   $self->render(text => "access not allowed", status => 403);
   return;
}

For more sophisticated authentication logic you may even utilize the under method of Mojolicious. This way you may even prevent execution of your controller action completely, since the wrapping code in under already handles unauthenticated users.

[fluca1978]

For more sophisticated authentication logic you may even utilize the under method of Mojolicious. This way you may even prevent execution of your controller action completely, since the wrapping code in under already handles unauthenticated users.

Yes, I know, but I have to run the code either with authenticated or not-authenticated uses, so I prefer to check the authentication in the controller method (in the above example I was quitting if not authenticated, but then I will add another path for unauthenticated users).
So far I'm using under to group routes:

my $issue_route = $urls->under( '/issue' );                                                                                                
     $issue_route->get( '/create')->to( controller => 'Issue',                                                                                  
                                      action => 'create' )                                                                                      
             ->name( q/issue_create/ );                                                                                                         
                                                                                                                                                
     $issue_route->get( '/list/' )->to( controller => 'Issue',                                                                                  
                                        action => 'list' )                                                                                      
             ->name( q/issue_list/ );                             
...

Please warn if the above is wrong.

[ghandmann]

No real warning, but if you really only want to group routes, you may simply use any instead.

I'm not 100% sure, but i would assume that under brings some (probably rather tiny) performance impact which can be omitted by using any.