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.4] add ability to create nested model controllers #18606

Merged
merged 1 commit into from
Apr 5, 2017
Merged

[5.4] add ability to create nested model controllers #18606

merged 1 commit into from
Apr 5, 2017

Conversation

browner12
Copy link
Contributor

when we have polymorphic relationships, we often have nested routes and controllers. for example, we have the following models: Client, Department, Document. Clients and Departments can have multiple Documents, so Documents are polymorphic to their owners.

In this situation we could have 2 controllers: ClientDocumentController and DepartmentDocumentController and the following routes:

Route::resource('clients.documents', 'ClientDocumentController');
Route::resource('departments.documents', 'DepartmentDocumentController');

make:controller is great for generating single model resource controllers but doesn't help with these nested relationships. this PR adds the ability to quickly generate these nested controllers by passing a 'parent' flag with the command.

art make:controller ClientDocumentController --resource=App/Document --parent=App/Client

- new stub for nested controllers
- ‘parent’ option for make command
- if we want a parent, determine the replacement values based on the
model name
@taylorotwell
Copy link
Member

I'm curious why you would really need a nested URL in the first place? If each document has a unique auto-incrementing ID or even a UUID, couldn't the url just be "/documents/{id}"?

@browner12
Copy link
Contributor Author

there are definitely many ways to tackle this, and I think part of it probably does come down to personal preference.

I agree the nested URL can become less important on the edit, update, and delete if you have an ID, but it seems almost essential for the create and store. And actually for the edit, update, and delete the parent model can be important for permissions.

If you decide to go with documents/{id}, the form design on a 'create' view becomes a little tricky. You would essentially need an input for both the client and department (and any other models that relate), which makes for a messy UI.

<form>
    <input name="department_id" />
    <input name="client_id" />
</form>

or you could make multiple views specific to each parent.

clientDocument/create.blade.php
departmentDocument/create.blade.php

which results in a lot of code duplication, and then you have to determine which view to render, and which model to attach it to.

IMO, it is much easier to have the parent in the URL.

clients/1/documents/create
departments/3/documents/create

Then you can have one view that gets passed the 'owner'. Your Client and Department can even implement an interface like Documentable, which helps make sure the view can call certain methods.

<h1>{{ $owner->getDocumentableName() }}</h1>

<form>
...
</form>

With many-to-many relationships this nested route pattern becomes even more important since you often do not have IDs or UUIDs. If we look at the classic User/Role, if we want to remove a role of the user, we can't do it with a flat url like

userrole/{id}

because our ID doesn't exist. We need a nested URL like

user/1/role/5

Again, there are definitely ways to accomplish this without nested URLs, but I believe they are a common valid way to handle this. In fact, you can find nested URLs on both Forge and Envoyer.

https://forge.laravel.com/servers/{server_id}/sites/{site_id}

https://envoyer.io/projects/{project_id}/deployments/{deployment_id}

@tillkruss tillkruss changed the title add ability to create nested model controllers [5.4] add ability to create nested model controllers Apr 3, 2017
@taylorotwell taylorotwell merged commit 81608d0 into laravel:5.4 Apr 5, 2017
@browner12 browner12 deleted the nested-controller branch April 5, 2017 14:59
itainathaniel added a commit to itainathaniel/docs-1 that referenced this pull request Jun 27, 2019
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