Commit a3d5343e authored by Douwe Maan's avatar Douwe Maan Committed by Robert Speicher

Merge branch 'issue_12758' into 'master'

Implement custom notification level options

![Screen_Shot_2016-06-17_at_15.31.43](/uploads/3fc47d2f461b3e8b67bb8acaa304cf99/Screen_Shot_2016-06-17_at_15.31.43.png)

![Screenshot_from_2016-06-15_10-52-27](/uploads/88dbdd21d97e80ee772fe08fa0c9b393/Screenshot_from_2016-06-15_10-52-27.png)

part of #12758 

See merge request !4389
parent 0d21f8c3
...@@ -71,6 +71,7 @@ v 8.9.0 (unreleased) ...@@ -71,6 +71,7 @@ v 8.9.0 (unreleased)
- Pipelines can be canceled only when there are running builds - Pipelines can be canceled only when there are running builds
- Allow authentication using personal access tokens - Allow authentication using personal access tokens
- Use downcased path to container repository as this is expected path by Docker - Use downcased path to container repository as this is expected path by Docker
- Custom notification settings
- Projects pending deletion will render a 404 page - Projects pending deletion will render a 404 page
- Measure queue duration between gitlab-workhorse and Rails - Measure queue duration between gitlab-workhorse and Rails
- Added Gfm autocomplete for labels - Added Gfm autocomplete for labels
......
...@@ -78,6 +78,7 @@ class Dispatcher ...@@ -78,6 +78,7 @@ class Dispatcher
when 'projects:show' when 'projects:show'
shortcut_handler = new ShortcutsNavigation() shortcut_handler = new ShortcutsNavigation()
new NotificationsForm()
new TreeView() if $('#tree-slider').length new TreeView() if $('#tree-slider').length
when 'groups:activity' when 'groups:activity'
new Activities() new Activities()
...@@ -129,6 +130,8 @@ class Dispatcher ...@@ -129,6 +130,8 @@ class Dispatcher
shortcut_handler = new ShortcutsDashboardNavigation() shortcut_handler = new ShortcutsDashboardNavigation()
when 'profiles' when 'profiles'
new Profile() new Profile()
new NotificationsForm()
new NotificationsDropdown()
when 'projects' when 'projects'
new Project() new Project()
new ProjectAvatar() new ProjectAvatar()
...@@ -136,8 +139,12 @@ class Dispatcher ...@@ -136,8 +139,12 @@ class Dispatcher
when 'edit' when 'edit'
shortcut_handler = new ShortcutsNavigation() shortcut_handler = new ShortcutsNavigation()
new ProjectNew() new ProjectNew()
when 'new', 'show' when 'new'
new ProjectNew() new ProjectNew()
when 'show'
new ProjectNew()
new ProjectShow()
new NotificationsDropdown()
when 'wikis' when 'wikis'
new Wikis() new Wikis()
shortcut_handler = new ShortcutsNavigation() shortcut_handler = new ShortcutsNavigation()
......
class @NotificationsDropdown
$ ->
$(document)
.off 'click', '.update-notification'
.on 'click', '.update-notification', (e) ->
e.preventDefault()
return if $(this).is('.is-active') and $(this).data('notification-level') is 'custom'
notificationLevel = $(@).data 'notification-level'
label = $(@).data 'notification-title'
form = $(this).parents('.notification-form:first')
form.find('.js-notification-loading').toggleClass 'fa-bell fa-spin fa-spinner'
form.find('#notification_setting_level').val(notificationLevel)
form.submit()
$(document)
.off 'ajax:success', '.notification-form'
.on 'ajax:success', '.notification-form', (e, data) ->
if data.saved
new Flash('Notification settings saved', 'notice')
$(e.currentTarget).closest('.notification-dropdown').replaceWith(data.html)
else
new Flash('Failed to save new settings', 'alert')
class @NotificationsForm
constructor: ->
@removeEventListeners()
@initEventListeners()
removeEventListeners: ->
$(document).off 'change', '.js-custom-notification-event'
initEventListeners: ->
$(document).on 'change', '.js-custom-notification-event', @toggleCheckbox
toggleCheckbox: (e) =>
$checkbox = $(e.currentTarget)
$parent = $checkbox.closest('.checkbox')
@saveEvent($checkbox, $parent)
showCheckboxLoadingSpinner: ($parent) ->
$parent
.addClass 'is-loading'
.find '.custom-notification-event-loading'
.removeClass 'fa-check'
.addClass 'fa-spin fa-spinner'
.removeClass 'is-done'
saveEvent: ($checkbox, $parent) ->
form = $parent.parents('form:first')
$.ajax(
url: form.attr('action')
method: form.attr('method')
dataType: 'json'
data: form.serialize()
beforeSend: =>
@showCheckboxLoadingSpinner($parent)
).done (data) ->
$checkbox.enable()
if data.saved
$parent
.find '.custom-notification-event-loading'
.toggleClass 'fa-spin fa-spinner fa-check is-done'
setTimeout(->
$parent
.removeClass 'is-loading'
.find '.custom-notification-event-loading'
.toggleClass 'fa-spin fa-spinner fa-check is-done'
, 2000)
...@@ -8,6 +8,10 @@ class @Profile ...@@ -8,6 +8,10 @@ class @Profile
$('.js-preferences-form').on 'change.preference', 'input[type=radio]', -> $('.js-preferences-form').on 'change.preference', 'input[type=radio]', ->
$(this).parents('form').submit() $(this).parents('form').submit()
# Automatically submit email form when it changes
$('#user_notification_email').on 'change', ->
$(this).parents('form').submit()
$('.update-username').on 'ajax:before', -> $('.update-username').on 'ajax:before', ->
$('.loading-username').show() $('.loading-username').show()
$(this).find('.update-success').hide() $(this).find('.update-success').hide()
......
...@@ -34,22 +34,6 @@ class @Project ...@@ -34,22 +34,6 @@ class @Project
$(@).parents('.no-password-message').remove() $(@).parents('.no-password-message').remove()
e.preventDefault() e.preventDefault()
$('.update-notification').on 'click', (e) ->
e.preventDefault()
notification_level = $(@).data 'notification-level'
label = $(@).data 'notification-title'
$('#notification_setting_level').val(notification_level)
$('#notification-form').submit()
$('#notifications-button').empty().append("<i class='fa fa-bell'></i>" + label + "<i class='fa fa-angle-down'></i>")
$(@).parents('ul').find('li.active').removeClass 'active'
$(@).parent().addClass 'active'
$('#notification-form').on 'ajax:success', (e, data) ->
if data.saved
new Flash("Notification settings saved", "notice")
else
new Flash("Failed to save new settings", "alert")
@projectSelectDropdown() @projectSelectDropdown()
......
...@@ -133,11 +133,6 @@ ...@@ -133,11 +133,6 @@
} }
} }
.btn-group:not(:first-child):not(:last-child) > .btn {
border-top-right-radius: 3px;
border-bottom-right-radius: 3px;
}
form { form {
margin-left: 10px; margin-left: 10px;
} }
...@@ -607,3 +602,20 @@ pre.light-well { ...@@ -607,3 +602,20 @@ pre.light-well {
} }
} }
} }
.custom-notifications-form {
.is-loading {
.custom-notification-event-loading {
display: inline-block;
}
}
}
.custom-notification-event-loading {
display: none;
margin-left: 5px;
&.is-done {
color: $gl-text-green;
}
}
class Groups::NotificationSettingsController < Groups::ApplicationController
before_action :authenticate_user!
def update
notification_setting = current_user.notification_settings_for(group)
saved = notification_setting.update_attributes(notification_setting_params)
render json: { saved: saved }
end
private
def notification_setting_params
params.require(:notification_setting).permit(:level)
end
end
class NotificationSettingsController < ApplicationController
before_action :authenticate_user!
def create
project = Project.find(params[:project][:id])
return render_404 unless can?(current_user, :read_project, project)
@notification_setting = current_user.notification_settings_for(project)
@saved = @notification_setting.update_attributes(notification_setting_params)
render_response
end
def update
@notification_setting = current_user.notification_settings.find(params[:id])
@saved = @notification_setting.update_attributes(notification_setting_params)
render_response
end
private
def render_response
render json: {
html: view_to_html_string("shared/notifications/_button", notification_setting: @notification_setting),
saved: @saved
}
end
def notification_setting_params
allowed_fields = NotificationSetting::EMAIL_EVENTS.dup
allowed_fields << :level
params.require(:notification_setting).permit(allowed_fields)
end
end
class Profiles::NotificationsController < Profiles::ApplicationController class Profiles::NotificationsController < Profiles::ApplicationController
def show def show
@user = current_user @user = current_user
@group_notifications = current_user.notification_settings.for_groups @group_notifications = current_user.notification_settings.for_groups.order(:id)
@project_notifications = current_user.notification_settings.for_projects @project_notifications = current_user.notification_settings.for_projects.order(:id)
@global_notification_setting = current_user.global_notification_setting @global_notification_setting = current_user.global_notification_setting
end end
def update def update
if current_user.update_attributes(user_params) && update_notification_settings if current_user.update_attributes(user_params)
flash[:notice] = "Notification settings saved" flash[:notice] = "Notification settings saved"
else else
flash[:alert] = "Failed to save new settings" flash[:alert] = "Failed to save new settings"
...@@ -19,16 +19,4 @@ class Profiles::NotificationsController < Profiles::ApplicationController ...@@ -19,16 +19,4 @@ class Profiles::NotificationsController < Profiles::ApplicationController
def user_params def user_params
params.require(:user).permit(:notification_email) params.require(:user).permit(:notification_email)
end end
def global_notification_setting_params
params.require(:global_notification_setting).permit(:level)
end
private
def update_notification_settings
return true unless global_notification_setting_params
current_user.global_notification_setting.update_attributes(global_notification_setting_params)
end
end end
class Projects::NotificationSettingsController < Projects::ApplicationController
before_action :authenticate_user!
def update
notification_setting = current_user.notification_settings_for(project)
saved = notification_setting.update_attributes(notification_setting_params)
render json: { saved: saved }
end
private
def notification_setting_params
params.require(:notification_setting).permit(:level)
end
end
...@@ -34,7 +34,7 @@ module NotificationsHelper ...@@ -34,7 +34,7 @@ module NotificationsHelper
def notification_description(level) def notification_description(level)
case level.to_sym case level.to_sym
when :participating when :participating
'You will only receive notifications from related resources' 'You will only receive notifications for threads you have participated in'
when :mention when :mention
'You will receive notifications only for comments in which you were @mentioned' 'You will receive notifications only for comments in which you were @mentioned'
when :watch when :watch
...@@ -43,6 +43,8 @@ module NotificationsHelper ...@@ -43,6 +43,8 @@ module NotificationsHelper
'You will not get any notifications via email' 'You will not get any notifications via email'
when :global when :global
'Use your global notification setting' 'Use your global notification setting'
when :custom
'You will only receive notifications for the events you choose'
end end
end end
...@@ -62,22 +64,14 @@ module NotificationsHelper ...@@ -62,22 +64,14 @@ module NotificationsHelper
end end
end end
def notification_level_radio_buttons # Identifier to trigger individually dropdowns and custom settings modals in the same view
html = "" def notifications_menu_identifier(type, notification_setting)
"#{type}-#{notification_setting.user_id}-#{notification_setting.source_id}-#{notification_setting.source_type}"
NotificationSetting.levels.each_key do |level| end
level = level.to_sym
next if level == :global
html << content_tag(:div, class: "radio") do
content_tag(:label, { value: level }) do
radio_button_tag(:"global_notification_setting[level]", level, @global_notification_setting.level.to_sym == level) +
content_tag(:div, level.to_s.capitalize, class: "level-title") +
content_tag(:p, notification_description(level))
end
end
end
html.html_safe # Create hidden field to send notification setting source to controller
def hidden_setting_source_input(notification_setting)
return unless notification_setting.source_type
hidden_field_tag "#{notification_setting.source_type.downcase}[id]", notification_setting.source_id
end end
end end
class NotificationSetting < ActiveRecord::Base class NotificationSetting < ActiveRecord::Base
enum level: { global: 3, watch: 2, mention: 4, participating: 1, disabled: 0 } enum level: { global: 3, watch: 2, mention: 4, participating: 1, disabled: 0, custom: 5 }
default_value_for :level, NotificationSetting.levels[:global] default_value_for :level, NotificationSetting.levels[:global]
...@@ -15,6 +15,24 @@ class NotificationSetting < ActiveRecord::Base ...@@ -15,6 +15,24 @@ class NotificationSetting < ActiveRecord::Base
scope :for_groups, -> { where(source_type: 'Namespace') } scope :for_groups, -> { where(source_type: 'Namespace') }
scope :for_projects, -> { where(source_type: 'Project') } scope :for_projects, -> { where(source_type: 'Project') }
EMAIL_EVENTS = [
:new_note,
:new_issue,
:reopen_issue,
:close_issue,
:reassign_issue,
:new_merge_request,
:reopen_merge_request,
:close_merge_request,
:reassign_merge_request,
:merge_merge_request
]
store :events, accessors: EMAIL_EVENTS, coder: JSON
before_create :set_events
before_save :events_to_boolean
def self.find_or_create_for(source) def self.find_or_create_for(source)
setting = find_or_initialize_by(source: source) setting = find_or_initialize_by(source: source)
...@@ -24,4 +42,21 @@ class NotificationSetting < ActiveRecord::Base ...@@ -24,4 +42,21 @@ class NotificationSetting < ActiveRecord::Base
setting setting
end end
# Set all event attributes to false when level is not custom or being initialized for UX reasons
def set_events
return if custom?
EMAIL_EVENTS.each do |event|
events[event] = false
end
end
# Validates store accessors values as boolean
# It is a text field so it does not cast correct boolean values in JSON
def events_to_boolean
EMAIL_EVENTS.each do |event|
events[event] = ActiveRecord::ConnectionAdapters::Column::TRUE_VALUES.include?(events[event])
end
end
end end
...@@ -29,9 +29,10 @@ class NotificationService ...@@ -29,9 +29,10 @@ class NotificationService
# * issue assignee if their notification level is not Disabled # * issue assignee if their notification level is not Disabled
# * project team members with notification level higher then Participating # * project team members with notification level higher then Participating
# * watchers of the issue's labels # * watchers of the issue's labels
# * users with custom level checked with "new issue"
# #
def new_issue(issue, current_user) def new_issue(issue, current_user)
new_resource_email(issue, issue.project, 'new_issue_email') new_resource_email(issue, issue.project, :new_issue_email)
end end
# When we close an issue we should send an email to: # When we close an issue we should send an email to:
...@@ -39,18 +40,20 @@ class NotificationService ...@@ -39,18 +40,20 @@ class NotificationService
# * issue author if their notification level is not Disabled # * issue author if their notification level is not Disabled
# * issue assignee if their notification level is not Disabled # * issue assignee if their notification level is not Disabled
# * project team members with notification level higher then Participating # * project team members with notification level higher then Participating
# * users with custom level checked with "close issue"
# #
def close_issue(issue, current_user) def close_issue(issue, current_user)
close_resource_email(issue, issue.project, current_user, 'closed_issue_email') close_resource_email(issue, issue.project, current_user, :closed_issue_email)
end end
# When we reassign an issue we should send an email to: # When we reassign an issue we should send an email to:
# #
# * issue old assignee if their notification level is not Disabled # * issue old assignee if their notification level is not Disabled
# * issue new assignee if their notification level is not Disabled # * issue new assignee if their notification level is not Disabled
# * users with custom level checked with "reassign issue"
# #
def reassigned_issue(issue, current_user) def reassigned_issue(issue, current_user)
reassign_resource_email(issue, issue.project, current_user, 'reassigned_issue_email') reassign_resource_email(issue, issue.project, current_user, :reassigned_issue_email)
end end
# When we add labels to an issue we should send an email to: # When we add labels to an issue we should send an email to:
...@@ -58,7 +61,7 @@ class NotificationService ...@@ -58,7 +61,7 @@ class NotificationService
# * watchers of the issue's labels # * watchers of the issue's labels
# #
def relabeled_issue(issue, added_labels, current_user) def relabeled_issue(issue, added_labels, current_user)
relabeled_resource_email(issue, added_labels, current_user, 'relabeled_issue_email') relabeled_resource_email(issue, added_labels, current_user, :relabeled_issue_email)
end end
# When create a merge request we should send an email to: # When create a merge request we should send an email to:
...@@ -66,18 +69,20 @@ class NotificationService ...@@ -66,18 +69,20 @@ class NotificationService
# * mr assignee if their notification level is not Disabled # * mr assignee if their notification level is not Disabled
# * project team members with notification level higher then Participating # * project team members with notification level higher then Participating
# * watchers of the mr's labels # * watchers of the mr's labels
# * users with custom level checked with "new merge request"
# #
def new_merge_request(merge_request, current_user) def new_merge_request(merge_request, current_user)
new_resource_email(merge_request, merge_request.target_project, 'new_merge_request_email') new_resource_email(merge_request, merge_request.target_project, :new_merge_request_email)
end end
# When we reassign a merge_request we should send an email to: # When we reassign a merge_request we should send an email to:
# #
# * merge_request old assignee if their notification level is not Disabled # * merge_request old assignee if their notification level is not Disabled
# * merge_request assignee if their notification level is not Disabled # * merge_request assignee if their notification level is not Disabled
# * users with custom level checked with "reassign merge request"
# #
def reassigned_merge_request(merge_request, current_user) def reassigned_merge_request(merge_request, current_user)
reassign_resource_email(merge_request, merge_request.target_project, current_user, 'reassigned_merge_request_email') reassign_resource_email(merge_request, merge_request.target_project, current_user, :reassigned_merge_request_email)
end end
# When we add labels to a merge request we should send an email to: # When we add labels to a merge request we should send an email to:
...@@ -85,15 +90,15 @@ class NotificationService ...@@ -85,15 +90,15 @@ class NotificationService
# * watchers of the mr's labels # * watchers of the mr's labels
# #
def relabeled_merge_request(merge_request, added_labels, current_user) def relabeled_merge_request(merge_request, added_labels, current_user)
relabeled_resource_email(merge_request, added_labels, current_user, 'relabeled_merge_request_email') relabeled_resource_email(merge_request, added_labels, current_user, :relabeled_merge_request_email)
end end
def close_mr(merge_request, current_user) def close_mr(merge_request, current_user)
close_resource_email(merge_request, merge_request.target_project, current_user, 'closed_merge_request_email') close_resource_email(merge_request, merge_request.target_project, current_user, :closed_merge_request_email)
end end
def reopen_issue(issue, current_user) def reopen_issue(issue, current_user)
reopen_resource_email(issue, issue.project, current_user, 'issue_status_changed_email', 'reopened') reopen_resource_email(issue, issue.project, current_user, :issue_status_changed_email, 'reopened')
end end
def merge_mr(merge_request, current_user) def merge_mr(merge_request, current_user)
...@@ -101,7 +106,7 @@ class NotificationService ...@@ -101,7 +106,7 @@ class NotificationService
merge_request, merge_request,
merge_request.target_project, merge_request.target_project,
current_user, current_user,
'merged_merge_request_email' :merged_merge_request_email
) )
end end
...@@ -110,7 +115,7 @@ class NotificationService ...@@ -110,7 +115,7 @@ class NotificationService
merge_request, merge_request,
merge_request.target_project, merge_request.target_project,
current_user, current_user,
'merge_request_status_email', :merge_request_status_email,
'reopened' 'reopened'
) )
end end
...@@ -153,6 +158,9 @@ class NotificationService ...@@ -153,6 +158,9 @@ class NotificationService
# Merge project watchers # Merge project watchers
recipients = add_project_watchers(recipients, note.project) recipients = add_project_watchers(recipients, note.project)
# Merge project with custom notification
recipients = add_custom_notifications(recipients, note.project, :new_note)
# Reject users with Mention notification level, except those mentioned in _this_ note. # Reject users with Mention notification level, except those mentioned in _this_ note.
recipients = reject_mention_users(recipients - mentioned_users, note.project) recipients = reject_mention_users(recipients - mentioned_users, note.project)
recipients = recipients + mentioned_users recipients = recipients + mentioned_users
...@@ -276,12 +284,31 @@ class NotificationService ...@@ -276,12 +284,31 @@ class NotificationService
protected protected
# Get project/group users with CUSTOM notification level
def add_custom_notifications(recipients, project, action)
user_ids = []
# Users with a notification setting on group or project
user_ids += notification_settings_for(project, :custom, action)
user_ids += notification_settings_for(project.group, :custom, action)
# Users with global level custom
users_with_project_level_global = notification_settings_for(project, :global)
users_with_group_level_global = notification_settings_for(project.group, :global)
global_users_ids = users_with_project_level_global.concat(users_with_group_level_global)
user_ids += users_with_global_level_custom(global_users_ids, action)
recipients.concat(User.find(user_ids))
end
# Get project users with WATCH notification level # Get project users with WATCH notification level
def project_watchers(project) def project_watchers(project)
project_members = project_member_notification(project) project_members = notification_settings_for(project)
users_with_project_level_global = notification_settings_for(project, :global)
users_with_group_level_global = notification_settings_for(project.group, :global)
users_with_project_level_global = project_member_notification(project, :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)
...@@ -290,33 +317,39 @@ class NotificationService ...@@ -290,33 +317,39 @@ class NotificationService
User.where(id: users_with_project_setting.concat(users_with_group_setting).uniq).to_a User.where(id: users_with_project_setting.concat(users_with_group_setting).uniq).to_a
end end
def project_member_notification(project, notification_level=nil) def notification_settings_for(resource, notification_level = nil, action = nil)
return [] unless resource
if notification_level if notification_level
project.notification_settings.where(level: NotificationSetting.levels[notification_level]).pluck(:user_id) settings = resource.notification_settings.where(level: NotificationSetting.levels[notification_level])
settings = settings.select { |setting| setting.events[action] } if action.present?
settings.map(&:user_id)
else else
project.notification_settings.pluck(:user_id) resource.notification_settings.pluck(:user_id)
end end
end end
def group_member_notification(project, notification_level) def users_with_global_level_watch(ids)
if project.group settings_with_global_level_of(:watch, ids).pluck(:user_id)
project.group.notification_settings.where(level: NotificationSetting.levels[notification_level]).pluck(:user_id)
else
[]
end
end end
def users_with_global_level_watch(ids) def users_with_global_level_custom(ids, action)
settings = settings_with_global_level_of(:custom, ids)
settings = settings.select { |setting| setting.events[action] }
settings.map(&:user_id)
end
def settings_with_global_level_of(level, ids)
NotificationSetting.where( NotificationSetting.where(
user_id: ids, user_id: ids,
source_type: nil, source_type: nil,
level: NotificationSetting.levels[:watch] level: NotificationSetting.levels[level]
).pluck(:user_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, :watch) users = notification_settings_for(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|
...@@ -330,7 +363,7 @@ class NotificationService ...@@ -330,7 +363,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, :watch) uids = notification_settings_for(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,7 +384,7 @@ class NotificationService ...@@ -351,7 +384,7 @@ class NotificationService
end end
def add_project_watchers(recipients, project) def add_project_watchers(recipients, project)
recipients.concat(project_watchers(project)).compact.uniq recipients.concat(project_watchers(project)).compact
end end
# Remove users with disabled notifications from array # Remove users with disabled notifications from array
...@@ -436,7 +469,7 @@ class NotificationService ...@@ -436,7 +469,7 @@ class NotificationService
end end
def new_resource_email(target, project, method) def new_resource_email(target, project, method)
recipients = build_recipients(target, project, target.author, action: :new) recipients = build_recipients(target, project, target.author, action: "new")
recipients.each do |recipient| recipients.each do |recipient|
mailer.send(method, recipient.id, target.id).deliver_later mailer.send(method, recipient.id, target.id).deliver_later
...@@ -444,7 +477,8 @@ class NotificationService ...@@ -444,7 +477,8 @@ class NotificationService
end end
def close_resource_email(target, project, current_user, method) def close_resource_email(target, project, current_user, method)
recipients = build_recipients(target, project, current_user) action = method == :merged_merge_request_email ? "merge" : "close"
recipients = build_recipients(target, project, current_user, action: action)
recipients.each do |recipient| recipients.each do |recipient|
mailer.send(method, recipient.id, target.id, current_user.id).deliver_later mailer.send(method, recipient.id, target.id, current_user.id).deliver_later
...@@ -455,7 +489,7 @@ class NotificationService ...@@ -455,7 +489,7 @@ class NotificationService
previous_assignee_id = previous_record(target, 'assignee_id') previous_assignee_id = previous_record(target, 'assignee_id')
previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
recipients = build_recipients(target, project, current_user, action: :reassign, previous_assignee: previous_assignee) recipients = build_recipients(target, project, current_user, action: "reassign", previous_assignee: previous_assignee)
recipients.each do |recipient| recipients.each do |recipient|
mailer.send( mailer.send(
...@@ -478,7 +512,7 @@ class NotificationService ...@@ -478,7 +512,7 @@ class NotificationService
end end
def reopen_resource_email(target, project, current_user, method, status) def reopen_resource_email(target, project, current_user, method, status)
recipients = build_recipients(target, project, current_user) recipients = build_recipients(target, project, current_user, action: "reopen")
recipients.each do |recipient| recipients.each do |recipient|
mailer.send(method, recipient.id, target.id, status, current_user.id).deliver_later mailer.send(method, recipient.id, target.id, status, current_user.id).deliver_later
...@@ -486,14 +520,20 @@ class NotificationService ...@@ -486,14 +520,20 @@ class NotificationService
end end
def build_recipients(target, project, current_user, action: nil, previous_assignee: nil) def build_recipients(target, project, current_user, action: nil, previous_assignee: nil)
custom_action = build_custom_key(action, target)
recipients = target.participants(current_user) recipients = target.participants(current_user)
recipients = add_project_watchers(recipients, project) recipients = add_project_watchers(recipients, project)
recipients = add_custom_notifications(recipients, project, custom_action)
recipients = reject_mention_users(recipients, project) recipients = reject_mention_users(recipients, project)
recipients = recipients.uniq
# Re-assign is considered as a mention of the new assignee so we add the # Re-assign is considered as a mention of the new assignee so we add the
# new assignee to the list of recipients after we rejected users with # new assignee to the list of recipients after we rejected users with
# the "on mention" notification level # the "on mention" notification level
if action == :reassign if [:reassign_merge_request, :reassign_issue].include?(custom_action)
recipients << previous_assignee if previous_assignee recipients << previous_assignee if previous_assignee
recipients << target.assignee recipients << target.assignee
end end
...@@ -501,7 +541,7 @@ class NotificationService ...@@ -501,7 +541,7 @@ class NotificationService
recipients = reject_muted_users(recipients, project) recipients = reject_muted_users(recipients, project)
recipients = add_subscribed_users(recipients, target) recipients = add_subscribed_users(recipients, target)
if action == :new if [:new_issue, :new_merge_request].include?(custom_action)
recipients = add_labels_subscribers(recipients, target) recipients = add_labels_subscribers(recipients, target)
end end
...@@ -531,4 +571,10 @@ class NotificationService ...@@ -531,4 +571,10 @@ class NotificationService
end end
end end
end end
# Build event key to search on custom notification level
# Check NotificationSetting::EMAIL_EVENTS
def build_custom_key(action, object)
"#{action}_#{object.class.name.underscore}".to_sym
end
end end
...@@ -9,5 +9,4 @@ ...@@ -9,5 +9,4 @@
= link_to group.name, group_path(group) = link_to group.name, group_path(group)
.pull-right .pull-right
= form_for [group, setting], remote: true, html: { class: 'update-notifications' } do |f| = render 'shared/notifications/button', notification_setting: setting
= f.select :level, NotificationSetting.levels.keys, {}, class: 'form-control trigger-submit'
...@@ -9,5 +9,4 @@ ...@@ -9,5 +9,4 @@
= link_to_project(project) = link_to_project(project)
.pull-right .pull-right
= form_for [project.namespace.becomes(Namespace), project, setting], remote: true, html: { class: 'update-notifications' } do |f| = render 'shared/notifications/button', notification_setting: setting
= f.select :level, NotificationSetting.levels.keys, {}, class: 'form-control trigger-submit'
...@@ -24,12 +24,15 @@ ...@@ -24,12 +24,15 @@
.form-group .form-group
= f.label :notification_email, class: "label-light" = f.label :notification_email, class: "label-light"
= f.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2" = f.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2"
.form-group
= f.label :notification_level, class: 'label-light'
= notification_level_radio_buttons
.prepend-top-default = label_tag :global_notification_level, "Global notification level", class: "label-light"
= f.submit 'Update settings', class: "btn btn-create" %br
.clearfix
.form-group.pull-left
= render 'shared/notifications/button', notification_setting: @global_notification_setting, left_align: true
.clearfix
%hr %hr
%h5 %h5
Groups (#{@group_notifications.count}) Groups (#{@group_notifications.count})
......
...@@ -29,13 +29,13 @@ ...@@ -29,13 +29,13 @@
.project-clone-holder .project-clone-holder
= render "shared/clone_panel" = render "shared/clone_panel"
.project-repo-buttons.project-right-buttons .project-repo-buttons.btn-group.project-right-buttons
- if current_user - if current_user
= render 'shared/members/access_request_buttons', source: @project = render 'shared/members/access_request_buttons', source: @project
.btn-group
= render "projects/buttons/download" = render "projects/buttons/download"
= render 'projects/buttons/dropdown' = render 'projects/buttons/dropdown'
= render 'projects/buttons/notifications' = render 'shared/notifications/button', notification_setting: @notification_setting
:javascript :javascript
new Star(); new Star();
- if @notification_setting
= form_for @notification_setting, url: namespace_project_notification_setting_path(@project.namespace.becomes(Namespace), @project), method: :patch, remote: true, html: { class: 'inline', id: 'notification-form' } do |f|
= f.hidden_field :level
.dropdown.hidden-sm
%button.btn.btn-default.notifications-btn#notifications-button{ data: { toggle: "dropdown" }, aria: { haspopup: "true", expanded: "false" } }
= icon('bell')
= notification_title(@notification_setting.level)
= icon('caret-down')
%ul.dropdown-menu.dropdown-menu-no-wrap.dropdown-menu-align-right.dropdown-menu-selectable.dropdown-menu-large{ role: "menu" }
- NotificationSetting.levels.each do |level|
= notification_list_item(level.first, @notification_setting)
- left_align = local_assigns[:left_align]
- if notification_setting
.dropdown.notification-dropdown.pull-right
= form_for notification_setting, remote: true, html: { class: "inline notification-form" } do |f|
= hidden_setting_source_input(notification_setting)
= f.hidden_field :level, class: "notification_setting_level"
.js-notification-toggle-btns
%div{ class: ("btn-group" if notification_setting.custom?) }
- if notification_setting.custom?
%button.dropdown-new.btn.btn-default.notifications-btn#notifications-button{ type: "button", data: { toggle: "modal", target: "#" + notifications_menu_identifier("modal", notification_setting) } }
= icon("bell", class: "js-notification-loading")
= notification_title(notification_setting.level)
%button.btn.dropdown-toggle{ data: { toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting) } }
%span.caret
.sr-only Toggle dropdown
- else
%button.dropdown-new.btn.btn-default.notifications-btn#notifications-button{ type: "button", data: { toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting) } }
= icon("bell", class: "js-notification-loading")
= notification_title(notification_setting.level)
= icon("caret-down")
= render "shared/notifications/notification_dropdown", notification_setting: notification_setting, left_align: left_align
= content_for :scripts_body do
= render "shared/notifications/custom_notifications", notification_setting: notification_setting
.modal.fade{ tabindex: "-1", role: "dialog", id: notifications_menu_identifier("modal", notification_setting), aria: { labelledby: "custom-notifications-title" } }
.modal-dialog
.modal-content
.modal-header
%button.close{ type: "button", data: { dismiss: "modal" }, aria: { label: "close" } }
%span{ aria: { hidden: "true" } } ×
%h4#custom-notifications-title.modal-title
Custom notification events
.modal-body
.container-fluid
= form_for notification_setting, html: { class: "custom-notifications-form" } do |f|
= hidden_setting_source_input(notification_setting)
.row
.col-lg-4
%h4.prepend-top-0
Notification events
%p
Custom notification levels are the same as participating levels. With custom notification levels you will also receive notifications for select events. To find out more, check out
= succeed "." do
%a{ href: "http://docs.gitlab.com/ce/workflow/notifications.html", target: "_blank"} notification emails
.col-lg-8
- NotificationSetting::EMAIL_EVENTS.each_with_index do |event, index|
- field_id = "#{notifications_menu_identifier("modal", notification_setting)}_notification_setting[#{event}]"
.form-group
.checkbox{ class: ("prepend-top-0" if index == 0) }
%label{ for: field_id }
= check_box("notification_setting", event, id: field_id, class: "js-custom-notification-event", checked: notification_setting.events[event])
%strong
= event.to_s.humanize
= icon("spinner spin", class: "custom-notification-event-loading")
- left_align = local_assigns[:left_align]
%ul.dropdown-menu.dropdown-menu-no-wrap.dropdown-menu-selectable.dropdown-menu-large{ role: "menu", class: [notifications_menu_identifier("dropdown", notification_setting), ("dropdown-menu-align-right" unless left_align)] }
- NotificationSetting.levels.each_key do |level|
- next if level == "custom"
- next if level == "global" && notification_setting.source.nil?
= notification_list_item(level, notification_setting)
%li.divider
%li
%a.update-notification{ href: "#", role: "button", class: ("is-active" if notification_setting.custom?), data: { toggle: "modal", target: "#" + notifications_menu_identifier("modal", notification_setting), notification_level: "custom", notification_title: "Custom" } }
%strong.dropdown-menu-inner-title Custom
%span.dropdown-menu-inner-content= notification_description("custom")
...@@ -123,9 +123,17 @@ Rails.application.routes.draw do ...@@ -123,9 +123,17 @@ Rails.application.routes.draw do
end end
end end
#
# Spam reports # Spam reports
#
resources :abuse_reports, only: [:new, :create] resources :abuse_reports, only: [:new, :create]
#
# Notification settings
#
resources :notification_settings, only: [:create, :update]
# #
# Import # Import
# #
...@@ -432,7 +440,6 @@ Rails.application.routes.draw do ...@@ -432,7 +440,6 @@ Rails.application.routes.draw do
resource :avatar, only: [:destroy] resource :avatar, only: [:destroy]
resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create] resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
resource :notification_setting, only: [:update]
end end
end end
...@@ -668,7 +675,6 @@ Rails.application.routes.draw do ...@@ -668,7 +675,6 @@ Rails.application.routes.draw do
resources :forks, only: [:index, :new, :create] resources :forks, only: [:index, :new, :create]
resource :import, only: [:new, :create, :show] resource :import, only: [:new, :create, :show]
resource :notification_setting, only: [:update]
resources :refs, only: [] do resources :refs, only: [] do
collection do collection do
......
class AddEventsToNotificationSettings < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
def change
add_column :notification_settings, :events, :text
end
end
...@@ -4,7 +4,7 @@ GitLab has a notification system in place to notify a user of events that are im ...@@ -4,7 +4,7 @@ GitLab has a notification system in place to notify a user of events that are im
## Notification settings ## Notification settings
Under user profile page you can find the notification settings. You can find notification settings under the user profile.
![notification settings](notifications/settings.png) ![notification settings](notifications/settings.png)
...@@ -20,6 +20,7 @@ Each of these settings have levels of notification: ...@@ -20,6 +20,7 @@ Each of these settings have levels of notification:
* Participating - receive notifications from related resources * Participating - receive notifications from related resources
* Watch - receive notifications from projects or groups user is a member of * Watch - receive notifications from projects or groups user is a member of
* Global - notifications as set at the global settings * Global - notifications as set at the global settings
* Custom - user will receive notifications when mentioned, is participant and custom selected events.
#### Global Settings #### Global Settings
...@@ -55,7 +56,7 @@ Below is the table of events users can be notified of: ...@@ -55,7 +56,7 @@ Below is the table of events users can be notified of:
| User added to project | User | Sent when user is added to project | | User added to project | User | Sent when user is added to project |
| Project access level changed | User | Sent when user project access level is changed | | Project access level changed | User | Sent when user project access level is changed |
| User added to group | User | Sent when user is added to group | | User added to group | User | Sent when user is added to group |
| Group access level changed | User | Sent when user group access level is changed | | Group access level changed | User | Sent when user group access level is changed |
| Project moved | Project members [1] | [1] not disabled | | Project moved | Project members [1] | [1] not disabled |
### Issue / Merge Request events ### Issue / Merge Request events
...@@ -71,6 +72,7 @@ In all of the below cases, the notification will be sent to: ...@@ -71,6 +72,7 @@ In all of the below cases, the notification will be sent to:
- Watchers: users with notification level "Watch" - Watchers: users with notification level "Watch"
- Subscribers: anyone who manually subscribed to the issue/merge request - Subscribers: anyone who manually subscribed to the issue/merge request
- Custom: Users with notification level "custom" who turned on notifications for any of the events present in the table below
| Event | Sent to | | Event | Sent to |
|------------------------|---------| |------------------------|---------|
......
...@@ -11,7 +11,7 @@ class Spinach::Features::ProfileNotifications < Spinach::FeatureSteps ...@@ -11,7 +11,7 @@ class Spinach::Features::ProfileNotifications < Spinach::FeatureSteps
end end
step 'I select Mention setting from dropdown' do step 'I select Mention setting from dropdown' do
select 'mention', from: 'notification_setting_level' first(:link, "On mention").trigger('click')
end end
step 'I should see Notification saved message' do step 'I should see Notification saved message' do
......
...@@ -126,7 +126,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps ...@@ -126,7 +126,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps
end end
step 'I click notifications drop down button' do step 'I click notifications drop down button' do
find('#notifications-button').click first('.notifications-btn').click
end end
step 'I choose Mention setting' do step 'I choose Mention setting' do
......
require 'spec_helper'
describe Groups::NotificationSettingsController do
let(:group) { create(:group) }
let(:user) { create(:user) }
describe '#update' do
context 'when not authorized' do
it 'redirects to sign in page' do
put :update,
group_id: group.to_param,
notification_setting: { level: :participating }
expect(response).to redirect_to(new_user_session_path)
end
end
context 'when authorized' do
before do
sign_in(user)
end
it 'returns success' do
put :update,
group_id: group.to_param,
notification_setting: { level: :participating }
expect(response.status).to eq 200
end
end
end
end
require 'spec_helper' require 'spec_helper'
describe Projects::NotificationSettingsController do describe NotificationSettingsController do
let(:project) { create(:empty_project) } let(:project) { create(:empty_project) }
let(:user) { create(:user) } let(:user) { create(:user) }
...@@ -8,13 +8,12 @@ describe Projects::NotificationSettingsController do ...@@ -8,13 +8,12 @@ describe Projects::NotificationSettingsController do
project.team << [user, :developer] project.team << [user, :developer]
end end
describe '#update' do describe '#create' do
context 'when not authorized' do context 'when not authorized' do
it 'redirects to sign in page' do it 'redirects to sign in page' do
put :update, post :create,
namespace_id: project.namespace.to_param, project: { id: project.id },
project_id: project.to_param, notification_setting: { level: :participating }
notification_setting: { level: :participating }
expect(response).to redirect_to(new_user_session_path) expect(response).to redirect_to(new_user_session_path)
end end
...@@ -26,23 +25,97 @@ describe Projects::NotificationSettingsController do ...@@ -26,23 +25,97 @@ describe Projects::NotificationSettingsController do
end end
it 'returns success' do it 'returns success' do
put :update, post :create,
namespace_id: project.namespace.to_param, project: { id: project.id },
project_id: project.to_param, notification_setting: { level: :participating }
notification_setting: { level: :participating }
expect(response.status).to eq 200 expect(response.status).to eq 200
end end
context 'and setting custom notification setting' do
let(:custom_events) do
events = {}
NotificationSetting::EMAIL_EVENTS.each do |event|
events[event] = "true"
end
end
it 'returns success' do
post :create,
project: { id: project.id },
notification_setting: { level: :participating, events: custom_events }
expect(response.status).to eq 200
end
end
end end
context 'not authorized' do context 'not authorized' do
let(:private_project) { create(:project, :private) } let(:private_project) { create(:project, :private) }
before { sign_in(user) } before { sign_in(user) }
it 'returns 404' do
post :create,
project: { id: private_project.id },
notification_setting: { level: :participating }
expect(response.status).to eq(404)
end
end
end
describe '#update' do
let(:notification_setting) { user.global_notification_setting }
context 'when not authorized' do
it 'redirects to sign in page' do
put :update,
id: notification_setting,
notification_setting: { level: :participating }
expect(response).to redirect_to(new_user_session_path)
end
end
context 'when authorized' do
before{ sign_in(user) }
it 'returns success' do
put :update,
id: notification_setting,
notification_setting: { level: :participating }
expect(response.status).to eq 200
end
context 'and setting custom notification setting' do
let(:custom_events) do
events = {}
NotificationSetting::EMAIL_EVENTS.each do |event|
events[event] = "true"
end
end
it 'returns success' do
put :update,
id: notification_setting,
notification_setting: { level: :participating, events: custom_events }
expect(response.status).to eq 200
end
end
end
context 'not authorized' do
let(:other_user) { create(:user) }
before { sign_in(other_user) }
it 'returns 404' do it 'returns 404' do
put :update, put :update,
namespace_id: private_project.namespace.to_param, id: notification_setting,
project_id: private_project.to_param,
notification_setting: { level: :participating } notification_setting: { level: :participating }
expect(response.status).to eq(404) expect(response.status).to eq(404)
......
...@@ -12,5 +12,30 @@ RSpec.describe NotificationSetting, type: :model do ...@@ -12,5 +12,30 @@ RSpec.describe NotificationSetting, type: :model do
it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:user) }
it { is_expected.to validate_presence_of(:level) } it { is_expected.to validate_presence_of(:level) }
it { is_expected.to validate_uniqueness_of(:user_id).scoped_to([:source_id, :source_type]).with_message(/already exists in source/) } it { is_expected.to validate_uniqueness_of(:user_id).scoped_to([:source_id, :source_type]).with_message(/already exists in source/) }
context "events" do
let(:user) { create(:user) }
let(:notification_setting) { NotificationSetting.new(source_id: 1, source_type: 'Project', user_id: user.id) }
before do
notification_setting.level = "custom"
notification_setting.new_note = "true"
notification_setting.new_issue = 1
notification_setting.close_issue = "1"
notification_setting.merge_merge_request = "t"
notification_setting.close_merge_request = "nil"
notification_setting.reopen_merge_request = "false"
notification_setting.save
end
it "parses boolean before saving" do
expect(notification_setting.new_note).to eq(true)
expect(notification_setting.new_issue).to eq(true)
expect(notification_setting.close_issue).to eq(true)
expect(notification_setting.merge_merge_request).to eq(true)
expect(notification_setting.close_merge_request).to eq(false)
expect(notification_setting.reopen_merge_request).to eq(false)
end
end
end end
end end
...@@ -428,8 +428,9 @@ describe API::API, api: true do ...@@ -428,8 +428,9 @@ describe API::API, api: true do
describe 'permissions' do describe 'permissions' do
context 'all projects' do context 'all projects' do
it 'Contains permission information' do before { project.team << [user, :master] }
project.team << [user, :master]
it 'contains permission information' do
get api("/projects", user) get api("/projects", user)
expect(response.status).to eq(200) expect(response.status).to eq(200)
...@@ -440,7 +441,7 @@ describe API::API, api: true do ...@@ -440,7 +441,7 @@ describe API::API, api: true do
end end
context 'personal project' do context 'personal project' do
it 'Sets project access and returns 200' do it 'sets project access and returns 200' do
project.team << [user, :master] project.team << [user, :master]
get api("/projects/#{project.id}", user) get api("/projects/#{project.id}", user)
...@@ -452,9 +453,11 @@ describe API::API, api: true do ...@@ -452,9 +453,11 @@ describe API::API, api: true do
end end
context 'group project' do context 'group project' do
let(:project2) { create(:project, group: create(:group)) }
before { project2.group.add_owner(user) }
it 'should set the owner and return 200' do it 'should set the owner and return 200' do
project2 = create(:project, group: create(:group))
project2.group.add_owner(user)
get api("/projects/#{project2.id}", user) get api("/projects/#{project2.id}", user)
expect(response.status).to eq(200) expect(response.status).to eq(200)
......
...@@ -46,6 +46,8 @@ describe NotificationService, services: true do ...@@ -46,6 +46,8 @@ describe NotificationService, services: true do
project.team << [issue.assignee, :master] project.team << [issue.assignee, :master]
project.team << [note.author, :master] project.team << [note.author, :master]
create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy') create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy')
update_custom_notification(:new_note, @u_guest_custom, project)
update_custom_notification(:new_note, @u_custom_global)
end end
describe :new_note do describe :new_note do
...@@ -53,7 +55,7 @@ describe NotificationService, services: true do ...@@ -53,7 +55,7 @@ describe NotificationService, services: true do
add_users_with_subscription(note.project, issue) add_users_with_subscription(note.project, issue)
# Ensure create SentNotification by noteable = issue 6 times, not noteable = note # Ensure create SentNotification by noteable = issue 6 times, not noteable = note
expect(SentNotification).to receive(:record).with(issue, any_args).exactly(7).times expect(SentNotification).to receive(:record).with(issue, any_args).exactly(8).times
ActionMailer::Base.deliveries.clear ActionMailer::Base.deliveries.clear
...@@ -62,10 +64,12 @@ describe NotificationService, services: true do ...@@ -62,10 +64,12 @@ describe NotificationService, services: true do
should_email(@u_watcher) should_email(@u_watcher)
should_email(note.noteable.author) should_email(note.noteable.author)
should_email(note.noteable.assignee) should_email(note.noteable.assignee)
should_email(@u_custom_global)
should_email(@u_mentioned) should_email(@u_mentioned)
should_email(@subscriber) should_email(@subscriber)
should_email(@watcher_and_subscriber) should_email(@watcher_and_subscriber)
should_email(@subscribed_participant) should_email(@subscribed_participant)
should_not_email(@u_guest_custom)
should_not_email(@u_guest_watcher) should_not_email(@u_guest_watcher)
should_not_email(note.author) should_not_email(note.author)
should_not_email(@u_participating) should_not_email(@u_participating)
...@@ -103,10 +107,12 @@ describe NotificationService, services: true do ...@@ -103,10 +107,12 @@ describe NotificationService, services: true do
before do before do
note.project.namespace_id = group.id note.project.namespace_id = group.id
note.project.group.add_user(@u_watcher, GroupMember::MASTER) note.project.group.add_user(@u_watcher, GroupMember::MASTER)
note.project.group.add_user(@u_custom_global, GroupMember::MASTER)
note.project.save note.project.save
@u_watcher.notification_settings_for(note.project).participating! @u_watcher.notification_settings_for(note.project).participating!
@u_watcher.notification_settings_for(note.project.group).global! @u_watcher.notification_settings_for(note.project.group).global!
update_custom_notification(:new_note, @u_custom_global)
ActionMailer::Base.deliveries.clear ActionMailer::Base.deliveries.clear
end end
...@@ -116,6 +122,8 @@ describe NotificationService, services: true do ...@@ -116,6 +122,8 @@ describe NotificationService, services: true do
should_email(note.noteable.author) should_email(note.noteable.author)
should_email(note.noteable.assignee) should_email(note.noteable.assignee)
should_email(@u_mentioned) should_email(@u_mentioned)
should_email(@u_custom_global)
should_not_email(@u_guest_custom)
should_not_email(@u_guest_watcher) should_not_email(@u_guest_watcher)
should_not_email(@u_watcher) should_not_email(@u_watcher)
should_not_email(note.author) should_not_email(note.author)
...@@ -136,6 +144,7 @@ describe NotificationService, services: true do ...@@ -136,6 +144,7 @@ describe NotificationService, services: true do
let(:admin) { create(:admin) } let(:admin) { create(:admin) }
let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) }
let(:note) { create(:note_on_issue, noteable: confidential_issue, project: project, note: "#{author.to_reference} #{assignee.to_reference} #{non_member.to_reference} #{member.to_reference} #{admin.to_reference}") } let(:note) { create(:note_on_issue, noteable: confidential_issue, project: project, note: "#{author.to_reference} #{assignee.to_reference} #{non_member.to_reference} #{member.to_reference} #{admin.to_reference}") }
let(:guest_watcher) { create_user_with_notification(:watch, "guest-watcher-confidential") }
it 'filters out users that can not read the issue' do it 'filters out users that can not read the issue' do
project.team << [member, :developer] project.team << [member, :developer]
...@@ -149,6 +158,7 @@ describe NotificationService, services: true do ...@@ -149,6 +158,7 @@ describe NotificationService, services: true do
should_not_email(non_member) should_not_email(non_member)
should_not_email(guest) should_not_email(guest)
should_not_email(guest_watcher)
should_email(author) should_email(author)
should_email(assignee) should_email(assignee)
should_email(member) should_email(member)
...@@ -223,6 +233,9 @@ describe NotificationService, services: true do ...@@ -223,6 +233,9 @@ describe NotificationService, services: true do
should_email(member) should_email(member)
end end
# it emails custom global users on mention
should_email(@u_custom_global)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(note.noteable.author) should_email(note.noteable.author)
should_not_email(note.author) should_not_email(note.author)
...@@ -241,13 +254,16 @@ describe NotificationService, services: true do ...@@ -241,13 +254,16 @@ describe NotificationService, services: true do
build_team(note.project) build_team(note.project)
ActionMailer::Base.deliveries.clear ActionMailer::Base.deliveries.clear
allow_any_instance_of(Commit).to receive(:author).and_return(@u_committer) allow_any_instance_of(Commit).to receive(:author).and_return(@u_committer)
update_custom_notification(:new_note, @u_guest_custom, project)
update_custom_notification(:new_note, @u_custom_global)
end end
describe '#new_note, #perform_enqueued_jobs' do describe '#new_note, #perform_enqueued_jobs' do
it do it do
notification.new_note(note) notification.new_note(note)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_custom_global)
should_email(@u_guest_custom)
should_email(@u_committer) should_email(@u_committer)
should_email(@u_watcher) should_email(@u_watcher)
should_not_email(@u_mentioned) should_not_email(@u_mentioned)
...@@ -288,6 +304,8 @@ describe NotificationService, services: true do ...@@ -288,6 +304,8 @@ describe NotificationService, services: true do
build_team(issue.project) build_team(issue.project)
add_users_with_subscription(issue.project, issue) add_users_with_subscription(issue.project, issue)
ActionMailer::Base.deliveries.clear ActionMailer::Base.deliveries.clear
update_custom_notification(:new_issue, @u_guest_custom, project)
update_custom_notification(:new_issue, @u_custom_global)
end end
describe '#new_issue' do describe '#new_issue' do
...@@ -297,6 +315,8 @@ describe NotificationService, services: true do ...@@ -297,6 +315,8 @@ describe NotificationService, services: true do
should_email(issue.assignee) should_email(issue.assignee)
should_email(@u_watcher) should_email(@u_watcher)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_custom_global)
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_not_email(@u_mentioned) should_not_email(@u_mentioned)
should_not_email(@u_participating) should_not_email(@u_participating)
...@@ -345,6 +365,7 @@ describe NotificationService, services: true do ...@@ -345,6 +365,7 @@ describe NotificationService, services: true do
notification.new_issue(confidential_issue, @u_disabled) notification.new_issue(confidential_issue, @u_disabled)
should_not_email(@u_guest_watcher)
should_not_email(non_member) should_not_email(non_member)
should_not_email(author) should_not_email(author)
should_not_email(guest) should_not_email(guest)
...@@ -356,12 +377,20 @@ describe NotificationService, services: true do ...@@ -356,12 +377,20 @@ describe NotificationService, services: true do
end end
describe '#reassigned_issue' do describe '#reassigned_issue' do
before do
update_custom_notification(:reassign_issue, @u_guest_custom, project)
update_custom_notification(:reassign_issue, @u_custom_global)
end
it 'emails new assignee' do it 'emails new assignee' do
notification.reassigned_issue(issue, @u_disabled) notification.reassigned_issue(issue, @u_disabled)
should_email(issue.assignee) should_email(issue.assignee)
should_email(@u_watcher) should_email(@u_watcher)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_custom_global)
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_email(@subscriber) should_email(@subscriber)
should_not_email(@unsubscriber) should_not_email(@unsubscriber)
...@@ -378,8 +407,10 @@ describe NotificationService, services: true do ...@@ -378,8 +407,10 @@ describe NotificationService, services: true do
should_email(@u_mentioned) should_email(@u_mentioned)
should_email(@u_watcher) should_email(@u_watcher)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_email(@subscriber) should_email(@subscriber)
should_email(@u_custom_global)
should_not_email(@unsubscriber) should_not_email(@unsubscriber)
should_not_email(@u_participating) should_not_email(@u_participating)
should_not_email(@u_disabled) should_not_email(@u_disabled)
...@@ -394,8 +425,10 @@ describe NotificationService, services: true do ...@@ -394,8 +425,10 @@ describe NotificationService, services: true do
should_email(issue.assignee) should_email(issue.assignee)
should_email(@u_watcher) should_email(@u_watcher)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_email(@subscriber) should_email(@subscriber)
should_email(@u_custom_global)
should_not_email(@unsubscriber) should_not_email(@unsubscriber)
should_not_email(@u_participating) should_not_email(@u_participating)
should_not_email(@u_disabled) should_not_email(@u_disabled)
...@@ -410,8 +443,10 @@ describe NotificationService, services: true do ...@@ -410,8 +443,10 @@ describe NotificationService, services: true do
should_email(issue.assignee) should_email(issue.assignee)
should_email(@u_watcher) should_email(@u_watcher)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_email(@subscriber) should_email(@subscriber)
should_email(@u_custom_global)
should_not_email(@unsubscriber) should_not_email(@unsubscriber)
should_not_email(@u_participating) should_not_email(@u_participating)
should_not_email(@u_disabled) should_not_email(@u_disabled)
...@@ -425,8 +460,10 @@ describe NotificationService, services: true do ...@@ -425,8 +460,10 @@ describe NotificationService, services: true do
expect(issue.assignee).to be @u_mentioned expect(issue.assignee).to be @u_mentioned
should_email(@u_watcher) should_email(@u_watcher)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_email(@subscriber) should_email(@subscriber)
should_email(@u_custom_global)
should_not_email(issue.assignee) should_not_email(issue.assignee)
should_not_email(@unsubscriber) should_not_email(@unsubscriber)
should_not_email(@u_participating) should_not_email(@u_participating)
...@@ -529,6 +566,12 @@ describe NotificationService, services: true do ...@@ -529,6 +566,12 @@ describe NotificationService, services: true do
end end
describe '#close_issue' do describe '#close_issue' do
before do
update_custom_notification(:close_issue, @u_guest_custom, project)
update_custom_notification(:close_issue, @u_custom_global)
end
it 'should sent email to issue assignee and issue author' do it 'should sent email to issue assignee and issue author' do
notification.close_issue(issue, @u_disabled) notification.close_issue(issue, @u_disabled)
...@@ -536,6 +579,8 @@ describe NotificationService, services: true do ...@@ -536,6 +579,8 @@ describe NotificationService, services: true do
should_email(issue.author) should_email(issue.author)
should_email(@u_watcher) should_email(@u_watcher)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_custom_global)
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_email(@subscriber) should_email(@subscriber)
should_email(@watcher_and_subscriber) should_email(@watcher_and_subscriber)
...@@ -575,6 +620,11 @@ describe NotificationService, services: true do ...@@ -575,6 +620,11 @@ describe NotificationService, services: true do
end end
describe '#reopen_issue' do describe '#reopen_issue' do
before do
update_custom_notification(:reopen_issue, @u_guest_custom, project)
update_custom_notification(:reopen_issue, @u_custom_global)
end
it 'should send email to issue assignee and issue author' do it 'should send email to issue assignee and issue author' do
notification.reopen_issue(issue, @u_disabled) notification.reopen_issue(issue, @u_disabled)
...@@ -582,6 +632,8 @@ describe NotificationService, services: true do ...@@ -582,6 +632,8 @@ describe NotificationService, services: true do
should_email(issue.author) should_email(issue.author)
should_email(@u_watcher) should_email(@u_watcher)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_custom_global)
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_email(@subscriber) should_email(@subscriber)
should_email(@watcher_and_subscriber) should_email(@watcher_and_subscriber)
...@@ -631,6 +683,11 @@ describe NotificationService, services: true do ...@@ -631,6 +683,11 @@ describe NotificationService, services: true do
end end
describe '#new_merge_request' do describe '#new_merge_request' do
before do
update_custom_notification(:new_merge_request, @u_guest_custom, project)
update_custom_notification(:new_merge_request, @u_custom_global)
end
it do it do
notification.new_merge_request(merge_request, @u_disabled) notification.new_merge_request(merge_request, @u_disabled)
...@@ -639,6 +696,8 @@ describe NotificationService, services: true do ...@@ -639,6 +696,8 @@ describe NotificationService, services: true do
should_email(@watcher_and_subscriber) should_email(@watcher_and_subscriber)
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_custom_global)
should_not_email(@u_participating) should_not_email(@u_participating)
should_not_email(@u_disabled) should_not_email(@u_disabled)
should_not_email(@u_lazy_participant) should_not_email(@u_lazy_participant)
...@@ -685,6 +744,11 @@ describe NotificationService, services: true do ...@@ -685,6 +744,11 @@ describe NotificationService, services: true do
end end
describe '#reassigned_merge_request' do describe '#reassigned_merge_request' do
before do
update_custom_notification(:reassign_merge_request, @u_guest_custom, project)
update_custom_notification(:reassign_merge_request, @u_custom_global)
end
it do it do
notification.reassigned_merge_request(merge_request, merge_request.author) notification.reassigned_merge_request(merge_request, merge_request.author)
...@@ -694,6 +758,8 @@ describe NotificationService, services: true do ...@@ -694,6 +758,8 @@ describe NotificationService, services: true do
should_email(@subscriber) should_email(@subscriber)
should_email(@watcher_and_subscriber) should_email(@watcher_and_subscriber)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_custom_global)
should_not_email(@unsubscriber) should_not_email(@unsubscriber)
should_not_email(@u_participating) should_not_email(@u_participating)
should_not_email(@u_disabled) should_not_email(@u_disabled)
...@@ -761,12 +827,19 @@ describe NotificationService, services: true do ...@@ -761,12 +827,19 @@ describe NotificationService, services: true do
end end
describe '#closed_merge_request' do describe '#closed_merge_request' do
before do
update_custom_notification(:close_merge_request, @u_guest_custom, project)
update_custom_notification(:close_merge_request, @u_custom_global)
end
it do it do
notification.close_mr(merge_request, @u_disabled) notification.close_mr(merge_request, @u_disabled)
should_email(merge_request.assignee) should_email(merge_request.assignee)
should_email(@u_watcher) should_email(@u_watcher)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_custom_global)
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_email(@subscriber) should_email(@subscriber)
should_email(@watcher_and_subscriber) should_email(@watcher_and_subscriber)
...@@ -807,6 +880,12 @@ describe NotificationService, services: true do ...@@ -807,6 +880,12 @@ describe NotificationService, services: true do
end end
describe '#merged_merge_request' do describe '#merged_merge_request' do
before do
update_custom_notification(:merge_merge_request, @u_guest_custom, project)
update_custom_notification(:merge_merge_request, @u_custom_global)
end
it do it do
notification.merge_mr(merge_request, @u_disabled) notification.merge_mr(merge_request, @u_disabled)
...@@ -816,6 +895,8 @@ describe NotificationService, services: true do ...@@ -816,6 +895,8 @@ describe NotificationService, services: true do
should_email(@subscriber) should_email(@subscriber)
should_email(@watcher_and_subscriber) should_email(@watcher_and_subscriber)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_custom_global)
should_email(@u_guest_custom)
should_not_email(@unsubscriber) should_not_email(@unsubscriber)
should_not_email(@u_participating) should_not_email(@u_participating)
should_not_email(@u_disabled) should_not_email(@u_disabled)
...@@ -853,6 +934,11 @@ describe NotificationService, services: true do ...@@ -853,6 +934,11 @@ describe NotificationService, services: true do
end end
describe '#reopen_merge_request' do describe '#reopen_merge_request' do
before do
update_custom_notification(:reopen_merge_request, @u_guest_custom, project)
update_custom_notification(:reopen_merge_request, @u_custom_global)
end
it do it do
notification.reopen_mr(merge_request, @u_disabled) notification.reopen_mr(merge_request, @u_disabled)
...@@ -862,6 +948,8 @@ describe NotificationService, services: true do ...@@ -862,6 +948,8 @@ describe NotificationService, services: true do
should_email(@subscriber) should_email(@subscriber)
should_email(@watcher_and_subscriber) should_email(@watcher_and_subscriber)
should_email(@u_guest_watcher) should_email(@u_guest_watcher)
should_email(@u_guest_custom)
should_email(@u_custom_global)
should_not_email(@unsubscriber) should_not_email(@unsubscriber)
should_not_email(@u_participating) should_not_email(@u_participating)
should_not_email(@u_disabled) should_not_email(@u_disabled)
...@@ -914,7 +1002,9 @@ describe NotificationService, services: true do ...@@ -914,7 +1002,9 @@ describe NotificationService, services: true do
should_email(@u_watcher) should_email(@u_watcher)
should_email(@u_participating) should_email(@u_participating)
should_email(@u_lazy_participant) should_email(@u_lazy_participant)
should_email(@u_custom_global)
should_not_email(@u_guest_watcher) should_not_email(@u_guest_watcher)
should_not_email(@u_guest_custom)
should_not_email(@u_disabled) should_not_email(@u_disabled)
end end
end end
...@@ -929,13 +1019,15 @@ describe NotificationService, services: true do ...@@ -929,13 +1019,15 @@ describe NotificationService, services: true do
@u_committer = create(:user, username: 'committer') @u_committer = create(:user, username: 'committer')
@u_not_mentioned = create_global_setting_for(create(:user, username: 'regular'), :participating) @u_not_mentioned = create_global_setting_for(create(:user, username: 'regular'), :participating)
@u_outsider_mentioned = create(:user, username: 'outsider') @u_outsider_mentioned = create(:user, username: 'outsider')
@u_custom_global = create_global_setting_for(create(:user, username: 'custom_global'), :custom)
# User to be participant by default # User to be participant by default
# This user does not contain any record in notification settings table # This user does not contain any record in notification settings table
# It should be treated with a :participating notification_level # It should be treated with a :participating notification_level
@u_lazy_participant = create(:user, username: 'lazy-participant') @u_lazy_participant = create(:user, username: 'lazy-participant')
create_guest_watcher @u_guest_watcher = create_user_with_notification(:watch, 'guest_watching')
@u_guest_custom = create_user_with_notification(:custom, 'guest_custom')
project.team << [@u_watcher, :master] project.team << [@u_watcher, :master]
project.team << [@u_participating, :master] project.team << [@u_participating, :master]
...@@ -945,6 +1037,7 @@ describe NotificationService, services: true do ...@@ -945,6 +1037,7 @@ describe NotificationService, services: true do
project.team << [@u_committer, :master] project.team << [@u_committer, :master]
project.team << [@u_not_mentioned, :master] project.team << [@u_not_mentioned, :master]
project.team << [@u_lazy_participant, :master] project.team << [@u_lazy_participant, :master]
project.team << [@u_custom_global, :master]
end end
def create_global_setting_for(user, level) def create_global_setting_for(user, level)
...@@ -955,10 +1048,20 @@ describe NotificationService, services: true do ...@@ -955,10 +1048,20 @@ describe NotificationService, services: true do
user user
end end
def create_guest_watcher def create_user_with_notification(level, username)
@u_guest_watcher = create(:user, username: 'guest_watching') user = create(:user, username: username)
setting = @u_guest_watcher.notification_settings_for(project) setting = user.notification_settings_for(project)
setting.level = :watch setting.level = level
setting.save
user
end
# Create custom notifications
# When resource is nil it means global notification
def update_custom_notification(event, user, resource = nil)
setting = user.notification_settings_for(resource)
setting.events[event] = true
setting.save setting.save
end end
......
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