-
Notifications
You must be signed in to change notification settings - Fork 421
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
API proof of concept: forums#show #665
Open
eostrom
wants to merge
33
commits into
rubysherpas:rails4
Choose a base branch
from
eostrom:api
base: rails4
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+817
−36
Open
Changes from 3 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
780197a
API proof of concept: forums#show
eostrom 2df1654
Fixed bug when no API version is specified.
eostrom f0520c1
Include topics in /forum/:id API.
eostrom 6080de5
JSON API compliance: relationships go inside data.
eostrom 676b2f6
Added posts count and views count.
eostrom 1d8d680
topics#show API: include last post.
eostrom 7e8ad68
Allow topics to be created without first posts.
eostrom c5a78c3
Give API routes priority.
eostrom 88ca5e7
Create Topic happy path.
eostrom aa614d9
Create Topic error handling.
eostrom 87019aa
Lock Jbuilder against incompatible upgrades.
eostrom 3b2ebb7
API: Added topics#show.
eostrom 30d7dce
API: Include posts in topics#show.
eostrom a60d5d5
Removed accidentally duplicated specs.
eostrom 3c867db
API: added posts#show.
eostrom 95c9abf
API: include forum ID in post JSON.
eostrom dc751a3
API: Added posts#update.
eostrom 054991a
Spec refactor: started abstracting API behavior.
eostrom bafbee5
Spec refactor: matchers for JSON API references.
eostrom b2684a8
API: Inherit from ForumsController.
eostrom 5efb90f
API: Extracted JsonApiController module.
eostrom fe776c5
API: Added forum relationship to posts and topics.
eostrom b97d486
API: send user relationships, not ID attributes.
eostrom 45b8ef9
API: Render included posts with a partial.
eostrom da075e8
Add topic slugs to JSON output.
eostrom 70e0ffe
API: Don't list unmoderated topics.
eostrom 92db379
Merged from rails4 into api branch.
eostrom 0543130
Added created_at field to topics list.
eostrom 21fa728
API: Added topic author ID to forums/show response.
eostrom 6617ae2
API: Include latest post author in topic list.
eostrom fbb1e72
API: Reduced code duplication in topic views.
eostrom 5991286
API: Hide unapproved posts from non-moderators.
eostrom b354f5b
Set last_post_at for topics with no posts.
eostrom File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
require_dependency "forem/application_controller" | ||
|
||
module Forem | ||
module Api | ||
module V1 | ||
class ForumsController < ApplicationController | ||
load_and_authorize_resource :class => 'Forem::Forum', :only => :show | ||
|
||
before_action { request.format = :json } | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
json.data do | ||
json.type "forums" | ||
json.(@forum, :id) | ||
json.attributes do | ||
json.(@forum, :title, :slug) | ||
end | ||
end | ||
|
||
json.relationships do | ||
json.topics do | ||
json.data @forum.topics do |topic| | ||
json.type 'topics' | ||
json.(topic, :id) | ||
end | ||
end | ||
end | ||
|
||
json.included @forum.topics do |topic| | ||
json.type 'topics' | ||
json.(topic, :id) | ||
json.attributes do | ||
json.(topic, :subject) | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,4 +31,5 @@ Gem::Specification.new do |s| | |
s.add_dependency 'select2-rails', '~> 3.5.4' | ||
s.add_dependency 'friendly_id', '~> 5.0.0' | ||
s.add_dependency 'cancancan', '~> 1.7' | ||
s.add_dependency 'jbuilder' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would personally lock the jbuilder version here, just in case of a major version bump that changes things in a breaking way. |
||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
module Forem | ||
module API | ||
class VersionRoutingConstraint | ||
def initialize(version) | ||
@version = version | ||
end | ||
|
||
def matches?(request) | ||
requested_this_version?(request) || | ||
# All real clients should specify an API version in their | ||
# request headers, but for ease of debugging (via curl, browsers, | ||
# etc.), the latest API version responds to requests that don't | ||
# specify. | ||
(no_requested_version?(request) && latest_version?) | ||
end | ||
|
||
private | ||
|
||
def requested_this_version?(request) | ||
requested_version(request) == @version | ||
end | ||
|
||
def no_requested_version?(request) | ||
!requested_version(request) | ||
end | ||
|
||
def requested_version(request) | ||
accept = request.headers['Accept'] | ||
accept && | ||
accept[/application\/vnd\.forem\+json; version=([0-9]+)/] && | ||
Integer($1) | ||
end | ||
|
||
# True if no later API version exists. | ||
def latest_version? | ||
"::Forem::Api::V#{@version + 1}".constantize | ||
# if the above succeeds, this is not the latest version | ||
false | ||
rescue NameError | ||
# no version beyond this one exists | ||
true | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
require 'spec_helper' | ||
|
||
describe 'Forums API', type: :request do | ||
let(:forum) { create(:forum) } | ||
let!(:topic) { create(:topic, forum: forum) } | ||
|
||
def api(method, action, params = {}) | ||
send method, action, params, | ||
Accept: 'application/vnd.forem+json; version=1' | ||
end | ||
|
||
describe '#show' do | ||
before { api :get, api_forum_path(forum) } | ||
|
||
let(:json) { JSON.parse(response.body).with_indifferent_access } | ||
let(:data) { json[:data] } | ||
|
||
it 'succeeds' do | ||
expect(response).to be_success | ||
end | ||
|
||
it 'represents the forum' do | ||
expect(data[:type]).to eq 'forums' | ||
expect(data[:id]).to eq forum.id | ||
expect(data[:attributes][:title]).to eq forum.title | ||
expect(data[:attributes][:slug]).to eq forum.slug | ||
end | ||
|
||
let(:related_topics) { json[:relationships][:topics] } | ||
let(:included_objects) { json[:included] } | ||
|
||
it 'describes topic relationships' do | ||
expect(related_topics[:data].length).to eq 1 | ||
related_topic = related_topics[:data].first | ||
|
||
expect(related_topic[:type]).to eq 'topics' | ||
expect(related_topic[:id]).to eq topic.id | ||
end | ||
|
||
it 'includes topic data' do | ||
expect(included_objects.length).to eq 1 | ||
included_topic = included_objects.first | ||
|
||
expect(included_topic[:type]).to eq 'topics' | ||
expect(included_topic[:id]).to eq topic.id | ||
expect(included_topic[:attributes][:subject]).to eq topic.subject | ||
end | ||
end | ||
end |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 theresponders
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 namedshow.application/vnd.forem+json.jbuilder
. I just wanted to call itshow.json.jbuilder
.I tried
Mime::Type.register 'application/vnd.forem+json', :json
, which works, but I get a warning about initializingMime::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 toshow.forem.jbuilder
. But it seems like there should be a way to use.json
.