summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Artur <felipefac@gmail.com>2016-06-17 00:17:20 -0300
committerFelipe Artur <felipefac@gmail.com>2016-06-17 00:17:20 -0300
commit33c61bd461d21dbb79db18e0c2424f53120e83c0 (patch)
tree2b0d60fe8b8a9c132c8c25f63131d333821f876e
parent24f5f071dcce529a3fbbc887ebce7bfc56353361 (diff)
parentf82ab42d0534950c1ceb458e0152f329df80ae9d (diff)
downloadgitlab-ce-33c61bd461d21dbb79db18e0c2424f53120e83c0.tar.gz
Re-use notifications dropdown/modal for user profile
-rw-r--r--app/assets/javascripts/dispatcher.js.coffee3
-rw-r--r--app/assets/javascripts/notifications_dropdown.js.coffee21
-rw-r--r--app/assets/javascripts/notifications_form.js.coffee12
-rw-r--r--app/assets/javascripts/profile.js.coffee4
-rw-r--r--app/assets/javascripts/project.js.coffee21
-rw-r--r--app/controllers/groups/notification_settings_controller.rb16
-rw-r--r--app/controllers/notification_settings_controller.rb34
-rw-r--r--app/controllers/profiles/notifications_controller.rb18
-rw-r--r--app/controllers/projects/notification_settings_controller.rb21
-rw-r--r--app/helpers/notifications_helper.rb24
-rw-r--r--app/services/notification_service.rb32
-rw-r--r--app/views/notifications/buttons/_notifications.html.haml24
-rw-r--r--app/views/profiles/notifications/_group_settings.html.haml3
-rw-r--r--app/views/profiles/notifications/_project_settings.html.haml3
-rw-r--r--app/views/profiles/notifications/show.html.haml13
-rw-r--r--app/views/projects/_home_panel.html.haml11
-rw-r--r--app/views/projects/buttons/_notifications.html.haml21
-rw-r--r--app/views/shared/notifications/_custom_notifications.html.haml28
-rw-r--r--app/views/shared/notifications/_notification_dropdown.html.haml12
-rw-r--r--config/routes.rb10
-rw-r--r--doc/workflow/notifications/settings.pngbin88149 -> 90986 bytes
-rw-r--r--lib/api/entities.rb3
-rw-r--r--spec/controllers/groups/notification_settings_controller_spec.rb32
-rw-r--r--spec/controllers/notification_settings_controller_spec.rb125
-rw-r--r--spec/controllers/projects/notification_settings_controller_spec.rb71
-rw-r--r--spec/requests/api/projects_spec.rb29
-rw-r--r--spec/services/notification_service_spec.rb90
27 files changed, 418 insertions, 263 deletions
diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee
index ad0d2617294..404101710b9 100644
--- a/app/assets/javascripts/dispatcher.js.coffee
+++ b/app/assets/javascripts/dispatcher.js.coffee
@@ -126,6 +126,8 @@ class Dispatcher
shortcut_handler = new ShortcutsDashboardNavigation()
when 'profiles'
new Profile()
+ new NotificationsForm()
+ new NotificationsDropdown()
when 'projects'
new Project()
new ProjectAvatar()
@@ -139,6 +141,7 @@ class Dispatcher
new ProjectNew()
when 'show'
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..15daf027c0a
--- /dev/null
+++ b/app/assets/javascripts/notifications_dropdown.js.coffee
@@ -0,0 +1,21 @@
+class @NotificationsDropdown
+ $ ->
+ $(document)
+ .off 'click', '.update-notification'
+ .on 'click', '.update-notification', (e) ->
+ e.preventDefault()
+ notificationLevel = $(@).data 'notification-level'
+ label = $(@).data 'notification-title'
+ form = $(this).parents('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
index cfe8e133b66..3432428702a 100644
--- a/app/assets/javascripts/notifications_form.js.coffee
+++ b/app/assets/javascripts/notifications_form.js.coffee
@@ -1,7 +1,5 @@
class @NotificationsForm
constructor: ->
- @form = $('.custom-notifications-form')
-
@removeEventListeners()
@initEventListeners()
@@ -14,7 +12,6 @@ class @NotificationsForm
toggleCheckbox: (e) =>
$checkbox = $(e.currentTarget)
$parent = $checkbox.closest('.checkbox')
-
@saveEvent($checkbox, $parent)
showCheckboxLoadingSpinner: ($parent) ->
@@ -26,11 +23,14 @@ class @NotificationsForm
.removeClass 'is-done'
saveEvent: ($checkbox, $parent) ->
+ form = $parent.parents('form:first')
+
$.ajax(
- url: @form.attr('action')
- method: 'patch'
+ url: form.attr('action')
+ method: form.attr('method')
dataType: 'json'
- data: @form.serialize()
+ data: form.serialize()
+
beforeSend: =>
@showCheckboxLoadingSpinner($parent)
).done (data) ->
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 236f0899147..d12bad97a05 100644
--- a/app/assets/javascripts/project.js.coffee
+++ b/app/assets/javascripts/project.js.coffee
@@ -34,27 +34,6 @@ class @Project
$(@).parents('.no-password-message').remove()
e.preventDefault()
- $(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/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..5d425ad8420
--- /dev/null
+++ b/app/controllers/notification_settings_controller.rb
@@ -0,0 +1,34 @@
+class NotificationSettingsController < ApplicationController
+ before_action :authenticate_user!
+
+ def create
+ project = current_user.projects.find(params[:project][:id])
+
+ @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("notifications/buttons/_notifications", 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 05fe5accc65..00000000000
--- a/app/controllers/projects/notification_settings_controller.rb
+++ /dev/null
@@ -1,21 +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: {
- html: view_to_html_string("projects/buttons/_notifications", locals: { project: @project, notification_setting: @notification_setting }),
- saved: saved
- }
- end
-
- private
-
- 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/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb
index 70d56e4ac08..77783cd7640 100644
--- a/app/helpers/notifications_helper.rb
+++ b/app/helpers/notifications_helper.rb
@@ -64,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/services/notification_service.rb b/app/services/notification_service.rb
index 5bdefcc8116..e455101f526 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -159,7 +159,7 @@ class NotificationService
recipients = add_project_watchers(recipients, note.project)
# Merge project with custom notification
- recipients = add_project_custom_notifications(recipients, note.project, :new_note)
+ 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)
@@ -277,12 +277,20 @@ class NotificationService
protected
# Get project/group users with CUSTOM notification level
- def add_project_custom_notifications(recipients, project, action)
+ 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
@@ -291,7 +299,8 @@ class NotificationService
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, :global)
+ users_with_group_level_global = notification_settings_for(project.group, :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)
@@ -313,11 +322,21 @@ class NotificationService
end
def users_with_global_level_watch(ids)
+ settings_with_global_level_of(:watch, ids).pluck(:user_id)
+ end
+
+ 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
@@ -497,7 +516,8 @@ class NotificationService
recipients = target.participants(current_user)
recipients = add_project_watchers(recipients, project)
- recipients = add_project_custom_notifications(recipients, project, custom_action)
+
+ recipients = add_custom_notifications(recipients, project, custom_action)
recipients = reject_mention_users(recipients, project)
recipients = recipients.uniq
diff --git a/app/views/notifications/buttons/_notifications.html.haml b/app/views/notifications/buttons/_notifications.html.haml
new file mode 100644
index 00000000000..03740f2587b
--- /dev/null
+++ b/app/views/notifications/buttons/_notifications.html.haml
@@ -0,0 +1,24 @@
+- if notification_setting
+ .dropdown.notification-dropdown.pull-right
+ = form_for notification_setting, remote: true, html: { class: "inline", id: "notification-form" } do |f|
+ = hidden_setting_source_input(notification_setting)
+ = f.hidden_field :level, class: "notification_setting_level"
+ .js-notification-toggle-btns
+ .btn-group
+ - 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
+
+ = content_for :scripts_body do
+ = render "shared/notifications/custom_notifications", notification_setting: notification_setting
diff --git a/app/views/profiles/notifications/_group_settings.html.haml b/app/views/profiles/notifications/_group_settings.html.haml
index f0cf82afe83..04becd5d860 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 'notifications/buttons/notifications', 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..664a17418db 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 'notifications/buttons/notifications', notification_setting: setting
diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml
index f2659ac14b5..646dd244596 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 'notifications/buttons/notifications', notification_setting: @global_notification_setting
+
+ .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..df725d81a39 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/app/views/projects/_home_panel.html.haml
@@ -29,13 +29,10 @@
.project-clone-holder
= render "shared/clone_panel"
- .project-repo-buttons.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'
+ .project-repo-buttons.btn-group.project-right-buttons
+ = render "projects/buttons/download"
+ = render 'projects/buttons/dropdown'
+ = render 'notifications/buttons/notifications', 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 4c2eafbd7df..00000000000
--- a/app/views/projects/buttons/_notifications.html.haml
+++ /dev/null
@@ -1,21 +0,0 @@
-- if @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/notifications/_custom_notifications.html.haml b/app/views/shared/notifications/_custom_notifications.html.haml
new file mode 100644
index 00000000000..f0b46c7a4c0
--- /dev/null
+++ b/app/views/shared/notifications/_custom_notifications.html.haml
@@ -0,0 +1,28 @@
+.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-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("", event, { name: "notification_setting[#{event}]", 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..e6e04b98c82
--- /dev/null
+++ b/app/views/shared/notifications/_notification_dropdown.html.haml
@@ -0,0 +1,12 @@
+%ul.dropdown-menu.dropdown-menu-no-wrap.dropdown-menu-align-right.dropdown-menu-selectable.dropdown-menu-large{ role: "menu", class: notifications_menu_identifier("dropdown", notification_setting) }
+ - NotificationSetting.levels.each_key do |level|
+ - next if level == "custom"
+ - next if level == "global" && notification_setting.source.nil?
+
+ = notification_list_item(level, notification_setting)
+
+ - unless notification_setting.custom?
+ %li.divider
+ %li
+ %a.update-notification{ href: "#", role: "button", data: { toggle: "modal", target: "#" + notifications_menu_identifier("modal", notification_setting), notification_level: "custom", notification_title: "Custom" } }
+ Custom
diff --git a/config/routes.rb b/config/routes.rb
index d52cbb22428..207298d7963 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
@@ -421,7 +429,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
@@ -653,7 +660,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/doc/workflow/notifications/settings.png b/doc/workflow/notifications/settings.png
index 1628e2d568c..7c6857aad1a 100644
--- a/doc/workflow/notifications/settings.png
+++ b/doc/workflow/notifications/settings.png
Binary files differ
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index cc29c7ef428..7818fc290f8 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -272,7 +272,8 @@ module API
expose :access_level
expose :notification_level do |member, options|
if member.notification_setting
- NotificationSetting.levels[member.notification_setting.level]
+ setting = member.notification_setting
+ { level: NotificationSetting.levels[setting.level], events: setting.events }
end
end
end
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 a726f70a64a..00000000000
--- a/spec/controllers/projects/notification_settings_controller_spec.rb
+++ /dev/null
@@ -1,71 +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
-
- 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
- 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/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index f167813e07d..2a2ac8710ee 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)
@@ -437,10 +438,18 @@ describe API::API, api: true do
to eq(Gitlab::Access::MASTER)
expect(json_response.first['permissions']['group_access']).to be_nil
end
+
+ it 'contains notification level information' do
+ get api("/projects", user)
+
+ expect(response.status).to eq(200)
+ expect(json_response.first['permissions']['project_access']['notification_level']['level']).to eq(NotificationSetting.levels[:global])
+ expect(json_response.first['permissions']['project_access']['notification_level'].keys).to include('events')
+ end
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 +461,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)
@@ -462,6 +473,14 @@ describe API::API, api: true do
expect(json_response['permissions']['group_access']['access_level']).
to eq(Gitlab::Access::OWNER)
end
+
+ it 'shows notification level information' do
+ get api("/projects/#{project2.id}", user)
+
+ expect(response.status).to eq(200)
+ expect(json_response['permissions']['group_access']['notification_level']['level']).to eq(NotificationSetting.levels[:global])
+ expect(json_response['permissions']['group_access']['notification_level'].keys).to include('events')
+ end
end
end
end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 616d0cd00f7..c4edb8c3fd0 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -46,7 +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)
+ update_custom_notification(:new_note, @u_guest_custom, project)
+ update_custom_notification(:new_note, @u_custom_global)
end
describe :new_note do
@@ -54,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
@@ -63,6 +64,7 @@ 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)
@@ -105,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
@@ -118,6 +122,7 @@ 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)
@@ -139,6 +144,10 @@ 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") }
+
+ before do
+ end
it 'filters out users that can not read the issue' do
project.team << [member, :developer]
@@ -152,6 +161,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)
@@ -226,6 +236,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)
@@ -244,13 +257,15 @@ 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)
+ 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)
@@ -292,7 +307,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)
+ update_custom_notification(:new_issue, @u_guest_custom, project)
+ update_custom_notification(:new_issue, @u_custom_global)
end
describe '#new_issue' do
@@ -303,6 +319,7 @@ describe NotificationService, services: true do
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)
@@ -351,6 +368,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)
@@ -362,7 +380,11 @@ describe NotificationService, services: true do
end
describe '#reassigned_issue' do
- before { update_custom_notification(:reassign_issue) }
+
+ 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)
@@ -371,6 +393,7 @@ describe NotificationService, services: true do
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)
@@ -390,6 +413,7 @@ describe NotificationService, services: true do
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)
@@ -407,6 +431,7 @@ describe NotificationService, services: true do
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)
@@ -424,6 +449,7 @@ describe NotificationService, services: true do
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)
@@ -440,6 +466,7 @@ describe NotificationService, services: true do
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)
@@ -542,7 +569,11 @@ describe NotificationService, services: true do
end
describe '#close_issue' do
- before { update_custom_notification(:close_issue) }
+
+ 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)
@@ -552,6 +583,7 @@ describe NotificationService, services: true do
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)
@@ -591,7 +623,10 @@ describe NotificationService, services: true do
end
describe '#reopen_issue' do
- before { update_custom_notification(:reopen_issue) }
+ 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)
@@ -601,6 +636,7 @@ describe NotificationService, services: true do
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)
@@ -650,7 +686,10 @@ describe NotificationService, services: true do
end
describe '#new_merge_request' do
- before { update_custom_notification(:new_merge_request) }
+ 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)
@@ -661,6 +700,7 @@ describe NotificationService, services: true do
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)
@@ -707,7 +747,10 @@ describe NotificationService, services: true do
end
describe '#reassigned_merge_request' do
- before { update_custom_notification(:reassign_merge_request) }
+ 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)
@@ -719,6 +762,7 @@ describe NotificationService, services: true do
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)
@@ -786,7 +830,10 @@ describe NotificationService, services: true do
end
describe '#closed_merge_request' do
- before { update_custom_notification(:close_merge_request) }
+ 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)
@@ -795,6 +842,7 @@ describe NotificationService, services: true do
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)
@@ -835,7 +883,11 @@ describe NotificationService, services: true do
end
describe '#merged_merge_request' do
- before { update_custom_notification(:merge_merge_request) }
+
+ 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)
@@ -846,6 +898,7 @@ 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)
@@ -884,7 +937,10 @@ describe NotificationService, services: true do
end
describe '#reopen_merge_request' do
- before { update_custom_notification(:reopen_merge_request) }
+ 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)
@@ -896,6 +952,7 @@ describe NotificationService, services: true do
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)
@@ -948,6 +1005,7 @@ 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)
@@ -964,6 +1022,7 @@ 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
@@ -981,6 +1040,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)
@@ -1000,8 +1060,10 @@ describe NotificationService, services: true do
user
end
- def update_custom_notification(event)
- setting = @u_guest_custom.notification_settings_for(project)
+ # 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