fix docviewer for historical submissions

The method used to find the Submission object via the
AttachmentAssociation won't work for previous versions of the
submission if they had different attachments. In scenerios where we
we have the submission information, so we'll take the
permissions for whitelists, user info, etc off the submission and
assignment directly rather than attempting to look up via the
attachment association.

fixes GRADE-1611

test plan:
 - Have DocViewer configured in your canvas instance
 - Have a course with an assignment and a student
 - As the student, upload/submit homework
 - As a teacher confirm that docviewer works in speedgrader
 - As the student, resubmit the homework
 - As a teacher confirm that docviewer works in speedgrader for the
   current submission
 - As a teacher confirm that docviewer works in speedgrader for the
   previous submission

Change-Id: I3bb34dbdbe9b254a2ff4e89f6dc89635ab2f0766
Reviewed-on: https://gerrit.instructure.com/164814
Reviewed-by: Gary Mei <gmei@instructure.com>
Tested-by: Jenkins
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>

app/controllers/canvadoc_sessions_controller.rb

@@ -35,14 +35,22 @@ class CanvadocSessionsController < ApplicationController
         enable_annotations: blob['enable_annotations']
       }
 
+
+      submission_id = blob["submission_id"]
+      if submission_id
+        submission = Submission.preload(:assignment).find(submission_id)
+        user_session_params = Canvadocs.user_session_params(@current_user, submission: submission)
+      else
+        user_session_params = Canvadocs.user_session_params(@current_user, attachment: attachment)
+      end
+
       if opts[:enable_annotations]
         # Docviewer only cares about the enrollment type when we're doing annotations
         opts[:enrollment_type] = blob["enrollment_type"]
         # If we STILL don't have a role, something went way wrong so let's be unauthorized.
         return render(plain: 'unauthorized', status: :unauthorized) if opts[:enrollment_type].blank?
 
-        submission_id = blob["submission_id"]
-        assignment = Submission.find(submission_id).assignment
+        assignment = submission.assignment
         opts[:audit_url] = submission_docviewer_audit_events_url(submission_id) if assignment.auditable?
         opts[:anonymous_instructor_annotations] = !!blob["anonymous_instructor_annotations"] if blob["anonymous_instructor_annotations"]
       end
@@ -54,7 +62,7 @@ class CanvadocSessionsController < ApplicationController
       # TODO: Remove the next line after the DocViewer Data Migration project RD-4702
       opts[:region] = attachment.shard.database_server.config[:region] || "none"
       attachment.submit_to_canvadocs(1, opts) unless attachment.canvadoc_available?
-      user_session_params = Canvadocs.user_session_params(attachment, @current_user)
+
       url = attachment.canvadoc.session_url(opts.merge(user_session_params))
       # For the purposes of reporting student viewership, we only
       # care if the original attachment owner is looking

lib/canvadocs.rb

@@ -210,11 +210,14 @@ module Canvadocs
     annotations_supported? && Canvas::Plugin.value_to_boolean(config["hijack_crocodoc_sessions"])
   end
 
-  def self.user_session_params(attachment, current_user)
-    submission = Submission.find_by(
-      id: AttachmentAssociation.where(context_type: 'Submission', attachment: attachment).select(:context_id)
-    )
-    return {} if submission.blank?
+  def self.user_session_params(current_user, attachment: nil, submission: nil)
+    if submission.nil?
+      return {} if attachment.nil?
+      submission = Submission.find_by(
+        id: AttachmentAssociation.where(context_type: 'Submission', attachment: attachment).select(:context_id)
+      )
+      return {} if submission.nil?
+    end
     assignment = submission.assignment
     enrollments = assignment.course.enrollments.index_by(&:user_id)
     session_params = current_user_session_params(submission, current_user, enrollments)

spec/controllers/canvadoc_sessions_controller_spec.rb

@@ -254,6 +254,13 @@ describe CanvadocSessionsController do
         get :show, params: {blob: blob.to_json, hmac: hmac}
       end
 
+      it "passes user information based on the submission (if past submission / missing attachment assocation)" do
+        @submission.attachment_associations.destroy_all
+        expect(@attachment.canvadoc).to receive(:session_url).with(hash_including(user_id: @student.global_id.to_s))
+
+        get :show, params: {blob: blob.to_json, hmac: hmac}
+      end
+
       it "sends anonymous_instructor_annotations when true in the blob" do
         blob[:anonymous_instructor_annotations] = true
 

spec/lib/canvadocs_spec.rb

