disable SpeedGrader if all the moderated grader spots are taken

fixes GRADE-1060

test plan:
* Create a course in an account with AMM on
* Add one student
* Add 3 teachers to the course
* Create a moderated assignment with a grader count of 1 and make
  teacher 3 the final grader
* Masquerade as teacher 1 and grade the student in SpeedGrader
* Masquerade as teacher 2 and try to access speed grader for the
  moderated assignment. You should not be able to access SpeedGrader
* Masquerade as teacher 3 and grade the student in SpeedGrader.
  It should work

Change-Id: I1db39700d9ebe59c21129061caddae31d29606c9
Reviewed-on: https://gerrit.instructure.com/150485
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Tested-by: Jenkins
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
QA-Review: Keith T. Garner <kgarner@instructure.com>
Product-Review: Sidharth Oberoi <soberoi@instructure.com>

app/controllers/discussion_topics_controller.rb

@@ -717,8 +717,8 @@ class DiscussionTopicsController < ApplicationController
                 :COURSE_ID => @sequence_asset.context.id,
               }
             end
-            if @topic.for_assignment? &&
-               @topic.assignment.grants_right?(@current_user, session, :grade) && @presenter.allows_speed_grader?
+            if @topic.for_assignment? && @presenter.allows_speed_grader? &&
+              @topic.assignment.can_view_speed_grader?(@current_user)
               env_hash[:SPEEDGRADER_URL_TEMPLATE] = named_context_url(@topic.assignment.context,
                                                                       :speed_grader_context_gradebook_url,
                                                                       :assignment_id => @topic.assignment.id,

app/controllers/gradebooks_controller.rb

@@ -574,6 +574,12 @@ class GradebooksController < ApplicationController
     return unless authorized_action(@context, @current_user, [:manage_grades, :view_all_grades])
 
     @assignment = @context.assignments.active.find(params[:assignment_id])
+
+    unless @assignment.can_view_speed_grader?(@current_user)
+      flash[:notice] = t('The maximum number of graders for this assignment has been reached.')
+      return redirect_to(course_gradebook_path(@context))
+    end
+
     if @assignment.unpublished?
       flash[:notice] = t(:speedgrader_enabled_only_for_published_content,
                          'SpeedGrader is enabled only for published content.')

app/models/assignment.rb

@@ -2792,6 +2792,21 @@ class Assignment < ActiveRecord::Base
     [grader_count, max_course_count].max
   end
 
+  def moderated_grader_limit_reached?
+    moderated_grading? && anonymous_moderated_marking? && moderation_graders.count >= grader_count
+  end
+
+  def can_be_moderated_grader?(user)
+    return false unless context.grants_any_right?(user, :manage_grades, :view_all_grades)
+    return true unless moderated_grader_limit_reached?
+    # Final grader can always be a moderated grader, and existing moderated graders can re-grade
+    final_grader_id == user.id || moderation_graders.where(user_id: user.id).exists?
+  end
+
+  def can_view_speed_grader?(user)
+    context.allows_speed_grader? && can_be_moderated_grader?(user)
+  end
+
   def can_view_other_grader_identities?(user)
     return false unless context.grants_any_right?(user, :manage_grades, :view_all_grades)
     return true unless anonymous_moderated_marking? && moderated_grading?

app/serializers/quizzes/quiz_serializer.rb

@@ -317,7 +317,7 @@ module Quizzes
     private
 
     def show_speedgrader?
-      quiz.assignment.present? && quiz.published? && context.allows_speed_grader?
+      quiz.assignment.present? && quiz.published? && quiz.assignment.can_view_speed_grader?(current_user)
     end
 
     def quiz_locked_for_user?

app/serializers/quizzes/quiz_statistics_serializer.rb

@@ -166,7 +166,7 @@ module Quizzes
     private
 
     def show_speed_grader?
-      quiz.assignment.present? && quiz.published? && context.allows_speed_grader?
+      quiz.assignment.present? && quiz.published? && quiz.assignment.can_view_speed_grader?(current_user)
     end
 
     def student_analysis_report

app/views/assignments/_grade_assignment.html.erb

@@ -18,7 +18,8 @@
 
 <ul class='page-action-list'>
   <h2><%= t 'titles.related_items', "Related Items" %></h2>
-  <% if (@can_view_grades || @can_grade || (@context.concluded? && can_do(@context, @current_user, :read_as_admin))) && @context.allows_speed_grader? %>
+  <% if (@can_view_grades || @can_grade || (@context.concluded? && can_do(@context, @current_user, :read_as_admin))) &&
+    @assignment.can_view_speed_grader?(@current_user) %>
     <li id="assignment-speedgrader-link"
       <% if @assignment.unpublished? %>
         class="hidden"

app/views/discussion_topics/show.html.erb

@@ -127,7 +127,7 @@
                   <li><a href="<%= context_url(@context, :context_discussion_topic_url, @topic.id) %>" class="delete_discussion" data-method="delete" rel="nofollow" data-confirm="<%= t :confirm_delete_discussion, 'Are you sure you want to delete this discussion?' %>"><i class="icon-trash"></i> <%= t :delete, 'Delete' %></a></li>
                 <% end %>
 
-                <% if @presenter.can_grade?(@current_user) && @presenter.allows_speed_grader? %>
+                <% if @presenter.can_grade?(@current_user) && @presenter.allows_speed_grader? && @topic.assignment.can_view_speed_grader?(@current_user) %>
                   <li><a href="<%= context_url(@topic.assignment.context,
                                                :speed_grader_context_gradebook_url,
                                                :assignment_id => @topic.assignment.id) %>"

