Spec coverage and refactor for the api/v1/accounts controllers (#3451)

18 files changed, 579 insertions, 230 deletions

diff --git a/app/controllers/api/v1/accounts/credentials_controller.rb b/app/controllers/api/v1/accounts/credentials_controller.rb
new file mode 100644
index 00000000000..8f2ded29e39
--- /dev/null
+++ b/app/controllers/api/v1/accounts/credentials_controller.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+class Api::V1::Accounts::CredentialsController < ApiController
+  before_action -> { doorkeeper_authorize! :write }, only: [:update]
+  before_action :require_user!
+
+  def show
+    @account = current_account
+    render 'api/v1/accounts/show'
+  end
+
+  def update
+    current_account.update!(account_params)
+    @account = current_account
+    render 'api/v1/accounts/show'
+  end
+
+  private
+
+  def account_params
+    params.permit(:display_name, :note, :avatar, :header)
+  end
+end
diff --git a/app/controllers/api/v1/accounts/follower_accounts_controller.rb b/app/controllers/api/v1/accounts/follower_accounts_controller.rb
new file mode 100644
index 00000000000..3e9da29e341
--- /dev/null
+++ b/app/controllers/api/v1/accounts/follower_accounts_controller.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+class Api::V1::Accounts::FollowerAccountsController < ApiController
+  before_action -> { doorkeeper_authorize! :read }
+  before_action :set_account
+  after_action :insert_pagination_headers
+
+  respond_to :json
+
+  def index
+    @accounts = load_accounts
+    render 'api/v1/accounts/index'
+  end
+
+  private
+
+  def set_account
+    @account = Account.find(params[:account_id])
+  end
+
+  def load_accounts
+    default_accounts.merge(paginated_follows).to_a
+  end
+
+  def default_accounts
+    Account.includes(:active_relationships).references(:active_relationships)
+  end
+
+  def paginated_follows
+    Follow.where(target_account: @account).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_account_followers_url pagination_params(max_id: pagination_max_id)
+    end
+  end
+
+  def prev_path
+    unless @accounts.empty?
+      api_v1_account_followers_url pagination_params(since_id: pagination_since_id)
+    end
+  end
+
+  def pagination_max_id
+    @accounts.last.active_relationships.first.id
+  end
+
+  def pagination_since_id
+    @accounts.first.active_relationships.first.id
+  end
+
+  def records_continue?
+    @accounts.size == limit_param(DEFAULT_ACCOUNTS_LIMIT)
+  end
+
+  def pagination_params(core_params)
+    params.permit(:limit).merge(core_params)
+  end
+end
diff --git a/app/controllers/api/v1/accounts/following_accounts_controller.rb b/app/controllers/api/v1/accounts/following_accounts_controller.rb
new file mode 100644
index 00000000000..732961aac10
--- /dev/null
+++ b/app/controllers/api/v1/accounts/following_accounts_controller.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+class Api::V1::Accounts::FollowingAccountsController < ApiController
+  before_action -> { doorkeeper_authorize! :read }
+  before_action :set_account
+  after_action :insert_pagination_headers
+
+  respond_to :json
+
+  def index
+    @accounts = load_accounts
+    render 'api/v1/accounts/index'
+  end
+
+  private
+
+  def set_account
+    @account = Account.find(params[:account_id])
+  end
+
+  def load_accounts
+    default_accounts.merge(paginated_follows).to_a
+  end
+
+  def default_accounts
+    Account.includes(:passive_relationships).references(:passive_relationships)
+  end
+
+  def paginated_follows
+    Follow.where(account: @account).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_account_following_index_url pagination_params(max_id: pagination_max_id)
+    end
+  end
+
+  def prev_path
+    unless @accounts.empty?
+      api_v1_account_following_index_url pagination_params(since_id: pagination_since_id)
+    end
+  end
+
+  def pagination_max_id
+    @accounts.last.passive_relationships.first.id
+  end
+
+  def pagination_since_id
+    @accounts.first.passive_relationships.first.id
+  end
+
+  def records_continue?
+    @accounts.size == limit_param(DEFAULT_ACCOUNTS_LIMIT)
+  end
+
+  def pagination_params(core_params)
+    params.permit(:limit).merge(core_params)
+  end
+end
diff --git a/app/controllers/api/v1/accounts/relationships_controller.rb b/app/controllers/api/v1/accounts/relationships_controller.rb
new file mode 100644
index 00000000000..d1a4f178b6c
--- /dev/null
+++ b/app/controllers/api/v1/accounts/relationships_controller.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+class Api::V1::Accounts::RelationshipsController < ApiController
+  before_action -> { doorkeeper_authorize! :read }
+  before_action :require_user!
+
+  respond_to :json
+
+  def index
+    @accounts = Account.where(id: account_ids).select('id')
+    @following = Account.following_map(account_ids, current_user.account_id)
+    @followed_by = Account.followed_by_map(account_ids, current_user.account_id)
+    @blocking = Account.blocking_map(account_ids, current_user.account_id)
+    @muting = Account.muting_map(account_ids, current_user.account_id)
+    @requested = Account.requested_map(account_ids, current_user.account_id)
+    @domain_blocking = Account.domain_blocking_map(account_ids, current_user.account_id)
+  end
+
+  private
+
+  def account_ids
+    @_account_ids ||= Array(params[:id]).map(&:to_i)
+  end
+end
diff --git a/app/controllers/api/v1/accounts/search_controller.rb b/app/controllers/api/v1/accounts/search_controller.rb
new file mode 100644
index 00000000000..6d4c6e4cf9a
--- /dev/null
+++ b/app/controllers/api/v1/accounts/search_controller.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+class Api::V1::Accounts::SearchController < ApiController
+  before_action -> { doorkeeper_authorize! :read }
+  before_action :require_user!
+
+  respond_to :json
+
+  def show
+    @accounts = account_search
+
+    render 'api/v1/accounts/index'
+  end
+
+  private
+
+  def account_search
+    AccountSearchService.new.call(
+      params[:q],
+      limit_param(DEFAULT_ACCOUNTS_LIMIT),
+      resolving_search?,
+      current_account
+    )
+  end
+
+  def resolving_search?
+    params[:resolve] == 'true'
+  end
+end
diff --git a/app/controllers/api/v1/accounts/statuses_controller.rb b/app/controllers/api/v1/accounts/statuses_controller.rb
new file mode 100644
index 00000000000..1e0d2a740b0
--- /dev/null
+++ b/app/controllers/api/v1/accounts/statuses_controller.rb
@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+class Api::V1::Accounts::StatusesController < ApiController
+  before_action -> { doorkeeper_authorize! :read }
+  before_action :set_account
+  after_action :insert_pagination_headers
+
+  respond_to :json
+
+  def index
+    @statuses = load_statuses
+  end
+
+  private
+
+  def set_account
+    @account = Account.find(params[:account_id])
+  end
+
+  def load_statuses
+    cached_account_statuses.tap do |statuses|
+      set_maps(statuses)
+    end
+  end
+
+  def cached_account_statuses
+    cache_collection account_statuses, Status
+  end
+
+  def account_statuses
+    default_statuses.tap do |statuses|
+      statuses.merge!(only_media_scope) if params[:only_media]
+      statuses.merge!(no_replies_scope) if params[:exclude_replies]
+    end
+  end
+
+  def default_statuses
+    permitted_account_statuses.paginate_by_max_id(
+      limit_param(DEFAULT_STATUSES_LIMIT),
+      params[:max_id],
+      params[:since_id]
+    )
+  end
+
+  def permitted_account_statuses
+    @account.statuses.permitted_for(@account, current_account)
+  end
+
+  def only_media_scope
+    Status.where(id: account_media_status_ids)
+  end
+
+  def account_media_status_ids
+    @account.media_attachments.attached.reorder(nil).select(:status_id).distinct
+  end
+
+  def no_replies_scope
+    Status.without_replies
+  end
+
+  def pagination_params(core_params)
+    params.permit(:limit, :only_media, :exclude_replies).merge(core_params)
+  end
+
+  def insert_pagination_headers
+    set_pagination_headers(next_path, prev_path)
+  end
+
+  def next_path
+    if records_continue?
+      api_v1_account_statuses_url pagination_params(max_id: pagination_max_id)
+    end
+  end
+
+  def prev_path
+    unless @statuses.empty?
+      api_v1_account_statuses_url pagination_params(since_id: pagination_since_id)
+    end
+  end
+
+  def records_continue?
+    @statuses.size == limit_param(DEFAULT_STATUSES_LIMIT)
+  end
+
+  def pagination_max_id
+    @statuses.last.id
+  end
+
+  def pagination_since_id
+    @statuses.first.id
+  end
+end
diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb
index bd6fb6a2a85..3b23e996d61 100644
--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@@ -1,73 +1,15 @@
 # frozen_string_literal: true
 
 class Api::V1::AccountsController < ApiController
-  before_action -> { doorkeeper_authorize! :read }, except: [:follow, :unfollow, :block, :unblock, :mute, :unmute, :update_credentials]
+  before_action -> { doorkeeper_authorize! :read }, except: [:follow, :unfollow, :block, :unblock, :mute, :unmute]
   before_action -> { doorkeeper_authorize! :follow }, only: [:follow, :unfollow, :block, :unblock, :mute, :unmute]
-  before_action -> { doorkeeper_authorize! :write }, only: [:update_credentials]
-  before_action :require_user!, except: [:show, :following, :followers, :statuses]
-  before_action :set_account, except: [:verify_credentials, :update_credentials, :suggestions, :search]
+  before_action :require_user!, except: [:show]
+  before_action :set_account
 
   respond_to :json
 
   def show; end
 
-  def verify_credentials
-    @account = current_user.account
-    render :show
-  end
-
-  def update_credentials
-    current_account.update!(account_params)
-    @account = current_account
-    render :show
-  end
-
-  def following
-    @accounts = Account.includes(:passive_relationships)
-                       .references(:passive_relationships)
-                       .merge(Follow.where(account: @account)
-                                    .paginate_by_max_id(limit_param(DEFAULT_ACCOUNTS_LIMIT), params[:max_id], params[:since_id]))
-                       .to_a
-
-    next_path = following_api_v1_account_url(pagination_params(max_id: @accounts.last.passive_relationships.first.id))     if @accounts.size == limit_param(DEFAULT_ACCOUNTS_LIMIT)
-    prev_path = following_api_v1_account_url(pagination_params(since_id: @accounts.first.passive_relationships.first.id)) unless @accounts.empty?
-
-    set_pagination_headers(next_path, prev_path)
-
-    render :index
-  end
-
-  def followers
-    @accounts = Account.includes(:active_relationships)
-                       .references(:active_relationships)
-                       .merge(Follow.where(target_account: @account)
-                                    .paginate_by_max_id(limit_param(DEFAULT_ACCOUNTS_LIMIT),
-                                                        params[:max_id],
-                                                        params[:since_id]))
-                       .to_a
-
-    next_path = followers_api_v1_account_url(pagination_params(max_id: @accounts.last.active_relationships.first.id))     if @accounts.size == limit_param(DEFAULT_ACCOUNTS_LIMIT)
-    prev_path = followers_api_v1_account_url(pagination_params(since_id: @accounts.first.active_relationships.first.id)) unless @accounts.empty?
-
-    set_pagination_headers(next_path, prev_path)
-
-    render :index
-  end
-
-  def statuses
-    @statuses = @account.statuses.permitted_for(@account, current_account).paginate_by_max_id(limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], params[:since_id])
-    @statuses = @statuses.where(id: MediaAttachment.where(account: @account).where.not(status_id: nil).reorder('').select('distinct status_id')) if params[:only_media]
-    @statuses = @statuses.without_replies if params[:exclude_replies]
-    @statuses = cache_collection(@statuses, Status)
-
-    set_maps(@statuses)
-
-    next_path = statuses_api_v1_account_url(statuses_pagination_params(max_id: @statuses.last.id))    if @statuses.size == limit_param(DEFAULT_STATUSES_LIMIT)
-    prev_path = statuses_api_v1_account_url(statuses_pagination_params(since_id: @statuses.first.id)) unless @statuses.empty?
-
-    set_pagination_headers(next_path, prev_path)
-  end
-
   def follow
     FollowService.new.call(current_user.account, @account.acct)
     set_relationship
