RCP: "done" closure

[adamkiss]

Is your feature request related to a problem? Please describe.
I'd like to log all requests - successes or otherwise.

Describe the solution you'd like
I think the simplest, most expressive solution would be to manually set some public attribute doneClosure, which would be called from the $form->done() method.

$form = new Form();
$form->doneClosure = fn($form) => log($form);

Describe alternatives you've considered
I considered passing the closure to the done method $form->done(fn($form) => log($form)) instead, but the form itself calls done() without any argument when failing, for instance, and I couldn't figure out better way to pass it around other than have it on the object and call it from done() if not null.

I currently extend the Form object - and the extension contains only the overloaded done() method and private doneClosure().

Additional context
I will PR this if accepted

[mzur]

Thanks for the suggestion! Considering this snippet from the example in the readme, why would you prefer the described "done closure" to something like this?

if ($kirby->request()->is('POST')) {
  log($form);
  $form->emailAction([
     'to' => 'me@example.com',
     'from' => 'info@example.com',
  ])->done();
}

[adamkiss]

@mzur Because, AFAICT, at that point in code, $form doesn't know if it's a success, or an error, or where that error happened, or at least I think that was my reasoning at the time.

Edit: Also, you can't modify flash and/or errors from outside, if your code requires it.

I now see I could theoretically do this via validates()?:

if ($kirby->request()->is('POST')) {
    $valid = $form->validates();
    // log stuff
    log($form);

    $form->someAction()->done();
} 

[mzur]

That makes sense. Calling validates() directly is not a good alternative, as it circumvents some of the Uniform logic. I'd like the implementation of your closure setter to support the same method chaining than the actions etc. of Uniform. Something like this:

$form->finally(fn ($form) => log($form))->someAction()->done();

The setter must be called before the actions, of course, otherwise the closure would not be called on error. I'm just not sure how to call the setter. finally() sounds ok but I'd expect it to be called at the end of the chain and not the beginning. onDone() could work but this might bring the impression that it is only called on success. Do you have another idea?

[adamkiss]

I've been working with Uniform again recently, and realised I never got around to replying to you! I am very sorry I left it here opened.

So, as it stands, I can either call withoutFlashing() and withoutRedirect() and completely manage the error/success myself, or another solution I came up with would be (slightly) inspired by GitHub actions (and Kirby hooks):

$form
	->on('error', function(Form $form) { /* on error done */})
	->on('success', function(Form $form) { /* on success done */})
	->on('always', function(Form $form) { /* always done */})
	->action();

Then there is question if this would support multiple hooks (for whatever reasons), but otherwise I think it's quite readable and relatively simple.

Edit: I've now started a PR #261

[mzur]

Ok thanks, so let's continue in #261.