support section ids in the submissions api, refs #5835

Change-Id: Ibbe8f3852e8674c963e735b3f0fe8982d5ad7244
Reviewed-on: https://gerrit.instructure.com/6014
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Hudson <hudson@instructure.com>

Author: codekitchen

app/controllers/submissions_api_controller.rb

@@ -21,8 +21,12 @@
 # API for accessing and updating submissions for an assignment. The submission
 # id in these URLs is the id of the student in the course, there is no separate
 # submission id exposed in these APIs.
+#
+# All submission actions can be performed with either the course id, or the
+# course section id. SIS ids can be used, prefixed by "sis_course_id:" or
+# "sis_section_id:" as described in the API documentation on SIS IDs.
 class SubmissionsApiController < ApplicationController
-  before_filter :require_context
+  before_filter :get_course_from_section, :require_context
 
   # @API
   #
@@ -33,7 +37,7 @@ def index
     if authorized_action(@context, @current_user, :manage_grades)
       @assignment = @context.assignments.active.find(params[:assignment_id])
       @submissions = @assignment.submissions.all(
-        :conditions => { :user_id => @context.student_ids })
+        :conditions => { :user_id => (@section || @context).student_ids })
 
       includes = Array(params[:include])
 
@@ -91,7 +95,7 @@ def for_students
       Api.assignment_ids_for_students_api = assignments.map(&:id)
       sql_includes = { :user => [] }
       sql_includes[:user] << :submissions_for_given_assignments unless assignments.empty?
