ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

master courses: allow blueprint associations via sis imports 

test plan:
* with blueprint courses enabled,
* create a blueprint course with an sis id
* create a course by uploading a sis batch with a
courses.csv file, adding a `blueprint_course_id` column
set to the blueprint course's sis id
* it should add the sis imported course as an
 associated course

closes #CNVS-37739

Change-Id: I9b0bced1a3e1f672effc69c10e929e7ee4cd258f
Reviewed-on: https://gerrit.instructure.com/116987
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins
QA-Review: Heath Hales <hhales@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>
This commit is contained in:
James Williams 2017-06-26 13:33:28 -06:00
parent 1f85000b4b
commit ad05fdbf8b
6 changed files with 141 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
app/models/master_courses/master_migration.rb
View File

@ -37,12 +37,14 @@ class MasterCourses::MasterMigration < ActiveRecord::Base
state :imports_failed # if one or more of the imports failed
end
class MigrationRunningError < StandardError; end
# create a new migration and queue it up (if we can)
def self.start_new_migration!(master_template, user, opts = {})
master_template.class.transaction do
master_template.lock!
if master_template.active_migration_running?
raise "cannot start new migration while another one is running"
raise MigrationRunningError.new("cannot start new migration while another one is running")
else
new_migration = master_template.master_migrations.create!({:user => user}.merge(opts))
master_template.active_migration = new_migration

46
app/models/master_courses/master_template.rb
View File

@ -244,4 +244,50 @@ class MasterCourses::MasterTemplate < ActiveRecord::Base
self.default_restrictions
end
end
def self.create_associations_from_sis(root_account, associations, messages, migrating_user=nil)
associations.keys.each_slice(50) do |master_sis_ids|
templates = self.active.for_full_course.joins(:course).
where(:courses => {:root_account_id => root_account, :sis_source_id => master_sis_ids}).
select("#{self.table_name}.*, courses.sis_source_id AS sis_source_id").to_a
if templates.count != master_sis_ids.count
(master_sis_ids - templates.map(&:sis_source_id)).each do |missing_id|
messages << "Unknown blueprint course \"#{missing_id}\""
end
end
templates.each do |template|
needs_migration = false
associations[template.sis_source_id].each_slice(50) do |associated_sis_ids|
data = root_account.all_courses.where(:sis_source_id => associated_sis_ids).not_master_courses.
joins("LEFT OUTER JOIN #{MasterCourses::ChildSubscription.quoted_table_name} AS mcs ON mcs.child_course_id=courses.id AND mcs.workflow_state<>'deleted'").
pluck(:id, :sis_source_id, "mcs.master_template_id")
if data.count != associated_sis_ids
(associated_sis_ids - data.map{|id, sis_id, t_id| sis_id}).each do |invalid_id|
messages << "Cannot associate course \"#{invalid_id}\" - is a blueprint course"
end
end
data.each do |id, associated_sis_id, master_template_id|
if master_template_id
if master_template_id != template.id
messages << "Cannot associate course \"#{associated_sis_id}\" - is associated to another blueprint course"
end # otherwise we don't need to do anything - it's already associated
else
needs_migration = true
template.add_child_course!(id)
end
end
end
if migrating_user && needs_migration
begin
MasterCourses::MasterMigration.start_new_migration!(template, migrating_user)
rescue MasterCourses::MasterMigration::MigrationRunningError
# meh
end
end
end
end
end
end

10
doc/api/sis_csv.md
View File

@ -479,6 +479,16 @@ YYYY-MM-DDTHH:MM:SSZ</td>
<td></td>
<td>on_campus, online, blended</td>
</tr>
<tr>
<td>blueprint_course_id</td>
<td>text</td>
<td></td>
<td></td>
<td>The SIS id of a pre-existing Blueprint course. When provided,
the current course will be set up to receive updates from the blueprint course.
Requires Blueprint Courses feature.
</td>
</tr>
</table>
<p>If the start_date is set, it will override the term start date. If the end_date is set, it will

16
lib/sis/course_importer.rb
View File

@ -23,8 +23,9 @@ module SIS
start = Time.now
courses_to_update_sis_batch_id = []
course_ids_to_update_associations = [].to_set
blueprint_associations = {}
importer = Work.new(@batch, @root_account, @logger, courses_to_update_sis_batch_id, course_ids_to_update_associations, messages, @batch_user)
importer = Work.new(@batch, @root_account, @logger, courses_to_update_sis_batch_id, course_ids_to_update_associations, messages, @batch_user, blueprint_associations)
Course.suspend_callbacks(:update_enrollments_later) do
Course.process_as_sis(@sis_options) do
Course.skip_updating_account_associations do
@ -37,6 +38,9 @@ module SIS
courses_to_update_sis_batch_id.in_groups_of(1000, false) do |batch|
Course.where(:id => batch).update_all(:sis_batch_id => @batch.id)
end if @batch
MasterCourses::MasterTemplate.create_associations_from_sis(@root_account, blueprint_associations, messages, @batch_user)
@logger.debug("Courses took #{Time.now - start} seconds")
return importer.success_count
end
@ -46,18 +50,19 @@ module SIS
class Work
attr_accessor :success_count
def initialize(batch, root_account, logger, a1, a2, m, batch_user)
def initialize(batch, root_account, logger, a1, a2, m, batch_user, blueprint_associations)
@batch = batch
@batch_user = batch_user
@root_account = root_account
@courses_to_update_sis_batch_id = a1
@course_ids_to_update_associations = a2
@blueprint_associations = blueprint_associations
@messages = m
@logger = logger
@success_count = 0
end
def add_course(course_id, term_id, account_id, fallback_account_id, status, start_date, end_date, abstract_course_id, short_name, long_name, integration_id, course_format)
def add_course(course_id, term_id, account_id, fallback_account_id, status, start_date, end_date, abstract_course_id, short_name, long_name, integration_id, course_format, blueprint_course_id)
state_changes = []
@logger.debug("Processing Course #{[course_id, term_id, account_id, fallback_account_id, status, start_date, end_date, abstract_course_id, short_name, long_name].inspect}")
@ -206,6 +211,11 @@ module SIS
@courses_to_update_sis_batch_id << course.id
end
if blueprint_course_id && !course.deleted?
@blueprint_associations[blueprint_course_id] ||= []
@blueprint_associations[blueprint_course_id] << course_id
end
course.update_enrolled_users if update_enrollments
@success_count += 1
end

