Add spec coverage and refactor authorize_follows controller (#2505)
This commit is contained in:
		
							parent
							
								
									a23eaf720e
								
							
						
					
					
						commit
						0618f09939
					
				
					 8 changed files with 170 additions and 53 deletions
				
			
		|  | @ -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 |  | ||||||
							
								
								
									
										61
									
								
								app/controllers/authorize_follows_controller.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										61
									
								
								app/controllers/authorize_follows_controller.rb
									
									
									
									
									
										Normal file
									
								
							|  | @ -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 | ||||||
|  | @ -71,8 +71,7 @@ Rails.application.routes.draw do | ||||||
|   resources :tags,  only: [:show] |   resources :tags,  only: [:show] | ||||||
| 
 | 
 | ||||||
|   # Remote follow |   # Remote follow | ||||||
|   get  :authorize_follow, to: 'authorize_follow#new' |   resource :authorize_follow, only: [:show, :create] | ||||||
|   post :authorize_follow, to: 'authorize_follow#create' |  | ||||||
| 
 | 
 | ||||||
|   namespace :admin do |   namespace :admin do | ||||||
|     resources :pubsubhubbub, only: [:index] |     resources :pubsubhubbub, only: [:index] | ||||||
|  |  | ||||||
|  | @ -1,6 +0,0 @@ | ||||||
| require 'rails_helper' |  | ||||||
| 
 |  | ||||||
| RSpec.describe AuthorizeFollowController, type: :controller do |  | ||||||
|   describe 'GET #new' |  | ||||||
|   describe 'POST #create' |  | ||||||
| end |  | ||||||
							
								
								
									
										108
									
								
								spec/controllers/authorize_follows_controller_spec.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										108
									
								
								spec/controllers/authorize_follows_controller_spec.rb
									
									
									
									
									
										Normal file
									
								
							|  | @ -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 | ||||||
		Loading…
	
		Reference in a new issue