From 0618f09939b7afc607bcd983139b91e056debe4d Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 26 Apr 2017 18:19:53 -0400 Subject: [PATCH] Add spec coverage and refactor authorize_follows controller (#2505) --- .../authorize_follow_controller.rb | 45 -------- .../authorize_follows_controller.rb | 61 ++++++++++ .../_card.html.haml | 0 .../error.html.haml | 0 .../show.html.haml} | 0 config/routes.rb | 3 +- .../authorize_follow_controller_spec.rb | 6 - .../authorize_follows_controller_spec.rb | 108 ++++++++++++++++++ 8 files changed, 170 insertions(+), 53 deletions(-) delete mode 100644 app/controllers/authorize_follow_controller.rb create mode 100644 app/controllers/authorize_follows_controller.rb rename app/views/{authorize_follow => authorize_follows}/_card.html.haml (100%) rename app/views/{authorize_follow => authorize_follows}/error.html.haml (100%) rename app/views/{authorize_follow/new.html.haml => authorize_follows/show.html.haml} (100%) delete mode 100644 spec/controllers/authorize_follow_controller_spec.rb create mode 100644 spec/controllers/authorize_follows_controller_spec.rb diff --git a/app/controllers/authorize_follow_controller.rb b/app/controllers/authorize_follow_controller.rb deleted file mode 100644 index 9b28a9455a..0000000000 --- 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 0000000000..f00646e204 --- /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 similarity index 100% rename from app/views/authorize_follow/_card.html.haml rename to 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 similarity index 100% rename from app/views/authorize_follow/error.html.haml rename to 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 similarity index 100% rename from app/views/authorize_follow/new.html.haml rename to app/views/authorize_follows/show.html.haml diff --git a/config/routes.rb b/config/routes.rb index 34c4fca4c7..9adaffcafd 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 954efd53e7..0000000000 --- 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 0000000000..f65b620cc1 --- /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