Skip to content

Commit 8ee71f6

Browse files
committed
cheap hack to skip overriding assignments
refs CNVS-6747 Test plan: * Make sure the new assignments index page still shows correct dates (test in a course that has assignments with and without overrides) * Test the assignments index api action and make sure it returns the right dates too Change-Id: Ibb6baaa5009fe95b68e160af2fb4e91d5ac8ef37 Reviewed-on: https://gerrit.instructure.com/22612 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Simon Williams <simon@instructure.com> QA-Review: Amber Taniuchi <amber@instructure.com> Product-Review: Cameron Matheson <cameron@instructure.com>
1 parent 920a6bf commit 8ee71f6

5 files changed

Lines changed: 81 additions & 25 deletions

File tree

app/controllers/assignment_groups_controller.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,16 @@ def index
8181

8282
params[:override_assignment_dates] ||= true
8383
override_dates = value_to_boolean(params[:override_assignment_dates])
84+
85+
if override_dates
86+
assignments_with_overrides = @context.assignments.active.except(:order)
87+
.joins(:assignment_overrides)
88+
.select("assignments.id")
89+
.uniq
90+
assignments_without_overrides = @groups.flat_map(&:active_assignments) -
91+
assignments_with_overrides
92+
assignments_without_overrides.each { |a| a.has_no_overrides = true }
93+
end
8494
end
8595

8696
respond_to do |format|

app/controllers/assignments_api_controller.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,14 @@ def index
326326
submissions = {}
327327
end
328328

