Skip to content

Commit

Permalink
prevent posting of moderated assignment with unselected grades
Browse files Browse the repository at this point in the history 
Moderated assignments will no longer allow posting of grades
unless all submissions with multiple provisional grades have one
of those grades selected.

closes CNVS-30040

Test Plan:
 1. Select or create:
    a. a course
    b. four students for the course
    c. two users with grading roles for the course
    d. a moderated assignment
 2. Add a reviewer to the assignment for all students
 3. As the last student, create a submission for the assignment
 4. As the first grader, grade all three students
 5. As the second grader, grade the first two students
 6. As the first grader, visit the assignment 'Moderate' page
 7. Attempt to Post the grades
 8. Verify:
    a. grades are not posted
    b. a message indicates the reason to the user
 9. Select either grade for ONLY the first student
10. Attempt to Post the grades
11. Verify:
    a. grades are not posted
    b. a message indicates the reason to the user
12. Select a grade for the second student
13. Attempt to Post the grades
14. Verify:
    a. grades ARE posted
    b. a message indicates success to the user
    c. the first and second students received the selected grades
    d. the third student received the one provisional grade
    e. the fourth student received no grade

Change-Id: I3e7361ece814fe6eae988dd35ea268fcad0d5935
Reviewed-on: https://gerrit.instructure.com/84412
Reviewed-by: Spencer Olson <[email protected]>
Tested-by: Jenkins
Reviewed-by: Neil Gupta <[email protected]>
QA-Review: Amber Taniuchi <[email protected]>
Product-Review: Christi Wruck
  • Loading branch information
@jneander
jneander committed Jul 18, 2016
1 parent df03406 commit 294ecc5
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 24 deletions.
24 changes: 17 additions & 7 deletions app/controllers/provisional_grades_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,12 @@ def publish
submissions = @assignment.submissions.preload(:all_submission_comments,
{ :provisional_grades => :rubric_assessments })
selections = @assignment.moderated_grading_selections.index_by(&:student_id)
submissions.each do |submission|

graded_submissions = submissions.select do |submission|
submission.provisional_grades.any?
end

grades_to_publish = graded_submissions.map do |submission|
if (selection = selections[submission.user_id])
# student in moderation: choose the selected provisional grade
selected_provisional_grade = submission.provisional_grades
Expand All @@ -215,16 +220,21 @@ def publish

# either the student is not in moderation, or not all provisional grades were entered
# choose the first one with a grade (there should only be one)
selected_provisional_grade ||= submission.provisional_grades
.select { |pg| pg.graded_at.present? }
.sort_by(&:created_at)
.first
unless selected_provisional_grade
provisional_grades = submission.provisional_grades
.select { |pg| pg.graded_at.present? }
selected_provisional_grade = provisional_grades.first if provisional_grades.count == 1
end

if selected_provisional_grade
selected_provisional_grade.publish!
unless selected_provisional_grade
return render json: { message: "All submissions must have a selected grade" },
status: :unprocessable_entity
end

selected_provisional_grade
end

grades_to_publish.each(&:publish!)
@context.touch_admins_later # just in case nothing got published
@assignment.update_attribute(:grades_published_at, Time.now.utc)
render :json => { :message => "OK" }
Expand Down
13 changes: 8 additions & 5 deletions app/jsx/assignments/actions/ModerationActions.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,14 @@ define([
dispatch(this.publishedGrades(I18n.t('Success! Grades were published to the grade book.')));
})
.catch((response) => {
if (response.status === 400) {
dispatch(this.publishGradesFailed(I18n.t('Assignment grades have already been published.')));
} else {
dispatch(this.publishGradesFailed(I18n.t('An error occurred publishing grades.')));
}
const errorMessages = {
400: I18n.t('Assignment grades have already been published.'),
422: I18n.t('All submissions must have a selected grade.')
};
let message =
errorMessages[response.status] ||
I18n.t('An error occurred publishing grades.');
dispatch(this.publishGradesFailed(message));
});
};
},
Expand Down
61 changes: 58 additions & 3 deletions spec/apis/v1/provisional_grades_api_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,20 +228,75 @@
end
end

