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 Eloquent Model Event Handling Attributes e.g. #[Listener('created')] #58

Merged
merged 5 commits into from
Sep 26, 2023

Conversation

marvinosswald
Copy link
Contributor

@marvinosswald marvinosswald commented Sep 10, 2023

Handling Events with listeners on your Model with lifts fantastic syntax, Thanks for making this library IMHO this should be part of the framework seems like a no-brainer to me ever since I saw Caleb's talk on Laracon Us this year.

This PR supports every Model Event documented.

Listener

#[Listener(queue: true)]
public function onUpdated(Product $product): void
{
    Log::info('onUpdated has been called');
}

#[Listener('updated')]
public function handleUpdatedEvent(Product $product): void
{
    Log::info('onUpdated has been called');
}

Observer

#[Observer(UserObserver::class)]
class User extends Authenticatable
{
    use HasFactory;
    //...
}

Dispatches

#[Dispatches(UserSaved::class, 'saved')]
class User extends Authenticatable
{
    use HasFactory;
    //...
}

Convention based

#[Dispatches(UserSaved::class)]
class User extends Authenticatable
{
    use HasFactory;
    //...
}

@WendellAdriel
Copy link
Owner

Hey @marvinosswald thanks for the kind words!
Thanks for the PR!

Can you provide some examples showing how these attributes would be better than the current syntax with some real life examples? Please create a discussion in the Ideas category for that and add the link of this PR there, so we can discuss there!

@WendellAdriel WendellAdriel marked this pull request as draft September 18, 2023 16:30
@marvinosswald marvinosswald marked this pull request as ready for review September 21, 2023 14:06
@marvinosswald
Copy link
Contributor Author

marvinosswald commented Sep 21, 2023

@WendellAdriel i've incorporated naming changes we discussed as well as adding queuable for Listeners

And because i like laravels general approach of overridable convention, we can now use #[Listener] without passing an explicit event name if the function name is prefixed with on and camel cased.

For custom event dispatching i did the same you can register events like this #[Dispatches(ProductSaving::class, 'saving') or just #[Dispatches(ProductSaving::class) which works if Saving can be found in the event name.

If you feel like those convention based usages are to ambigious let me know and i remove them again, just felt so much more comfortable to me.

@marvinosswald marvinosswald changed the title Add Eloquent Model Event Handling Attributes e.g. #[Created] Add Eloquent Model Event Handling Attributes e.g. #[Listener('created')] Sep 21, 2023
Copy link
Owner

@WendellAdriel WendellAdriel left a comment

Choose a reason for hiding this comment

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

I just made some observations regarding the code.
Also, since this is a new feature with a lot of details, can you also create a PR in the docs repo for it, please?

src/Attributes/Events/Dispatches.php Outdated Show resolved Hide resolved
src/Attributes/Events/Listener.php Outdated Show resolved Hide resolved
src/Concerns/Events/Events.php Show resolved Hide resolved
src/Concerns/Events/ListenerHandler.php Show resolved Hide resolved
src/Concerns/Events/ListenerHandler.php Show resolved Hide resolved
tests/Feature/Events/SoftDeletesEventsTest.php Outdated Show resolved Hide resolved
tests/Feature/Events/SoftDeletesEventsTest.php Outdated Show resolved Hide resolved
tests/Feature/Events/StandardEventsTest.php Outdated Show resolved Hide resolved
@marvinosswald
Copy link
Contributor Author

marvinosswald commented Sep 22, 2023

@WendellAdriel i'll be working on a docs PR, i'll prepare a PR based on unchecked Markdown. Please advice if there is a better possibility with gitbooks, thanks.

@marvinosswald
Copy link
Contributor Author

@WendellAdriel created a PR on the docs repo for this: WendellAdriel/laravel-lift-docs#2

Copy link
Owner

@WendellAdriel WendellAdriel left a comment

Choose a reason for hiding this comment

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

All good here! 🔥

@WendellAdriel
Copy link
Owner

I'm going to merge this but I'll wait for the docs to be ready to release the new version!
Thanks for the contribution @marvinosswald!!! 🔥 💪

@WendellAdriel WendellAdriel merged commit 0cca806 into WendellAdriel:main Sep 26, 2023
4 checks passed
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.

None yet

2 participants