ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix sorting of calendar_events api 

the sort keys used for the calendar_events api were not stable (could
contain NULLs) which caused pagination to skip some items, depending on
how postgres optimized the query

for additional information, see:
http://www.postgresql.org/docs/9.3/static/queries-limit.html

closes CNVS-13497

test plan:
 * create 51+ assignments in a course (pagination boundary)
   - some with no due date
   - some with due dates
   - some with overridden due dates
   ! creation order is critical !
     creation order of each sample type above should be mixed with the
     creation of others. additionally, the dated variants should be
     created in chronological as well as reverse-chronological order.
 * create 51+ calendar events in a course (pagination boundary)
   - some undated
   - some with dates
   ! creation order is critical !
     creation order of each sample type above should be mixed with the
     creation of others. additionally, the dated variants should be
     created in chronological as well as reverse-chronological order.
 * ensure the following pages account for all above items:
   - syllabus
   - calendar

Change-Id: If1de63d8dcfbf6cfc17613e47263c5a2aa25fd4d
Reviewed-on: https://gerrit.instructure.com/38653
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Joel Hough <joel@instructure.com>
QA-Review: Steven Shepherd <sshepherd@instructure.com>
Product-Review: Janelle Seegmiller <jseegmiller@instructure.com>
This commit is contained in:
Mark Severson 2014-08-01 16:28:00 -06:00
parent 0a7d44b815
commit 52f84cbe49
2 changed files with 41 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/controllers/calendar_events_api_controller.rb
View File

@ -713,7 +713,7 @@ class CalendarEventsApiController < ApplicationController
# Fully ordering by due_at requires examining all the overrides linked and as it applies to
# specific people, sections, etc. This applies the base assignment due_at for ordering
# as a more sane default then natural DB order. No, it isn't perfect but much better.
scope = assignment_context_scope.active.order_by_base_due_at
scope = assignment_context_scope.active.order_by_base_due_at.order('assignments.id ASC')
scope = scope.send(*date_scope_and_args(:due_between_with_overrides)) unless @all_events
scope
@ -768,7 +768,7 @@ class CalendarEventsApiController < ApplicationController
end
def calendar_event_scope
scope = CalendarEvent.active.order_by_start_at
scope = CalendarEvent.active.order_by_start_at.order(:id)
if @current_user
scope = scope.for_user_and_context_codes(@current_user, @context_codes, @section_codes)
else
@ -795,7 +795,7 @@ class CalendarEventsApiController < ApplicationController
if dates_list.empty?
assignments << assignment
return assignments
next assignments
end
original_dates, overridden_dates = dates_list.partition { |date| date[:base] }

58
spec/apis/v1/calendar_events_api_spec.rb
View File

@ -117,19 +117,28 @@ describe CalendarEventsApiController, type: :request do
end
end
it 'should paginate events' do
ids = 5.times.map { |i| @course.calendar_events.create(:title => "#{i}", :start_at => '2012-01-08 12:00:00').id }
json = api_call(:get, "/api/v1/calendar_events?start_date=2012-01-08&end_date=2012-01-08&context_codes[]=course_#{@course.id}&per_page=2", {
:controller => 'calendar_events_api', :action => 'index', :format => 'json',
:context_codes => ["course_#{@course.id}"], :start_date => '2012-01-08', :end_date => '2012-01-08', :per_page => '2'})
json.size.should eql 2
response.headers['Link'].should match(%r{<http://www.example.com/api/v1/calendar_events\?.*page=2.*>; rel="next",<http://www.example.com/api/v1/calendar_events\?.*page=1.*>; rel="first",<http://www.example.com/api/v1/calendar_events\?.*page=3.*>; rel="last"})
it 'should sort and paginate events' do
undated = (1..7).map {|i| @course.calendar_events.create(title: "undated:#{i}", start_at: nil, end_at: nil).id }
dated = (1..18).map {|i| @course.calendar_events.create(title: "dated:#{i}", start_at: Time.parse('2012-01-20 12:00:00').advance(days: -i)).id }
ids = dated.reverse + undated
json = api_call(:get, "/api/v1/calendar_events?start_date=2012-01-08&end_date=2012-01-08&context_codes[]=course_#{@course.id}&per_page=2&page=3", {
json = api_call(:get, "/api/v1/calendar_events?all_events=1&context_codes[]=course_#{@course.id}&per_page=10", {
:controller => 'calendar_events_api', :action => 'index', :format => 'json',
:context_codes => ["course_#{@course.id}"], :start_date => '2012-01-08', :end_date => '2012-01-08', :per_page => '2', :page => '3'})
json.size.should eql 1
:context_codes => ["course_#{@course.id}"], :all_events => 1, :per_page => '10'})
response.headers['Link'].should match(%r{<http://www.example.com/api/v1/calendar_events\?.*page=2.*>; rel="next",<http://www.example.com/api/v1/calendar_events\?.*page=1.*>; rel="first",<http://www.example.com/api/v1/calendar_events\?.*page=3.*>; rel="last"})
json.map{|a| a['id']}.should eql ids[0...10]
json = api_call(:get, "/api/v1/calendar_events?all_events=1&context_codes[]=course_#{@course.id}&per_page=10&page=2", {
:controller => 'calendar_events_api', :action => 'index', :format => 'json',
:context_codes => ["course_#{@course.id}"], :all_events => 1, :per_page => '10', :page => '2'})
response.headers['Link'].should match(%r{<http://www.example.com/api/v1/calendar_events\?.*page=3.*>; rel="next",<http://www.example.com/api/v1/calendar_events\?.*page=1.*>; rel="prev",<http://www.example.com/api/v1/calendar_events\?.*page=1.*>; rel="first",<http://www.example.com/api/v1/calendar_events\?.*page=3.*>; rel="last"})
json.map{|a| a['id']}.should eql ids[10...20]
json = api_call(:get, "/api/v1/calendar_events?all_events=1&context_codes[]=course_#{@course.id}&per_page=10&page=3", {
:controller => 'calendar_events_api', :action => 'index', :format => 'json',
:context_codes => ["course_#{@course.id}"], :all_events => 1, :per_page => '10', :page => '3'})
response.headers['Link'].should match(%r{<http://www.example.com/api/v1/calendar_events\?.*page=2.*>; rel="prev",<http://www.example.com/api/v1/calendar_events\?.*page=1.*>; rel="first",<http://www.example.com/api/v1/calendar_events\?.*page=3.*>; rel="last"})
json.map{|a| a['id']}.should eql ids[20...25]
end
it 'should ignore invalid end_dates' do
@ -787,19 +796,28 @@ describe CalendarEventsApiController, type: :request do
json.map { |event| event['title'] }.should == %w[1 2 3]
end
it 'should paginate assignments' do
ids = create_assignments(@course.id, 25, due_at: '2012-01-08 12:00:00')
json = api_call(:get, "/api/v1/calendar_events?type=assignment&start_date=2012-01-08&end_date=2012-01-08&context_codes[]=course_#{@course.id}&per_page=10", {
:controller => 'calendar_events_api', :action => 'index', :format => 'json', :type => 'assignment',
:context_codes => ["course_#{@course.id}"], :start_date => '2012-01-08', :end_date => '2012-01-08', :per_page => '10'})
json.size.should eql 10
response.headers['Link'].should match(%r{<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=2.*>; rel="next",<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=1.*>; rel="first",<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=3.*>; rel="last"})
it 'should sort and paginate assignments' do
undated = (1..7).map {|i| create_assignments(@course.id, 1, title: "#{@course.id}:#{i}", due_at: nil).first }
dated = (1..18).map {|i| create_assignments(@course.id, 1, title: "#{@course.id}:#{i}", due_at: Time.parse('2012-01-20 12:00:00').advance(days: -i)).first }
ids = dated.reverse + undated
json = api_call(:get, "/api/v1/calendar_events?type=assignment&start_date=2012-01-08&end_date=2012-01-08&context_codes[]=course_#{@course.id}&per_page=10&page=3", {
json = api_call(:get, "/api/v1/calendar_events?type=assignment&all_events=1&context_codes[]=course_#{@course.id}&per_page=10", {
:controller => 'calendar_events_api', :action => 'index', :format => 'json', :type => 'assignment',
:context_codes => ["course_#{@course.id}"], :start_date => '2012-01-08', :end_date => '2012-01-08', :per_page => '10', :page => '3'})
json.size.should eql 5
:context_codes => ["course_#{@course.id}"], :all_events => 1, :per_page => '10'})
response.headers['Link'].should match(%r{<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=2.*>; rel="next",<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=1.*>; rel="first",<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=3.*>; rel="last"})
json.map{|a| a['id']}.should eql ids[0...10].map{|id| "assignment_#{id}"}
json = api_call(:get, "/api/v1/calendar_events?type=assignment&all_events=1&context_codes[]=course_#{@course.id}&per_page=10&page=2", {
:controller => 'calendar_events_api', :action => 'index', :format => 'json', :type => 'assignment',
:context_codes => ["course_#{@course.id}"], :all_events => 1, :per_page => '10', :page => '2'})
response.headers['Link'].should match(%r{<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=3.*>; rel="next",<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=1.*>; rel="prev",<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=1.*>; rel="first",<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=3.*>; rel="last"})
json.map{|a| a['id']}.should eql ids[10...20].map{|id| "assignment_#{id}"}
json = api_call(:get, "/api/v1/calendar_events?type=assignment&all_events=1&context_codes[]=course_#{@course.id}&per_page=10&page=3", {
:controller => 'calendar_events_api', :action => 'index', :format => 'json', :type => 'assignment',
:context_codes => ["course_#{@course.id}"], :all_events => 1, :per_page => '10', :page => '3'})
response.headers['Link'].should match(%r{<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=2.*>; rel="prev",<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=1.*>; rel="first",<http://www.example.com/api/v1/calendar_events.*type=assignment&.*page=3.*>; rel="last"})
json.map{|a| a['id']}.should eql ids[20...25].map{|id| "assignment_#{id}"}
end
it 'should ignore invalid end_dates' do