329+
if override_dates
330+
assignments_with_overrides = @assignments.joins(:assignment_overrides)
331+
.select("assignments.id")
332+
@assignments = @assignments.all
333+
assignments_without_overrides = @assignments - assignments_with_overrides
334+
assignments_without_overrides.each { |a| a.has_no_overrides = true }
335+
end
336+
329337
hashes = @assignments.map do |assignment|
330338
submission = submissions[assignment.id]
331339
assignment_json(assignment, @current_user, session,

lib/assignment_override_applicator.rb

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ module AssignmentOverrideApplicator
2323
def self.assignment_overridden_for(assignment_or_quiz, user)
2424
return assignment_or_quiz if assignment_or_quiz.overridden_for?(user)
2525

26+
# this is a cheap hack to avoid unnecessary work (especially stupid
27+
# simply_versioned queries)
28+
if assignment_or_quiz.has_no_overrides
29+
return setup_overridden_clone(assignment_or_quiz)
30+
end
31+
2632
overrides = self.overrides_for_assignment_and_user(assignment_or_quiz, user)
2733

2834
result_assignment_or_quiz = self.assignment_with_overrides(assignment_or_quiz, overrides)
@@ -85,7 +91,7 @@ def self.overrides_for_assignment_and_user(assignment_or_quiz, user)
8591
# NOT exclude deleted overrides, yet
8692
key = assignment_or_quiz.is_a?(Quiz) ? :quiz_id : :assignment_id
8793
adhoc_membership = AssignmentOverrideStudent.where(key => assignment_or_quiz, :user_id => user).first
88-
94+
8995
overrides << adhoc_membership.assignment_override if adhoc_membership
9096

9197
if assignment_or_quiz.is_a?(Assignment) && assignment_or_quiz.group_category && group = user.current_groups.where(:group_category_id => assignment_or_quiz.group_category_id).first
@@ -143,35 +149,45 @@ def self.overrides_for_assignment_and_user(assignment_or_quiz, user)
143149
end
144150
end
145151

152+
# really takes an assignment or quiz but who wants to type out
153+
# assignment_or_quiz all the time?
154+
def self.setup_overridden_clone(assignment, overrides = [])
155+
clone = assignment.clone
156+
157+
# ActiveRecord::Base#clone nils out the primary key; put it back
158+
clone.id = assignment.id
159+
self.copy_preloaded_associations_to_clone(assignment,
160+
clone)
161+
yield(clone) if block_given?
162+
163+
clone.applied_overrides = overrides
164+
clone.without_overrides = assignment
165+
clone.overridden = true
166+
clone.readonly!
167+
168+
new_record = assignment.instance_variable_get(:@new_record)
169+
clone.instance_variable_set(:@new_record, new_record)
170+
171+
clone
172+
end
173+
146174
# apply the overrides calculated by collapsed_overrides to a clone of the
147175
# assignment or quiz which can then be used in place of the original object.
148176
# the clone is marked readonly to prevent saving
149177
def self.assignment_with_overrides(assignment_or_quiz, overrides)
150178
unoverridden_assignment_or_quiz = assignment_or_quiz.without_overrides
151-
# ActiveRecord::Base#clone nils out the primary key; put it back
152-
cloned_assignment_or_quiz = unoverridden_assignment_or_quiz.clone
153-
cloned_assignment_or_quiz.id = unoverridden_assignment_or_quiz.id
154-
self.copy_preloaded_associations_to_clone(assignment_or_quiz,
155-
cloned_assignment_or_quiz)
156-
157-
# update attributes with overrides
158-
if overrides
159-
self.collapsed_overrides(unoverridden_assignment_or_quiz, overrides).each do |field,value|
160-
# for any times in the value set, bring them back from raw UTC into the
161-
# current Time.zone before placing them in the assignment
162-
value = value.in_time_zone if value && value.respond_to?(:in_time_zone) && !value.is_a?(Date)
163-
cloned_assignment_or_quiz.write_attribute(field, value)
179+
180+
self.setup_overridden_clone(unoverridden_assignment_or_quiz,
181+
overrides) do |cloned_assignment_or_quiz|
182+
if overrides
183+
self.collapsed_overrides(unoverridden_assignment_or_quiz, overrides).each do |field,value|
184+
# for any times in the value set, bring them back from raw UTC into the
185+
# current Time.zone before placing them in the assignment
186+
value = value.in_time_zone if value && value.respond_to?(:in_time_zone) && !value.is_a?(Date)
187+
cloned_assignment_or_quiz.write_attribute(field, value)
188+
end
164189
end
165190
end
166-
cloned_assignment_or_quiz.applied_overrides = overrides
167-
cloned_assignment_or_quiz.without_overrides = unoverridden_assignment_or_quiz
168-
cloned_assignment_or_quiz.overridden = true
169-
cloned_assignment_or_quiz.readonly!
170-
171-
new_record = unoverridden_assignment_or_quiz.instance_variable_get(:@new_record)
172-
cloned_assignment_or_quiz.instance_variable_set(:@new_record, new_record)
173-
174-
cloned_assignment_or_quiz
175191
end
176192

177193
def self.copy_preloaded_associations_to_clone(orig, clone)

lib/dates_overridable.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
module DatesOverridable
2-
attr_accessor :applied_overrides, :overridden_for_user, :overridden
2+
attr_accessor :applied_overrides, :overridden_for_user, :overridden,
3+
:has_no_overrides
34
attr_writer :without_overrides
45

56
def self.included(base)

spec/lib/assignment_override_applicator_spec.rb

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,28 @@ def setup_overridden_assignments(section_due_at, assignment_due_at)
830830
@unoverridden_assignment.overridden_for_user.should == nil
831831
end
832832
end
833-
833+
834+
describe "Overridable#has_no_overrides" do
835+
before do
836+
student_in_course
837+
@assignment = assignment_model(:course => @course,
838+
:due_at => 1.week.from_now)
839+
o = assignment_override_model(:assignment => @assignment,
840+
:due_at => 1.week.ago)
841+
o.assignment_override_students.create! user: @student
842+
end
843+
844+
it "makes assignment_overridden_for lie!" do
845+
truly_overridden_assignment = AssignmentOverrideApplicator.assignment_overridden_for(@assignment, @student)
846+
847+
@assignment.has_no_overrides = true
848+
fake_overridden_assignment = AssignmentOverrideApplicator.assignment_overridden_for(@assignment, @student)
849+
fake_overridden_assignment.overridden.should be_true
850+
fake_overridden_assignment.due_at.should_not == truly_overridden_assignment.due_at
851+
fake_overridden_assignment.due_at.should == @assignment.due_at
852+
end
853+
end
854+
834855
it "should use the full stack" do
835856
student_in_course
836857
original_due_at = 3.days.from_now

0 commit comments

Comments
 (0)