allow zip file import migrations on users and groups

test plan:
 - use the List Migration Systems endpoint on
   a group and a user, and confirm only the
   zip_file_importer is returned
 - use the Content Migrations API to import a zip file
   into folders belonging to Users and Groups
   - try both the POST and the URL workflows
   - ensure migration types other than zip_file_importer
     are rejected in User and Group context

fixes CNVS-11218

Change-Id: I4b407ce76f0b5df60cc9f99795a6d9d3ddf81ae2
Reviewed-on: https://gerrit.instructure.com/33723
QA-Review: Clare Strong <clare@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: James Williams  <jamesw@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
Product-Review: Bracken Mosbacker <bracken@instructure.com>
This commit is contained in:
Jeremy Stanley 2014-04-22 11:55:08 -06:00
parent ec8dc289f9
commit f84003358f
8 changed files with 173 additions and 19 deletions

View File

@ -190,6 +190,13 @@ class ContentMigrationsController < ApplicationController
render :json => content_migration_json(@content_migration, @current_user, session)
end
def migration_plugin_supported?(plugin)
# FIXME most migration types don't support Account either, but plugins that do would have to set additional_contexts
# in order to not be broken by this
@context.is_a?(Course) || @context.is_a?(Account) || Array(plugin.settings[:additional_contexts]).include?(@context.class.to_s)
end
private :migration_plugin_supported?
# @API Create a content migration
#
# Create a content migration. If the migration requires a file to be uploaded
@ -302,6 +309,10 @@ class ContentMigrationsController < ApplicationController
if !@plugin
return render(:json => { :message => t('bad_migration_type', "Invalid migration_type") }, :status => :bad_request)
end
unless migration_plugin_supported?(@plugin)
return render(:json => { :message => t('unsupported_migration_type', "Unsupported migration_type for context") }, :status => :bad_request)
end
settings = @plugin.settings || {}
if settings[:requires_file_upload]
if !(params[:pre_attachment] && params[:pre_attachment][:name].present?) && !(params[:settings] && params[:settings][:file_url].present?)
@ -353,7 +364,7 @@ class ContentMigrationsController < ApplicationController
#
# @returns [Migrator]
def available_migrators
systems = ContentMigration.migration_plugins(true)
systems = ContentMigration.migration_plugins(true).select{|sys| migration_plugin_supported?(sys)}
json = systems.map{|p| {
:type => p.id,
:requires_file_upload => !!p.settings[:requires_file_upload],

View File

@ -350,6 +350,7 @@ class ContentMigration < ActiveRecord::Base
end
def check_quiz_id_prepender
return unless self.context.respond_to?(:assessment_questions)
if !migration_settings[:id_prepender] && (!migration_settings[:overwrite_questions] || !migration_settings[:overwrite_quizzes])
# only prepend an id if the course already has some migrated questions/quizzes
if self.context.assessment_questions.where('assessment_questions.migration_id IS NOT NULL').exists? ||

View File

@ -59,6 +59,7 @@ class Group < ActiveRecord::Base
has_many :collaborations, :as => :context, :order => 'title, created_at', :dependent => :destroy
has_many :media_objects, :as => :context
has_many :zip_file_imports, :as => :context
has_many :content_migrations, :as => :context
belongs_to :avatar_attachment, :class_name => "Attachment"
before_validation :ensure_defaults

View File

@ -238,6 +238,7 @@ class User < ActiveRecord::Base
has_many :zip_file_imports, :as => :context
has_many :messages
has_many :sis_batches
has_many :content_migrations, :as => :context
has_one :profile, :class_name => 'UserProfile'
alias :orig_profile :profile

View File

@ -945,19 +945,23 @@ routes.draw do
end
scope(:controller => :content_migrations) do
get 'courses/:course_id/content_migrations/migrators', :action => :available_migrators, :path_name => 'course_content_migration_migrators_list'
get 'courses/:course_id/content_migrations/:id', :action => :show, :path_name => 'course_content_migration'
get 'courses/:course_id/content_migrations', :action => :index, :path_name => 'course_content_migration_list'
post 'courses/:course_id/content_migrations', :action => :create, :path_name => 'course_content_migration_create'
put 'courses/:course_id/content_migrations/:id', :action => :update, :path_name => 'course_content_migration_update'
get 'courses/:course_id/content_migrations/:id/selective_data', :action => :content_list, :path_name => 'course_content_migration_selective_data'
%w(course group user).each do |context|
get "#{context.pluralize}/:#{context}_id/content_migrations/migrators", :action => :available_migrators, :path_name => "#{context}_content_migration_migrators_list"
get "#{context.pluralize}/:#{context}_id/content_migrations/:id", :action => :show, :path_name => "#{context}_content_migration"
get "#{context.pluralize}/:#{context}_id/content_migrations", :action => :index, :path_name => "#{context}_content_migration_list"
post "#{context.pluralize}/:#{context}_id/content_migrations", :action => :create, :path_name => "#{context}_content_migration_create"
put "#{context.pluralize}/:#{context}_id/content_migrations/:id", :action => :update, :path_name => "#{context}_content_migration_update"
get "#{context.pluralize}/:#{context}_id/content_migrations/:id/selective_data", :action => :content_list, :path_name => "#{context}_content_migration_selective_data"
end
end
scope(:controller => :migration_issues) do
get 'courses/:course_id/content_migrations/:content_migration_id/migration_issues/:id', :action => :show, :path_name => 'course_content_migration_migration_issue'
get 'courses/:course_id/content_migrations/:content_migration_id/migration_issues', :action => :index, :path_name => 'course_content_migration_migration_issue_list'
post 'courses/:course_id/content_migrations/:content_migration_id/migration_issues', :action => :create, :path_name => 'course_content_migration_migration_issue_create'
put 'courses/:course_id/content_migrations/:content_migration_id/migration_issues/:id', :action => :update, :path_name => 'course_content_migration_migration_issue_update'
%w(course group user).each do |context|
get "#{context.pluralize}/:#{context}_id/content_migrations/:content_migration_id/migration_issues/:id", :action => :show, :path_name => "#{context}_content_migration_migration_issue"
get "#{context.pluralize}/:#{context}_id/content_migrations/:content_migration_id/migration_issues", :action => :index, :path_name => "#{context}_content_migration_migration_issue_list"
post "#{context.pluralize}/:#{context}_id/content_migrations/:content_migration_id/migration_issues", :action => :create, :path_name => "#{context}_content_migration_migration_issue_create"
put "#{context.pluralize}/:#{context}_id/content_migrations/:content_migration_id/migration_issues/:id", :action => :update, :path_name => "#{context}_content_migration_migration_issue_update"
end
end
scope(:controller => :discussion_topics_api) do

View File

@ -145,7 +145,7 @@ Canvas::Plugin.register 'zip_file_importer', :export_system, {
:display_name => lambda { I18n.t :zip_file_display, 'File Import' },
:author => 'Instructure',
:author_website => 'http://www.instructure.com',
:description => lambda { I18n.t :zip_file_description, 'Migration plugin for unpacking plain .zip files into a course' },
:description => lambda { I18n.t :zip_file_description, 'Migration plugin for unpacking .zip archives into course, group, or user files' },
:version => '1.0.0',
:select_text => lambda { I18n.t :zip_file_file_description, "Unzip .zip file into folder" },
:sort_order => 2,
@ -154,7 +154,8 @@ Canvas::Plugin.register 'zip_file_importer', :export_system, {
:requires_file_upload => true,
:no_selective_import => true,
:required_options_validator => Canvas::Migration::Validators::ZipImporterValidator,
:required_settings => [:source_folder_id]
:required_settings => [:source_folder_id],
:additional_contexts => %w(User Group)
},
}
Canvas::Plugin.register 'common_cartridge_importer', :export_system, {

View File

@ -64,6 +64,41 @@ describe ContentMigrationsController, type: :request do
api_call(:get, @migration_url, @params)
@course.reload.folders.should_not be_empty
end
context "User" do
before do
@migration = @user.content_migrations.create
@migration.migration_type = 'zip_file_import'
@migration.user = @user
@migration.save!
@migration_url = "/api/v1/users/#{@user.id}/content_migrations"
@params = @params.reject{ |k| k == :course_id }.merge(user_id: @user.id)
end
it "should return list" do
json = api_call(:get, @migration_url, @params)
json.length.should == 1
json.first['id'].should == @migration.id
end
end
context "Group" do
before do
group_with_user user: @user
@migration = @group.content_migrations.create
@migration.migration_type = 'zip_file_import'
@migration.user = @user
@migration.save!
@migration_url = "/api/v1/groups/#{@group.id}/content_migrations"
@params = @params.reject{ |k| k == :course_id }.merge(group_id: @group.id)
end
it "should return list" do
json = api_call(:get, @migration_url, @params)
json.length.should == 1
json.first['id'].should == @migration.id
end
end
end
describe 'show' do
@ -144,6 +179,40 @@ describe ContentMigrationsController, type: :request do
@migration.should be_failed
@migration.migration_issues.first.description.should == "The file upload process timed out."
end
context "User" do
before do
@migration = @user.content_migrations.create
@migration.migration_type = 'zip_file_import'
@migration.user = @user
@migration.save!
@migration_url = "/api/v1/users/#{@user.id}/content_migrations/#{@migration.id}"
@params = @params.reject{ |k| k == :course_id }.merge(user_id: @user.id, id: @migration.to_param)
end
it "should return migration" do
json = api_call(:get, @migration_url, @params)
json['id'].should == @migration.id
end
end
context "Group" do
before do
group_with_user user: @user
@migration = @group.content_migrations.create
@migration.migration_type = 'zip_file_import'
@migration.user = @user
@migration.save!
@migration_url = "/api/v1/groups/#{@group.id}/content_migrations/#{@migration.id}"
@params = @params.reject{ |k| k == :course_id }.merge(group_id: @group.id, id: @migration.to_param)
end
it "should return migration" do
json = api_call(:get, @migration_url, @params)
json['id'].should == @migration.id
end
end
end
describe 'create' do
@ -292,6 +361,46 @@ describe ContentMigrationsController, type: :request do
end
end
context "User" do
before do
@migration_url = "/api/v1/users/#{@user.id}/content_migrations"
@params = @params.reject{|k| k == :course_id}.merge(:user_id => @user.to_param)
@folder = Folder.root_folders(@user).first
end
it "should error for an unsupported type" do
json = api_call(:post, @migration_url, @params, {:migration_type => 'common_cartridge_importer'},
{}, :expected_status => 400)
json.should == {"message"=>"Unsupported migration_type for context"}
end
it "should queue a migration" do
json = api_call(:post, @migration_url, @params,
{ :migration_type => 'zip_file_importer',
:settings => { :file_url => 'http://example.com/oi.zip',
:folder_id => @folder.id }})
migration = ContentMigration.find json['id']
migration.context.should eql @user
end
end
context "Group" do
before do
group_with_user user: @user
@migration_url = "/api/v1/groups/#{@group.id}/content_migrations"
@params = @params.reject{|k| k == :course_id}.merge(:group_id => @group.to_param)
@folder = Folder.root_folders(@group).first
end
it "should queue a migration" do
json = api_call(:post, @migration_url, @params,
{ :migration_type => 'zip_file_importer',
:settings => { :file_url => 'http://example.com/oi.zip',
:folder_id => @folder.id }})
migration = ContentMigration.find json['id']
migration.context.should eql @group
end
end
end
describe 'update' do
@ -383,6 +492,19 @@ describe ContentMigrationsController, type: :request do
"required_settings" => []
}]
end
it "should filter by context type" do
Canvas::Plugin.stubs(:all_for_tag).returns([Canvas::Plugin.find('common_cartridge_importer'),
Canvas::Plugin.find('zip_file_importer')])
json = api_call(:get, "/api/v1/users/#{@user.id}/content_migrations/migrators",
{:controller=>"content_migrations", :action=>"available_migrators", :format=>"json", :user_id=>@user.to_param})
json.should == [{
"type" => "zip_file_importer",
"requires_file_upload" => true,
"name" => "Unzip .zip file into folder",
"required_settings" => ['source_folder_id']
}]
end
end
describe 'content selection' do

View File

@ -2276,12 +2276,11 @@ equation: <img class="equation_image" title="Log_216" src="/equation_images/Log_
end
context "zip file import" do
it "should import" do
course_with_teacher
def test_zip_import(context)
zip_path = File.join(File.dirname(__FILE__) + "/../fixtures/migration/file.zip")
cm = ContentMigration.new(:context => @course, :user => @user,)
cm = ContentMigration.new(:context => context, :user => @user,)
cm.migration_type = 'zip_file_importer'
cm.migration_settings[:folder_id] = Folder.root_folders(@course).first.id
cm.migration_settings[:folder_id] = Folder.root_folders(context).first.id
cm.save!
attachment = Attachment.new
@ -2295,8 +2294,22 @@ equation: <img class="equation_image" title="Log_216" src="/equation_images/Log_
cm.queue_migration
run_jobs
@course.reload
@course.attachments.count.should == 1
context.reload.attachments.count.should == 1
end
it "should import into a course" do
course_with_teacher
test_zip_import(@course)
end
it "should import into a user" do
user
test_zip_import(@user)
end
it "should import into a group" do
group_with_user
test_zip_import(@group)
end
end