search

arnoson / kirby-forms

Form builder and handler for Kirby 4
MIT License
10 stars 3 forks source link

Form state incorrect if notification does not send #12

Closed n-kort closed 6 months ago

n-kort commented 1 year ago

I'm running into another issue with the form/notification setup. I haven't setup any SMTP settings so I assume Kirby just uses PHPMailer to send the notifications (directly to spam). This works locally, running a Kirby site with Laravel Valet. On my server though, configured with dokku, I guess the emails are not sending at all.

The issue is, in this case the form does page does not update to reflect that the form content has actually been saved. It just returns 302 and loads the form again, with the data you've just submitted. The entries do show up in Kirby however.

This is for sure an issue with my email setup, but perhaps it should show the success message to the user nonetheless?

Edit: the issue on the server is: /usr/sbin/sendmail: not found, which makes sense.

n-kort commented 1 year ago

Update: adding postfix to the dokku app means the form is updating correctly. Server now says postdrop: warning: unable to look up public/pickup: No such file or directory so emails are obviously not sending but that's not the plugin's fault :). I suppose it gets further through the submit -> save -> notify -> render form flow than before.

arnoson commented 1 year ago

So do I understand correct, that the email issues come from the server setup, but you suggest that the form should be more fault tolerant and not crash in case the email can't be send?

n-kort commented 1 year ago

Yes :)

Not urgent of course, using a proper SMTP transport resolves the problem, I was just getting confused that the form wasn't updating even though the data was being saved.

arnoson commented 1 year ago

Yes I agree this would be nice. A failed notification email shouldn't concern the user. Under the hood the form just uses Kirby Uniforms email action, so the error suppression should happen there. I'm on vacation soon, but PRs are welcome (but a merge would have to wait until september)

arnoson commented 6 months ago

I thought about this and decided to run the save action last, so if anything fails before, like the email action, the whole form fails. So we don't end up in the weird state you described. If the user sees an error, he/she knows that the form couldn't be send. Maybe the confirmation email would have contained an important information, or the website relies on the notification email because they don't check the kirby panel regularly.

This of course would mean correct error handling. Right now the errors that happen inside the actions don't show up, which I want to fix soon!