Commit b8f38437 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Update NotificationService to use NotificationSettings instead of membership

Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent 7ea1bcab
# == Notifiable concern
#
# Contains notification functionality
#
module Notifiable
extend ActiveSupport::Concern
included do
validates :notification_level, inclusion: { in: Notification.project_notification_levels }, presence: true
end
def notification
@notification ||= Notification.new(self)
end
end
...@@ -27,6 +27,7 @@ class Group < Namespace ...@@ -27,6 +27,7 @@ class Group < Namespace
has_many :users, through: :group_members has_many :users, through: :group_members
has_many :project_group_links, dependent: :destroy has_many :project_group_links, dependent: :destroy
has_many :shared_projects, through: :project_group_links, source: :project has_many :shared_projects, through: :project_group_links, source: :project
has_many :notification_settings, dependent: :destroy, as: :source
validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
validate :visibility_level_allowed_by_projects validate :visibility_level_allowed_by_projects
......
...@@ -19,7 +19,6 @@ ...@@ -19,7 +19,6 @@
class Member < ActiveRecord::Base class Member < ActiveRecord::Base
include Sortable include Sortable
include Notifiable
include Gitlab::Access include Gitlab::Access
attr_accessor :raw_invite_token attr_accessor :raw_invite_token
...@@ -170,6 +169,10 @@ class Member < ActiveRecord::Base ...@@ -170,6 +169,10 @@ class Member < ActiveRecord::Base
end end
end end
def notification
@notification ||= user.notification_settings.find_by(source: source)
end
private private
def send_invite def send_invite
......
...@@ -30,30 +30,12 @@ class Notification ...@@ -30,30 +30,12 @@ class Notification
end end
end end
delegate :disabled?, :participating?, :watch?, :global?, :mention?, to: :target
def initialize(target) def initialize(target)
@target = target @target = target
end end
def disabled?
target.notification_level == N_DISABLED
end
def participating?
target.notification_level == N_PARTICIPATING
end
def watch?
target.notification_level == N_WATCH
end
def global?
target.notification_level == N_GLOBAL
end
def mention?
target.notification_level == N_MENTION
end
def level def level
target.notification_level target.notification_level
end end
......
...@@ -154,6 +154,7 @@ class Project < ActiveRecord::Base ...@@ -154,6 +154,7 @@ class Project < ActiveRecord::Base
has_many :project_group_links, dependent: :destroy has_many :project_group_links, dependent: :destroy
has_many :invited_groups, through: :project_group_links, source: :group has_many :invited_groups, through: :project_group_links, source: :group
has_many :todos, dependent: :destroy has_many :todos, dependent: :destroy
has_many :notification_settings, dependent: :destroy, as: :source
has_one :import_data, dependent: :destroy, class_name: "ProjectImportData" has_one :import_data, dependent: :destroy, class_name: "ProjectImportData"
......
...@@ -193,6 +193,9 @@ class User < ActiveRecord::Base ...@@ -193,6 +193,9 @@ class User < ActiveRecord::Base
# Notification level # Notification level
# Note: When adding an option, it MUST go on the end of the array. # Note: When adding an option, it MUST go on the end of the array.
#
# TODO: Add '_prefix: :notification' to enum when update to Rails 5. https://github.com/rails/rails/pull/19813
# Because user.notification_disabled? is much better than user.disabled?
enum notification_level: [:disabled, :participating, :watch, :global, :mention] enum notification_level: [:disabled, :participating, :watch, :global, :mention]
alias_attribute :private_token, :authentication_token alias_attribute :private_token, :authentication_token
......
...@@ -253,8 +253,8 @@ class NotificationService ...@@ -253,8 +253,8 @@ class NotificationService
def project_watchers(project) def project_watchers(project)
project_members = project_member_notification(project) project_members = project_member_notification(project)
users_with_project_level_global = project_member_notification(project, Notification::N_GLOBAL) users_with_project_level_global = project_member_notification(project, :global)
users_with_group_level_global = group_member_notification(project, Notification::N_GLOBAL) users_with_group_level_global = group_member_notification(project, :global)
users = users_with_global_level_watch([users_with_project_level_global, users_with_group_level_global].flatten.uniq) users = users_with_global_level_watch([users_with_project_level_global, users_with_group_level_global].flatten.uniq)
users_with_project_setting = select_project_member_setting(project, users_with_project_level_global, users) users_with_project_setting = select_project_member_setting(project, users_with_project_level_global, users)
...@@ -264,18 +264,16 @@ class NotificationService ...@@ -264,18 +264,16 @@ class NotificationService
end end
def project_member_notification(project, notification_level=nil) def project_member_notification(project, notification_level=nil)
project_members = project.project_members
if notification_level if notification_level
project_members.where(notification_level: notification_level).pluck(:user_id) project.notification_settings.where(level: NotificationSetting.levels[notification_level]).pluck(:user_id)
else else
project_members.pluck(:user_id) project.notification_settings.pluck(:user_id)
end end
end end
def group_member_notification(project, notification_level) def group_member_notification(project, notification_level)
if project.group if project.group
project.group.group_members.where(notification_level: notification_level).pluck(:user_id) project.group.notification_settings.where(level: NotificationSetting.levels[notification_level]).pluck(:user_id)
else else
[] []
end end
...@@ -284,13 +282,13 @@ class NotificationService ...@@ -284,13 +282,13 @@ class NotificationService
def users_with_global_level_watch(ids) def users_with_global_level_watch(ids)
User.where( User.where(
id: ids, id: ids,
notification_level: Notification::N_WATCH notification_level: NotificationSetting.levels[:watch]
).pluck(:id) ).pluck(:id)
end end
# Build a list of users based on project notifcation settings # Build a list of users based on project notifcation settings
def select_project_member_setting(project, global_setting, users_global_level_watch) def select_project_member_setting(project, global_setting, users_global_level_watch)
users = project_member_notification(project, Notification::N_WATCH) users = project_member_notification(project, :watch)
# If project setting is global, add to watch list if global setting is watch # If project setting is global, add to watch list if global setting is watch
global_setting.each do |user_id| global_setting.each do |user_id|
...@@ -304,7 +302,7 @@ class NotificationService ...@@ -304,7 +302,7 @@ class NotificationService
# Build a list of users based on group notification settings # Build a list of users based on group notification settings
def select_group_member_setting(project, project_members, global_setting, users_global_level_watch) def select_group_member_setting(project, project_members, global_setting, users_global_level_watch)
uids = group_member_notification(project, Notification::N_WATCH) uids = group_member_notification(project, :watch)
# Group setting is watch, add to users list if user is not project member # Group setting is watch, add to users list if user is not project member
users = [] users = []
...@@ -351,20 +349,20 @@ class NotificationService ...@@ -351,20 +349,20 @@ class NotificationService
users.reject do |user| users.reject do |user|
next user.notification.send(method_name) unless project next user.notification.send(method_name) unless project
member = project.project_members.find_by(user_id: user.id) setting = user.notification_settings.find_by(source: project)
if !member && project.group if !setting && project.group
member = project.group.group_members.find_by(user_id: user.id) setting = user.notification_settings.find_by(source: group)
end end
# reject users who globally set mention notification and has no membership # reject users who globally set mention notification and has no setting per project/group
next user.notification.send(method_name) unless member next user.notification.send(method_name) unless setting
# reject users who set mention notification in project # reject users who set mention notification in project
next true if member.notification.send(method_name) next true if setting.send(method_name)
# reject users who have N_MENTION in project and disabled in global settings # reject users who have mention level in project and disabled in global settings
member.notification.global? && user.notification.send(method_name) setting.global? && user.notification.send(method_name)
end end
end end
......
...@@ -89,11 +89,11 @@ describe NotificationService, services: true do ...@@ -89,11 +89,11 @@ describe NotificationService, services: true do
note.project.group.add_user(@u_watcher, GroupMember::MASTER) note.project.group.add_user(@u_watcher, GroupMember::MASTER)
note.project.save note.project.save
user_project = note.project.project_members.find_by_user_id(@u_watcher.id) user_project = note.project.project_members.find_by_user_id(@u_watcher.id)
user_project.notification_level = Notification::N_PARTICIPATING user_project.notification.level = :participating
user_project.save user_project.save
group_member = note.project.group.group_members.find_by_user_id(@u_watcher.id) group_member = note.project.group.group_members.find_by_user_id(@u_watcher.id)
group_member.notification_level = Notification::N_GLOBAL group_member.notification.level = :global
group_member.save group_member.notification.save
ActionMailer::Base.deliveries.clear ActionMailer::Base.deliveries.clear
end end
...@@ -215,7 +215,7 @@ describe NotificationService, services: true do ...@@ -215,7 +215,7 @@ describe NotificationService, services: true do
end end
it do it do
@u_committer.update_attributes(notification_level: Notification::N_MENTION) @u_committer.update_attributes(notification_level: :mention)
notification.new_note(note) notification.new_note(note)
should_not_email(@u_committer) should_not_email(@u_committer)
end end
...@@ -246,7 +246,7 @@ describe NotificationService, services: true do ...@@ -246,7 +246,7 @@ describe NotificationService, services: true do
end end
it do it do
issue.assignee.update_attributes(notification_level: Notification::N_MENTION) issue.assignee.update_attributes(notification_level: :mention)
notification.new_issue(issue, @u_disabled) notification.new_issue(issue, @u_disabled)
should_not_email(issue.assignee) should_not_email(issue.assignee)
...@@ -596,13 +596,13 @@ describe NotificationService, services: true do ...@@ -596,13 +596,13 @@ describe NotificationService, services: true do
end end
def build_team(project) def build_team(project)
@u_watcher = create(:user, notification_level: Notification::N_WATCH) @u_watcher = create(:user, notification_level: :watch)
@u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) @u_participating = create(:user, notification_level: :participating)
@u_participant_mentioned = create(:user, username: 'participant', notification_level: Notification::N_PARTICIPATING) @u_participant_mentioned = create(:user, username: 'participant', notification_level: :participating)
@u_disabled = create(:user, notification_level: Notification::N_DISABLED) @u_disabled = create(:user, notification_level: :disabled)
@u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_MENTION) @u_mentioned = create(:user, username: 'mention', notification_level: :mention)
@u_committer = create(:user, username: 'committer') @u_committer = create(:user, username: 'committer')
@u_not_mentioned = create(:user, username: 'regular', notification_level: Notification::N_PARTICIPATING) @u_not_mentioned = create(:user, username: 'regular', notification_level: :participating)
@u_outsider_mentioned = create(:user, username: 'outsider') @u_outsider_mentioned = create(:user, username: 'outsider')
project.team << [@u_watcher, :master] project.team << [@u_watcher, :master]
...@@ -617,8 +617,8 @@ describe NotificationService, services: true do ...@@ -617,8 +617,8 @@ describe NotificationService, services: true do
def add_users_with_subscription(project, issuable) def add_users_with_subscription(project, issuable)
@subscriber = create :user @subscriber = create :user
@unsubscriber = create :user @unsubscriber = create :user
@subscribed_participant = create(:user, username: 'subscribed_participant', notification_level: Notification::N_PARTICIPATING) @subscribed_participant = create(:user, username: 'subscribed_participant', notification_level: :participating)
@watcher_and_subscriber = create(:user, notification_level: Notification::N_WATCH) @watcher_and_subscriber = create(:user, notification_level: :watch)
project.team << [@subscribed_participant, :master] project.team << [@subscribed_participant, :master]
project.team << [@subscriber, :master] project.team << [@subscriber, :master]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment