Refactor and spec coverage for api/v1/timelines actions (#3482)

app/controllers/api/v1/timelines/base_controller.rb

@@ -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

app/controllers/api/v1/timelines/home_controller.rb

@@ -1,12 +1,15 @@
 # frozen_string_literal: true
 
-module Api::V1::Timelines
+class Api::V1::Timelines::HomeController < ApiController
-  class HomeController < BaseController
   before_action -> { doorkeeper_authorize! :read }, only: [:show]
   before_action :require_user!, only: [:show]
+  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }
+
+  respond_to :json
 
   def show
     @statuses = load_statuses
+    render 'api/v1/timelines/show'
   end
 
   private
@@ -18,7 +21,7 @@ module Api::V1::Timelines
   end
 
   def cached_home_statuses
-      cache_collection home_statuses
+    cache_collection home_statuses, Status
   end
 
   def home_statuses
@@ -33,12 +36,27 @@ module Api::V1::Timelines
     Feed.new(:home, current_account)
   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: @statuses.last.id)
+    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: @statuses.first.id)
+    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

app/controllers/api/v1/timelines/public_controller.rb

@@ -1,9 +1,13 @@
 # frozen_string_literal: true
 
-module Api::V1::Timelines
+class Api::V1::Timelines::PublicController < ApiController
-  class PublicController < BaseController
+  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }
+
+  respond_to :json
+
   def show
     @statuses = load_statuses
+    render 'api/v1/timelines/show'
   end
 
   private
@@ -15,7 +19,7 @@ module Api::V1::Timelines
   end
 
   def cached_public_statuses
-      cache_collection public_statuses
+    cache_collection public_statuses, Status
   end
 
   def public_statuses
@@ -30,12 +34,27 @@ module Api::V1::Timelines
     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: @statuses.last.id)
+    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: @statuses.first.id)
+    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

app/controllers/api/v1/timelines/tag_controller.rb

@@ -1,11 +1,14 @@
 # frozen_string_literal: true
 
-module Api::V1::Timelines
+class Api::V1::Timelines::TagController < ApiController
-  class TagController < BaseController
   before_action :load_tag
+  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }
+
+  respond_to :json
 
   def show
     @statuses = load_statuses
+    render 'api/v1/timelines/show'
   end
 
   private
@@ -21,7 +24,7 @@ module Api::V1::Timelines
   end
 
   def cached_tagged_statuses
-      cache_collection tagged_statuses
+    cache_collection tagged_statuses, Status
   end
 
   def tagged_statuses
@@ -40,12 +43,27 @@ module Api::V1::Timelines
     Status.as_tag_timeline(@tag, 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_tag_url params[:id], pagination_params(max_id: @statuses.last.id)
+    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: @statuses.first.id)
+    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

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

spec/routing/api_timelines_spec.rb

@@ -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