diff options
25 files changed, 361 insertions, 264 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..51faa5fcace 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,7 @@ class @NotificationsForm toggleCheckbox: (e) => $checkbox = $(e.currentTarget) $parent = $checkbox.closest('.checkbox') - + console.log($parent) @saveEvent($checkbox, $parent) showCheckboxLoadingSpinner: ($parent) -> @@ -26,11 +24,14 @@ class @NotificationsForm .removeClass 'is-done' saveEvent: ($checkbox, $parent) -> + form = $parent.parents('form:first') + console.log(form) + $.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 50c21fc0d49..8790a66ba14 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -62,22 +62,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 369bc874843..9decebe330c 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) @@ -257,12 +257,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 @@ -271,7 +279,7 @@ 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) @@ -293,11 +301,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 @@ -477,7 +495,7 @@ 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..5e81e6c96b9 --- /dev/null +++ b/app/views/notifications/buttons/_notifications.html.haml @@ -0,0 +1,25 @@ +- if notification_setting + .dropdown.notification-dropdown.pull-right + - url = notification_setting.persisted? ? notification_setting_path(notification_setting) : notification_settings_path(notification_setting) + + = 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.level != "custom" + %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") + - else + %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.btn-danger.dropdown-toggle{ data: { toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting) } } + %span.caret + .sr-only Toggle dropdown + = 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..ef65ac53302 100644 --- a/app/views/profiles/notifications/show.html.haml +++ b/app/views/profiles/notifications/show.html.haml @@ -24,12 +24,12 @@ .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" + .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 f5bc1b4e409..df725d81a39 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -32,7 +32,7 @@ .project-repo-buttons.btn-group.project-right-buttons = render "projects/buttons/download" = render 'projects/buttons/dropdown' - = render 'projects/buttons/notifications' + = 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 47a12e6f8cb..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.level != "custom" - %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") - - else - .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 - = 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/notifications/_custom_notifications.html.haml index 4e446fe2d10..f0b46c7a4c0 100644 --- a/app/views/shared/projects/_custom_notifications.html.haml +++ b/app/views/shared/notifications/_custom_notifications.html.haml @@ -1,4 +1,4 @@ -#custom-notifications-modal.modal.fade{ tabindex: "-1", role: "dialog", aria: { labelledby: "custom-notifications-title" } } +.modal.fade{ tabindex: "-1", role: "dialog", id: notifications_menu_identifier("modal", notification_setting), aria: { labelledby: "custom-notifications-title" } } .modal-dialog .modal-content .modal-header @@ -6,10 +6,11 @@ %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| - = f.hidden_field :level + = 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 @@ -20,7 +21,7 @@ .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"}) + = check_box("", event, { name: "notification_setting[#{event}]", class: "js-custom-notification-event", checked: notification_setting.events[event] }) %strong = event.to_s.humanize 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/app/views/shared/projects/_notification_dropdown.html.haml b/app/views/shared/projects/_notification_dropdown.html.haml deleted file mode 100644 index 6388f907eb3..00000000000 --- a/app/views/shared/projects/_notification_dropdown.html.haml +++ /dev/null @@ -1,9 +0,0 @@ -%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) - - if @notification_setting.level != "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/config/routes.rb b/config/routes.rb index 95fbe7dd9df..3dad841d498 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -118,10 +118,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 @@ -416,7 +424,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 @@ -648,7 +655,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/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/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 49eaeabb8f4..05cbb354cf4 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) @@ -138,6 +143,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] @@ -149,6 +158,7 @@ describe NotificationService, services: true do notification.new_note(note) should_not_email(non_member) + 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,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) @@ -289,7 +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) + update_custom_notification(:new_issue, @u_guest_custom, project) + update_custom_notification(:new_issue, @u_custom_global) end describe '#new_issue' do @@ -300,6 +316,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) @@ -345,6 +362,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_email(assignee) @@ -355,7 +373,10 @@ 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) @@ -364,6 +385,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) @@ -383,6 +405,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) @@ -400,6 +423,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) @@ -417,6 +441,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) @@ -433,6 +458,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) @@ -531,7 +557,10 @@ 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) @@ -541,6 +570,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) @@ -580,7 +610,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) @@ -590,6 +623,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) @@ -639,7 +673,11 @@ 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) @@ -650,6 +688,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) @@ -696,7 +735,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) @@ -708,6 +750,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) @@ -775,7 +818,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) @@ -784,6 +830,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) @@ -824,7 +871,10 @@ 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) @@ -835,6 +885,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) @@ -873,7 +924,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) @@ -885,6 +939,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) @@ -937,6 +992,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) @@ -953,6 +1009,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 @@ -970,6 +1027,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) @@ -989,8 +1047,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 |