context "with partial provisional grades" do
it "publishes the first provisional grade if none have been explicitly selected" do
context "with one provisional grade" do
it "publishes the only provisional grade if none have been explicitly selected" do
course_with_user("TaEnrollment", course: @course, active_all: true)
second_ta = @user
@submission = @assignment.submit_homework(@student, body: "hello")
@assignment.moderated_grading_selections.create!(student: @student)
@assignment.grade_student(@student, grader: @ta, score: 72, provisional: true)
@assignment.grade_student(@student, grader: second_ta, score: 88, provisional: true)

api_call_as_user(@teacher, :post, @path, @params)

expect(@submission.reload.score).to eq 72
end
end

context "with multiple provisional grades" do
before(:once) do
course_with_user("TaEnrollment", course: @course, active_all: true)
@second_ta = @user
end

it "publishes even when some submissions have no grades" do
@submission = @assignment.submit_homework(@student, body: "hello")
@assignment.moderated_grading_selections.create!(student: @student)

@user = @teacher
raw_api_call(:post, @path, @params)

expect(response.response_code).to eq(200)
expect(@submission.reload.score).to be_nil
expect(@assignment.reload.grades_published_at).not_to be_nil
end

it "does not publish if none have been explicitly selected" do
@submission = @assignment.submit_homework(@student, body: "hello")
@assignment.moderated_grading_selections.create!(student: @student)
@assignment.grade_student(@student, grader: @ta, score: 72, provisional: true)
@assignment.grade_student(@student, grader: @second_ta, score: 88, provisional: true)

@user = @teacher
raw_api_call(:post, @path, @params)

expect(response.response_code).to eq(422)
expect(@submission.reload).not_to be_graded
expect(@assignment.reload.grades_published_at).to be_nil
end

it "does not publish any if not all have been explicitly selected" do
student_1 = @student
student_2 = student_in_course(active_all: true, course: @course).user
submission_1 = @assignment.submit_homework(student_1, body: "hello")
submission_2 = @assignment.submit_homework(student_2, body: "hello")
selection_1 = @assignment.moderated_grading_selections.create!(student: student_1)
@assignment.moderated_grading_selections.create!(student: student_2)
@assignment.grade_student(student_1, grader: @ta, score: 12, provisional: true)
@assignment.grade_student(student_1, grader: @second_ta, score: 34, provisional: true)
@assignment.grade_student(student_2, grader: @ta, score: 56, provisional: true)
@assignment.grade_student(student_2, grader: @second_ta, score: 78, provisional: true)

selection_1.update_attribute(:selected_provisional_grade_id, submission_1.provisional_grade(@ta))

@user = @teacher
raw_api_call(:post, @path, @params)

