add submission posted notification

This replaces the Assignment Unmuted notification for students and
observers with a Submission Posted notification

fixes GRADE-2377

Test Plan

In Old Gradebook
- Create an assignment.
- Mute it.
- Unmute it.
- Verify an "Assignment Unmuted" notification occurs for the student.
  Use `your_canvas.com/users/:USER_ID/messages` as a quick way to
  check.

In New Gradebook
- Create an assignment with a manual post policy (M1).
- Create another assignment with a manual post policy (M2).
- Create an assignment with an automatic post policy (A1).

- Grade M1.
- Verify no notifications were sent out to the student.
- Post grades for M1.
- Verify that the student received a Submission Posted notification
  and that they did not receive an Assignment Unmuted notification.

- Grade M2.
- Change M2's post policy to be automatic.
- Verify no notifications were sent out to the student.
- Post grades for M2.
- Verify that the student received a Submission Posted notification
  and that they did not receive an Assignment Unmuted notification.

- Grade A1.
- Verify that the student did not receive a Submission Posted
  notification.

Change-Id: I1a5132a6ca1a9536e890768dc67a4814bb2448f9
Reviewed-on: https://gerrit.instructure.com/208161
Tested-by: Jenkins
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Keith Garner <kgarner@instructure.com>
QA-Review: Derek Bender <djbender@instructure.com>
Product-Review: Jonathan Fenton <jfenton@instructure.com>

app/messages/assignment_unmuted.email.erb

@@ -1,16 +1,12 @@
-<%
-  assignment = asset.is_a?(Assignment) ? asset : asset.assignment
-%>
-
 <% define_content :link do %>
-  <%= polymorphic_url([assignment.context, assignment]) %>
+  <%= polymorphic_url([asset.context, asset]) %>
 <% end %>
 
 <% define_content :subject do %>
-  <%= t :subject, "Assignment Unmuted: %{title}, %{course}", :title => assignment.title, :course => assignment.context.name %>
+  <%= t :subject, "Assignment Unmuted: %{title}, %{course}", :title => asset.title, :course => asset.context.name %>
 <% end %>
 
-<%= t :body, <<-BODY, :title => assignment.title, :course => assignment.context.name, :url => content(:link)
+<%= t :body, <<-BODY, :title => asset.title, :course => asset.context.name, :url => content(:link)
 Your instructor has released grade changes and new comments for %{title}. These changes are now viewable.
 
 Your can view it here:

app/messages/assignment_unmuted.email.html.erb

@@ -1,13 +1,9 @@
-<%
-  assignment = asset.is_a?(Assignment) ? asset : asset.assignment
-%>
-
 <% define_content :link do %>
-  <%= polymorphic_url([assignment.context, assignment]) %>
+  <%= polymorphic_url([asset.context, asset]) %>
 <% end %>
 
 <% define_content :subject do %>
-  <%= t :subject, "Assignment Unmuted: %{title}, %{course}", :title => assignment.title, :course => assignment.context.name %>
+  <%= t :subject, "Assignment Unmuted: %{title}, %{course}", :title => asset.title, :course => asset.context.name %>
 <% end %>
 
 <% define_content :footer_link do %>
@@ -16,4 +12,4 @@
   </a>
 <% end %>
 
-<p><%= t :body, "Your instructor has released grade changes and new comments for %{title}. These changes are now viewable.", :title => assignment.title %></p>
+<p><%= t :body, "Your instructor has released grade changes and new comments for %{title}. These changes are now viewable.", :title => asset.title %></p>

app/messages/assignment_unmuted.sms.erb

@@ -1,8 +1,4 @@
-<%
+<%= t :body, <<-BODY, :title => asset.title, :course => asset.context.name, :website => HostUrl.context_host(asset.context)
-  assignment = asset.is_a?(Assignment) ? asset : asset.assignment
-%>
-
-<%= t :body, <<-BODY, :title => assignment.title, :course => assignment.context.name, :website => HostUrl.context_host(assignment.context)
 Your instructor has released grades and comments for %{title}, %{course}.
 
 More info at %{website}

app/messages/assignment_unmuted.summary.erb

@@ -1,12 +1,7 @@
-<%
-  assignment = asset.is_a?(Assignment) ? asset : asset.assignment
-%>
-
 <% define_content :link do %>
