Fix sis due date validations

Fixes: SIS-2869, SIS-2768

Notes: This commit is also for Mastery Paths.
       Repeat the first six steps for Mastery Paths.
       However, instead of a section override create
       a Mastery Path override. If you need help
       setting up Mastery Paths ping me.

Test plan:
- Enable the new SIS integration settings
- Check the `Require due date` setting
- Navigate to a course with more than one section
- Navigate to the assignments tab
- Create an assignment with one section override and
  add a due date
- Confirm the assignment saves
- Repeat the same steps with two section overrides
- Confirm the assignment saves

Change-Id: I4ce249f24e381fb14698a784b6cf80d4dc839c31
Reviewed-on: https://gerrit.instructure.com/115330
Reviewed-by: Brad Humphrey <brad@instructure.com>
Product-Review: Brad Humphrey <brad@instructure.com>
QA-Review: Mark McDermott <mmcdermott@instructure.com>
Tested-by: Jenkins
This commit is contained in:
Nick Houle 2017-06-13 14:28:25 -06:00
parent 711f54fc97
commit 7c7c827f8b
4 changed files with 77 additions and 8 deletions

View File

@ -75,7 +75,7 @@ class Assignment < ActiveRecord::Base
has_one :external_tool_tag, :class_name => 'ContentTag', :as => :context, :inverse_of => :context, :dependent => :destroy
validates_associated :external_tool_tag, :if => :external_tool?
validate :group_category_changes_ok?
validate :due_date_ok?, :unless => :has_active_assignment_overrides?
validate :due_date_ok?, :unless => :active_assignment_overrides?
validate :assignment_overrides_due_date_ok?
validate :discussion_group_ok?
validate :positive_points_possible?
@ -2518,6 +2518,15 @@ class Assignment < ActiveRecord::Base
end
end
def validate_overrides_for_sis(overrides)
unless AssignmentUtil.sis_integration_settings_enabled?(context) && AssignmentUtil.due_date_required_for_account?(context)
@skip_due_date_validation = true
return
end
raise ActiveRecord::RecordInvalid unless assignment_overrides_due_date_ok?(overrides)
@skip_due_date_validation = true
end
private
def due_date_ok?
@ -2526,16 +2535,19 @@ class Assignment < ActiveRecord::Base
end
end
def assignment_overrides_due_date_ok?
def assignment_overrides_due_date_ok?(overrides={})
if AssignmentUtil.due_date_required?(self)
if active_assignment_overrides.where(due_at: nil).count > 0
overrides = self.assignment_overrides.empty? ? overrides : self.assignment_overrides
if overrides.select{|o| o['due_at'].nil?}.length > 0
errors.add(:due_at, I18n.t("cannot be blank for any assignees when Post to Sis is checked"))
return false
end
return true
end
end
def has_active_assignment_overrides?
active_assignment_overrides.count > 0
def active_assignment_overrides?
@skip_due_date_validation || self.assignment_overrides.length > 0
end
def assignment_name_length_ok?

View File

@ -734,6 +734,7 @@ module Api::V1::Assignment
return :forbidden unless grading_periods_allow_assignment_overrides_batch_create?(assignment, overrides)
assignment.transaction do
assignment.validate_overrides_for_sis(overrides)
assignment.save_without_broadcasting!
batch_update_assignment_overrides(assignment, overrides, user)
end
@ -753,6 +754,7 @@ module Api::V1::Assignment
return :forbidden unless grading_periods_allow_assignment_overrides_batch_update?(assignment, prepared_batch)
assignment.transaction do
assignment.validate_overrides_for_sis(prepared_batch)
assignment.save_without_broadcasting!
perform_batch_update_assignment_overrides(assignment, prepared_batch)
end

View File

@ -1909,13 +1909,15 @@ describe AssignmentsApiController, type: :request do
end
context "sis validations enabled" do
it 'saves with a section with a valid due_date' do
before do
a = @course.account
a.enable_feature!(:new_sis_integrations)
a.settings[:sis_syncing] = {value: true}
a.settings[:sis_require_assignment_due_date] = {value: true}
a.save!
end
it 'saves with a section override with a valid due_date' do
assignment_params = {
'post_to_sis' => true,
'assignment_overrides' => {
@ -1928,8 +1930,29 @@ describe AssignmentsApiController, type: :request do
json = api_create_assignment_in_course(@course, assignment_params)
# expect(json["errors"]).to be_nil
expect(json["errors"]).to have_key('due_at')
expect(json["errors"]).to be_nil
end
it 'does not save with a section override without a due date' do
assignment_params = {
'post_to_sis' => true,
'assignment_overrides' => {
'0' => {
'course_section_id' => @course.default_section.id,
'due_at' => nil
}
}
}
json = api_create_assignment_in_course(@course, assignment_params)
expect(json["errors"]&.keys).to eq ['due_at']
end
it 'does not save without a due date' do
json = api_create_assignment_in_course(@course, 'post_to_sis' => true)
expect(json["errors"]&.keys).to eq ['due_at']
end
end
end

View File

@ -4343,6 +4343,38 @@ describe Assignment do
end
end
describe "validate_overrides_for_sis" do
def api_create_assignment_in_course(course,assignment_params)
api_call(:post,
"/api/v1/courses/#{course.id}/assignments.json",
{
:controller => 'assignments_api',
:action => 'create',
:format => 'json',
:course_id => course.id.to_s
}, {:assignment => assignment_params })
end
let(:assignment) do
@course.assignments.new(assignment_valid_attributes)
end
before do
assignment.post_to_sis = true
assignment.context.account.stubs(:sis_syncing).returns({value: true})
assignment.context.account.stubs(:feature_enabled?).with('new_sis_integrations').returns(true)
assignment.context.account.stubs(:sis_require_assignment_due_date).returns({value: true})
end
it "raises an invalid record error if overrides are invalid" do
overrides = [{
'course_section_id' => @course.default_section.id,
'due_at' => nil
}]
expect{assignment.validate_overrides_for_sis(overrides)}.to raise_error(ActiveRecord::RecordInvalid)
end
end
describe "max_name_length" do
let(:assignment) do
@course.assignments.new(assignment_valid_attributes)