From 6e82029a1633aced2d25e1d9d3ea613f22094057 Mon Sep 17 00:00:00 2001 From: Rob Orton Date: Tue, 24 Sep 2019 16:03:10 -0600 Subject: [PATCH] add disabled to grade_passback_setting closes CORE-3356 flag: none test plan - specs should pass Change-Id: If1cc3258aceb5036a70c5426299838dca9729e99 Reviewed-on: https://gerrit.instructure.com/210870 Tested-by: Jenkins Reviewed-by: James Williams QA-Review: James Williams Product-Review: James Williams --- app/controllers/courses_controller.rb | 5 +++-- lib/sis/course_importer.rb | 3 ++- spec/apis/v1/courses_api_spec.rb | 11 +++++++++++ spec/lib/sis/csv/course_importer_spec.rb | 22 ++++++++++++---------- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index b924d99709b..5f363362d39 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -729,7 +729,7 @@ class CoursesController < ApplicationController # The grading standard id to set for the course. If no value is provided for this argument the current grading_standard will be un-set from this course. # # @argument course[grade_passback_setting] [String] - # Optional. The grade_passback_setting for the course. Only 'nightly_sync' and '' are allowed + # Optional. The grade_passback_setting for the course. Only 'nightly_sync', 'disabled', and '' are allowed # # @argument course[course_format] [String] # Optional. Specifies the format of the course. (Should be 'on_campus', 'online', or 'blended') @@ -3260,7 +3260,8 @@ class CoursesController < ApplicationController private def update_grade_passback_setting(grade_passback_setting) - unless grade_passback_setting.blank? || grade_passback_setting == 'nightly_sync' + valid_states = Setting.get('valid_grade_passback_settings', 'nightly_sync,disabled').split(',') + unless grade_passback_setting.blank? || valid_states.include?(grade_passback_setting) @course.errors.add(:grade_passback_setting, t("Invalid grade_passback_setting")) end @course.grade_passback_setting = grade_passback_setting.presence diff --git a/lib/sis/course_importer.rb b/lib/sis/course_importer.rb index 63c069d33a7..12ddc320208 100644 --- a/lib/sis/course_importer.rb +++ b/lib/sis/course_importer.rb @@ -71,7 +71,8 @@ module SIS raise ImportError, "No long_name given for course #{course_id}" if long_name.blank? && abstract_course_id.blank? raise ImportError, "Improper status \"#{status}\" for course #{course_id}" unless status =~ /\A(active|deleted|completed|unpublished)/i raise ImportError, "Invalid course_format \"#{course_format}\" for course #{course_id}" unless course_format.blank? || course_format =~ /\A(online|on_campus|blended|not_set)/i - raise ImportError, "Invalid grade_passback_setting \"#{grade_passback_setting}\" for course #{course_id}" unless grade_passback_setting.blank? || grade_passback_setting =~ /\A(nightly_sync|not_set)/i + valid_grade_passback_settings = (Setting.get('valid_grade_passback_settings', 'nightly_sync,disabled').split(',') << 'not_set') + raise ImportError, "Invalid grade_passback_setting \"#{grade_passback_setting}\" for course #{course_id}" unless grade_passback_setting.blank? || valid_grade_passback_settings.include?(grade_passback_setting.downcase.strip) return if @batch.skip_deletes? && status =~ /deleted/i course = @root_account.all_courses.where(sis_source_id: course_id).take diff --git a/spec/apis/v1/courses_api_spec.rb b/spec/apis/v1/courses_api_spec.rb index 3f57a0186dd..be86d374a96 100644 --- a/spec/apis/v1/courses_api_spec.rb +++ b/spec/apis/v1/courses_api_spec.rb @@ -1177,6 +1177,17 @@ describe CoursesController, type: :request do expect(@course.reload.grade_passback_setting).to eq 'nightly_sync' end + it "should update the grade_passback_setting to disabled" do + api_call(:put, @path, @params, course: { grade_passback_setting: 'disabled' }) + expect(@course.reload.grade_passback_setting).to eq 'disabled' + end + + it "should update the grade_passback_setting to custom setting" do + Setting.set('valid_grade_passback_settings', 'one,two,three') + api_call(:put, @path, @params, course: { grade_passback_setting: 'one' }) + expect(@course.reload.grade_passback_setting).to eq 'one' + end + it "should remove the grade_passback_setting" do @course.update_attribute(:grade_passback_setting, 'nightly_sync') api_call(:put, @path, @params, course: { grade_passback_setting: '' }) diff --git a/spec/lib/sis/csv/course_importer_spec.rb b/spec/lib/sis/csv/course_importer_spec.rb index 9c692138e23..42d73dbabf4 100644 --- a/spec/lib/sis/csv/course_importer_spec.rb +++ b/spec/lib/sis/csv/course_importer_spec.rb @@ -763,14 +763,16 @@ describe SIS::CSV::CourseImporter do end it 'sets and updates grade_passback_setting' do + Setting.set('valid_grade_passback_settings', 'disabled,nightly_sync,other') process_csv_data_cleanly( "course_id,short_name,long_name,account_id,term_id,status,grade_passback_setting", - "test_1,TC 101,Test Course 101,,,active,nightly_sync", - "test_2,TC 102,Test Course 102,,,active,nightly_sync", + "test_1,TC 101,Test Course 101,,,active,disabled", + "test_2,TC 102,Test Course 102,,,active,other", "test_3,TC 103,Test Course 103,,,active,nightly_sync" ) - expect(Course.find_by_sis_source_id('test_1').grade_passback_setting).to eq 'nightly_sync' - expect(Course.find_by_sis_source_id('test_2').grade_passback_setting).to eq 'nightly_sync' + expect(Course.where(sis_source_id: 'test_1').take.grade_passback_setting).to eq 'disabled' + expect(Course.where(sis_source_id: 'test_2').take.grade_passback_setting).to eq 'other' + expect(Course.where(sis_source_id: 'test_3').take.grade_passback_setting).to eq 'nightly_sync' process_csv_data_cleanly( "course_id,short_name,long_name,account_id,term_id,status,grade_passback_setting", @@ -778,15 +780,15 @@ describe SIS::CSV::CourseImporter do "test_2,TC 102,Test Course 102,,,active,\"\"", "test_3,TC 103,Test Course 103,,,active,nightly_sync" ) - expect(Course.find_by_sis_source_id('test_1').grade_passback_setting).to be_nil - expect(Course.find_by_sis_source_id('test_2').grade_passback_setting).to be_nil - expect(Course.find_by_sis_source_id('test_3').grade_passback_setting).to eq 'nightly_sync' + expect(Course.where(sis_source_id: 'test_1').take.grade_passback_setting).to be_nil + expect(Course.where(sis_source_id: 'test_2').take.grade_passback_setting).to be_nil + expect(Course.where(sis_source_id: 'test_3').take.grade_passback_setting).to eq 'nightly_sync' process_csv_data_cleanly( "course_id,short_name,long_name,account_id,term_id,status", "test_3,TC 103,Test Course 103,,,active" ) - expect(Course.find_by_sis_source_id('test_3').grade_passback_setting).to eq 'nightly_sync' + expect(Course.where(sis_source_id: 'test_3').take.grade_passback_setting).to eq 'nightly_sync' end it 'respects stuck grade_passback setting' do @@ -794,7 +796,7 @@ describe SIS::CSV::CourseImporter do "course_id,short_name,long_name,account_id,term_id,status,grade_passback_setting", "test_1,TC 101,Test Course 101,,,active,nightly_sync" ) - expect((course = Course.find_by_sis_source_id('test_1')).grade_passback_setting).to eq 'nightly_sync' + expect((course = Course.where(sis_source_id: 'test_1').take).grade_passback_setting).to eq 'nightly_sync' course.grade_passback_setting=nil course.save! @@ -802,7 +804,7 @@ describe SIS::CSV::CourseImporter do "course_id,short_name,long_name,account_id,term_id,status,grade_passback_setting", "test_1,TC 101,Test Course 101,,,active,nightly_sync" ) - expect(Course.find_by_sis_source_id('test_1').grade_passback_setting).to be_nil + expect(Course.where(sis_source_id: 'test_1').take.grade_passback_setting).to be_nil end end end