mauricerenck / komments

A Kirby comment plugin
MIT License
45 stars 8 forks source link

Bridgy Fed webmention comments don't have author email address #60

Closed srijan closed 1 year ago

srijan commented 1 year ago

Hi. I was trying to setup indieConnector + komments, and faced an issue (after working around mauricerenck/indieConnector#8):

The webmentions from mastodon/fediverse via fed.brid.gy do not have author email address, so I get the following error:

AH01071: Got error 'PHP message: Whoops\Exception\ErrorException: Undefined array key "email" in /var/www/html/site/plugins/komments/utils/receiveKomment.php:54
Stack trace:
#0 /var/www/html/site/plugins/komments/utils/receiveKomment.php(54): Whoops\Run->handleError()
#1 /var/www/html/site/plugins/komments/components/hooks.php(31): mauricerenck\Komments\KommentReceiver->createKomment()
#2 [internal function]: Kirby\Cms\App->mauricerenck\Komments\{closure}()
#3 /var/www/html/vendor/getkirby/cms/src/Toolkit/Controller.php(58): Closure->call()
#4 /var/www/html/vendor/getkirby/cms/src/Cms/Event.php(165): Kirby\Toolkit\Controller->call()
#5 /var/www/html/vendor/getkirby/cms/src/Cms/App.php(1700): Kirby\Cms\Event->call()
#6 /var/www/html/site/plugins/indieconnector/utils/hookHelper.php(18): Kirby\Cms\App->trigger()
#7 /var/www/html/site/plugins/indieconnector/index.php(63): mauricerenck\IndieConnector\HookHelper->triggerHook()
#8 [internal function]: Kirby\Http\Route->mauricerenck\IndieConnector\{closure}()
#9 /var/www/htm...'

If I add a check array_key_exists('email', $webmention['author']) and pass an empty string as authorEmail if the email does not exist, it works.

Should I raise a PR with this change? Are there other fields you recommend checking for existence before saving the comment?

mauricerenck commented 1 year ago

Thank you for that feedback :) That is actually a fault of a newly added feature, where I added the option to save the email address of the sender of a comment. I didn't had in mind that there might be no address in case of a webmention…

I will change the setEmail function, so the handling of unset/empty addresses is completely in its domain. I'll also extend the tests to cover that case.

srijan commented 1 year ago

It works now. Thanks :)