From 8bac0350d16cb8e2770c089c91d71f8038404de5 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <mjankowski@thoughtbot.com>
Date: Wed, 19 Apr 2017 07:52:37 -0400
Subject: [PATCH] Restful refactor of accounts/ routes (#2133)

* Add routing specs for accounts followers and following actions

* Use more restful route naming for public account follow pages

Moves two actions:
- accounts#followers to accounts/follower_accounts#index
- accounts#following to accounts/following_accounts#index

Adds routing spec to ensure prior URLs are preserved.
---
 app/controllers/account_follow_controller.rb  | 12 +++++
 .../account_unfollow_controller.rb            | 12 +++++
 app/controllers/accounts_controller.rb        | 37 +-------------
 .../concerns/account_controller_concern.rb    | 51 +++++++++++++++++++
 .../follower_accounts_controller.rb           | 15 ++++++
 .../following_accounts_controller.rb          | 15 ++++++
 app/views/accounts/_follow_grid.html.haml     |  7 +++
 app/views/accounts/_header.html.haml          | 12 ++---
 app/views/follower_accounts/index.html.haml   |  6 +++
 app/views/following_accounts/index.html.haml  |  6 +++
 config/routes.rb                              | 11 ++--
 .../account_follow_controller_spec.rb         | 24 +++++++++
 .../account_unfollow_controller_spec.rb       | 24 +++++++++
 spec/controllers/accounts_controller_spec.rb  | 14 -----
 .../follower_accounts_controller_spec.rb      | 14 +++++
 .../following_accounts_controller_spec.rb     | 14 +++++
 spec/requests/link_headers_spec.rb            | 33 ++++++++++++
 spec/routing/accounts_routing_spec.rb         | 31 +++++++++++
 18 files changed, 275 insertions(+), 63 deletions(-)
 create mode 100644 app/controllers/account_follow_controller.rb
 create mode 100644 app/controllers/account_unfollow_controller.rb
 create mode 100644 app/controllers/concerns/account_controller_concern.rb
 create mode 100644 app/controllers/follower_accounts_controller.rb
 create mode 100644 app/controllers/following_accounts_controller.rb
 create mode 100644 app/views/accounts/_follow_grid.html.haml
 create mode 100644 app/views/follower_accounts/index.html.haml
 create mode 100644 app/views/following_accounts/index.html.haml
 create mode 100644 spec/controllers/account_follow_controller_spec.rb
 create mode 100644 spec/controllers/account_unfollow_controller_spec.rb
 create mode 100644 spec/controllers/follower_accounts_controller_spec.rb
 create mode 100644 spec/controllers/following_accounts_controller_spec.rb
 create mode 100644 spec/requests/link_headers_spec.rb
 create mode 100644 spec/routing/accounts_routing_spec.rb

diff --git a/app/controllers/account_follow_controller.rb b/app/controllers/account_follow_controller.rb
new file mode 100644
index 00000000000..185a355f8f3
--- /dev/null
+++ b/app/controllers/account_follow_controller.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class AccountFollowController < ApplicationController
+  include AccountControllerConcern
+
+  before_action :authenticate_user!
+
+  def create
+    FollowService.new.call(current_user.account, @account.acct)
+    redirect_to account_path(@account)
+  end
+end
diff --git a/app/controllers/account_unfollow_controller.rb b/app/controllers/account_unfollow_controller.rb
new file mode 100644
index 00000000000..378ec86dc62
--- /dev/null
+++ b/app/controllers/account_unfollow_controller.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class AccountUnfollowController < ApplicationController
+  include AccountControllerConcern
+
+  before_action :authenticate_user!
+
+  def create
+    UnfollowService.new.call(current_user.account, @account)
+    redirect_to account_path(@account)
+  end
+end
diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb
index d4f157614d7..8eda9633651 100644
--- a/app/controllers/accounts_controller.rb
+++ b/app/controllers/accounts_controller.rb
@@ -1,12 +1,7 @@
 # frozen_string_literal: true
 
 class AccountsController < ApplicationController
-  layout 'public'
-
-  before_action :set_account
-  before_action :set_link_headers
-  before_action :authenticate_user!, only: [:follow, :unfollow]
-  before_action :check_account_suspension
+  include AccountControllerConcern
 
   def show
     respond_to do |format|
@@ -24,39 +19,9 @@ class AccountsController < ApplicationController
     end
   end
 
-  def follow
-    FollowService.new.call(current_user.account, @account.acct)
-    redirect_to account_path(@account)
-  end
-
-  def unfollow
-    UnfollowService.new.call(current_user.account, @account)
-    redirect_to account_path(@account)
-  end
-
-  def followers
-    @followers = @account.followers.order('follows.created_at desc').page(params[:page]).per(12)
-  end
-
-  def following
-    @following = @account.following.order('follows.created_at desc').page(params[:page]).per(12)
-  end
-
   private
 
   def set_account
     @account = Account.find_local!(params[:username])
   end
-
-  def set_link_headers
-    response.headers['Link'] = LinkHeader.new([[webfinger_account_url, [%w(rel lrdd), %w(type application/xrd+xml)]], [account_url(@account, format: 'atom'), [%w(rel alternate), %w(type application/atom+xml)]]])
-  end
-
-  def webfinger_account_url
-    webfinger_url(resource: @account.to_webfinger_s)
-  end
-
-  def check_account_suspension
-    gone if @account.suspended?
-  end
 end
diff --git a/app/controllers/concerns/account_controller_concern.rb b/app/controllers/concerns/account_controller_concern.rb
new file mode 100644
index 00000000000..d36fc8c9362
--- /dev/null
+++ b/app/controllers/concerns/account_controller_concern.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+module AccountControllerConcern
+  extend ActiveSupport::Concern
+
+  FOLLOW_PER_PAGE = 12
+
+  included do
+    layout 'public'
+    before_action :set_account
+    before_action :set_link_headers
+    before_action :check_account_suspension
+  end
+
+  private
+
+  def set_account
+    @account = Account.find_local!(params[:account_username])
+  end
+
+  def set_link_headers
+    response.headers['Link'] = LinkHeader.new(
+      [
+        webfinger_account_link,
+        atom_account_url_link,
+      ]
+    )
+  end
+
+  def webfinger_account_link
+    [
+      webfinger_account_url,
+      [%w(rel lrdd), %w(type application/xrd+xml)],
+    ]
+  end
+
+  def atom_account_url_link
+    [
+      account_url(@account, format: 'atom'),
+      [%w(rel alternate), %w(type application/atom+xml)],
+    ]
+  end
+
+  def webfinger_account_url
+    webfinger_url(resource: @account.to_webfinger_s)
+  end
+
+  def check_account_suspension
+    gone if @account.suspended?
+  end
+end
diff --git a/app/controllers/follower_accounts_controller.rb b/app/controllers/follower_accounts_controller.rb
new file mode 100644
index 00000000000..165cbf1b56e
--- /dev/null
+++ b/app/controllers/follower_accounts_controller.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class FollowerAccountsController < ApplicationController
+  include AccountControllerConcern
+
+  def index
+    @accounts = ordered_accounts.page(params[:page]).per(FOLLOW_PER_PAGE)
+  end
+
+  private
+
+  def ordered_accounts
+    @account.followers.order('follows.created_at desc')
+  end
+end
diff --git a/app/controllers/following_accounts_controller.rb b/app/controllers/following_accounts_controller.rb
new file mode 100644
index 00000000000..faeea144ac4
--- /dev/null
+++ b/app/controllers/following_accounts_controller.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class FollowingAccountsController < ApplicationController
+  include AccountControllerConcern
+
+  def index
+    @accounts = ordered_accounts.page(params[:page]).per(FOLLOW_PER_PAGE)
+  end
+
+  private
+
+  def ordered_accounts
+    @account.following.order('follows.created_at desc')
+  end
+end
diff --git a/app/views/accounts/_follow_grid.html.haml b/app/views/accounts/_follow_grid.html.haml
new file mode 100644
index 00000000000..322a0ebf4e1
--- /dev/null
+++ b/app/views/accounts/_follow_grid.html.haml
@@ -0,0 +1,7 @@
+.accounts-grid
+  - if accounts.empty?
+    = render partial: 'accounts/nothing_here'
+  - else
+    = render partial: 'accounts/grid_card', collection: accounts, as: :account, cached: true
+
+= paginate accounts
diff --git a/app/views/accounts/_header.html.haml b/app/views/accounts/_header.html.haml
index 6538885a01e..ed13ab57ca9 100644
--- a/app/views/accounts/_header.html.haml
+++ b/app/views/accounts/_header.html.haml
@@ -2,9 +2,9 @@
   - if user_signed_in? && current_account.id != account.id && !current_account.requested?(account)
     .controls
       - if current_account.following?(account)
-        = link_to t('accounts.unfollow'), unfollow_account_path(account), data: { method: :post }, class: 'button'
+        = link_to t('accounts.unfollow'), account_unfollow_path(account), data: { method: :post }, class: 'button'
       - else
-        = link_to t('accounts.follow'), follow_account_path(account), data: { method: :post }, class: 'button'
+        = link_to t('accounts.follow'), account_follow_path(account), data: { method: :post }, class: 'button'
   - elsif !user_signed_in?
     .controls
       .remote-follow
@@ -24,11 +24,11 @@
         = link_to short_account_url(account), class: 'u-url u-uid' do
           %span.counter-label= t('accounts.posts')
           %span.counter-number= number_with_delimiter account.statuses_count
-      .counter{ class: active_nav_class(following_account_url(account)) }
-        = link_to following_account_url(account) do
+      .counter{ class: active_nav_class(account_following_index_url(account)) }
+        = link_to account_following_index_url(account) do
           %span.counter-label= t('accounts.following')
           %span.counter-number= number_with_delimiter account.following_count
-      .counter{ class: active_nav_class(followers_account_url(account)) }
-        = link_to followers_account_url(account) do
+      .counter{ class: active_nav_class(account_followers_url(account)) }
+        = link_to account_followers_url(account) do
           %span.counter-label= t('accounts.followers')
           %span.counter-number= number_with_delimiter account.followers_count
diff --git a/app/views/follower_accounts/index.html.haml b/app/views/follower_accounts/index.html.haml
new file mode 100644
index 00000000000..c30d601e612
--- /dev/null
+++ b/app/views/follower_accounts/index.html.haml
@@ -0,0 +1,6 @@
+- content_for :page_title do
+  = t('accounts.people_who_follow', name: display_name(@account))
+
+= render 'accounts/header', account: @account
+
+= render 'accounts/follow_grid', accounts: @accounts
diff --git a/app/views/following_accounts/index.html.haml b/app/views/following_accounts/index.html.haml
new file mode 100644
index 00000000000..cd3737591cb
--- /dev/null
+++ b/app/views/following_accounts/index.html.haml
@@ -0,0 +1,6 @@
+- content_for :page_title do
+  = t('accounts.people_followed_by', name: display_name(@account))
+
+= render 'accounts/header', account: @account
+
+= render 'accounts/follow_grid', accounts: @accounts
diff --git a/config/routes.rb b/config/routes.rb
index fef5bf916dd..bc67016a487 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -37,13 +37,10 @@ Rails.application.routes.draw do
     get :remote_follow,  to: 'remote_follow#new'
     post :remote_follow, to: 'remote_follow#create'
 
-    member do
-      get :followers
-      get :following
-
-      post :follow
-      post :unfollow
-    end
+    resources :followers, only: [:index], controller: :follower_accounts
+    resources :following, only: [:index], controller: :following_accounts
+    resource :follow, only: [:create], controller: :account_follow
+    resource :unfollow, only: [:create], controller: :account_unfollow
   end
 
   get '/@:username', to: 'accounts#show', as: :short_account
diff --git a/spec/controllers/account_follow_controller_spec.rb b/spec/controllers/account_follow_controller_spec.rb
new file mode 100644
index 00000000000..479101c67de
--- /dev/null
+++ b/spec/controllers/account_follow_controller_spec.rb
@@ -0,0 +1,24 @@
+require 'rails_helper'
+
+describe AccountFollowController do
+  render_views
+  let(:user) { Fabricate(:user) }
+  let(:alice) { Fabricate(:account, username: 'alice') }
+
+  describe 'POST #create' do
+    before do
+      sign_in(user)
+    end
+
+    it 'redirects to account path' do
+      service = double
+      allow(FollowService).to receive(:new).and_return(service)
+      allow(service).to receive(:call)
+
+      post :create, params: { account_username: alice.username }
+
+      expect(service).to have_received(:call).with(user.account, 'alice')
+      expect(response).to redirect_to(account_path(alice))
+    end
+  end
+end
diff --git a/spec/controllers/account_unfollow_controller_spec.rb b/spec/controllers/account_unfollow_controller_spec.rb
new file mode 100644
index 00000000000..1f28bf4ab86
--- /dev/null
+++ b/spec/controllers/account_unfollow_controller_spec.rb
@@ -0,0 +1,24 @@
+require 'rails_helper'
+
+describe AccountUnfollowController do
+  render_views
+  let(:user) { Fabricate(:user) }
+  let(:alice) { Fabricate(:account, username: 'alice') }
+
+  describe 'POST #create' do
+    before do
+      sign_in(user)
+    end
+
+    it 'redirects to account path' do
+      service = double
+      allow(UnfollowService).to receive(:new).and_return(service)
+      allow(service).to receive(:call)
+
+      post :create, params: { account_username: alice.username }
+
+      expect(service).to have_received(:call).with(user.account, alice)
+      expect(response).to redirect_to(account_path(alice))
+    end
+  end
+end
diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb
index d2c93c7079f..94d10d78f13 100644
--- a/spec/controllers/accounts_controller_spec.rb
+++ b/spec/controllers/accounts_controller_spec.rb
@@ -44,18 +44,4 @@ RSpec.describe AccountsController, type: :controller do
       end
     end
   end
-
-  describe 'GET #followers' do
-    it 'returns http success' do
-      get :followers, params: { username: alice.username }
-      expect(response).to have_http_status(:success)
-    end
-  end
-
-  describe 'GET #following' do
-    it 'returns http success' do
-      get :following, params: { username: alice.username }
-      expect(response).to have_http_status(:success)
-    end
-  end
 end
diff --git a/spec/controllers/follower_accounts_controller_spec.rb b/spec/controllers/follower_accounts_controller_spec.rb
new file mode 100644
index 00000000000..82d2b2067c5
--- /dev/null
+++ b/spec/controllers/follower_accounts_controller_spec.rb
@@ -0,0 +1,14 @@
+require 'rails_helper'
+
+describe FollowerAccountsController do
+  render_views
+  let(:alice) { Fabricate(:account, username: 'alice') }
+
+  describe 'GET #index' do
+    it 'returns http success' do
+      get :index, params: { account_username: alice.username }
+
+      expect(response).to have_http_status(:success)
+    end
+  end
+end
diff --git a/spec/controllers/following_accounts_controller_spec.rb b/spec/controllers/following_accounts_controller_spec.rb
new file mode 100644
index 00000000000..5b5a6fe5f2f
--- /dev/null
+++ b/spec/controllers/following_accounts_controller_spec.rb
@@ -0,0 +1,14 @@
+require 'rails_helper'
+
+describe FollowingAccountsController do
+  render_views
+  let(:alice) { Fabricate(:account, username: 'alice') }
+
+  describe 'GET #index' do
+    it 'returns http success' do
+      get :index, params: { account_username: alice.username }
+
+      expect(response).to have_http_status(:success)
+    end
+  end
+end
diff --git a/spec/requests/link_headers_spec.rb b/spec/requests/link_headers_spec.rb
new file mode 100644
index 00000000000..3947806cccd
--- /dev/null
+++ b/spec/requests/link_headers_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe 'Link headers' do
+  describe 'on the account show page' do
+    let(:account) { Fabricate(:account, username: 'test') }
+
+    before do
+      get short_account_path(username: account)
+    end
+
+    it 'contains webfinger url in link header' do
+      link_header = link_header_with_type('application/xrd+xml')
+
+      expect(link_header.href).to match 'http://www.example.com/.well-known/webfinger?resource=acct%3Atest%40cb6e6126.ngrok.io'
+      expect(link_header.attr_pairs.first).to eq %w[rel lrdd]
+    end
+
+    it 'contains atom url in link header' do
+      link_header = link_header_with_type('application/atom+xml')
+
+      expect(link_header.href).to eq 'http://www.example.com/users/test.atom'
+      expect(link_header.attr_pairs.first).to eq %w[rel alternate]
+    end
+
+    def link_header_with_type(type)
+      response.headers['Link'].links.find do |link|
+        link.attr_pairs.any? { |pair| pair == ['type', type] }
+      end
+    end
+  end
+end
diff --git a/spec/routing/accounts_routing_spec.rb b/spec/routing/accounts_routing_spec.rb
new file mode 100644
index 00000000000..d04cb27f04a
--- /dev/null
+++ b/spec/routing/accounts_routing_spec.rb
@@ -0,0 +1,31 @@
+require 'rails_helper'
+
+describe 'Routes under accounts/' do
+  describe 'the route for accounts who are followers of an account' do
+    it 'routes to the followers action with the right username' do
+      expect(get('/users/name/followers')).
+        to route_to('follower_accounts#index', account_username: 'name')
+    end
+  end
+
+  describe 'the route for accounts who are followed by an account' do
+    it 'routes to the following action with the right username' do
+      expect(get('/users/name/following')).
+        to route_to('following_accounts#index', account_username: 'name')
+    end
+  end
+
+  describe 'the route for following an account' do
+    it 'routes to the follow create action with the right username' do
+      expect(post('/users/name/follow')).
+        to route_to('account_follow#create', account_username: 'name')
+    end
+  end
+
+  describe 'the route for unfollowing an account' do
+    it 'routes to the unfollow create action with the right username' do
+      expect(post('/users/name/unfollow')).
+        to route_to('account_unfollow#create', account_username: 'name')
+    end
+  end
+end