recoginize urls with a '+' for spaces on html import

this also makes it so that urls in html are exported with
'%20' instead of '+' from now on

refs #4903

Change-Id: I8982b7e38a151101e33a7ea33301bff64f56079d
Reviewed-on: https://gerrit.instructure.com/4710
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>

lib/cc/cc_helper.rb

@@ -145,13 +145,12 @@ module CCHelper
           }.each do |type, obj_class|
             if type != 'wiki' && sub_spot =~ %r{\A#{type}/(\d+)([^\s"]*)$}
               # it's pointing to a specific file or object
-              obj = obj_class.find($1) rescue nil
+              obj = obj_class.find_by_id($1)
               rest = $2
               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)
-                  # attachment filenames are already url encoded
-                  new_url = "#{folder}/#{obj.filename}#{CCHelper.file_query_string(rest)}"
+                  new_url = "#{folder}/#{URI.escape(obj.display_name)}#{CCHelper.file_query_string(rest)}"
                 elsif migration_id = CCHelper.create_key(obj)
                   new_url = "#{OBJECT_TOKEN}/#{type}/#{migration_id}"
                 end

lib/imported_html_converter.rb

@@ -88,10 +88,18 @@ class ImportedHtmlConverter
   def self.replace_relative_file_url(rel_path, context, course_path)
     new_url = nil
     rel_path, qs = rel_path.split('?', 2)
+    # This is for backward-compatibility: canvas attachment filenames are escaped
+    # with '+' for spaces and older exports have files with that instead of %20
+    alt_rel_path = rel_path.gsub('+', ' ')
     if context.respond_to?(:attachment_path_id_lookup) &&
         context.attachment_path_id_lookup &&
-        context.attachment_path_id_lookup[rel_path]
-      if file = context.attachments.find_by_migration_id(context.attachment_path_id_lookup[rel_path])
+        (context.attachment_path_id_lookup[rel_path] || context.attachment_path_id_lookup[alt_rel_path])
+      if context.attachment_path_id_lookup[rel_path]
+        file = context.attachments.find_by_migration_id(context.attachment_path_id_lookup[rel_path])
+      else
+        file = context.attachments.find_by_migration_id(context.attachment_path_id_lookup[alt_rel_path])
+      end
+      if file
         new_url = "/courses/#{context.id}/files/#{file.id}"
         # support other params in the query string, that were exported from the
         # original path components and query string. see

spec/lib/cc/importer/cc_converter_spec.rb

@@ -471,6 +471,18 @@ describe "Common Cartridge importing" do
     mod2 = @copy_to.context_modules.create(:name => "some module")
     mod2.migration_id = CC::CCHelper.create_key(mod)
     mod2.save!
+    # Create files for the wiki text to reference
+    from_root = Folder.root_folders(@copy_from).first
+    from_dir = Folder.create!(:name => "sub & folder", :parent_folder => from_root, :context => @copy_from)
+    from_att = Attachment.create!(:filename => 'picture+%2B+cropped.png', :display_name => "picture + cropped.png", :uploaded_data => StringIO.new('pretend .png data'), :folder => from_dir, :context => @copy_from)
+    
+    to_root = Folder.root_folders(@copy_to).first
+    to_dir = Folder.create!(:name => "sub & folder", :parent_folder => to_root, :context => @copy_to)
+    to_att = Attachment.create!(:filename => 'picture+%2B+cropped.png', :display_name => "picture + cropped.png", :uploaded_data => StringIO.new('pretend .png data'), :folder => to_dir, :context => @copy_to)
+    to_att.migration_id = CC::CCHelper.create_key(from_att)
+    to_att.save
+    path = to_att.full_display_path.gsub('course files/', '')
+    @copy_to.attachment_path_id_lookup = {path => to_att.migration_id}
     
     body_with_link = %{<p>Watup? <strong>eh?</strong>
       <a href=\"/courses/%s/assignments\">Assignments</a>
@@ -478,12 +490,12 @@ describe "Common Cartridge importing" do
       <a href=\"/courses/%s/wiki/assignments\">Assignments wiki link</a>
       <a href=\"/courses/%s/modules\">Modules</a>
       <a href=\"/courses/%s/modules/%s\">some module</a>
-      </p>
+      <img src="/courses/%s/files/%s/preview" alt="picture.png" /></p>
       <div>
         <div><img src="http://www.instructure.com/images/header-logo.png"></div>
         <div><img src="http://www.instructure.com/images/header-logo.png"></div>
       </div>}
-    page = @copy_from.wiki.wiki_pages.create!(:title => "some page", :body => body_with_link % [ @copy_from.id, @copy_from.id, @copy_from.id, @copy_from.id, @copy_from.id, mod.id ])
+    page = @copy_from.wiki.wiki_pages.create!(:title => "some page", :body => body_with_link % [ @copy_from.id, @copy_from.id, @copy_from.id, @copy_from.id, @copy_from.id, mod.id, @copy_from.id, from_att.id ])
     @copy_from.save!
     
     #export to html file
@@ -499,7 +511,7 @@ describe "Common Cartridge importing" do
     page_2 = @copy_to.wiki.wiki_pages.find_by_migration_id(migration_id)
     page_2.title.should == page.title
     page_2.url.should == page.url
-    page_2.body.should == (body_with_link % [ @copy_to.id, @copy_to.id, @copy_to.id, @copy_to.id, @copy_to.id, mod2.id ]).gsub(/png">/, 'png" />')
+    page_2.body.should == (body_with_link % [ @copy_to.id, @copy_to.id, @copy_to.id, @copy_to.id, @copy_to.id, mod2.id, @copy_to.id, to_att.id ]).gsub(/png">/, 'png" />')
   end
   
   it "should import assignments" do