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

properly handle selective external migration imports 

The selective imports mostly worked before, but if a
given section was completely unchecked the previous
code would import it anyway. This fixes that and makes all
the decision code in one place and easy to test.

Test Plan:
 * Import a content package and only choose certain items
 * Only the items you chose should be imported

closes #5528

Change-Id: Ia6c0c1f5f9a44497053733101140cc74d6239129
Reviewed-on: https://gerrit.instructure.com/9799
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Bracken Mosbacker 2012-04-02 16:38:05 -06:00
parent 8b7d1f77be
commit 961aad6d3b
20 changed files with 132 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/models/assignment.rb
View File

@ -1487,7 +1487,7 @@ class Assignment < ActiveRecord::Base
assignments = data['assignments'] ? data['assignments']: []
to_import = migration.to_import 'assignments'
assignments.each do |assign|
if assign['migration_id'] && (!to_import || to_import[assign['migration_id']])
if migration.import_object?("assignments", assign['migration_id'])
begin
import_from_migration(assign, migration.context)
rescue

3
app/models/assignment_group.rb
View File

@ -183,9 +183,8 @@ class AssignmentGroup < ActiveRecord::Base
def self.process_migration(data, migration)
groups = data['assignment_groups'] ? data['assignment_groups']: []
to_import = migration.to_import 'assignment_groups'
groups.each do |group|
if group['migration_id'] && (!to_import || to_import[group['migration_id']])
if migration.import_object?("assignment_groups", group['migration_id'])
begin
import_from_migration(group, migration.context)
rescue

3
app/models/attachment.rb
View File

@ -179,9 +179,8 @@ class Attachment < ActiveRecord::Base
def self.process_migration(data, migration)
attachments = data['file_map'] ? data['file_map']: {}
# TODO i18n
to_import = migration.to_import 'files'
attachments.values.each do |att|
if !att['is_folder'] && att['migration_id'] && (!to_import || to_import[att['migration_id']])
if !att['is_folder'] && migration.import_object?("files", att['migration_id'])
begin
import_from_migration(att, migration.context)
rescue

3
app/models/calendar_event.rb
View File

@ -419,9 +419,8 @@ class CalendarEvent < ActiveRecord::Base
def self.process_migration(data, migration)
events = data['calendar_events'] ? data['calendar_events']: []
to_import = migration.to_import 'events'
events.each do |event|
if event['migration_id'] && (!to_import || to_import[event['migration_id']])
if migration.import_object?("events", event['migration_id'])
begin
import_from_migration(event, migration.context)
rescue

20
app/models/content_migration.rb
View File

@ -222,7 +222,23 @@ class ContentMigration < ActiveRecord::Base
end
def to_import(val)
migration_settings[:migration_ids_to_import][:copy][val] rescue nil
migration_settings[:migration_ids_to_import] && migration_settings[:migration_ids_to_import][:copy] && migration_settings[:migration_ids_to_import][:copy][val]
end
def import_object?(asset_type, mig_id)
return false unless mig_id
return true unless migration_settings[:migration_ids_to_import] && migration_settings[:migration_ids_to_import][:copy] && migration_settings[:migration_ids_to_import][:copy].length > 0
return true if is_set?(to_import(:everything))
return true if is_set?(to_import("all_#{asset_type}"))
return false unless to_import(asset_type)
is_set?(to_import(asset_type)[mig_id])
end
def is_set?(option)
Canvas::Plugin::value_to_boolean option
end
def import_content
@ -295,6 +311,8 @@ class ContentMigration < ActiveRecord::Base
if ce.workflow_state == 'exported_for_course_copy'
# use the exported attachment as the import archive
self.attachment = ce.attachment
migration_settings[:migration_ids_to_import] ||= {:copy=>{}}
migration_settings[:migration_ids_to_import][:copy][:everything] = true
self.save
worker = Canvas::Migration::Worker::CCWorker.new
worker.migration_id = self.id

3
app/models/context_external_tool.rb
View File

@ -354,9 +354,8 @@ class ContextExternalTool < ActiveRecord::Base
def self.process_migration(data, migration)
tools = data['external_tools'] ? data['external_tools']: []
to_import = migration.to_import 'external_tools'
tools.each do |tool|
if tool['migration_id'] && (!to_import || to_import[tool['migration_id']])
if migration.import_object?("external_tools", tool['migration_id'])
item = import_from_migration(tool, migration.context)
if item.consumer_key == 'fake' || item.shared_secret == 'fake'
migration.add_warning(t('external_tool_attention_needed', 'The security parameters for the external tool "%{tool_name}" need to be set in Course Settings.', :tool_name => item.name))

