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] Adds ability to generate single action controller #24843

Merged
merged 3 commits into from
Jul 22, 2018
Merged

[5.6] Adds ability to generate single action controller #24843

merged 3 commits into from
Jul 22, 2018

Conversation

hivokas
Copy link
Contributor

@hivokas hivokas commented Jul 14, 2018

This pull request adds ability to generate single action controller by specifying --action flag like this:

php artisan make:controller ShowProfile --action

@sisve
Copy link
Contributor

sisve commented Jul 14, 2018

The --action parameter is ambiguous to me. Aren't all controllers containing actions? And a --single-action is also weird, that makes it sound like the user need to know how many actions their controllers will contain up-front.

Would it make sense to extend it to allow the user to automatically create actions? make:controller --actions=showThis,doThat,invokeSomething, which would allow them to make:controller --actions=__invoke?

Or perhaps a make:controller --only-a-invoke-method-as-single-action?

@deleugpn
Copy link
Contributor

You might be better off calling it a Handler instead of a Controller.

@taylorotwell
Copy link
Member

If we're going to add this at all I think I would prefer an option like --invokable or something instead of --action.

@hivokas
Copy link
Contributor Author

hivokas commented Jul 14, 2018

@taylorotwell it's --invokable now

@ogunkarakus
Copy link

@hivokas @taylorotwell How to define named route for this situation? Please enlighten me.

@devcircus
Copy link
Contributor

@ogunkarakus

Route::post('/comment', 'StoreCommentController')->name('comment.store');

@gofish543
Copy link

Why would you want to generate a single action controller vs a regular controller?
What is the different between the two?
What is the net gain?

@deleugpn
Copy link
Contributor

deleugpn commented Jul 17, 2018

Relevant: https://jenssegers.com/85/goodbye-controllers-hello-request-handlers

I have been exclusively using Handlers only for a year now and I don't think I'll ever go back to Controllers.

@gofish543
Copy link

gofish543 commented Jul 18, 2018

Read the article, great read. Love the concept.

So the current command does here is not really making a controller per say, but making a handler.

If Laravel was to implement this functionality why not classify these as their own group.
Create a separate artisan command, make:handler, and place it under the folder app\Http\Handlers.

In my opinion, this would be a great item to add to a 5.7 release (if there is time as I do not know the ETA on 5.7) or 5.8.

In it's current state this PR sounds like it is trying to make a controller become something it isn't. I'd definitely use this if it was expanded upon, similar to the above mentioned idea, and placed into the framework as it's own group of responders.

@devcircus
Copy link
Contributor

IMO single action controllers are very much a part of modern web development. No one is making a controller something it's not. It's simply a controller with a single action that handles one specific purpose instead of a controller with 7 actions and 7 different purposes.
I have my own package that handles this, but glad to see it baked in.

@martinbean
Copy link
Contributor

@gofish543 A single-action controller is still a controller. It’s still taking a request, working with domain models, and returning a response.

@hivokas
Copy link
Contributor Author

hivokas commented Jul 22, 2018

@taylorotwell added information about --invokable flag to docs in this PR: laravel/docs#4389

@slavarazum
Copy link
Contributor

@hivokas Nice idea :)
@taylorotwell I have a question. What's wrong with my previous PR? - #17821
Looks demotivating for me...

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

9 participants