@@ -35,7 +35,6 @@ describe Canvadocs do
       )
     end
 
-    let(:session_params) { Canvadocs.user_session_params(attachment, @current_user) }
     let(:user_filter) { session_params[:user_filter] }
 
     before(:each) do
@@ -45,271 +44,564 @@ describe Canvadocs do
       @current_user = student
     end
 
-    describe 'parameters describing the current user' do
-      it 'includes the short name of the current user with commas removed' do
-        # This format for the name is in line with what we send in
-        # the canvadocs default user options.
-        expect(session_params[:user_name]).to eq 'Sev the Student'
+    context 'when passed an attachment' do
+      let(:session_params) { Canvadocs.user_session_params(@current_user, attachment: attachment) }
+
+      # We don't really want this behaviour long term, but that's the
+      # difference between sending an attachment and sending in the
+      # submission
+      it "returns empty hash if it can't find the submission starting with the attachment" do
+        submission.attachment_associations.destroy_all
+        expect(session_params).to be_empty
       end
 
-      it 'includes the real global ID of the current user' do
-        expect(session_params[:user_id]).to eq student.global_id.to_s
+      # We don't really want this behaviour long term, but that's the
+      # difference between sending an attachment and sending in the
+      # submission
+      it "returns empty if not passed an attachment or a submission" do
+        expect(Canvadocs.user_session_params(@current_user)).to be_empty
       end
 
-      context 'when a student is viewing' do
-        it 'includes a user_role of "student"' do
-          expect(session_params[:user_role]).to eq 'student'
+      describe 'parameters describing the current user' do
+        it 'includes the short name of the current user with commas removed' do
+          # This format for the name is in line with what we send in
+          # the canvadocs default user options.
+          expect(session_params[:user_name]).to eq 'Sev the Student'
         end
 
-        it 'includes the anonymous ID of the student' do
-          expect(session_params[:user_anonymous_id]).to eq submission.anonymous_id
-        end
-      end
-
-      context 'when a grader is viewing' do
-        before(:each) do
-          @current_user = teacher
+        it 'includes the real global ID of the current user' do
+          expect(session_params[:user_id]).to eq student.global_id.to_s
         end
 
-        it 'includes a user_role that is based on the user enrollment type' do
-          expect(session_params[:user_role]).to eq 'teacher'
+        context 'when a student is viewing' do
+          it 'includes a user_role of "student"' do
+            expect(session_params[:user_role]).to eq 'student'
+          end
+
+          it 'includes the anonymous ID of the student' do
+            expect(session_params[:user_anonymous_id]).to eq submission.anonymous_id
+          end
         end
 
-        it 'does not include an anonymous ID if the assignment is not moderated' do
-          assignment.update!(moderated_grading: false)
-          expect(session_params).not_to include(:user_anonymous_id)
-        end
-
-        context 'when the assignment is moderated' do
+        context 'when a grader is viewing' do
           before(:each) do
-            assignment.update!(moderated_grading: true, final_grader: teacher, grader_count: 1)
+            @current_user = teacher
           end
 
-          it 'includes the anonymous ID of the grader when the grader has taken a slot' do
-            assignment.moderation_graders.create!(user: teacher, anonymous_id: 'abcde', slot_taken: true)
-            expect(session_params[:user_anonymous_id]).to eq 'abcde'
+          it 'includes a user_role that is based on the user enrollment type' do
+            expect(session_params[:user_role]).to eq 'teacher'
           end
 
-          it 'includes the anonymous ID of the grader when the grader has not taken a slot' do
-            assignment.moderation_graders.create!(user: teacher, anonymous_id: 'abcde', slot_taken: false)
-            expect(session_params[:user_anonymous_id]).to eq 'abcde'
+          it 'does not include an anonymous ID if the assignment is not moderated' do
+            assignment.update!(moderated_grading: false)
+            expect(session_params).not_to include(:user_anonymous_id)
           end
 
-          it 'does not include the anonymous ID if the grader does not have a moderation_grader record' do
-            expect(session_params[:user_anonymous_id]).to be nil
+          context 'when the assignment is moderated' do
+            before(:each) do
+              assignment.update!(moderated_grading: true, final_grader: teacher, grader_count: 1)
+            end
+
+            it 'includes the anonymous ID of the grader when the grader has taken a slot' do
+              assignment.moderation_graders.create!(user: teacher, anonymous_id: 'abcde', slot_taken: true)
+              expect(session_params[:user_anonymous_id]).to eq 'abcde'
+            end
+
+            it 'includes the anonymous ID of the grader when the grader has not taken a slot' do
+              assignment.moderation_graders.create!(user: teacher, anonymous_id: 'abcde', slot_taken: false)
+              expect(session_params[:user_anonymous_id]).to eq 'abcde'
+            end
+
+            it 'does not include the anonymous ID if the grader does not have a moderation_grader record' do
+              expect(session_params[:user_anonymous_id]).to be nil
+            end
+          end
+        end
+      end
+
+      describe 'user filter' do
+        let(:student_real_data) { {type: 'real', role: 'student', id: student.global_id.to_s, name: 'Sev the Student'} }
+        let(:student_anonymous_data) { hash_including(type: 'anonymous', role: 'student', id: submission.anonymous_id) }
+        let(:teacher_real_data) { {type: 'real', role: 'teacher', id: teacher.global_id.to_s, name: 'Gise the Grader'} }
+
+        context 'for an unmoderated anonymized assignment' do
+          before(:each) do
+            assignment.update!(anonymous_grading: true, muted: true)
+          end
+
+          context 'when a student is viewing' do
+            before(:each) do
+              @current_user = student
+            end
+
+            it 'includes only the current user' do
+              @current_user = student
+              expect(user_filter).to include(student_real_data)
+            end
+
+            it 'requests that all returned annotations belong to users in the user_filter' do
+              expect(session_params[:restrict_annotations_to_user_filter]).to be true
+            end
+          end
+
+          context 'when a teacher is viewing' do
+            before(:each) do
+              @current_user = teacher
+            end
+
+            it 'includes anonymous information for the student' do
+              expect(user_filter).to include(student_anonymous_data)
+            end
+
+            it 'does not request that all returned annotations belong to users in the user_filter' do
+              expect(session_params[:restrict_annotations_to_user_filter]).to be false
+            end
+          end
+        end
+
+        context 'for an unmoderated non-anonymized assignment' do
+          before(:each) do
+            assignment.update!(anonymous_grading: false, muted: false)
+          end
+
+          context 'when a student is viewing' do
+            before(:each) do
+              @current_user = student
+            end
+
+            it 'omits the user filter entirely when a student is viewing' do
+              expect(session_params).not_to include(:user_filter)
+            end
+
+            it 'omits the restrict_annotations_to_user_filter entirely' do
+              expect(session_params).not_to have_key(:restrict_annotations_to_user_filter)
+            end
+          end
+
+          context 'when a teacher is viewing' do
+            before(:each) do
+              @current_user = teacher
+            end
+
+            it 'omits the user filter entirely' do
+              expect(session_params).not_to include(:user_filter)
+            end
+
+            it 'omits the restrict_annotations_to_user_filter entirely' do
+              expect(session_params).not_to have_key(:restrict_annotations_to_user_filter)
+            end
+          end
+        end
+
+        context 'for a moderated assignment' do
+          let(:final_grader) { teacher }
+          let(:final_grader_real_data) do
+            { type: 'real', role: 'teacher', id: teacher.global_id.to_s, name: 'Gise the Grader' }
+          end
+
+          let(:final_grader_anonymous_data) { hash_including(type: 'anonymous', role: 'teacher', id: 'qqqqq') }
+          let(:provisional_grader_anonymous_data) { hash_including(type: 'anonymous', role: 'ta', id: 'wwwww') }
+          let(:provisional_grader) { User.create!(name: 'Publius Provisional', short_name: 'Pub, the Prov') }
+          let(:provisional_grader_real_data) do
+            { type: 'real', role: 'ta', id: provisional_grader.global_id.to_s, name: 'Pub the Prov' }
+          end
+
+          before(:each) do
+            assignment.update!(moderated_grading: true, final_grader: final_grader, grader_count: 1)
+            assignment.moderation_graders.create!(user: final_grader, anonymous_id: 'qqqqq')
+            course.enroll_ta(provisional_grader).accept(true)
+            assignment.moderation_graders.create!(user: provisional_grader, anonymous_id: 'wwwww')
+          end
+
+          context 'when a student is viewing' do
+            before(:each) do
+              @current_user = student
+            end
+
+            it 'includes the student' do
+              expect(user_filter).to include(student_real_data)
+            end
+
+            it 'excludes graders if grades are not published' do
+              expect(user_filter).not_to include(hash_including(role: 'teacher'))
+            end
+
+            it 'includes the selected grader if grades are published' do
+              submission.update!(grader: provisional_grader)
+              attachment.associate_with(submission)
+              assignment.update!(grades_published_at: Time.zone.now)
+              expect(user_filter).to include(provisional_grader_real_data)
+            end
+
+            it 'excludes graders that were not selected if grades are published' do
+              submission.update!(grader: provisional_grader)
+              attachment.associate_with(submission)
+              assignment.update!(grades_published_at: Time.zone.now)
+              expect(user_filter).not_to include(final_grader_real_data)
+            end
+
+            it 'requests that all returned annotations belong to users in the user_filter' do
+              expect(session_params[:restrict_annotations_to_user_filter]).to be true
+            end
+          end
+
+          context 'when a provisional grader is viewing' do
+            before(:each) do
+              @current_user = provisional_grader
+            end
+
+            it 'includes real data for the current grader' do
+              expect(user_filter).to include(provisional_grader_real_data)
+            end
+
+            it 'includes anonymous student data when anonymizing students' do
+              assignment.update!(anonymous_grading: true, muted: true)
+              expect(user_filter).to include(student_anonymous_data)
+            end
+
+            it 'returns an anonymous name for the student when anonymizing students' do
+              assignment.update!(anonymous_grading: true, muted: true)
+              student_entry = user_filter.find { |entry| entry[:role] == 'student' }
+              expect(student_entry[:name]).to eq 'Student'
+            end
+
+            it 'includes real student data when not anonymizing students' do
+              assignment.update!(anonymous_grading: false, muted: false)
+              expect(user_filter).to include(student_real_data)
+            end
+
+            it 'includes real grader data when showing identities of other graders' do
+              assignment.update!(grader_comments_visible_to_graders: true, graders_anonymous_to_graders: false)
+              expect(user_filter).to include(final_grader_real_data)
+            end
+
+            it 'includes anonymous grader data when hiding identities of other graders' do
+              assignment.update!(grader_comments_visible_to_graders: true, graders_anonymous_to_graders: true)
+              expect(user_filter).to include(final_grader_anonymous_data)
+            end
+
+            it 'returns names in the format "Grader #" for graders whose identities are hidden' do
+              assignment.update!(grader_comments_visible_to_graders: true, graders_anonymous_to_graders: true)
+              final_grader_entry = user_filter.find { |entry| entry[:id] == 'qqqqq' }
+              expect(final_grader_entry[:name]).to eq 'Grader 1'
+            end
+
+            it 'omits other graders when comments from other graders are hidden' do
+              assignment.update!(grader_comments_visible_to_graders: false)
+              user_filter_ids = user_filter.map { |entry| entry[:id] }
+              expect(user_filter_ids).to match_array([student.global_id.to_s, provisional_grader.global_id.to_s])
+            end
+
+            it 'requests that all returned annotations belong to users in the user_filter' do
+              expect(session_params[:restrict_annotations_to_user_filter]).to be true
+            end
+          end
+
+          context 'when the final grader is viewing' do
+            before(:each) do
+              @current_user = final_grader
+            end
+
+            it 'includes real grader data when grader names are visible to the final grader' do
+              assignment.update!(grader_names_visible_to_final_grader: true)
+              expect(user_filter).to include(provisional_grader_real_data)
+            end
+
+            it 'includes anonymous grader data when grader names are not visible to the final grader' do
+              assignment.update!(grader_names_visible_to_final_grader: false)
+              expect(user_filter).to include(provisional_grader_anonymous_data)
+            end
+
+            it 'returns names in the format "Grader #" for graders not visible to the final grader' do
+              assignment.update!(grader_names_visible_to_final_grader: false)
+              final_grader_entry = user_filter.find { |entry| entry[:id] == 'wwwww' }
+              expect(final_grader_entry[:name]).to eq 'Grader 2'
+            end
+
+            it 'always includes other graders when the final grader is viewing' do
+              assignment.update!(grader_comments_visible_to_graders: false)
+              user_filter_ids = user_filter.map { |entry| entry[:id] }
+              expected_ids = [final_grader.global_id.to_s, provisional_grader.global_id.to_s, student.global_id.to_s]
+              expect(user_filter_ids).to match_array(expected_ids)
+            end
+
+            it 'requests that all returned annotations belong to users in the user_filter' do
+              expect(session_params[:restrict_annotations_to_user_filter]).to be true
+            end
           end
         end
       end
     end
 
-    describe 'user filter' do
-      let(:student_real_data) { {type: 'real', role: 'student', id: student.global_id.to_s, name: 'Sev the Student'} }
-      let(:student_anonymous_data) { hash_including(type: 'anonymous', role: 'student', id: submission.anonymous_id) }
-      let(:teacher_real_data) { {type: 'real', role: 'teacher', id: teacher.global_id.to_s, name: 'Gise the Grader'} }
+    context 'when passed a submission' do
+      let(:session_params) { Canvadocs.user_session_params(@current_user, submission: submission) }
 
-      context 'for an unmoderated anonymized assignment' do
-        before(:each) do
-          assignment.update!(anonymous_grading: true, muted: true)
+      describe 'parameters describing the current user' do
+        it 'includes the short name of the current user with commas removed' do
+          # This format for the name is in line with what we send in
+          # the canvadocs default user options.
+          expect(session_params[:user_name]).to eq 'Sev the Student'
+        end
+
+        it 'includes the real global ID of the current user' do
+          expect(session_params[:user_id]).to eq student.global_id.to_s
         end
 
         context 'when a student is viewing' do
-          before(:each) do
-            @current_user = student
+          it 'includes a user_role of "student"' do
+            expect(session_params[:user_role]).to eq 'student'
           end
 
