Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update event_listeners.rst #220

Open
wants to merge 19 commits into
base: 5.x
Choose a base branch
from

Conversation

ifeoluwafavour
Copy link
Contributor

Documentation for Event Listeners component #215

Documentation for Event Listeners component mautic#215
Copy link
Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Thanks so much for the great work on this PR @ifeoluwafavour - it's fantastic to see these docs coming over to the new documentation!

I've left a bunch of suggestions to fix the many Vale suggestions from Reviewdog, have a look and see what you think!

docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
@ifeoluwafavour
Copy link
Contributor Author

Hello @RCheesley
Thank you for your kind words. I'm learning a lot!

I have applied the suggested changes. To accurately follow the external link syntax, I created a pull request to include Symfony's docs event subscribers link file to the link folder:
https://github.com/mautic/developer-documentation-new/pull/221/checks

Copy link
Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Some tweaks to fix the heading nestings, and I've asked the devs to do a technical review, too! Thanks for the great work so far!

docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
docs/plugins/event_listeners.rst Outdated Show resolved Hide resolved
ifeoluwafavour and others added 5 commits November 7, 2024 09:25
correct heading syntax

Co-authored-by: Ruth Cheesley <[email protected]>
changed to unique heading name instead of 'subscribers'
@ifeoluwafavour
Copy link
Contributor Author

Thank you @RCheesley for your guidance so far!

I changed 'Subscribers' to 'Event Sunscribers' in the recent commit. Is it a better heading name?

Copy link
Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Looks like just a vale fix and we're good to go! 🚀 Thanks @ifeoluwafavour for all the hard work here!

// ...


2) The Event class that is received by the listeners. This class should extend ``Symfony\Component\EventDispatcher\Event``. It's created when the event is dispatched and should have any information listeners need to act on it.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
2) The Event class that is received by the listeners. This class should extend ``Symfony\Component\EventDispatcher\Event``. It's created when the event is dispatched and should have any information listeners need to act on it.
2) The Event class received by the listeners. This class should extend ``Symfony\Component\EventDispatcher\Event``. It's created when dispatching the event and should have any information listeners need to act on it.

Vale recommendations

Comment on lines +11 to +51
namespace MauticPlugin\HelloWorldBundle\EventListener;

use Mautic\LeadBundle\Event as Events;
use Mautic\LeadBundle\LeadEvents;

/**
* Class LeadSubscriber
*
* @package Mautic\LeadBundle\EventListener
*/
class LeadSubscriber extends CommonSubscriber
{

/**
* @return array
*/
static public function getSubscribedEvents()
{
return array(
LeadEvents::LEAD_POST_SAVE => array('onLeadPostSave', 0),
LeadEvents::LEAD_POST_DELETE => array('onLeadDelete', 0)
);
}

public function onLeadPostSave(LeadEvent $event)
{
$lead = $event->getLead();

// do something
}

public function onLeadDelete(LeadEvent $event)
{
$lead = $event->getLead();

$deletedId = $lead->deletedId;

// do something
}
}
// ...
Copy link
Member

Choose a reason for hiding this comment

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

A little bit more modern PHP:

Suggested change
namespace MauticPlugin\HelloWorldBundle\EventListener;
use Mautic\LeadBundle\Event as Events;
use Mautic\LeadBundle\LeadEvents;
/**
* Class LeadSubscriber
*
* @package Mautic\LeadBundle\EventListener
*/
class LeadSubscriber extends CommonSubscriber
{
/**
* @return array
*/
static public function getSubscribedEvents()
{
return array(
LeadEvents::LEAD_POST_SAVE => array('onLeadPostSave', 0),
LeadEvents::LEAD_POST_DELETE => array('onLeadDelete', 0)
);
}
public function onLeadPostSave(LeadEvent $event)
{
$lead = $event->getLead();
// do something
}
public function onLeadDelete(LeadEvent $event)
{
$lead = $event->getLead();
$deletedId = $lead->deletedId;
// do something
}
}
// ...
namespace MauticPlugin\HelloWorldBundle\EventListener;
use Mautic\LeadBundle\LeadEvent;
use Mautic\LeadBundle\LeadEvents;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
final class LeadSubscriber extends EventSubscriberInterface
{
static public function getSubscribedEvents(): array
{
return [
LeadEvents::LEAD_POST_SAVE => ['onLeadPostSave', 0],
LeadEvents::LEAD_POST_DELETE => ['onLeadDelete', 0],
];
}
public function onLeadPostSave(LeadEvent $event): void
{
$lead = $event->getLead();
// do something
}
public function onLeadDelete(LeadEvent $event): void
{
$lead = $event->getLead();
$deletedId = $lead->deletedId;
// do something
}
}
// ...

