Setting for observers to see student names in notifications

Test plan
- As an observer (who is observing a student
  either through the observer role linked to
  a student or through an observer enrollment
  linked to a student enrollment
- Verify that you can see the setting in user's
  notification settings
- Verify that when you send a submission graded
  or submission grade changed notification
  (email, html email, twitter, text, summary)
  you get the student's name in the notification
- Other notifications should be updated in the
  future, but will be on other commits

Change-Id: Ia05b42b8d0a80e5aa41de2cc5151caa258142fda
Reviewed-on: https://gerrit.instructure.com/167854
Tested-by: Jenkins
Product-Review: Matthew Goodwin <mattg@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>

app/coffeescripts/notifications/NotificationPreferences.js

@@ -69,6 +69,7 @@ export default class NotificationPreferences {
     this.channels = this.options.channels || []
     this.categories = this.options.categories || []
     this.policies = this.options.policies || []
+    this.showObservedNames = this.options.show_observed_names
 
     // Give each channel a 'name'
     this.channels.forEach((c) => {
@@ -172,6 +173,12 @@ export default class NotificationPreferences {
       channels: this.channels,
       eventGroups,
       buttonData: this.buttonData,
+      showObservedNames: {
+        available: this.showObservedNames != null,
+        name: 'send_observed_names_in_notifications',
+        on: this.showObservedNames,
+        label: I18n.t('Show name of observed students in notifications.'),
+      }
     }))
 
     // Display Bootstrap-like popover tooltip on category names. Allow entire cell to trigger popup.
@@ -226,7 +233,7 @@ export default class NotificationPreferences {
     $notificationPrefs.find('.user-pref-check').on('change', (e) => {
       const check = $(e.currentTarget)
       const checkStatus = check.attr('checked') === 'checked'
-      // Send user prefernce value to server
+      // Send user preference value to server
       const data = {user: {}}
       data.user[check.attr('name')] = checkStatus
       this.$notificationSaveStatus.disableWhileLoading($.ajaxJSON(this.updateUrl, 'PUT', data, null, () =>

app/controllers/profile_controller.rb

@@ -225,6 +225,7 @@ class ProfileController < ApplicationController
     @context = @user.profile
     @active_tab = 'notifications'
 
+
     # Get the list of Notification models (that are treated like categories) that make up the full list of Categories.
     full_category_list = Notification.dashboard_categories(@user)
     categories = full_category_list.map do |category|
@@ -242,6 +243,7 @@ class ProfileController < ApplicationController
       :policies => NotificationPolicy.setup_with_default_policies(@user, full_category_list).map { |p| notification_policy_json(p, @user, session).tap { |json| json[:communication_channel_id] = p.communication_channel_id } },
       :categories => categories,
       :update_url => communication_update_profile_path,
+      :show_observed_names => @user.observer_enrollments.any? || @user.as_observer_observation_links.any? ? @user.send_observed_names_in_notifications? : nil
       },
       :READ_PRIVACY_INFO => @user.preferences[:read_notification_privacy_info],
       :ACCOUNT_PRIVACY_NOTICE => @domain_root_account.settings[:external_notification_warning]

app/helpers/messages/send_student_names_helper.rb

@@ -0,0 +1,22 @@
+#
+# Copyright (C) 2018 - 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/>.
+
+module Messages::SendStudentNamesHelper
+  def send_student_names(asset, user)
+    user != asset.user && user.try(:send_observed_names_in_notifications?)
+  end
+end

app/messages/assignment_due_date_changed.email.erb

@@ -5,7 +5,7 @@
 <% define_content :subject do %>
   <%= t('assignment_due_date_changed', 'Assignment Due Date Changed: %{assignment_name}, %{course_name}', :assignment_name => asset.title, :course_name => asset.context.name) %>
 <% end %>
-      
+
 <%= t('assignment_due_date_changed_sentence', 'The due date for the assignment, %{assignment_name}, for the course, %{course_name}, has changed to:', :assignment_name => asset.title, :course_name => asset.context.name) %>
 
 <% if asset.due_at %>

app/messages/submission_grade_changed.email.erb

@@ -8,7 +8,11 @@
   <%= t :subject, "Grade Changed: %{assignment}, %{context}", :assignment => asset.assignment.title, :context => asset.assignment.context.name %>
 <% end %>
 
-<%= t :body, "The grade on your assignment, %{assignment_title} has been changed.", :assignment_title => asset.assignment.title %>
+<% if send_student_names(asset, user) %>
+<%= t "The grade on assignment %{assignment_title} has been changed for %{name}.", :assignment_title => asset.assignment.title, name: asset.user.name %>
+<% else %>
+<%= t :body, "The grade on your assignment %{assignment_title} has been changed.", :assignment_title => asset.assignment.title %>
+<% end %>
 
 <%= t :regraded_date, "re-graded: %{date}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")) %>
 <% if user.try(:send_scores_in_emails?, asset.assignment.context.root_account) %>

app/messages/submission_grade_changed.email.html.erb

@@ -12,7 +12,11 @@
   </a>
 <% end %>
 
-<p><%= t :body, "The grade on your assignment, %{assignment_title} has been changed.", :assignment_title => asset.assignment.title %></p>
+<% if send_student_names(asset, user) %>
+  <p><%= t "The grade on assignment %{assignment_title} has been changed for %{name}.", :assignment_title => asset.assignment.title, name: asset.user.name %></p>
+<% else %>
+  <p><%= t :body, "The grade on your assignment %{assignment_title} has been changed.", :assignment_title => asset.assignment.title %></p>
+<% end %>
 
 <p><%= t :regraded_date, "re-graded: %{date}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")) %></p>
 

app/messages/submission_grade_changed.sms.erb

@@ -1,4 +1,8 @@
-<%= t :body_sms, "Your grade for %{title}, %{context} just changed.", :title => asset.assignment.title, :context => asset.assignment.context.name %>
+<% if send_student_names(asset, user) %>
+<%= t "The grade for %{title} in %{context} for %{name} just changed.", :title => asset.assignment.title, :context => asset.assignment.context.name, name: asset.user.name %>
+<% else %>
+  <%= t :body_sms, "Your grade for %{title} in %{context} just changed.", :title => asset.assignment.title, :context => asset.assignment.context.name %>
+<% end %>
 
 <%= t :regraded_date, "re-graded: %{date}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")) %>
 <% if asset.score && user.try(:send_scores_in_emails?, asset.assignment.context.root_account) %>

app/messages/submission_grade_changed.summary.erb

@@ -6,6 +6,9 @@
   <%= t :subject, "Grade Changed: %{title}, %{context}", :title => asset.assignment.title, :context => asset.assignment.context.name %>
 <% end %>
 
+<% if send_student_names(asset, user) %>
+  <%= t "For %{name}", name: asset.user.name %>
+<% end %>
 <%= t :regraded_date, "re-graded: %{date}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")) %>
 <% if asset.score && user.try(:send_scores_in_emails?, asset.assignment.context.root_account) %>
   <%= t :score, "score:  %{score} out of %{total}", :score => asset.score, :total => (asset.assignment.points_possible || t(:not_applicable, "N/A")) %>

app/messages/submission_grade_changed.twitter.erb

@@ -1,7 +1,11 @@
 <% define_content :link do -%>
   <%= polymorphic_url([asset.assignment.context, asset.assignment, :submission], id: asset.user) %>
 <% end -%>
-<%= t :tweet, "Canvas Alert - Grade Change: %{date}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")) %>
+<% if send_student_names(asset, user) %>
+  <%= t "Canvas Alert - Grade Change: %{date} for %{name}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")), name: asset.user.name %>
+<% else %>
+  <%= t :tweet, "Canvas Alert - Grade Change: %{date}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")) %>
+<% end %>
 
 <% if asset.score -%>
 <%= t :score, "score:  %{score} out of %{total}", :score => asset.score, :total => (asset.assignment.points_possible || t(:not_applicable, "N/A")) %>

app/messages/submission_graded.email.erb

@@ -8,7 +8,11 @@
   <%= t :subject, "Assignment Graded: %{assignment}, %{context}", :assignment => asset.assignment.title, :context => asset.assignment.context.name %>
 <% end %>
 
-<%= t :body, "Your assignment, %{assignment}, has been graded.", :assignment => asset.assignment.title %>
+<% if send_student_names(asset, user) %>
+<%= t "%{assignment} has been graded for %{name}.", assignment: asset.assignment.title, name: asset.user.name %>
+<% else %>
+<%= t :body, "Your assignment %{assignment} has been graded.", :assignment => asset.assignment.title %>
+<% end %>
 
 <%= t :graded_date, "graded: %{date}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")) %>
 <% if user.try(:send_scores_in_emails?, asset.assignment.context.root_account) %>

app/messages/submission_graded.email.html.erb

@@ -12,7 +12,11 @@
   </a>
 <% end %>
 
-<p><%= t :body, "Your assignment, %{assignment}, has been graded.", :assignment => asset.assignment.title %></p>
+<% if send_student_names(asset, user) %>
+<p><%= t "%{assignment} has been graded for %{name}.", assignment: asset.assignment.title, name: asset.user.name %></p>
+<% else %>
+<p><%= t :body, "Your assignment %{assignment} has been graded.", :assignment => asset.assignment.title %></p>
+<% end %>
 
 <p><%= t :graded_date, "graded: %{date}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")) %></p>
 

app/messages/submission_graded.sms.erb

@@ -1,4 +1,8 @@
+<% if send_student_names(asset, user) %>
+<%= t "%{assignment}, %{context} has been graded for %{name}.", :assignment => asset.assignment.title, :context => asset.assignment.context.name, name: asset.user.name %>
+<% else %>
 <%= t :sms_body, "%{assignment}, %{context} has been graded.", :assignment => asset.assignment.title, :context => asset.assignment.context.name %>
+<% end %>
 
 <%= t :graded_date, "graded: %{date}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")) %>
 <% if asset.score && user.try(:send_scores_in_emails?, asset.assignment.context.root_account) %>

app/messages/submission_graded.summary.erb

@@ -6,6 +6,9 @@
   <%= t :subject, "Assignment Graded: %{assignment}, %{context}", :assignment => asset.assignment.title, :context => asset.assignment.context.name %>
 <% end %>
 
+<% if send_student_names(asset, user) %>
+  <%= t "For %{name}", name: asset.user.name %>
+<% end %>
 <%= t :graded_date, "graded: %{date}", :date => (datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set")) %>
 <% if asset.score && user.try(:send_scores_in_emails?, asset.assignment.context.root_account) %>
   <%= t :score, "score:  %{score} out of %{total}", :score => asset.score, :total => (asset.assignment.points_possible || t(:not_applicable, "N/A")) %>

app/messages/submission_graded.twitter.erb

@@ -1,7 +1,11 @@
 <% define_content :link do -%>
   <%= polymorphic_url([asset.assignment.context, asset.assignment]) %>
 <% end -%>
-<%= t :tweet, "Canvas Alert - Graded: %{assignment}, %{context}", :assignment => asset.assignment.title, :context => asset.assignment.context.name %>
+<% if send_student_names(asset, user) %>
+  <%= t "Canvas Alert - Graded: %{assignment}, %{context} for %{name}", :assignment => asset.assignment.title, :context => asset.assignment.context.name, name: user.asset.name %>
+<% else %>
+  <%= t :tweet, "Canvas Alert - Graded: %{assignment}, %{context}", :assignment => asset.assignment.title, :context => asset.assignment.context.name %>
+<% end %>
 
 <%= datetime_string(force_zone(asset.graded_at)) rescue t(:no_date_set, "No Date Set") %>
 

app/models/message.rb

@@ -26,6 +26,7 @@ class Message < ActiveRecord::Base
   include HtmlTextHelper
   include Workflow
   include Messages::PeerReviewsHelper
+  include Messages::SendStudentNamesHelper
 
   extend TextHelper
 

app/models/notification_policy.rb

@@ -59,10 +59,10 @@ class NotificationPolicy < ActiveRecord::Base
         bool_val = (value == 'true')
         # save the preference as a symbol (convert from string)
         case key.to_sym
-          when :send_scores_in_emails
+          when :send_scores_in_emails, :send_observed_names_in_notifications
             # Only set if a root account and the root account allows the setting.
             if params[:root_account].settings[:allow_sending_scores_in_emails] != false
-              user.preferences[:send_scores_in_emails] = bool_val
+              user.preferences[key.to_sym] = bool_val
             end
           when :no_submission_comments_inbox
             user.preferences[:no_submission_comments_inbox] = bool_val

app/models/user.rb

@@ -1460,6 +1460,10 @@ class User < ActiveRecord::Base
     preferences[:send_scores_in_emails] == true && root_account.settings[:allow_sending_scores_in_emails] != false
   end
 
+  def send_observed_names_in_notifications?
+    preferences[:send_observed_names_in_notifications] == true
+  end
+
   def close_announcement(announcement)
     preferences[:closed_notifications] ||= []
     # serialize ids relative to the user

app/views/jst/profiles/notification_preferences.handlebars

@@ -3,6 +3,17 @@
     <span><i class='{{icon}}' /> {{title}}</span>
   {{/each}}
 </div>
+{{#if showObservedNames.available}}
+  <div class="user-preference">
+    <label class="checkbox" for="show-name-observers">
+      {{checkbox showObservedNames.name
+        id="show-name-observers"
+        class="user-pref-check"
+        checked=showObservedNames.on}}
+      {{showObservedNames.label}}
+    </label>
+  </div>
+{{/if}}
 
 {{#each eventGroups}}
   <table cellspacing="0"

spec/messages/submission_grade_changed.erb_spec.rb

@@ -53,5 +53,16 @@ describe 'submission_grade_changed' do
       message = generate_message(:submission_grade_changed, :summary, asset, user: user)
       expect(message.body).not_to match(/score:/)
     end
+
+    it "should include the submission's submitter name if receiver is not the submitter and has the setting turned on" do
+      observer = user_model
+      message = generate_message(:submission_grade_changed, :summary, asset, user: observer)
+      expect(message.body).not_to match("For #{@submission.user.name}")
+
+      observer.preferences[:send_observed_names_in_notifications] = true
+      observer.save!
+      message = generate_message(:submission_grade_changed, :summary, asset, user: observer)
+      expect(message.body).to match("For #{@submission.user.name}")
+    end
   end
 end

spec/messages/submission_graded.erb_spec.rb

@@ -28,4 +28,15 @@ describe "submission_graded" do
   let(:notification_name) { :submission_graded }
 
   include_examples "a message"
+
+  it "should include the submission's submitter name if receiver is not the submitter and has the setting turned on" do
+    observer = user_model
+    message = generate_message(:submission_graded, :summary, asset, user: observer)
+    expect(message.body).not_to match("For #{@submission.user.name}")
+
+    observer.preferences[:send_observed_names_in_notifications] = true
+    observer.save!
+    message = generate_message(:submission_graded, :summary, asset, user: observer)
+    expect(message.body).to match("For #{@submission.user.name}")
+  end
 end