-          it 'includes only the current user' do
-            @current_user = student
-            expect(user_filter).to include(student_real_data)
-          end
-
-          it 'requests that all returned annotations belong to users in the user_filter' do
-            expect(session_params[:restrict_annotations_to_user_filter]).to be true
+          it 'includes the anonymous ID of the student' do
+            expect(session_params[:user_anonymous_id]).to eq submission.anonymous_id
           end
         end
 
-        context 'when a teacher is viewing' do
+        context 'when a grader is viewing' do
           before(:each) do
             @current_user = teacher
           end
 
-          it 'includes anonymous information for the student' do
-            expect(user_filter).to include(student_anonymous_data)
+          it 'includes a user_role that is based on the user enrollment type' do
+            expect(session_params[:user_role]).to eq 'teacher'
           end
 
-          it 'does not request that all returned annotations belong to users in the user_filter' do
-            expect(session_params[:restrict_annotations_to_user_filter]).to be false
+          it 'does not include an anonymous ID if the assignment is not moderated' do
+            assignment.update!(moderated_grading: false)
+            expect(session_params).not_to include(:user_anonymous_id)
+          end
+
+          context 'when the assignment is moderated' do
+            before(:each) do
+              assignment.update!(moderated_grading: true, final_grader: teacher, grader_count: 1)
+            end
+
+            it 'includes the anonymous ID of the grader when the grader has taken a slot' do
+              assignment.moderation_graders.create!(user: teacher, anonymous_id: 'abcde', slot_taken: true)
+              expect(session_params[:user_anonymous_id]).to eq 'abcde'
+            end
+
+            it 'includes the anonymous ID of the grader when the grader has not taken a slot' do
+              assignment.moderation_graders.create!(user: teacher, anonymous_id: 'abcde', slot_taken: false)
+              expect(session_params[:user_anonymous_id]).to eq 'abcde'
+            end
+
+            it 'does not include the anonymous ID if the grader does not have a moderation_grader record' do
+              expect(session_params[:user_anonymous_id]).to be nil
+            end
           end
         end
       end
 
-      context 'for an unmoderated non-anonymized assignment' do
-        before(:each) do
-          assignment.update!(anonymous_grading: false, muted: false)
-        end
+      describe 'user filter' do
+        let(:student_real_data) { {type: 'real', role: 'student', id: student.global_id.to_s, name: 'Sev the Student'} }
+        let(:student_anonymous_data) { hash_including(type: 'anonymous', role: 'student', id: submission.anonymous_id) }
+        let(:teacher_real_data) { {type: 'real', role: 'teacher', id: teacher.global_id.to_s, name: 'Gise the Grader'} }
 
-        context 'when a student is viewing' do
+        context 'for an unmoderated anonymized assignment' do
           before(:each) do
-            @current_user = student
-          end
-
-          it 'omits the user filter entirely when a student is viewing' do
-            expect(session_params).not_to include(:user_filter)
-          end
-
-          it 'omits the restrict_annotations_to_user_filter entirely' do
-            expect(session_params).not_to have_key(:restrict_annotations_to_user_filter)
-          end
-        end
-
-        context 'when a teacher is viewing' do
-          before(:each) do
-            @current_user = teacher
-          end
-
-          it 'omits the user filter entirely' do
-            expect(session_params).not_to include(:user_filter)
-          end
-
-          it 'omits the restrict_annotations_to_user_filter entirely' do
-            expect(session_params).not_to have_key(:restrict_annotations_to_user_filter)
-          end
-        end
-      end
-
-      context 'for a moderated assignment' do
-        let(:final_grader) { teacher }
-        let(:final_grader_real_data) do
-          { type: 'real', role: 'teacher', id: teacher.global_id.to_s, name: 'Gise the Grader' }
-        end
-
-        let(:final_grader_anonymous_data) { hash_including(type: 'anonymous', role: 'teacher', id: 'qqqqq') }
-        let(:provisional_grader_anonymous_data) { hash_including(type: 'anonymous', role: 'ta', id: 'wwwww') }
-        let(:provisional_grader) { User.create!(name: 'Publius Provisional', short_name: 'Pub, the Prov') }
-        let(:provisional_grader_real_data) do
-          { type: 'real', role: 'ta', id: provisional_grader.global_id.to_s, name: 'Pub the Prov' }
-        end
-
-        before(:each) do
-          assignment.update!(moderated_grading: true, final_grader: final_grader, grader_count: 1)
-          assignment.moderation_graders.create!(user: final_grader, anonymous_id: 'qqqqq')
-          course.enroll_ta(provisional_grader).accept(true)
-          assignment.moderation_graders.create!(user: provisional_grader, anonymous_id: 'wwwww')
-        end
-
-        context 'when a student is viewing' do
-          before(:each) do
-            @current_user = student
-          end
-
-          it 'includes the student' do
-            expect(user_filter).to include(student_real_data)
-          end
-
-          it 'excludes graders if grades are not published' do
-            expect(user_filter).not_to include(hash_including(role: 'teacher'))
-          end
-
-          it 'includes the selected grader if grades are published' do
-            submission.update!(grader: provisional_grader)
-            attachment.associate_with(submission)
-            assignment.update!(grades_published_at: Time.zone.now)
-            expect(user_filter).to include(provisional_grader_real_data)
-          end
-
-          it 'excludes graders that were not selected if grades are published' do
-            submission.update!(grader: provisional_grader)
-            attachment.associate_with(submission)
-            assignment.update!(grades_published_at: Time.zone.now)
-            expect(user_filter).not_to include(final_grader_real_data)
-          end
-
-          it 'requests that all returned annotations belong to users in the user_filter' do
-            expect(session_params[:restrict_annotations_to_user_filter]).to be true
-          end
-        end
-
-        context 'when a provisional grader is viewing' do
-          before(:each) do
-            @current_user = provisional_grader
-          end
-
-          it 'includes real data for the current grader' do
-            expect(user_filter).to include(provisional_grader_real_data)
-          end
-
-          it 'includes anonymous student data when anonymizing students' do
             assignment.update!(anonymous_grading: true, muted: true)
-            expect(user_filter).to include(student_anonymous_data)
           end
 
-          it 'returns an anonymous name for the student when anonymizing students' do
-            assignment.update!(anonymous_grading: true, muted: true)
-            student_entry = user_filter.find { |entry| entry[:role] == 'student' }
-            expect(student_entry[:name]).to eq 'Student'
+          context 'when a student is viewing' do
+            before(:each) do
+              @current_user = student
+            end
+
+            it 'includes only the current user' do
+              @current_user = student
+              expect(user_filter).to include(student_real_data)
+            end
+
+            it 'requests that all returned annotations belong to users in the user_filter' do
+              expect(session_params[:restrict_annotations_to_user_filter]).to be true
+            end
           end
 
-          it 'includes real student data when not anonymizing students' do
+          context 'when a teacher is viewing' do
+            before(:each) do
+              @current_user = teacher
+            end
+
+            it 'includes anonymous information for the student' do
+              expect(user_filter).to include(student_anonymous_data)
+            end
+
+            it 'does not request that all returned annotations belong to users in the user_filter' do
+              expect(session_params[:restrict_annotations_to_user_filter]).to be false
+            end
+          end
+        end
+
+        context 'for an unmoderated non-anonymized assignment' do
+          before(:each) do
             assignment.update!(anonymous_grading: false, muted: false)
-            expect(user_filter).to include(student_real_data)
           end
 
-          it 'includes real grader data when showing identities of other graders' do
-            assignment.update!(grader_comments_visible_to_graders: true, graders_anonymous_to_graders: false)
-            expect(user_filter).to include(final_grader_real_data)
+          context 'when a student is viewing' do
+            before(:each) do
+              @current_user = student
+            end
+
+            it 'omits the user filter entirely when a student is viewing' do
+              expect(session_params).not_to include(:user_filter)
+            end
+
+            it 'omits the restrict_annotations_to_user_filter entirely' do
+              expect(session_params).not_to have_key(:restrict_annotations_to_user_filter)
+            end
           end
 
