Modules & assignments LTI tool selection shows 100 results

Also adds basic test coverage around Api.paginate

Fixes PLAT-1357

Test Plan:

1) Create a test course and add 100 LTI tools to it:
course = Course.find(15)

(1..100).each do |n|
  cet = ContextExternalTool.new(
    url: "https://www.example.com",
    name: "Tool ##{n}",
    shared_secret:"foo",
    consumer_key:"bar")
  cet.context = course
  cet.save!
end

2) Try to add an LTI tool to a module or as an
   assignment, see that you can see all 100 tools.


Change-Id: I9c59d2286f928ad726917e9e794967dcf6ffca9d
Reviewed-on: https://gerrit.instructure.com/73680
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Matthew Wheeler <mwheeler@instructure.com>
This commit is contained in:
Matthew Wheeler 2016-03-03 14:21:29 -07:00
parent aabcb2b8ec
commit f0c1b8ca8b
4 changed files with 47 additions and 5 deletions

View File

@ -35,8 +35,9 @@ module Lti
if authorized_action(@context, @current_user, :update)
placements = params['placements'] || []
collection = AppLaunchCollator.bookmarked_collection(@context, placements)
pagination_args = {max_per_page: 100}
respond_to do |format|
launch_defs = Api.paginate(collection, self, named_context_url(@context, :api_v1_context_launch_definitions_url, include_host: true))
launch_defs = Api.paginate(collection, self, named_context_url(@context, :api_v1_context_launch_definitions_url, include_host: true), pagination_args)
format.json { render :json => AppLaunchCollator.launch_definitions(launch_defs, placements) }
end
end

View File

@ -83,7 +83,7 @@
<i class="icon-link"></i>
<%= t :external_tool_notice, "Select a tool from the list below, or enter a URL for an external tool you already know is configured with Basic LTI to add a link to it to this *module*.", :wrapper => '<span class="holder_type">\1</span>' %>
</div>
<a href="<%= context_url(@context, controller:'lti/lti_apps', action:'launch_definitions', :placements => lti_app_placements, :per_page => '50') %>" class="external_tools_url" style="display: none;">&nbsp;</a>
<a href="<%= context_url(@context, controller:'lti/lti_apps', action:'launch_definitions', :placements => lti_app_placements, :per_page => '100') %>" class="external_tools_url" style="display: none;">&nbsp;</a>
<table>
<tr>
<td colspan="2">

View File

@ -257,10 +257,14 @@ module Api
Setting.get('api_max_per_page', '50').to_i
end
def self.per_page
Setting.get('api_per_page', '10').to_i
end
def self.per_page_for(controller, options={})
per_page = controller.params[:per_page] || options[:default] || Setting.get('api_per_page', '10')
per_page_requested = controller.params[:per_page] || options[:default] || per_page
max = options[:max] || max_per_page
[[per_page.to_i, 1].max, max.to_i].min
[[per_page_requested.to_i, 1].max, max.to_i].min
end
# Add [link HTTP Headers](http://www.w3.org/Protocols/9707-link-header.html) for pagination

View File

@ -716,7 +716,8 @@ describe Api do
let(:collection) { [1, 2, 3] }
it "should not raise Folio::InvalidPage for pages past the end" do
expect(Api.paginate(collection, controller, 'example.com', page: collection.size + 1, per_page: 1)).
controller = stub('controller', request: request, response: response, params: {per_page: 1})
expect(Api.paginate(collection, controller, 'example.com', page: collection.size + 1)).
to eq []
end
@ -738,6 +739,42 @@ describe Api do
to raise_error(Folio::InvalidPage)
end
end
describe "page size limits" do
let(:collection) { (1..101).to_a }
context "with no max_per_page argument" do
it "should limit to the default max_per_page" do
controller = stub('controller', request: request, response: response, params: {per_page: Api.max_per_page + 5})
expect(Api.paginate(collection, controller, 'example.com').size).
to eq Api.max_per_page
end
end
context "with no per_page parameter" do
it "should limit to the default per_page" do
controller = stub('controller', request: request, response: response, params: {})
expect(Api.paginate(collection, controller, 'example.com').size).
to eq Api.per_page
end
end
context "with per_page parameter > max_per_page argument" do
let(:controller) { stub('controller', request: request, response: response, params: {per_page: 100}) }
it "should take the smaller of the max_per_page arugment and the per_page param" do
expect(Api.paginate(collection, controller, 'example.com', {max_per_page: 75}).size).
to eq 75
end
end
context "with per_page parameter < max_per_page argument" do
let(:controller) { stub('controller', request: request, response: response, params: {per_page: 75}) }
it "should take the smaller of the max_per_page arugment and the per_page param" do
expect(Api.paginate(collection, controller, 'example.com', {max_per_page: 100}).size).
to eq 75
end
end
end
end
context ".build_links" do