fix the filename identification when submitting an assignment
When we’re submitting a file using the LTI tool, in some situations, we can face a tool that doesn't send the text field in the content item payload. This field is used to infer the filename and extension of the submission file. As a fallback, we'll try to recover the filename from the URL. closes INTEROP-6483 flag=none test-plan * Have an LTI tool installed (you can use lti-1.3-test-tool); You could use this commit https://gerrit.instructure.com/c/lti-1.3-test-tool/+/257525 to facilitate the tests in case it was not submitted yet. * Have a Course recorded; * Have a Student enrolled in this Course; * Have an Assignment with online submission recorded to this Course; * As a Student, you should be able to access the Assignment to submit your homework. On the submission homework page, you should be able to find LTI tool (usually rendered in a tab); * After launching the tool, you should be able to select File as the Content Item Type and fill the File URL field and submit; * After submitting the Assignment, you should be able to access the Submission Details page, and validate if the specified file was properly created/recorded (name and image); * I used https://dummyimage.com to generate some images like: * https://dummyimage.com/300/09f/sample * https://dummyimage.com/300/09f/sample.png * https://dummyimage.com/300/09f * https://dummyimage.com/300 * You should check if when the LTI tool sends the File Name (text) field and everything works as expected; Change-Id: Iae5f128492834f8efc8934de347cfa5d9944ac1b Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/257523 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Xander Moffatt <xmoffatt@instructure.com> QA-Review: Xander Moffatt <xmoffatt@instructure.com> Product-Review: Karl Lloyd <karl@instructure.com>
This commit is contained in:
parent
66464cbbf6
commit
2d605454cc
|
@ -621,7 +621,8 @@ class SubmissionsApiController < ApplicationController
|
|||
@assignment = api_find(@context.assignments.active, params[:assignment_id])
|
||||
@user = get_user_considering_section(params[:user_id])
|
||||
if @assignment.allowed_extensions.any?
|
||||
extension = infer_upload_filename(params).split('.').last&.downcase || File.mime_types[infer_upload_content_type(params)]
|
||||
filename = infer_upload_filename(params)
|
||||
extension = filename&.split('.')&.last&.downcase || File.mime_types[infer_upload_content_type(params)]
|
||||
reject!(t('unable to find extension')) unless extension
|
||||
reject!(t('filetype not allowed')) unless @assignment.allowed_extensions.include?(extension)
|
||||
end
|
||||
|
|
|
@ -18,6 +18,8 @@
|
|||
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
|
||||
require 'uri'
|
||||
|
||||
module Api::V1::Attachment
|
||||
include Api::V1::Json
|
||||
include Api::V1::Locked
|
||||
|
@ -172,8 +174,20 @@ module Api::V1::Attachment
|
|||
hash
|
||||
end
|
||||
|
||||
def infer_filename_from_url(url)
|
||||
return url if url.blank?
|
||||
|
||||
uri = URI.parse(url)
|
||||
|
||||
File.basename(uri.path)
|
||||
rescue URI::InvalidURIError
|
||||
nil
|
||||
end
|
||||
|
||||
def infer_upload_filename(params)
|
||||
params[:name] || params[:filename]
|
||||
return nil unless params
|
||||
|
||||
params[:name] || params[:filename] || infer_filename_from_url(params[:url])
|
||||
end
|
||||
|
||||
def infer_upload_content_type(params, default_mimetype = nil)
|
||||
|
|
|
@ -4454,9 +4454,9 @@ describe 'Submissions API', type: :request do
|
|||
end
|
||||
end
|
||||
|
||||
it "returns upload_params" do
|
||||
it "returns upload_params infering the filename from the URL" do
|
||||
upload_json = {
|
||||
"filename" => nil,
|
||||
"filename" => "test",
|
||||
"content_type" => nil,
|
||||
"target_url" => "http://example.com/test"
|
||||
}
|
||||
|
|
|
@ -58,4 +58,64 @@ describe Api::V1::Attachment do
|
|||
expect(json.fetch('thumbnail_url')).to eq json.fetch('url')
|
||||
end
|
||||
end
|
||||
|
||||
describe '#infer_upload_filename' do
|
||||
it { expect(infer_upload_filename(nil)).to be_nil }
|
||||
|
||||
it { expect(infer_upload_filename({})).to be_nil }
|
||||
|
||||
it 'return the name when it is given' do
|
||||
params = { name: 'filename', filename: 'filename.jpg' }
|
||||
|
||||
expect(infer_upload_filename(params)).to eq 'filename'
|
||||
end
|
||||
|
||||
it 'return the filename when it is given' do
|
||||
params = { filename: 'filename.png' }
|
||||
|
||||
expect(infer_upload_filename(params)).to eq 'filename.png'
|
||||
end
|
||||
|
||||
it 'return the filename inferred from the url when it is given' do
|
||||
params = { url: 'http://www.example.com/foo/bar/filename.jpg' }
|
||||
|
||||
expect(infer_upload_filename(params)).to eq 'filename.jpg'
|
||||
end
|
||||
end
|
||||
|
||||
describe '#infer_filename_from_url' do
|
||||
it { expect(infer_filename_from_url(nil)).to be_nil }
|
||||
|
||||
it { expect(infer_filename_from_url('')).to be_empty }
|
||||
|
||||
it 'return the filename with extension when it is given' do
|
||||
url = 'http://www.example.com/foo/bar/filename.jpeg?foo=bar×tamp=123'
|
||||
|
||||
expect(infer_filename_from_url(url)).to eq 'filename.jpeg'
|
||||
end
|
||||
|
||||
it 'return the last path when the URL does not have the filename in the path' do
|
||||
url = 'http://www.example.com/foo'
|
||||
|
||||
expect(infer_filename_from_url(url)).to eq 'foo'
|
||||
|
||||
url = 'https://docs.google.com/spreadsheets/d/xpto/edit#gid=0'
|
||||
|
||||
expect(infer_filename_from_url(url)).to eq 'edit'
|
||||
|
||||
url = 'http://example.com/sites/download.xpto'
|
||||
|
||||
expect(infer_filename_from_url(url)).to eq 'download.xpto'
|
||||
|
||||
url = 'https://via.placeholder.com/150?text=thumbnail'
|
||||
|
||||
expect(infer_filename_from_url(url)).to eq '150'
|
||||
end
|
||||
|
||||
it 'return empty when URL only have the domain' do
|
||||
url = 'http://www.example.com'
|
||||
|
||||
expect(infer_filename_from_url(url)).to be_empty
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue