Skip to content

Commit

Permalink
graphql: make assignment override instrumenter work
Browse files Browse the repository at this point in the history
closes RECNVS-308

Test plan:
  * set up assignments with overrides in a variety of situations
  * assignment dates should be overridden appropriately
  * assignment due dates are *not* overridden when enumerating
    assignmentOverrides

Change-Id: Iefec250bbcf095fb40cfb3b204f3521676e0d9f9
Reviewed-on: https://gerrit.instructure.com/142505
Tested-by: Jenkins
Reviewed-by: Keith T. Garner <[email protected]>
Reviewed-by: Jonathan Featherstone <[email protected]>
QA-Review: Collin Parrish <[email protected]>
Product-Review: Cameron Matheson <[email protected]>
  • Loading branch information
cmatheson committed Mar 6, 2018
1 parent 052c748 commit 5866d1e
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 8 deletions.
17 changes: 9 additions & 8 deletions app/graphql/assignment_override_instrumenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@ class AssignmentOverrideInstrumenter
# overriding assignments only when necessary (for example, when the
# due date is requested). Thoughts/potential issues/caveats:
#
# * the implementation of needs_overriding? needs to be done frd.
# There are many other factors that should be considered. At the
# very least: when lock_at/unlock_at are requested; if
# assignment#only_visible_to_overrides is true. Probably other
# things.
#
# * I love that this automatically applies to assignments no matter
# where they appear in the schema. I don't love that this is
# modifying the resolver of every item in the schema. The best
Expand Down Expand Up @@ -56,8 +50,15 @@ def instrument(type, field)
end
end

# TODO: make this real
ATTRIBUTES_NEED_OVERRIDING = %w[dueAt allDay dallDayDate unlockAt lockAt].freeze

# assignments are automatically overridden in graphql whenever one of the
# dueAt/lockAt/unlockAt fields are requested *UNLESS* the user is also
# requesting the list of assignment_overrides. The rationale is that the
# user won't have a way of seeing the assignment's default due date if it's
# overriden
def self.needs_overriding?(selections)
selections.key? "dueAt"
ATTRIBUTES_NEED_OVERRIDING.any? { |attr| selections.key?(attr) } &&
!selections.key?("assignmentOverrides")
end
end
45 changes: 45 additions & 0 deletions spec/graphql/assignment_override_instrumenter_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#
# Copyright (C) 2018 Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#

require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')

describe AssignmentOverrideInstrumenter do
context '#needs_overriding?' do
it "doesn't override when no overrideable attributes are requested" do
expect(
AssignmentOverrideInstrumenter.needs_overriding?("id" => true)
).to eq false
end

it "overrides when overrideable attributes are requested" do
%w[dueAt unlockAt lockAt].each { |overrideable_attribute|
expect(
AssignmentOverrideInstrumenter.needs_overriding?(overrideable_attribute => true)
).to eq true
}
end

it "doesn't override if assignment_overrides are requested" do
expect(
AssignmentOverrideInstrumenter.needs_overriding?(
"dueAt" => true, "assignmentOverrides" => true
)
).to eq false
end
end
end

0 comments on commit 5866d1e

Please sign in to comment.