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

[5.6] Add dispatchNow to Dispatchable Trait. #23399

Merged
merged 1 commit into from
Mar 6, 2018

Conversation

jrseliga
Copy link
Contributor

@jrseliga jrseliga commented Mar 6, 2018

@taylorotwell I wasn't sure if tests were necessary, as BusDispatcherTest@testDispatchNowShouldNeverQueue seems to cover this.

@devcircus
Copy link
Contributor

the dispatch method, in Dispatchable, defers to dispatchNow if the job isn't queued.

@jrseliga
Copy link
Contributor Author

jrseliga commented Mar 6, 2018

@devcircus I was under the same impression based on information I found on stackoverflow and Laracasts forum.

I see the dispatchNow fallback behavior on Dispatcher@dispatch, however, executing a Job from within a Controller for example, that is meant to perform an action and return data back synchronously is actually returned with an instance of PendingDispatch

Perhaps I've just missed something obvious?

dispatch-now

@devcircus
Copy link
Contributor

Ahh yes for Dispatchable, that is true. The DispatchesJobs trait is included by default in the base controller and has both "dispatch" and "dispatchNow".
Also, I tested on a project I'm working on and had forgotten I had made a custom dispatch method that handled async and sync jobs.

@jrseliga jrseliga changed the title Add dispatchNow to Dispatchable Trait. [5.6] Add dispatchNow to Dispatchable Trait. Mar 6, 2018
@taylorotwell taylorotwell merged commit ab526b7 into laravel:5.6 Mar 6, 2018
@jrseliga jrseliga deleted the dispatch-now-dispatchable branch March 7, 2018 03:53
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