-          it 'includes anonymous grader data when hiding identities of other graders' do
-            assignment.update!(grader_comments_visible_to_graders: true, graders_anonymous_to_graders: true)
-            expect(user_filter).to include(final_grader_anonymous_data)
-          end
+          context 'when a teacher is viewing' do
+            before(:each) do
+              @current_user = teacher
+            end
 
-          it 'returns names in the format "Grader #" for graders whose identities are hidden' do
-            assignment.update!(grader_comments_visible_to_graders: true, graders_anonymous_to_graders: true)
-            final_grader_entry = user_filter.find { |entry| entry[:id] == 'qqqqq' }
-            expect(final_grader_entry[:name]).to eq 'Grader 1'
-          end
+            it 'omits the user filter entirely' do
+              expect(session_params).not_to include(:user_filter)
+            end
 
-          it 'omits other graders when comments from other graders are hidden' do
-            assignment.update!(grader_comments_visible_to_graders: false)
-            user_filter_ids = user_filter.map { |entry| entry[:id] }
-            expect(user_filter_ids).to match_array([student.global_id.to_s, provisional_grader.global_id.to_s])
-          end
-
-          it 'requests that all returned annotations belong to users in the user_filter' do
-            expect(session_params[:restrict_annotations_to_user_filter]).to be true
+            it 'omits the restrict_annotations_to_user_filter entirely' do
+              expect(session_params).not_to have_key(:restrict_annotations_to_user_filter)
+            end
           end
         end
 
-        context 'when the final grader is viewing' do
+        context 'for a moderated assignment' do
+          let(:final_grader) { teacher }
+          let(:final_grader_real_data) do
+            { type: 'real', role: 'teacher', id: teacher.global_id.to_s, name: 'Gise the Grader' }
+          end
+
+          let(:final_grader_anonymous_data) { hash_including(type: 'anonymous', role: 'teacher', id: 'qqqqq') }
+          let(:provisional_grader_anonymous_data) { hash_including(type: 'anonymous', role: 'ta', id: 'wwwww') }
+          let(:provisional_grader) { User.create!(name: 'Publius Provisional', short_name: 'Pub, the Prov') }
+          let(:provisional_grader_real_data) do
+            { type: 'real', role: 'ta', id: provisional_grader.global_id.to_s, name: 'Pub the Prov' }
+          end
+
           before(:each) do
-            @current_user = final_grader
+            assignment.update!(moderated_grading: true, final_grader: final_grader, grader_count: 1)
+            assignment.moderation_graders.create!(user: final_grader, anonymous_id: 'qqqqq')
+            course.enroll_ta(provisional_grader).accept(true)
+            assignment.moderation_graders.create!(user: provisional_grader, anonymous_id: 'wwwww')
           end
 
-          it 'includes real grader data when grader names are visible to the final grader' do
-            assignment.update!(grader_names_visible_to_final_grader: true)
-            expect(user_filter).to include(provisional_grader_real_data)
+          context 'when a student is viewing' do
+            before(:each) do
+              @current_user = student
+            end
+
+            it 'includes the student' do
+              expect(user_filter).to include(student_real_data)
+            end
+
+            it 'excludes graders if grades are not published' do
+              expect(user_filter).not_to include(hash_including(role: 'teacher'))
+            end
+
+            it 'includes the selected grader if grades are published' do
+              submission.update!(grader: provisional_grader)
+              attachment.associate_with(submission)
+              assignment.update!(grades_published_at: Time.zone.now)
+              expect(user_filter).to include(provisional_grader_real_data)
+            end
+
+            it 'excludes graders that were not selected if grades are published' do
+              submission.update!(grader: provisional_grader)
+              attachment.associate_with(submission)
+              assignment.update!(grades_published_at: Time.zone.now)
+              expect(user_filter).not_to include(final_grader_real_data)
+            end
+
+            it 'requests that all returned annotations belong to users in the user_filter' do
+              expect(session_params[:restrict_annotations_to_user_filter]).to be true
+            end
           end
 