*****************
The easiest way to listen to various events is to use an event subscriber. Read more about subscribers in :xref:`symfony-event-subscribers`.

Plugin event subscribers can extend ``\Mautic\CoreBundle\EventListener\CommonSubscriber`` which gives access to commonly used dependencies and also allows registering the subscriber service through the config file for the bundle. See :ref:`plugins/config:Service config items` for more information on registering event services.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Plugin event subscribers can extend ``\Mautic\CoreBundle\EventListener\CommonSubscriber`` which gives access to commonly used dependencies and also allows registering the subscriber service through the config file for the bundle. See :ref:`plugins/config:Service config items` for more information on registering event services.
Plugin event subscribers can extend ``Symfony\Component\EventDispatcher\EventSubscriberInterface`` which gives access to commonly used dependencies and also allows registering the subscriber service through autowiring.

Comment on lines +81 to +83
/**
* Class HelloWorldEvents
*/
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/**
* Class HelloWorldEvents
*/

The comment says the same thing as the class itself so it's not necessary

// ...


2) The Event class that is received by the listeners. This class should extend ``Symfony\Component\EventDispatcher\Event``. It's created when the event is dispatched and should have any information listeners need to act on it.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
2) The Event class that is received by the listeners. This class should extend ``Symfony\Component\EventDispatcher\Event``. It's created when the event is dispatched and should have any information listeners need to act on it.
2) The Event class that is received by the listeners. This class should extend ``Symfony\Contracts\EventDispatcher\Event``. It's created when the event is dispatched and should have any information listeners need to act on it.

Comment on lines +102 to +137
<?php
// plugins\HelloWorldBundle\Event\ArmageddonEvent.php

namespace MauticPlugin\HelloWorldBundle\Event;

use Symfony\Component\EventDispatcher\Event;
use MauticPlugin\HelloWorldBundle\Entity\World;

class ArmageddonEvent extends Event
{
/** @var World */
protected $world;

/** @var bool */
protected $falseAlarm = false;

public function __construct(World $world)
{
$this->world = $world;
}

public function shouldPanic()
{
return ('earth' == $this->world->getName());
}

public function setIsFalseAlarm()
{
$this->falseAlarm = true;
}

public function getIsFalseAlarm()
{
return $this->falseAlarm;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Modern PHP with types and updated Symfony namespace for Symfony 5+

Suggested change
<?php
// plugins\HelloWorldBundle\Event\ArmageddonEvent.php
namespace MauticPlugin\HelloWorldBundle\Event;
use Symfony\Component\EventDispatcher\Event;
use MauticPlugin\HelloWorldBundle\Entity\World;
class ArmageddonEvent extends Event
{
/** @var World */
protected $world;
/** @var bool */
protected $falseAlarm = false;
public function __construct(World $world)
{
$this->world = $world;
}
public function shouldPanic()
{
return ('earth' == $this->world->getName());
}
public function setIsFalseAlarm()
{
$this->falseAlarm = true;
}
public function getIsFalseAlarm()
{
return $this->falseAlarm;
}
}
<?php
// plugins\HelloWorldBundle\Event\ArmageddonEvent.php
namespace MauticPlugin\HelloWorldBundle\Event;
use Symfony\Contracts\EventDispatcher\Event;
use MauticPlugin\HelloWorldBundle\Entity\World;
final class ArmageddonEvent extends Event
{
private bool $falseAlarm = false;
public function __construct(private World $world)
{
}
public function shouldPanic(): bool
{
return ('earth' == $this->world->getName());
}
public function setIsFalseAlarm(): void
{
$this->falseAlarm = true;
}
public function getIsFalseAlarm(): bool
{
return $this->falseAlarm;
}
}

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.

3 participants