add override updates to update_assignment mutation

this updates the graphql scheme to allow assignment overrides
as input to the update_assignment mutation. the mutation just
converts the ids into internal ids and passes whatever it gets
to the normal backend process.

an important change here is the AssignmentUpdateProxy. please
read the comments in code.

Test-Plan:
- run unit tests because graphql is very unit test friendly
- ask carl to +1 qa it.. or, use graphiql to create/update/delete
  the overrides. starting examples can be found in the unit tests.

fixes: ADMIN-2301
Change-Id: I35d523b54346d71513ca08f448dcf479be0a8bb5
Reviewed-on: https://gerrit.instructure.com/176819
Tested-by: Jenkins
Reviewed-by: Carl Kibler <ckibler@instructure.com>
Reviewed-by: Mysti Sadler <mysti@instructure.com>
QA-Review: Mysti Sadler <mysti@instructure.com>
Product-Review: Mysti Sadler <mysti@instructure.com>
This commit is contained in:
Rex Fleischer 2019-01-04 00:52:35 -08:00
parent f510a4fddb
commit 08e9fbcca2
5 changed files with 443 additions and 27 deletions

View File

@ -16,8 +16,59 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
class Mutations::AssignmentOverrideCreateOrUpdate < GraphQL::Schema::InputObject
argument :id, ID, required: false
argument :due_at, Types::DateTimeType, required: false
argument :lock_at, Types::DateTimeType, required: false
argument :unlock_at, Types::DateTimeType, required: false
argument :section_id, ID, required: false
argument :group_id, ID, required: false
argument :student_ids, [ID], required: false
end
class Mutations::UpdateAssignment < Mutations::BaseMutation
include Api::V1::Assignment
# we are required to wrap the update method with a proxy class because
# we are required to include `Api` for instance methods within the module.
# the main problem is that including the `Api` module conflicts with the
# `Mutations::BaseMutation` class. so we have to segregate the two.
#
# probably a good idea to segregate anyways so we dont accidentally include
# processing we dont want.
class ApiProxy
include Api
include Api::V1::Assignment
def initialize(request, working_assignment, session, current_user)
@request = request
@working_assignment = working_assignment
@session = session
@current_user = current_user
end
attr_reader :session
def grading_periods?
@working_assignment.try(:grading_periods?)
end
def strong_anything
ArbitraryStrongishParams::ANYTHING
end
def value_to_boolean(value)
Canvas::Plugin.value_to_boolean(value)
end
def process_incoming_html_content(html)
Api::Html::Content.process_incoming(html)
end
def load_root_account
@domain_root_account = @request.env['canvas.domain_root_account'] || LoadAccount.default_domain_root_account
end
end
graphql_name "UpdateAssignment"
@ -27,6 +78,7 @@ class Mutations::UpdateAssignment < Mutations::BaseMutation
argument :state, Types::AssignmentType::AssignmentStateType, required: false
argument :due_at, Types::DateTimeType, required: false
argument :description, String, required: false
argument :assignment_overrides, [Mutations::AssignmentOverrideCreateOrUpdate], required: false
# the return data if the update is successful
field :assignment, Types::AssignmentType, null: true
@ -34,7 +86,6 @@ class Mutations::UpdateAssignment < Mutations::BaseMutation
def resolve(input:)
assignment_id = GraphQLHelpers.parse_relay_or_legacy_id(input[:id], "Assignment")
# make this a field so callbacks can reference
begin
@working_assignment = Assignment.find(assignment_id)
rescue ActiveRecord::RecordNotFound
@ -44,6 +95,8 @@ class Mutations::UpdateAssignment < Mutations::BaseMutation
# check permissions asap
raise GraphQL::ExecutionError, "insufficient permission" unless @working_assignment.grants_right? current_user, :update
update_proxy = ApiProxy.new(context[:request], @working_assignment, context[:session], current_user)
# to use the update_api_assignment method, we have to modify some of the
# input. first, update_api_assignment doesnt expect a :state key. instead,
# it expects a :published key of boolean type.
@ -51,7 +104,7 @@ class Mutations::UpdateAssignment < Mutations::BaseMutation
# need to handle those as well.
input_hash = input.to_h
other_update_on_assignment = false
if input_hash.has_key? :state
if input_hash.key? :state
asked_state = input_hash.delete :state
case asked_state
when "unpublished"
@ -67,13 +120,28 @@ class Mutations::UpdateAssignment < Mutations::BaseMutation
end
end
# prepare the overrides if there are any
if input_hash.key? :assignment_overrides
update_proxy.load_root_account
input_hash[:assignment_overrides].each do |override|
if override[:id].blank?
override.delete :id
else
override[:id] = GraphQLHelpers.parse_relay_or_legacy_id(override[:id], "AssignmentOverride")
end
override[:course_section_id] = GraphQLHelpers.parse_relay_or_legacy_id(override[:section_id], "Section") if override.key? :section_id
override[:group_id] = GraphQLHelpers.parse_relay_or_legacy_id(override[:group_id], "Group") if override.key? :group_id
override[:student_ids] = override[:student_ids].map { |id| GraphQLHelpers.parse_relay_or_legacy_id(id, "User") } if override.key? :student_ids
end
end
# make sure to do other required updates
self.send(other_update_on_assignment) if other_update_on_assignment
# normal update now
@working_assignment.content_being_saved_by(current_user)
@working_assignment.updating_user = current_user
result = update_api_assignment(@working_assignment, ActionController::Parameters.new(input_hash), current_user, @working_assignment.context)
result = update_proxy.update_api_assignment(@working_assignment, ActionController::Parameters.new(input_hash), current_user, @working_assignment.context)
# return the result
if [:ok, :created].include? result
@ -103,24 +171,4 @@ class Mutations::UpdateAssignment < Mutations::BaseMutation
@working_assignment.restore
end
#
# the methods below here are required callbacks from the update_api_assignment method.
#
def grading_periods?
@working_assignment.try(:grading_periods?)
end
def strong_anything
ArbitraryStrongishParams::ANYTHING
end
def value_to_boolean(value)
Canvas::Plugin.value_to_boolean(value)
end
def process_incoming_html_content(html)
Api::Html::Content.process_incoming(html)
end
end