@@ -111,24 +53,6 @@ class Api::V1::AccountsController < ApiController
     render :relationship
   end
 
-  def relationships
-    ids = params[:id].is_a?(Enumerable) ? params[:id].map(&:to_i) : [params[:id].to_i]
-
-    @accounts        = Account.where(id: ids).select('id')
-    @following       = Account.following_map(ids, current_user.account_id)
-    @followed_by     = Account.followed_by_map(ids, current_user.account_id)
-    @blocking        = Account.blocking_map(ids, current_user.account_id)
-    @muting          = Account.muting_map(ids, current_user.account_id)
-    @requested       = Account.requested_map(ids, current_user.account_id)
-    @domain_blocking = Account.domain_blocking_map(ids, current_user.account_id)
-  end
-
-  def search
-    @accounts = AccountSearchService.new.call(params[:q], limit_param(DEFAULT_ACCOUNTS_LIMIT), params[:resolve] == 'true', current_account)
-
-    render :index
-  end
-
   private
 
   def set_account
@@ -143,16 +67,4 @@ class Api::V1::AccountsController < ApiController
     @requested       = Account.requested_map([@account.id], current_user.account_id)
     @domain_blocking = Account.domain_blocking_map([@account.id], current_user.account_id)
   end
-
-  def pagination_params(core_params)
-    params.permit(:limit).merge(core_params)
-  end
-
-  def statuses_pagination_params(core_params)
-    params.permit(:limit, :only_media, :exclude_replies).merge(core_params)
-  end
-
-  def account_params
-    params.permit(:display_name, :note, :avatar, :header)
-  end
 end