3
app/models/context_module.rb
View File

@ -646,9 +646,8 @@ class ContextModule < ActiveRecord::Base
def self.process_migration(data, migration)
modules = data['modules'] ? data['modules'] : []
to_import = migration.to_import 'modules'
modules.each do |mod|
if mod['migration_id'] && (!to_import || to_import[mod['migration_id']])
if migration.import_object?("modules", mod['migration_id'])
begin
import_from_migration(mod, migration.context)
rescue

6
app/models/discussion_topic.rb
View File

@ -652,9 +652,8 @@ class DiscussionTopic < ActiveRecord::Base
def self.process_migration(data, migration)
announcements = data['announcements'] ? data['announcements']: []
to_import = migration.to_import 'announcements'
announcements.each do |event|
if event['migration_id'] && (!to_import || to_import[event['migration_id']])
if migration.import_object?("announcements", event['migration_id'])
event[:type] = 'announcement'
begin
import_from_migration(event, migration.context)
@ -665,13 +664,12 @@ class DiscussionTopic < ActiveRecord::Base
end
topics = data['discussion_topics'] ? data['discussion_topics']: []
topics_to_import = migration.to_import 'topics'
topic_entries_to_import = migration.to_import 'topic_entries'
topics.each do |topic|
context = Group.find_by_context_id_and_context_type_and_migration_id(migration.context.id, migration.context.class.to_s, topic['group_id']) if topic['group_id']
context ||= migration.context
if context
if topic['migration_id'] && (!topics_to_import || topics_to_import[topic['migration_id']])
if migration.import_object?("topics", topic['migration_id'])
begin
import_from_migration(topic.merge({:topic_entries_to_import => topic_entries_to_import}), context)
rescue

3
app/models/group.rb
View File

@ -377,9 +377,8 @@ class Group < ActiveRecord::Base
def self.process_migration(data, migration)
groups = data['groups'] ? data['groups']: []
to_import = migration.to_import 'groups'
groups.each do |group|
if group['migration_id'] && (!to_import || to_import[group['migration_id']])
if migration.import_object?("groups", group['migration_id'])
begin
import_from_migration(group, migration.context)
rescue

3
app/models/quiz.rb
View File

@ -1085,10 +1085,9 @@ class Quiz < ActiveRecord::Base
def self.process_migration(data, migration, question_data)
assessments = data['assessments'] ? data['assessments']['assessments'] : []
to_import = migration.to_import 'quizzes'
assessments.each do |assessment|
migration_id = assessment['migration_id'] || assessment['assessment_id']
if migration_id && (!to_import || to_import[migration_id])
if migration.import_object?("quizzes", migration_id)
allow_update = false
# allow update if we find an existing item based on this migration setting
if item_id = migration.migration_settings[:quiz_id_to_update]

3
app/models/rubric.rb
View File

@ -264,9 +264,8 @@ class Rubric < ActiveRecord::Base
def self.process_migration(data, migration)
rubrics = data['rubrics'] ? data['rubrics']: []
to_import = migration.to_import 'rubrics'
rubrics.each do |rubric|
if rubric['migration_id'] && (!to_import || to_import[rubric['migration_id']])
if migration.import_object?("rubrics", rubric['migration_id'])
begin
import_from_migration(rubric, migration.context)
rescue

1
app/models/wiki_page.rb
View File

@ -390,6 +390,7 @@ class WikiPage < ActiveRecord::Base
def self.process_migration(data, migration)
wikis = data['wikis'] ? data['wikis']: []
wikis.each do |wiki|
next unless migration.import_object?("wikis", wiki['migration_id'])
begin
import_from_migration(wiki, migration.context) if wiki
rescue

6
lib/cc/importer/cc_worker.rb
View File

@ -45,10 +45,8 @@ module Canvas::Migration
end
cm.migration_settings[:worker_class] = converter_class.name
if cm.migration_settings[:migration_ids_to_import] && cm.migration_settings[:migration_ids_to_import][:copy]
cm.migration_settings[:migration_ids_to_import][:copy][:assessment_questions] = true
else
cm.migration_settings[:migration_ids_to_import] = {:copy=>{:assessment_questions=>true}}
if !cm.migration_settings[:migration_ids_to_import] || !cm.migration_settings[:migration_ids_to_import][:copy]
cm.migration_settings[:migration_ids_to_import] = {:copy=>{:everything => true}}
end
cm.workflow_state = :exported
cm.progress = 0

