fix submissions.zip filenames frd (and fix broken spec)

fixes CNVS-7636

This fixes a failing spec and also resolves some remaining edge cases
where _\d+_ could have inappropriately happened.

Test plan:
  * Make an assignment with students named something like:
    - student 1
    - student 2!
  * Download the submissions zip
  * Make sure uploading them works (their submissions are not ignored)

Change-Id: Iad0de754f618a68f815780b89df0aaa9ea7afd16
Reviewed-on: https://gerrit.instructure.com/23490
QA-Review: Amber Taniuchi <amber@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cameron Matheson <cameron@instructure.com>

Author: cmatheson

lib/content_zipper.rb

@@ -92,10 +92,10 @@ def zip_assignment(zip_attachment, assignment)
           users_name = students[submission.user_id].sortable_name
           # necessary because we use /_\d+_/ to infer the user/attachment
           # ids when teachers upload graded submissions
-          users_name.gsub! /[_ ](\d+)[_ ]/, '-\1-'
+          users_name.gsub! /_(\d+)_/, '-\1-'
 
-          filename = users_name + (submission.late? ? " LATE-" : "-") + submission.user_id.to_s
-          filename = filename.gsub(/ /, "_").gsub(/[^-\w]/, "").downcase
+          filename = users_name + (submission.late? ? " LATE_" : "_") + submission.user_id.to_s
+          filename = filename.gsub(/ /, "-").gsub(/[^-\w]/, "-").downcase
           content = nil
           if submission.submission_type == "online_upload"
             submission.attachments.each do |attachment|

spec/lib/content_zipper_spec.rb

@@ -24,20 +24,23 @@
       s1, s2 = 2.times.map { course_with_student ; @student }
       s1.update_attribute :sortable_name, 'some_999_, _1234_guy'
       s2.update_attribute :sortable_name, 'other 567, guy 8'
-      [s1, s2].each { |s| submission_model(:user => s) }
+      [s1, s2].each { |s|
+        submission_model user: s, assignment: @assignment, body: "blah"
+      }
       attachment = Attachment.new(:display_name => 'my_download.zip')
       attachment.user = @teacher
       attachment.workflow_state = 'to_be_zipped'
       attachment.context = @assignment
       attachment.save!
       ContentZipper.process_attachment(attachment, @teacher)
       expected_file_patterns = [
-        /other-567-_guy-8-/,
-        /some-999-_-1234-guy/,
+        /other-567--guy-8/,
+        /some-999----1234-guy/,
       ]
       Zip::ZipFile.foreach(attachment.reload.full_filename).each { |f|
         f.name.should =~ expected_file_patterns.shift
       }
+      expected_file_patterns.should be_empty
     end
 
     it "should zip up online_url submissions" do
@@ -54,7 +57,7 @@
       attachment.workflow_state.should == 'zipped'
       Zip::ZipFile.foreach(attachment.full_filename) do |f|
         if f.file?
-          f.name.should =~ /some-999-_-1234-guy/
+          f.name.should =~ /some-999----1234-guy/
           f.get_input_stream.read.should match(%r{This submission was a url, we're taking you to the url link now.})
           f.get_input_stream.read.should be_include("http://www.instructure.com/")
         end
@@ -116,7 +119,7 @@
 
       ContentZipper.process_attachment(attachment, @teacher)
       sub_count = 0
-      expected_file_names = [/group_0/, /group_1/]
+      expected_file_names = [/group-0/, /group-1/]
       Zip::ZipFile.foreach(attachment.full_filename) do |f|
         f.name.should =~ expected_file_names.shift
         sub_count += 1