API proof of concept: forums#show #665
Conversation
* route is /api/forums/:id (may be slug)
* versioning is done via Accept header
* views are built with jbuilder
* authentication is done via session cookies (or whatever the
host app is using)
Also abstracted some test code, in anticipation of including both topics and posts.
Posts count only includes posts the current user is allowed to know about.
| class ForumsController < ApplicationController | ||
| load_and_authorize_resource :class => 'Forem::Forum', :only => :show | ||
|
|
||
| before_action { request.format = :json } |
There was a problem hiding this comment.
I thought that this kind of request format setting could be done at the routes level but I can't find a solution that works. I tried coming up with my own and googling for one. Nothing that I tried works.
Instead, putting respond_to :json at the top of the controller works, but this requires the use of the responders gem (which is alright imo). I think this is a better method than forcing the request format this way.
There was a problem hiding this comment.
respond_to doesn't seem to address the problem I was trying to solve. The problem is that I'm using a custom MIME type (application/vnd.forem+json), as recommended in any number of blog posts, but because of that, Rails looks for a view named show.application/vnd.forem+json.jbuilder. I just wanted to call it show.json.jbuilder.
I tried Mime::Type.register 'application/vnd.forem+json', :json, which works, but I get a warning about initializing Mime::JSON twice. There doesn't seem to be a way to say "Just treat this MIME type as JSON, too." (register_alias doesn't do it.)
It looks like a solution is to register the MIME type as :forem and then rename the views to show.forem.jbuilder. But it seems like there should be a way to use .json.
The standard UI automatically creates a post when a topic is created, but API-based UIs might not. Previously an error would be raised when creating a topic with no post, and also when approving it. Alternatively, if we want to require an initial post, we should add a validation.
I ran into an issue where `/api/topics` was routed to TopicsController#index with a `forum_id` of `"api"`. With this change, `/api/` routes go to the API. This is a temporary fix, as it creates other problems if someone decides to start a forum called "API". We could add `api` to the reserved words list for Friendly ID, but some installations may already have an API forum. We could give the API routes a prefix that can't be generated by Friendly ID's default generator (e.g., `@api`). Or we could get rid of the prefix entirely, and just use the `Accepts` header to determine whether a request is for the HTML UI or the API.
This API controller inherits basic authorization and flow control from
the HTML UI controller, and overrides methods to return results via
JSON API. Alternatives include:
1. Build all-new controllers for the API (as I started to do with
forums), and remember to keep authorization and such in sync
between the API and the HTML UI.
2. Add API functionality directly to the HTML UI controllers,
using `respond_to` and conditionals.
Still evaluating these options.
Tried to cover all of the MUSTs and some of the SHOULDs from the JSON API spec. Specific errors are just returned as text messages for now. Most of the new controller code will be extracted to a mixin module. Likewise, the spec code can be extracted to shared behaviors.
|
Thanks, I'll make those changes next. I've just pushed four new commits that contain tentative answers to questions I ran into:
I'm pressing ahead with these tentative answers, but let me know if you have thoughts on them. |
Also reworked TopicsController#find_topic so that (a) the API subclass can override the "not found" behavior, and (b) the method works as a `before_action` with slightly simpler code.
Also fixed an internal error when a post is submitted with no attributes.
Also moved some PostsController action code into `before_filter`s so that the API's own `before_filter`s get a chance to act.
We now have a general notion of how create, update, and show requests behave, and can focus on the specifics of what data a successful request returns. Also made api/forums#show properly return 409 Forbidden.
This is the pattern we're using for the other controllers, to get permissions checking and such. Also removed unnecessary `respond_to` from `ForumsController#show`, and wrote a test to confirm that it was unnecessary.
Handles the routine mechanics of rendering the right responses.
Also implemented helpers for rendering relationship JSON.
This reduces some code duplication, but as a result sends a little more info than necessary in the included post. If this is a problem, we can get around it by passing info to the partial about what relationships to reference.
Relies on the existing permission controls in the parent controller.
Fixed routes conflict, and RSpec 3 deprecation warnings from the API specs.
So the client can display a reasonable "last active" time for a topic with no posts.
... by using the post partial instead of sending a subset of attributes.
... by extracting a partial with attributes and has-one relationships.
It's used as a proxy for "recent topics," so it makes sense to include new topics, even if they haven't received any replies.
THIS IS NOT A REAL PULL REQUEST AND SHOULD NOT BE MERGED. It's just the easiest way for me to illustrate the approach I'm using so far and ask for feedback. This follows discussion in #664.
@radar mentioned API keys for authentication, but I don't need them for my app, so I'm keeping it simple.
I'm writing request specs, and not feature or controller specs. It just seems like the natural level for testing this functionality.
I haven't thought about how to represent errors (or read about how JSON API does it). Nor have I started figuring out associations.
I created a separate controller hierarchy, rather than try to cram versioned API functionality into the regular controllers. I am not 100% sure about this.
I am also not sure it wouldn't be better to use JSONAPI::Resources instead of creating the views manually with jbuilder.