export all files with filename instead of display name

module content tags will rename a file if there is a
title set

closes #4343

Change-Id: I4e90921b8dab62b452f599135d683515bd342ba1
Reviewed-on: https://gerrit.instructure.com/3260
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Zach Wily <zach@instructure.com>

app/models/context_module.rb

@@ -738,8 +738,13 @@ class ContextModule < ActiveRecord::Base
       # this is a file of some kind
       file = self.context.attachments.find_by_migration_id(hash[:migration_id]) if hash[:migration_id]
       if file
+        title = hash[:title] || hash[:linked_resource_title]
+        if file.display_name != title
+          file.display_name = title
+          file.save
+        end
         item = self.add_item({
-          :title => hash[:title] || hash[:linked_resource_title],
+          :title => title,
           :type => 'attachment',
           :id => file.id,
           :indent => hash[:indents].to_i

lib/cc/cc_helper.rb

@@ -138,7 +138,7 @@ module CCHelper
             if obj && obj.respond_to?(:grants_right?) && obj.grants_right?(user, nil, :read)
               if type == 'files'
                 folder = obj.folder.full_name.gsub(/course( |%20)files/, WEB_CONTENT_TOKEN)
-                new_url = "#{folder}/#{obj.display_name}#{file_query_string(rest)}"
+                new_url = "#{folder}/#{obj.filename}#{file_query_string(rest)}"
               elsif migration_id = CCHelper.create_key(obj)
                 new_url = "#{OBJECT_TOKEN}/#{type}/#{migration_id}"
               end

lib/cc/topic_resources.rb

@@ -77,7 +77,7 @@ module CC
       if topic.attachment
         doc.attachments do |atts|
           folder = topic.attachment.folder.full_name.gsub("course files", CCHelper::WEB_CONTENT_TOKEN)
-          path = "#{folder}/#{topic.attachment.display_name}"
+          path = "#{folder}/#{topic.attachment.filename}"
           atts.attachment(:href=>path)
         end
       end

lib/cc/web_resources.rb

@@ -30,7 +30,7 @@ module CC
           next
         end
 
-        path = File.join(folder_names, file.display_name)
+        path = File.join(folder_names, file.filename)
         migration_id = CCHelper.create_key(file)
         hidden_locked_files[:hidden_files] << migration_id if file.hidden?
         @resources.resource(

lib/content_zipper.rb

@@ -282,8 +282,8 @@ class ContentZipper
     folder.file_attachments.scoped(:conditions => "file_state IN ('available', 'hidden')").select{|a| !@user || a.grants_right?(@user, nil, :download)}.each do |attachment|
       callback.call(attachment, folder_names) if callback
       @context = folder.context
-      @logger.debug("  found attachment: #{attachment.display_name}")
-      path = folder_names.empty? ? attachment.display_name : File.join(folder_names, attachment.display_name)
+      @logger.debug("  found attachment: #{attachment.filename}")
+      path = folder_names.empty? ? attachment.filename : File.join(folder_names, attachment.filename)
       if add_attachment_to_zip(attachment, zipfile, path)
         @files_added = true
       end

spec/lib/cc/importer/cc_converter_spec.rb

@@ -333,6 +333,7 @@ describe "Common Cartridge importing" do
     mod1 = @copy_from.context_modules.create!(:name => "some module", :unlock_at => 1.week.from_now)
     mod2 = @copy_from.context_modules.create!(:name => "next module")
     mod3 = @copy_from.context_modules.create!(:name => "url module")
+    mod4 = @copy_from.context_modules.create!(:name => "attachment module")
     mod2.prerequisites = [{:type=>"context_module", :name=>mod1.name, :id=>mod1.id}]
     mod2.require_sequential_progress = true
     mod2.save!
@@ -358,6 +359,16 @@ describe "Common Cartridge importing" do
     mod3.add_item({ :title => 'Example 1', :type => 'external_url', :url => 'http://a.example.com/' })
     mod3.add_item({ :title => 'Example 2', :type => 'external_url', :url => 'http://b.example.com/' })
     
+    # attachments are migrated with just their filename as display_name, 
+    # but if a content tag has a different title the display_name should update
+    att = Attachment.create!(:filename => 'boring.txt', :display_name => "Super exciting!", :uploaded_data => StringIO.new('even more boring'), :folder => Folder.unfiled_folder(@copy_from), :context => @copy_from)
+    att.display_name.should == "Super exciting!"
+    # create @copy_to attachment with normal display_name
+    att_2 = Attachment.create!(:filename => 'boring.txt', :uploaded_data => StringIO.new('even more boring'), :folder => Folder.unfiled_folder(@copy_to), :context => @copy_to)
+    att_2.migration_id = CC::CCHelper.create_key(att)
+    att_2.save
+    mod4.add_item({:title => att.display_name, :type => "attachment", :id => att.id})
+    
     #export to xml
     builder = Builder::XmlMarkup.new(:indent=>2)
     @resource.create_module_meta(builder)
@@ -368,6 +379,7 @@ describe "Common Cartridge importing" do
     hash[0] = hash[0].with_indifferent_access
     hash[1] = hash[1].with_indifferent_access
     hash[2] = hash[2].with_indifferent_access
+    hash[3] = hash[3].with_indifferent_access
     ContextModule.process_migration({'modules'=>hash}, @migration)
     @copy_to.save!
     
@@ -395,6 +407,11 @@ describe "Common Cartridge importing" do
     mod3_2.content_tags.length.should == 2
     mod3_2.content_tags[0].url.should == "http://a.example.com/"
     mod3_2.content_tags[1].url.should == "http://b.example.com/"
+    
+    mod4_2 = @copy_to.context_modules.find_by_migration_id(CC::CCHelper.create_key(mod4))
+    mod4_2.content_tags.first.title.should == att.display_name
+    att_2.reload
+    att_2.display_name.should == att.display_name
   end
 
   it "should translate attachment links on import" do