-          it 'includes anonymous grader data when grader names are not visible to the final grader' do
-            assignment.update!(grader_names_visible_to_final_grader: false)
-            expect(user_filter).to include(provisional_grader_anonymous_data)
+          context 'when a provisional grader is viewing' do
+            before(:each) do
+              @current_user = provisional_grader
+            end
+
+            it 'includes real data for the current grader' do
+              expect(user_filter).to include(provisional_grader_real_data)
+            end
+
+            it 'includes anonymous student data when anonymizing students' do
+              assignment.update!(anonymous_grading: true, muted: true)
+              expect(user_filter).to include(student_anonymous_data)
+            end
+
+            it 'returns an anonymous name for the student when anonymizing students' do
+              assignment.update!(anonymous_grading: true, muted: true)
+              student_entry = user_filter.find { |entry| entry[:role] == 'student' }
+              expect(student_entry[:name]).to eq 'Student'
+            end
+
+            it 'includes real student data when not anonymizing students' do
+              assignment.update!(anonymous_grading: false, muted: false)
+              expect(user_filter).to include(student_real_data)
+            end
+
+            it 'includes real grader data when showing identities of other graders' do
+              assignment.update!(grader_comments_visible_to_graders: true, graders_anonymous_to_graders: false)
+              expect(user_filter).to include(final_grader_real_data)
+            end
+
+            it 'includes anonymous grader data when hiding identities of other graders' do
+              assignment.update!(grader_comments_visible_to_graders: true, graders_anonymous_to_graders: true)
+              expect(user_filter).to include(final_grader_anonymous_data)
+            end
+
+            it 'returns names in the format "Grader #" for graders whose identities are hidden' do
+              assignment.update!(grader_comments_visible_to_graders: true, graders_anonymous_to_graders: true)
+              final_grader_entry = user_filter.find { |entry| entry[:id] == 'qqqqq' }
+              expect(final_grader_entry[:name]).to eq 'Grader 1'
+            end
+
+            it 'omits other graders when comments from other graders are hidden' do
+              assignment.update!(grader_comments_visible_to_graders: false)
+              user_filter_ids = user_filter.map { |entry| entry[:id] }
+              expect(user_filter_ids).to match_array([student.global_id.to_s, provisional_grader.global_id.to_s])
+            end
+
+            it 'requests that all returned annotations belong to users in the user_filter' do
+              expect(session_params[:restrict_annotations_to_user_filter]).to be true
+            end
           end
 
-          it 'returns names in the format "Grader #" for graders not visible to the final grader' do
-            assignment.update!(grader_names_visible_to_final_grader: false)
-            final_grader_entry = user_filter.find { |entry| entry[:id] == 'wwwww' }
-            expect(final_grader_entry[:name]).to eq 'Grader 2'
-          end
+          context 'when the final grader is viewing' do
+            before(:each) do
+              @current_user = final_grader
+            end
 
-          it 'always includes other graders when the final grader is viewing' do
-            assignment.update!(grader_comments_visible_to_graders: false)
-            user_filter_ids = user_filter.map { |entry| entry[:id] }
-            expected_ids = [final_grader.global_id.to_s, provisional_grader.global_id.to_s, student.global_id.to_s]
-            expect(user_filter_ids).to match_array(expected_ids)
-          end
+            it 'includes real grader data when grader names are visible to the final grader' do
+              assignment.update!(grader_names_visible_to_final_grader: true)
+              expect(user_filter).to include(provisional_grader_real_data)
+            end
 
-          it 'requests that all returned annotations belong to users in the user_filter' do
-            expect(session_params[:restrict_annotations_to_user_filter]).to be true
+            it 'includes anonymous grader data when grader names are not visible to the final grader' do
+              assignment.update!(grader_names_visible_to_final_grader: false)
+              expect(user_filter).to include(provisional_grader_anonymous_data)
+            end
+
+            it 'returns names in the format "Grader #" for graders not visible to the final grader' do
+              assignment.update!(grader_names_visible_to_final_grader: false)
+              final_grader_entry = user_filter.find { |entry| entry[:id] == 'wwwww' }
+              expect(final_grader_entry[:name]).to eq 'Grader 2'
+            end
+
+            it 'always includes other graders when the final grader is viewing' do
+              assignment.update!(grader_comments_visible_to_graders: false)
+              user_filter_ids = user_filter.map { |entry| entry[:id] }
+              expected_ids = [final_grader.global_id.to_s, provisional_grader.global_id.to_s, student.global_id.to_s]
+              expect(user_filter_ids).to match_array(expected_ids)
+            end
+
+            it 'requests that all returned annotations belong to users in the user_filter' do
+              expect(session_params[:restrict_annotations_to_user_filter]).to be true
+            end
           end
         end
       end