use root_account association when possible

when our context is an assignment we currently are loading a
rubric_association to identify the root_account

test plan
 - specs should pass

fixes VICE-1280
flag = none

Change-Id: I9d9493da1d80cb8208e999619700f657a8c00bca
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/263328
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Tested-by: Mauricio Ribeiro <mauricio.ribeiro@instructure.com>
Reviewed-by: Mauricio Ribeiro <mauricio.ribeiro@instructure.com>
QA-Review: Mauricio Ribeiro <mauricio.ribeiro@instructure.com>
Product-Review: Mauricio Ribeiro <mauricio.ribeiro@instructure.com>
This commit is contained in:
Rob Orton 2021-04-20 22:39:54 -06:00
parent d4eed94274
commit 0653ff2f53
2 changed files with 18 additions and 7 deletions

View File

@ -370,21 +370,29 @@ class Message < ActiveRecord::Base
end end
# infer a root account associated with the context that the user can log in to # infer a root account associated with the context that the user can log in to
def link_root_account def link_root_account(pre_loaded_account: nil)
context = pre_loaded_account
@root_account ||= begin @root_account ||= begin
context = self.context context ||= self.context
if context.is_a?(CommunicationChannel) && @data&.root_account_id if context.is_a?(CommunicationChannel) && @data&.root_account_id
root_account = Account.where(id: @data.root_account_id).first root_account = Account.where(id: @data.root_account_id).first
context = root_account if root_account context = root_account if root_account
end end
context = context.assignment if context.respond_to?(:assignment) && context.assignment # root_account is on lots of objects, use it when we can.
context = context.root_account if context.respond_to?(:root_account)
# some of these `context =` may not be relevant now that we have
# root_account on many classes, but root_account doesn't respond to them
# and so it's fast, and there are a lot of ways to generate a message.
context = context.assignment.root_account if context.respond_to?(:assignment) && context.assignment
context = context.rubric_association.context if context.respond_to?(:rubric_association) && context.rubric_association context = context.rubric_association.context if context.respond_to?(:rubric_association) && context.rubric_association
context = context.appointment_group.contexts.first if context.respond_to?(:appointment_group) && context.appointment_group context = context.appointment_group.contexts.first if context.respond_to?(:appointment_group) && context.appointment_group
context = context.master_template.course if context.respond_to?(:master_template) && context.master_template context = context.master_template.course if context.respond_to?(:master_template) && context.master_template
context = context.context if context.respond_to?(:context) context = context.context if context.respond_to?(:context)
context = context.account if context.respond_to?(:account) context = context.account if context.respond_to?(:account)
context = context.root_account if context.respond_to?(:root_account) context = context.root_account if context.respond_to?(:root_account)
# Going through SisPseudonym.for is important since the account could change
if context && context.respond_to?(:root_account) if context && context.respond_to?(:root_account)
p = SisPseudonym.for(user, context, type: :implicit, require_sis: false) p = SisPseudonym.for(user, context, type: :implicit, require_sis: false)
context = p.account if p context = p.account if p
@ -606,9 +614,12 @@ class Message < ActiveRecord::Base
# path_type - The path to send the message across, e.g, 'email'. # path_type - The path to send the message across, e.g, 'email'.
# #
# Returns nothing. # Returns nothing.
def parse!(path_type=nil) def parse!(path_type=nil, root_account: nil)
raise StandardError, "Cannot parse without a context" unless self.context raise StandardError, "Cannot parse without a context" unless self.context
# set @root_account using our pre_loaded_account, because link_root_account
# is called many times.
link_root_account(pre_loaded_account: root_account)
# Get the users timezone but maintain the original timezone in order to set it back at the end # Get the users timezone but maintain the original timezone in order to set it back at the end
original_time_zone = Time.zone.name || "UTC" original_time_zone = Time.zone.name || "UTC"
user_time_zone = self.user.try(:time_zone) || root_account_time_zone || original_time_zone user_time_zone = self.user.try(:time_zone) || root_account_time_zone || original_time_zone

View File

@ -153,7 +153,7 @@ class NotificationMessageCreator
def build_summary_for(user, policy) def build_summary_for(user, policy)
user.shard.activate do user.shard.activate do
message = user.messages.build(message_options_for(user)) message = user.messages.build(message_options_for(user))
message.parse!('summary') message.parse!('summary', root_account: @account)
delayed_message = policy.delayed_messages.build(:notification => @notification, delayed_message = policy.delayed_messages.build(:notification => @notification,
:frequency => policy.frequency, :frequency => policy.frequency,
# policy.communication_channel should # policy.communication_channel should
@ -179,7 +179,7 @@ class NotificationMessageCreator
return if @notification.summarizable? && too_many_messages_for?(user) && ['email', 'sms'].include?(channel.path_type) return if @notification.summarizable? && too_many_messages_for?(user) && ['email', 'sms'].include?(channel.path_type)
message_options = message_options_for(user) message_options = message_options_for(user)
message = user.messages.build(message_options.merge(communication_channel: channel, to: channel.path)) message = user.messages.build(message_options.merge(communication_channel: channel, to: channel.path))
message&.parse! message&.parse!(root_account: @account)
message.workflow_state = 'bounced' if channel.bouncing? message.workflow_state = 'bounced' if channel.bouncing?
message message
end end
@ -205,7 +205,7 @@ class NotificationMessageCreator
# if a user has never logged in, let's not spam the dashboard for no reason. # if a user has never logged in, let's not spam the dashboard for no reason.
return unless @notification.dashboard? && @notification.show_in_feed? && !user.pre_registered? return unless @notification.dashboard? && @notification.show_in_feed? && !user.pre_registered?
message = user.messages.build(message_options_for(user).merge(:to => 'dashboard')) message = user.messages.build(message_options_for(user).merge(:to => 'dashboard'))
message.parse! message.parse!(root_account: @account)
message message
end end