diff --git a/app/views/api/v1/accounts/relationships.rabl b/app/views/api/v1/accounts/relationships/index.rabl
index 022ea2ac499..022ea2ac499 100644
--- a/app/views/api/v1/accounts/relationships.rabl
+++ b/app/views/api/v1/accounts/relationships/index.rabl
diff --git a/app/views/api/v1/accounts/statuses.rabl b/app/views/api/v1/accounts/statuses/index.rabl
index 44d29d91ba2..44d29d91ba2 100644
--- a/app/views/api/v1/accounts/statuses.rabl
+++ b/app/views/api/v1/accounts/statuses/index.rabl
diff --git a/config/routes.rb b/config/routes.rb
index b1590f9291d..5c1c8e594f8 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -169,19 +169,18 @@ Rails.application.routes.draw do
         end
       end
 
+      namespace :accounts do
+        get :verify_credentials, to: 'credentials#show'
+        patch :update_credentials, to: 'credentials#update'
+        resource :search, only: :show, controller: :search
+        resources :relationships, only: :index
+      end
       resources :accounts, only: [:show] do
-        collection do
-          get :relationships
-          get :verify_credentials
-          patch :update_credentials
-          get :search
-        end
+        resources :statuses, only: :index, controller: 'accounts/statuses'
+        resources :followers, only: :index, controller: 'accounts/follower_accounts'
+        resources :following, only: :index, controller: 'accounts/following_accounts'
 
         member do
