Skip to content

Document Mautic 8 class-name dispatch for WebhookBundle events - #646

Draft
promptless-for-oss wants to merge 3 commits into
mautic:7.2from
Promptless:promptless/pr-17202-webhook-events-by-class
Draft

Document Mautic 8 class-name dispatch for WebhookBundle events#646
promptless-for-oss wants to merge 3 commits into
mautic:7.2from
Promptless:promptless/pr-17202-webhook-events-by-class

Conversation

@promptless-for-oss

Copy link
Copy Markdown
Contributor

Open in Promptless

Mautic 8 converts three WebhookBundle events to Symfony 4.3-style dispatch-by-class-name, so a Plugin that subscribes to them must key getSubscribedEvents() on the event class instead of the WebhookEvents string constant. A subscriber left keyed on a converted constant stops firing silently — with no exception and no log — so plugin developers need to know which events changed and which did not.

This adds a note to the "Available events" section of docs/plugins/event_listeners.rst explaining the new keying convention: the three converted events (WebhookBuilderEvent, WebhookQueueEvent, WebhookRequestEvent) key on the event class, while the shared-class WebhookEvent family (WEBHOOK_PRE_SAVE, WEBHOOK_POST_SAVE, WEBHOOK_PRE_DELETE, WEBHOOK_POST_DELETE, WEBHOOK_KILL) still dispatches by constant. It mirrors the note added for the sibling IntegrationsBundle conversion.

Trigger Events

Note that WebhookBuilderEvent, WebhookQueueEvent, and WebhookRequestEvent
now dispatch by class name (Symfony 4.3 style), so subscribers must key
getSubscribedEvents() on the event class rather than the WebhookEvents
constant. Mirrors mautic/mautic PR #17202.

.. note::