-  <%= polymorphic_url([assignment.context, assignment]) %>
+  <%= polymorphic_url([asset.context, asset]) %>
 <% end %>
 
 <% define_content :subject do %>
-  <%= t :subject, "Grades and comments released for: %{title}, %{course}", :title => assignment.title, :course => assignment.context.name %>
+  <%= t :subject, "Grades and comments released for: %{title}, %{course}", :title => asset.title, :course => asset.context.name %>
 <% end %>
-

app/messages/notification_types.yml

@@ -132,6 +132,8 @@
       delay_for: <%= 60*60 %>
     - name: Submission Grade Changed
       delay_for: <%= 5*60 %>
+    - name: Submission Posted
+      delay_for: 0
     - name: Quiz Regrade Finished
       delay_for: 0
 

app/messages/submission_posted.email.erb

@@ -0,0 +1,17 @@
+<% assignment = asset.assignment %>
+
+<% define_content :link do %>
+  <%= polymorphic_url([assignment.context, assignment, asset]) %>
+<% end %>
+
+<% define_content :subject do %>
+  <%= t "Submission Posted: %{title}, %{course}", title: assignment.title, course: assignment.context.name %>
+<% end %>
+
+<%= t <<-BODY, title: assignment.title, course: assignment.context.name, url: content(:link)
+Your instructor has released grade changes and new comments for %{title}. These changes are now viewable.
+
+Your can view it here:
+%{url}
+BODY
+%>

app/messages/submission_posted.email.html.erb

@@ -0,0 +1,17 @@
+<% assignment = asset.assignment %>
+
+<% define_content :link do %>
+  <%= polymorphic_url([assignment.context, assignment, asset]) %>
+<% end %>
+
+<% define_content :subject do %>
+  <%= t "Submission Posted: %{title}, %{course}", title: assignment.title, course: assignment.context.name %>
+<% end %>
+
+<% define_content :footer_link do %>
+  <a href="<%= content(:link) %>">
+    <%= t :link, "You can view it here" %>
+  </a>
+<% end %>
+
+<p><%= t :body, "Your instructor has released grade changes and new comments for %{title}. These changes are now viewable.", :title => assignment.title %></p>

app/messages/submission_posted.sms.erb

@@ -0,0 +1,8 @@
+<% assignment = asset.assignment %>
+
+<%= t <<-BODY, title: assignment.title, course: assignment.context.name, url: polymorphic_url([assignment.context, assignment, asset])
+Your instructor has released grade changes and new comments for %{title}, %{course}.
+
+More info at %{url}
+BODY
+%>

app/messages/submission_posted.summary.erb

@@ -0,0 +1,9 @@
+<% assignment = asset.assignment %>
+
+<% define_content :link do %>
+  <%= polymorphic_url([asset.context, assignment, asset]) %>
+<% end %>
+
+<% define_content :subject do %>
+  <%= t "Grade changes and new comments released for: %{title}, %{course}", title: assignment.title, course: assignment.context.name %>
+<% end %>

app/models/broadcast_policies/submission_policy.rb

@@ -72,13 +72,10 @@ module BroadcastPolicies
       user_has_visibility?
     end
 
-    def should_dispatch_assignment_unmuted?
+    def should_dispatch_submission_posted?
-      # This is no longer handled by the assignment in the Post Policies era,
+      # When Post Policies aren't enabled, the assignment handles notifications
-      # as subsets of submissions can be posted at a time. Re-using an existing
+      # via the Assignment Unmuted notification.
-      # Notification while hotfixing, but eventually this deserves its own
-      # submission posted Notification.
       return false unless assignment.course.post_policies_enabled? &&
-        assignment.post_manually? &&
         submission.grade_posting_in_progress
 
       submission.reload

app/models/submission.rb

@@ -1786,11 +1786,11 @@ class Submission < ActiveRecord::Base
         should_dispatch_submission_grade_changed?
     }
 
-    p.dispatch :assignment_unmuted
+    p.dispatch :submission_posted
     p.to { [student] + User.observing_students_in_course(student, assignment.context) }
     p.whenever { |submission|
       BroadcastPolicies::SubmissionPolicy.new(submission).
-        should_dispatch_assignment_unmuted?
+        should_dispatch_submission_posted?
     }
 
   end

db/migrate/20190828164410_add_submission_posted_notification.rb

