fix discussion importing with attachments

also tweak a couple random little things because why not

test plan:
* create a course with a file
* import the package referenced in the ticket
* should not attach the file to every imported
 discussion topic

closes #CNVS-14520

Change-Id: Icf30b0a62d0d8f8ab6ced84990def1525ff061a4
Reviewed-on: https://gerrit.instructure.com/41760
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Clare Strong <clare@instructure.com>

app/models/importers/discussion_topic_importer.rb

@@ -99,9 +99,13 @@ module Importers
         item.workflow_state = 'active'
       end
 
-      item.attachment           = context.attachments.where(migration_id: options[:attachment_migration_id]).first
-      item.external_feed        = context.external_feeds.where(migration_id: options[:external_feed_migration_id]).first
-      item.assignment           = fetch_assignment
+      if options[:attachment_migration_id].present?
+        item.attachment = context.attachments.where(migration_id: options[:attachment_migration_id]).first
+      end
+      if options[:external_feed_migration_id].present?
+        item.external_feed = context.external_feeds.where(migration_id: options[:external_feed_migration_id]).first
+      end
+      item.assignment = fetch_assignment
 
       if options[:attachment_ids].present?
         item.message += Attachment.attachment_list_from_migration(context, options[:attachment_ids])

gems/canvas_unzip/lib/canvas_unzip.rb

@@ -100,6 +100,7 @@ class CanvasUnzip
 
     file = File.open(archive_filename)
     mime_type = File.mime_type?(file)
+
     if mime_type == 'application/x-gzip'
       file = Zlib::GzipReader.new(file)
       mime_type = 'application/x-tar' # it may not actually be a tar though, so rescue if there's a problem
@@ -120,10 +121,10 @@ class CanvasUnzip
           index += 1
         end
       rescue Gem::Package::TarInvalidError
-        raise UnknownArchiveType
+        raise UnknownArchiveType, "invalid tar"
       end
     else
-      raise UnknownArchiveType
+      raise UnknownArchiveType, "unknown mime type #{mime_type}"
     end
   end
 

spec/lib/cc/importer/common_cartridge_converter_spec.rb

@@ -557,7 +557,7 @@ describe "LTI tool combination" do
   before(:all) do
     archive_file_path = File.join(File.dirname(__FILE__) + "/../../../fixtures/migration/cc_lti_combine_test.zip")
     unzipped_file_path = File.join(File.dirname(archive_file_path), "cc_#{File.basename(archive_file_path, '.zip')}", 'oi')
-    @export_folder = File.join(File.dirname(archive_file_path), "cc_cc_lti_combine_test.")
+    @export_folder = File.join(File.dirname(archive_file_path), "cc_cc_lti_combine_test")
     @converter = CC::Importer::Standard::Converter.new(:export_archive_path=>archive_file_path, :course_name=>'oi', :base_download_dir=>unzipped_file_path)
     @converter.export
     @course_data = @converter.course.with_indifferent_access

spec/models/importers/discussion_topic_importer_spec.rb

@@ -26,6 +26,7 @@ describe Importers::DiscussionTopicImporter do
         data = get_import_data(system, 'discussion_topic')
         data = data.first
         data = data.with_indifferent_access
+
         context = get_import_context(system)
 
         data[:topics_to_import] = {}
@@ -76,4 +77,17 @@ describe Importers::DiscussionTopicImporter do
     end
   end
 
+  it "should not attach files when no attachment_migration_id is specified" do
+    data = get_import_data('bb8', 'discussion_topic').first.with_indifferent_access
+    context = get_import_context('bb8')
+
+    data[:attachment_migration_id] = nil
+    attachment_model(:context => context) # create a file with no migration id
+
+    data[:topics_to_import] = {data[:migration_id] => true}
+    Importers::DiscussionTopicImporter.import_from_migration(data, context)
+
+    topic = DiscussionTopic.find_by_migration_id(data[:migration_id])
+    topic.attachment.should be_nil
+  end
 end