-          get :statuses
-          get :followers
-          get :following
-
           post :follow
           post :unfollow
           post :block
diff --git a/spec/controllers/api/v1/accounts/credentials_controller_spec.rb b/spec/controllers/api/v1/accounts/credentials_controller_spec.rb
new file mode 100644
index 00000000000..89dbca11190
--- /dev/null
+++ b/spec/controllers/api/v1/accounts/credentials_controller_spec.rb
@@ -0,0 +1,55 @@
+require 'rails_helper'
+
+describe Api::V1::Accounts::CredentialsController do
+  render_views
+
+  let(:user)  { Fabricate(:user, account: Fabricate(:account, username: 'alice')) }
+  let(:token) { double acceptable?: true, resource_owner_id: user.id }
+
+  before do
+    allow(controller).to receive(:doorkeeper_token) { token }
+  end
+
+  describe 'GET #show' do
+    it 'returns http success' do
+      get :show
+      expect(response).to have_http_status(:success)
+    end
+  end
+
+  describe 'PATCH #update' do
+    describe 'with valid data' do
+      before do
+        patch :update, params: {
+          display_name: "Alice Isn't Dead",
+          note: "Hi!\n\nToot toot!",
+          avatar: fixture_file_upload('files/avatar.gif', 'image/gif'),
+          header: fixture_file_upload('files/attachment.jpg', 'image/jpeg'),
+        }
+      end
+
+      it 'returns http success' do
+        expect(response).to have_http_status(:success)
+      end
+
+      it 'updates account info' do
+        user.account.reload
+
+        expect(user.account.display_name).to eq("Alice Isn't Dead")
+        expect(user.account.note).to eq("Hi!\n\nToot toot!")
+        expect(user.account.avatar).to exist
+        expect(user.account.header).to exist
+      end
+    end
+
+    describe 'with invalid data' do
+      before do
+        patch :update, params: { note: 'This is too long. ' * 10 }
+      end
+
+      it 'returns http unprocessable entity' do
+        expect(response).to have_http_status(:unprocessable_entity)
+      end
+    end
+  end
+end
diff --git a/spec/controllers/api/v1/accounts/follower_accounts_controller_spec.rb b/spec/controllers/api/v1/accounts/follower_accounts_controller_spec.rb
new file mode 100644
index 00000000000..171852c755d
--- /dev/null
+++ b/spec/controllers/api/v1/accounts/follower_accounts_controller_spec.rb
@@ -0,0 +1,21 @@
+require 'rails_helper'
+
+describe Api::V1::Accounts::FollowerAccountsController do
+  render_views
+
+  let(:user)  { Fabricate(:user, account: Fabricate(:account, username: 'alice')) }
+  let(:token) { double acceptable?: true, resource_owner_id: user.id }
+
+  before do
+    Fabricate(:follow, target_account: user.account)
+    allow(controller).to receive(:doorkeeper_token) { token }
+  end
+
+  describe 'GET #index' do
+    it 'returns http success' do
+      get :index, params: { account_id: user.account.id, limit: 1 }
+
+      expect(response).to have_http_status(:success)
+    end
+  end
+end
diff --git a/spec/controllers/api/v1/accounts/following_accounts_controller_spec.rb b/spec/controllers/api/v1/accounts/following_accounts_controller_spec.rb
new file mode 100644
index 00000000000..a4cad9163e7
--- /dev/null
+++ b/spec/controllers/api/v1/accounts/following_accounts_controller_spec.rb
@@ -0,0 +1,21 @@
+require 'rails_helper'
+
+describe Api::V1::Accounts::FollowingAccountsController do
+  render_views
+
+  let(:user)  { Fabricate(:user, account: Fabricate(:account, usernam