Skip to content

Commit da7512c

Browse files
committed
excused assignments backend
closes CNVS-17549 closes CNVS-17553 Test plan: * excuse and unexcuse assignments from the api * student grades should be correct Change-Id: Id8e0fa9edb404bdf65bff0bfc4c79c6f33e8a7e4 Reviewed-on: https://gerrit.instructure.com/53409 Tested-by: Jenkins Reviewed-by: Josh Simpson <jsimpson@instructure.com> QA-Review: Adrian Foong <afoong@instructure.com> Product-Review: Cameron Matheson <cameron@instructure.com>
1 parent 05f4b91 commit da7512c

14 files changed

Lines changed: 208 additions & 47 deletions

app/coffeescripts/grade_calculator.coffee

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ define [
1010
[trueList, falseList]
1111

1212
class GradeCalculator
13-
# each submission needs fields: score, points_possible, assignment_id, assignment_group_id
13+
# each submission needs fields: score, points_possible, assignment_id, assignment_group_id, excused
1414
# to represent assignments that the student hasn't submitted, pass a
1515
# submission with score == null
1616
#
@@ -62,6 +62,9 @@ define [
6262
s = assignment_id: assignmentId, score: null
6363
submissions.push dummySubmissions...
6464

65+
# filter out excused assignments
66+
submissions = _(submissions).filter (s) -> not s.excused
67+
6568
submissionsByAssignment = arrayToObj submissions, "assignment_id"
6669

6770
submissionData = _(submissions).map (s) =>

app/controllers/submissions_api_controller.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,11 @@
132132
# "description": "Whether the assignment is visible to the user who submitted the assignment. Submissions where `assignment_visible` is false no longer count towards the student's grade and the assignment can no longer be accessed by the student. `assignment_visible` becomes false for submissions that do not have a grade and whose assignment is no longer assigned to the student's section.",
133133
# "example": true,
134134
# "type": "boolean"
135+
# },
136+
# "excused": {
137+
# "description": "Whether the assignment is excused. Excused assignments have no impact on a user's grade.",
138+
# "example": true,
139+
# "type": "boolean"
135140
# }
136141
# }
137142
# }
@@ -536,6 +541,8 @@ def create_file
536541
# a posted_grade in the "points" or "percentage" format is sent, the grade
537542
# will only be accepted if the grade equals one of those two values.
538543
#
544+
# @argument submission[excuse] [Boolean]
545+
# Sets the "excused" status of an assignment.
539546
#
540547
# @argument rubric_assessment [RubricAssessment]
541548
# Assign a rubric assessment to this assignment submission. The
@@ -593,8 +600,9 @@ def update
593600
submission = { :grader => @current_user }
594601
if params[:submission].is_a?(Hash)
595602
submission[:grade] = params[:submission].delete(:posted_grade)
603+
submission[:excuse] = params[:submission].delete(:excuse)
596604
end
597-
if submission[:grade]
605+
if submission[:grade] || submission[:excuse]
598606
@submissions = @assignment.grade_student(@user, submission)
599607
@submission = @submissions.first
600608
else

app/models/assignment.rb

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -855,7 +855,8 @@ def submitted_count
855855
(submittable_type? || submission_types == "discussion_topic") &&
856856
context.grants_right?(user, session, :participate_as_student) &&
857857
!locked_for?(user) &&
858-
visible_to_user?(user)
858+
visible_to_user?(user) &&
859+
!excused_for?(user)
859860
}
860861
can :submit and can :attach_submission_comment_files
861862

@@ -976,6 +977,8 @@ def grade_student(original_student, opts={})
976977
group_comment = Canvas::Plugin.value_to_boolean(opts.delete(:group_comment))
977978
group, students = group_students(original_student)
978979
grader = opts.delete :grader
980+
grade = opts.delete :grade
981+
score = opts.delete :score
979982
comment = {
980983
:comment => (opts.delete :comment),
981984
:attachments => (opts.delete :comment_attachments),
@@ -986,22 +989,36 @@ def grade_student(original_student, opts={})
986989
}
987990
comment[:group_comment_id] = CanvasSlug.generate_securish_uuid if group_comment && group
988991
submissions = []
992+
993+
if opts.key? :excuse
994+
opts[:excused] = Canvas::Plugin.value_to_boolean(opts.delete(:excuse))
995+
end
996+
997+
grade_group_students = !grade_group_students_individually && !opts.key?(:excused)
998+
989999
find_or_create_submissions(students) do |submission|
9901000
submission_updated = false
9911001
submission.skip_grade_calc = opts[:skip_grade_calc]
9921002
student = submission.user
993-
if student == original_student || !grade_group_students_individually
1003+
if student == original_student || grade_group_students
9941004
previously_graded = submission.grade.present?
9951005
next if previously_graded && dont_overwrite_grade
1006+
1007+
did_grade = false
9961008
submission.attributes = opts
9971009
submission.assignment_id = self.id
9981010
submission.user_id = student.id
9991011
submission.grader_id = grader.try(:id)
1000-
if !opts[:grade] || opts[:grade] == ""
1012+
1013+
if grade.present? || score.present?
1014+
submission.grade = grade
1015+
submission.score = score
1016+
submission.excused = false
1017+
else
10011018
submission.score = nil
10021019
submission.grade = nil
10031020
end
1004-
did_grade = false
1021+
10051022
if submission.grade
10061023
did_grade = true
10071024
submission.score = self.grade_to_score(submission.grade)
@@ -1943,6 +1960,11 @@ def unpublish
19431960
self.save
19441961
end
19451962

1963+
def excused_for?(user)
1964+
s = submissions.where(user_id: user.id).first_or_initialize
1965+
s.excused?
1966+
end
1967+
19461968
# simply versioned models are always marked new_record, but for our purposes
19471969
# they are not new. this ensures that assignment override caching works as
19481970
# intended for versioned assignments

app/models/submission.rb

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ def new_version_needed?
246246
end
247247

248248
def update_final_score
249-
if score_changed?
249+
if score_changed? || excused_changed?
250250
if skip_grade_calc
251251
Rails.logger.info "GRADES: NOT recomputing scores for submission #{global_id} because skip_grade_calc was set"
252252
else
@@ -1030,8 +1030,15 @@ def missing?
10301030
end
10311031
alias_method :missing, :missing?
10321032

1033+
# QUESTIONS ABOUT EXCUSED:
1034+
# * what happens for group assignments? excuse individually
1035+
# * can't excuse for group assignments in speedgrader 1.0
1036+
# * TODO make sure Assignment#representatives is updated accordingly
1037+
#
1038+
# QUESTIONS FOR ME:
1039+
# * are we messing up graded / not graded counts???
10331040
def graded?
1034-
!!self.score && self.workflow_state == 'graded'
1041+
excused? || (!!score && workflow_state == 'graded')
10351042
end
10361043
end
10371044
include Tardiness
@@ -1127,6 +1134,16 @@ def attach_screenshot(attachment)
11271134
self.save!
11281135
end
11291136

1137+
def excused=(excused)
1138+
if excused
1139+
self[:excused] = true
1140+
self.grade = nil
1141+
self.score = nil
1142+
else
1143+
self[:excused] = false
1144+
end
1145+
end
1146+
11301147
def comments_for(user)
11311148
grants_right?(user, :read_grade)? submission_comments : visible_submission_comments
11321149
end
@@ -1145,7 +1162,7 @@ def update_participation
11451162
return if assignment.deleted? || assignment.muted?
11461163
return unless self.user_id
11471164

1148-
if score_changed? || grade_changed?
1165+
if score_changed? || grade_changed? || excused_changed?
11491166
ContentParticipation.create_or_update({
11501167
:content => self,
11511168
:user => self.user,
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
class AddExcusedToSubmissions < ActiveRecord::Migration
2+
tag :predeploy
3+
4+
def change
5+
add_column :submissions, :excused, :boolean
6+
end
7+
end

lib/api/v1/submission.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ def submission_json(submission, assignment, user, session, context = nil, includ
9090
hash
9191
end
9292

93-
SUBMISSION_JSON_FIELDS = %w(id user_id url score grade attempt submission_type submitted_at body assignment_id graded_at grade_matches_current_submission grader_id workflow_state).freeze
93+
SUBMISSION_JSON_FIELDS = %w(id user_id url score grade excused attempt submission_type submitted_at body assignment_id graded_at grade_matches_current_submission grader_id workflow_state).freeze
9494
SUBMISSION_JSON_METHODS = %w(late).freeze
95-
SUBMISSION_OTHER_FIELDS = %w(attachments discussion_entries)
95+
SUBMISSION_OTHER_FIELDS = %w(attachments discussion_entries).freeze
9696

9797
def submission_attempt_json(attempt, assignment, user, session, version_idx = nil, context = nil)
9898
context ||= assignment.context

lib/grade_calculator.rb

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def compute_scores
5656
except(:order, :select).
5757
for_user(@user_ids).
5858
where(assignment_id: @assignments.map(&:id)).
59-
select("submissions.id, user_id, assignment_id, score")
59+
select("submissions.id, user_id, assignment_id, score, excused")
6060
submissions_by_user = @submissions.group_by(&:user_id)
6161

6262
scores = []
@@ -147,13 +147,16 @@ def create_group_sums(submissions, user_id, ignore_ungraded=true)
147147
s = nil if @ignore_muted && a.muted?
148148

149149
{
150-
:assignment => a,
151-
:submission => s,
152-
:score => s && s.score,
153-
:total => a.points_possible || 0,
150+
assignment: a,
151+
submission: s,
152+
score: s && s.score,
153+
total: a.points_possible || 0,
154+
excused: s && s.excused?,
154155
}
155156
end
157+
156158
group_submissions.reject! { |s| s[:score].nil? } if ignore_ungraded
159+
group_submissions.reject! { |s| s[:excused] }
157160
group_submissions.each { |s| s[:score] ||= 0 }
158161

159162
logged_submissions = group_submissions.map { |s| loggable_submission(s) }

spec/apis/v1/gradebook_history_api_spec.rb

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -142,19 +142,6 @@
142142
@submission4 = @assignment2.submit_homework(@student1)
143143
end
144144

145-
it 'should return submission version API objects' do
146-
@submission4.update_attributes!(:graded_at => 24.hours.ago, :grader_id => @other_grader.id, :score => 70)
147-
148-
json = api_call_as_user(@teacher, :get, "/api/v1/courses/#{@course.id}/gradebook_history/feed.json", {
149-
:controller => 'gradebook_history_api',
150-
:action => 'feed',
151-
:format => 'json',
152-
:course_id => @course.id.to_s
153-
}).first
154-
155-
expect(json.keys.sort).to eq ["assignment_id", "assignment_name", "attempt", "body", "current_grade", "current_graded_at", "current_grader", "grade", "grade_matches_current_submission", "graded_at", "grader", "grader_id", "id", "late", "preview_url", "score", "submission_type", "submitted_at", "url", "user_id", "user_name", "workflow_state"]
156-
end
157-
158145
it 'should return all applicable versions' do
159146
@submission1.update_attributes!(:graded_at => Time.zone.now, :grader_id => @grader.id, :score => 100)
160147
@submission2.update_attributes!(:graded_at => Time.zone.now, :grader_id => @super_grader.id, :score => 90)

spec/apis/v1/stream_items_api_spec.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@
281281
'created_at' => StreamItem.last.created_at.as_json,
282282
'updated_at' => StreamItem.last.updated_at.as_json,
283283
'grade' => '12',
284+
'excused' => false,
284285
'grader_id' => @teacher.id,
285286
'graded_at' => @sub.graded_at.as_json,
286287
'score' => 12,
@@ -386,6 +387,7 @@
386387
'created_at' => StreamItem.last.created_at.as_json,
387388
'updated_at' => StreamItem.last.updated_at.as_json,
388389
'grade' => nil,
390+
'excused' => nil,
389391
'grader_id' => nil,
390392
'graded_at' => nil,
391393
'score' => nil,

spec/apis/v1/submissions_api_spec.rb

Lines changed: 56 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ def submit_homework(assignment, student, opts = {:body => "test!"})
5858
"preview_url" => "http://www.example.com/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{student.id}?preview=1",
5959
"user_id"=>student.id,
6060
"grade"=>nil,
61+
"excused" => nil,
6162
"grader_id"=>nil,
6263
"body"=>nil,
6364
"submitted_at"=>nil,
@@ -604,6 +605,7 @@ def submission_with_comment
604605
expect(json).to eq({
605606
"id"=>sub1.id,
606607
"grade"=>"A-",
608+
"excused" => sub1.excused,
607609
"grader_id"=>@teacher.id,
608610
"graded_at"=>sub1.graded_at.as_json,
609611
"body"=>"test!",
@@ -769,6 +771,7 @@ def submission_with_comment
769771
res =
770772
[{"id"=>sub1.id,
771773
"grade"=>"A-",
774+
"excused" => sub1.excused,
772775
"grader_id"=>@teacher.id,
773776
"graded_at"=>sub1.graded_at.as_json,
774777
"body"=>"test!",
@@ -799,6 +802,7 @@ def submission_with_comment
799802
"submission_history"=>
800803
[{"id"=>sub1.id,
801804
"grade"=>nil,
805+
"excused" => nil,
802806
"grader_id"=>nil,
803807
"graded_at"=>nil,
804808
"body"=>"test!",
@@ -815,6 +819,7 @@ def submission_with_comment
815819
"late"=>false},
816820
{"id"=>sub1.id,
817821
"grade"=>nil,
822+
"excused" => nil,
818823
"grader_id"=>nil,
819824
"graded_at"=>nil,
820825
"assignment_id" => a1.id,
@@ -837,6 +842,7 @@ def submission_with_comment
837842
"late"=>false},
838843
{"id"=>sub1.id,
839844
"grade"=>"A-",
845+
"excused" => false,
840846
"grader_id"=>@teacher.id,
841847
"graded_at"=>sub1.graded_at.as_json,
842848
"assignment_id" => a1.id,
@@ -910,6 +916,7 @@ def submission_with_comment
910916
"late"=>false},
911917
{"id"=>sub2.id,
912918
"grade"=>"F",
919+
"excused" => sub2.excused,
913920
"grader_id"=>@teacher.id,
914921
"graded_at"=>sub2.graded_at.as_json,
915922
"assignment_id" => a1.id,
@@ -920,6 +927,7 @@ def submission_with_comment
920927
"submission_history"=>
921928
[{"id"=>sub2.id,
922929
"grade"=>"F",
930+
"excused" => nil,
923931
"grader_id"=>@teacher.id,
924932
"graded_at"=>sub2.graded_at.as_json,
925933
"assignment_id" => a1.id,
@@ -1830,24 +1838,57 @@ def call_to_submissions_show(opts={})
18301838
end
18311839
end
18321840

1833-
it "should allow grading an uncreated submission" do
1834-
student = user(:active_all => true)
1835-
course_with_teacher(:active_all => true)
1836-
@course.enroll_student(student).accept!
1837-
a1 = @course.assignments.create!(:title => 'assignment1', :grading_type => 'letter_grade', :points_possible => 15)
1841+
describe '#update' do
1842+
before :once do
1843+
student_in_course(:active_all => true)
1844+
teacher_in_course(:active_all => true)
1845+
@assignment = @course.assignments.create!(:title => 'assignment1', :grading_type => 'letter_grade', :points_possible => 15)
18381846

1839-
json = api_call(:put,
1840-
"/api/v1/courses/#{@course.id}/assignments/#{a1.id}/submissions/#{student.id}.json",
1841-
{ :controller => 'submissions_api', :action => 'update',
1842-
:format => 'json', :course_id => @course.id.to_s,
1843-
:assignment_id => a1.id.to_s, :user_id => student.id.to_s },
1844-
{ :submission => { :posted_grade => 'B' } })
1847+
end
18451848

1846-
expect(Submission.count).to eq 1
1847-
@submission = Submission.first
1849+
it "should allow grading an uncreated submission" do
1850+
json = api_call(:put,
1851+
"/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}.json",
1852+
{ :controller => 'submissions_api', :action => 'update',
1853+
:format => 'json', :course_id => @course.id.to_s,
1854+
:assignment_id => @assignment.id.to_s, :user_id => @student.id.to_s },
1855+
{ :submission => { :posted_grade => 'B' } })
18481856

1849-
expect(json['grade']).to eq 'B'
1850-
expect(json['score']).to eq 12.9
1857+
expect(Submission.count).to eq 1
1858+
@submission = Submission.first
1859+
1860+
expect(json['grade']).to eq 'B'
1861+
expect(json['score']).to eq 12.9
1862+
end
1863+
1864+
it "can excuse assignments" do
1865+
json = api_call(:put,
1866+
"/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}.json",
1867+
{ :controller => 'submissions_api', :action => 'update',
1868+
:format => 'json', :course_id => @course.id.to_s,
1869+
:assignment_id => @assignment.id.to_s, :user_id => @student.id.to_s },
1870+
{ :submission => { excuse: "1" } })
1871+
1872+
submission = @assignment.submission_for_student(@student)
1873+
expect(submission).to be_excused
1874+
expect(json['excused']).to eq true
1875+
end
1876+
1877+
it "can unexcuse assignments" do
1878+
@assignment.grade_student(@student, excuse: true)
1879+
submission = @assignment.submission_for_student(@student)
1880+
expect(submission).to be_excused
1881+
1882+
json = api_call(:put,
1883+
"/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}.json",
1884+
{ :controller => 'submissions_api', :action => 'update',
1885+
:format => 'json', :course_id => @course.id.to_s,
1886+
:assignment_id => @assignment.id.to_s, :user_id => @student.id.to_s },
1887+
{ :submission => { excuse: "0" } })
1888+
1889+
expect(submission.reload).not_to be_excused
1890+
expect(json['excused']).to eq false
1891+
end
18511892
end
18521893

18531894
it "should allow posting grade by sis id" do

0 commit comments

Comments
 (0)