Skip to content

Commit

Permalink
allow notifications to be sent for global announcements
Browse files Browse the repository at this point in the history
test plan:
* should be able to mark the checkbox when creating
 a global announcement for an account (other than site
 admin) to queue a job to send the announcement in
 a notification to all applicable users

closes #CORE-3384

Change-Id: I0540196164be283f3acc6c6b5232a945e1916fbe
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/212448
Tested-by: Jenkins
Product-Review: Lauren Williams <[email protected]>
Reviewed-by: Mysti Lilla <[email protected]>
Reviewed-by: Jeremy Stanley <[email protected]>
QA-Review: Jeremy Putnam <[email protected]>
  • Loading branch information
maneframe committed Oct 31, 2019
1 parent f1e792b commit 47feefb
Show file tree
Hide file tree
Showing 16 changed files with 470 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export default class NotificationGroupMappings {
Parent: [],
Groups: ['membership_update'],
Conferences: ['recording_ready'],
Alerts: ['other', 'content_link_error']
Alerts: ['other', 'content_link_error', 'account_notification']
}
}

Expand Down
11 changes: 8 additions & 3 deletions app/controllers/account_notifications_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,13 @@ def create
def update
account_notification = @account.announcements.find(params[:id])
if account_notification
account_notification.attributes = params.require(:account_notification).
permit(:subject, :icon, :message, :start_at, :end_at, :required_account_service, :months_in_display_cycle, :domain_specific)
notification_params = params.require(:account_notification).
permit(:subject, :icon, :message, :start_at, :end_at, :required_account_service, :months_in_display_cycle, :domain_specific, :send_message)
account_notification.attributes = notification_params

if value_to_boolean(notification_params[:send_message])
account_notification.messages_sent_at = nil # reset if we're explicitly re-sending messages
end

existing_roles = account_notification.account_notification_roles.map(&:role)
requested_roles = roles_to_add(params[:account_notification_roles])
Expand Down Expand Up @@ -326,6 +331,6 @@ def roles_to_add(role_params)

def account_notification_params
params.require(:account_notification).
permit(:subject, :icon, :message, :start_at, :end_at, :required_account_service, :months_in_display_cycle, :domain_specific)
permit(:subject, :icon, :message, :start_at, :end_at, :required_account_service, :months_in_display_cycle, :domain_specific, :send_message)
end
end
4 changes: 2 additions & 2 deletions app/helpers/account_notification_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ def accessible_message_icon_text(icon_type)

def roles_message(account)
if account.root_account?
t "(If none are selected, send to everyone in the entire account)"
t "(If none are selected, show to everyone in the entire account)"
else
t "(If none are selected, send to everyone in the entire sub-account)"
t "(If none are selected, show to everyone in the entire sub-account)"
end
end
end
11 changes: 11 additions & 0 deletions app/messages/account_notification.email.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<% define_content :link do %>
<%= dashboard_url %>
<% end %>

<% define_content :subject do %>
<%= before_label asset.subject %> <%= asset.account.name %>
<% end %>

<%= html_to_text(asset.message, :base_url => dashboard_url) %>

<%= content :link %>
21 changes: 21 additions & 0 deletions app/messages/account_notification.email.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<% define_content :link do %>
<%= dashboard_url %>
<% end %>

<% define_content :subject do %>
<%= before_label asset.subject %> <%= asset.account.name %>
<% end %>

<% define_content :footer_link do %>
<a href="<%= content(:link) %>">
<%= t :link, "View Announcement on Dashboard" %>
</a>
<% end %>

<%= html_to_simple_html(asset.message,
:base_url => dashboard_url,
:tags => ['tr', 'td', 'table', 'tbody', 'caption', 'span'],
:attributes => {
'table' => [ 'border', 'style', 'cellspacing', 'cellpadding' ],
'span' => ['style']
}) %>
5 changes: 5 additions & 0 deletions app/messages/notification_types.yml
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@
delay_for: 0
- name: Blueprint Sync Complete
delay_for: 0

- category: Content Link Error
notifications:
- name: Content Link Error

- category: Account Notification
notifications:
- name: Account Notification
95 changes: 94 additions & 1 deletion app/models/account_notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
class AccountNotification < ActiveRecord::Base
validates_presence_of :start_at, :end_at, :subject, :message, :account_id
validate :validate_dates
validate :send_message_not_set_for_site_admin
belongs_to :account, :touch => true
belongs_to :user
has_many :account_notification_roles, dependent: :destroy
Expand All @@ -26,6 +27,7 @@ class AccountNotification < ActiveRecord::Base
sanitize_field :message, CanvasSanitize::SANITIZE

after_save :create_alert
after_save :queue_message_broadcast

