From ad52cabec3e43f2b1534f9ca0468c12a26e5d414 Mon Sep 17 00:00:00 2001 From: John Corrigan Date: Mon, 13 Apr 2015 18:10:02 -0500 Subject: [PATCH] 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 QA-Review: Adam Stone Product-Review: Hilary Scharton --- app/models/rubric_assessment.rb | 18 ++++++++++++++++-- app/models/submission_comment.rb | 3 ++- .../assignments/PeerReviewsSelector.handlebars | 5 +++-- spec/models/rubric_assessment_spec.rb | 16 ++++++++++++++++ spec/models/submission_comment_spec.rb | 9 +++++++++ 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/app/models/rubric_assessment.rb b/app/models/rubric_assessment.rb index 8ab60089fd6..54f1a40516e 100644 --- a/app/models/rubric_assessment.rb +++ b/app/models/rubric_assessment.rb @@ -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 diff --git a/app/models/submission_comment.rb b/app/models/submission_comment.rb index 8825689e6a3..bb053e2f405 100644 --- a/app/models/submission_comment.rb +++ b/app/models/submission_comment.rb @@ -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 diff --git a/app/views/jst/assignments/PeerReviewsSelector.handlebars b/app/views/jst/assignments/PeerReviewsSelector.handlebars index bbee071bb86..0b864bdeac6 100644 --- a/app/views/jst/assignments/PeerReviewsSelector.handlebars +++ b/app/views/jst/assignments/PeerReviewsSelector.handlebars @@ -97,10 +97,11 @@