app/views/quizzes/quizzes/show.html.erb

@@ -81,7 +81,8 @@
           </li>
         <% end %>
 
-        <% if (can_do(@quiz, @current_user, :grade) || can_do(@quiz, @current_user, :review_grades)) && @quiz.available? && @quiz.assignment && @context.allows_speed_grader? && @quiz.published? %>
+        <% if (can_do(@quiz, @current_user, :grade) || can_do(@quiz, @current_user, :review_grades)) && @quiz.available? &&
+          @quiz.published? && !!@quiz.assignment&.can_view_speed_grader?(@current_user) %>
           <% speed_grader_title = t(:speed_grader, "SpeedGrader™") %>
           <li>
             <a target="_blank" href="<%= context_url(@context, :speed_grader_context_gradebook_url, :assignment_id => @quiz.assignment.id) %>">

spec/controllers/discussion_topics_controller_spec.rb

@@ -431,6 +431,14 @@ describe DiscussionTopicsController do
       expect(assigns[:js_env][:DISCUSSION][:SPEEDGRADER_URL_TEMPLATE]).to be_nil
     end
 
+    it "should hide speedgrader when moderated graders limit is reached" do
+      user_session(@teacher)
+      course_topic(user: @teacher, with_assignment: true)
+      allow_any_instance_of(Assignment).to receive(:can_be_moderated_grader?).and_return(false)
+      get 'show', params: {:course_id => @course.id, :id => @topic.id}
+      expect(assigns[:js_env][:DISCUSSION][:SPEEDGRADER_URL_TEMPLATE]).to be_nil
+    end
+
     it "should setup speedgrader template for variable substitution" do
       user_session(@teacher)
       course_topic(user: @teacher, with_assignment: true)

spec/controllers/gradebooks_controller_spec.rb

@@ -1484,6 +1484,14 @@ describe GradebooksController do
       expect(flash[:notice]).to eq 'SpeedGrader is disabled for this course'
     end
 
+    it "redirects if the assignment's moderated grader limit is reached" do
+      allow_any_instance_of(Assignment).to receive(:moderated_grader_limit_reached?).and_return(true)
+
+      get 'speed_grader', params: {:course_id => @course.id, :assignment_id => @assignment.id}
+      expect(response).to be_redirect
+      expect(flash[:notice]).to eq 'The maximum number of graders for this assignment has been reached.'
+    end
+
     it "redirects if the assignment is unpublished" do
       @assignment.unpublish
       get 'speed_grader', params: {course_id: @course, assignment_id: @assignment.id}

spec/integration/quizzes_spec.rb

@@ -114,6 +114,12 @@ describe Quizzes::QuizzesController do
         get "/courses/#{@course.id}/quizzes/#{@quiz.id}"
         expect(response.body).not_to match(%r{SpeedGrader})
       end
+
+      it "should not link to SpeedGrader when moderated grader limit is reached" do
+        allow_any_instance_of(Assignment).to receive(:can_view_speed_grader?).and_return(false)
+        get "/courses/#{@course.id}/quizzes/#{@quiz.id}"
+        expect(response.body).not_to match(%r{SpeedGrader})
+      end
     end
   end
 

spec/models/assignment_spec.rb