3
lib/sis/csv/course_importer.rb
View File

@ -47,7 +47,8 @@ module SIS
end
begin
importer.add_course(row['course_id'], row['term_id'], row['account_id'], row['fallback_account_id'], row['status'], start_date, end_date, row['abstract_course_id'], row['short_name'], row['long_name'], row['integration_id'], row['course_format'])
importer.add_course(row['course_id'], row['term_id'], row['account_id'], row['fallback_account_id'], row['status'], start_date, end_date,
row['abstract_course_id'], row['short_name'], row['long_name'], row['integration_id'], row['course_format'], row['blueprint_course_id'])
rescue ImportError => e
messages << "#{e}"
end

67
spec/lib/sis/csv/course_importer_spec.rb
View File

@ -570,4 +570,71 @@ describe SIS::CSV::CourseImporter do
)
expect(importer.warnings.map(&:last)).to include "Invalid course_format \"FAT32\" for course test_1"
end
context "blueprint courses" do
before :once do
account_model
@mc = @account.courses.create!(:sis_source_id => "blahprint")
@template = MasterCourses::MasterTemplate.set_as_master_course(@mc)
end
it "should give a warning when trying to associate an existing blueprint course" do
mc2 = @account.courses.create!(:sis_source_id => "anothermastercourse")
template2 = MasterCourses::MasterTemplate.set_as_master_course(mc2)
importer = process_csv_data(
"course_id,short_name,long_name,status,blueprint_course_id",
"#{mc2.sis_source_id},shortname,long name,active,#{@mc.sis_source_id}"
)
expect(importer.warnings.map(&:last)).to include("Cannot associate course \"#{mc2.sis_source_id}\" - is a blueprint course")
end
it "should give a warning when trying to associate an already associated course" do
mc2 = @account.courses.create!(:sis_source_id => "anothermastercourse")
template2 = MasterCourses::MasterTemplate.set_as_master_course(mc2)
ac = @account.courses.create!(:sis_source_id => "anassociatedcourse")
template2.add_child_course!(ac)
importer = process_csv_data(
"course_id,short_name,long_name,status,blueprint_course_id",
"#{ac.sis_source_id},shortname,long name,active,#{@mc.sis_source_id}"
)
expect(importer.warnings.map(&:last)).to include("Cannot associate course \"#{ac.sis_source_id}\" - is associated to another blueprint course")
end
it "shouldn't fail if a course is already associated to the target" do
ac = @account.courses.create!(:sis_source_id => "anassociatedcourse")
@template.add_child_course!(ac)
process_csv_data_cleanly(
"course_id,short_name,long_name,status,blueprint_course_id",
"#{ac.sis_source_id},shortname,long name,active,#{@mc.sis_source_id}"
)
end
it "should be able to associate courses in bulk" do
c1 = @account.courses.create!(:sis_source_id => "acourse1")
c2 = @account.courses.create!(:sis_source_id => "acourse2")
mc2 = @account.courses.create!(:sis_source_id => "anothermastercourse")
template2 = MasterCourses::MasterTemplate.set_as_master_course(mc2)
c3 = @account.courses.create!(:sis_source_id => "acourse3")
process_csv_data_cleanly(
"course_id,short_name,long_name,status,blueprint_course_id",
"#{c1.sis_source_id},shortname,long name,active,#{@mc.sis_source_id}",
"#{c2.sis_source_id},shortname,long name,active,#{@mc.sis_source_id}",
"#{c3.sis_source_id},shortname,long name,active,#{mc2.sis_source_id}"
)
expect(@template.child_subscriptions.active.pluck(:child_course_id)).to match_array([c1.id, c2.id])
expect(template2.child_subscriptions.active.pluck(:child_course_id)).to eq([c3.id])
end
it "should try to queue a migration afterwards" do
account_admin_user(:active_all => true)
c1 = @account.courses.create!(:sis_source_id => "acourse1")
process_csv_data_cleanly(
"course_id,short_name,long_name,status,blueprint_course_id",
"#{c1.sis_source_id},shortname,long name,active,#{@mc.sis_source_id}",
:batch => @account.sis_batches.create!(:user => @admin, :data => {})
)
mm = @template.master_migrations.last
expect(mm).to be_queued
end
end
end