Add spec coverage and refactor authorize_follows controller (#2505)

8 files changed, 170 insertions, 53 deletions

diff --git a/app/controllers/authorize_follow_controller.rb b/app/controllers/authorize_follow_controller.rb
deleted file mode 100644
index 9b28a9455..000000000
--- a/app/controllers/authorize_follow_controller.rb
+++ /dev/null
@@ -1,45 +0,0 @@
-# frozen_string_literal: true
-
-class AuthorizeFollowController < ApplicationController
-  layout 'public'
-
-  before_action :authenticate_user!
-
-  def new
-    uri = Addressable::URI.parse(acct_param).normalize
-
-    if uri.path && %w(http https).include?(uri.scheme)
-      set_account_from_url
-    else
-      set_account_from_acct
-    end
-
-    render :error if @account.nil?
-  end
-
-  def create
-    @account = FollowService.new.call(current_account, acct_param).try(:target_account)
-
-    if @account.nil?
-      render :error
-    else
-      redirect_to web_url("accounts/#{@account.id}")
-    end
-  rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError
-    render :error
-  end
-
-  private
-
-  def set_account_from_url
-    @account = FetchRemoteAccountService.new.call(acct_param)
-  end
-
-  def set_account_from_acct
-    @account = FollowRemoteAccountService.new.call(acct_param)
-  end
-
-  def acct_param
-    params[:acct].gsub(/\Aacct:/, '')
-  end
-end
diff --git a/app/controllers/authorize_follows_controller.rb b/app/controllers/authorize_follows_controller.rb
new file mode 100644
index 000000000..f00646e20
--- /dev/null
+++ b/app/controllers/authorize_follows_controller.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+class AuthorizeFollowsController < ApplicationController
+  layout 'public'
+
+  before_action :authenticate_user!
+
+  def show
+    @account = located_account || render(:error)
+  end
+
+  def create
+    @account = follow_attempt.try(:target_account)
+
+    if @account.nil?
+      render :error
+    else
+      redirect_to web_url("accounts/#{@account.id}")
+    end
+  rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError
+    render :error
+  end
+
+  private
+
+  def follow_attempt
+    FollowService.new.call(current_account, acct_without_prefix)
+  end
+
+  def located_account
+    if acct_param_is_url?
+      account_from_remote_fetch
+    else
+      account_from_remote_follow
+    end
+  end
+
+  def account_from_remote_fetch
+    FetchRemoteAccountService.new.call(acct_without_prefix)
+  end
+
+  def account_from_remote_follow
+    FollowRemoteAccountService.new.call(acct_without_prefix)
+  end
+
+  def acct_param_is_url?
+    parsed_uri.path && %w[http https].include?(parsed_uri.scheme)
+  end
+
+  def parsed_uri
+    Addressable::URI.parse(acct_without_prefix).normalize
+  end
+
+  def acct_without_prefix
+    acct_params.gsub(/\Aacct:/, '')
+  end
+
+  def acct_params
+    params.fetch(:acct, '')
+  end
+end
diff --git a/app/views/authorize_follow/_card.html.haml b/app/views/authorize_follows/_card.html.haml
index ee76f628d..ee76f628d 100644
--- a/app/views/authorize_follow/_card.html.haml
+++ b/app/views/authorize_follows/_card.html.haml
diff --git a/app/views/authorize_follow/error.html.haml b/app/views/authorize_follows/error.html.haml
index 88d33b68d..88d33b68d 100644
--- a/app/views/authorize_follow/error.html.haml
+++ b/app/views/authorize_follows/error.html.haml
diff --git a/app/views/authorize_follow/new.html.haml b/app/views/authorize_follows/show.html.haml
index 3b60df058..3b60df058 100644
--- a/app/views/authorize_follow/new.html.haml
+++ b/app/views/authorize_follows/show.html.haml
diff --git a/config/routes.rb b/config/routes.rb
index 34c4fca4c..9adaffcaf 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -71,8 +71,7 @@ Rails.application.routes.draw do
   resources :tags,  only: [:show]
 
   # Remote follow
-  get  :authorize_follow, to: 'authorize_follow#new'
-  post :authorize_follow, to: 'authorize_follow#create'
+  resource :authorize_follow, only: [:show, :create]
 
   namespace :admin do
     resources :pubsubhubbub, only: [:index]
diff --git a/spec/controllers/authorize_follow_controller_spec.rb b/spec/controllers/authorize_follow_controller_spec.rb
deleted file mode 100644
index 954efd53e..000000000
--- a/spec/controllers/authorize_follow_controller_spec.rb
+++ /dev/null
@@ -1,6 +0,0 @@
-require 'rails_helper'
-
-RSpec.describe AuthorizeFollowController, type: :controller do
-  describe 'GET #new'
-  describe 'POST #create'
-end
diff --git a/spec/controllers/authorize_follows_controller_spec.rb b/spec/controllers/authorize_follows_controller_spec.rb
new file mode 100644
index 000000000..f65b620cc
--- /dev/null
+++ b/spec/controllers/authorize_follows_controller_spec.rb
@@ -0,0 +1,108 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe AuthorizeFollowsController do
+  describe 'GET #show' do
+    describe 'when signed out' do
+      it 'redirects to sign in page' do
+        get :show
+
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+
+    describe 'when signed in' do
+      let(:user) { Fabricate(:user) }
+      let(:account) { Fabricate(:account, user: user) }
+
+      before do
+        sign_in(user)
+      end
+
+      it 'renders error without acct param' do
+        get :show
+
+        expect(response).to render_template(:error)
+      end
+
+      it 'renders error when account cant be found' do
+        service = double
+        allow(FollowRemoteAccountService).to receive(:new).and_return(service)
+        allow(service).to receive(:call).with('missing@hostname').and_return(nil)
+
+        get :show, params: { acct: 'acct:missing@hostname' }
+
+        expect(response).to render_template(:error)
+        expect(service).to have_received(:call).with('missing@hostname')
+      end
+
+      it 'sets account from url' do
+        account = double
+        service = double
+        allow(FetchRemoteAccountService).to receive(:new).and_return(service)
+        allow(service).to receive(:call).with('http://example.com').and_return(account)
+
+        get :show, params: { acct: 'http://example.com' }
+
+        expect(response).to have_http_status(:success)
+        expect(service).to have_received(:call).with('http://example.com')
+      end
+
+      it 'sets account from acct uri' do
+        account = double
+        service = double
+        allow(FollowRemoteAccountService).to receive(:new).and_return(service)
+        allow(service).to receive(:call).with('found@hostname').and_return(account)
+
+        get :show, params: { acct: 'acct:found@hostname' }
+
+        expect(response).to have_http_status(:success)
+        expect(service).to have_received(:call).with('found@hostname')
+      end
+    end
+  end
+
+  describe 'POST #create' do
+    describe 'when signed out' do
+      it 'redirects to sign in page' do
+        post :create
+
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+
+    describe 'when signed in' do
+      let(:user) { Fabricate(:user) }
+      let(:account) { Fabricate(:account, user: user) }
+
+      before do
+        sign_in(user)
+      end
+
+      it 'shows error when account not found' do
+        service = double
+        allow(FollowService).to receive(:new).and_return(service)
+        allow(service).to receive(:call).with(account, 'user@hostname').and_return(nil)
+
+        post :create, params: { acct: 'acct:user@hostname' }
+
+        expect(service).to have_received(:call).with(account, 'user@hostname')
+        expect(response).to render_template(:error)
+      end
+
+      it 'follows account when found' do
+        target_account = double(id: '123')
+        result_account = double(target_account: target_account)
+        service = double
+        allow(FollowService).to receive(:new).and_return(service)
+        allow(service).to receive(:call).with(account, 'user@hostname').and_return(result_account)
+
+        post :create, params: { acct: 'acct:user@hostname' }
+
+        expect(service).to have_received(:call).with(account, 'user@hostname')
+        expect(response).to redirect_to(web_url('accounts/123'))
+      end
+    end
+  end
+end