Tip 1: Give nested resources a dedicated controller

[adamwathan]

When dealing with nested resources in an application, you often end up with routes like this:

/podcasts/{id}/episodes

When you have an endpoint like this, which controller do you use and what do you call the action?

❌ Option 1: PodcastsController

Since /podcasts is the top level resource, it's common to use the PodcastsController for these sorts of nested index actions.

This can work but it forces you to define a "custom action"; something that isn't part of the 7 standard REST/CRUD actions that exist on a resource controller.

In this example, we started with an action on PodcastsController called listEpisodes. If we want to stick to only the 7 standard controller actions, what else could we do?

❌ Option 2: EpisodesController

Since this action is a list of episodes, you might think "hey, let's make this EpisodesController@index!"

But there's a problem: we already have an EpisodesController@index action! In our case, it lists all episodes across all podcasts.

Antipattern: Optional route parameters

One thing I've seen folks do in this case is re-use EpisodesController@index for two different use cases: listing all episodes or listing an individual podcast's episodes.

That usually looks like this in a routes file:

Route::get('/episodes',               'EpisodesController@index');
Route::get('/podcasts/{id}/episodes', 'EpisodesController@index');

...and like this in the controller:

class EpisodesController
{
    public function index($id = null)
    {
        if ($id === null) {
            $episodes = Episode::with('podcast')->recent()->paginate(25);

            return view('episodes.index', [
                'episodes' => $episodes,
            ]);
        } else {

            $podcast = Podcast::with('episodes')->findOrFail($id);

            abort_unless($podcast->isVisibleTo(Auth::user()), 404);

            return view('podcasts.list-episodes', [
                'podcast' => $podcast,
            ]);
        }
    }
}

The problem with this approach is that you are taking two completely different actions from the user's point of view and stuffing them all into one action that has to reverse-engineer the intent of the user based on the presence of the route parameter.

These actions have literally nothing in common:

• Load different models
• Have different permission checks
• Return different views
• Pass different data to the views

If our goal is to use more controllers to simplify our code, this isn't helping. We actually have one less controller action than before, resulting in another controller action getting much more complicated.

Don't reuse a single controller action for two different user actions.

✅ Option 3: Dedicated PodcastEpisodesController

If we want to stick to only standard actions, that means we want to use an index action here. But what is the resource we are requesting an index of?

You could say it's episodes, but we're not requesting an index of all episodes. It's a specific podcast's episodes, so you could say we want an index of "podcast episodes".

If we create a brand new controller for this, here's what our route definition would look like:

Route::get('/podcasts/{id}/episodes', 'PodcastEpisodesController@index');

...and here's the controller with it's action:

class PodcastEpisodesController extends Controller
{
    public function index($id)
    {
        $podcast = Podcast::with('episodes')->findOrFail($id);

        abort_unless($podcast->isVisibleTo(Auth::user()), 404);

        return view('podcast-episodes.index', [
            'podcast' => $podcast,
        ]);
    }
}

When taking this approach, I usually make my template folder structure match the controller name, as you can see above with the podcast-episodes.index view.

Side benefit: Attracting existing nested resource endpoints

If you look in the routes file, you'll see we have two other episodes related endpoints nested under the top-level podcasts resource:

Route::get('/podcasts/{id}/episodes/new', 'EpisodesController@create');
Route::post('/podcasts/{id}/episodes',    'EpisodesController@store');

This isn't bad on it's own, but it's sort of awkward that inside of EpisodesController, the $id route parameter has multiple meanings.

For example, here it represents an Episode ID:

public function show($id)
{
    $episode = Episode::with('podcast')->findOrFail($id);

    abort_unless($episode->isVisibleTo(Auth::user()), 404);

    return view('episodes.show', [
        'episode' => $episode,
    ]);
}

...but in our create action, it represents a Podcast ID:

public function create($id)
{
    $podcast = Auth::user()->podcasts()->findOrFail($id);

    return view('episodes.create', ['podcast' => $podcast]);
}

To me this is a bit of a smell.

Each controller should only be concerned about the ID of one type of resource.

