direct import of content export
test plan: 1. do a common cartridge export via the API and save the id of the ContentExport (you need not actually download the file) 2. render the API docs and see how the content migration API can accept settings[content_export_id] in lieu of a file upload or file_url 3. test importing the export into another course via the content_migrations API and content_export_id 4. ensure you get an error if you specify the id of an export that you do not have access to (i.e., you did not create the export, you are not an account admin, and you are not a teacher in the export's source course) closes ADMIN-2815 flag=none Change-Id: I9e405bc573f734bc180735a4f2c10654eaa2aea6 Reviewed-on: https://gerrit.instructure.com/204482 QA-Review: Anju Reddy <areddy@instructure.com> Tested-by: Jenkins Product-Review: Jeremy Stanley <jeremy@instructure.com> Reviewed-by: James Williams <jamesw@instructure.com>
This commit is contained in:
parent
591280a37c
commit
c5ab512178
|
@ -248,6 +248,10 @@ class ContentMigrationsController < ApplicationController
|
|||
#
|
||||
# @argument settings[file_url] [string] A URL to download the file from. Must not require authentication.
|
||||
#
|
||||
# @argument settings[content_export_id] [String]
|
||||
# The id of a ContentExport to import. This allows you to import content previously exported from Canvas
|
||||
# without needing to download and re-upload it.
|
||||
#
|
||||
# @argument settings[source_course_id] [String]
|
||||
# The course to copy from for a course copy migration. (required if doing
|
||||
# course copy)
|
||||
|
@ -330,8 +334,8 @@ class ContentMigrationsController < ApplicationController
|
|||
|
||||
settings = @plugin.settings || {}
|
||||
if settings[:requires_file_upload]
|
||||
if !(params[:pre_attachment] && params[:pre_attachment][:name].present?) && !(params[:settings] && params[:settings][:file_url].present?)
|
||||
return render(:json => {:message => t('must_upload_file', "File upload or url is required")}, :status => :bad_request)
|
||||
if params.dig(:pre_attachment, :name).blank? && params.dig(:settings, :file_url).blank? && params.dig(:settings, :content_export_id).blank?
|
||||
return render(:json => {:message => t("File upload, file_url, or content_export_id is required")}, :status => :bad_request)
|
||||
end
|
||||
end
|
||||
source_course = lookup_sis_source_course
|
||||
|
@ -522,8 +526,14 @@ class ContentMigrationsController < ApplicationController
|
|||
end
|
||||
@content_migration.save!
|
||||
@content_migration.reset_job_progress
|
||||
elsif !params.has_key?(:do_not_run) || !Canvas::Plugin.value_to_boolean(params[:do_not_run])
|
||||
@content_migration.queue_migration(@plugin)
|
||||
else
|
||||
if params.dig(:settings, :content_export_id).present?
|
||||
return false unless link_content_export_attachment
|
||||
end
|
||||
|
||||
if !params.has_key?(:do_not_run) || !Canvas::Plugin.value_to_boolean(params[:do_not_run])
|
||||
@content_migration.queue_migration(@plugin)
|
||||
end
|
||||
end
|
||||
|
||||
render :json => content_migration_json(@content_migration, @current_user, session, preflight_json)
|
||||
|
@ -540,4 +550,22 @@ class ContentMigrationsController < ApplicationController
|
|||
course_count <= 100
|
||||
end
|
||||
end
|
||||
|
||||
def link_content_export_attachment
|
||||
ret = false
|
||||
export = ContentExport.find_by_id(params[:settings][:content_export_id])
|
||||
if export && export.grants_right?(@current_user, session, :read)
|
||||
if export.workflow_state == 'exported' && export.attachment
|
||||
@content_migration.attachment = export.attachment.clone_for(@content_migration)
|
||||
ret = true
|
||||
else
|
||||
render(:json => { :message => "content export is incomplete" }, :status => :bad_request)
|
||||
end
|
||||
else
|
||||
render(:json => { :message => "invalid content export" }, :status => :bad_request)
|
||||
end
|
||||
@content_migration.workflow_state = 'pre_process_error' unless ret
|
||||
@content_migration.save!
|
||||
ret
|
||||
end
|
||||
end
|
||||
|
|
|
@ -101,6 +101,11 @@ class ContentMigration < ActiveRecord::Base
|
|||
read_or_initialize_attribute(:migration_settings, {}.with_indifferent_access)
|
||||
end
|
||||
|
||||
# this is needed by Attachment#clone_for, which is used to allow a ContentExport to be directly imported
|
||||
def attachments
|
||||
Attachment.where(id: self.attachment_id)
|
||||
end
|
||||
|
||||
def update_migration_settings(new_settings)
|
||||
new_settings.each do |key, val|
|
||||
migration_settings[key] = val
|
||||
|
|
|
@ -402,7 +402,7 @@ describe ContentMigrationsController, type: :request do
|
|||
it "should error if upload file required but not provided" do
|
||||
@post_params.delete :pre_attachment
|
||||
json = api_call(:post, @migration_url, @params, @post_params, {}, :expected_status => 400)
|
||||
expect(json).to eq({"message"=>"File upload or url is required"})
|
||||
expect(json).to eq({"message"=>"File upload, file_url, or content_export_id is required"})
|
||||
end
|
||||
|
||||
it "should queue the migration when file finishes uploading" do
|
||||
|
@ -452,6 +452,56 @@ describe ContentMigrationsController, type: :request do
|
|||
|
||||
end
|
||||
|
||||
context "by content_export_id" do
|
||||
def stub_export(context, user, state, stub_attachment)
|
||||
export = context.content_exports.create
|
||||
export.export_type = 'common_cartridge'
|
||||
export.workflow_state = state
|
||||
export.user = user
|
||||
export.attachment = export.attachments.create!(filename: 'test.imscc', uploaded_data: StringIO.new('test file')) if stub_attachment
|
||||
export.save
|
||||
export
|
||||
end
|
||||
|
||||
it "links the file from the content export to the content migration" do
|
||||
export = stub_export(@course, @user, 'exported', true)
|
||||
post_params = { migration_type: 'common_cartridge_importer', settings: { content_export_id: export.id }}
|
||||
json = api_call(:post, @migration_url, @params, post_params)
|
||||
migration = ContentMigration.find json['id']
|
||||
expect(migration.attachment).not_to be_nil
|
||||
expect(migration.attachment.root_attachment).to eq export.attachment
|
||||
end
|
||||
|
||||
it "verifies the content export exists" do
|
||||
post_params = { migration_type: 'common_cartridge_importer', settings: { content_export_id: 0 }}
|
||||
json = api_call(:post, @migration_url, @params, post_params)
|
||||
expect(response.status).to eq 400
|
||||
expect(json['message']).to eq 'invalid content export'
|
||||
expect(ContentMigration.last).to be_pre_process_error
|
||||
end
|
||||
|
||||
it "verifies the user has permission to read the content export" do
|
||||
me = @user
|
||||
course_with_teacher(active_all: true)
|
||||
export = stub_export(@course, @teacher, 'exported', true)
|
||||
@user = me
|
||||
post_params = { migration_type: 'common_cartridge_importer', settings: { content_export_id: export.id }}
|
||||
json = api_call(:post, @migration_url, @params, post_params)
|
||||
expect(response.status).to eq 400
|
||||
expect(json['message']).to eq 'invalid content export'
|
||||
expect(ContentMigration.last).to be_pre_process_error
|
||||
end
|
||||
|
||||
it "rejects an incomplete export" do
|
||||
export = stub_export(@course, @user, 'exporting', false)
|
||||
post_params = { migration_type: 'common_cartridge_importer', settings: { content_export_id: export.id }}
|
||||
json = api_call(:post, @migration_url, @params, post_params)
|
||||
expect(response.status).to eq 400
|
||||
expect(json['message']).to eq 'content export is incomplete'
|
||||
expect(ContentMigration.last).to be_pre_process_error
|
||||
end
|
||||
end
|
||||
|
||||
context "by LTI extension" do
|
||||
it "should queue migration with LTI url sent" do
|
||||
#@migration.fail_with_error!(nil) # clear out old migration
|
||||
|
|
Loading…
Reference in New Issue