Allow SG rubric scoring for anonymous assignments

Allow graders to score rubrics for anonymous assignments in SpeedGrader,
using the anonymous ID in place of the user ID to communicate.

closes GRADE-1127

Test plan:
- Have a course with some teachers and students
- (if the Anonymous Moderated Marking feature flag still exists by
  the time you read this, enable it)
- Create an assignment with anonymous grading enabled, and a second
  assignment with anonymous grading disabled
- Attach a rubric to each assignment (doesn't have to be the same
  rubric)
- Open SpeedGrader and evaluate some students vis-à-vis the rubric
- Ensure that the above works correctly for both assignments, and that
  your updates are preserved on reloading the page

Change-Id: I18e467d04b333b5c30f21e43791178c3271ca59b
Reviewed-on: https://gerrit.instructure.com/152276
Tested-by: Jenkins
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
QA-Review: Jeremy Neander <jneander@instructure.com>
Product-Review: Sidharth Oberoi <soberoi@instructure.com>

app/controllers/rubric_assessments_controller.rb

@@ -41,10 +41,8 @@ class RubricAssessmentsController < ApplicationController
 
     # only check if there's no @assessment object, since that's the only time
     # this param matters (find_asset_for_assessment)
-    user_id = params[:rubric_assessment][:user_id]
-    if !@assessment && user_id !~ Api::ID_REGEX
-      raise ActiveRecord::RecordNotFound
-    end
+    user_id = @assessment.present? ? @assessment.user_id : resolve_user_id
+    raise ActiveRecord::RecordNotFound if user_id.blank?
 
     # Funky flow to avoid a double-render, re-work it if you like
     if @assessment && !authorized_action(@assessment, @current_user, :update)
@@ -56,7 +54,7 @@ class RubricAssessmentsController < ApplicationController
         opts[:final] = true if value_to_boolean(params[:final]) && @context.grants_right?(@current_user, :moderate_grades)
       end
 
-      @asset, @user = @association_object.find_asset_for_assessment(@association, @assessment ? @assessment.user_id : user_id, opts)
+      @asset, @user = @association_object.find_asset_for_assessment(@association, user_id, opts)
       return render_unauthorized_action unless @association.user_can_assess_for?(assessor: @current_user, assessee: @user)
 
       @assessment = @association.assess(:assessor => @current_user, :user => @user, :artifact => @asset, :assessment => params[:rubric_assessment],
@@ -96,4 +94,17 @@ class RubricAssessmentsController < ApplicationController
       end
     end
   end
+
+  private
+  def resolve_user_id
+    user_id = params[:rubric_assessment][:user_id]
+    if user_id
+      user_id =~ Api::ID_REGEX ? user_id.to_i : nil
+    elsif params[:rubric_assessment][:anonymous_id]
+      Submission.find_by!(
+        anonymous_id: params[:rubric_assessment][:anonymous_id],
+        assignment_id: @association.association_id
+      ).user_id
+    end
+  end
 end

public/javascripts/speed_grader.js

@@ -874,6 +874,11 @@ function initRubricStuff(){
         data['final'] = '1';
       }
     }
+    if (isAnonymous) {
+      data[`rubric_assessment[${anonymizableUserId}]`] = data['rubric_assessment[user_id]'];
+      delete data['rubric_assessment[user_id]'];
+    }
+
     data['graded_anonymously'] = utils.shouldHideStudentNames();
     var url = $(".update_rubric_assessment_url").attr('href');
     var method = "POST";