Now that we have a dedicated PodcastEpisodesController, we can move our create and store actions alongside our new index action, and now the $id parameter will always refer to a Podcast ID:

Route::get('/podcasts/{id}/episodes',     'PodcastEpisodesController@index');
Route::post('/podcasts/{id}/episodes',    'PodcastEpisodesController@store');
Route::get('/podcasts/{id}/episodes/new', 'PodcastEpisodesController@create');

After this refactoring, we're left with 3 controllers:

• PodcastsController, with 7 standard actions and 5 custom actions
• EpisodesController, with 4 standard actions and no custom actions
• PodcastEpisodesController, with 3 standard actions and no custom actions

Next up: Treat properties that are edited independently like a separate resource

[uxweb]

Thanks @adamwathan, this is really useful, beautiful and mind-blowing for me!

[hoseinz3]

@adamwathan thank you. very useful

[shubbarm]

Thanks :)

[arodbits]

A very useful post @adamwathan ! Thanks.

[mariordev]

This is great, thank you @adamwathan!

[ptournet]

@adamwathan, I was wondering if there was a particular reason for you to use

public function show($id)
{
    $episode = Episode::with('podcast')->findOrFail($id);

instead of

public function show(Episode $episode)
{

with {$episode} instead of {$id} in the route definition ?

[adamwathan]

@ptournet I don't really use route model binding very often so just habit. Don't like that it forces me to add a type-hint unless I set up the binding manually 😉

[nikolaynizruhin]

What if I want to remove all episodes for a given podcast?
Route::delete('/podcasts/{id}/episodes', 'PodcastEpisodesController@destroy');
Is it okay?

[adamwathan]

@nikolaynizruhin Yep I think that's the right move!

[nikolaynizruhin]

Thanks!

[PeterKravets]

Hi Adam!
But why don’t use namespaces for this?
Like, we could have one EpisodesController in the root, and one in the Podcasts folder: Podcasts/EpisodesController (instead of PodacstEpisodesController).
What do you think about this? Is it fine?

[adamwathan]

Personally I don't like overly nested folder structures or files with the same name in different directories. I find things easier to navigate with flatter project structures.

[pbgneff]

Even if there are 100 files in a single directory?

[adamwathan]

Yep doesn't bother me!

[MichaelDeBoey]

@pbgneff Makes it easier to import too without renaming

[Nartub600]

How is it recommended to go for the update route?

Usually you have Route::patch('/episodes/{id}', 'EpisodesController@update');.

Following this approach I end up with Route::patch('/podcasts/{podcast_id}/episodes/{episode_id}', 'PodcastEpisodesController@update');

But this breaks the rule of the controller being concerned only about the ID of one resource type. Is it ok to just use the non-nested resource controller here (as in the first case)? I don't think I care about the Podcast ID if I have the Episode ID, is that correct?

[adamwathan]

@Nartub600 I would keep the update action on the EpisodesController, we still have an EpisodesController for these actions even after this refactoring:

https://github.com/adamwathan/laracon2017/blob/master/app/Http/Controllers/EpisodesController.php#L41

[Orest-Divintari]

Hi @adamwathan

Is it considered wrong to display the episodes of a specific podcast using a show method

class PodcastController
{
    public function show($id)
    {
         Podcast::with(“epidodes”)->whereId($id)->firstOrFail();
    }
}

[sergejostir]

@Orest-Divintari, it's okay, you can even see it in the examples:

laracon2017/app/Http/Controllers/PodcastsController.php

Lines 43 to 53 in 796c788

	public function show($id)
	{
	$podcast = Podcast::findOrFail($id);
	abort_unless($podcast->isVisibleTo(Auth::user()), 404);
	return view('podcasts.show', [
	'podcast' => $podcast,
	'episodes' => $podcast->recentEpisodes(5),
	]);
	}

laracon2017/app/Http/Controllers/EpisodesController.php

Lines 19 to 28 in 796c788

	public function show($id)
	{
	$episode = Episode::with('podcast')->findOrFail($id);
	abort_unless($episode->isVisibleTo(Auth::user()), 404);
	return view('episodes.show', [
	'episode' => $episode,
	]);
	}