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

Add support for events, listeners, and dispatchers #64

Open
wants to merge 92 commits into
base: main
Choose a base branch
from

Conversation

JoelAlphonso
Copy link
Contributor

@JoelAlphonso JoelAlphonso commented Sep 12, 2022

Provide league\event\dispatcher as a pimple container dependency in app package

Changes

Features

  • app: add event dispatcher provided by league/event (258be3f)
  • app: add EventDispatcherTrait (01fa79f)
  • app: add PropertyEvent for Property related events (9588d07)
  • elfinder: dispatch FileWasUploaded on elfinder upload.presave (85bc7a3)
  • event: add a base event and a (wip) generic event (c14167e)
  • event: add a charcoal EventDispatcher which extends League\Event\EventDispatcher to add logging (e18846a)
  • event: add a FileWasUploaded event (a5ab5c6)
  • event: add an AbstractEventListener and EventListenerInterface (1025394)
  • event: Add an event service provider and improve listeners registration (78bf487)
  • event: add events related to object (8080cb7)
  • event: add interruptable contract for events which provides events with a mean to interrupt the event chain and provide a reason for it (4c4d5b3)
  • event: add support for ListenerSubscriber through config files and add AbstractListenerSubscriber.php (84f8911)
  • object: add an ObjectServiceProvider for object package services (a122ba4)
  • property-event: add data and fix event const for PropertyEvent (d8ad508)
  • property: (wip) add EventDispatcherTrait to AbstractProperty (8ad2ce7)
  • property: dispatch events for pre-save and save event for ImageProperty (2a03032)
  • proxies: implement static proxies system using the facade model (e9ffdcf)
  • revision: add properties and propertyBlacklist options for Revisions (baafa78)
  • revision: add a listener to hook revision generation (6e5155f)
  • revision: add a revision service to handle all revision related operations (07c37c3)
  • revision: add RevisionConfig.php to cast revisions config nodes to (e4d2429)
  • revision: remove revision generation from Content.php (6f561de)
  • revision: use the revisions service everywhere and refactor it to require setting a model on the service (61c19f3)
  • translation-parser: add translation-parser script tag to default.admin.config (df684f1)

Bug Fixes

  • event: fix bad references to event package (3d9c4a2)
  • event: fix missing renaming (4cc8bf7)
  • event: rename event services container keys to be prefixed with app/ (a8f639c)
  • exception: fix missing sprintf context (d69a37b)
  • image-property: remove event dispatch for property for now (886532b)
  • psr4: fix some namespacing issues (70a7757)
  • revision: fix included properties and properties options (ff0bd59)
  • revisions: fix remaining revisionService renaming (2336a50)
  • translation-parser: fix paths related problems with translation-parser script (29349dc)

Todo

composer.json Outdated Show resolved Hide resolved
packages/app/README.md Outdated Show resolved Hide resolved
@mcaskill mcaskill changed the title Pulling joel_feat_event-dispatcher into main Add support for events, listeners, and dispatchers Sep 12, 2022
@JoelAlphonso JoelAlphonso force-pushed the joel_feat_event-dispatcher branch 3 times, most recently from 64b146f to 48006be Compare September 14, 2022 14:56
Copy link
Collaborator

@mcaskill mcaskill left a comment

Choose a reason for hiding this comment

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

Hard-coded references to the cache and www directories should be replaced with paths in This should use AppConfig.

For the Static Website, the static directory path should be defined from a configuration option (something like static_website.path or whatever matches similar options for Cache, View, and Metadata options).

I'm unfamiliar with the Static Website feature, what's the difference between cache/static and www/static?

Copy link
Collaborator

@mcaskill mcaskill left a comment

Choose a reason for hiding this comment

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

For the Object events, should we use Model instead?

JoelAlphonso and others added 19 commits November 11, 2022 15:11
…nts with a mean to interrupt the event chain and provide a reason for it
Co-authored-by: Chauncey McAskill <[email protected]>
- add a simple bootstrapping mechanic
- add Facade
- add Event Facade
Co-authored-by: Chauncey McAskill <[email protected]>
Co-authored-by: Chauncey McAskill <[email protected]>
JoelAlphonso and others added 3 commits November 11, 2022 15:17
Changed:
- Replaced ArrayAccess with Pimple Container as resolver for stricter binding and future transition to PSR-11.
- Added missing types `void` and `object` to various methods.
- Moved `RuntimeException` if instance is not an object from `__callStatic()` to `resolveFacadeInstance`
Moved anonymous function on "upload.presave" event to a method, `dispatchEventOnUploadPreSave`, for easier customization.
mcaskill and others added 2 commits November 18, 2022 16:51
Co-authored-by: Xavier Aymond <[email protected]>
Despite `$pimple` being the correct parameter name [1], using `$container` falls in line with all other occurrences in service providers and elsewhere.

The inconsistent parameter can pose problems with PHP 8's named arguments and for static analysis tools like PHPStan and Psalm which will flag this as an error.

This can be corrected in a future changeset that replaces all type-hints of Pimple's Container with the PSR-11 Container interface.

[1]: https://github.com/silexphp/Pimple/blob/v3.5.0/src/Pimple/ServiceProviderInterface.php#L43

Co-authored-by: Xavier Aymond <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants