fix anonymous peer review checkbox on graded discussion form.

fixes CNVS-19782

The checkbox used to determine whether or not an assignment's peer
reviews should be considered anonymous, which is part of a backbone
view, did not function properly if it was 'nested' (i.e., if the name on
the field should be `assiginment[anonymous_peer_reviews]`, instead of just
`anonymous_peer_reviews`. The `checkbox` helper is already setup to
handle this; we just needed to pass the `nested` value to it, and to
update how we determine whether or not the checkbox should appear
checked, so as to avoid manually setting the checkbox's `name`
attribute.

Additionally, this patch fixes a bug in the
`RubricAssessment#considered_anonymous?` method, which would blow up if
the model did not have a related rubric_association. I've added a
protection to two other methods that would have thrown exceptions in the
absence of a rubric_assocation as well.

Finally, I updated the logic in the SubmissionComment policy
declaration, so that when trying to `:read_author`, a student can always
see the name of a teacher, even if the comment is marked as anonymous.

test plan:
- as a teacher, create a graded discussion topic. When doing say, enable
  peer reviews, and enable anonymous peer reviews.
- as a student, submit an entry for the graded discussion topic.
- as a teacher, assign a peer review to a student for the submission
  above.
- as the reviewer student, submit a review.
- as the reviewed student, navigate to the submission details page
  (/courses/:course_id/assignments/:assignment_id/submissions/:user_id)
  and observe the comment list on the right hand side of the page.
- observe that the name of the reviewer student is not present, and
  instead it says 'Anonymous User'.
- as the teacher, navigate to the same submission page, and observe the
  comment list on the right hand side of the page.
- observe that the name of the reviewer student IS present.
- as the teacher, navigate to the same submission page, and observe the
  comment list on the right hand side of the page
- as the teacher, leave a comment on the submitted assignment.
- as the reviewed student, navigate to the submission details page, and
  observe the comment list on the right hand side of the page.
- observe that the name of the reviewer teacher IS present.

Change-Id: I572dd7fa319cd784e59f00057898fcea5349c899
Reviewed-on: https://gerrit.instructure.com/52099
Tested-by: Jenkins
Reviewed-by: Mike Nomitch <mnomitch@instructure.com>
QA-Review: Adam Stone <astone@instructure.com>
Product-Review: Hilary Scharton <hilary@instructure.com>
This commit is contained in:
John Corrigan 2015-04-13 18:10:02 -05:00
parent 6176943fe4
commit ad52cabec3
5 changed files with 46 additions and 5 deletions

View File

@ -53,7 +53,14 @@ class RubricAssessment < ActiveRecord::Base
def update_outcomes_for_assessment(outcome_ids=[])
return if outcome_ids.empty?
alignments = self.rubric_association.association_object.learning_outcome_alignments.where(learning_outcome_id: outcome_ids)
alignments = if self.rubric_association.present?
self.rubric_association.association_object.learning_outcome_alignments.where({
learning_outcome_id: outcome_ids
})
else
[]
end
(self.data || []).each do |rating|
if rating[:learning_outcome_id]
alignments.each do |alignment|
@ -125,7 +132,13 @@ class RubricAssessment < ActiveRecord::Base
def update_assessment_requests
requests = self.assessment_requests
requests += self.rubric_association.assessment_requests.where(assessor_id: self.assessor_id, asset_id: self.artifact_id, asset_type: self.artifact_type)
if self.rubric_association.present?
requests += self.rubric_association.assessment_requests.where({
assessor_id: self.assessor_id,
asset_id: self.artifact_id,
asset_type: self.artifact_type
})
end
requests.each { |a|
a.attributes = {:rubric_assessment => self, :assessor => self.assessor}
a.complete
@ -202,6 +215,7 @@ class RubricAssessment < ActiveRecord::Base
end
def considered_anonymous?
return false unless self.rubric_association.present?
self.rubric_association.association_type == 'Assignment' &&
self.rubric_association.association_object.anonymous_peer_reviews?
end

View File

@ -103,7 +103,8 @@ class SubmissionComment < ActiveRecord::Base
given { |user, session|
!self.anonymous? ||
self.author == user ||
self.submission.assignment.context.grants_right?(user, session, :view_all_grades)
self.submission.assignment.context.grants_right?(user, session, :view_all_grades) ||
self.submission.assignment.context.grants_right?(self.author, session, :view_all_grades)
}
can :read_author
end

View File

@ -97,10 +97,11 @@
</div>
<label class="checkbox" for="anonymous_peer_reviews">
{{checkbox "anonymousPeerReviews"
{{checkbox "anonymous_peer_reviews"
checked=anonymousPeerReviews
id="anonymous_peer_reviews"
name="anonymous_peer_reviews"
aria-controls="anonymous_peer_reviews"
prefix=prefix
disabled=peerReviewsFrozen}}
{{#t "labels.anonymous_peer_reviews"}}
Peer Reviews Appear Anonymously

View File

@ -155,5 +155,21 @@ describe RubricAssessment do
expect(@assessment.grants_right?(@teacher, :read_assessor)).to be_truthy
end
end
describe "#considered_anonymous?" do
let_once(:assessment) {
RubricAssessment.create!({
artifact: @assignment.find_or_create_submission(@student),
assessment_type: 'peer_review',
assessor: student_in_course(active_all: true).user,
rubric: @rubric,
user: @student
})
}
it "should not blow up without a rubric_association" do
expect{assessment.considered_anonymous?}.not_to raise_error
end
end
end
end

View File

@ -193,6 +193,11 @@ This text has a http://www.google.com link in it...
author: @student2,
comment: "My peer review comment."
})
@teacher_comment = @submission.add_comment({
author: @teacher,
comment: "My teacher review comment."
})
end
it "should mark submission comment as anonymous" do
@ -206,6 +211,10 @@ This text has a http://www.google.com link in it...
it "should allow teacher to see reviewer name" do
expect(@reviewer_comment.grants_right?(@teacher, :read_author)).to be_truthy
end
it "should allow reviewed to see a teacher comment" do
expect(@teacher_comment.grants_right?(@student1, :read_author)).to be_truthy
end
end
end