diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-06-17 23:28:22 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-06-17 23:28:22 +0000 |
commit | 00906b5bb6cde8cb60281109060a519a54000c61 (patch) | |
tree | f251efc0af4bebc2920c3ac4fc1398759cc490f9 | |
parent | 33d8972bf96d490e0a67750491249abf3d2e0d54 (diff) | |
parent | 4b204f071e4b626d4034fe431cebc902ae6caa78 (diff) | |
download | gitlab-ce-00906b5bb6cde8cb60281109060a519a54000c61.tar.gz |
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
34 files changed, 643 insertions, 249 deletions
diff --git a/CHANGELOG b/CHANGELOG index de9d71dae97..362b5bd580a 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 b560500cce6..7fbff9214cf 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 00000000000..74d2298c1fa --- /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 00000000000..3432428702a --- /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 26a12423521..1583d1ba6f9 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 07be85a32a5..d12bad97a05 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 855d86cb238..ed8e9d6915b 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 de13b16ccf2..00000000000 --- 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 00000000000..eddd03cc229 --- /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 40d1906a53f..b8b71d295f6 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 7d81cc03c73..00000000000 --- 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 50c21fc0d49..77783cd7640 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 0ce87968e46..d41fc7073c6 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 c125b8aff29..19832a19b2b 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 f0cf82afe83..537bba21f4a 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 e0fad555c09..5b2a69b8891 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 f2659ac14b5..5afd83a522e 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 2b19ee93eea..f6bfa567fd0 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 a7a97181096..00000000000 --- 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 00000000000..ff1cf966a9b --- /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 00000000000..b704981e3db --- /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 00000000000..d3258ee64cb --- /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 987d8507763..de6094fa0ed 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -123,10 +123,18 @@ Rails.application.routes.draw do end end + # # Spam reports + # resources :abuse_reports, only: [:new, :create] # + # Notification settings + # + resources :notification_settings, only: [:create, :update] + + + # # Import # namespace :import do @@ -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 00000000000..609596f45e4 --- /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 cbca94c0b5e..fe4485e148a 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. ![notification settings](notifications/settings.png) @@ -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 Binary files differindex e5b50ee2494..7c6857aad1a 100644 --- a/doc/workflow/notifications/settings.png +++ b/doc/workflow/notifications/settings.png diff --git a/features/steps/profile/notifications.rb b/features/steps/profile/notifications.rb index a96f35ada51..979f4692d5a 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 2a1a8e776f0..98b57e5cbfb 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 0786e45515a..00000000000 --- 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 00000000000..15d155833b4 --- /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 c5d17d97ec9..00000000000 --- 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 4e24e89b008..df336a6effe 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 f167813e07d..01eb4b44b83 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 e871a103d42..776a6ab5edb 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 |