@@ -0,0 +1,34 @@
+#
+# Copyright (C) 2019 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+
+class AddSubmissionPostedNotification < ActiveRecord::Migration[5.2]
+  tag :predeploy
+
+  def up
+    return unless Shard.current == Shard.default
+    Canvas::MessageHelper.create_notification({
+      name: "Submission Posted",
+      delay_for: 0,
+      category: "Grading"
+    })
+  end
+
+  def down
+    return unless Shard.current == Shard.default
+    Notification.where(name: "Submission Posted").delete_all
+  end
+end

spec/messages/submission_posted.erb_spec.rb

@@ -0,0 +1,88 @@
+#
+# Copyright (C) 2019 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+require "spec_helper"
+require_relative "messages_helper"
+
+describe "submission_posted" do
+  let_once(:asset) { submission }
+  let_once(:assignment) { course.assignments.create!(title: "assignment 1") }
+  let_once(:course) { Course.create!(name: "course 1") }
+  let_once(:notification_name) { :submission_posted }
+  let_once(:student) { course.enroll_student(User.create!, enrollment_state: :active).user }
+  let_once(:submission) { assignment.submissions.find_by!(user: student) }
+  let_once(:submission_url) { "/courses/#{course.id}/assignments/#{assignment.id}/submissions/#{student.id}" }
+
+  context "email" do
+    let_once(:path_type) { :email }
+
+    it "includes a message subject" do
+      message = generate_message(notification_name, path_type, asset, {})
+      expect(message.subject).to eql "Submission Posted: assignment 1, course 1"
+    end
+
+    it "includes a message body" do
+      message = generate_message(notification_name, path_type, asset, {})
+      expect(message.body).to include "Your instructor has released grade changes and new comments"
+    end
+
+    it "includes a link to the submission" do
+      message = generate_message(notification_name, path_type, asset, {})
+      expect(message.body).to include submission_url
+    end
+
+    it "html includes a message body" do
+      message = generate_message(notification_name, path_type, asset, {})
+      expect(message.html_body).to include "Your instructor has released grade changes and new comments"
+    end
+
+    it "html includes a link to the submission" do
+      message = generate_message(notification_name, path_type, asset, {})
+      expect(message.html_body).to include submission_url
+    end
+  end
+
+  context "sms" do
+    let_once(:path_type) { :sms }
+
+    it "includes a message subject" do
+      message = generate_message(notification_name, path_type, asset, {})
+      expect(message.subject).to eql "Canvas Alert"
+    end
+
+    it "includes a message body" do
+      message = generate_message(notification_name, path_type, asset, {})
+      expect(message.body).to include "Your instructor has released grade changes and new comments"
+    end
+
+    it "includes a link to the submission" do
+      message = generate_message(notification_name, path_type, asset, {})
+      expect(message.body).to include submission_url
+    end
+  end
+
+  context "summary" do
+    let_once(:path_type) { :summary }
+
+    it "includes a message subject" do
+      message = generate_message(notification_name, path_type, asset, {})
+      expected_subject = "Grade changes and new comments released for: #{assignment.title}, #{course.name}"
+      expect(message.subject).to eql expected_subject
+    end
+  end
+end

spec/models/broadcast_policies/submission_policy_spec.rb

@@ -199,12 +199,12 @@ module BroadcastPolicies
       specify { wont_send_when{ allow(policy).to receive(:user_has_visibility?).and_return(false)}}
     end
 
-    describe "#should_dispatch_assignment_unmuted?" do
+    describe "#should_dispatch_submission_posted?" do
       let_once(:course) { Course.create! }
+      let_once(:student) { User.create! }
       let(:assignment) { course.assignments.create! }
       let(:policy) { SubmissionPolicy.new(submission) }
       let(:submission) { assignment.submissions.find_by(user: student) }
-      let_once(:student) { User.create! }
 
       before(:once) do
         course.enroll_student(student)
@@ -219,27 +219,27 @@ module BroadcastPolicies
       it "returns true when the submission is being posted and the assignment posts manually" do
         submission.update!(posted_at: Time.zone.now)
         submission.grade_posting_in_progress = true
-        expect(policy.should_dispatch_assignment_unmuted?).to be true
+        expect(policy.should_dispatch_submission_posted?).to be true
       end
 
-      it "returns false when the submission is being posted and the assignment posts automatically" do
+      it "returns true when the submission is being posted and the assignment posts automatically" do
         assignment.ensure_post_policy(post_manually: false)
         submission.update!(posted_at: Time.zone.now)
         submission.grade_posting_in_progress = true
-        expect(policy.should_dispatch_assignment_unmuted?).to be false
+        expect(policy.should_dispatch_submission_posted?).to be true
       end
 
       it "returns false when Post Policies are not enabled" do
         course.disable_feature!(:new_gradebook)
         submission.update!(posted_at: Time.zone.now)
         submission.grade_posting_in_progress = true
-        expect(policy.should_dispatch_assignment_unmuted?).to be false
+        expect(policy.should_dispatch_submission_posted?).to be false
       end
 
       it "returns false when the submission was posted longer than an hour ago" do
         submission.update!(posted_at: 2.hours.ago)
         submission.grade_posting_in_progress = true
-        expect(policy.should_dispatch_assignment_unmuted?).to be false
+        expect(policy.should_dispatch_submission_posted?).to be false
       end
     end
   end

spec/models/submission_spec.rb

@@ -1534,39 +1534,76 @@ describe Submission do
         expect(submission.messages_sent.keys).to eq ['Group Assignment Submitted Late']
       end
 
-      context "Assignment Unmuted" do
+      context "Submission Posted" do
         let(:submission) { @assignment.submissions.find_by!(user: @student) }
+        let(:submission_posted_messages) do
+          Message.where(
+            communication_channel: @student.email_channel,
+            notification: @submission_posted_notification
+          )
+        end
 
         before(:once) do
-          @assignment_unmuted_notification = Notification.create!(name: "Assignment Unmuted")
+          @submission_posted_notification = Notification.find_or_create_by(
+            category: "Grading",
+            name: "Submission Posted"
+          )
           @course.enable_feature!(:new_gradebook)
           PostPolicy.enable_feature!
           @student.update!(email: "fakeemail@example.com")
           @student.email_channel.update!(workflow_state: :active)
         end
 
-        it "sends a notification when a submission is posted and assignment posts manually" do
+        it "does not send a notification when a submission is not being posted" do
-          @assignment.ensure_post_policy(post_manually: true)
+          expect { submission.update!(body: "hello") }.not_to change { submission_posted_messages.count }
-
-          expect {
-            @assignment.post_submissions(submission_ids: [submission.id])
-          }.to change {
-            DelayedMessage.where(
-              communication_channel: @student.email_channel,
-              notification: @assignment_unmuted_notification
-            ).count
-          }.by(1)
         end
 
-        it "does not send a notification when a submission is posted and assignment posts automatically" do
+        context "when grade_posting_in_progress is true" do
-          expect {
+          before(:each) do
-            @assignment.grade_student(@student, grader: @teacher, score: 10)
+            submission.grade_posting_in_progress = true
-          }.not_to change {
+          end
-            DelayedMessage.where(
+
-              communication_channel: @student.email_channel,
+          it "sends a notification when a submission is posted and assignment posts manually" do
-              notification: @assignment_unmuted_notification
+            @assignment.ensure_post_policy(post_manually: true)
-            ).count
+
-          }
+            expect {
+              submission.update!(posted_at: Time.zone.now)
+            }.to change {
+              submission_posted_messages.count
+            }.by(1)
+          end
+
+          it "sends a notification when a submission is posted and assignment posts automatically" do
+            expect {
+              submission.update!(posted_at: Time.zone.now)
+            }.to change {
+              submission_posted_messages.count
+            }.by(1)
+          end
+        end
+
+        context "when grade_posting_in_progress is false" do
+          before(:each) do
+            submission.grade_posting_in_progress = false
+          end
+
+          it "does not send a notification when a submission is posted and assignment posts manually" do
+            @assignment.ensure_post_policy(post_manually: true)
+
+            expect {
+              submission.update!(posted_at: Time.zone.now)
+            }.not_to change {
+              submission_posted_messages.count
+            }
+          end
+
+          it "does not send a notification when a submission is posted and assignment posts automatically" do
+            expect {
+              submission.update!(posted_at: Time.zone.now)
+            }.not_to change {
+              submission_posted_messages.count
+            }
+          end
         end
       end
     end