ensure filename is sent by store_for_attachment

test plan:
 - with and without inst-fs enabled,
   - ensure both sis import upload paths work
     (both a multipart upload and a raw post)

flag=none
fixes FOO-4400

Change-Id: I2e868989ec4552cc53ed980c24b22578cada6f0c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/344058
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: Xander Moffatt <xmoffatt@instructure.com>
This commit is contained in:
Jeremy Stanley 2024-03-29 09:06:04 -06:00
parent 81f0da52db
commit 119052fd73
3 changed files with 10 additions and 4 deletions

View File

@ -20,16 +20,16 @@
class Attachments::Storage class Attachments::Storage
def self.store_for_attachment(attachment, data) def self.store_for_attachment(attachment, data)
if InstFS.enabled? if InstFS.enabled?
attachment.filename ||= detect_filename(data)
instfs_uuid = InstFS.direct_upload( instfs_uuid = InstFS.direct_upload(
file_object: data, file_object: data,
file_name: attachment.display_name file_name: attachment.display_name.presence || attachment.filename || "attachment"
) )
attachment.instfs_uuid = instfs_uuid attachment.instfs_uuid = instfs_uuid
attachment.md5 = Digest::SHA2.new(512).file(data).hexdigest if digest_file? data attachment.md5 = Digest::SHA2.new(512).file(data).hexdigest if digest_file? data
# populate attachment fields if they were not already set # populate attachment fields if they were not already set
attachment.size ||= data.size attachment.size ||= data.size
attachment.filename ||= detect_filename(data)
attachment.content_type ||= attachment.detect_mimetype(data) attachment.content_type ||= attachment.detect_mimetype(data)
attachment.workflow_state = "processed" attachment.workflow_state = "processed"
else else

View File

@ -63,7 +63,7 @@ class SisBatch < ActiveRecord::Base
# If you are going to change any settings on the batch before it's processed, # If you are going to change any settings on the batch before it's processed,
# do it in the block passed into this method, so that the changes are saved # do it in the block passed into this method, so that the changes are saved
# before the batch is marked created and eligible for processing. # before the batch is marked created and eligible for processing.
def self.create_with_attachment(account, import_type, attachment, user = nil) def self.create_with_attachment(account, import_type, file_obj, user = nil)
account.shard.activate do account.shard.activate do
batch = SisBatch.new batch = SisBatch.new
batch.account = account batch.account = account
@ -73,7 +73,7 @@ class SisBatch < ActiveRecord::Base
batch.user = user batch.user = user
batch.save batch.save
att = Attachment.create_data_attachment(batch, attachment) att = Attachment.create_data_attachment(batch, file_obj, file_obj.original_filename)
batch.attachment = att batch.attachment = att
yield batch if block_given? yield batch if block_given?

View File

@ -33,6 +33,12 @@ describe Attachments::Storage do
expect(@attachment.instfs_uuid).to eq(@uuid) expect(@attachment.instfs_uuid).to eq(@uuid)
end end
it "infers a filename for direct_upload" do
att = Attachment.new
expect(InstFS).to receive(:direct_upload).with(file_object: @file, file_name: "a.png")
Attachments::Storage.store_for_attachment(att, @file)
end
it "calls attachment_fu methods if inst-fs is not enabled" do it "calls attachment_fu methods if inst-fs is not enabled" do
allow(InstFS).to receive(:enabled?).and_return(false) allow(InstFS).to receive(:enabled?).and_return(false)
expect(@attachment).to receive(:uploaded_data=) expect(@attachment).to receive(:uploaded_data=)