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

Tip 4: Think of different states as different resources #4

Merged
merged 1 commit into from
Aug 2, 2017

Conversation

adamwathan
Copy link
Owner

The last custom actions we have are PodcastsController@publish and PodcastsController@unpublish.

Here are the endpoints:

Route::post('/podcasts/{id}/publish',   'PodcastsController@publish');
Route::post('/podcasts/{id}/unpublish', 'PodcastsController@unpublish');

At first glance you might think:

"All we're doing is updating the published_at column, let's use PodcastsController@update for this."

But we already have an update action, and as we've already discussed, trying to cram two actions into one is a recipe for complexity.

So how can we model "publish" as it's own standard REST action?

If we ask ourselves the same question we did in the previous refactoring:

"After publishing a podcast, what do I have now that I didn't have before?"

...one answer that comes to mind is a "published podcast."

So what would it look like to model "publishing a podcast" as "creating a published podcast"?

✅ Create a dedicated PublishedPodcastsController

In these situations, it can often be helpful to think of a resource in a certain state as it's own independent resource.

If we are creating a new "published podcast", the standard endpoint structure look like this:

- Route::post('/podcasts/{id}/publish', 'PodcastsController@publish');
+ Route::post('/published-podcasts',    'PublishedPodcastsController@store');

As before, we lose the {id} route parameter here, so we'll need to pass that through the request body.

This feels pretty natural when you think of the request body as being the "raw materials" needed to create the resource. When you are creating a published podcast, the raw materials needed are an existing unpublished podcast, and the best way to represent that entity is with it's identifier.

Here's what our new controller action would look like:

class PublishedPodcastsController
{
    public function store()
    {
        $podcast = Auth::user()->podcasts()->findOrFail(request('podcast_id'));

        $podcast->publish();

        return $podcast;
    }
}

One thing to note here is the return value. Previously we were returning an empty 204 response, but now that we're "creating" a new resource, returning the model seems more appropriate.

Modelling unpublish

Just like subscribe, if publish becomes creating a published podcast, then unpublish could become destroying a published podcast.

This one probably requires the biggest mental leap of any of our refactorings so far, but I promise that in practice it doesn't end up feeling as weird as it first sounds.

Here's what the endpoint would become:

- Route::post('/podcasts/{id}/unpublish',   'PodcastsController@unpublish');
+ Route::delete('/published-podcasts/{id}', 'PublishedPodcastsController@destroy');

"Published podcasts" don't have any sort of special unique ID; instead their ID is the same as a regular podcast's ID, so that's what our {id} parameter will represent here.

Here's what the full controller would look like:

class PublishedPodcastsController extends Controller
{
    public function store()
    {
        $podcast = Auth::user()->podcasts()->findOrFail(request('podcast_id'));

        $podcast->publish();

        return $podcast;
    }

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

        $podcast->unpublish();

        return $podcast;
    }
}

After this refactoring, here's where we stand:

  • PodcastsController, with 7 standard actions and no custom actions
  • EpisodesController, with 4 standard actions and no custom actions
  • PodcastEpisodesController, with 3 standard actions and no custom actions
  • PodcastCoverImageController, with 1 standard action and no custom actions
  • SubscriptionsController, with 2 standard actions and no custom actions
  • PublishedPodcastsController, with 2 standard actions and no custom actions

That's 6 controllers with an average of ~3 methods per controller. Small, clean, and simple!

@DanielDarrenJones
Copy link

DanielDarrenJones commented Aug 8, 2017

In this instance I would probably prefer to have

- Route::post('/podcasts/{id}/publish', 'PodcastsController@publish');
+ Route::post('/podcasts/{id}/publish',    'PublishedPodcastsController@store');

The url schema feels more connected to the model, especially if I was pushing this logic out as a public API, its much more logical to use.

That said great tip.

@MichaelDeBoey
Copy link

@DanielDarrenJones What about destroying then?
It feels weird to do a delete on /podcasts/{id}/unpublish, no?

@DanielDarrenJones
Copy link

DanielDarrenJones commented Apr 10, 2019

yeah looking back I would probably model this differently, with a publish_at attribute on the podcast and the publish action would simply be an update request to update that, with the unpublish either being another update to that, or a delete request to /podcast/:id

After working a lot more with API’s I like to keep really strict to the rest actions on a model, but would love to discuss it more with you, hit me up on twitter if you have any questions @DannyDJones

@loekwetzels
Copy link

Hi @adamwathan, I've only just watched your "Cruddy by design" talk yesterday. I really like this approach. I've got a question though: What are your thoughts about this:

in routes:

- Route::get('/podcasts', 'PodcastsController@index');
+ Route::get('/podcasts', 'PublishedPodcastsController@index');

And of move this PodcastsController@index action to PublishedPodcastsController@index because, well... we're only listing Published Podcasts

public function index()
{
    $podcasts = Podcast::published()->paginate(24);
    return view('podcasts.index', [
        'podcasts' => $podcasts,
    ]);
}

@MrShnaider
Copy link

@loekwetzels I faced almost the same problem and came here to see who had already solved it and how. Only I have a slightly more complicated one.

In terms of presentation, I need to model a podcast that can be published and unpublished. And then display them on separate pages, to have all unpublished podcasts on another page.

I couldn't think of anything better than to pass information about the publication in the query parameter:

Route::get('/podcasts', 'PodcastsController@index');
/podcasts - "index" published podcasts
/podcasts?published=true - also "index" published podcasts

/podcasts?published=false - "index" unpublished podcasts

It seems like the query parameters are just created to impose restrictions on the selection.

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

5 participants