From d6774d2ca39b66ba6a80e707dbd58a6b391ee274 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <mjankowski@thoughtbot.com>
Date: Wed, 31 May 2017 14:27:17 -0400
Subject: [PATCH] Refactor and spec coverage for api/v1/timelines actions
 (#3482)

---
 .../api/v1/timelines/base_controller.rb       | 30 -------
 .../api/v1/timelines/home_controller.rb       | 80 ++++++++++-------
 .../api/v1/timelines/public_controller.rb     | 77 ++++++++++------
 .../api/v1/timelines/tag_controller.rb        | 90 +++++++++++--------
 .../api/v1/timelines/{base => }/show.rabl     |  0
 spec/routing/api_routing_spec.rb              | 20 +++++
 spec/routing/api_timelines_spec.rb            | 18 ----
 7 files changed, 171 insertions(+), 144 deletions(-)
 delete mode 100644 app/controllers/api/v1/timelines/base_controller.rb
 rename app/views/api/v1/timelines/{base => }/show.rabl (100%)
 create mode 100644 spec/routing/api_routing_spec.rb
 delete mode 100644 spec/routing/api_timelines_spec.rb

diff --git a/app/controllers/api/v1/timelines/base_controller.rb b/app/controllers/api/v1/timelines/base_controller.rb
deleted file mode 100644
index 4eb29e74a2..0000000000
--- a/app/controllers/api/v1/timelines/base_controller.rb
+++ /dev/null
@@ -1,30 +0,0 @@
-# frozen_string_literal: true
-
-module Api::V1::Timelines
-  class BaseController < ApiController
-    respond_to :json
-    after_action :insert_pagination_headers, unless: -> { @statuses.empty? }
-
-    private
-
-    def cache_collection(raw)
-      super(raw, Status)
-    end
-
-    def pagination_params(core_params)
-      params.permit(:local, :limit).merge(core_params)
-    end
-
-    def insert_pagination_headers
-      set_pagination_headers(next_path, prev_path)
-    end
-
-    def next_path
-      raise 'Override in child controllers'
-    end
-
-    def prev_path
-      raise 'Override in child controllers'
-    end
-  end
-end
diff --git a/app/controllers/api/v1/timelines/home_controller.rb b/app/controllers/api/v1/timelines/home_controller.rb
index 33ff48b398..29e570fa5e 100644
--- a/app/controllers/api/v1/timelines/home_controller.rb
+++ b/app/controllers/api/v1/timelines/home_controller.rb
@@ -1,44 +1,62 @@
 # frozen_string_literal: true
 
-module Api::V1::Timelines
-  class HomeController < BaseController
-    before_action -> { doorkeeper_authorize! :read }, only: [:show]
-    before_action :require_user!, only: [:show]
+class Api::V1::Timelines::HomeController < ApiController
+  before_action -> { doorkeeper_authorize! :read }, only: [:show]
+  before_action :require_user!, only: [:show]
+  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }
 
-    def show
-      @statuses = load_statuses
-    end
+  respond_to :json
 
-    private
+  def show
+    @statuses = load_statuses
+    render 'api/v1/timelines/show'
+  end
 
-    def load_statuses
-      cached_home_statuses.tap do |statuses|
-        set_maps(statuses)
-      end
-    end
+  private
 
-    def cached_home_statuses
-      cache_collection home_statuses
+  def load_statuses
+    cached_home_statuses.tap do |statuses|
+      set_maps(statuses)
     end
+  end
 
-    def home_statuses
-      account_home_feed.get(
-        limit_param(DEFAULT_STATUSES_LIMIT),
-        params[:max_id],
-        params[:since_id]
-      )
-    end
+  def cached_home_statuses
+    cache_collection home_statuses, Status
+  end
 
-    def account_home_feed
-      Feed.new(:home, current_account)
-    end
+  def home_statuses
+    account_home_feed.get(
+      limit_param(DEFAULT_STATUSES_LIMIT),
+      params[:max_id],
+      params[:since_id]
+    )
+  end
 
