use display name for files in downloaded zips fixes #6731

test plan:
 * upload a file
 * rename it via the ui
 * download the folder as a zip
 * the file should be named what it is in the UI

Change-Id: I61c29d18d5384d24303e6ce57423fb055884e5ec
Reviewed-on: https://gerrit.instructure.com/10418
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
This commit is contained in:
Cody Cutrer 2012-04-30 12:08:55 -06:00
parent 5868bd8b34
commit 3172e45ccd
2 changed files with 25 additions and 9 deletions

View File

@ -306,7 +306,7 @@ class ContentZipper
callback.call(attachment, folder_names) if callback callback.call(attachment, folder_names) if callback
@context = folder.context @context = folder.context
@logger.debug(" found attachment: #{attachment.unencoded_filename}") @logger.debug(" found attachment: #{attachment.unencoded_filename}")
path = folder_names.empty? ? attachment.filename : File.join(folder_names, attachment.unencoded_filename) path = folder_names.empty? ? attachment.display_name : File.join(folder_names, attachment.display_name)
@files_added = false unless add_attachment_to_zip(attachment, zipfile, path) @files_added = false unless add_attachment_to_zip(attachment, zipfile, path)
end end
folder.active_sub_folders.select{|f| !@check_user || f.grants_right?(@user, nil, :read_contents)}.each do |sub_folder| folder.active_sub_folders.select{|f| !@check_user || f.grants_right?(@user, nil, :read_contents)}.each do |sub_folder|

View File

@ -105,16 +105,16 @@ describe ContentZipper do
names = [] names = []
@attachment.reload @attachment.reload
Zip::ZipFile.foreach(@attachment.full_filename) {|f| names << f.name if f.file? } Zip::ZipFile.foreach(@attachment.full_filename) {|f| names << f.name if f.file? }
names names.sort
end end
context "in a private course" do context "in a private course" do
it "should give logged in students some files" do it "should give logged in students some files" do
zipped_files_for_user(@user).should == ['visible.png', 'visible/sub-vis.png'] zipped_files_for_user(@user).should == ['visible.png', 'visible/sub-vis.png'].sort
end end
it "should give logged in teachers all files" do it "should give logged in teachers all files" do
zipped_files_for_user(@teacher).should == ["locked/sub-locked-vis.png", "hidden/sub-hidden.png", "hidden.png", "visible.png", "visible/sub-locked.png", "visible/sub-vis.png", "locked.png"] zipped_files_for_user(@teacher).should == ["locked/sub-locked-vis.png", "hidden/sub-hidden.png", "hidden.png", "visible.png", "visible/sub-locked.png", "visible/sub-vis.png", "locked.png"].sort
end end
it "should give logged out people no files" do it "should give logged out people no files" do
@ -122,7 +122,7 @@ describe ContentZipper do
end end
it "should give all files if check_user=false" do it "should give all files if check_user=false" do
zipped_files_for_user(nil, false).should == ["locked/sub-locked-vis.png", "hidden/sub-hidden.png", "hidden.png", "visible.png", "visible/sub-locked.png", "visible/sub-vis.png", "locked.png"] zipped_files_for_user(nil, false).should == ["locked/sub-locked-vis.png", "hidden/sub-hidden.png", "hidden.png", "visible.png", "visible/sub-locked.png", "visible/sub-vis.png", "locked.png"].sort
end end
end end
@ -133,19 +133,19 @@ describe ContentZipper do
end end
it "should give logged in students some files" do it "should give logged in students some files" do
zipped_files_for_user(@user).should == ['visible.png', 'visible/sub-vis.png'] zipped_files_for_user(@user).should == ['visible.png', 'visible/sub-vis.png'].sort
end end
it "should give logged in teachers all files" do it "should give logged in teachers all files" do
zipped_files_for_user(@teacher).should == ["locked/sub-locked-vis.png", "hidden/sub-hidden.png", "hidden.png", "visible.png", "visible/sub-locked.png", "visible/sub-vis.png", "locked.png"] zipped_files_for_user(@teacher).should == ["locked/sub-locked-vis.png", "hidden/sub-hidden.png", "hidden.png", "visible.png", "visible/sub-locked.png", "visible/sub-vis.png", "locked.png"].sort
end end
it "should give logged out people the same thing as students" do it "should give logged out people the same thing as students" do
zipped_files_for_user(nil).should == ['visible.png', 'visible/sub-vis.png'] zipped_files_for_user(nil).should == ['visible.png', 'visible/sub-vis.png'].sort
end end
it "should give all files if check_user=false" do it "should give all files if check_user=false" do
zipped_files_for_user(nil, false).should == ["locked/sub-locked-vis.png", "hidden/sub-hidden.png", "hidden.png", "visible.png", "visible/sub-locked.png", "visible/sub-vis.png", "locked.png"] zipped_files_for_user(nil, false).should == ["locked/sub-locked-vis.png", "hidden/sub-hidden.png", "hidden.png", "visible.png", "visible/sub-locked.png", "visible/sub-vis.png", "locked.png"].sort
end end
end end
end end
@ -161,5 +161,21 @@ describe ContentZipper do
ContentZipper.process_attachment(attachment, @user) ContentZipper.process_attachment(attachment, @user)
attachment.workflow_state.should == 'zipped' attachment.workflow_state.should == 'zipped'
end end
it "should use the display name" do
course_with_student(:active_all => true)
folder = Folder.root_folders(@course).first
attachment_model(:uploaded_data => stub_png_data('hidden.png'), :content_type => 'image/png', :folder => folder, :display_name => 'otherfile.png')
attachment = Attachment.new(:display_name => 'my_download.zip')
attachment.user_id = @user.id
attachment.workflow_state = 'to_be_zipped'
attachment.context = folder
attachment.save!
ContentZipper.process_attachment(attachment, @user)
attachment.reload
names = []
Zip::ZipFile.foreach(attachment.full_filename) {|f| names << f.name if f.file? }
names.should == ['otherfile.png']
end
end end
end end