extend submissions for_student api for gradebook2

Change-Id: I32c9ae92495ebb3fecca52983a3b50dcd162b620
Reviewed-on: https://gerrit.instructure.com/4399
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Ryan Shaw <ryan@instructure.com>

app/controllers/submissions_api_controller.rb

@@ -49,29 +49,63 @@ class SubmissionsApiController < ApplicationController
   #
   # @argument student_ids[] List of student ids to return submissions for. At least one is required.
   # @argument assignment_ids[] List of assignments to return submissions for. If none are given, submissions for all assignments are returned.
-  # @argument include[] ["submission_history"|"submission_comments"|"rubric_assessment"] Associations to include with the group.
+  # @argument grouped If this argument is present, the response will be grouped by student, rather than a flat array of submissions.
+  # @argument include[] ["submission_history"|"submission_comments"|"rubric_assessment"|"total_scores"] Associations to include with the group. `total_scores` requires the `grouped` argument.
+  #
+  # @example_response
+  #
+  # Without grouped:
+  #
+  # [
+  #   { "assignment_id": 100, grade: 5, "user_id": 1, ... },
+  #   { "assignment_id": 101, grade: 6, "user_id": 2, ... }
+  #
+  # With grouped:
+  #
+  # [
+  #   {
+  #     "user_id": 1,
+  #     "submissions: [
+  #       { "assignment_id": 100, grade: 5, ... },
+  #       { "assignment_id": 101, grade: 6, ... }
+  #     ]
+  #   }
+  # ]
   def for_students
     if authorized_action(@context, @current_user, :manage_grades)
       student_ids = map_user_ids(params[:student_ids])
       raise ActiveRecord::RecordNotFound if student_ids.blank?
 
-      assignment_ids = Array(params[:assignment_ids]).map(&:to_i)
-
-      scope = @context.submissions.scoped(:include => :assignment)
-
-      if assignment_ids.present?
-        @submissions = scope.all(
-          :conditions => {:user_id => student_ids, :assignment_id => assignment_ids})
-      else
-        @submissions = scope.all(
-          :conditions => {:user_id => student_ids})
-      end
-
       includes = Array(params[:include])
 
-      result = @submissions.map { |s| submission_json(s, s.assignment, includes) }
+      assignment_ids = @context.assignments.active.all(:select => :id).map(&:id)
+      requested_assignment_ids = Array(params[:assignment_ids]).map(&:to_i)
+      assignment_ids &= requested_assignment_ids if requested_assignment_ids.present?
 
-      render :json => result.to_json
+      Api.assignment_ids_for_students_api = assignment_ids
+      scope = @context.student_enrollments.scoped(
+        :include => { :user => :submissions_for_given_assignments },
+        :conditions => { 'users.id' => student_ids })
+
+      result = scope.map do |enrollment|
+        student = enrollment.user
+        hash = { :user_id => student.id, :submissions => [] }
+        student.submissions_for_given_assignments.each do |submission|
+          hash[:submissions] << submission_json(submission, submission.assignment, includes)
+        end
+        if includes.include?('total_scores') && params[:grouped].present?
+          hash.merge!(
+            :computed_final_score => enrollment.computed_final_score,
+            :computed_current_score => enrollment.computed_current_score)
+        end
+        hash
+      end
+
+      unless params[:grouped].present?
+        result = result.inject([]) { |arr, user_info| arr.concat(user_info[:submissions]) }
+      end
+
+      render :json => result
     end
   end
 
@@ -230,12 +264,17 @@ class SubmissionsApiController < ApplicationController
     hash
   end
 
-  SUBMISSION_JSON_FIELDS = %w(user_id url score grade attempt submission_type submitted_at body assignment_id grade_matches_current_submission)
+  SUBMISSION_JSON_FIELDS = %w(user_id url score grade attempt submission_type submitted_at body assignment_id grade_matches_current_submission).freeze
 
   def submission_attempt_json(attempt, assignment, version_idx = nil)
