Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
[Webhook] update doc#19974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:6.4
Are you sure you want to change the base?
[Webhook] update doc#19974
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| ------------------------------------------------------------------ | ||
| It is important to note that when the incoming webhook is processed by the :class:`WebhookController`, you have the option to handle the consumption of remote events asynchronously. | ||
| Indeed, this can be configured using a bus, with the default setting pointing to the Messenger component's default bus. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
todo: show config
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
bb79ac4 to581043fCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Just had time for a quick review of the first bit of this document.
I was a bit confused on "who was the consumer and who was the provider".
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
mxr576 commentedSep 28, 2024
Thanks for this PR, the Webhook component deserves a high quality and detailed documentation! |
faizanakram99 commentedJan 10, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@javiereguiluz what is preventing this PR from being merged? Is there anything pending? The server part of webhook still remains undocumented, it is a super useful feature of symfony especially when combined with api platform (or simply openapi docs), i think not many people know symfony can also send webhooks (not just consume them). |
| Symfony Webhook, when used alongside Symfony RemoteEvent, streamlines the management of these fundamental phases. | ||
| A Single Entry Endpoint: Receive | ||
| -------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
shouldn't this be a sub-heading of theConsuming Webhooks section instead ? With the new doc covering both consuming and providing webhooks, I think we should have only 3h2 titles:Installation,Consuming webhooks andProviding webhooks
| The routing name must be unique as this is what connects the provider with your | ||
| webhook consumer code. | ||
| .. code-block:: yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this should use aconfiguration-block to show the configuration in all formats
| For example, you can use Mercure to broadcast updates to clients of the hub, among other actions ...:: | ||
| // src/Webhook/ExampleRequestParser.php | ||
| #[AsRemoteEventConsumer('my_first_parser')] # routing name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| #[AsRemoteEventConsumer('my_first_parser')]# routing name | |
| #[AsRemoteEventConsumer('my_first_parser')]// routing name |
We don't use# as a delimiter for line comments in the Symfony coding standards
| you will need to configure the settings (as mentioned earlier) and also create your own consumer. | ||
| This is necessary because it involves your own business logic and your specific reactions to the remote event(s) that may be received from the built-in parsers. | ||
| Usage in Combination with the Mailer Component |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm not sure having 2 sections about using it in combination with mailer and notifier in the page (one in the section about the parser and one in the section about the handler) makes sense. I think it might be better to organize the content the other way around:
- usage with mailer
- usage with notifier
- usage with custom webhooks (talking about both the request parser and the handler)
| It's crucial to understand that the :class:`Symfony\\Component\\Webhook\\Controller\\WebhookController` itself remains provider-agnostic, utilizing | ||
| a routing mechanism to determine which parser should handle incoming webhooks for analysis. | ||
| As mentioned earlier, incoming webhooks require a specific prefix to be directed to the :class:`Symfony\\Component\\Webhook\\Controller\\WebhookController`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this section gives far too much details about the WebhookController, which are irrelevant for most users as they won't ever touch it directly, while not explaining the high-level picture:
webhooks are received on a URL
<prefix>/<routing-name>and routed to the appropriate request parser and handler based on the routing name.
That's the info that users need to have, with configuration examples in all formats for the routing configuration
thinking about this again, I think we should rather talk about |
Update the webhook documentation