diff --git a/CHANGELOG b/CHANGELOG
index de9d71dae97cb9f50f2bc9d5d108dfe04cfff813..362b5bd580a636d0fa361eb65cf48464b911dfc8 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -74,6 +74,7 @@ v 8.9.0 (unreleased)
- Pipelines can be canceled only when there are running builds
- Allow authentication using personal access tokens
- Use downcased path to container repository as this is expected path by Docker
+ - Custom notification settings
- Projects pending deletion will render a 404 page
- Measure queue duration between gitlab-workhorse and Rails
- Added Gfm autocomplete for labels
diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee
index b560500cce6b060ecfcd195b79b7d0c827c29967..7fbff9214cf57abd86327a8566517c2a0c15117b 100644
--- a/app/assets/javascripts/dispatcher.js.coffee
+++ b/app/assets/javascripts/dispatcher.js.coffee
@@ -78,6 +78,7 @@ class Dispatcher
when 'projects:show'
shortcut_handler = new ShortcutsNavigation()
+ new NotificationsForm()
new TreeView() if $('#tree-slider').length
when 'groups:activity'
new Activities()
@@ -129,6 +130,8 @@ class Dispatcher
shortcut_handler = new ShortcutsDashboardNavigation()
when 'profiles'
new Profile()
+ new NotificationsForm()
+ new NotificationsDropdown()
when 'projects'
new Project()
new ProjectAvatar()
@@ -136,8 +139,12 @@ class Dispatcher
when 'edit'
shortcut_handler = new ShortcutsNavigation()
new ProjectNew()
- when 'new', 'show'
+ when 'new'
new ProjectNew()
+ when 'show'
+ new ProjectNew()
+ new ProjectShow()
+ new NotificationsDropdown()
when 'wikis'
new Wikis()
shortcut_handler = new ShortcutsNavigation()
diff --git a/app/assets/javascripts/notifications_dropdown.js.coffee b/app/assets/javascripts/notifications_dropdown.js.coffee
new file mode 100644
index 0000000000000000000000000000000000000000..74d2298c1fa74bbf4f5110b90340e51519d89a4c
--- /dev/null
+++ b/app/assets/javascripts/notifications_dropdown.js.coffee
@@ -0,0 +1,24 @@
+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')
diff --git a/app/assets/javascripts/notifications_form.js.coffee b/app/assets/javascripts/notifications_form.js.coffee
new file mode 100644
index 0000000000000000000000000000000000000000..3432428702a868cc1f03aa4304cac98f01bf892b
--- /dev/null
+++ b/app/assets/javascripts/notifications_form.js.coffee
@@ -0,0 +1,49 @@
+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)
diff --git a/app/assets/javascripts/profile.js.coffee b/app/assets/javascripts/profile.js.coffee
index 26a12423521b95691e72f17b54719117a012cba6..1583d1ba6f96c49a931f630a584d140595281998 100644
--- a/app/assets/javascripts/profile.js.coffee
+++ b/app/assets/javascripts/profile.js.coffee
@@ -8,6 +8,10 @@ class @Profile
$('.js-preferences-form').on 'change.preference', 'input[type=radio]', ->
$(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', ->
$('.loading-username').show()
$(this).find('.update-success').hide()
diff --git a/app/assets/javascripts/project.js.coffee b/app/assets/javascripts/project.js.coffee
index 07be85a32a5cfa43b813626512de3ab8646cf77c..d12bad97a0517ebbf23b35cb2180a3651b76e45a 100644
--- a/app/assets/javascripts/project.js.coffee
+++ b/app/assets/javascripts/project.js.coffee
@@ -34,22 +34,6 @@ class @Project
$(@).parents('.no-password-message').remove()
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()
diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss
index 855d86cb238ca030b9d2a641c09e25021f4794c4..ed8e9d6915b84cff63db3573ba98fa6f087da464 100644
--- a/app/assets/stylesheets/pages/projects.scss
+++ b/app/assets/stylesheets/pages/projects.scss
@@ -128,11 +128,6 @@
}
}
- .btn-group:not(:first-child):not(:last-child) > .btn {
- border-top-right-radius: 3px;
- border-bottom-right-radius: 3px;
- }
-
form {
margin-left: 10px;
}
@@ -603,3 +598,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;
+ }
+}
diff --git a/app/controllers/groups/notification_settings_controller.rb b/app/controllers/groups/notification_settings_controller.rb
deleted file mode 100644
index de13b16ccf210cb65f1d9dbd10be9cc360cd880b..0000000000000000000000000000000000000000
--- a/app/controllers/groups/notification_settings_controller.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-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
diff --git a/app/controllers/notification_settings_controller.rb b/app/controllers/notification_settings_controller.rb
new file mode 100644
index 0000000000000000000000000000000000000000..eddd03cc22949777ba0b03cc6993639b7a91e798
--- /dev/null
+++ b/app/controllers/notification_settings_controller.rb
@@ -0,0 +1,36 @@
+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
diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb
index 40d1906a53f4bd5838aed40e4b42fa983794b5bf..b8b71d295f6d2757d3aa9da3417df86411a258f4 100644
--- a/app/controllers/profiles/notifications_controller.rb
+++ b/app/controllers/profiles/notifications_controller.rb
@@ -1,13 +1,13 @@
class Profiles::NotificationsController < Profiles::ApplicationController
def show
@user = current_user
- @group_notifications = current_user.notification_settings.for_groups
- @project_notifications = current_user.notification_settings.for_projects
+ @group_notifications = current_user.notification_settings.for_groups.order(:id)
+ @project_notifications = current_user.notification_settings.for_projects.order(:id)
@global_notification_setting = current_user.global_notification_setting
end
def update
- if current_user.update_attributes(user_params) && update_notification_settings
+ if current_user.update_attributes(user_params)
flash[:notice] = "Notification settings saved"
else
flash[:alert] = "Failed to save new settings"
@@ -19,16 +19,4 @@ class Profiles::NotificationsController < Profiles::ApplicationController
def user_params
params.require(:user).permit(:notification_email)
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
diff --git a/app/controllers/projects/notification_settings_controller.rb b/app/controllers/projects/notification_settings_controller.rb
deleted file mode 100644
index 7d81cc03c73f6a91bc322ca5221654ed163a03cb..0000000000000000000000000000000000000000
--- a/app/controllers/projects/notification_settings_controller.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-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
diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb
index 50c21fc0d49da9af38ba75550c40902cb84dd698..77783cd764060c392a1d922a75e564274ea46c82 100644
--- a/app/helpers/notifications_helper.rb
+++ b/app/helpers/notifications_helper.rb
@@ -34,7 +34,7 @@ module NotificationsHelper
def notification_description(level)
case level.to_sym
when :participating
- 'You will only receive notifications from related resources'
+ 'You will only receive notifications for threads you have participated in'
when :mention
'You will receive notifications only for comments in which you were @mentioned'
when :watch
@@ -43,6 +43,8 @@ module NotificationsHelper
'You will not get any notifications via email'
when :global
'Use your global notification setting'
+ when :custom
+ 'You will only receive notifications for the events you choose'
end
end
@@ -62,22 +64,14 @@ module NotificationsHelper
end
end
- def notification_level_radio_buttons
- html = ""
-
- NotificationSetting.levels.each_key do |level|
- 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
+ # Identifier to trigger individually dropdowns and custom settings modals in the same view
+ def notifications_menu_identifier(type, notification_setting)
+ "#{type}-#{notification_setting.user_id}-#{notification_setting.source_id}-#{notification_setting.source_type}"
+ 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
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
index 0ce87968e46bfc7a37d975fb294255f0071ddd87..d41fc7073c631f97a6b832df8b8b7ee0e63f9ad2 100644
--- a/app/models/notification_setting.rb
+++ b/app/models/notification_setting.rb
@@ -1,5 +1,5 @@
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]
@@ -15,6 +15,24 @@ class NotificationSetting < ActiveRecord::Base
scope :for_groups, -> { where(source_type: 'Namespace') }
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)
setting = find_or_initialize_by(source: source)
@@ -24,4 +42,21 @@ class NotificationSetting < ActiveRecord::Base
setting
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
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index c125b8aff29b908053b379766ea07c647d9fc45c..19832a19b2b3a8d2584851642ee3e53bdd0be097 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -29,9 +29,10 @@ class NotificationService
# * issue assignee if their notification level is not Disabled
# * project team members with notification level higher then Participating
# * watchers of the issue's labels
+ # * users with custom level checked with "new issue"
#
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
# When we close an issue we should send an email to:
@@ -39,18 +40,20 @@ class NotificationService
# * issue author 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
+ # * users with custom level checked with "close issue"
#
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
# When we reassign an issue we should send an email to:
#
# * issue old 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)
- reassign_resource_email(issue, issue.project, current_user, 'reassigned_issue_email')
+ reassign_resource_email(issue, issue.project, current_user, :reassigned_issue_email)
end
# When we add labels to an issue we should send an email to:
@@ -58,7 +61,7 @@ class NotificationService
# * watchers of the issue's labels
#
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
# When create a merge request we should send an email to:
@@ -66,18 +69,20 @@ class NotificationService
# * mr assignee if their notification level is not Disabled
# * project team members with notification level higher then Participating
# * watchers of the mr's labels
+ # * users with custom level checked with "new merge request"
#
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
# 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 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)
- 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
# When we add labels to a merge request we should send an email to:
@@ -85,15 +90,15 @@ class NotificationService
# * watchers of the mr's labels
#
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
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
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
def merge_mr(merge_request, current_user)
@@ -101,7 +106,7 @@ class NotificationService
merge_request,
merge_request.target_project,
current_user,
- 'merged_merge_request_email'
+ :merged_merge_request_email
)
end
@@ -110,7 +115,7 @@ class NotificationService
merge_request,
merge_request.target_project,
current_user,
- 'merge_request_status_email',
+ :merge_request_status_email,
'reopened'
)
end
@@ -153,6 +158,9 @@ class NotificationService
# Merge project watchers
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.
recipients = reject_mention_users(recipients - mentioned_users, note.project)
recipients = recipients + mentioned_users
@@ -276,12 +284,31 @@ class NotificationService
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
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_with_project_setting = select_project_member_setting(project, users_with_project_level_global, users)
@@ -290,33 +317,39 @@ class NotificationService
User.where(id: users_with_project_setting.concat(users_with_group_setting).uniq).to_a
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
- 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
- project.notification_settings.pluck(:user_id)
+ resource.notification_settings.pluck(:user_id)
end
end
- def group_member_notification(project, notification_level)
- if project.group
- project.group.notification_settings.where(level: NotificationSetting.levels[notification_level]).pluck(:user_id)
- else
- []
- end
+ def users_with_global_level_watch(ids)
+ settings_with_global_level_of(:watch, ids).pluck(:user_id)
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(
user_id: ids,
source_type: nil,
- level: NotificationSetting.levels[:watch]
- ).pluck(:user_id)
+ level: NotificationSetting.levels[level]
+ )
end
# Build a list of users based on project notifcation settings
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
global_setting.each do |user_id|
@@ -330,7 +363,7 @@ class NotificationService
# Build a list of users based on group notification settings
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
users = []
@@ -351,7 +384,7 @@ class NotificationService
end
def add_project_watchers(recipients, project)
- recipients.concat(project_watchers(project)).compact.uniq
+ recipients.concat(project_watchers(project)).compact
end
# Remove users with disabled notifications from array
@@ -436,7 +469,7 @@ class NotificationService
end
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|
mailer.send(method, recipient.id, target.id).deliver_later
@@ -444,7 +477,8 @@ class NotificationService
end
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|
mailer.send(method, recipient.id, target.id, current_user.id).deliver_later
@@ -455,7 +489,7 @@ class NotificationService
previous_assignee_id = previous_record(target, '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|
mailer.send(
@@ -478,7 +512,7 @@ class NotificationService
end
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|
mailer.send(method, recipient.id, target.id, status, current_user.id).deliver_later
@@ -486,14 +520,20 @@ class NotificationService
end
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 = add_project_watchers(recipients, project)
+
+ recipients = add_custom_notifications(recipients, project, custom_action)
recipients = reject_mention_users(recipients, project)
+ recipients = recipients.uniq
+
# 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
# the "on mention" notification level
- if action == :reassign
+ if [:reassign_merge_request, :reassign_issue].include?(custom_action)
recipients << previous_assignee if previous_assignee
recipients << target.assignee
end
@@ -501,7 +541,7 @@ class NotificationService
recipients = reject_muted_users(recipients, project)
recipients = add_subscribed_users(recipients, target)
- if action == :new
+ if [:new_issue, :new_merge_request].include?(custom_action)
recipients = add_labels_subscribers(recipients, target)
end
@@ -531,4 +571,10 @@ class NotificationService
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
diff --git a/app/views/profiles/notifications/_group_settings.html.haml b/app/views/profiles/notifications/_group_settings.html.haml
index f0cf82afe831d03a419640e66aadfaf1a5f86a38..537bba21f4a6824d87cc3ac08d7da2bd5175b27c 100644
--- a/app/views/profiles/notifications/_group_settings.html.haml
+++ b/app/views/profiles/notifications/_group_settings.html.haml
@@ -9,5 +9,4 @@
= link_to group.name, group_path(group)
.pull-right
- = form_for [group, setting], remote: true, html: { class: 'update-notifications' } do |f|
- = f.select :level, NotificationSetting.levels.keys, {}, class: 'form-control trigger-submit'
+ = render 'shared/notifications/button', notification_setting: setting
diff --git a/app/views/profiles/notifications/_project_settings.html.haml b/app/views/profiles/notifications/_project_settings.html.haml
index e0fad555c098a897b309fe42c4182f6d3748e1e3..5b2a69b8891601cd967bfd1530ec3747dc48a146 100644
--- a/app/views/profiles/notifications/_project_settings.html.haml
+++ b/app/views/profiles/notifications/_project_settings.html.haml
@@ -9,5 +9,4 @@
= link_to_project(project)
.pull-right
- = form_for [project.namespace.becomes(Namespace), project, setting], remote: true, html: { class: 'update-notifications' } do |f|
- = f.select :level, NotificationSetting.levels.keys, {}, class: 'form-control trigger-submit'
+ = render 'shared/notifications/button', notification_setting: setting
diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml
index f2659ac14b54e37c57131881132040b4b7b375a5..5afd83a522e56acf6302f1ad1ab9ecf9c9498545 100644
--- a/app/views/profiles/notifications/show.html.haml
+++ b/app/views/profiles/notifications/show.html.haml
@@ -24,12 +24,15 @@
.form-group
= f.label :notification_email, class: "label-light"
= 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
- = f.submit 'Update settings', class: "btn btn-create"
+ = label_tag :global_notification_level, "Global notification level", class: "label-light"
+ %br
+ .clearfix
+ .form-group.pull-left
+ = render 'shared/notifications/button', notification_setting: @global_notification_setting, left_align: true
+
+ .clearfix
+
%hr
%h5
Groups (#{@group_notifications.count})
diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml
index 2b19ee93eea6645bc7070fb80d47ed5329ef11f9..f6bfa567fd05c0168c29a74f59308fcb73fa8723 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/app/views/projects/_home_panel.html.haml
@@ -29,13 +29,13 @@
.project-clone-holder
= render "shared/clone_panel"
- .project-repo-buttons.project-right-buttons
+ .project-repo-buttons.btn-group.project-right-buttons
- if current_user
= render 'shared/members/access_request_buttons', source: @project
- .btn-group
- = render "projects/buttons/download"
- = render 'projects/buttons/dropdown'
- = render 'projects/buttons/notifications'
+
+ = render "projects/buttons/download"
+ = render 'projects/buttons/dropdown'
+ = render 'shared/notifications/button', notification_setting: @notification_setting
:javascript
new Star();
diff --git a/app/views/projects/buttons/_notifications.html.haml b/app/views/projects/buttons/_notifications.html.haml
deleted file mode 100644
index a7a97181096932d82026e4ae8feff5274405ad83..0000000000000000000000000000000000000000
--- a/app/views/projects/buttons/_notifications.html.haml
+++ /dev/null
@@ -1,11 +0,0 @@
-- 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)
diff --git a/app/views/shared/notifications/_button.html.haml b/app/views/shared/notifications/_button.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..ff1cf966a9b4caf200f9d09ebcc7db975a8fd8d7
--- /dev/null
+++ b/app/views/shared/notifications/_button.html.haml
@@ -0,0 +1,25 @@
+- 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
diff --git a/app/views/shared/notifications/_custom_notifications.html.haml b/app/views/shared/notifications/_custom_notifications.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..b704981e3dbfdcabec388392611647077a7e53c9
--- /dev/null
+++ b/app/views/shared/notifications/_custom_notifications.html.haml
@@ -0,0 +1,31 @@
+.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")
diff --git a/app/views/shared/notifications/_notification_dropdown.html.haml b/app/views/shared/notifications/_notification_dropdown.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..d3258ee64cb42dacef7106f882e6b773a86509dc
--- /dev/null
+++ b/app/views/shared/notifications/_notification_dropdown.html.haml
@@ -0,0 +1,13 @@
+- 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")
diff --git a/config/routes.rb b/config/routes.rb
index 987d8507763b12aa5ef0e6b6470dfa95642ce7be..de6094fa0edfdada440fd54816fe0fa2c33b68c2 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -123,9 +123,17 @@ Rails.application.routes.draw do
end
end
+ #
# Spam reports
+ #
resources :abuse_reports, only: [:new, :create]
+ #
+ # Notification settings
+ #
+ resources :notification_settings, only: [:create, :update]
+
+
#
# Import
#
@@ -432,7 +440,6 @@ Rails.application.routes.draw do
resource :avatar, only: [:destroy]
resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
- resource :notification_setting, only: [:update]
end
end
@@ -668,7 +675,6 @@ Rails.application.routes.draw do
resources :forks, only: [:index, :new, :create]
resource :import, only: [:new, :create, :show]
- resource :notification_setting, only: [:update]
resources :refs, only: [] do
collection do
diff --git a/db/migrate/20160617301627_add_events_to_notification_settings.rb b/db/migrate/20160617301627_add_events_to_notification_settings.rb
new file mode 100644
index 0000000000000000000000000000000000000000..609596f45e455b57bfb245c7c8007bf89bdb3943
--- /dev/null
+++ b/db/migrate/20160617301627_add_events_to_notification_settings.rb
@@ -0,0 +1,7 @@
+class AddEventsToNotificationSettings < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ def change
+ add_column :notification_settings, :events, :text
+ end
+end
diff --git a/doc/workflow/notifications.md b/doc/workflow/notifications.md
index cbca94c0b5eacb6c8af6c4b55e15fc0d16b6a1bb..fe4485e148a629423eb1939c11b9f618a8036a77 100644
--- a/doc/workflow/notifications.md
+++ b/doc/workflow/notifications.md
@@ -4,7 +4,7 @@ GitLab has a notification system in place to notify a user of events that are im
## Notification settings
-Under user profile page you can find the notification settings.
+You can find notification settings under the user profile.
@@ -20,6 +20,7 @@ Each of these settings have levels of notification:
* Participating - receive notifications from related resources
* Watch - receive notifications from projects or groups user is a member of
* Global - notifications as set at the global settings
+* Custom - user will receive notifications when mentioned, is participant and custom selected events.
#### Global Settings
@@ -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 |
| 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 |
-| 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 |
### Issue / Merge Request events
@@ -71,6 +72,7 @@ In all of the below cases, the notification will be sent to:
- Watchers: users with notification level "Watch"
- 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 |
|------------------------|---------|
diff --git a/doc/workflow/notifications/settings.png b/doc/workflow/notifications/settings.png
index e5b50ee249478f8d5cd3601e801fe7b7d93d10c2..7c6857aad1a88f92b42b5ea30656d0ff0b75fc63 100644
Binary files a/doc/workflow/notifications/settings.png and b/doc/workflow/notifications/settings.png differ
diff --git a/features/steps/profile/notifications.rb b/features/steps/profile/notifications.rb
index a96f35ada51e821a43a26188c4652eb0a737af1f..979f4692d5a62d0d16248a7799c029c21285c612 100644
--- a/features/steps/profile/notifications.rb
+++ b/features/steps/profile/notifications.rb
@@ -11,7 +11,7 @@ class Spinach::Features::ProfileNotifications < Spinach::FeatureSteps
end
step 'I select Mention setting from dropdown' do
- select 'mention', from: 'notification_setting_level'
+ first(:link, "On mention").trigger('click')
end
step 'I should see Notification saved message' do
diff --git a/features/steps/project/project.rb b/features/steps/project/project.rb
index 2a1a8e776f02e109202c5d52c6be553bed643a8b..98b57e5cbfbe21766426c3854520d6544a487bf0 100644
--- a/features/steps/project/project.rb
+++ b/features/steps/project/project.rb
@@ -126,7 +126,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps
end
step 'I click notifications drop down button' do
- find('#notifications-button').click
+ first('.notifications-btn').click
end
step 'I choose Mention setting' do
diff --git a/spec/controllers/groups/notification_settings_controller_spec.rb b/spec/controllers/groups/notification_settings_controller_spec.rb
deleted file mode 100644
index 0786e45515a51c637970e54ab0c86233130ec215..0000000000000000000000000000000000000000
--- a/spec/controllers/groups/notification_settings_controller_spec.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-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
diff --git a/spec/controllers/notification_settings_controller_spec.rb b/spec/controllers/notification_settings_controller_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..15d155833b4f0ec61fbeb4eca28b732f9fbdf87d
--- /dev/null
+++ b/spec/controllers/notification_settings_controller_spec.rb
@@ -0,0 +1,125 @@
+require 'spec_helper'
+
+describe NotificationSettingsController do
+ let(:project) { create(:empty_project) }
+ let(:user) { create(:user) }
+
+ before do
+ project.team << [user, :developer]
+ end
+
+ describe '#create' do
+ context 'when not authorized' do
+ it 'redirects to sign in page' do
+ post :create,
+ project: { id: project.id },
+ 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
+ post :create,
+ project: { id: project.id },
+ 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
+ post :create,
+ project: { id: project.id },
+ notification_setting: { level: :participating, events: custom_events }
+
+ expect(response.status).to eq 200
+ end
+ end
+ end
+
+ context 'not authorized' do
+ let(:private_project) { create(:project, :private) }
+ 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
+ put :update,
+ id: notification_setting,
+ notification_setting: { level: :participating }
+
+ expect(response.status).to eq(404)
+ end
+ end
+ end
+end
diff --git a/spec/controllers/projects/notification_settings_controller_spec.rb b/spec/controllers/projects/notification_settings_controller_spec.rb
deleted file mode 100644
index c5d17d97ec9f0dcdb116d0440302a5ad56331730..0000000000000000000000000000000000000000
--- a/spec/controllers/projects/notification_settings_controller_spec.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-require 'spec_helper'
-
-describe Projects::NotificationSettingsController do
- let(:project) { create(:empty_project) }
- let(:user) { create(:user) }
-
- before do
- project.team << [user, :developer]
- end
-
- describe '#update' do
- context 'when not authorized' do
- it 'redirects to sign in page' do
- put :update,
- namespace_id: project.namespace.to_param,
- project_id: project.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,
- namespace_id: project.namespace.to_param,
- project_id: project.to_param,
- notification_setting: { level: :participating }
-
- expect(response.status).to eq 200
- end
- end
-
- context 'not authorized' do
- let(:private_project) { create(:project, :private) }
- before { sign_in(user) }
-
- it 'returns 404' do
- put :update,
- namespace_id: private_project.namespace.to_param,
- project_id: private_project.to_param,
- notification_setting: { level: :participating }
-
- expect(response.status).to eq(404)
- end
- end
- end
-end
diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb
index 4e24e89b00830c9cce0f6abe946262539577429a..df336a6effed56ea210f20065305c7830453cf57 100644
--- a/spec/models/notification_setting_spec.rb
+++ b/spec/models/notification_setting_spec.rb
@@ -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(:level) }
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
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index f167813e07dcfae10063fa51d8cfeba8945c3866..01eb4b44b83ff8911c9922f1297ec094a0026eff 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -428,8 +428,9 @@ describe API::API, api: true do
describe 'permissions' do
context 'all projects' do
- it 'Contains permission information' do
- project.team << [user, :master]
+ before { project.team << [user, :master] }
+
+ it 'contains permission information' do
get api("/projects", user)
expect(response.status).to eq(200)
@@ -440,7 +441,7 @@ describe API::API, api: true do
end
context 'personal project' do
- it 'Sets project access and returns 200' do
+ it 'sets project access and returns 200' do
project.team << [user, :master]
get api("/projects/#{project.id}", user)
@@ -452,9 +453,11 @@ describe API::API, api: true do
end
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
- project2 = create(:project, group: create(:group))
- project2.group.add_owner(user)
get api("/projects/#{project2.id}", user)
expect(response.status).to eq(200)
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index e871a103d42a96b3b513c072af434da80090b2eb..776a6ab5edbd4ed8233515e6f3594bf287564328 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -46,6 +46,8 @@ describe NotificationService, services: true do
project.team << [issue.assignee, :master]
project.team << [note.author, :master]
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
describe :new_note do
@@ -53,7 +55,7 @@ describe NotificationService, services: true do
add_users_with_subscription(note.project, issue)
# 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
@@ -62,10 +64,12 @@ describe NotificationService, services: true do
should_email(@u_watcher)
should_email(note.noteable.author)
should_email(note.noteable.assignee)
+ should_email(@u_custom_global)
should_email(@u_mentioned)
should_email(@subscriber)
should_email(@watcher_and_subscriber)
should_email(@subscribed_participant)
+ should_not_email(@u_guest_custom)
should_not_email(@u_guest_watcher)
should_not_email(note.author)
should_not_email(@u_participating)
@@ -103,10 +107,12 @@ describe NotificationService, services: true do
before do
note.project.namespace_id = group.id
note.project.group.add_user(@u_watcher, GroupMember::MASTER)
+ note.project.group.add_user(@u_custom_global, GroupMember::MASTER)
note.project.save
@u_watcher.notification_settings_for(note.project).participating!
@u_watcher.notification_settings_for(note.project.group).global!
+ update_custom_notification(:new_note, @u_custom_global)
ActionMailer::Base.deliveries.clear
end
@@ -116,6 +122,8 @@ describe NotificationService, services: true do
should_email(note.noteable.author)
should_email(note.noteable.assignee)
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_watcher)
should_not_email(note.author)
@@ -136,6 +144,7 @@ describe NotificationService, services: true do
let(:admin) { create(:admin) }
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(:guest_watcher) { create_user_with_notification(:watch, "guest-watcher-confidential") }
it 'filters out users that can not read the issue' do
project.team << [member, :developer]
@@ -149,6 +158,7 @@ describe NotificationService, services: true do
should_not_email(non_member)
should_not_email(guest)
+ should_not_email(guest_watcher)
should_email(author)
should_email(assignee)
should_email(member)
@@ -223,6 +233,9 @@ describe NotificationService, services: true do
should_email(member)
end
+ # it emails custom global users on mention
+ should_email(@u_custom_global)
+
should_email(@u_guest_watcher)
should_email(note.noteable.author)
should_not_email(note.author)
@@ -241,13 +254,16 @@ describe NotificationService, services: true do
build_team(note.project)
ActionMailer::Base.deliveries.clear
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
describe '#new_note, #perform_enqueued_jobs' do
it do
notification.new_note(note)
-
should_email(@u_guest_watcher)
+ should_email(@u_custom_global)
+ should_email(@u_guest_custom)
should_email(@u_committer)
should_email(@u_watcher)
should_not_email(@u_mentioned)
@@ -288,6 +304,8 @@ describe NotificationService, services: true do
build_team(issue.project)
add_users_with_subscription(issue.project, issue)
ActionMailer::Base.deliveries.clear
+ update_custom_notification(:new_issue, @u_guest_custom, project)
+ update_custom_notification(:new_issue, @u_custom_global)
end
describe '#new_issue' do
@@ -297,6 +315,8 @@ describe NotificationService, services: true do
should_email(issue.assignee)
should_email(@u_watcher)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
+ should_email(@u_custom_global)
should_email(@u_participant_mentioned)
should_not_email(@u_mentioned)
should_not_email(@u_participating)
@@ -345,6 +365,7 @@ describe NotificationService, services: true do
notification.new_issue(confidential_issue, @u_disabled)
+ should_not_email(@u_guest_watcher)
should_not_email(non_member)
should_not_email(author)
should_not_email(guest)
@@ -356,12 +377,20 @@ describe NotificationService, services: true do
end
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
notification.reassigned_issue(issue, @u_disabled)
should_email(issue.assignee)
should_email(@u_watcher)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
+ should_email(@u_custom_global)
should_email(@u_participant_mentioned)
should_email(@subscriber)
should_not_email(@unsubscriber)
@@ -378,8 +407,10 @@ describe NotificationService, services: true do
should_email(@u_mentioned)
should_email(@u_watcher)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
should_email(@u_participant_mentioned)
should_email(@subscriber)
+ should_email(@u_custom_global)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_disabled)
@@ -394,8 +425,10 @@ describe NotificationService, services: true do
should_email(issue.assignee)
should_email(@u_watcher)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
should_email(@u_participant_mentioned)
should_email(@subscriber)
+ should_email(@u_custom_global)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_disabled)
@@ -410,8 +443,10 @@ describe NotificationService, services: true do
should_email(issue.assignee)
should_email(@u_watcher)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
should_email(@u_participant_mentioned)
should_email(@subscriber)
+ should_email(@u_custom_global)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_disabled)
@@ -425,8 +460,10 @@ describe NotificationService, services: true do
expect(issue.assignee).to be @u_mentioned
should_email(@u_watcher)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
should_email(@u_participant_mentioned)
should_email(@subscriber)
+ should_email(@u_custom_global)
should_not_email(issue.assignee)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
@@ -529,6 +566,12 @@ describe NotificationService, services: true do
end
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
notification.close_issue(issue, @u_disabled)
@@ -536,6 +579,8 @@ describe NotificationService, services: true do
should_email(issue.author)
should_email(@u_watcher)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
+ should_email(@u_custom_global)
should_email(@u_participant_mentioned)
should_email(@subscriber)
should_email(@watcher_and_subscriber)
@@ -575,6 +620,11 @@ describe NotificationService, services: true do
end
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
notification.reopen_issue(issue, @u_disabled)
@@ -582,6 +632,8 @@ describe NotificationService, services: true do
should_email(issue.author)
should_email(@u_watcher)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
+ should_email(@u_custom_global)
should_email(@u_participant_mentioned)
should_email(@subscriber)
should_email(@watcher_and_subscriber)
@@ -631,6 +683,11 @@ describe NotificationService, services: true do
end
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
notification.new_merge_request(merge_request, @u_disabled)
@@ -639,6 +696,8 @@ describe NotificationService, services: true do
should_email(@watcher_and_subscriber)
should_email(@u_participant_mentioned)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
+ should_email(@u_custom_global)
should_not_email(@u_participating)
should_not_email(@u_disabled)
should_not_email(@u_lazy_participant)
@@ -685,6 +744,11 @@ describe NotificationService, services: true do
end
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
notification.reassigned_merge_request(merge_request, merge_request.author)
@@ -694,6 +758,8 @@ describe NotificationService, services: true do
should_email(@subscriber)
should_email(@watcher_and_subscriber)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
+ should_email(@u_custom_global)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_disabled)
@@ -761,12 +827,19 @@ describe NotificationService, services: true do
end
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
notification.close_mr(merge_request, @u_disabled)
should_email(merge_request.assignee)
should_email(@u_watcher)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
+ should_email(@u_custom_global)
should_email(@u_participant_mentioned)
should_email(@subscriber)
should_email(@watcher_and_subscriber)
@@ -807,6 +880,12 @@ describe NotificationService, services: true do
end
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
notification.merge_mr(merge_request, @u_disabled)
@@ -816,6 +895,8 @@ describe NotificationService, services: true do
should_email(@subscriber)
should_email(@watcher_and_subscriber)
should_email(@u_guest_watcher)
+ should_email(@u_custom_global)
+ should_email(@u_guest_custom)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_disabled)
@@ -853,6 +934,11 @@ describe NotificationService, services: true do
end
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
notification.reopen_mr(merge_request, @u_disabled)
@@ -862,6 +948,8 @@ describe NotificationService, services: true do
should_email(@subscriber)
should_email(@watcher_and_subscriber)
should_email(@u_guest_watcher)
+ should_email(@u_guest_custom)
+ should_email(@u_custom_global)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_disabled)
@@ -914,7 +1002,9 @@ describe NotificationService, services: true do
should_email(@u_watcher)
should_email(@u_participating)
should_email(@u_lazy_participant)
+ should_email(@u_custom_global)
should_not_email(@u_guest_watcher)
+ should_not_email(@u_guest_custom)
should_not_email(@u_disabled)
end
end
@@ -929,13 +1019,15 @@ describe NotificationService, services: true do
@u_committer = create(:user, username: 'committer')
@u_not_mentioned = create_global_setting_for(create(:user, username: 'regular'), :participating)
@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
# This user does not contain any record in notification settings table
# It should be treated with a :participating notification_level
@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_participating, :master]
@@ -945,6 +1037,7 @@ describe NotificationService, services: true do
project.team << [@u_committer, :master]
project.team << [@u_not_mentioned, :master]
project.team << [@u_lazy_participant, :master]
+ project.team << [@u_custom_global, :master]
end
def create_global_setting_for(user, level)
@@ -955,10 +1048,20 @@ describe NotificationService, services: true do
user
end
- def create_guest_watcher
- @u_guest_watcher = create(:user, username: 'guest_watching')
- setting = @u_guest_watcher.notification_settings_for(project)
- setting.level = :watch
+ def create_user_with_notification(level, username)
+ user = create(:user, username: username)
+ setting = user.notification_settings_for(project)
+ 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
end