@@ -6144,6 +6144,153 @@ describe Assignment do
     end
   end
 
+  describe '#moderated_grader_limit_reached?' do
+    before(:once) do
+      @course = Course.create!
+      teacher = User.create!
+      second_teacher = User.create!
+      @ta = User.create!
+      @course.enroll_teacher(teacher, enrollment_state: 'active')
+      @course.enroll_teacher(second_teacher, enrollment_state: 'active')
+      @course.enroll_ta(@ta, enrollment_state: 'active')
+      @assignment = @course.assignments.create!(
+        final_grader: teacher,
+        grader_count: 2,
+        moderated_grading: true
+      )
+      @assignment.moderation_graders.create!(user: second_teacher, anonymous_id: '12345')
+    end
+
+    it 'returns false if anonymous moderated marking is off' do
+      @assignment.moderation_graders.create!(user: @ta, anonymous_id: '54321')
+      expect(@assignment.moderated_grader_limit_reached?).to eq false
+    end
+
+    context 'when anonymous moderated marking is enabled' do
+      before :once do
+        @course.root_account.enable_feature!(:anonymous_moderated_marking)
+      end
+
+      it 'returns false if all grader slots are not filled' do
+        expect(@assignment.moderated_grader_limit_reached?).to eq false
+      end
+
+      it 'returns true if all grader slots are filled' do
+        @assignment.moderation_graders.create!(user: @ta, anonymous_id: '54321')
+        expect(@assignment.moderated_grader_limit_reached?).to eq true
+      end
+
+      it 'returns false if moderated grading is off' do
+        @assignment.moderation_graders.create!(user: @ta, anonymous_id: '54321')
+        @assignment.moderated_grading = false
+        expect(@assignment.moderated_grader_limit_reached?).to eq false
+      end
+    end
+  end
+
+  describe '#can_be_moderated_grader?' do
+    before(:once) do
+      @course = Course.create!
+      @teacher = User.create!
+      @second_teacher = User.create!
+      @final_teacher = User.create!
+      @course.enroll_teacher(@teacher, enrollment_state: 'active')
+      @course.enroll_teacher(@second_teacher, enrollment_state: 'active')
+      @course.enroll_teacher(@final_teacher, enrollment_state: 'active')
+      @assignment = @course.assignments.create!(
+        final_grader: @final_teacher,
+        grader_count: 2,
+        moderated_grading: true
+      )
+      @assignment.moderation_graders.create!(user: @second_teacher, anonymous_id: '12345')
+      @course.root_account.enable_feature!(:anonymous_moderated_marking)
+    end
+
+    shared_examples 'grader permissions are checked' do
+      it 'returns true when the user has default teacher permissions' do
+        expect(@assignment.can_be_moderated_grader?(@teacher)).to be true
+      end
+
+      it 'returns true when the user has permission to only manage grades' do
+        @course.root_account.role_overrides.create!(permission: 'manage_grades', enabled: true, role: teacher_role)
+        @course.root_account.role_overrides.create!(permission: 'view_all_grades', enabled: false, role: teacher_role)
+        expect(@assignment.can_be_moderated_grader?(@teacher)).to be true
+      end
+
+      it 'returns true when the user has permission to only view all grades' do
+        @course.root_account.role_overrides.create!(permission: 'manage_grades', enabled: false, role: teacher_role)
+        @course.root_account.role_overrides.create!(permission: 'view_all_grades', enabled: true, role: teacher_role)
+        expect(@assignment.can_be_moderated_grader?(@teacher)).to be true
+      end
+
+      it 'returns false when the user does not have sufficient privileges' do
+        @course.root_account.role_overrides.create!(permission: 'manage_grades', enabled: false, role: teacher_role)
+        @course.root_account.role_overrides.create!(permission: 'view_all_grades', enabled: false, role: teacher_role)
+        expect(@assignment.can_be_moderated_grader?(@teacher)).to be false
+      end
+    end
+
+    context 'when the assignment is not moderated' do
+      before :once do
+        @assignment.update!(moderated_grading: false)
+      end
+
+      it_behaves_like 'grader permissions are checked'
+    end
+
+    context 'when the assignment is moderated' do
+      it_behaves_like 'grader permissions are checked'
+
+      context 'and moderator limit is reached' do
+        before :once do
+          @assignment.update!(grader_count: 1)
+        end
+
+        it 'returns false' do
+          expect(@assignment.can_be_moderated_grader?(@teacher)).to be false
+        end
+
+        it 'returns true if user is one of the moderators' do
+          expect(@assignment.can_be_moderated_grader?(@second_teacher)).to be true
+        end
+
+        it 'returns true if user is the final grader' do
+          expect(@assignment.can_be_moderated_grader?(@final_teacher)).to be true
+        end
+      end
+    end
+  end
+
+  describe '#can_view_speed_grader?' do
+    before :once do
+      @course = Course.create!
+      @teacher = User.create!
+      @course.enroll_teacher(@teacher, enrollment_state: 'active')
+      @assignment = @course.assignments.create!(
+        final_grader: @teacher,
+        grader_count: 2,
+        moderated_grading: true
+      )
+    end
+
+    it 'returns false when the course does not allow speed grader' do
+      expect(@assignment.context).to receive(:allows_speed_grader?).and_return false
+      expect(@assignment.can_view_speed_grader?(@teacher)).to be false
+    end
+
+    it 'returns false when user cannot be moderated grader' do
+      expect(@assignment.context).to receive(:allows_speed_grader?).and_return true
+      expect(@assignment).to receive(:can_be_moderated_grader?).and_return false
+      expect(@assignment.can_view_speed_grader?(@teacher)).to be false
+    end
+
+    it 'returns true when the course allows speed grader and user can be grader' do
+      expect(@assignment.context).to receive(:allows_speed_grader?).and_return true
+      expect(@assignment).to receive(:can_be_moderated_grader?).and_return true
+      expect(@assignment.can_view_speed_grader?(@teacher)).to be true
+    end
+  end
+
   describe 'Anonymous Moderated Marking setting validation' do
     before(:once) do
       @course.account.enable_feature!(:anonymous_moderated_marking)