Since Mautic 8, Mautic dispatches three ``Mautic\WebhookBundle\Event`` events by the event object alone, so the event class is the event name. This matches the Symfony 4.3 dispatch style.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mautic 8 (PR #17202) dispatches WebhookBuilderEvent (getEvents()) by the event object alone via $this->dispatcher->dispatch($event), dropping the former WebhookEvents::WEBHOOK_ON_BUILD name argument.

Source: https://github.com/mautic/mautic/blob/f1c6d9d439b7fcd1da1f4db71bf5c8878e7cf0f1/app/bundles/WebhookBundle/Model/WebhookModel.php#L213-L214


.. note::

Since Mautic 8, Mautic dispatches three ``Mautic\WebhookBundle\Event`` events by the event object alone, so the event class is the event name. This matches the Symfony 4.3 dispatch style.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

queueWebhook() now checks hasListeners(Events\WebhookQueueEvent::class) and dispatches WebhookQueueEvent by class alone, replacing the former WebhookEvents::WEBHOOK_QUEUE_ON_ADD constant usage.

Source: https://github.com/mautic/mautic/blob/f1c6d9d439b7fcd1da1f4db71bf5c8878e7cf0f1/app/bundles/WebhookBundle/Model/WebhookModel.php#L277-L279


.. note::

Since Mautic 8, Mautic dispatches three ``Mautic\WebhookBundle\Event`` events by the event object alone, so the event class is the event name. This matches the Symfony 4.3 dispatch style.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CampaignHelper::fireWebhook() dispatches WebhookRequestEvent by the event object alone, replacing the former WebhookEvents::WEBHOOK_ON_REQUEST constant usage.

Source: https://github.com/mautic/mautic/blob/f1c6d9d439b7fcd1da1f4db71bf5c8878e7cf0f1/app/bundles/WebhookBundle/Helper/CampaignHelper.php#L38-L39


Since Mautic 8, Mautic dispatches three ``Mautic\WebhookBundle\Event`` events by the event object alone, so the event class is the event name. This matches the Symfony 4.3 dispatch style.

* For a converted event, key ``getSubscribedEvents()`` on the event class, for example ``WebhookBuilderEvent::class``, not on the matching ``Mautic\WebhookBundle\WebhookEvents`` constant or its string value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converted subscriber keys getSubscribedEvents() on WebhookBuilderEvent::class and WebhookQueueEvent::class instead of the former WebhookEvents string constants.

Source: https://github.com/mautic/mautic/blob/f1c6d9d439b7fcd1da1f4db71bf5c8878e7cf0f1/app/bundles/EmailBundle/EventListener/WebhookSubscriber.php#L26-L27

Since Mautic 8, Mautic dispatches three ``Mautic\WebhookBundle\Event`` events by the event object alone, so the event class is the event name. This matches the Symfony 4.3 dispatch style.

* For a converted event, key ``getSubscribedEvents()`` on the event class, for example ``WebhookBuilderEvent::class``, not on the matching ``Mautic\WebhookBundle\WebhookEvents`` constant or its string value.
* The ``WebhookEvents`` constants remain in the codebase but are no longer used to dispatch these events, so a subscriber still keyed on a converted constant won't fire. It fails silently: it throws no exception and logs nothing, and simply never runs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WebhookEvents constants WEBHOOK_ON_BUILD, WEBHOOK_QUEUE_ON_ADD, and WEBHOOK_ON_REQUEST remain defined in the class for backward compatibility even though they are no longer used to dispatch these three events.

Source: https://github.com/mautic/mautic/blob/f1c6d9d439b7fcd1da1f4db71bf5c8878e7cf0f1/app/bundles/WebhookBundle/WebhookEvents.php#L52-L73

Comment thread docs/plugins/event_listeners.rst Outdated

* For a converted event, key ``getSubscribedEvents()`` on the event class, for example ``WebhookBuilderEvent::class``, not on the matching ``Mautic\WebhookBundle\WebhookEvents`` constant or its string value.
* The ``WebhookEvents`` constants remain in the codebase but are no longer used to dispatch these events, so a subscriber still keyed on a converted constant won't fire. It fails silently: it throws no exception and logs nothing, and simply never runs.
* Mautic 8 converted only ``WebhookBuilderEvent``, ``WebhookQueueEvent``, and ``WebhookRequestEvent``, each of whose class maps to a single name. Families whose class serves several names, such as ``WebhookEvent`` (dispatched for ``WEBHOOK_PRE_SAVE``, ``WEBHOOK_POST_SAVE``, ``WEBHOOK_PRE_DELETE``, ``WEBHOOK_POST_DELETE``, and ``WEBHOOK_KILL``), still dispatch by their ``WebhookEvents`` constants, so keep keying on the constant for those.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dispatchEvent() still maps WEBHOOK_PRE_SAVE/POST_SAVE/PRE_DELETE/POST_DELETE to the shared WebhookEvent class and dispatches by the WebhookEvents string constant ($name), unconverted because one class serves several event names.

Source: https://github.com/mautic/mautic/blob/f1c6d9d439b7fcd1da1f4db71bf5c8878e7cf0f1/app/bundles/WebhookBundle/Model/WebhookModel.php#L629-L657

Comment thread docs/plugins/event_listeners.rst Outdated

* For a converted event, key ``getSubscribedEvents()`` on the event class, for example ``WebhookBuilderEvent::class``, not on the matching ``Mautic\WebhookBundle\WebhookEvents`` constant or its string value.
* The ``WebhookEvents`` constants remain in the codebase but are no longer used to dispatch these events, so a subscriber still keyed on a converted constant won't fire. It fails silently: it throws no exception and logs nothing, and simply never runs.
* Mautic 8 converted only ``WebhookBuilderEvent``, ``WebhookQueueEvent``, and ``WebhookRequestEvent``, each of whose class maps to a single name. Families whose class serves several names, such as ``WebhookEvent`` (dispatched for ``WEBHOOK_PRE_SAVE``, ``WEBHOOK_POST_SAVE``, ``WEBHOOK_PRE_DELETE``, ``WEBHOOK_POST_DELETE``, and ``WEBHOOK_KILL``), still dispatch by their ``WebhookEvents`` constants, so keep keying on the constant for those.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

killWebhook() still constructs the shared WebhookEvent class and dispatches by the WebhookEvents::WEBHOOK_KILL string constant, unconverted.

Source: https://github.com/mautic/mautic/blob/f1c6d9d439b7fcd1da1f4db71bf5c8878e7cf0f1/app/bundles/WebhookBundle/Model/WebhookModel.php#L412-L417


.. note::

Since Mautic 8, Mautic dispatches three ``Mautic\WebhookBundle\Event`` events by the event object alone, so the event class is the event name. This matches the Symfony 4.3 dispatch style.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symfony 4.3 event-dispatcher changelog documents the dispatch(Event $event, string $eventName = null) signature change (event object first, name optional/derived from class), which is the dispatch style Mautic 8 now uses for the converted WebhookBundle events.

Source: https://github.com/symfony/event-dispatcher/blob/v4.3.0/CHANGELOG.md

@adiati98 adiati98 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@promptless-for-oss please address the Vale warnings. We use hyphen instead of parentheses.

@promptless-for-oss

Copy link
Copy Markdown
Contributor Author

Thanks @adiati98 — on it. I'll address the Vale warnings and switch the parenthetical asides to hyphens, run Vale, and push the updates to this PR shortly.

…side

Convert the parenthetical aside in the Available events note to the
repo's spaced-hyphen form, per maintainer review on PR mautic#646.
@promptless-for-oss

Copy link
Copy Markdown
Contributor Author

Done @adiati98 — converted the parenthetical aside in the WebhookBundle events note to the spaced-hyphen form, which also clears the Vale (Google.Parens) warning. vale docs/plugins/event_listeners.rst now returns zero findings. Pushed to this PR branch — ready for another look.

@promptless-for-oss

Copy link
Copy Markdown
Contributor Author

I noticed that some CI checks failed for this PR. I'm investigating whether the failures are caused by this suggestion. If they're unrelated or pre-existing, I'll leave this suggestion unchanged and create a separate suggestion if a standalone docs fix is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants