Guarantee order of notification settings

3 files changed, 12 insertions, 14 deletions

diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 11738afecb9..f78be3882d6 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -82,16 +82,8 @@ class Notify < BaseMailer
     group_notification_email = nil
 
     if notification_group
-      # Get notification group's and ancestors' notification settings
-      group_ids = notification_group.self_and_ancestors_ids
-      notification_settings = notification_group.notification_settings.where(user: @current_user) # rubocop: disable CodeReuse/ActiveRecord
-
-      # Exploit notification_group.self_and_ancestors_ids being ordered from
-      # most nested to least nested to iterate through group ancestors
-      group_ids.each do |group_id|
-        group_notification_email = notification_settings.find { |ns| ns.source_id == group_id }&.notification_email
-        break if group_notification_email.present?
-      end
+      notification_settings = notification_group.notification_settings(hierarchy_order: :asc).where(user: @current_user) # rubocop: disable CodeReuse/ActiveRecord
+      group_notification_email = notification_settings.find { |n| n.notification_email.present? }&.notification_email
     end
 
     # Return group-specific email address if present, otherwise return global
diff --git a/app/models/group.rb b/app/models/group.rb
index 53331a19776..25d246f64e7 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -126,10 +126,16 @@ class Group < Namespace
   # Overrides notification_settings has_many association
   # This allows to apply notification settings from parent groups
   # to child groups and projects.
-  def notification_settings
+  def notification_settings(hierarchy_order: nil)
     source_type = self.class.base_class.name
+    settings = NotificationSetting.where(source_type: source_type, source_id: self_and_ancestors_ids)
 
-    NotificationSetting.where(source_type: source_type, source_id: self_and_ancestors_ids)
+    return settings unless hierarchy_order && self_and_ancestors_ids.length > 1
+
+    settings
+      .joins("LEFT JOIN (#{self_and_ancestors(hierarchy_order: hierarchy_order).to_sql}) AS ordered_groups ON notification_settings.source_id = ordered_groups.id")
+      .select('notification_settings.*, ordered_groups.depth AS depth')
+      .order("ordered_groups.depth #{hierarchy_order}")
   end
 
   def to_reference(_from = nil, full: nil)
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index f7c31890198..3c270c7396a 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -206,12 +206,12 @@ class Namespace < ApplicationRecord
       .ancestors(upto: top, hierarchy_order: hierarchy_order)
   end
 
-  def self_and_ancestors
+  def self_and_ancestors(hierarchy_order: nil)
     return self.class.where(id: id) unless parent_id
 
     Gitlab::ObjectHierarchy
       .new(self.class.where(id: id))
-      .base_and_ancestors
+      .base_and_ancestors(hierarchy_order: hierarchy_order)
   end
 
   # Returns all the descendants of the current namespace.