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

[8.x] Model Broadcasting - Adding broadcastWith() and broadcastAs() support #38137

Merged
merged 16 commits into from
Jul 27, 2021
Merged

[8.x] Model Broadcasting - Adding broadcastWith() and broadcastAs() support #38137

merged 16 commits into from
Jul 27, 2021

Conversation

clemblanco
Copy link
Contributor

@clemblanco clemblanco commented Jul 26, 2021

This PR adds two optional model methods broadcastAs() and broadcastWith() when using Model Broadcasting.


When using broadcasting basic Events, we can specify which broadcast data and broadcast name should be used using both broadcastAs() and broadcastWith().

However, when using Model Broadcasting, we don't have that flexibility as it stands.

Similar to what the framework already does for broadcastOn(), we could use:

public function broadcastAs($event)
{
    return match($event) {
        'created' => 'server.created',
    };
}
public function broadcastWith($event)
{
    return match($event) {
        'created' => ['id' => $this->user->id],
    };
}

Just like broadcastOn():

  1. not defining these methods will default to Laravel's default behaviour
  2. not specifying a default within your match() statement will default to Laravel's default behaviour
  3. if you'd like to always see the same broadcast data and/or name published, no matter the event, you could omit the match() expression (or switch() for php 7) and always return the same string or array

Bonus: missing unit tests

  • The ability to use broadcastOn() to specify a channel for given events was not covered
  • The default behaviours for the broadcast data and name were not covered

composer.json Outdated Show resolved Hide resolved
@driesvints driesvints marked this pull request as draft July 26, 2021 13:09
@driesvints
Copy link
Member

Converted this to a draft until you've managed to fix the tests. Feel free to mark as ready when you're done.

@clemblanco clemblanco marked this pull request as ready for review July 26, 2021 17:01
@clemblanco
Copy link
Contributor Author

@driesvints It's now ready even thought it seems like PHP 8.1 tests are marked as passed while it looks like GitHub Actions say otherwise.

image

Are you aware of this?

@driesvints
Copy link
Member

driesvints commented Jul 26, 2021

@clemblanco yes, feel free to ignore those.

@clemblanco
Copy link
Contributor Author

@driesvints In that case it's ready for you to have a look. Let me know what you think.✌️

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.

3 participants