ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

ensure todos have needed fields during api calls 

This commit fixes a regression from d30084, in which calls to
Api::V1::assignment_json were not being provided needed fields such as
'turnitin_settings' when accessed through the todos api. This caused the
instantiated ActiveRecord objects to throw an exception because the
field(s) had not been assigned.

test plan:
  - Ensure you have a todo, e.g. an assignment that you haven't
    submitted but is due in the future.
  - Make an authenticated API request for todos
    (e.g. /api/v1/users/self/todo ).
  - The API response should be successful and include todo information.
    The API response should not contain an error message, or have an
    HTTP status indicating errors such as 500.

fixes CNVS-3362

Change-Id: I8af8f17590ac8e37fa2a545c9e49deadf6f3d2b6
Reviewed-on: https://gerrit.instructure.com/17176
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
QA-Review: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Stanley Stuart 2013-01-26 09:33:54 -07:00 committed by Brian Palmer
parent ba69b52791
commit 6a618ecea9
3 changed files with 113 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
app/models/assignment.rb
View File

@ -70,6 +70,37 @@ class Assignment < ActiveRecord::Base
end
end
API_NEEDED_FIELDS = %w(
id
title
context_id
context_type
position
points_possible
grading_type
due_at
description
lock_at
unlock_at
assignment_group_id
peer_reviews
automatic_peer_reviews
peer_reviews_due_at
peer_review_count
submission_types
group_category_id
grade_group_students_individually
turnitin_enabled
turnitin_settings
allowed_extensions
muted
needs_grading_count
could_be_locked
freeze_on_copy
copied
all_day
all_day_date
)
# create a shim for plugins that use the old association name. this is
# TEMPORARY. the plugins should update to use the new association name, and
# once they're updated, this shim removed. DO NOT USE in new code.
@ -1382,8 +1413,8 @@ class Assignment < ActiveRecord::Base
# This should only be used in the course drop down to show assignments needing a submission
named_scope :need_submitting_info, lambda{|user_id, limit, ignored_ids|
ignored_ids ||= []
{:select => 'id, title, points_possible, due_at, context_id, context_type, submission_types, description, could_be_locked, needs_grading_count, all_day_date,' +
'(SELECT name FROM courses WHERE id = assignments.context_id) AS context_name',
{:select => API_NEEDED_FIELDS.join( ',' ) +
',(SELECT name FROM courses WHERE id = assignments.context_id) AS context_name',
:conditions =>["(SELECT COUNT(id) FROM submissions
WHERE assignment_id = assignments.id
AND submission_type IS NOT NULL
@ -1397,8 +1428,8 @@ class Assignment < ActiveRecord::Base
named_scope :need_grading_info, lambda{|limit, ignore_ids|
ignore_ids ||= []
{
:select => 'assignments.id, title, points_possible, due_at, context_id, context_type, submission_types, description, could_be_locked, all_day_date,' +
'(SELECT name FROM courses WHERE id = assignments.context_id) AS context_name, needs_grading_count',
:select => API_NEEDED_FIELDS.join(',') +
',(SELECT name FROM courses WHERE id = assignments.context_id) AS context_name, needs_grading_count',
:conditions => "needs_grading_count > 0 #{ignore_ids.empty? ? "" : "AND id NOT IN (#{ignore_ids.join(',')})"}",
:limit => limit,
:order=>'due_at ASC'

31
app/models/user.rb
View File

@ -1519,9 +1519,18 @@ class User < ActiveRecord::Base
def assignments_needing_submitting(opts={})
ActiveRecord::Base::ConnectionSpecification.with_environment(:slave) do
course_codes = opts[:contexts] ? (Array(opts[:contexts]).map(&:asset_string) & current_student_enrollment_course_codes) : current_student_enrollment_course_codes
ignored_ids = ignored_items(:submitting).select{|key, val| key.match(/\Aassignment_/) }.map{|key, val| key.sub(/\Aassignment_/, "") }
Assignment.for_context_codes(course_codes).active.due_before(1.week.from_now).
course_codes = if opts[:contexts]
(Array(opts[:contexts]).map(&:asset_string) &
current_student_enrollment_course_codes)
else
current_student_enrollment_course_codes
end
ignored_ids = ignored_items(:submitting).
select{|key, val| key.match(/\Aassignment_/) }.
map{|key, val| key.sub(/\Aassignment_/, "") }
Assignment.for_context_codes(course_codes).
active.
due_before(1.week.from_now).
expecting_submission.due_after(opts[:due_after] || 4.weeks.ago).
need_submitting_info(id, opts[:limit] || 15, ignored_ids).
not_locked
@ -1540,9 +1549,19 @@ class User < ActiveRecord::Base
def assignments_needing_grading(opts={})
ActiveRecord::Base::ConnectionSpecification.with_environment(:slave) do
course_codes = opts[:contexts] ? (Array(opts[:contexts]).map(&:asset_string) & current_admin_enrollment_course_codes) : current_admin_enrollment_course_codes
ignored_ids = ignored_items(:grading).select{|key, val| key.match(/\Aassignment_/) }.map{|key, val| key.sub(/\Aassignment_/, "") }
Assignment.for_context_codes(course_codes).active.expecting_submission.need_grading_info(opts[:limit] || 15, ignored_ids).reject{|a| a.needs_grading_count_for_user(self) == 0}
course_codes = if opts[:contexts]
(Array(opts[:contexts]).map(&:asset_string) &
current_admin_enrollment_course_codes)
else
current_admin_enrollment_course_codes
end
ignored_ids = ignored_items(:grading).
select{|key, val| key.match(/\Aassignment_/) }.
map{|key, val| key.sub(/\Aassignment_/, "") }
Assignment.for_context_codes(course_codes).active.
expecting_submission.
need_grading_info(opts[:limit] || 15, ignored_ids).
reject{|a| a.needs_grading_count_for_user(self) == 0}
end
end
memoize :assignments_needing_grading

93
spec/apis/v1/todo_items_api_spec.rb
View File

@ -18,14 +18,22 @@
require File.expand_path(File.dirname(__FILE__) + '/../api_spec_helper')
describe UsersController, :type => :integration do
include Api
include Api::V1::Assignment
def update_assignment_json
@a1_json['assignment'] = controller.assignment_json(@a1,@user,session)
@a2_json['assignment'] = controller.assignment_json(@a2,@user,session)
end
before do
course_with_teacher(:active_all => true, :user => user_with_pseudonym(:active_all => true))
@teacher = course_with_teacher(:active_all => true, :user => user_with_pseudonym(:active_all => true))
@teacher_course = @course
@student_course = course(:active_all => true)
@student_course.enroll_student(@user).accept!
# an assignment i need to submit (needs_submitting)
@a = Assignment.create!(:context => @student_course, :due_at => 6.days.from_now, :title => 'required work', :submission_types => 'online_text_entry', :points_possible => 10)
@a1 = Assignment.create!(:context => @student_course, :due_at => 6.days.from_now, :title => 'required work', :submission_types => 'online_text_entry', :points_possible => 10)
# an assignment i created, and a student who submits the assignment (needs_grading)
@a2 = Assignment.create!(:context => @teacher_course, :due_at => 1.day.from_now, :title => 'text', :submission_types => 'online_text_entry', :points_possible => 15)
@ -34,41 +42,21 @@ describe UsersController, :type => :integration do
@user = @me
@teacher_course.enroll_student(student).accept!
@sub = @a2.reload.submit_homework(student, :submission_type => 'online_text_entry', :body => 'done')
@a2.reload
@a1_json =
{
'type' => 'submitting',
'assignment' => {
'name' => 'required work',
'description' => nil,
'id' => @a.id,
'course_id' => @student_course.id,
'muted' => false,
'points_possible' => 10,
'submission_types' => ['online_text_entry'],
'due_at' => @a.due_at.as_json,
'html_url' => course_assignment_url(@a.context_id, @a),
},
'ignore' => api_v1_users_todo_ignore_url(@a.asset_string, 'submitting', :permanent => 0),
'ignore_permanently' => api_v1_users_todo_ignore_url(@a.asset_string, 'submitting', :permanent => 1),
'html_url' => "#{course_assignment_url(@a.context_id, @a.id)}#submit",
'assignment' => {},
'ignore' => api_v1_users_todo_ignore_url(@a1.asset_string, 'submitting', :permanent => 0),
'ignore_permanently' => api_v1_users_todo_ignore_url(@a1.asset_string, 'submitting', :permanent => 1),
'html_url' => "#{course_assignment_url(@a1.context_id, @a1.id)}#submit",
'context_type' => 'Course',
'course_id' => @student_course.id,
}
@a2_json =
{
'type' => 'grading',
'assignment' => {
'name' => 'text',
'description' => nil,
'id' => @a2.id,
'course_id' => @teacher_course.id,
'muted' => false,
'points_possible' => 15,
'needs_grading_count' => 1,
'submission_types' => ['online_text_entry'],
'due_at' => @a2.due_at.as_json,
'html_url' => course_assignment_url(@a2.context_id, @a2),
},
'assignment' => {},
'needs_grading_count' => 1,
'ignore' => api_v1_users_todo_ignore_url(@a2.asset_string, 'grading', :permanent => 0),
'ignore_permanently' => api_v1_users_todo_ignore_url(@a2.asset_string, 'grading', :permanent => 1),
@ -83,7 +71,10 @@ describe UsersController, :type => :integration do
student2 = user(:active_all => true)
@user = @me
@teacher_course.enroll_student(student2).accept!
@sub2 = @a2.reload.submit_homework(student2, :submission_type => 'online_text_entry', :body => 'me too')
@sub2 = @a2.reload.submit_homework(student2,
:submission_type => 'online_text_entry',
:body => 'me too')
@a2.reload
end
it "should check for auth" do
@ -101,17 +92,23 @@ describe UsersController, :type => :integration do
it "should return a global user todo list" do
json = api_call(:get, "/api/v1/users/self/todo",
:controller => "users", :action => "todo_items", :format => "json")
json.sort_by { |t| t['assignment']['id'] }.should == [@a1_json, @a2_json]
update_assignment_json
json = json.sort_by { |t| t['assignment']['id'] }
compare_json json.first, @a1_json
compare_json json.second, @a2_json
end
it "should return a course-specific todo list" do
json = api_call(:get, "/api/v1/courses/#{@student_course.id}/todo",
:controller => "courses", :action => "todo_items", :format => "json", :course_id => @student_course.to_param)
json.should == [@a1_json]
a1_json = api_call(:get, "/api/v1/courses/#{@student_course.id}/todo",
:controller => "courses", :action => "todo_items",
:format => "json", :course_id => @student_course.to_param)
json = api_call(:get, "/api/v1/courses/#{@teacher_course.id}/todo",
:controller => "courses", :action => "todo_items", :format => "json", :course_id => @teacher_course.to_param)
json.should == [@a2_json]
a2_json = api_call(:get, "/api/v1/courses/#{@teacher_course.id}/todo",
:controller => "courses", :action => "todo_items",
:format => "json", :course_id => @teacher_course.to_param)
update_assignment_json
compare_json( a1_json.first, @a1_json )
compare_json( a2_json.first, @a2_json )
end
it "should return a list for users who are both teachers and students" do
@ -120,16 +117,22 @@ describe UsersController, :type => :integration do
json = api_call(:get, "/api/v1/users/self/todo",
:controller => "users", :action => "todo_items", :format => "json")
@a1_json.deep_merge!({ 'assignment' => { 'needs_grading_count' => 0 } })
json.sort_by { |t| t['assignment']['id'] }.should == [@a1_json, @a2_json]
json = json.sort_by { |t| t['assignment']['id'] }
update_assignment_json
compare_json( json.first, @a1_json )
compare_json( json.second, @a2_json )
end
it "should ignore a todo item permanently" do
api_call(:delete, @a2_json['ignore_permanently'],
:controller => "users", :action => "ignore_item", :format => "json", :purpose => "grading", :asset_string => "assignment_#{@a2.id}", :permanent => "1")
:controller => "users", :action => "ignore_item",
:format => "json", :purpose => "grading",
:asset_string => "assignment_#{@a2.id}", :permanent => "1")
response.should be_success
json = api_call(:get, "/api/v1/courses/#{@teacher_course.id}/todo",
:controller => "courses", :action => "todo_items", :format => "json", :course_id => @teacher_course.to_param)
:controller => "courses", :action => "todo_items",
:format => "json", :course_id => @teacher_course.to_param)
json.should == []
# after new student submission, still ignored
@ -154,6 +157,16 @@ describe UsersController, :type => :integration do
:controller => "courses", :action => "todo_items", :format => "json", :course_id => @teacher_course.to_param)
@a2_json['needs_grading_count'] = 2
@a2_json['assignment']['needs_grading_count'] = 2
json.should == [@a2_json]
update_assignment_json
compare_json( json.first, @a2_json )
end
it "works correctly when turnitin is enabled" do
@a2.context.any_instantiation.expects(:turnitin_enabled?).returns true
json = api_call(:get, "/api/v1/users/self/todo",
:controller => "users", :action => "todo_items",
:format => "json")
response.should be_success
end
end