diff options
author | http://jneen.net/ <jneen@jneen.net> | 2017-08-18 13:40:09 -0700 |
---|---|---|
committer | http://jneen.net/ <jneen@jneen.net> | 2017-08-21 15:34:25 -0700 |
commit | 3676275a5a07492bb24a8b1925f7221c375a2f42 (patch) | |
tree | e0785e7a60b19564b44a1d6623c0140e38b5ffaa | |
parent | 7059af29800da478edbe04f4edf63fd7698909f5 (diff) | |
download | gitlab-ce-3676275a5a07492bb24a8b1925f7221c375a2f42.tar.gz |
don't rely on order of notification levels
factor out #suitable_notification_level? and check manually by
notification level. this makes the notification logic clear and actually
reflect what is in the documentation as to what should happen with each
setting.
-rw-r--r-- | app/models/notification_recipient.rb | 53 |
1 files changed, 26 insertions, 27 deletions
diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb index dc862565a71..183e098d819 100644 --- a/app/models/notification_recipient.rb +++ b/app/models/notification_recipient.rb @@ -27,46 +27,45 @@ class NotificationRecipient @notification_setting ||= find_notification_setting end - def raw_notification_level - notification_setting&.level&.to_sym - end - def notification_level - # custom is treated the same as watch if it's enabled - otherwise it's - # set to :custom, meaning to send exactly when our type is :participating - # or :mention. - @notification_level ||= - case raw_notification_level - when :custom - if @custom_action && notification_setting&.event_enabled?(@custom_action) - :watch - else - :custom - end - else - raw_notification_level - end + @notification_level ||= notification_setting&.level&.to_sym end def notifiable? return false unless has_access? return false if own_activity? - return true if @type == :subscription - - return false if notification_level.nil? || notification_level == :disabled - - return %i[participating mention].include?(@type) if notification_level == :custom + # even users with :disabled notifications receive manual subscriptions + return !unsubscribed? if @type == :subscription - return false if %i[watch participating].include?(notification_level) && excluded_watcher_action? - - return false unless NotificationSetting.levels[notification_level] <= NotificationSetting.levels[@type] + return false unless suitable_notification_level? + # check this last because it's expensive + # nobody should receive notifications if they've specifically unsubscribed return false if unsubscribed? true end + def suitable_notification_level? + case notification_level + when :disabled, nil + false + when :custom + custom_enabled? || %i[participating mention].include?(@type) + when :watch, :participating + !excluded_watcher_action? + when :mention + @type == :mention + else + false + end + end + + def custom_enabled? + @custom_action && notification_setting&.event_enabled?(@custom_action) + end + def unsubscribed? return false unless @target return false unless @target.respond_to?(:subscriptions) @@ -98,7 +97,7 @@ class NotificationRecipient def excluded_watcher_action? return false unless @custom_action - return false if raw_notification_level == :custom + return false if notification_level == :custom NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action) end |