expect(response.response_code).to eq(422)
expect(submission_1.reload).not_to be_graded
expect(submission_2.reload).not_to be_graded
expect(@assignment.reload.grades_published_at).to be_nil
end
end
end
end
end
37 changes: 28 additions & 9 deletions spec/coffeescripts/jsx/assignments/actions/ModerationActionsSpec.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ define [
payload:
students: ['test']

sinon.stub(@client, 'get').returns(Promise.resolve(fakeResponse))
@stub(@client, 'get').returns(Promise.resolve(fakeResponse))
ModerationActions.apiGetStudents(@client)((action) ->
deepEqual action, gotStudentsAction
start()
Expand All @@ -174,7 +174,7 @@ define [
fakeResponse = {data: ['test'], headers: fakeHeaders}

callCount = 0
sinon.stub(@client, 'get').returns(Promise.resolve(fakeResponse))
@stub(@client, 'get').returns(Promise.resolve(fakeResponse))
ModerationActions.apiGetStudents(@client)((action) ->
callCount++
if callCount >= 2
Expand Down Expand Up @@ -206,7 +206,7 @@ define [
payload:
message: 'Success! Grades were published to the grade book.'

sinon.stub(@client, 'post').returns(Promise.resolve(fakeResponse))
@stub(@client, 'post').returns(Promise.resolve(fakeResponse))
ModerationActions.publishGrades(@client)((action) ->
equal action.type, publishGradesAction.type, 'type matches'
equal action.payload.message, publishGradesAction.payload.message, 'has proper message'
Expand All @@ -225,7 +225,26 @@ define [
payload:
message: 'Assignment grades have already been published.'

sinon.stub(@client, 'post').returns(Promise.reject(fakeResponse))
@stub(@client, 'post').returns(Promise.reject(fakeResponse))
ModerationActions.publishGrades(@client)((action) ->
equal action.type, publishGradesAction.type, 'type matches'
equal action.payload.message, publishGradesAction.payload.message, 'has proper message'
start()
, getState)

asyncTest "dispatches publishGradesFailed action with selected grades message on 422 failure", ->
getState = ->
urls:
publish_grades_url: 'some_url'
fakeResponse =
status: 422

publishGradesAction =
type: ModerationActions.PUBLISHED_GRADES_FAILED
payload:
message: 'All submissions must have a selected grade.'

@stub(@client, 'post').returns(Promise.reject(fakeResponse))
ModerationActions.publishGrades(@client)((action) ->
equal action.type, publishGradesAction.type, 'type matches'
equal action.payload.message, publishGradesAction.payload.message, 'has proper message'
Expand All @@ -244,7 +263,7 @@ define [
payload:
message: 'An error occurred publishing grades.'

sinon.stub(@client, 'post').returns(Promise.reject(fakeResponse))
@stub(@client, 'post').returns(Promise.reject(fakeResponse))
ModerationActions.publishGrades(@client)((action) ->
equal action.type, publishGradesAction.type, 'type matches'
equal action.payload.message, publishGradesAction.payload.message, 'has proper message'
Expand Down Expand Up @@ -282,7 +301,7 @@ define [
payload:
message: 'Reviewers successfully added'

fakePost = sinon.stub(@client, 'post').returns(Promise.resolve(fakeResponse))
fakePost = @stub(@client, 'post').returns(Promise.resolve(fakeResponse))
ModerationActions.addStudentToModerationSet(@client)((action) ->
ok fakePost.calledWith(fakeUrl, {student_ids: [1, 2]}), 'called with the correct params'
equal action.type, moderationSetUpdatedAction.type, 'type matches'
Expand All @@ -307,7 +326,7 @@ define [
payload:
message: 'A problem occurred adding reviewers.'

sinon.stub(@client, 'post').returns(Promise.reject(fakeResponse))
@stub(@client, 'post').returns(Promise.reject(fakeResponse))
ModerationActions.addStudentToModerationSet(@client)((action) ->
equal action.type, moderationSetUpdateFailedAction.type, 'type matches'
equal action.payload.message, moderationSetUpdateFailedAction.payload.message, 'has proper message'
Expand Down Expand Up @@ -341,7 +360,7 @@ define [
student_id: 1
selected_provisional_grade_id: 42

fakePost = sinon.stub(@client, 'put').returns(Promise.resolve(fakeResponse))
fakePost = @stub(@client, 'put').returns(Promise.resolve(fakeResponse))
ModerationActions.selectProvisionalGrade(42, @client)((action) ->
ok fakePost.calledWith(fakeUrl+"/"+ "42"+"/select"), 'called with the correct params'
equal action.type, ModerationActions.SELECT_MARK, 'type matches'
Expand All @@ -362,7 +381,7 @@ define [
fakeResponse =
status: 404

fakePost = sinon.stub(@client, 'put').returns(Promise.resolve(fakeResponse))
fakePost = @stub(@client, 'put').returns(Promise.resolve(fakeResponse))
ModerationActions.selectProvisionalGrade(42, @client)((action) ->
ok fakePost.calledWith(fakeUrl+"/"+ "42"+"/select"), 'called with the correct params'
equal action.type, ModerationActions.SELECTING_PROVISIONAL_GRADES_FAILED, 'type matches'
Expand Down

0 comments on commit 294ecc5

Please sign in to comment.