From 2925372ff44347fa7066c380a5d51dd35f80682f Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Sat, 10 Jun 2017 03:39:26 -0400 Subject: [PATCH] Move create/destroy actions for api/v1/statuses to namespace (#3678) Each of mute, favourite, reblog has been updated to: - Have a separate controller with just a create and destroy action - Preserve historical route names to not break the API - Mild refactoring to break up long methods --- .../api/v1/statuses/favourites_controller.rb | 38 +++++ .../api/v1/statuses/mutes_controller.rb | 41 ++++++ .../api/v1/statuses/reblogs_controller.rb | 35 +++++ app/controllers/api/v1/statuses_controller.rb | 59 +------- config/routes.rb | 21 ++- .../v1/statuses/favourites_controller_spec.rb | 66 +++++++++ .../api/v1/statuses/mutes_controller_spec.rb | 50 +++++++ .../v1/statuses/reblogs_controller_spec.rb | 66 +++++++++ .../api/v1/statuses_controller_spec.rb | 131 ------------------ spec/routing/api_routing_spec.rb | 32 +++++ 10 files changed, 341 insertions(+), 198 deletions(-) create mode 100644 app/controllers/api/v1/statuses/favourites_controller.rb create mode 100644 app/controllers/api/v1/statuses/mutes_controller.rb create mode 100644 app/controllers/api/v1/statuses/reblogs_controller.rb create mode 100644 spec/controllers/api/v1/statuses/favourites_controller_spec.rb create mode 100644 spec/controllers/api/v1/statuses/mutes_controller_spec.rb create mode 100644 spec/controllers/api/v1/statuses/reblogs_controller_spec.rb diff --git a/app/controllers/api/v1/statuses/favourites_controller.rb b/app/controllers/api/v1/statuses/favourites_controller.rb new file mode 100644 index 00000000000..b6fb13cc073 --- /dev/null +++ b/app/controllers/api/v1/statuses/favourites_controller.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::FavouritesController < Api::BaseController + include Authorization + + before_action -> { doorkeeper_authorize! :write } + before_action :require_user! + + respond_to :json + + def create + @status = favourited_status + render 'api/v1/statuses/show' + end + + def destroy + @status = requested_status + @favourites_map = { @status.id => false } + + UnfavouriteWorker.perform_async(current_user.account_id, @status.id) + + render 'api/v1/statuses/show' + end + + private + + def favourited_status + service_result.status.reload + end + + def service_result + FavouriteService.new.call(current_user.account, requested_status) + end + + def requested_status + Status.find(params[:status_id]) + end +end diff --git a/app/controllers/api/v1/statuses/mutes_controller.rb b/app/controllers/api/v1/statuses/mutes_controller.rb new file mode 100644 index 00000000000..eab88f2efdb --- /dev/null +++ b/app/controllers/api/v1/statuses/mutes_controller.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::MutesController < Api::BaseController + include Authorization + + before_action -> { doorkeeper_authorize! :write } + before_action :require_user! + before_action :set_status + before_action :set_conversation + + respond_to :json + + def create + current_account.mute_conversation!(@conversation) + @mutes_map = { @conversation.id => true } + + render 'api/v1/statuses/show' + end + + def destroy + current_account.unmute_conversation!(@conversation) + @mutes_map = { @conversation.id => false } + + render 'api/v1/statuses/show' + end + + private + + 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 set_conversation + @conversation = @status.conversation + raise Mastodon::ValidationError if @conversation.nil? + end +end diff --git a/app/controllers/api/v1/statuses/reblogs_controller.rb b/app/controllers/api/v1/statuses/reblogs_controller.rb new file mode 100644 index 00000000000..ee9c5b3a624 --- /dev/null +++ b/app/controllers/api/v1/statuses/reblogs_controller.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::ReblogsController < Api::BaseController + include Authorization + + before_action -> { doorkeeper_authorize! :write } + before_action :require_user! + + respond_to :json + + def create + @status = ReblogService.new.call(current_user.account, status_for_reblog) + render 'api/v1/statuses/show' + end + + def destroy + @status = status_for_destroy.reblog + @reblogs_map = { @status.id => false } + + authorize status_for_destroy, :unreblog? + RemovalWorker.perform_async(status_for_destroy.id) + + render 'api/v1/statuses/show' + end + + private + + def status_for_reblog + Status.find params[:status_id] + end + + def status_for_destroy + current_user.account.statuses.where(reblog_of_id: params[:status_id]).first! + end +end diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 7227a653661..9aa1cbc4d17 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -3,11 +3,10 @@ class Api::V1::StatusesController < Api::BaseController include Authorization - 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 :authorize_if_got_token, except: [:create, :destroy] + before_action -> { doorkeeper_authorize! :write }, only: [:create, :destroy] 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] + before_action :set_status, only: [:show, :context, :card] respond_to :json @@ -56,53 +55,6 @@ class Api::V1::StatusesController < Api::BaseController render_empty end - def reblog - @status = ReblogService.new.call(current_user.account, Status.find(params[:id])) - render :show - end - - def unreblog - reblog = Status.where(account_id: current_user.account, reblog_of_id: params[:id]).first! - @status = reblog.reblog - @reblogs_map = { @status.id => false } - - authorize reblog, :unreblog? - - RemovalWorker.perform_async(reblog.id) - - render :show - end - - def favourite - @status = FavouriteService.new.call(current_user.account, Status.find(params[:id])).status.reload - render :show - end - - def unfavourite - @status = Status.find(params[:id]) - @favourites_map = { @status.id => false } - - UnfavouriteWorker.perform_async(current_user.account_id, @status.id) - - render :show - end - - def mute - current_account.mute_conversation!(@conversation) - - @mutes_map = { @conversation.id => true } - - render :show - end - - def unmute - current_account.unmute_conversation!(@conversation) - - @mutes_map = { @conversation.id => false } - - render :show - end - private def set_status @@ -113,11 +65,6 @@ class Api::V1::StatusesController < Api::BaseController raise ActiveRecord::RecordNotFound end - def set_conversation - @conversation = @status.conversation - raise Mastodon::ValidationError if @conversation.nil? - end - def status_params params.permit(:status, :in_reply_to_id, :sensitive, :spoiler_text, :visibility, media_ids: []) end diff --git a/config/routes.rb b/config/routes.rb index d0236a43acf..601330d3978 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -129,22 +129,21 @@ Rails.application.routes.draw do 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 + resources :reblogged_by, controller: :reblogged_by_accounts, only: :index + resources :favourited_by, controller: :favourited_by_accounts, only: :index + resource :reblog, only: :create + post :unreblog, to: 'reblogs#destroy' + + resource :favourite, only: :create + post :unfavourite, to: 'favourites#destroy' + + resource :mute, only: :create + post :unmute, to: 'mutes#destroy' end member do get :context get :card - - post :reblog - post :unreblog - post :favourite - post :unfavourite - post :mute - post :unmute end end diff --git a/spec/controllers/api/v1/statuses/favourites_controller_spec.rb b/spec/controllers/api/v1/statuses/favourites_controller_spec.rb new file mode 100644 index 00000000000..eb77072d259 --- /dev/null +++ b/spec/controllers/api/v1/statuses/favourites_controller_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::Statuses::FavouritesController 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 'POST #create' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + post :create, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'updates the favourites count' do + expect(status.favourites.count).to eq 1 + end + + it 'updates the favourited attribute' do + expect(user.account.favourited?(status)).to be true + end + + it 'return json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:id]).to eq status.id + expect(hash_body[:favourites_count]).to eq 1 + expect(hash_body[:favourited]).to be true + end + end + + describe 'POST #destroy' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + FavouriteService.new.call(user.account, status) + post :destroy, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'updates the favourites count' do + expect(status.favourites.count).to eq 0 + end + + it 'updates the favourited attribute' do + expect(user.account.favourited?(status)).to be false + end + end + end +end diff --git a/spec/controllers/api/v1/statuses/mutes_controller_spec.rb b/spec/controllers/api/v1/statuses/mutes_controller_spec.rb new file mode 100644 index 00000000000..1f8c29e3d8b --- /dev/null +++ b/spec/controllers/api/v1/statuses/mutes_controller_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::Statuses::MutesController 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 'POST #create' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + post :create, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'creates a conversation mute' do + expect(ConversationMute.find_by(account: user.account, conversation_id: status.conversation_id)).to_not be_nil + end + end + + describe 'POST #destroy' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + user.account.mute_conversation!(status.conversation) + post :destroy, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'destroys the conversation mute' do + expect(ConversationMute.find_by(account: user.account, conversation_id: status.conversation_id)).to be_nil + end + end + end +end diff --git a/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb b/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb new file mode 100644 index 00000000000..36c323736bb --- /dev/null +++ b/spec/controllers/api/v1/statuses/reblogs_controller_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::Statuses::ReblogsController 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 'POST #create' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + post :create, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'updates the reblogs count' do + expect(status.reblogs.count).to eq 1 + end + + it 'updates the reblogged attribute' do + expect(user.account.reblogged?(status)).to be true + end + + it 'return json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:reblog][:id]).to eq status.id + expect(hash_body[:reblog][:reblogs_count]).to eq 1 + expect(hash_body[:reblog][:reblogged]).to be true + end + end + + describe 'POST #destroy' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + ReblogService.new.call(user.account, status) + post :destroy, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'updates the reblogs count' do + expect(status.reblogs.count).to eq 0 + end + + it 'updates the reblogged attribute' do + expect(user.account.reblogged?(status)).to be false + 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 8754c03a24d..3d65180ab3a 100644 --- a/spec/controllers/api/v1/statuses_controller_spec.rb +++ b/spec/controllers/api/v1/statuses_controller_spec.rb @@ -59,137 +59,6 @@ RSpec.describe Api::V1::StatusesController, type: :controller do expect(Status.find_by(id: status.id)).to be nil end end - - describe 'POST #reblog' do - let(:status) { Fabricate(:status, account: user.account) } - - before do - post :reblog, params: { id: status.id } - end - - it 'returns http success' do - expect(response).to have_http_status(:success) - end - - it 'updates the reblogs count' do - expect(status.reblogs.count).to eq 1 - end - - it 'updates the reblogged attribute' do - expect(user.account.reblogged?(status)).to be true - end - - it 'return json with updated attributes' do - hash_body = body_as_json - - expect(hash_body[:reblog][:id]).to eq status.id - expect(hash_body[:reblog][:reblogs_count]).to eq 1 - expect(hash_body[:reblog][:reblogged]).to be true - end - end - - describe 'POST #unreblog' do - let(:status) { Fabricate(:status, account: user.account) } - - before do - post :reblog, params: { id: status.id } - post :unreblog, params: { id: status.id } - end - - it 'returns http success' do - expect(response).to have_http_status(:success) - end - - it 'updates the reblogs count' do - expect(status.reblogs.count).to eq 0 - end - - it 'updates the reblogged attribute' do - expect(user.account.reblogged?(status)).to be false - end - end - - describe 'POST #favourite' do - let(:status) { Fabricate(:status, account: user.account) } - - before do - post :favourite, params: { id: status.id } - end - - it 'returns http success' do - expect(response).to have_http_status(:success) - end - - it 'updates the favourites count' do - expect(status.favourites.count).to eq 1 - end - - it 'updates the favourited attribute' do - expect(user.account.favourited?(status)).to be true - end - - it 'return json with updated attributes' do - hash_body = body_as_json - - expect(hash_body[:id]).to eq status.id - expect(hash_body[:favourites_count]).to eq 1 - expect(hash_body[:favourited]).to be true - end - end - - describe 'POST #unfavourite' do - let(:status) { Fabricate(:status, account: user.account) } - - before do - post :favourite, params: { id: status.id } - post :unfavourite, params: { id: status.id } - end - - it 'returns http success' do - expect(response).to have_http_status(:success) - end - - it 'updates the favourites count' do - expect(status.favourites.count).to eq 0 - end - - it 'updates the favourited attribute' do - expect(user.account.favourited?(status)).to be false - end - end - - describe 'POST #mute' do - let(:status) { Fabricate(:status, account: user.account) } - - before do - post :mute, params: { id: status.id } - end - - it 'returns http success' do - expect(response).to have_http_status(:success) - end - - it 'creates a conversation mute' do - expect(ConversationMute.find_by(account: user.account, conversation_id: status.conversation_id)).to_not be_nil - end - end - - describe 'POST #unmute' do - let(:status) { Fabricate(:status, account: user.account) } - - before do - post :mute, params: { id: status.id } - post :unmute, params: { id: status.id } - end - - it 'returns http success' do - expect(response).to have_http_status(:success) - end - - it 'destroys the conversation mute' do - expect(ConversationMute.find_by(account: user.account, conversation_id: status.conversation_id)).to be_nil - end - end end context 'without an oauth token' do diff --git a/spec/routing/api_routing_spec.rb b/spec/routing/api_routing_spec.rb index 6c093f19dd7..2683ccb8d12 100644 --- a/spec/routing/api_routing_spec.rb +++ b/spec/routing/api_routing_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rails_helper' describe 'API routes' do @@ -50,6 +52,36 @@ describe 'API routes' do expect(get('/api/v1/statuses/123/favourited_by')). to route_to('api/v1/statuses/favourited_by_accounts#index', status_id: '123') end + + it 'routes reblog' do + expect(post('/api/v1/statuses/123/reblog')). + to route_to('api/v1/statuses/reblogs#create', status_id: '123') + end + + it 'routes unreblog' do + expect(post('/api/v1/statuses/123/unreblog')). + to route_to('api/v1/statuses/reblogs#destroy', status_id: '123') + end + + it 'routes favourite' do + expect(post('/api/v1/statuses/123/favourite')). + to route_to('api/v1/statuses/favourites#create', status_id: '123') + end + + it 'routes unfavourite' do + expect(post('/api/v1/statuses/123/unfavourite')). + to route_to('api/v1/statuses/favourites#destroy', status_id: '123') + end + + it 'routes mute' do + expect(post('/api/v1/statuses/123/mute')). + to route_to('api/v1/statuses/mutes#create', status_id: '123') + end + + it 'routes unmute' do + expect(post('/api/v1/statuses/123/unmute')). + to route_to('api/v1/statuses/mutes#destroy', status_id: '123') + end end describe 'Timeline routes' do