spec/presenters/discussion_topic_presenter_spec.rb

@@ -188,7 +188,6 @@ describe DiscussionTopicPresenter do
       it "returns true when assignment published" do
         expect(presenter.allows_speed_grader?).to eq true
       end
-
     end
   end
 

spec/serializers/quizzes/quiz_serializer_spec.rb

@@ -126,10 +126,10 @@ describe Quizzes::QuizSerializer do
 
     # nil when context doesn't allow speedgrader
     allow(@quiz).to receive(:published?).and_return true
-    expect(@context).to receive(:allows_speed_grader?).and_return false
+    expect(assignment).to receive(:can_view_speed_grader?).with(@user).and_return false
     expect(@serializer.as_json[:quiz][:speed_grader_url]).to be_nil
 
-    expect(@context).to receive(:allows_speed_grader?).and_return true
+    expect(assignment).to receive(:can_view_speed_grader?).with(@user).and_return true
     json = @serializer.as_json[:quiz]
     expect(json[:speed_grader_url]).to eq(
       controller.send(:speed_grader_course_gradebook_url, @quiz.context, assignment_id: @quiz.assignment.id)

spec/serializers/quizzes/quiz_statistics_serializer_spec.rb

@@ -26,9 +26,15 @@ describe Quizzes::QuizStatisticsSerializer do
     end
   end
 
+  let :assignment do
+    context.assignments.create!(title: 'quiz assignment')
+  end
+
   let :quiz do
     context.quizzes.build(title: 'banana split').tap do |quiz|
       quiz.id = 1
+      quiz.assignment = assignment
+      quiz.workflow_state = 'available'
       quiz.save!
     end
   end
@@ -117,6 +123,18 @@ describe Quizzes::QuizStatisticsSerializer do
     expect(@json['links']['quiz']).to eq 'http://example.com/api/v1/courses/1/quizzes/1'
   end
 
+  it 'serializes speed_grader url' do
+    allow(quiz.assignment).to receive(:can_view_speed_grader?).and_return true
+    expect(subject.as_json[:quiz_statistics][:speed_grader_url]).to eq(
+      controller.send(:speed_grader_course_gradebook_url, quiz.context, assignment_id: quiz.assignment.id)
+    )
+  end
+
+  it 'does not serialize speed_grader url if user cannot view speed grader' do
+    allow(quiz.assignment).to receive(:can_view_speed_grader?).and_return false
+    expect(subject.as_json[:quiz_statistics][:speed_grader_url]).to be_nil
+  end
+
   it 'stringifies question_statistics ids' do
     allow(subject).to receive_messages(student_analysis_report: {
       questions: [ ['question', { id: 5 }] ]