ACCOUNT_SERVICE_NOTIFICATION_FLAGS = %w[account_survey_notifications]
validates_inclusion_of :required_account_service, in: ACCOUNT_SERVICE_NOTIFICATION_FLAGS, allow_nil: true
Expand All @@ -43,7 +45,7 @@ def create_alert
self.send_later_enqueue_args(:create_alert, {
:run_at => self.start_at,
:on_conflict => :overwrite,
:singleton => "create_notification_alert:#{self.id}"
:singleton => "create_notification_alert:#{self.global_id}"
})
return
end
Expand Down Expand Up @@ -195,4 +197,95 @@ def self.display_for_user?(user_id, months_in_period, current_time = Time.now.ut
mod_value = (Random.new(periods_since_start_time).rand(months_in_period) + months_into_current_period) % months_in_period
user_id % months_in_period == mod_value
end

attr_accessor :message_recipients
has_a_broadcast_policy

set_broadcast_policy do |p|
p.dispatch :account_notification
p.to { self.message_recipients }
p.whenever { |record|
record.should_send_message? && record.message_recipients.present?
}
end

def send_message_not_set_for_site_admin
if self.send_message? && self.account.site_admin?
# i mean maybe we could try but there are almost certainly better ways to send mass emails than this
errors.add(:send_message, 'Cannot send messages for site admin accounts')
end
end

def should_send_message?
self.send_message? && !self.messages_sent_at &&
(self.start_at.nil? || (self.start_at < Time.now.utc)) &&
(self.end_at.nil? || (self.end_at > Time.now.utc))
end

def queue_message_broadcast
if self.send_message? && !self.messages_sent_at && !self.message_recipients
self.send_later_enqueue_args(:broadcast_messages, {
:run_at => self.start_at || Time.now.utc,
:on_conflict => :overwrite,
:singleton => "account_notification_broadcast_messages:#{self.global_id}",
:max_attempts => 1})
end
end

def self.users_per_message_batch
Setting.get("account_notification_message_batch_size", "1000").to_i
end

def broadcast_messages
return unless self.should_send_message? # sanity check before we start grabbing user ids

# don't try to send a message to an entire account in one job
self.applicable_user_ids.each_slice(self.class.users_per_message_batch) do |sliced_user_ids|
begin
self.message_recipients = sliced_user_ids.map{|id| "user_#{id}"}
self.save # trigger the broadcast policy
ensure
self.message_recipients = nil
end
end
self.update_attribute(:messages_sent_at, Time.now.utc)
end

def applicable_user_ids
roles = self.account_notification_roles.preload(:role).to_a.map(&:role)
Shackles.activate(:slave) do
self.class.applicable_user_ids_for_account_and_roles(self.account, roles)
end
end

def self.applicable_user_ids_for_account_and_roles(account, roles)
account.shard.activate do
all_account_ids = Account.sub_account_ids_recursive(account.id) + [account.id]
user_ids = Set.new
get_everybody = roles.empty?

course_roles = roles.select{|role| role.course_role?}.map(&:role_for_shard)
if get_everybody || course_roles.any?
Course.find_ids_in_ranges do |min_id, max_id|
course_ids = Course.active.where(:id => min_id..max_id, :account_id => all_account_ids).pluck(:id)
next unless course_ids.any?
course_ids.each_slice(50) do |sliced_course_ids|
scope = Enrollment.active_or_pending.where(:course_id => sliced_course_ids)
scope = scope.where(:role_id => course_roles) unless get_everybody
user_ids += scope.distinct.pluck(:user_id)
end
end
end

account_roles = roles.select{|role| role.account_role?}.map(&:role_for_shard)
if get_everybody || account_roles.any?
AccountUser.find_ids_in_ranges do |min_id, max_id|
scope = AccountUser.where(:id => min_id..max_id).active.where(:account_id => all_account_ids)
scope = scope.where(:role_id => account_roles) unless get_everybody
user_ids += scope.distinct.pluck(:user_id)
end
end
user_ids.to_a.sort
end
end
end
2 changes: 1 addition & 1 deletion app/models/delayed_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class DelayedMessage < ActiveRecord::Base
:attachment, :assignment_override, :group_membership, :calendar_event,
:wiki_page, :assessment_request, :account_user, :web_conference,
:account, :user, :appointment_group, :collaborator, :account_report,
:alert, :content_migration,
:alert, :content_migration, :account_notification,
{
context_communication_channel: 'CommunicationChannel',
quiz_submission: 'Quizzes::QuizSubmission',
Expand Down
10 changes: 10 additions & 0 deletions app/models/notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,8 @@ def default_frequency(user = nil)
FREQ_IMMEDIATELY
when 'Content Link Error'
FREQ_DAILY
when 'Account Notification'
FREQ_IMMEDIATELY
else
FREQ_DAILY
end
Expand Down Expand Up @@ -350,6 +352,7 @@ def names
t 'names.blueprint_sync_complete', 'Blueprint Sync Complete'
t 'names.blueprint_content_added', 'Blueprint Content Added'
t 'names.content_link_error', 'Content Link Error'
t 'names.account_notification', 'Account Notification'
end

# TODO: i18n ... show these anywhere we show the category today
Expand Down Expand Up @@ -379,6 +382,7 @@ def category_names
t 'categories.recording_ready', 'Recording Ready'
t 'categories.blueprint', 'Blueprint'
t 'categories.content_link_error', 'Content Link Error'
t 'categories.account_notification', 'Account Notification'
end

# Translatable display text to use when representing the category to the user.
Expand Down Expand Up @@ -440,6 +444,8 @@ def category_display_name
t(:blueprint_display, 'Blueprint Sync')
when 'Content Link Error'
t(:content_link_error_display, 'Content Link Error')
when 'Account Notification'
t(:account_notification_display, 'Global Announcements')
else
t(:missing_display_display, "For %{category} notifications", :category => category)
end
Expand Down Expand Up @@ -555,6 +561,10 @@ def category_description
Location and content of a failed link that a student has interacted with
CONTLINK
when 'Account Notification'
mt(:account_notification_description, <<-EOS)
Institution-wide announcements (also displayed on Dashboard pages)
EOS
else
t(:missing_description_description, "For %{category} notifications", :category => category)
end
Expand Down
8 changes: 5 additions & 3 deletions app/models/notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,26 @@

class Notifier
def send_notification(record, dispatch, messages, to_list, data=nil)
recipient_keys = (to_list || []).compact.map{|o| o.is_a?(String) ? o : o.asset_string}
messages = DelayedNotification.send_later_if_production_enqueue_args(
:process,
{:priority => 30, :max_attempts => 1},
record,
messages,
(to_list || []).compact.map(&:asset_string),
recipient_keys,
data
)

messages ||= DelayedNotification.new(
:asset => record,
:notification => messages,
:recipient_keys => (to_list || []).compact.map(&:asset_string),
:recipient_keys => recipient_keys,
:data => data
)

if Rails.env.test?
record.messages_sent[dispatch] = messages.is_a?(DelayedNotification) ? messages.process : messages
record.messages_sent[dispatch] ||= []
record.messages_sent[dispatch] += messages.is_a?(DelayedNotification) ? messages.process : messages
end

messages
Expand Down
16 changes: 15 additions & 1 deletion app/views/accounts/_edit_account_notification.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@

<div class="ic-Form-control">
<div class="ic-Label" id="aria-announcements-send-to-group">
<%= t "Send to" %>
<%= t "Show to" %>
<%= roles_message(@account) %>
</div>
<div class="grid-row">
Expand Down Expand Up @@ -179,6 +179,20 @@
</div>
</div>

<% unless @account.site_admin? %>
<div class="ic-Form-control ic-Form-control--checkbox">
<%= f.check_box :send_message, :checked => announcement.send_message? && !announcement.messages_sent_at,
:id => "account_notification_send_message_#{announcement.id}" %>
<% if announcement.messages_sent_at? %>
<%= f.label :send_message, t("Re-send notification directly to users when announcement starts"),
:class => "ic-Label", :for => "account_notification_send_message_#{announcement.id}" %>
<% else %>
<%= f.label :send_message, t("Send notification directly to users when announcement starts"),
:class => "ic-Label", :for => "account_notification_send_message_#{announcement.id}" %>
<% end %>
</div>
<% end %>

<div class="ic-Form-actions">
<%= button_tag(t("Cancel"), :class => "element_toggler btn button-secondary edit_cancel_focus", 'aria-controls' => "edit_notification_form_#{announcement.id}", 'data-cancel-focus-id' => announcement.id) %>
<%= button_tag(t("Save Changes"), :class => 'btn btn-primary') %>
Expand Down
9 changes: 8 additions & 1 deletion app/views/accounts/settings.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,7 @@ acknowledge that you have read and agreed to the
<% end %>
<div class="ic-Form-control">
<div class="ic-Label" id="aria-announcements-send-to-group">
<%= t "Send to" %>
<%= t "Show to" %>
<%= roles_message(@account) %>
</div>
<div class="grid-row">
Expand Down Expand Up @@ -1155,6 +1155,13 @@ acknowledge that you have read and agreed to the

</div>

<% unless @account.site_admin? %>
<div class="ic-Form-control ic-Form-control--checkbox">
<%= f.check_box :send_message %>
<%= f.label :send_message, t("Send notification directly to users when announcement starts"), :class => "ic-Label" %>
</div>
<% end %>

<div class="ic-Form-actions">
<button type="Button" class="element_toggler Button Button--secondary add_notification_cancel_focus" aria-controls="add_notification_form">
<%= t('#buttons.cancel', %{Cancel}) %>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#
# 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 AddSendMessageToAccountNotifications < ActiveRecord::Migration[5.2]
tag :predeploy
disable_ddl_transaction!

def up
if Shard.current.default? && !::Rails.env.test?
Canvas::MessageHelper.create_notification({
name: 'Account Notification',
delay_for: 0,
category: 'Account Notification'
})
end

add_column :account_notifications, :send_message, :boolean
change_column_default(:account_notifications, :send_message, false)
DataFixup::BackfillNulls.run(AccountNotification, :send_message, default_value: false)
change_column_null(:account_notifications, :send_message, false)

add_column :account_notifications, :messages_sent_at, :datetime
end

def down
remove_column :account_notifications, :send_message
remove_column :account_notifications, :messages_sent_at

if Shard.current.default?
Notification.where(name: 'Account Notification').delete_all
end
end
end
Loading

0 comments on commit 47feefb

Please sign in to comment.