From 1885839ec25a917762e1f1714345913c5ddf230c Mon Sep 17 00:00:00 2001 From: James Williams Date: Wed, 5 Dec 2018 07:35:49 -0700 Subject: [PATCH] don't truncate the seconds off of cached due dates but still use the truncated one to compare to grading periods closes #COMMS-1796 Change-Id: Ibcbc160f374cfc32010799c67b58589c39e43b96 Reviewed-on: https://gerrit.instructure.com/174278 Tested-by: Jenkins Reviewed-by: Steven Burnett QA-Review: Steven Burnett Product-Review: Steven Burnett --- lib/effective_due_dates.rb | 14 ++++--- spec/lib/due_date_cacher_spec.rb | 42 ++++++++++----------- spec/models/quizzes/quiz_submission_spec.rb | 2 +- spec/models/submission_spec.rb | 9 ++++- 4 files changed, 39 insertions(+), 28 deletions(-) diff --git a/lib/effective_due_dates.rb b/lib/effective_due_dates.rb index c4aba576732..743f3429c36 100644 --- a/lib/effective_due_dates.rb +++ b/lib/effective_due_dates.rb @@ -216,7 +216,8 @@ class EffectiveDueDates os.user_id AS student_id, o.assignment_id, o.id AS override_id, - date_trunc('minute', o.due_at) AS due_at, + date_trunc('minute', o.due_at) AS trunc_due_at, + o.due_at, o.set_type AS override_type, o.due_at_overridden, 1 AS priority @@ -235,7 +236,8 @@ class EffectiveDueDates gm.user_id AS student_id, o.assignment_id, o.id AS override_id, - date_trunc('minute', o.due_at) AS due_at, + date_trunc('minute', o.due_at) AS trunc_due_at, + o.due_at, o.set_type AS override_type, o.due_at_overridden, 1 AS priority @@ -256,7 +258,8 @@ class EffectiveDueDates e.user_id AS student_id, o.assignment_id, o.id AS override_id, - date_trunc('minute', o.due_at) AS due_at, + date_trunc('minute', o.due_at) AS trunc_due_at, + o.due_at, o.set_type AS override_type, o.due_at_overridden, 1 AS priority @@ -279,7 +282,8 @@ class EffectiveDueDates e.user_id AS student_id, a.id as assignment_id, NULL::integer AS override_id, - date_trunc('minute', a.due_at) AS due_at, + date_trunc('minute', a.due_at) AS trunc_due_at, + a.due_at, 'Everyone Else'::varchar AS override_type, FALSE AS due_at_overridden, 2 AS priority @@ -385,7 +389,7 @@ class EffectiveDueDates FROM calculated_overrides overrides -- match the effective due date with its grading period LEFT OUTER JOIN applied_grading_periods periods ON - periods.start_date < overrides.due_at AND overrides.due_at <= periods.end_date + periods.start_date < overrides.trunc_due_at AND overrides.trunc_due_at <= periods.end_date SQL end end diff --git a/spec/lib/due_date_cacher_spec.rb b/spec/lib/due_date_cacher_spec.rb index f543194f6e5..d4d2ce2972e 100644 --- a/spec/lib/due_date_cacher_spec.rb +++ b/spec/lib/due_date_cacher_spec.rb @@ -502,7 +502,7 @@ describe DueDateCacher do @assignment.save! cacher.recompute - expect(submission.reload.cached_due_date).to eq @assignment.due_at.change(sec: 0) + expect(submission.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0) end it "should set the cached_due_date to nil if the assignment has no due_at" do @@ -537,7 +537,7 @@ describe DueDateCacher do @override.save! cacher.recompute - expect(submission.reload.cached_due_date).to eq @override.due_at.change(sec: 0) + expect(submission.reload.cached_due_date).to eq @override.due_at.change(usec: 0) end it "should prefer override's due_at over assignment's nil" do @@ -548,7 +548,7 @@ describe DueDateCacher do @assignment.save! cacher.recompute - expect(submission.reload.cached_due_date).to eq @override.due_at.change(sec: 0) + expect(submission.reload.cached_due_date).to eq @override.due_at.change(usec: 0) end it "should prefer override's nil over assignment's due_at" do @@ -564,7 +564,7 @@ describe DueDateCacher do @override.save! cacher.recompute - expect(submission.reload.cached_due_date).to eq @assignment.due_at.change(sec: 0) + expect(submission.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0) end it "does not update submissions for students with concluded enrollments" do @@ -597,12 +597,12 @@ describe DueDateCacher do it "should apply to students in the adhoc set" do cacher.recompute - expect(@submission2.reload.cached_due_date).to eq @override.due_at.change(sec: 0) + expect(@submission2.reload.cached_due_date).to eq @override.due_at.change(usec: 0) end it "should not apply to students not in the adhoc set" do cacher.recompute - expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(sec: 0) + expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0) end it "does not update submissions for students with concluded enrollments" do @@ -633,11 +633,11 @@ describe DueDateCacher do end it "should apply to students in that section" do - expect(@submission2.reload.cached_due_date).to eq @override.due_at.change(sec: 0) + expect(@submission2.reload.cached_due_date).to eq @override.due_at.change(usec: 0) end it "should not apply to students in other sections" do - expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(sec: 0) + expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0) end it "should not apply to non-active enrollments in that section" do @@ -645,7 +645,7 @@ describe DueDateCacher do :enrollment_state => 'deleted', :section => @course_section, :allow_multiple_enrollments => true) - expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(sec: 0) + expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0) end end @@ -676,18 +676,18 @@ describe DueDateCacher do it "should apply to students in that group" do cacher.recompute - expect(@submission2.reload.cached_due_date).to eq @override.due_at.change(sec: 0) + expect(@submission2.reload.cached_due_date).to eq @override.due_at.change(usec: 0) end it "should not apply to students not in the group" do cacher.recompute - expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(sec: 0) + expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0) end it "should not apply to non-active memberships in that group" do cacher.recompute @group.add_user(@student1, 'deleted') - expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(sec: 0) + expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0) end it "does not update submissions for students with concluded enrollments" do @@ -718,7 +718,7 @@ describe DueDateCacher do @override1.save! cacher.recompute - expect(submission.reload.cached_due_date).to eq @override1.due_at.change(sec: 0) + expect(submission.reload.cached_due_date).to eq @override1.due_at.change(usec: 0) end it "should prefer second override's due_at if latest" do @@ -726,7 +726,7 @@ describe DueDateCacher do @override2.save! cacher.recompute - expect(submission.reload.cached_due_date).to eq @override2.due_at.change(sec: 0) + expect(submission.reload.cached_due_date).to eq @override2.due_at.change(usec: 0) end it "should be nil if first override's nil" do @@ -778,7 +778,7 @@ describe DueDateCacher do @override1.save! cacher.recompute - expect(@submission1.reload.cached_due_date).to eq @override1.due_at.change(sec: 0) + expect(@submission1.reload.cached_due_date).to eq @override1.due_at.change(usec: 0) end it "should use second override where the first doesn't apply" do @@ -786,7 +786,7 @@ describe DueDateCacher do @override2.save! cacher.recompute - expect(@submission2.reload.cached_due_date).to eq @override2.due_at.change(sec: 0) + expect(@submission2.reload.cached_due_date).to eq @override2.due_at.change(usec: 0) end it "should use the best override where both apply" do @@ -794,7 +794,7 @@ describe DueDateCacher do @override1.save! cacher.recompute - expect(@submission2.reload.cached_due_date).to eq @override2.due_at.change(sec: 0) + expect(@submission2.reload.cached_due_date).to eq @override2.due_at.change(usec: 0) end end @@ -816,11 +816,11 @@ describe DueDateCacher do end it "should apply to submission on the overridden assignment" do - expect(@submission1.reload.cached_due_date).to eq @override.due_at.change(sec: 0) + expect(@submission1.reload.cached_due_date).to eq @override.due_at.change(usec: 0) end it "should not apply to apply to submission on the other assignment" do - expect(@submission2.reload.cached_due_date).to eq @assignment.due_at.change(sec: 0) + expect(@submission2.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0) end end @@ -910,8 +910,8 @@ describe DueDateCacher do let(:due_at) { Time.zone.now + 1.day } # Remove seconds, following the lead of EffectiveDueDates - let(:original_due_at_formatted) { original_due_at.change(sec: 0).iso8601 } - let(:due_at_formatted) { due_at.change(sec: 0).iso8601 } + let(:original_due_at_formatted) { original_due_at.change(usec: 0).iso8601 } + let(:due_at_formatted) { due_at.change(usec: 0).iso8601 } let(:event_type) { 'submission_updated' } diff --git a/spec/models/quizzes/quiz_submission_spec.rb b/spec/models/quizzes/quiz_submission_spec.rb index 7f433c3fc7a..54045d3ce34 100644 --- a/spec/models/quizzes/quiz_submission_spec.rb +++ b/spec/models/quizzes/quiz_submission_spec.rb @@ -1624,7 +1624,7 @@ describe Quizzes::QuizSubmission do finished_at: Time.zone.now, user: @user, quiz: quiz, workflow_state: :complete ) - expected_seconds_late = (Time.zone.now - 60.seconds - 5.minutes.ago.change(sec: 0)).to_i + expected_seconds_late = (Time.zone.now - 60.seconds - 5.minutes.ago.change(usec: 0)).to_i expect(qs.submission.seconds_late).to eql(expected_seconds_late) end end diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index 6986f799153..1fd74fe3251 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -280,7 +280,14 @@ describe Submission do override.save! submission = @assignment.submissions.find_by!(user: @user) - expect(submission.cached_due_date).to eq override.reload.due_at.change(sec: 0) + expect(submission.cached_due_date).to eq override.reload.due_at.change(usec: 0) + end + + it "should not truncate seconds off of cached due dates" do + time = DateTime.parse("2018-12-24 23:59:59") + @assignment.update_attribute(:due_at, time) + submission = @assignment.submissions.find_by!(user: @user) + expect(submission.cached_due_date.to_i).to eq time.to_i end context 'due date changes after student submits' do