BIN
spec/fixtures/migration/cc_ark_test.zip vendored Normal file
View File

Binary file not shown.

1
spec/lib/cc/importer/common_cartridge_converter_spec.rb
View File

@ -174,6 +174,7 @@ describe "More Standard Common Cartridge importing" do
@migration = Object.new
@migration.stubs(:to_import).returns(nil)
@migration.stubs(:context).returns(@copy_to)
@migration.stubs(:import_object?).returns(true)
end
it "should properly handle top-level resource references" do

1
spec/lib/importer/import_from_migration_spec.rb
View File

@ -47,6 +47,7 @@ describe Course do
:assignments => {'1865116014002' => true, '1865116155002' => true, '4407365899221' => true, '4469882339231' => true},
:outline_folders => {'1865116206002' => true, '1865116207002' => true},
:quizzes => {'1865116175002' => true},
:all_groups => true,
:shift_dates=>"1",
:old_start_date=>"Jan 23, 2009",
:old_end_date=>"Apr 10, 2009",

46
spec/models/content_migration_spec.rb
View File

@ -334,4 +334,50 @@ describe ContentMigration do
end
context "import_object?" do
before do
@cm = ContentMigration.new
end
it "should return true for everything if there are no copy options" do
@cm.import_object?("content_migrations", CC::CCHelper.create_key(@cm)).should == true
end
it "should return true for everything if 'everything' is selected" do
@cm.migration_ids_to_import = {:copy => {:everything => "1"}}
@cm.import_object?("content_migrations", CC::CCHelper.create_key(@cm)).should == true
end
it "should return true if there are no copy options" do
@cm.migration_ids_to_import = {:copy => {}}
@cm.import_object?("content_migrations", CC::CCHelper.create_key(@cm)).should == true
end
it "should return false for nil objects" do
@cm.import_object?("content_migrations", nil).should == false
end
it "should return true for all object types if the all_ option is true" do
@cm.migration_ids_to_import = {:copy => {:all_content_migrations => "1"}}
@cm.import_object?("content_migrations", CC::CCHelper.create_key(@cm)).should == true
end
it "should return false for objects not selected" do
@cm.save!
@cm.migration_ids_to_import = {:copy => {:all_content_migrations => "0"}}
@cm.import_object?("content_migrations", CC::CCHelper.create_key(@cm)).should == false
@cm.migration_ids_to_import = {:copy => {:content_migrations => {}}}
@cm.import_object?("content_migrations", CC::CCHelper.create_key(@cm)).should == false
@cm.migration_ids_to_import = {:copy => {:content_migrations => {CC::CCHelper.create_key(@cm) => "0"}}}
@cm.import_object?("content_migrations", CC::CCHelper.create_key(@cm)).should == false
end
it "should return true for selected objects" do
@cm.save!
@cm.migration_ids_to_import = {:copy => {:content_migrations => {CC::CCHelper.create_key(@cm) => "1"}}}
@cm.import_object?("content_migrations", CC::CCHelper.create_key(@cm)).should == true
end
end
end

3
spec/selenium/common.rb
View File

@ -824,7 +824,8 @@ end
"testfile5.zip" => "3dc43133-840a-46c8-ea17-3e4bef74af37",
"attachments.zip" => File.read(File.dirname(__FILE__) + "/../fixtures/attachments.zip"),
"graded.png" => File.read(File.dirname(__FILE__) + '/../../public/images/graded.png'),
"cc_full_test.zip" => File.read(File.dirname(__FILE__) + '/../fixtures/migration/cc_full_test.zip')
"cc_full_test.zip" => File.read(File.dirname(__FILE__) + '/../fixtures/migration/cc_full_test.zip'),
"cc_ark_test.zip" => File.read(File.dirname(__FILE__) + '/../fixtures/migration/cc_ark_test.zip')
}
def get_file(filename, data = nil)

64
spec/selenium/content_migrations_spec.rb
View File

