ignore attachment_associations that aren't linked to attachments

closes GRADE-264

Test Plan 1: attachment_id is nil
1. Create a 'file upload' assignment.
2. Submit as a student and attach a file.
3. Go into a rails console and adjust the attachment_association so that
   it no longer points to the attachment:

   assignment = Assignment.find(<assignment-id>)
   submission = assignment.submissions.find_by(user_id: <student-id>)
   association = submission.attachment_associations.first
   association.update!(attachment_id: nil)

4. Open the assignment in SpeedGrader and verify there is not a 500
   thrown.

Test Plan 2: attachment_id points to a deleted attachment
1. Create a 'file upload' assignment.
2. Submit as a student and attach a file.
3. Go into a rails console and permanently destroy the attachment:

   assignment = Assignment.find(<assignment-id>)
   submission = assignment.submissions.find_by(user_id: <student-id>)
   attachment = submission.attachments.first
   attachment.destroy_permanently!

4. Open the assignment in SpeedGrader and verify there is not a 500
   thrown.
Change-Id: I61b867ef74ae49e2173068e237bfd8c0c75a9348
Reviewed-on: https://gerrit.instructure.com/125450
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Tested-by: Jenkins
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Pert Eilers <peilers@instructure.com>

app/models/submission.rb

@@ -1530,7 +1530,7 @@ class Submission < ActiveRecord::Base
     end
 
     attachments_by_submission = submissions.map do |s|
-      [s, attachments_by_id.values_at(*attachment_ids_by_submission[s]).flatten.uniq]
+      [s, attachments_by_id.values_at(*attachment_ids_by_submission[s]).flatten.compact.uniq]
     end
     Hash[attachments_by_submission]
   end

spec/models/submission_spec.rb

@@ -2916,6 +2916,15 @@ describe Submission do
         end
       end
 
+      it "filters out deleted attachments" do
+        student = student_in_course(active_all: true).user
+        attachment = attachment_model(filename: "submission.doc", context: student)
+        submission = @assignment.submit_homework(student, attachments: [attachment])
+        attachment.destroy_permanently!
+        submission_with_attachments = Submission.bulk_load_versioned_attachments([submission]).first
+        expect(submission_with_attachments.versioned_attachments).to be_empty
+      end
+
       it "includes url submission attachments" do
         s = submission_for_some_user
         s.attachment = attachment_model(filename: "screenshot.jpg",
@@ -2984,6 +2993,42 @@ describe Submission do
         result = Submission.bulk_load_attachments_for_submissions(s)
         expect(result).to eq(expected_attachments_for_submissions)
       end
+
+      it "filters out attachment associations that don't point to an attachment" do
+        student = student_in_course(active_all: true).user
+        attachment = attachment_model(filename: "submission.doc", context: student)
+        submission = @assignment.submit_homework(student, attachments: [attachment])
+        submission.attachment_associations.find_by(attachment_id: attachment.id).update!(attachment_id: nil)
+        attachments = Submission.bulk_load_attachments_for_submissions([submission]).first.second
+        expect(attachments).to be_empty
+      end
+
+      it "filters out attachment associations that point to deleted attachments" do
+        student = student_in_course(active_all: true).user
+        attachment = attachment_model(filename: "submission.doc", context: student)
+        submission = @assignment.submit_homework(student, attachments: [attachment])
+        attachment.destroy_permanently!
+        attachments = Submission.bulk_load_attachments_for_submissions([submission]).first.second
+        expect(attachments).to be_empty
+      end
+
+      it "includes valid attachments and filters out deleted attachments" do
+        student = student_in_course(active_all: true).user
+        attachment = attachment_model(filename: "submission.doc", context: student)
+        submission = @assignment.submit_homework(student, attachments: [attachment])
+        attachment.destroy_permanently!
+
+        another_student = student_in_course(active_all: true).user
+        another_attachment = attachment_model(filename: "submission.doc", context: another_student)
+        another_submission = @assignment.submit_homework(another_student, attachments: [another_attachment])
+
+        bulk_loaded_submissions = Submission.bulk_load_attachments_for_submissions([submission, another_submission])
+        submission_attachments = bulk_loaded_submissions.find { |s| s.first.id == submission.id }.second
+        expect(submission_attachments).to be_empty
+
+        another_submission_attachments = bulk_loaded_submissions.find { |s| s.first.id == another_submission.id }.second
+        expect(another_submission_attachments).not_to be_empty
+      end
     end
   end