+    json_fields = SUBMISSION_JSON_FIELDS
+    if params[:response_fields]
+      json_fields = json_fields & params[:response_fields]
+    end
+
     hash = attempt.as_json(
       :include_root => false,
-      :only => SUBMISSION_JSON_FIELDS)
+      :only => json_fields)
 
     hash['preview_url'] = course_assignment_submission_url(
       @context, assignment, attempt[:user_id], 'preview' => '1',

app/models/user.rb

@@ -1478,4 +1478,11 @@ class User < ActiveRecord::Base
   def sis_user_id
     pseudonym.try(:sis_user_id)
   end
+
+  # association with dynamic, filtered join condition for submissions.
+  # This is messy, but in ActiveRecord 2 this is the only way to do an eager
+  # loading :include condition that has dynamic join conditions. It looks like
+  # there's better solutions in AR 3.
+  # See also e.g., http://makandra.com/notes/983-dynamic-conditions-for-belongs_to-has_many-and-has_one-associations
+  has_many :submissions_for_given_assignments, :include => [:assignment, :submission_comments], :conditions => 'submissions.assignment_id IN (#{Api.assignment_ids_for_students_api.join(",")})', :class_name => 'Submission'
 end

lib/api.rb

@@ -56,4 +56,6 @@ module Api
     end
   end
 
+  # See User.submissions_for_given_assignments and SubmissionsApiController#for_students
+  mattr_accessor :assignment_ids_for_students_api
 end

spec/apis/v1/submissions_api_spec.rb

@@ -363,7 +363,7 @@ describe SubmissionsApiController, :type => :integration do
           { :student_ids => [student1.to_param] })
 
     json.size.should == 2
-    json.all? { |submission| submission['user_id'].should == student1.id }.should be_true
+    json.each { |submission| submission['user_id'].should == student1.id }
 
     json = api_call(:get,
           "/api/v1/courses/#{@course.id}/students/submissions.json",
@@ -395,6 +395,80 @@ describe SubmissionsApiController, :type => :integration do
     json.all? { |submission| submission['assignment_id'].should == a1.id }.should be_true
   end
 
+  it "should return student submissions grouped by student" do
+    student1 = user(:active_all => true)
+    student2 = user_with_pseudonym(:active_all => true)
+    student2.pseudonym.update_attribute(:sis_user_id, 'my-student-id')
+
+    course_with_teacher(:active_all => true)
+
+    @course.enroll_student(student1).accept!
+    @course.enroll_student(student2).accept!
+
+    a1 = @course.assignments.create!(:title => 'assignment1', :grading_type => 'letter_grade', :points_possible => 15)
+    a2 = @course.assignments.create!(:title => 'assignment2', :grading_type => 'letter_grade', :points_possible => 25)
+
+    submit_homework(a1, student1)
+    submit_homework(a2, student1)
+    submit_homework(a1, student2)
+
+    json = api_call(:get,
+          "/api/v1/courses/#{@course.id}/students/submissions.json",
+          { :controller => 'submissions_api', :action => 'for_students',
+            :format => 'json', :course_id => @course.to_param },
+          { :student_ids => [student1.to_param], :grouped => '1' })
+
+    json.size.should == 1
+    json.first['submissions'].size.should == 2
+    json.each { |user| user['user_id'].should == student1.id }
+
+    json = api_call(:get,
+          "/api/v1/courses/#{@course.id}/students/submissions.json",
+          { :controller => 'submissions_api', :action => 'for_students',
+            :format => 'json', :course_id => @course.to_param },
+          { :student_ids => [student1.to_param, student2.to_param], :grouped => '1' })
+
+    json.size.should == 2
+    json.map { |u| u['submissions'] }.flatten.size.should == 3
+
+    json = api_call(:get,
+          "/api/v1/courses/#{@course.id}/students/submissions.json",
+          { :controller => 'submissions_api', :action => 'for_students',
+            :format => 'json', :course_id => @course.to_param },
+          { :student_ids => [student1.to_param, student2.to_param],
+            :assignment_ids => [a1.to_param], :grouped => '1' })
+
+    json.size.should == 2
+    json.each { |user| user['submissions'].each { |s| s['assignment_id'].should == a1.id } }
+  end
+
+  it "should return students with no submissions when grouped" do
+    student1 = user(:active_all => true)
+    student2 = user_with_pseudonym(:active_all => true)
+    student2.pseudonym.update_attribute(:sis_user_id, 'my-student-id')
+
+    course_with_teacher(:active_all => true)
+
+    @course.enroll_student(student1).accept!
+    @course.enroll_student(student2).accept!
+
+    a1 = @course.assignments.create!(:title => 'assignment1', :grading_type => 'letter_grade', :points_possible => 15)
+    a2 = @course.assignments.create!(:title => 'assignment2', :grading_type => 'letter_grade', :points_possible => 25)
+
+    submit_homework(a1, student1)
+    submit_homework(a2, student1)
+
+    json = api_call(:get,
+          "/api/v1/courses/#{@course.id}/students/submissions.json",
+          { :controller => 'submissions_api', :action => 'for_students',
+            :format => 'json', :course_id => @course.to_param },
+          { :student_ids => [student1.to_param, student2.to_param], :grouped => '1' })
+
+    json.size.should == 2
+    json.detect { |u| u['user_id'] == student1.id }['submissions'].size.should == 2
+    json.detect { |u| u['user_id'] == student2.id }['submissions'].size.should == 0
+  end
+
   it "should allow grading an uncreated submission" do
     student = user(:active_all => true)
     course_with_teacher(:active_all => true)