-    def next_path
-      api_v1_timelines_home_url pagination_params(max_id: @statuses.last.id)
-    end
+  def account_home_feed
+    Feed.new(:home, current_account)
+  end
 
-    def prev_path
-      api_v1_timelines_home_url pagination_params(since_id: @statuses.first.id)
-    end
+  def insert_pagination_headers
+    set_pagination_headers(next_path, prev_path)
+  end
+
+  def pagination_params(core_params)
+    params.permit(:local, :limit).merge(core_params)
+  end
+
+  def next_path
+    api_v1_timelines_home_url pagination_params(max_id: pagination_max_id)
+  end
+
+  def prev_path
+    api_v1_timelines_home_url pagination_params(since_id: pagination_since_id)
+  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/timelines/public_controller.rb b/app/controllers/api/v1/timelines/public_controller.rb
index 644c7a6f17..cd3663d5f4 100644
--- a/app/controllers/api/v1/timelines/public_controller.rb
+++ b/app/controllers/api/v1/timelines/public_controller.rb
@@ -1,41 +1,60 @@
 # frozen_string_literal: true
 
-module Api::V1::Timelines
-  class PublicController < BaseController
-    def show
-      @statuses = load_statuses
-    end
+class Api::V1::Timelines::PublicController < ApiController
+  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }
 
-    private
+  respond_to :json
 
-    def load_statuses
-      cached_public_statuses.tap do |statuses|
-        set_maps(statuses)
-      end
-    end
+  def show
+    @statuses = load_statuses
+    render 'api/v1/timelines/show'
+  end
 
-    def cached_public_statuses
-      cache_collection public_statuses
-    end
+  private
 
-    def public_statuses
-      public_timeline_statuses.paginate_by_max_id(
-        limit_param(DEFAULT_STATUSES_LIMIT),
-        params[:max_id],
-        params[:since_id]
-      )
+  def load_statuses
+    cached_public_statuses.tap do |statuses|
+      set_maps(statuses)
     end
+  end
 
-    def public_timeline_statuses
-      Status.as_public_timeline(current_account, params[:local])
-    end
+  def cached_public_statuses
+    cache_collection public_statuses, Status
+  end
 
-    def next_path
-      api_v1_timelines_public_url pagination_params(max_id: @statuses.last.id)
-    end
+  def public_statuses
+    public_timeline_statuses.paginate_by_max_id(
+      limit_param(DEFAULT_STATUSES_LIMIT),
+      params[:max_id],
+      params[:since_id]
+    )
+  end
 
-    def prev_path
-      api_v1_timelines_public_url pagination_params(since_id: @statuses.first.id)
-    end
+  def public_timeline_statuses
+    Status.as_public_timeline(current_account, params[:local])
+  end
+
+  def insert_pagination_headers
+    set_pagination_headers(next_path, prev_path)
+  end
+
+  def pagination_params(core_params)
+    params.permit(:local, :limit).merge(core_params)
+  end
+
+  def next_path
+    api_v1_timelines_public_url pagination_params(max_id: pagination_max_id)
+  end
+
+  def prev_path
+    api_v1_timelines_public_url pagination_params(since_id: pagination_since_id)
+  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/timelines/tag_controller.rb b/app/controllers/api/v1/timelines/tag_controller.rb
index 818f49d3de..0481f5debe 100644
--- a/app/controllers/api/v1/timelines/tag_controller.rb
+++ b/app/controllers/api/v1/timelines/tag_controller.rb
@@ -1,51 +1,69 @@
 # frozen_string_literal: true
 
-module Api::V1::Timelines
-  class TagController < BaseController
-    before_action :load_tag
+class Api::V1::Timelines::TagController < ApiController
+  before_action :load_tag
+  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }
 
-    def show
-      @statuses = load_statuses
-    end
+  respond_to :json
 
-    private
+  def show
+    @statuses = load_statuses
+    render 'api/v1/timelines/show'
+  end
 
-    def load_tag
-      @tag = Tag.find_by(name: params[:id].downcase)
-    end
+  private
 
