Skip to content

Commit

Permalink
conditionally allow total pages on account_courses endpoint
Browse files Browse the repository at this point in the history
test plan
 - last_page should be nil for account_courses endpoint
 - with setting set, it should not be nil
 - see specs, they should pass

fixes VICE-1996
flag=none

Change-Id: Ibaf8aff044650f6c0ffcc7f7ccbd8fa99cd53e0d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272598
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Matthew Lemon <mlemon@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
  • Loading branch information
roor0 committed Aug 31, 2021
1 parent b1871e1 commit 65abe7c
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 1 deletion.
4 changes: 3 additions & 1 deletion app/controllers/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,9 @@ def courses_api

page_opts = {}
# don't calculate a total count for this endpoint.
page_opts[:total_entries] = nil
if params[:search_term] || !@account.allow_last_page_on_account_courses?
page_opts[:total_entries] = nil
end

all_precalculated_permissions = nil
GuardRail.activate(:secondary) do
Expand Down
1 change: 1 addition & 0 deletions app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ def resolved_outcome_calculation_method
# push notifications which may be routed through US datacenters by Google/Apple
add_setting :enable_push_notifications, boolean: true, root_only: true, default: true
add_setting :allow_last_page_on_course_users, boolean: true, root_only: true, default: false
add_setting :allow_last_page_on_account_courses, boolean: true, root_only: true, default: false

def settings=(hash)
if hash.is_a?(Hash) || hash.is_a?(ActionController::Parameters)
Expand Down
10 changes: 10 additions & 0 deletions spec/controllers/accounts_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1223,6 +1223,16 @@ def admin_logged_in(account)
expect(response.headers.to_a.find { |a| a.first == "Link" }.last).to_not include("last")
end

it "should set pagination total_pages/last page link if account setting is set" do
@account.settings[:allow_last_page_on_account_courses] = true
@account.save!
admin_logged_in(@account)
get 'courses_api', params: {:account_id => @account.id, per_page: 1}

expect(response).to be_successful
expect(response.headers.to_a.find { |a| a.first == "Link" }.last).to include("last")
end

it "should properly remove sections from includes" do
@s1 = @course.course_sections.create!
@course.enroll_student(user_factory(:active_all => true), :section => @s1, :allow_multiple_enrollments => true)
Expand Down

0 comments on commit 65abe7c

Please sign in to comment.