@ -34,13 +34,13 @@ describe "external migrations" do
end
end
end
it "should import a common cartridge" do
def run_import(file)
login_as(@teacher.email, @password)
get "/courses/#{@course.id}/imports/migrate"
filename, fullpath, data = get_file("cc_full_test.zip")
filename, fullpath, data = get_file(file)
click_option('#choose_migration_system', 'Common Cartridge 1.0/1.1/1.2 Package')
driver.find_element(:css, '#config_options').find_element(:name, 'export_file').send_keys(fullpath)
@ -52,23 +52,15 @@ describe "external migrations" do
@migration.attachment.should_not be_nil
job = @migration.export_content
job.invoke_job
@migration.reload
@migration.workflow_state.should == 'exported'
get "/courses/#{@course.id}/imports/migrate/#{@migration.id}"
wait_for_ajaximations
keep_trying_until { find_with_jquery("#copy_everything").should be_displayed }
driver.find_element(:id, 'copy_everything').click
driver.find_element(:id, 'copy_all_quizzes').click if Qti.migration_executable
driver.find_element(:id, 'copy_folders_I_00001_R_').click
driver.find_element(:id, 'copy_folders_I_00006_Media_').click
driver.find_element(:id, 'copy_folders_I_media_R_').click
driver.find_element(:id, 'copy_modules_I_00000_').click
driver.find_element(:id, 'copy_topics_I_00009_R_').click
driver.find_element(:id, 'copy_topics_I_00006_R_').click
driver.find_element(:id, 'copy_external_tools_I_00010_R_').click
yield driver
expect_new_page_load {
driver.find_element(:id, 'copy_context_form').submit
@ -78,10 +70,24 @@ describe "external migrations" do
@migration.reload
@migration.import_content_without_send_later
}
driver.current_url.ends_with?("/courses/#{@course.id}").should == true
@course.reload
end
it "should import a common cartridge" do
run_import("cc_full_test.zip") do |driver|
driver.find_element(:id, 'copy_everything').click
driver.find_element(:id, 'copy_all_quizzes').click if Qti.migration_executable
driver.find_element(:id, 'copy_folders_I_00001_R_').click
driver.find_element(:id, 'copy_folders_I_00006_Media_').click
driver.find_element(:id, 'copy_folders_I_media_R_').click
driver.find_element(:id, 'copy_modules_I_00000_').click
driver.find_element(:id, 'copy_topics_I_00009_R_').click
driver.find_element(:id, 'copy_topics_I_00006_R_').click
driver.find_element(:id, 'copy_external_tools_I_00010_R_').click
end
@course.discussion_topics.count.should == 2
@course.quizzes.count.should == 1 if Qti.migration_executable
@course.attachments.count.should == 3
@ -90,4 +96,30 @@ describe "external migrations" do
@course.folders.count.should == 4
end
it "should selectively import a common cartridge" do
run_import("cc_ark_test.zip") do |driver|
driver.find_element(:id, 'copy_everything').click
driver.find_element(:id, 'copy_assignments_i183e1527a34b34e8151ffc6dec6cd140_').click
driver.find_element(:id, 'copy_quizzes_i2da11ea691f704f9b32ed3fa563af30e_').click
driver.find_element(:id, 'copy_files_i6d69d81475a73c4214327e7d4ad5630f_').click
driver.find_element(:id, 'copy_modules_i2410bad2b8623a94d9b662ced95406e0_').click
driver.find_element(:id, 'copy_topics_icdbdc4aab17bdd59c6b07f0336de1ce0_').click
driver.find_element(:id, 'copy_topics_ie28afa86290a7c5dfbe78453cc9b8d28_').click
driver.find_element(:id, 'copy_assignment_groups_i0928a83d992c891aa083bcffc1913b67_').click
driver.find_element(:id, 'copy_all_external_tools').click
end
# 2 because the announcement is returned for this too.
@course.discussion_topics.count.should == 2
@course.attachments.count.should == 1
@course.quizzes.count.should == 1 if Qti.migration_executable
@course.attachments.count.should == 1
@course.context_modules.count.should == 1
@course.wiki.wiki_pages.count.should == 0
@course.folders.count.should == 1
@course.context_external_tools.count.should == 2
end
end

2
vendor/plugins/qti_exporter/lib/workers/qti_worker.rb vendored
View File

@ -29,7 +29,7 @@ module Canvas::Migration
Canvas::Migration::Worker::clear_exported_data(export_folder_path)
end
cm.migration_settings[:migration_ids_to_import] = {:copy=>{:assessment_questions=>true}}.merge(cm.migration_settings[:migration_ids_to_import] || {})
cm.migration_settings[:migration_ids_to_import] = {:copy=>{:everything=>true}}.merge(cm.migration_settings[:migration_ids_to_import] || {})
if path = converter.course[:files_import_root_path]
cm.migration_settings[:files_import_root_path] = path
end