View File

@ -505,7 +505,8 @@ module Api::V1::Assignment
end
response
rescue ActiveRecord::RecordInvalid
rescue ActiveRecord::RecordInvalid => e
assignment.errors.add('invalid_record', e)
false
rescue Lti::AssignmentSubscriptionsHelper::AssignmentSubscriptionError => e
assignment.errors.add('plagiarism_tool_subscription', e)

View File

@ -205,6 +205,16 @@ type AssignmentOverrideConnection {
pageInfo: PageInfo!
}
input AssignmentOverrideCreateOrUpdate {
dueAt: DateTime
groupId: ID
id: ID
lockAt: DateTime
sectionId: ID
studentIds: [ID!]
unlockAt: DateTime
}
# An edge in a connection.
type AssignmentOverrideEdge {
# A cursor for use in pagination.
@ -1044,6 +1054,7 @@ scalar URL
# Autogenerated input type of UpdateAssignment
input UpdateAssignmentInput {
assignmentOverrides: [AssignmentOverrideCreateOrUpdate!]
description: String
dueAt: DateTime
id: ID!

View File

@ -0,0 +1,356 @@
#
# Copyright (C) 2018 - present 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 "spec_helper"
require "helpers/graphql_type_tester"
require 'set'
describe Mutations::UpdateAssignment do
before(:once) do
@account = Account.create!
@course = @account.courses.create!
@course.require_assignment_group
@course.save!
@teacher = @course.enroll_teacher(User.create!, enrollment_state: 'active').user
@student = @course.enroll_student(User.create!, enrollment_state: 'active').user
@assignment_group = @course.assignment_groups.first
group_category = GroupCategory.create(:name => "Example Group Category", :context => @course)
@group = @course.groups.create!(:group_category => group_category)
@group.users << @student
@assignment_id = @assignment_group.assignments.create!(context: @course, name: "Example Assignment", group_category: group_category).id
end
def execute_with_input(update_input, user_executing=@teacher)
mutation_command = <<~GQL
mutation {
updateAssignment(input: {
#{update_input}
}) {
assignment {
_id
name
state
description
dueAt
assignmentOverrides {
nodes {
_id
title
allDay
dueAt
lockAt
unlockAt
set {
... on AdhocStudents {
students {_id}
}
... on Section {
_id
}
... on Group {
_id
}
}
}
}
}
errors {
attribute
message
}
}
}
GQL
context = {current_user: user_executing, request: ActionDispatch::TestRequest.create, session: {}}
CanvasSchema.execute(mutation_command, context: context)
end
#
# different assert helpers to make sure we are testing each override is
# completely correct every time. if we want to extend the checks for an
# individual override, please put it in here to make sure that every way
# that an override is mutated is checked.
#
def assert_adhoc_override(result_override, student_ids, assignment_id=@assignment_id)
expect(result_override["set"]["students"].map { |s| s["_id"].to_i }.to_set).to eq student_ids.to_set
override = Assignment.find(assignment_id).assignment_overrides.detect { |e| e.id.to_s == result_override["_id"] }
expect(override).to_not be_nil
override_set = override.set
expect(override_set.length).to eq student_ids.length
result_override["_id"]
end
def assert_section_override(result_override, section_id, assignment_id=@assignment_id)
expect(result_override["set"]["_id"]).to eq section_id.to_s
override = Assignment.find(assignment_id).assignment_overrides.detect { |e| e.id.to_s == result_override["_id"] }
expect(override).to_not be_nil
override_set = override.set
expect(override_set.class).to eq CourseSection
expect(override_set.id).to eq section_id
result_override["_id"]
end
def assert_group_override(result_override, group_id, assignment_id=@assignment_id)
expect(result_override["set"]["_id"]).to eq group_id.to_s
override = Assignment.find(assignment_id).assignment_overrides.detect { |e| e.id.to_s == result_override["_id"] }
expect(override).to_not be_nil
override_set = override.set
expect(override_set.class).to eq Group
expect(override_set.id).to eq group_id
result_override["_id"]
end
def assert_no_errors_and_get_overrides(result)
expect(result.dig('errors')).to be_nil
expect(result.dig('data', 'updateAssignment', 'errors')).to be_nil
expect(result.dig('data', 'updateAssignment', 'assignment', '_id')).to eq @assignment_id.to_s
result.dig('data', 'updateAssignment', 'assignment', 'assignmentOverrides', 'nodes')
end
#
# test start here
#
it "can create adhoc override" do
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
studentIds: ["#{@student.id}"]
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
expect(result_overrides.length).to eq 1
expect(Assignment.find(@assignment_id).active_assignment_overrides.length).to eq 1
assert_adhoc_override(result_overrides[0], [@student.id])
end
it "can create section override" do
section = @course.course_sections.create!
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
sectionId: "#{section.id}"
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
expect(result_overrides.length).to eq 1
expect(Assignment.find(@assignment_id).active_assignment_overrides.length).to eq 1
assert_section_override(result_overrides[0], section.id)
end
it "can create group override" do
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
groupId: "#{@group.id}"
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
expect(result_overrides.length).to eq 1
expect(Assignment.find(@assignment_id).active_assignment_overrides.length).to eq 1
assert_group_override(result_overrides[0], @group.id)
end
it "errors on no override info" do
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
}
]
GQL
expect(result.dig('errors')).to be_nil
expect(result.dig('data', 'updateAssignment', 'errors', 0, 'message')).to eq "one of student_ids, group_id, or course_section_id is required"
end
it "can create multiple overrides then remove one" do
section = @course.course_sections.create!
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
studentIds: ["#{@student.id}"]
}
{
sectionId: "#{section.id}"
}
{
groupId: "#{@group.id}"
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
expect(result_overrides.length).to eq 3
expect(Assignment.find(@assignment_id).active_assignment_overrides.length).to eq 3
# for whatever reason, the update get returned in reverse order. not going to fight it unless asked
group_override_id = assert_group_override(result_overrides[0], @group.id)
assert_section_override(result_overrides[1], section.id)
adhoc_override_id = assert_adhoc_override(result_overrides[2], [@student.id])
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
id: "#{adhoc_override_id}"
studentIds: ["#{@student.id}"]
}
{
id: "#{group_override_id}"
groupId: "#{@group.id}"
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
expect(result_overrides.length).to eq 2
expect(Assignment.find(@assignment_id).active_assignment_overrides.length).to eq 2
expect(result_overrides[0]["_id"]).to eq group_override_id
expect(result_overrides[1]["_id"]).to eq adhoc_override_id
assert_group_override(result_overrides[0], @group.id)
assert_adhoc_override(result_overrides[1], [@student.id])
end
it "can update an adhoc override students" do
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
studentIds: ["#{@student.id}"]
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
expect(result_overrides.length).to eq 1
expect(Assignment.find(@assignment_id).active_assignment_overrides.length).to eq 1
override_id = assert_adhoc_override(result_overrides[0], [@student.id])
@new_student_1 = @course.enroll_student(User.create!, enrollment_state: 'active').user
@new_student_2 = @course.enroll_student(User.create!, enrollment_state: 'active').user
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
id: "#{override_id}"
studentIds: ["#{@student.id}", "#{@new_student_1.id}", "#{@new_student_2.id}"]
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
expect(result_overrides.length).to eq 1
expect(result_overrides[0]["_id"]).to eq override_id
expect(Assignment.find(@assignment_id).active_assignment_overrides.length).to eq 1
expect(Assignment.find(@assignment_id).active_assignment_overrides[0].id).to eq override_id.to_i
assert_adhoc_override(result_overrides[0], [@student.id, @new_student_1.id, @new_student_2.id])
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
id: "#{override_id}"
studentIds: ["#{@student.id}", "#{@new_student_2.id}"]
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
expect(result_overrides.length).to eq 1
expect(result_overrides[0]["_id"]).to eq override_id
expect(Assignment.find(@assignment_id).active_assignment_overrides.length).to eq 1
expect(Assignment.find(@assignment_id).active_assignment_overrides[0].id).to eq override_id.to_i
assert_adhoc_override(result_overrides[0], [@student.id, @new_student_2.id])
end
def setup_field_update_test(field_testing, field_graphql)
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
studentIds: ["#{@student.id}"]
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
override_id = assert_adhoc_override(result_overrides[0], [@student.id])
override_check = Assignment.find(@assignment_id).active_assignment_overrides[0]
expect(override_check.send(field_testing)).to be_nil
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
id: "#{override_id}"
#{field_graphql}: "2018-01-01T01:00:00Z"
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
assert_adhoc_override(result_overrides[0], [@student.id])
override_check = Assignment.find(@assignment_id).active_assignment_overrides[0]
expect(override_check.send(field_testing)).to eq "2018-01-01T01:00:00Z"
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
id: "#{override_id}"
}
]
GQL
result_overrides = assert_no_errors_and_get_overrides(result)
assert_adhoc_override(result_overrides[0], [@student.id])
override_check = Assignment.find(@assignment_id).active_assignment_overrides[0]
expect(override_check.send(field_testing)).to be_nil
end
it "can update override dueAt" do
setup_field_update_test(:due_at, "dueAt")
end
it "can update override lockAt" do
setup_field_update_test(:lock_at, "lockAt")
end
it "can update override unlockAt" do
setup_field_update_test(:unlock_at, "unlockAt")
end
it "error shows when override set overlaps" do
section = @course.course_sections.create!
result = execute_with_input <<~GQL
id: "#{@assignment_id}"
assignmentOverrides: [
{
sectionId: "#{section.id}"
unlockAt: "2018-01-01T01:00:00Z"
}
{
sectionId: "#{section.id}"
unlockAt: "2018-01-02T01:00:00Z"
}
]
GQL
expect(result.dig('errors')).to be_nil
expect(result.dig('data', 'updateAssignment', 'errors', 0, 'message')).to eq "Validation failed: Set has already been taken"
end
end

View File

@ -48,8 +48,8 @@ describe Mutations::UpdateAssignment do
}
}
GQL
context = {current_user: user_executing, request: ActionDispatch::TestRequest.create}
return CanvasSchema.execute(mutation_command, context: context)
context = {current_user: user_executing, request: ActionDispatch::TestRequest.create, session: {}}
CanvasSchema.execute(mutation_command, context: context)
end
it "can do basic update on name" do