-    def load_statuses
-      cached_tagged_statuses.tap do |statuses|
-        set_maps(statuses)
-      end
-    end
+  def load_tag
+    @tag = Tag.find_by(name: params[:id].downcase)
+  end
 
-    def cached_tagged_statuses
-      cache_collection tagged_statuses
+  def load_statuses
+    cached_tagged_statuses.tap do |statuses|
+      set_maps(statuses)
     end
+  end
 
-    def tagged_statuses
-      if @tag.nil?
-        []
-      else
-        tag_timeline_statuses.paginate_by_max_id(
-          limit_param(DEFAULT_STATUSES_LIMIT),
-          params[:max_id],
-          params[:since_id]
-        )
-      end
-    end
+  def cached_tagged_statuses
+    cache_collection tagged_statuses, Status
+  end
 
-    def tag_timeline_statuses
-      Status.as_tag_timeline(@tag, current_account, params[:local])
+  def tagged_statuses
+    if @tag.nil?
+      []
+    else
+      tag_timeline_statuses.paginate_by_max_id(
+        limit_param(DEFAULT_STATUSES_LIMIT),
+        params[:max_id],
+        params[:since_id]
+      )
     end
+  end
 
-    def next_path
-      api_v1_timelines_tag_url params[:id], pagination_params(max_id: @statuses.last.id)
-    end
+  def tag_timeline_statuses
+    Status.as_tag_timeline(@tag, current_account, params[:local])
+  end
 
-    def prev_path
-      api_v1_timelines_tag_url params[:id], pagination_params(since_id: @statuses.first.id)
-    end
+  def insert_pagination_headers
+    set_pagination_headers(next_path, prev_path)
+  end
+
+  def pagination_params(core_params)
+    params.permit(:local, :limit).merge(core_params)
+  end
+
+  def next_path
+    api_v1_timelines_tag_url params[:id], pagination_params(max_id: pagination_max_id)
+  end
+
+  def prev_path
+    api_v1_timelines_tag_url params[:id], pagination_params(since_id: pagination_since_id)
+  end
+
+  def pagination_max_id
+    @statuses.last.id
+  end
+
+  def pagination_since_id
+    @statuses.first.id
   end
 end
diff --git a/app/views/api/v1/timelines/base/show.rabl b/app/views/api/v1/timelines/show.rabl
similarity index 100%
rename from app/views/api/v1/timelines/base/show.rabl
rename to app/views/api/v1/timelines/show.rabl
diff --git a/spec/routing/api_routing_spec.rb b/spec/routing/api_routing_spec.rb
new file mode 100644
index 0000000000..973b4801d9
--- /dev/null
+++ b/spec/routing/api_routing_spec.rb
@@ -0,0 +1,20 @@
+require 'rails_helper'
+
+describe 'API routes' do
+  describe 'Timeline routes' do
+    it 'routes to home timeline' do
+      expect(get('/api/v1/timelines/home')).
+        to route_to('api/v1/timelines/home#show')
+    end
+
+    it 'routes to public timeline' do
+      expect(get('/api/v1/timelines/public')).
+        to route_to('api/v1/timelines/public#show')
+    end
+
+    it 'routes to tag timeline' do
+      expect(get('/api/v1/timelines/tag/test')).
+        to route_to('api/v1/timelines/tag#show', id: 'test')
+    end
+  end
+end
diff --git a/spec/routing/api_timelines_spec.rb b/spec/routing/api_timelines_spec.rb
deleted file mode 100644
index 31717209f0..0000000000
--- a/spec/routing/api_timelines_spec.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-require 'rails_helper'
-
-describe 'API timeline routes' do
-  it 'routes to home timeline' do
-    expect(get('/api/v1/timelines/home')).
-      to route_to('api/v1/timelines/home#show')
-  end
-
-  it 'routes to public timeline' do
-    expect(get('/api/v1/timelines/public')).
-      to route_to('api/v1/timelines/public#show')
-  end
-
-  it 'routes to tag timeline' do
-    expect(get('/api/v1/timelines/tag/test')).
-      to route_to('api/v1/timelines/tag#show', id: 'test')
-  end
-end