Problem/Motivation

In #2264049: Create an Events topic we provided a solution to document events, more background there.

One example would be

    /**
     * Denotes an event at the very beginning of request dispatching.
     *
     * This event allows you to create a response for a request before any
     * other code in the framework is executed. The event listener method
     * receives a \Symfony\Component\HttpKernel\Event\GetResponseEvent
     * instance.
     *
     * @Event
     *
     * @var string
     */
    const REQUEST = 'kernel.request';

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because it is not a bug (doesn't effect functionality), and not a feature request.
Unfrozen changes Unfrozen because it only changes documentation

This is allowed because documentation changes are unfrozen. (Does not need disruption evaluated.)

Proposed resolution

  • Find all events in Drupal core, all classes with the name *Events, probably
  • Add the @Event annotation to all of them.
  • On top of that describe what kind of $event object is passed in, see previous example.

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Agree on formatting of the example

User interface changes

No.

API changes

No.

Comments

dawehner’s picture

dawehner’s picture

jhodgdon’s picture

In the issue summary for the other issue, I proposed that the documentation should include:
- What the event is
- What it is used for
- What kind of $event object is passed along.
- @see links to related classes (event class, representative class where event is dispatched, representative class where event is subscribed to).

Let's treat this as the "gold standard" for great event docs; at a minimum I think we want to make sure we have the first three items documented.

As a note, the example in the issue summary is maybe not great. It seems like the first-line summary should probably be standardized... maybe something like:

Indicates a termination event, which is triggered when ...

???

dawehner’s picture

Issue summary: View changes

As a note, the example in the issue summary is maybe not great. It seems like the first-line summary should probably be standardized... maybe something like:

Well, this is an example of symfony, which we obviously can't "fix", but to be honest this is a really uncommon example (not used by Drupal).
Updated the IS with another example.

jhodgdon’s picture

Hm. The suggested doc block needs a one-line summary... and normally one-line summaries are not declarative sentences like "The foobar function does this" or "The FOOBAR constant is this", but more like "Calculates the foo of the bar" for functions, and "Indicates that foo is very bar-like" for constants.

So again, I'd suggest we adopt a standardized one-line summary, something more like:

Denotes an event at the very beginning of request dispatching.

or something like that. (Indicates? Encodes? Denotes? I am not sure what verb to use.)
Also any mention of a class in a doc block should start with a fully-qualified namespace, so \Symfony\... (start with backslash).

And we do not use @api in our docs.

yesct’s picture

Issue summary: View changes

added beta evaluation to summary.

yesct’s picture

Issue summary: View changes

moved summary reason outside the table so it showed after the table.

jhodgdon’s picture

I like the example currently in the issue summary.

dawehner’s picture

Sorry totally forgot about this issue ...

I like the example currently in the issue summary.

Great, does that mean we can work on the policy? See my latest comment on the critical one: #2264049-38: Create an Events topic

eojthebrave’s picture

Here's a list of the events I could find dispatched by some Drupal component or another. Based on searching for uses of EventDispatcher::dispatch(). There may be others but this is what I could find.

Should we open separate tickets for each set of events so that it's easier to review, or just do them all in this ticket?

ConfigEvents
$this->eventDispatcher->dispatch(ConfigEvents::DELETE, new ConfigCrudEvent($this));
$this->eventDispatcher->dispatch(ConfigEvents::SAVE, new ConfigCrudEvent($this));
$this->eventDispatcher->dispatch(ConfigEvents::RENAME, new ConfigRenameEvent($config, $old_name));
$this->eventDispatcher->dispatch(ConfigEvents::IMPORT, new ConfigImporterEvent($this));
$this->eventDispatcher->dispatch(ConfigEvents::IMPORT_VALIDATE, new ConfigImporterEvent($this));
$this->eventDispatcher->dispatch(ConfigEvents::COLLECTION_INFO, $this->configCollectionInfo);

EntityTypeEvents
$this->eventDispatcher->dispatch(EntityTypeEvents::CREATE, new EntityTypeEvent($entity_type));
$this->eventDispatcher->dispatch(EntityTypeEvents::DELETE, new EntityTypeEvent($entity_type));
$this->eventDispatcher->dispatch(EntityTypeEvents::UPDATE, new EntityTypeEvent($entity_type, $original));

FieldStorageDefinitionEvents
$this->eventDispatcher->dispatch(FieldStorageDefinitionEvents::CREATE, new FieldStorageDefinitionEvent($storage_definition));
$this->eventDispatcher->dispatch(FieldStorageDefinitionEvents::DELETE, new FieldStorageDefinitionEvent($storage_definition));
$this->eventDispatcher->dispatch(FieldStorageDefinitionEvents::UPDATE, new FieldStorageDefinitionEvent($storage_definition, $original));

RenderEvents
$this->eventDispatcher->dispatch(RenderEvents::SELECT_PAGE_DISPLAY_VARIANT, $event);

RoutingEvents
$this->dispatcher->dispatch(RoutingEvents::DYNAMIC, new RouteBuildEvent($collection));
$this->dispatcher->dispatch(RoutingEvents::ALTER, new RouteBuildEvent($collection));
$this->dispatcher->dispatch(RoutingEvents::FINISHED, new Event());

BlockEvents
$this->dispatcher->dispatch(BlockEvents::ACTIVE_CONTEXT, new BlockContextEvent())->getContexts();

This block event, which is just a string and not a constant. Bug?
core/modules/block/src/BlockForm.php
BlockForm::form
$form_state->setTemporaryValue('gathered_contexts', $this->dispatcher->dispatch(BlockEvents::ADMINISTRATIVE_CONTEXT, new BlockContextEvent())->getContexts());

LanguageConfigOverrideEvents
$this->eventDispatcher->dispatch(LanguageConfigOverrideEvents::DELETE_OVERRIDE, new LanguageConfigOverrideCrudEvent($this));
$this->eventDispatcher->dispatch(LanguageConfigOverrideEvents::SAVE_OVERRIDE, new LanguageConfigOverrideCrudEvent($this));
jhodgdon’s picture

I think we should make separate issues for the different modules/components.

I don't see the problem with the block event. It looks like the first argument to dispatch() is still a constant?

And by the way, the API module is ready to display @Event on api.drupal.org on the new Events topic... once we deploy a new version to api.drupal.org anyway, which I'll probably make happen in the next two weeks. Hooray! It already works on my test site with the Symfony events, which already have @Event in them.

jhodgdon’s picture

Update: as the Symfony events already have @Event on them, and the API module is deployed, they are now being displayed on api.drupal.org here:
https://api.drupal.org/api/drupal/core!modules!system!core.api.php/group...

The sooner we can get this done with our event constants, the better!

jhodgdon’s picture

Perfect, thanks!

jhodgdon’s picture

Title: Add @Event to all events defined by drupal core » [meta] Add @Event to all events defined by drupal core
eojthebrave’s picture

I think we can close this now. All of the child issues are resolved.

I also updated https://www.drupal.org/coding-standards/docs with information about the @Event tag since that didn't get done in #2264049: Create an Events topic, though that got closed already.

jhodgdon’s picture

Great! Thanks for all the patches too!

jhodgdon’s picture

Status: Active » Fixed
dawehner’s picture

Great work!

jhodgdon’s picture

webchick’s picture

So. AWESOME. Thanks so much, folks!!

xano’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.