-      scope = @context.student_enrollments.scoped(
+      scope = (@section || @context).student_enrollments.scoped(
         :include => sql_includes,
         :conditions => { 'users.id' => student_ids })
 
@@ -127,8 +131,8 @@ def for_students
   # @argument include[] ["submission_history"|"submission_comments"|"rubric_assessment"] Associations to include with the group.
   def show
     @assignment = @context.assignments.active.find(params[:assignment_id])
-    params[:id] = map_user_ids([params[:id]]).first
-    @submission = @assignment.submissions.find_by_user_id(params[:id]) or raise ActiveRecord::RecordNotFound
+    @user = get_user_considering_section(params[:id])
+    @submission = @assignment.submissions.find_by_user_id(@user.id) or raise ActiveRecord::RecordNotFound
     if authorized_action(@submission, @current_user, :read)
       includes = Array(params[:include])
       render :json => submission_json(@submission, @assignment, includes).to_json
@@ -191,7 +195,7 @@ def show
   def update
     if authorized_action(@context, @current_user, :manage_grades)
       @assignment = @context.assignments.active.find(params[:assignment_id])
-      @user = api_find(@context.students_visible_to(@current_user), params[:id])
+      @user = get_user_considering_section(params[:id])
 
       submission = {}
       if params[:submission].is_a?(Hash)
@@ -354,7 +358,21 @@ def media_comment_json(media_comment_id, media_comment_type)
   end
 
   def map_user_ids(user_ids)
-    Api.map_ids(user_ids, User).compact
+    Api.map_ids(user_ids, User).compact unless user_ids.blank?
+  end
+
+  def get_course_from_section
+    if params[:section_id]
+      @section = api_find(CourseSection, params.delete(:section_id))
+      params[:course_id] = @section.course_id
+    end
   end
 
+  def get_user_considering_section(user_id)
+    scope = @context.students_visible_to(@current_user)
+    if @section
+      scope = scope.scoped(:conditions => { 'enrollments.course_section_id' => @section.id })
+    end
+    api_find(scope, user_id)
+  end
 end

app/models/course_section.rb

@@ -27,6 +27,7 @@ class CourseSection < ActiveRecord::Base
   belongs_to :sis_cross_listed_section
   belongs_to :account
   has_many :enrollments, :include => :user, :conditions => ['enrollments.workflow_state != ?', 'deleted'], :dependent => :destroy
+  has_many :students, :through => :student_enrollments, :source => :user, :order => :sortable_name
   has_many :student_enrollments, :class_name => 'StudentEnrollment', :conditions => ['enrollments.workflow_state != ? AND enrollments.workflow_state != ? AND enrollments.workflow_state != ? AND enrollments.workflow_state != ?', 'deleted', 'completed', 'rejected', 'inactive'], :include => :user
   has_many :admin_enrollments, :class_name => 'Enrollment', :conditions => "(enrollments.type = 'TaEnrollment' or enrollments.type = 'TeacherEnrollment')"
   has_many :users, :through => :enrollments

config/routes.rb

@@ -647,7 +647,6 @@
       courses.get 'courses/:id', :action => :show
       courses.get 'courses/:course_id/sections', :action => :sections, :path_name => 'course_sections'
       courses.get 'courses/:course_id/students', :action => :students
-      courses.get 'courses/:course_id/students/submissions', :controller => :submissions_api, :action => :for_students, :path_name => 'course_student_submissions'
       courses.get 'courses/:course_id/activity_stream', :action => :activity_stream
     end
 
@@ -660,8 +659,16 @@
 
     api.with_options(:controller => :submissions_api) do |submissions|
       submissions.get 'courses/:course_id/assignments/:assignment_id/submissions', :action => :index, :path_name => 'course_assignment_submissions'
+      submissions.get 'sections/:section_id/assignments/:assignment_id/submissions', :action => :index, :path_name => 'section_assignment_submissions'
+
+      submissions.get 'courses/:course_id/students/submissions', :controller => :submissions_api, :action => :for_students, :path_name => 'course_student_submissions'
+      submissions.get 'sections/:section_id/students/submissions', :controller => :submissions_api, :action => :for_students, :path_name => 'section_student_submissions'
+
       submissions.get 'courses/:course_id/assignments/:assignment_id/submissions/:id', :action => :show
+      submissions.get 'sections/:section_id/assignments/:assignment_id/submissions/:id', :action => :show
+
       submissions.put 'courses/:course_id/assignments/:assignment_id/submissions/:id', :action => :update, :path_name => 'course_assignment_submission'
+      submissions.put 'sections/:section_id/assignments/:assignment_id/submissions/:id', :action => :update, :path_name => 'section_assignment_submission'
     end
 
     api.get 'courses/:course_id/assignment_groups', :controller => :assignment_groups, :action => :index, :path_name => 'course_assignment_groups'

doc/templates/rest/method_details/html/method_signature.erb

@@ -1,4 +1,8 @@
+<% @routes.each do |route| %>
+<% route_path = route.segments.inject("") { |str,s| str << s.to_s }.sub(/\(\.:format\)\?/, '').sub(/\/$/, '')
+   next if route_path =~ /\.json$/ %>
 <h3>
-  <%= @route.conditions[:method].to_s.upcase %> <%= @route.segments.inject("") { |str,s| str << s.to_s }.sub(/\(\.:format\)\?/, '').sub(/\/$/, '') %>
-  <span class='defined-in'><a href="https://github.com/instructure/canvas-lms/blob/master/app/controllers/<%= @route.requirements[:controller] %>_controller.rb"><%= "#{@route.requirements[:controller].camelize}Controller\##{@route.requirements[:action]}" %></a></span>
+  <%= route.conditions[:method].to_s.upcase %> <%= route_path %>
+  <span class='defined-in'><a href="https://github.com/instructure/canvas-lms/blob/master/app/controllers/<%= route.requirements[:controller] %>_controller.rb"><%= "#{route.requirements[:controller].camelize}Controller\##{route.requirements[:action]}" %></a></span>
 </h3>
+<% end %>

doc/templates/rest/method_details/html/setup.rb

@@ -20,7 +20,7 @@ def init
   sections :header, [:method_signature, T('docstring')]
   @controller = object.parent.path.underscore.sub("_controller", '')
   @action = object.path.sub(/^.*#/, '')
-  @route = ApiRouteSet.apis.first.api_methods_for_controller_and_action(@controller, @action).first
+  @routes = ApiRouteSet.apis.first.api_methods_for_controller_and_action(@controller, @action)
 end
 
 def header

lib/api.rb

@@ -60,6 +60,8 @@ def self.map_ids(ids, collection)
       { 'sis_user_id' => 'pseudonyms.sis_user_id', 'sis_login_id' => 'pseudonyms.sis_source_id' },
     Account.table_name =>
       { 'sis_account_id' => 'sis_source_id' },
+    CourseSection.table_name =>
+      { 'sis_section_id' => 'sis_source_id' },
   }
 
   def self.sis_find_params_for_collection(collection, id, sis_find_params = nil)

spec/apis/v1/submissions_api_spec.rb

@@ -50,6 +50,86 @@ def submit_homework(assignment, student, opts = {:body => "test!"})
     response.status.should match /404/
   end
 
+  describe "using section ids" do
+    before do
+      @student1 = user(:active_all => true)
+      course_with_teacher(:active_all => true)
+      @default_section = @course.default_section
+      @section = factory_with_protected_attributes(@course.course_sections, :sis_source_id => 'my-section-sis-id', :name => 'section2')
+      @course.enroll_user(@student1, 'StudentEnrollment', :section => @section).accept!
+    end
+
+    it "should list submissions" do
+      quiz = Quiz.create!(:title => 'quiz1', :context => @course)
+      quiz.did_edit!
+      quiz.offer!
+      a1 = quiz.assignment
+      sub = a1.find_or_create_submission(@student1)
+      sub.submission_type = 'online_quiz'
+      sub.workflow_state = 'submitted'
+      sub.save!
+
+      json = api_call(:get,
+            "/api/v1/sections/#{@default_section.id}/assignments/#{a1.id}/submissions.json",
+            { :controller => 'submissions_api', :action => 'index',
+              :format => 'json', :section_id => @default_section.id.to_s,
+              :assignment_id => a1.id.to_s },
+            { :include => %w(submission_history submission_comments rubric_assessment) })
+      json.size.should == 0
+
+      json = api_call(:get,
+            "/api/v1/sections/sis_section_id:my-section-sis-id/assignments/#{a1.id}/submissions.json",
+            { :controller => 'submissions_api', :action => 'index',
+              :format => 'json', :section_id => 'sis_section_id:my-section-sis-id',
+              :assignment_id => a1.id.to_s },
+            { :include => %w(submission_history submission_comments rubric_assessment) })
+      json.size.should == 1
+      json.first['user_id'].should == @student1.id
+
+      json = api_call(:get,
+            "/api/v1/sections/#{@default_section.id}/students/submissions",
+            { :controller => 'submissions_api', :action => 'for_students',
+              :format => 'json', :section_id => @default_section.id.to_s },
+              :student_ids => [@student1.id])
+      json.size.should == 0
+
+      json = api_call(:get,
+            "/api/v1/sections/sis_section_id:my-section-sis-id/students/submissions",
+            { :controller => 'submissions_api', :action => 'for_students',
+              :format => 'json', :section_id => 'sis_section_id:my-section-sis-id' },
+              :student_ids => [@student1.id])
+      json.size.should == 1
+    end
+
+    it "should post to submissions" do
+      a1 = @course.assignments.create!({:title => 'assignment1', :grading_type => 'percent', :points_possible => 10})
+
+      raw_api_call(:put,
+                      "/api/v1/sections/#{@default_section.id}/assignments/#{a1.id}/submissions/#{@student1.id}",
+      { :controller => 'submissions_api', :action => 'update',
+        :format => 'json', :section_id => @default_section.id.to_s,
+        :assignment_id => a1.id.to_s, :id => @student1.id.to_s },
+        { :submission => { :posted_grade => '75%' } })
+      response.status.should == "404 Not Found"
+
+      json = api_call(:put,
+                      "/api/v1/sections/sis_section_id:my-section-sis-id/assignments/#{a1.id}/submissions/#{@student1.id}",
+      { :controller => 'submissions_api', :action => 'update',
+        :format => 'json', :section_id => 'sis_section_id:my-section-sis-id',
+        :assignment_id => a1.id.to_s, :id => @student1.id.to_s },
+        { :submission => { :posted_grade => '75%' } })
+
+      Submission.count.should == 1
+      @submission = Submission.first
+
+      json['score'].should == 7.5
+      json['grade'].should == '75%'
+    end
+
+    it "should return submissions for a section" do
+    end
+  end
+
   it "should return student discussion entries for discussion_topic assignments" do
     @student = user(:active_all => true)
     course_with_teacher(:active_all => true)