From 5282ba862a75cbbec81892a6a4cca7e798850178 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Fri, 9 Jun 2017 14:12:40 -0400 Subject: [PATCH] Move reblogged_by and favourited_by actions out of api/v1/statuses and into unique controllers (#3646) * Add specs for api statuses routes * Update favourited_by and reblogged_by api routes * Move methods into new controllers * Use load_accounts methods to simplify index actions * Clean up load_accounts methods * Clean up link header generation * Check for link headers in specs * Remove unused actions from api/v1/statuses controller * Remove specs for moved actions --- .../favourited_by_accounts_controller.rb | 82 +++++++++++++++++++ .../reblogged_by_accounts_controller.rb | 79 ++++++++++++++++++ app/controllers/api/v1/statuses_controller.rb | 34 +------- config/routes.rb | 9 +- .../favourited_by_accounts_controller_spec.rb | 66 +++++++++++++++ .../reblogged_by_accounts_controller_spec.rb | 65 +++++++++++++++ .../api/v1/statuses_controller_spec.rb | 70 ---------------- spec/routing/api_routing_spec.rb | 14 +++- 8 files changed, 314 insertions(+), 105 deletions(-) create mode 100644 app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb create mode 100644 app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb create mode 100644 spec/controllers/api/v1/statuses/favourited_by_accounts_controller_spec.rb create mode 100644 spec/controllers/api/v1/statuses/reblogged_by_accounts_controller_spec.rb diff --git a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb new file mode 100644 index 0000000000..e581849395 --- /dev/null +++ b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController + include Authorization + + before_action :authorize_if_got_token + before_action :set_status + after_action :insert_pagination_headers + + respond_to :json + + def index + @accounts = load_accounts + render 'api/v1/statuses/accounts' + end + + private + + def load_accounts + default_accounts.merge(paginated_favourites).to_a + end + + def default_accounts + Account + .includes(:favourites) + .references(:favourites) + .where(favourites: { status_id: @status.id }) + end + + def paginated_favourites + Favourite.paginate_by_max_id( + limit_param(DEFAULT_ACCOUNTS_LIMIT), + params[:max_id], + params[:since_id] + ) + end + + def insert_pagination_headers + set_pagination_headers(next_path, prev_path) + end + + def next_path + if records_continue? + api_v1_status_favourited_by_index_url pagination_params(max_id: pagination_max_id) + end + end + + def prev_path + unless @accounts.empty? + api_v1_status_favourited_by_index_url pagination_params(since_id: pagination_since_id) + end + end + + def pagination_max_id + @accounts.last.favourites.last.id + end + + def pagination_since_id + @accounts.first.favourites.first.id + end + + def records_continue? + @accounts.size == limit_param(DEFAULT_ACCOUNTS_LIMIT) + end + + def set_status + @status = Status.find(params[:status_id]) + authorize @status, :show? + rescue Mastodon::NotPermittedError + # Reraise in order to get a 404 instead of a 403 error code + raise ActiveRecord::RecordNotFound + end + + def authorize_if_got_token + request_token = Doorkeeper::OAuth::Token.from_request(request, *Doorkeeper.configuration.access_token_methods) + doorkeeper_authorize! :read if request_token + end + + def pagination_params(core_params) + params.permit(:limit).merge(core_params) + end +end diff --git a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb new file mode 100644 index 0000000000..43593d3c5a --- /dev/null +++ b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController + include Authorization + + before_action :authorize_if_got_token + before_action :set_status + after_action :insert_pagination_headers + + respond_to :json + + def index + @accounts = load_accounts + render 'api/v1/statuses/accounts' + end + + private + + def load_accounts + default_accounts.merge(paginated_statuses).to_a + end + + def default_accounts + Account.includes(:statuses).references(:statuses) + end + + def paginated_statuses + Status.where(reblog_of_id: @status.id).paginate_by_max_id( + limit_param(DEFAULT_ACCOUNTS_LIMIT), + params[:max_id], + params[:since_id] + ) + end + + def insert_pagination_headers + set_pagination_headers(next_path, prev_path) + end + + def next_path + if records_continue? + api_v1_status_reblogged_by_index_url pagination_params(max_id: pagination_max_id) + end + end + + def prev_path + unless @accounts.empty? + api_v1_status_reblogged_by_index_url pagination_params(since_id: pagination_since_id) + end + end + + def pagination_max_id + @accounts.last.statuses.last.id + end + + def pagination_since_id + @accounts.first.statuses.first.id + end + + def records_continue? + @accounts.size == limit_param(DEFAULT_ACCOUNTS_LIMIT) + end + + def set_status + @status = Status.find(params[:status_id]) + authorize @status, :show? + rescue Mastodon::NotPermittedError + # Reraise in order to get a 404 instead of a 403 error code + raise ActiveRecord::RecordNotFound + end + + def authorize_if_got_token + request_token = Doorkeeper::OAuth::Token.from_request(request, *Doorkeeper.configuration.access_token_methods) + doorkeeper_authorize! :read if request_token + end + + def pagination_params(core_params) + params.permit(:limit).merge(core_params) + end +end diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 53fb1619e7..7227a65366 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -5,8 +5,8 @@ class Api::V1::StatusesController < Api::BaseController before_action :authorize_if_got_token, except: [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite, :mute, :unmute] before_action -> { doorkeeper_authorize! :write }, only: [:create, :destroy, :reblog, :unreblog, :favourite, :unfavourite, :mute, :unmute] - before_action :require_user!, except: [:show, :context, :card, :reblogged_by, :favourited_by] - before_action :set_status, only: [:show, :context, :card, :reblogged_by, :favourited_by, :mute, :unmute] + before_action :require_user!, except: [:show, :context, :card] + before_action :set_status, only: [:show, :context, :card, :mute, :unmute] before_action :set_conversation, only: [:mute, :unmute] respond_to :json @@ -33,36 +33,6 @@ class Api::V1::StatusesController < Api::BaseController render_empty if @card.nil? end - def reblogged_by - @accounts = Account.includes(:statuses) - .references(:statuses) - .merge(Status.where(reblog_of_id: @status.id) - .paginate_by_max_id(limit_param(DEFAULT_ACCOUNTS_LIMIT), params[:max_id], params[:since_id])) - .to_a - - next_path = reblogged_by_api_v1_status_url(pagination_params(max_id: @accounts.last.statuses.last.id)) if @accounts.size == limit_param(DEFAULT_ACCOUNTS_LIMIT) - prev_path = reblogged_by_api_v1_status_url(pagination_params(since_id: @accounts.first.statuses.first.id)) unless @accounts.empty? - - set_pagination_headers(next_path, prev_path) - - render :accounts - end - - def favourited_by - @accounts = Account.includes(:favourites) - .references(:favourites) - .where(favourites: { status_id: @status.id }) - .merge(Favourite.paginate_by_max_id(limit_param(DEFAULT_ACCOUNTS_LIMIT), params[:max_id], params[:since_id])) - .to_a - - next_path = favourited_by_api_v1_status_url(pagination_params(max_id: @accounts.last.favourites.last.id)) if @accounts.size == limit_param(DEFAULT_ACCOUNTS_LIMIT) - prev_path = favourited_by_api_v1_status_url(pagination_params(since_id: @accounts.first.favourites.first.id)) unless @accounts.empty? - - set_pagination_headers(next_path, prev_path) - - render :accounts - end - def create @status = PostStatusService.new.call(current_user.account, status_params[:status], diff --git a/config/routes.rb b/config/routes.rb index f5fc2b5d68..d0236a43ac 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -128,11 +128,16 @@ Rails.application.routes.draw do # JSON / REST API namespace :v1 do resources :statuses, only: [:create, :show, :destroy] do + scope module: :statuses do + with_options only: :index do + resources :reblogged_by, controller: :reblogged_by_accounts + resources :favourited_by, controller: :favourited_by_accounts + end + end + member do get :context get :card - get :reblogged_by - get :favourited_by post :reblog post :unreblog diff --git a/spec/controllers/api/v1/statuses/favourited_by_accounts_controller_spec.rb b/spec/controllers/api/v1/statuses/favourited_by_accounts_controller_spec.rb new file mode 100644 index 0000000000..1acb990a00 --- /dev/null +++ b/spec/controllers/api/v1/statuses/favourited_by_accounts_controller_spec.rb @@ -0,0 +1,66 @@ +require 'rails_helper' + +RSpec.describe Api::V1::Statuses::FavouritedByAccountsController, type: :controller do + render_views + + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } + let(:token) { double acceptable?: true, resource_owner_id: user.id, application: app } + + context 'with an oauth token' do + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + describe 'GET #index' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + Fabricate(:favourite, status: status) + end + + it 'returns http success' do + get :index, params: { status_id: status.id, limit: 1 } + expect(response).to have_http_status(:success) + expect(response.headers['Link'].links.size).to eq(2) + end + end + + end + + context 'without an oauth token' do + before do + allow(controller).to receive(:doorkeeper_token) { nil } + end + + context 'with a private status' do + let(:status) { Fabricate(:status, account: user.account, visibility: :private) } + + describe 'GET #index' do + before do + Fabricate(:favourite, status: status) + end + + it 'returns http unautharized' do + get :index, params: { status_id: status.id } + expect(response).to have_http_status(:missing) + end + end + end + + context 'with a public status' do + let(:status) { Fabricate(:status, account: user.account, visibility: :public) } + + describe 'GET #index' do + before do + Fabricate(:favourite, status: status) + end + + it 'returns http success' do + get :index, params: { status_id: status.id } + expect(response).to have_http_status(:success) + end + end + end + end +end diff --git a/spec/controllers/api/v1/statuses/reblogged_by_accounts_controller_spec.rb b/spec/controllers/api/v1/statuses/reblogged_by_accounts_controller_spec.rb new file mode 100644 index 0000000000..c5624023f1 --- /dev/null +++ b/spec/controllers/api/v1/statuses/reblogged_by_accounts_controller_spec.rb @@ -0,0 +1,65 @@ +require 'rails_helper' + +RSpec.describe Api::V1::Statuses::RebloggedByAccountsController, type: :controller do + render_views + + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } + let(:token) { double acceptable?: true, resource_owner_id: user.id, application: app } + + context 'with an oauth token' do + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + describe 'GET #index' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + Fabricate(:status, reblog_of_id: status.id) + end + + it 'returns http success' do + get :index, params: { status_id: status.id, limit: 1 } + expect(response).to have_http_status(:success) + expect(response.headers['Link'].links.size).to eq(2) + end + end + end + + context 'without an oauth token' do + before do + allow(controller).to receive(:doorkeeper_token) { nil } + end + + context 'with a private status' do + let(:status) { Fabricate(:status, account: user.account, visibility: :private) } + + describe 'GET #index' do + before do + Fabricate(:status, reblog_of_id: status.id) + end + + it 'returns http unautharized' do + get :index, params: { status_id: status.id } + expect(response).to have_http_status(:missing) + end + end + end + + context 'with a public status' do + let(:status) { Fabricate(:status, account: user.account, visibility: :public) } + + describe 'GET #index' do + before do + Fabricate(:status, reblog_of_id: status.id) + end + + it 'returns http success' do + get :index, params: { status_id: status.id } + expect(response).to have_http_status(:success) + end + end + end + end +end diff --git a/spec/controllers/api/v1/statuses_controller_spec.rb b/spec/controllers/api/v1/statuses_controller_spec.rb index ac3b2dc7d6..8754c03a24 100644 --- a/spec/controllers/api/v1/statuses_controller_spec.rb +++ b/spec/controllers/api/v1/statuses_controller_spec.rb @@ -34,32 +34,6 @@ RSpec.describe Api::V1::StatusesController, type: :controller do end end - describe 'GET #reblogged_by' do - let(:status) { Fabricate(:status, account: user.account) } - - before do - post :reblog, params: { id: status.id } - end - - it 'returns http success' do - get :reblogged_by, params: { id: status.id } - expect(response).to have_http_status(:success) - end - end - - describe 'GET #favourited_by' do - let(:status) { Fabricate(:status, account: user.account) } - - before do - post :favourite, params: { id: status.id } - end - - it 'returns http success' do - get :favourited_by, params: { id: status.id } - expect(response).to have_http_status(:success) - end - end - describe 'POST #create' do before do post :create, params: { status: 'Hello world' } @@ -250,28 +224,6 @@ RSpec.describe Api::V1::StatusesController, type: :controller do expect(response).to have_http_status(:missing) end end - - describe 'GET #reblogged_by' do - before do - post :reblog, params: { id: status.id } - end - - it 'returns http unautharized' do - get :reblogged_by, params: { id: status.id } - expect(response).to have_http_status(:missing) - end - end - - describe 'GET #favourited_by' do - before do - post :favourite, params: { id: status.id } - end - - it 'returns http unautharized' do - get :favourited_by, params: { id: status.id } - expect(response).to have_http_status(:missing) - end - end end context 'with a public status' do @@ -301,28 +253,6 @@ RSpec.describe Api::V1::StatusesController, type: :controller do expect(response).to have_http_status(:success) end end - - describe 'GET #reblogged_by' do - before do - post :reblog, params: { id: status.id } - end - - it 'returns http success' do - get :reblogged_by, params: { id: status.id } - expect(response).to have_http_status(:success) - end - end - - describe 'GET #favourited_by' do - before do - post :favourite, params: { id: status.id } - end - - it 'returns http success' do - get :favourited_by, params: { id: status.id } - expect(response).to have_http_status(:success) - end - end end end end diff --git a/spec/routing/api_routing_spec.rb b/spec/routing/api_routing_spec.rb index 6dfd074a08..6c093f19dd 100644 --- a/spec/routing/api_routing_spec.rb +++ b/spec/routing/api_routing_spec.rb @@ -39,7 +39,19 @@ describe 'API routes' do to route_to('api/v1/accounts/relationships#index') end end - + + describe 'Statuses routes' do + it 'routes reblogged_by' do + expect(get('/api/v1/statuses/123/reblogged_by')). + to route_to('api/v1/statuses/reblogged_by_accounts#index', status_id: '123') + end + + it 'routes favourited_by' do + expect(get('/api/v1/statuses/123/favourited_by')). + to route_to('api/v1/statuses/favourited_by_accounts#index', status_id: '123') + end + end + describe 'Timeline routes' do it 'routes to home timeline' do expect(get('/api/v1/timelines/home')).