New campaign action: Send a webhook #4357
Conversation
…ign-action-remote-call-url
…campaign-action-remote-call-url
|
This is brilliant! |
escopecz
added
feature
| return $event->setResult(true); | ||
| } | ||
| } catch (\Exception $e) { | ||
| } |
There was a problem hiding this comment.
We have to let the users know why the request failed. It fails silently without any clue of what happened. Whether it returned 500 or timeouted. Could you please add some error messages so the users won't report errors like "the campaign URL call doesn't work", but give them information so they could debug themselves?
You can inspire here: https://github.com/mautic/mautic/blob/staging/app/bundles/PluginBundle/EventListener/CampaignSubscriber.php#L116
There was a problem hiding this comment.
Improved. Please check it
| */ | ||
| protected $connector; | ||
|
|
||
| /** | ||
| * CampaignSubscriber constructor. | ||
| * | ||
| * @param IpLookupHelper $ipLookupHelper | ||
| * @param AuditLogModel $auditLogModel |
There was a problem hiding this comment.
Add the new dependency to the annotation please.
| $data = array_flip(AbstractFormFieldHelper::parseList($data)); | ||
| if (in_array($method, ['get', 'trace'])) { | ||
| $response = $this->connector->$method( | ||
| $config['url'], |
There was a problem hiding this comment.
Maybe the $data should be added as a URL query params in this case?
escopecz
added
pending-feedback
| } | ||
| if (in_array($response->code, [200, 201])) { | ||
| return $event->setResult(true); | ||
| } |
There was a problem hiding this comment.
I'd also add something like
else {
$event->setFailed('Response returned HTTP code '.$response->code);
}
I'm not sure whether the error is run through the translator later. Maybe you'll have to add Translator as a dependency to this Subscriber.
There was a problem hiding this comment.
Ok, should be resolved with the previous commit
| return $event->setResult(true); | ||
| } | ||
| } catch (\Exception $e) { | ||
| return $event->setResult($e->getMessage()); |
There was a problem hiding this comment.
The method should be setFailed instead of setResult.
kuzmany
changed the title
escopecz
added
ready-to-test
| return; | ||
| } | ||
| $config = $event->getConfig(); | ||
| $timeout = 10; |
There was a problem hiding this comment.
Does timeout get set to the value specified by the user?
There was a problem hiding this comment.
Yes, already update with timeout option. Check screenshot
|
Rather than just an authorization header, wouldn't make more sense to allow any custom header similar to the way you can add any data? |
|
@Loanatik please send examples, I don't know about it |
|
I don't personally have a big need for this, though I'd imagine others will: I have not tested the data section for tokens. Am I correct in assuming that that they should work? |
kuzmany
changed the title
|
Do we have a status on this? Any idea when it will be included? Cheers! :) |
|
@calevans Mautic 2.10 |
Moved to WebhookBundle
|
Please re-test. |
|
It's failing some CI tests. Please look at those when you get a minute. |
|
@escopecz morning. fixed |
…as they are not applicable to this scenario and added missing DELETE
alanhartless
added
the
pending-test-confirmation
escopecz
added
ready-to-commit
escopecz
removed
ready-to-commit
|
Does it support the payload like in the attached picture |
I want to know the status / response of a api call in send a webhook, too, do you know that? |
Description:
Just add new action with curl call to url
Return true If response code = 200 or 201.
Note: this PR contains #4355 which should be merged first.
Thanks to @Loanatik for donate this feature.
Steps to test this PR: