fix issues using timestamp as a pagination bookmark

when a timestamp is round-tripped through a page parameter,
it comes back as a string which was failing to line up with
the bookmarked row for one of two reasons:
 1. the date string was in local time (with a UTC offset
    that the database was ignoring)
 2. the date string truncated the fractional seconds part

solve both issues by explicitly converting timestamps to
UTC and preserving fractional seconds

test plan:
 - have two courses in different shards selected in your
   calendar, having assignments with due dates in both
   (at least 50 such assignments in one of the courses)
 - all the assignments should show up on your calendar
   (just once each)

fixes LS-2156

flag = none

Change-Id: Ia2010e08653520923e1c3f05d17697e1a3cd8826
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/263546
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Eric Saupe <eric.saupe@instructure.com>
QA-Review: Eric Saupe <eric.saupe@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Jeremy Stanley 2021-04-23 10:57:21 -06:00
parent c950b62d23
commit dfed7a0371
3 changed files with 65 additions and 1 deletions

View File

@ -76,7 +76,7 @@ class BookmarkedCollection::Collection < Array
attr_accessor :include_bookmark
def bookmark_to_page(bookmark)
bookmark && "bookmark:#{::JSONToken.encode(bookmark)}"
bookmark && "bookmark:#{::JSONToken.encode(format_dates(bookmark))}"
end
def page_to_bookmark(page)
@ -118,4 +118,15 @@ class BookmarkedCollection::Collection < Array
def has_more!
self.next_bookmark = bookmark_for(last)
end
def format_dates(bookmark)
# to_json will record local time + UTC offset with seconds truncated, both of which will screw up pagination
if bookmark.respond_to?(:iso8601)
bookmark.utc.iso8601(6)
elsif bookmark.is_a?(Array)
bookmark.map { |e| format_dates(e) }
else
bookmark
end
end
end

View File

@ -102,6 +102,24 @@ describe "BookmarkedCollection::Collection" do
end
end
describe "round-tripping dates" do
it "converts to UTC" do
timestamp = DateTime.parse('2020-12-31T22:00:00-09:00').in_time_zone('Alaska')
page = @collection.bookmark_to_page([1, timestamp])
expect(page).to match(/^bookmark:/)
bookmark = @collection.page_to_bookmark(page)
expect(bookmark).to eq([1, "2021-01-01T07:00:00.000000Z"])
end
it "preserves fractional times" do
timestamp = DateTime.parse('2020-02-22T22:22:22.22Z')
page = @collection.bookmark_to_page([1, [2, timestamp]])
expect(page).to match(/^bookmark:/)
bookmark = @collection.page_to_bookmark(page)
expect(bookmark).to eq([1, [2, "2020-02-22T22:22:22.220000Z"]])
end
end
describe "#current_page=" do
it "should set current_bookmark to nil if nil" do
@collection.current_bookmark = "some value"

View File

@ -2363,6 +2363,41 @@ describe CalendarEventsApiController, type: :request do
end
end
end
context "sharding" do
specs_require_sharding
before :once do
@c0 = @course
@a0 = @c0.assignments.create(workflow_state: 'published', due_at: 1.day.from_now)
@shard1.activate do
@shard1_account = Account.create!
@c1 = course_with_teacher(user: @me, account: @shard1_account, enrollment_state: 'active').course
@a1 = @c1.assignments.create(workflow_state: 'published', due_at: 2.days.from_now)
end
end
it "paginates assignments from multiple shards correctly" do
json = api_call(:get, "/api/v1/calendar_events?type=assignment&context_codes[]=course_#{@c0.id}&context_codes[]=course_#{@c1.id}&all_events=1&per_page=1",
controller: 'calendar_events_api', action: 'index', format: 'json', type: 'assignment',
context_codes: [@c0.asset_string, @c1.global_asset_string], all_events: 1, per_page: 1)
expect(json.size).to eq 1
expect(json[0]['id']).to eq @a0.asset_string
links = Api.parse_pagination_links(response.headers['Link'])
next_link = links.detect { |link| link[:rel] == 'next' }
expect(next_link).not_to be_nil
json = api_call(:get, next_link[:uri].to_s, controller: 'calendar_events_api', action: 'index',
format: 'json', type: 'assignment', context_codes: [@c0.asset_string, @c1.global_asset_string],
all_events: 1, per_page: 1, page: next_link['page'])
expect(json.size).to eq 1
expect(json[0]['id']).to eq @a1.asset_string
links = Api.parse_pagination_links(response.headers['Link'])
expect(links.detect { |link| link[:rel] == 'next' }).to be_nil
end
end
end
context "user index" do