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

Updates for Laravel Usage #5

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

fideloper
Copy link

@fideloper fideloper commented Jul 25, 2022

This is a draft PR so you can see where my thoughts are.

  1. Changed default user/team models to what Laravel uses for the 90% use case
    • user: App\Models\User
    • team: App\Models\Team
  2. Transformers check if a model/event has a toUserList() method and uses that, else uses default behavior.

TODO/Ideas

  • Readme / Instructions
  • Needs some testing for a "real usage" to make sure my local development setup isn't causing hidden issues.
    • php artisan vendor:publish --provider Userlist\Laravel\UserlistServiceProvider doesn't work?
  • Figure out how events will hook into Laravel's events
  • Figure out queueing data sent to Userlist, and/or perhaps making use of __destruct() similar to bug tracking software

@fideloper
Copy link
Author

fideloper commented Jul 26, 2022

@benedikt One thing we should brainstorm on is how to do Events within Laravel. This goes towards deciding how to handle queues as well.

A "thin" (simpler) version of this is just to have Laravel do the heavy lifting. Here's what that looks like:

# Create an Event
php artisan make:event PaymentFailed

# Create a listener (subscriber) to the above event
# Have it default to being queued
php artisan make:listener RegisterFailedPayment --queued --event PaymentFailed

Then the PHP looks a bit like this:

# The event
class PaymentFailed {

    public function __construct(
         public Team $team
    ) {}
}


# The event handler
use Userlist\Laravel\Contracts\Push;

class RegisterFailedPayment implements ShouldQueue {
    use InteractsWithQueue;

    public function __construct(
        protected Push $userlist
    ){}

    public function handle(PaymentFailed $event)
    {
        // todo: Transform for userlist
        $this->userlist->event($event);
    }
}

Usage looks like this:

// Fire the event. Listeners for the event will be called
// In our example, that is the RegisterFailedPayment listener.
PaymentFailed::dispatch($team);

The DirectPush class doesn't need to care about queueing because Laravel's event handler (listener) is already queued automatically by Laravel in this setup.

A TODO however is to figure out how to handle the Event transformer. It assumes the event is an array and that the array has a user and company array key.

This is the lightest-weight, least magical ("principal of least surprise") way of doing this, and handles queues for us (we don't need to care about queues in this setup, it's up to the developer).

What do you think? Should we dig deeper to try to do queueing for users? (I think either way we need to figure out events - a way for people to attach users/companies).

@benedikt
Copy link
Contributor

@fideloper Can we make the event transform smart enough to check for a toUserlist method to convert events into Push API payloads?

As for the event listeners, I feel like creating a new listener for every single event that should be sent to Userlist is a bit too much. Do you think it's possible to leverage event subscribers and maybe adding a configuration variable of events to send to Userlist?

Not quite sure how event subscribers and queues interact, though. 🤔

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.

2 participants