OK, while GitHub is down, a code design question:
I always get analysis paralysis with non-standard RESTful actions in rails apps (and generally).
I've got Jobs, and I want to be able to cancel (and re-activate) them. It's not as simple as setting a checkbox; there's a few other things that need to happen., so I can't just use the existing JobsController#update action.
Here are my options as I see it:
1. Just add cancel and reactivate to existing jobs controller.
The routes would be something like:
POST /admin/jobs/cancel/:job_id
POST /admin/jobs/reactivate/:job_id
(Not RESTful; assuming this is a bad idea)
2. Create a JobCancellationsController with create and destroy actions.
Re-activating a job is destroy-ing a JobCancellation resource.
I'll use nested routes as per below:
resources :jobs, except: :show do
resource :job_cancellation, only: [:create, :destroy]
end
which by default will give me something like
a)
POST /admin/jobs/:job_id/job_cancellation
DELETE /admin/jobs/:job_id/job_cancellation
I could tidy the routes themselves up, without changing the controller, to be like:
b)
POST /admin/jobs/:job_id/cancellation
DELETE /admin/jobs/:job_id/cancellation
Although that doesn't seem very intuitive - 'cancellation' would be better as cancel. So I can change the routes while keeping the controller the same:
c)
POST /admin/jobs/:job_id/cancel
DELETE /admin/jobs/:job_id/cancel
That first route now makes sense (although it's not RESTful strictly speaking?), but the second doesn't... "deleting a job cancel"? So you'd change it to something like:
d)
POST /admin/jobs/:job_id/cancel
POST /admin/jobs/:job_id/reactivate
Now the routes makes sense, but look suspiciously close to option 1) above, even though the routes do map to RESTful actions in the JobCancellationsController rather than non-RESTful actions in the JobsController. And it seems very bizzare to have the POST /admin/jobs/:job_id/reactivate route mapping to the JobCancellationsController#destroy action.
To avoid that last odity with JobCancellationsController#destroy, I could instead do:
3. Similar to option 2, but create two controllers: JobCancellationsController with a create action only, and JobReactivationsController with a create action only.
What's the 'correct' way to do this? Or at least, which are the 'incorrect' ways that I can quickly eliminate? Is there a totally different, better way that I've missed?
After a discussion on the Ruby Australia slack channel, I've consolidated the advice into the below:
The Answer
Just add cancel and reactivate to the existing JobsController.
Use this code:
resources :jobs do
post :cancel, on: :member
post :reactivate, on: :member
end
to create routes like:
POST /admin/jobs/:job_id/cancel
POST /admin/jobs/:job_id/reactivate
This is simple and intuitive, even if it's not strictly RESTful.
(Another advised approach was to PATCH to the existing update method with a status of cancelled; though I guess that'd require a conditional in your update method for cancels vs regular updates)
Other useful points that came out in the discussion
resourcing things that aren't resources is a common trap when you're being sold "REST is the best!" (ie, Jobs are a resource, but JobCancellations aren't really, so don't try to make them a resource).
If you don't need the ability to delete a job, you could use the destroy action to cancel jobs instead of a cancel action.
IMO going full REST is only useful in certain contexts that you aren't likely to encounter unless you're at a big corp.
Make an API that makes sense. Rails just provides ways to do CRUD stuff easily. That ties in to restful but isn't all of it.
the REST (or HATEOS) way to do it is to have /jobs/1234.json include a cancelLink: {url: "url", method: "POST", rel: "link to the docs"} property
when we were building our API we had an unofficial rule that if we weren't sure what to do, we'd just copy what github do, as we like their API.
fwiw, I’ve used a bunch of "rest" APIs that were well designed according to rest, but horrible to use as a developer. It’s more important that you A) cover primitives and B) make it friendly to use than you abide by REST (or HATEOS, or whatever you're focusing on today) all the time.
All of the above basically points to advice I always try to tell myself: "It's most important that the code is simple and easy to understand. Design patters are useful only to the extent that they help you achieve that goal. If a design pattern doesn't achieve that goal, it's likely you're using it incorrectly, or applying it to an incorrect situation, or following it too rigidly".
In this specific case why not just have
# Cancel a job (handled by your cancellations controller)
DELETE /admin/jobs/:job_id
# Likewise, reactivate a job if so instructed by the request body
PATCH /admin/jobs/:job_id
There's a lot of potential to run in to the general "you're not doing REST right" debate that I don't think I really care to get into; this particular case looks like it has a decent solution.
FWIW, do you really need to allow reactivation of the same job? It might be more clear to require creating a new job once cancelled. Perhaps offering some mechanism to clone an existing job.
