clean up paginate calls

fixes CNVS-8791
fixes CNVS-8795

 * "without_count: true" -> "total_entries: nil"
 * move order clauses to the pagination target, rather than being in the
   pagination call
 * clean up implementation of first/last page links in Api.paginate

test-plan:
  - have an account report with at least two instances
  - fetch /api/v1/accounts/:account_id/reports/:report
  - response should have most recent report instance first

  - have two subaccounts under an account
  - fetch /api/v1/accounts/:id/sub_accounts?recursive=true&per_page=1
  - Links response header should not have a link with rel=last
  - fetch /api/v1/accounts/:id/sub_accounts?recursive=false&per_page=1
  - Links response header should have a link with rel=last embedding
    page=2

  - load /error_reports
  - should have most recent reports first

  - fetch /api/v1/conversations/batches
  - should have oldest batch first

Change-Id: Ifef79b193720a09ad7fe059ed23e930c97d10f59
Reviewed-on: https://gerrit.instructure.com/26535
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
This commit is contained in:
Jacob Fugal 2013-11-20 17:16:42 -07:00
parent 59a05a5bb9
commit ac39cfcf22
6 changed files with 10 additions and 10 deletions

View File

@ -147,8 +147,7 @@ class AccountReportsController < ApplicationController
def index
if authorized_action(@context, @current_user, :read_reports)
reports = Api.paginate(type_scope, self, url_for({:action => :index, :controller => :account_reports}),
:order => 'start_at DESC')
reports = Api.paginate(type_scope.order('start_at DESC'), self, url_for({:action => :index, :controller => :account_reports}))
render :json => account_reports_json(reports, @current_user, session)
end

View File

@ -95,7 +95,7 @@ class AccountsController < ApplicationController
end
@accounts = Api.paginate(@accounts, self, api_v1_sub_accounts_url,
:without_count => recursive)
:total_entries => recursive ? nil : @accounts.count)
render :json => @accounts.map { |a| account_json(a, @current_user, session, []) }
end

View File

@ -303,10 +303,9 @@ class ConversationsController < ApplicationController
# }
# ]
def batches
batches = Api.paginate(@current_user.conversation_batches.in_progress,
batches = Api.paginate(@current_user.conversation_batches.in_progress.order(:id),
self,
api_v1_conversations_batches_url,
:order => :id)
api_v1_conversations_batches_url)
render :json => batches.map{ |m| conversation_batch_json(m, @current_user, session) }
end

View File

@ -38,7 +38,11 @@ class ErrorsController < ApplicationController
@reports = @reports.where(:category => params[:category])
end
@reports = @reports.paginate(:per_page => PER_PAGE, :page => params[:page], :order => 'id DESC', :without_count => true)
# temporary handling of having total_entries nil, since the will_paginate
# view helper doesn't handle it yet (it's making its way through gerrit)
@reports = @reports.order('id DESC').paginate(:per_page => PER_PAGE, :page => params[:page] + 1, :total_entries => nil)
@reports.total_entries = @reports.offset + @reports.size
@reports.pop if @reports.size > params[:page]
end
def show

View File

@ -171,10 +171,10 @@ class PageViewsController < ApplicationController
end
page_views = @user.page_views(date_options)
url = api_v1_user_page_views_url(url_options)
@page_views = Api.paginate(page_views, self, url, :order => 'created_at DESC', :without_count => :true)
respond_to do |format|
format.json do
@page_views = Api.paginate(page_views, self, url, :total_entries => nil)
render :json => page_views_json(@page_views, @current_user, session)
end
format.csv do

View File

@ -201,8 +201,6 @@ module Api
# The collection needs to be a will_paginate collection (or act like one)
# a new, paginated collection will be returned
def self.paginate(collection, controller, base_url, pagination_args = {})
pagination_args[:total_entries] = nil if pagination_args[:without_count]
pagination_args.reverse_merge!(
page: controller.params[:page],
per_page: per_page_for(controller,