diff options
18 files changed, 354 insertions, 78 deletions
diff --git a/CHANGELOG b/CHANGELOG index 07d2d950bbc..48c418a8beb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -54,6 +54,7 @@ v 8.9.0 (unreleased) - Remove 'main language' feature - Pipelines can be canceled only when there are running builds - Use downcased path to container repository as this is expected path by Docker + - Customized notification settings for projects - Projects pending deletion will render a 404 page - Measure queue duration between gitlab-workhorse and Rails - Make Omniauth providers specs to not modify global configuration diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index 29ac0f70b30..ad0d2617294 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -75,6 +75,7 @@ class Dispatcher when 'projects:show' shortcut_handler = new ShortcutsNavigation() + new NotificationsForm() new TreeView() if $('#tree-slider').length when 'groups:activity' new Activities() diff --git a/app/assets/javascripts/notifications_form.js.coffee b/app/assets/javascripts/notifications_form.js.coffee new file mode 100644 index 00000000000..cfe8e133b66 --- /dev/null +++ b/app/assets/javascripts/notifications_form.js.coffee @@ -0,0 +1,49 @@ +class @NotificationsForm + constructor: -> + @form = $('.custom-notifications-form') + + @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) -> + $.ajax( + url: @form.attr('action') + method: 'patch' + 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/project.js.coffee b/app/assets/javascripts/project.js.coffee index 07be85a32a5..236f0899147 100644 --- a/app/assets/javascripts/project.js.coffee +++ b/app/assets/javascripts/project.js.coffee @@ -34,21 +34,26 @@ 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") + $(document) + .off 'click', '.update-notification' + .on 'click', '.update-notification', (e) -> + e.preventDefault() + notificationLevel = $(@).data 'notification-level' + label = $(@).data 'notification-title' + $('.js-notification-loading').toggleClass 'fa-bell fa-spin fa-spinner' + $('#notification_setting_level').val(notificationLevel) + $('#notification-form').submit() + + $(document) + .off 'ajax:success', '#notification-form' + .on 'ajax:success', '#notification-form', (e, data) -> + if data.saved + new Flash('Notification settings saved', 'notice') + $('.js-notification-toggle-btns') + .closest('.notification-dropdown') + .replaceWith(data.html) + 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 0e4cefc55c2..768e2ad7665 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -133,11 +133,6 @@ } } - .btn-group:not(:first-child):not(:last-child) > .btn { - border-top-right-radius: 3px; - border-bottom-right-radius: 3px; - } - form { margin-left: 10px; } @@ -607,3 +602,20 @@ pre.light-well { } } } + +.custom-notifications-form { + .is-loading { + .custom-notification-event-loading { + display: inline-block; + } + } +} + +.custom-notification-event-loading { + display: none; + margin-left: 5px; + + &.is-done { + color: $gl-text-green; + } +} diff --git a/app/controllers/projects/notification_settings_controller.rb b/app/controllers/projects/notification_settings_controller.rb index 7d81cc03c73..05fe5accc65 100644 --- a/app/controllers/projects/notification_settings_controller.rb +++ b/app/controllers/projects/notification_settings_controller.rb @@ -2,15 +2,20 @@ 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) + @notification_setting = current_user.notification_settings_for(project) + saved = @notification_setting.update_attributes(notification_setting_params) - render json: { saved: saved } + render json: { + html: view_to_html_string("projects/buttons/_notifications", locals: { project: @project, notification_setting: @notification_setting }), + saved: saved + } end private def notification_setting_params - params.require(:notification_setting).permit(:level) + allowed_fields = NotificationSetting::EMAIL_EVENTS.dup + allowed_fields << :level + params.require(:notification_setting).permit(allowed_fields) 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 f804ac171c4..5bdefcc8116 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_project_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 @@ -268,12 +276,22 @@ class NotificationService protected + # Get project/group users with CUSTOM notification level + def add_project_custom_notifications(recipients, project, action) + user_ids = [] + + user_ids += notification_settings_for(project, :custom, action) + user_ids += notification_settings_for(project.group, :custom, 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 = project_member_notification(project, :global) - users_with_group_level_global = group_member_notification(project, :global) + users_with_project_level_global = notification_settings_for(project, :global) + users_with_group_level_global = notification_settings_for(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) @@ -282,19 +300,15 @@ 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) - if notification_level - project.notification_settings.where(level: NotificationSetting.levels[notification_level]).pluck(:user_id) - else - project.notification_settings.pluck(:user_id) - end - end + def notification_settings_for(resource, notification_level = nil, action = nil) + return [] unless resource - def group_member_notification(project, notification_level) - if project.group - project.group.notification_settings.where(level: NotificationSetting.levels[notification_level]).pluck(:user_id) + if notification_level + 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 - [] + resource.notification_settings.pluck(:user_id) end end @@ -308,7 +322,7 @@ class NotificationService # 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| @@ -322,7 +336,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 = [] @@ -343,7 +357,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 @@ -428,7 +442,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 @@ -436,7 +450,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 @@ -447,7 +462,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( @@ -470,7 +485,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 @@ -478,14 +493,19 @@ 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_project_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 @@ -493,7 +513,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 @@ -523,4 +543,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/projects/buttons/_notifications.html.haml b/app/views/projects/buttons/_notifications.html.haml index a7a97181096..4c2eafbd7df 100644 --- a/app/views/projects/buttons/_notifications.html.haml +++ b/app/views/projects/buttons/_notifications.html.haml @@ -1,11 +1,21 @@ - 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) + .dropdown.notification-dropdown.pull-right + = 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 + .js-notification-toggle-btns + - if @notification_setting.custom? + .btn-group + %button.dropdown-new.btn.btn-default.notifications-btn#notifications-button{ type: "button", data: { toggle: "modal", target: "#custom-notifications-modal" } } + = icon("bell", class: "js-notification-loading") + = notification_title(@notification_setting.level) + %button.btn.btn-danger.dropdown-toggle{ data: { toggle: "dropdown", target: ".notification-dropdown" } } + %span.caret + .sr-only Toggle dropdown + - else + %button.dropdown-new.btn.btn-default.notifications-btn#notifications-button{ type: "button", data: { toggle: "dropdown", target: ".notification-dropdown" } } + = icon("bell", class: "js-notification-loading") + = notification_title(@notification_setting.level) + = icon("caret-down") + = render "shared/projects/notification_dropdown" + = content_for :scripts_body do + = render "shared/projects/custom_notifications" diff --git a/app/views/shared/projects/_custom_notifications.html.haml b/app/views/shared/projects/_custom_notifications.html.haml new file mode 100644 index 00000000000..ec4bad4ba3a --- /dev/null +++ b/app/views/shared/projects/_custom_notifications.html.haml @@ -0,0 +1,26 @@ +#custom-notifications-modal.modal.fade{ tabindex: "-1", role: "dialog", 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, url: namespace_project_notification_setting_path(@project.namespace.becomes(Namespace), @project), method: :patch, html: { class: "custom-notifications-form" } do |f| + .row + .col-lg-3 + %h4.prepend-top-0 + Notification events + .col-lg-9 + - NotificationSetting::EMAIL_EVENTS.each do |event, index| + = index + .form-group + .checkbox{ class: ("prepend-top-0" if index == 0) } + %label{ for: "events_#{event}" } + = check_box("notification_setting", event, {id: "events_#{event}", class: "js-custom-notification-event"}) + + %strong + = event.to_s.humanize + = icon("spinner spin", class: "custom-notification-event-loading") diff --git a/app/views/shared/projects/_notification_dropdown.html.haml b/app/views/shared/projects/_notification_dropdown.html.haml new file mode 100644 index 00000000000..d7bea8b69fc --- /dev/null +++ b/app/views/shared/projects/_notification_dropdown.html.haml @@ -0,0 +1,9 @@ +%ul.dropdown-menu.dropdown-menu-no-wrap.dropdown-menu-align-right.dropdown-menu-selectable.dropdown-menu-large{ role: "menu" } + - NotificationSetting.levels.each do |level| + - if level.first != "custom" + = notification_list_item(level.first, @notification_setting) + - unless @notification_setting.custom? + %li.divider + %li + %a.update-notification{ href: "#", role: "button", data: { toggle: "modal", target: "#custom-notifications-modal", notification_level: "custom", notification_title: "Custom" } } + Custom diff --git a/db/migrate/20160614301627_add_events_to_notification_settings.rb b/db/migrate/20160614301627_add_events_to_notification_settings.rb new file mode 100644 index 00000000000..609596f45e4 --- /dev/null +++ b/db/migrate/20160614301627_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..98f44a5b07b 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 - Notification is set based on events selected by the user.(Available only in project level) #### 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..1628e2d568c 100644 --- a/doc/workflow/notifications/settings.png +++ b/doc/workflow/notifications/settings.png 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/projects/notification_settings_controller_spec.rb b/spec/controllers/projects/notification_settings_controller_spec.rb index c5d17d97ec9..a726f70a64a 100644 --- a/spec/controllers/projects/notification_settings_controller_spec.rb +++ b/spec/controllers/projects/notification_settings_controller_spec.rb @@ -33,6 +33,25 @@ describe Projects::NotificationSettingsController do 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, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + notification_setting: { level: :participating, events: custom_events } + + expect(response.status).to eq 200 + end + end end context 'not authorized' do 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/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index e871a103d42..616d0cd00f7 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -46,6 +46,7 @@ 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) end describe :new_note do @@ -66,6 +67,7 @@ describe NotificationService, services: true do 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) @@ -116,6 +118,7 @@ describe NotificationService, services: true do should_email(note.noteable.author) should_email(note.noteable.assignee) should_email(@u_mentioned) + should_not_email(@u_guest_custom) should_not_email(@u_guest_watcher) should_not_email(@u_watcher) should_not_email(note.author) @@ -241,13 +244,14 @@ 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) end describe '#new_note, #perform_enqueued_jobs' do it do notification.new_note(note) - should_email(@u_guest_watcher) + should_email(@u_guest_custom) should_email(@u_committer) should_email(@u_watcher) should_not_email(@u_mentioned) @@ -288,6 +292,7 @@ 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) end describe '#new_issue' do @@ -297,6 +302,7 @@ 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_not_email(@u_mentioned) should_not_email(@u_participating) @@ -356,12 +362,15 @@ describe NotificationService, services: true do end describe '#reassigned_issue' do + before { update_custom_notification(:reassign_issue) } + 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_participant_mentioned) should_email(@subscriber) should_not_email(@unsubscriber) @@ -378,6 +387,7 @@ 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_not_email(@unsubscriber) @@ -394,6 +404,7 @@ 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_not_email(@unsubscriber) @@ -410,6 +421,7 @@ 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_not_email(@unsubscriber) @@ -425,6 +437,7 @@ 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_not_email(issue.assignee) @@ -529,6 +542,8 @@ describe NotificationService, services: true do end describe '#close_issue' do + before { update_custom_notification(:close_issue) } + it 'should sent email to issue assignee and issue author' do notification.close_issue(issue, @u_disabled) @@ -536,6 +551,7 @@ 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_participant_mentioned) should_email(@subscriber) should_email(@watcher_and_subscriber) @@ -575,6 +591,8 @@ describe NotificationService, services: true do end describe '#reopen_issue' do + before { update_custom_notification(:reopen_issue) } + it 'should send email to issue assignee and issue author' do notification.reopen_issue(issue, @u_disabled) @@ -582,6 +600,7 @@ 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_participant_mentioned) should_email(@subscriber) should_email(@watcher_and_subscriber) @@ -631,6 +650,8 @@ describe NotificationService, services: true do end describe '#new_merge_request' do + before { update_custom_notification(:new_merge_request) } + it do notification.new_merge_request(merge_request, @u_disabled) @@ -639,6 +660,7 @@ 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_not_email(@u_participating) should_not_email(@u_disabled) should_not_email(@u_lazy_participant) @@ -685,6 +707,8 @@ describe NotificationService, services: true do end describe '#reassigned_merge_request' do + before { update_custom_notification(:reassign_merge_request) } + it do notification.reassigned_merge_request(merge_request, merge_request.author) @@ -694,6 +718,7 @@ describe NotificationService, services: true do should_email(@subscriber) should_email(@watcher_and_subscriber) should_email(@u_guest_watcher) + should_email(@u_guest_custom) should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) @@ -761,12 +786,15 @@ describe NotificationService, services: true do end describe '#closed_merge_request' do + before { update_custom_notification(:close_merge_request) } + 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_participant_mentioned) should_email(@subscriber) should_email(@watcher_and_subscriber) @@ -807,6 +835,8 @@ describe NotificationService, services: true do end describe '#merged_merge_request' do + before { update_custom_notification(:merge_merge_request) } + it do notification.merge_mr(merge_request, @u_disabled) @@ -816,6 +846,7 @@ describe NotificationService, services: true do should_email(@subscriber) should_email(@watcher_and_subscriber) should_email(@u_guest_watcher) + should_email(@u_guest_custom) should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) @@ -853,6 +884,8 @@ describe NotificationService, services: true do end describe '#reopen_merge_request' do + before { update_custom_notification(:reopen_merge_request) } + it do notification.reopen_mr(merge_request, @u_disabled) @@ -862,6 +895,7 @@ describe NotificationService, services: true do should_email(@subscriber) should_email(@watcher_and_subscriber) should_email(@u_guest_watcher) + should_email(@u_guest_custom) should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) @@ -915,6 +949,7 @@ describe NotificationService, services: true do should_email(@u_participating) should_email(@u_lazy_participant) should_not_email(@u_guest_watcher) + should_not_email(@u_guest_custom) should_not_email(@u_disabled) end end @@ -935,7 +970,8 @@ describe NotificationService, services: true do # 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] @@ -955,10 +991,18 @@ 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 + + def update_custom_notification(event) + setting = @u_guest_custom.notification_settings_for(project) + setting.events[event] = true setting.save end |