@@ -2539,7 +2544,7 @@ EG = {
   setOrUpdateSubmission: function(submission){
 
     // find the student this submission belongs to and update their submission with this new one, if they dont have a submission, set this as their submission.
-    var student =  jsonData.studentMap[submission.user_id];
+    const student = jsonData.studentMap[submission[anonymizableUserId]];
     if (!student) return;
 
     student.submission = student.submission || {};

spec/controllers/rubric_assessments_controller_spec.rb

@@ -177,9 +177,62 @@ describe RubricAssessmentsController do
       expect(response).to be_success
     end
   end
-end
 
-def setup_course_assessment
+  describe 'user ID handling' do
+    before(:each) do
+      setup_course_assessment
+      @assignment.find_or_create_submission(@student1).update!(anonymous_id: 'abcde')
+    end
+
+    let(:base_request_params) { { course_id: @course.id, rubric_association_id: @rubric_association.id } }
+
+    context 'when updating an existing assessment' do
+      it 'looks up the assessment by the passed-in ID' do
+        request_params = base_request_params.merge(
+          id: @rubric_assessment.id,
+          rubric_assessment: {assessment_type: 'no_reason'}
+        )
+        put 'update', params: request_params
+        expect(response).to be_success
+      end
+    end
+
+    context 'when creating a new assessment' do
+      it 'accepts user IDs in the user_id field' do
+        assessment_params = {user_id: @student1.id, assessment_type: 'no_reason'}
+        post 'create', params: base_request_params.merge(rubric_assessment: assessment_params)
+        expect(response).to be_success
+      end
+
+      it 'does not accept non-numerical IDs in the user_id field' do
+        invalid_assessment_params = {user_id: 'abcde', assessment_type: 'no_reason'}
+        post 'create', params: base_request_params.merge(rubric_assessment: invalid_assessment_params)
+        expect(response).to be_not_found
+      end
+    end
+
+    context 'for anonymous IDs' do
+      it 'accepts anonymous IDs matching a submission for the assignment' do
+        assessment_params = {anonymous_id: 'abcde', assessment_type: 'no reason'}
+        post 'create', params: base_request_params.merge(rubric_assessment: assessment_params)
+        expect(response).to be_success
+      end
+
+      it 'does not recognize anonymous IDs that do not match a submission for the assignment' do
+        unknown_assessment_params = {anonymous_id: @student1.id, assessment_type: 'no reason'}
+        post 'create', params: base_request_params.merge(rubric_assessment: unknown_assessment_params)
+        expect(response).to be_not_found
+      end
+
+      it 'does not recognize user IDs in the anonymous_id field' do
+        invalid_assessment_params = {anonymous_id: @student1.id, assessment_type: 'no reason'}
+        post 'create', params: base_request_params.merge(rubric_assessment: invalid_assessment_params)
+        expect(response).to be_not_found
+      end
+    end
+  end
+
+  def setup_course_assessment
     course_with_teacher_logged_in(:active_all => true)
     @student1 = factory_with_protected_attributes(User, :name => "student 1", :workflow_state => "registered")
     @student2 = factory_with_protected_attributes(User, :name => "student 2", :workflow_state => "registered")
@@ -196,4 +249,5 @@ def setup_course_assessment
     student3_asset = @assignment.find_or_create_submission(@student3)
     @rubric_association.assessment_requests.create!(user: @student1, asset: student1_asset, assessor: @student2, assessor_asset: student2_asset)
     @rubric_association.assessment_requests.create!(user: @student1, asset: student1_asset, assessor: @student3, assessor_asset: student3_asset)
+  end
 end

spec/javascripts/jsx/speed_graderSpec.js

@@ -1150,8 +1150,117 @@ QUnit.module('SpeedGrader - clicking save rubric button', function(hooks) {
     $('.save_rubric_button').trigger('click');
     strictEqual(disableWhileLoadingStub.callCount, 1);
   });
+
+  test('sends the user ID in rubric_assessment[user_id] if the assignment is not anonymous', () => {
+    SpeedGrader.EG.domReady();
+    sinon.stub(window.rubricAssessment, 'assessmentData').returns({ 'rubric_assessment[user_id]': '1234' });
+    $('.save_rubric_button').trigger('click');
+
+    const [, , data] = $.ajaxJSON.lastCall.args;
+    strictEqual(data['rubric_assessment[user_id]'], '1234');
+    window.rubricAssessment.assessmentData.restore();
+  })
 });
 
+QUnit.module('SpeedGrader - clicking save rubric button for an anonymous assignment', (hooks) => {
+  let disableWhileLoadingStub
+
+  hooks.beforeEach(() => {
+    sinon.stub($, 'ajaxJSON');
+    disableWhileLoadingStub = sinon.stub($.fn, 'disableWhileLoading');
+    fakeENV.setup({
+      anonymous_moderated_marking_enabled: true,
+      assignment_id: '27',
+      course_id: '3',
+      help_url: '',
+      show_help_menu_item: false,
+      RUBRIC_ASSESSMENT: {},
+    });
+
+    sinon.stub(SpeedGrader.EG, 'handleFragmentChanged')
+    sinon.stub(window.rubricAssessment, 'assessmentData').returns({ 'rubric_assessment[user_id]': 'abcde' });
+
+    fixtures.innerHTML = `
+      <button class="save_rubric_button"></button>
+      <div id="speedgrader_comment_textarea_mount_point"></div>
+      <div id="speedgrader-settings"></div>
+    `
+    SpeedGrader.setup()
+    SpeedGrader.EG.currentStudent = {
+      id: 4,
+      name: 'P. Sextus Rubricius',
+      submission_state: 'not_graded',
+      submission: {
+        grading_period_id: 8,
+        score: 7,
+        grade: 70,
+        submission_comments: [],
+        submission_history: [{}]
+      }
+    };
+    window.jsonData = {
+      gradingPeriods: {},
+      id: 27,
+      GROUP_GRADING_MODE: false,
+      points_possible: 10,
+      anonymous_grading: true,
+      submissions: [],
+      context: {
+        students: [
+          {
+            id: 4,
+            name: 'P. Sextus Rubricius'
+          }
+        ],
+        enrollments: [
+          {
+            user_id: 4,
+            workflow_state: 'active',
+            course_section_id: 1
+          }
+        ],
+        active_course_sections: [1]
+      },
+      studentMap: {
+        4: SpeedGrader.EG.currentStudent
+      }
+    }
+    ENV.SUBMISSION = {
+      grading_role: 'teacher'
+    };
+    ENV.RUBRIC_ASSESSMENT = {
+      assessment_type: 'grading',
+      assessor_id: 1
+    };
+
+    SpeedGrader.EG.jsonReady();
+  })
+
+  hooks.afterEach(() => {
+    window.rubricAssessment.assessmentData.restore();
+    SpeedGrader.EG.handleFragmentChanged.restore();
+
+    fixtures.innerHTML = ''
+    fakeENV.teardown();
+    disableWhileLoadingStub.restore();
+    $.ajaxJSON.restore();
+  })
+
+  test('sends the anonymous submission ID in rubric_assessment[anonymous_id] if the assignment is anonymous', () => {
+    $('.save_rubric_button').trigger('click');
+
+    const [, , data] = $.ajaxJSON.lastCall.args;
+    strictEqual(data['rubric_assessment[anonymous_id]'], 'abcde');
+  })
+
+  test('omits rubric_assessment[user_id] if the assignment is anonymous', () => {
+    $('.save_rubric_button').trigger('click');
+
+    const [, , data] = $.ajaxJSON.lastCall.args;
+    notOk('rubric_assessment[user_id]' in data)
+  })
+})
+
 QUnit.module('SpeedGrader - no gateway timeout', {
   setup () {
     fakeENV.setup();