fix peer review notification bug

When students have the 'Invitation' notification preference enabled and
they are enrolled in an unpublished course, Canvas would send them
notifications for Collaborations and Peer-Reviews that they were invited
to. Also, students were able to access the collaboration before the
course was published by clicking on the link sent in the notication.

Test Plan:
- Create an unpublished course with at least one teacher and two
  students. Set the student's "Invitation" notification preference to
  ASAP.
- You can accept the course invitation for the students by updating the
  enrollment state in the rails console
- As the teacher, navigate to the unpublished course
- Create a Collaboration or an Assignment with Peer Reviews (don't
  publish the assignment)
- Manually assign a student a peer-review
- Check the student's email or /users/<id>/messages
- Note that there is no notification of the peer review
- Publish the Course but not the assignment
- Unassign and reassign the student a peer-review
- Again check the student's email or /users/<id>/messages
- Note that there is no notification of the peer review
- Publish the assignment and unasign/reasign a peer review
- Again check the student's email or /users/<id>/messages
- Note that now there is a notification of the peer review

fixes KNO-213
flag=none

Change-Id: Ic95b00523ba6acb05a5cee64447c81dcb259b3ce
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/223575
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Steven Burnett <sburnett@instructure.com>
QA-Review: Steven Burnett <sburnett@instructure.com>
Product-Review: Steven Burnett <sburnett@instructure.com>

app/models/assessment_request.rb

@@ -58,7 +58,12 @@ class AssessmentRequest < ActiveRecord::Base
     p.dispatch :peer_review_invitation
     p.to { self.assessor }
     p.whenever { |record|
-      record.assigned? && @send_reminder && !rubric_association
+      send_notification = record.assigned? && @send_reminder && !rubric_association
+      # Do not send notifications if the context is an unpublished course
+      # or if the asset is a submission and the assignment is unpublished
+      send_notification = false if self.context.is_a?(Course) && !self.context.workflow_state.in?(['available', 'completed'])
+      send_notification = false if self.asset.is_a?(Submission) && self.asset.assignment.workflow_state != "published"
+      send_notification
     }
   end
 

app/models/collaborator.rb

@@ -28,8 +28,12 @@ class Collaborator < ActiveRecord::Base
     p.to {
       users = self.group_id.nil? ? [self.user] : self.group.users - [self.user]
       if self.context.is_a?(Course)
-        enrolled_user_ids = self.context.enrollments.active_by_date.where(:user_id => users).pluck(:user_id).to_set
-        users = users.select{|u| enrolled_user_ids.include?(u.id)}
+        if !self.context.workflow_state.in?(['available', 'completed'])
+          users = [] # do not send notifications to any users if the course is unpublished
+        else
+          enrolled_user_ids = self.context.enrollments.active_by_date.where(:user_id => users).pluck(:user_id).to_set
+          users = users.select{|u| enrolled_user_ids.include?(u.id)}
+        end
       end
       if self.collaboration.collaboration_type == 'google_docs'
         users.map(&:gmail_channel)

spec/models/assessment_request_spec.rb

@@ -40,6 +40,40 @@ describe AssessmentRequest do
     end
   end
 
+  describe 'peer review invitations' do
+    before :once do
+      @student.communication_channels.create!(:path => 'test@example.com').confirm!
+      @notification_name = "Peer Review Invitation"
+      notification = Notification.create!(:name => @notification_name, :category => 'Invitation')
+      NotificationPolicy.create!(:notification => notification, :communication_channel => @student.communication_channel, :frequency => 'immediately')
+    end
+
+    it 'should send a notification if the course and assignment are published' do
+      @request.send_reminder!
+      expect(@request.messages_sent.keys).to include(@notification_name)
+    end
+
+    it 'should not send a notification if the course is unpublished' do
+      submission = @assignment.find_or_create_submission(@user)
+      assessor_submission = @assignment.find_or_create_submission(@review_student)
+      @course.update!(workflow_state: 'created')
+      peer_review_request = AssessmentRequest.create!(user: @submission_student, asset: submission, assessor_asset: assessor_submission, assessor: @review_student)
+      peer_review_request.send_reminder!
+
+      expect(peer_review_request.messages_sent.keys).to be_empty
+    end
+
+    it 'should not send a notification if the assignment is unpublished' do
+      @assignment.update!(workflow_state: 'unpublished')
+      submission = @assignment.find_or_create_submission(@user)
+      assessor_submission = @assignment.find_or_create_submission(@review_student)
+      peer_review_request = AssessmentRequest.create!(user: @submission_student, asset: submission, assessor_asset: assessor_submission, assessor: @review_student)
+      peer_review_request.send_reminder!
+
+      expect(peer_review_request.messages_sent.keys).to be_empty
+    end
+  end
+
   describe "notifications" do
 
     let(:notification_name) { 'Rubric Assessment Submission Reminder' }

spec/models/collaborator_spec.rb

@@ -63,10 +63,21 @@ describe Collaborator do
         messages_sent.keys).to include 'Collaboration Invitation'
     end
 
+    it 'should not notify members of a group that have not accepted the course enrollemnt' do
+      group = group_model(:name => 'Test group', :context => @course)
+      user = user_with_pseudonym(:active_all => true)
+      @course.enroll_student(user)
+      group.add_user(user, 'active')
+      @collaboration.update_members([], [group.id])
+      expect(@collaboration.collaborators.detect { |c| c.group_id.present? }.
+        messages_sent.keys).to be_empty
+    end
+
     it 'should not notify members of a group in an unpublished course' do
       group = group_model(:name => 'Test group', :context => @course)
       user = user_with_pseudonym(:active_all => true)
       @course.enroll_student(user)
+      user.enrollments.first.accept!
       @course.update_attribute(:workflow_state, 'claimed')
       group.add_user(user, 'active')
       @collaboration.update_members([], [group.id])