diff --git a/CHANGELOG b/CHANGELOG
index 382318a203cc5829f90bbf265d510f79be2a4bfb..07274ab5c1de23f8c5baef4ff3583ce7939ae92d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -31,6 +31,7 @@ v 8.7.0 (unreleased)
- Hide `Create a group` help block when creating a new project in a group
- Implement 'TODOs View' as an option for dashboard preferences !3379 (Elias W.)
- Gracefully handle notes on deleted commits in merge requests (Stan Hu)
+ - Decouple membership and notifications
- Fix creation of merge requests for orphaned branches (Stan Hu)
- API: Ability to retrieve a single tag (Robert Schilling)
- Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla)
diff --git a/app/assets/javascripts/profile.js.coffee b/app/assets/javascripts/profile.js.coffee
index ae87c6c4e40bba1dc50e6e1bba288e7504fac0fd..f4a2562885dfa5d4f7e69430f8ca3652c50ceb23 100644
--- a/app/assets/javascripts/profile.js.coffee
+++ b/app/assets/javascripts/profile.js.coffee
@@ -18,8 +18,11 @@ class @Profile
$(this).find('.btn-save').enable()
$(this).find('.loading-gif').hide()
- $('.update-notifications').on 'ajax:complete', ->
- $(this).find('.btn-save').enable()
+ $('.update-notifications').on 'ajax:success', (e, data) ->
+ if data.saved
+ new Flash("Notification settings saved", "notice")
+ else
+ new Flash("Failed to save new settings", "alert")
@bindEvents()
diff --git a/app/assets/javascripts/project.js.coffee b/app/assets/javascripts/project.js.coffee
index 87d313ed67c271e7549758c765535fc0ea607b18..07be85a32a5cfa43b813626512de3ab8646cf77c 100644
--- a/app/assets/javascripts/project.js.coffee
+++ b/app/assets/javascripts/project.js.coffee
@@ -37,19 +37,20 @@ class @Project
$('.update-notification').on 'click', (e) ->
e.preventDefault()
notification_level = $(@).data 'notification-level'
- $('#notification_level').val(notification_level)
+ label = $(@).data 'notification-title'
+ $('#notification_setting_level').val(notification_level)
$('#notification-form').submit()
- label = null
- switch notification_level
- when 0 then label = ' Disabled '
- when 1 then label = ' Participating '
- when 2 then label = ' Watching '
- when 3 then label = ' Global '
- when 4 then label = ' On Mention '
$('#notifications-button').empty().append("<i class='fa fa-bell'></i>" + label + "<i class='fa fa-angle-down'></i>")
$(@).parents('ul').find('li.active').removeClass 'active'
$(@).parent().addClass 'active'
+ $('#notification-form').on 'ajax:success', (e, data) ->
+ if data.saved
+ new Flash("Notification settings saved", "notice")
+ else
+ new Flash("Failed to save new settings", "alert")
+
+
@projectSelectDropdown()
projectSelectDropdown: ->
diff --git a/app/controllers/groups/notification_settings_controller.rb b/app/controllers/groups/notification_settings_controller.rb
new file mode 100644
index 0000000000000000000000000000000000000000..de13b16ccf210cb65f1d9dbd10be9cc360cd880b
--- /dev/null
+++ b/app/controllers/groups/notification_settings_controller.rb
@@ -0,0 +1,16 @@
+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/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb
index 1fd1d6882dfee07654367a6ad2272a073e319397..18ee55c839a649f11c787829119e0c637d40351c 100644
--- a/app/controllers/profiles/notifications_controller.rb
+++ b/app/controllers/profiles/notifications_controller.rb
@@ -1,39 +1,18 @@
class Profiles::NotificationsController < Profiles::ApplicationController
def show
@user = current_user
- @notification = current_user.notification
- @project_members = current_user.project_members
- @group_members = current_user.group_members
+ @group_notifications = current_user.notification_settings.for_groups
+ @project_notifications = current_user.notification_settings.for_projects
end
def update
- type = params[:notification_type]
-
- @saved = if type == 'global'
- current_user.update_attributes(user_params)
- elsif type == 'group'
- group_member = current_user.group_members.find(params[:notification_id])
- group_member.notification_level = params[:notification_level]
- group_member.save
- else
- project_member = current_user.project_members.find(params[:notification_id])
- project_member.notification_level = params[:notification_level]
- project_member.save
- end
-
- respond_to do |format|
- format.html do
- if @saved
- flash[:notice] = "Notification settings saved"
- else
- flash[:alert] = "Failed to save new settings"
- end
-
- redirect_back_or_default(default: profile_notifications_path)
- end
-
- format.js
+ if current_user.update_attributes(user_params)
+ flash[:notice] = "Notification settings saved"
+ else
+ flash[:alert] = "Failed to save new settings"
end
+
+ redirect_back_or_default(default: profile_notifications_path)
end
def user_params
diff --git a/app/controllers/projects/notification_settings_controller.rb b/app/controllers/projects/notification_settings_controller.rb
new file mode 100644
index 0000000000000000000000000000000000000000..7d81cc03c73f6a91bc322ca5221654ed163a03cb
--- /dev/null
+++ b/app/controllers/projects/notification_settings_controller.rb
@@ -0,0 +1,16 @@
+class Projects::NotificationSettingsController < Projects::ApplicationController
+ before_action :authenticate_user!
+
+ def update
+ notification_setting = current_user.notification_settings_for(project)
+ saved = notification_setting.update_attributes(notification_setting_params)
+
+ render json: { saved: saved }
+ end
+
+ private
+
+ def notification_setting_params
+ params.require(:notification_setting).permit(:level)
+ end
+end
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 3cc37e59855d6476aca1a63abb2063fa65bda875..3768efe142aa2adf1b614f10feaaf64c972813eb 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -101,14 +101,18 @@ class ProjectsController < Projects::ApplicationController
respond_to do |format|
format.html do
+ if current_user
+ @membership = @project.team.find_member(current_user.id)
+
+ if @membership
+ @notification_setting = current_user.notification_settings_for(@project)
+ end
+ end
+
if @project.repository_exists?
if @project.empty_repo?
render 'projects/empty'
else
- if current_user
- @membership = @project.team.find_member(current_user.id)
- end
-
render :show
end
else
diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb
index 499c655d2bf21991eb9955aff39ca948665cee64..54ab9179efc00f121ad3d920b800392e39bc0ccc 100644
--- a/app/helpers/notifications_helper.rb
+++ b/app/helpers/notifications_helper.rb
@@ -1,48 +1,48 @@
module NotificationsHelper
include IconsHelper
- def notification_icon(notification)
- if notification.disabled?
- icon('volume-off', class: 'ns-mute')
- elsif notification.participating?
- icon('volume-down', class: 'ns-part')
- elsif notification.watch?
- icon('volume-up', class: 'ns-watch')
- else
- icon('circle-o', class: 'ns-default')
+ def notification_icon_class(level)
+ case level.to_sym
+ when :disabled
+ 'microphone-slash'
+ when :participating
+ 'volume-up'
+ when :watch
+ 'eye'
+ when :mention
+ 'at'
+ when :global
+ 'globe'
end
end
- def notification_list_item(notification_level, user_membership)
- case notification_level
- when Notification::N_DISABLED
- update_notification_link(Notification::N_DISABLED, user_membership, 'Disabled', 'microphone-slash')
- when Notification::N_PARTICIPATING
- update_notification_link(Notification::N_PARTICIPATING, user_membership, 'Participate', 'volume-up')
- when Notification::N_WATCH
- update_notification_link(Notification::N_WATCH, user_membership, 'Watch', 'eye')
- when Notification::N_MENTION
- update_notification_link(Notification::N_MENTION, user_membership, 'On mention', 'at')
- when Notification::N_GLOBAL
- update_notification_link(Notification::N_GLOBAL, user_membership, 'Global', 'globe')
- else
- # do nothing
- end
+ def notification_icon(level, text = nil)
+ icon("#{notification_icon_class(level)} fw", text: text)
end
- def update_notification_link(notification_level, user_membership, title, icon)
- content_tag(:li, class: active_level_for(user_membership, notification_level)) do
- link_to '#', class: 'update-notification', data: { notification_level: notification_level } do
- icon("#{icon} fw", text: title)
- end
+ def notification_title(level)
+ case level.to_sym
+ when :participating
+ 'Participate'
+ when :mention
+ 'On mention'
+ else
+ level.to_s.titlecase
end
end
- def notification_label(user_membership)
- Notification.new(user_membership).to_s
- end
+ def notification_list_item(level, setting)
+ title = notification_title(level)
+
+ data = {
+ notification_level: level,
+ notification_title: title
+ }
- def active_level_for(user_membership, level)
- 'active' if user_membership.notification_level == level
+ content_tag(:li, class: ('active' if setting.level == level)) do
+ link_to '#', class: 'update-notification', data: data do
+ notification_icon(level, title)
+ end
+ end
end
end
diff --git a/app/models/concerns/notifiable.rb b/app/models/concerns/notifiable.rb
deleted file mode 100644
index d7dcd97911dd4825987855a38d5723e6550180d4..0000000000000000000000000000000000000000
--- a/app/models/concerns/notifiable.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-# == Notifiable concern
-#
-# Contains notification functionality
-#
-module Notifiable
- extend ActiveSupport::Concern
-
- included do
- validates :notification_level, inclusion: { in: Notification.project_notification_levels }, presence: true
- end
-
- def notification
- @notification ||= Notification.new(self)
- end
-end
diff --git a/app/models/group.rb b/app/models/group.rb
index b332601c59b78bc70e1904aa644fb626034c05f0..9a04ac70d350a07d8fb3bf056e16da88240dc82f 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -27,6 +27,7 @@ class Group < Namespace
has_many :users, through: :group_members
has_many :project_group_links, dependent: :destroy
has_many :shared_projects, through: :project_group_links, source: :project
+ has_many :notification_settings, dependent: :destroy, as: :source
validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
validate :visibility_level_allowed_by_projects
diff --git a/app/models/member.rb b/app/models/member.rb
index ca08007b7eb9ef032f673589dd348a9aa2f43e87..60efafef211791648a2420a7988a359873f0ed58 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -19,7 +19,6 @@
class Member < ActiveRecord::Base
include Sortable
- include Notifiable
include Gitlab::Access
attr_accessor :raw_invite_token
@@ -56,12 +55,15 @@ class Member < ActiveRecord::Base
before_validation :generate_invite_token, on: :create, if: -> (member) { member.invite_email.present? }
after_create :send_invite, if: :invite?
+ after_create :create_notification_setting, unless: :invite?
after_create :post_create_hook, unless: :invite?
after_update :post_update_hook, unless: :invite?
after_destroy :post_destroy_hook, unless: :invite?
delegate :name, :username, :email, to: :user, prefix: true
+ default_value_for :notification_level, NotificationSetting.levels[:global]
+
class << self
def find_by_invite_token(invite_token)
invite_token = Devise.token_generator.digest(self, :invite_token, invite_token)
@@ -160,6 +162,14 @@ class Member < ActiveRecord::Base
send_invite
end
+ def create_notification_setting
+ user.notification_settings.find_or_create_for(source)
+ end
+
+ def notification_setting
+ @notification_setting ||= user.notification_settings_for(source)
+ end
+
private
def send_invite
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index 65d2ea00570db9e0a440ea872c382f67f0504cd2..9fb474a1a93d8fd54dc278d789cfa13485c1c8af 100644
--- a/app/models/members/group_member.rb
+++ b/app/models/members/group_member.rb
@@ -24,7 +24,6 @@ class GroupMember < Member
# Make sure group member points only to group as it source
default_value_for :source_type, SOURCE_TYPE
- default_value_for :notification_level, Notification::N_GLOBAL
validates_format_of :source_type, with: /\ANamespace\z/
default_scope { where(source_type: SOURCE_TYPE) }
diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb
index 560d1690e1432de9698378c9515eddccc60d81a9..07ddb02ae9dd6bba8da9b3ea4947d222ca60bc83 100644
--- a/app/models/members/project_member.rb
+++ b/app/models/members/project_member.rb
@@ -27,7 +27,6 @@ class ProjectMember < Member
# Make sure project member points only to project as it source
default_value_for :source_type, SOURCE_TYPE
- default_value_for :notification_level, Notification::N_GLOBAL
validates_format_of :source_type, with: /\AProject\z/
default_scope { where(source_type: SOURCE_TYPE) }
diff --git a/app/models/notification.rb b/app/models/notification.rb
deleted file mode 100644
index 171b8df45c2fd05bed7daf0c0eff06e375bb2ec0..0000000000000000000000000000000000000000
--- a/app/models/notification.rb
+++ /dev/null
@@ -1,77 +0,0 @@
-class Notification
- #
- # Notification levels
- #
- N_DISABLED = 0
- N_PARTICIPATING = 1
- N_WATCH = 2
- N_GLOBAL = 3
- N_MENTION = 4
-
- attr_accessor :target
-
- class << self
- def notification_levels
- [N_DISABLED, N_MENTION, N_PARTICIPATING, N_WATCH]
- end
-
- def options_with_labels
- {
- disabled: N_DISABLED,
- participating: N_PARTICIPATING,
- watch: N_WATCH,
- mention: N_MENTION,
- global: N_GLOBAL
- }
- end
-
- def project_notification_levels
- [N_DISABLED, N_MENTION, N_PARTICIPATING, N_WATCH, N_GLOBAL]
- end
- end
-
- def initialize(target)
- @target = target
- end
-
- def disabled?
- target.notification_level == N_DISABLED
- end
-
- def participating?
- target.notification_level == N_PARTICIPATING
- end
-
- def watch?
- target.notification_level == N_WATCH
- end
-
- def global?
- target.notification_level == N_GLOBAL
- end
-
- def mention?
- target.notification_level == N_MENTION
- end
-
- def level
- target.notification_level
- end
-
- def to_s
- case level
- when N_DISABLED
- 'Disabled'
- when N_PARTICIPATING
- 'Participating'
- when N_WATCH
- 'Watching'
- when N_MENTION
- 'On mention'
- when N_GLOBAL
- 'Global'
- else
- # do nothing
- end
- end
-end
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
new file mode 100644
index 0000000000000000000000000000000000000000..5001738f411f43a05c678d7edf6d0fd85bbda8d6
--- /dev/null
+++ b/app/models/notification_setting.rb
@@ -0,0 +1,28 @@
+class NotificationSetting < ActiveRecord::Base
+ enum level: { disabled: 0, participating: 1, watch: 2, global: 3, mention: 4 }
+
+ default_value_for :level, NotificationSetting.levels[:global]
+
+ belongs_to :user
+ belongs_to :source, polymorphic: true
+
+ validates :user, presence: true
+ validates :source, presence: true
+ validates :level, presence: true
+ validates :user_id, uniqueness: { scope: [:source_type, :source_id],
+ message: "already exists in source",
+ allow_nil: true }
+
+ scope :for_groups, -> { where(source_type: 'Namespace') }
+ scope :for_projects, -> { where(source_type: 'Project') }
+
+ def self.find_or_create_for(source)
+ setting = find_or_initialize_by(source: source)
+
+ unless setting.persisted?
+ setting.save
+ end
+
+ setting
+ end
+end
diff --git a/app/models/project.rb b/app/models/project.rb
index 3e1f04b41585627fd4e7e3ac0206c4587a905204..c3d427059022bc37d901538a0d2f6b38158a9d18 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -154,6 +154,7 @@ class Project < ActiveRecord::Base
has_many :project_group_links, dependent: :destroy
has_many :invited_groups, through: :project_group_links, source: :group
has_many :todos, dependent: :destroy
+ has_many :notification_settings, dependent: :destroy, as: :source
has_one :import_data, dependent: :destroy, class_name: "ProjectImportData"
diff --git a/app/models/user.rb b/app/models/user.rb
index 2b0bee2099f2f5110bb1b08be29fca08e298e4da..031315debd75f7c0e2e3ae9a75fb4f926e28dfa4 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -143,6 +143,7 @@ class User < ActiveRecord::Base
has_many :spam_logs, dependent: :destroy
has_many :builds, dependent: :nullify, class_name: 'Ci::Build'
has_many :todos, dependent: :destroy
+ has_many :notification_settings, dependent: :destroy
#
# Validations
@@ -157,7 +158,7 @@ class User < ActiveRecord::Base
presence: true,
uniqueness: { case_sensitive: false }
- validates :notification_level, inclusion: { in: Notification.notification_levels }, presence: true
+ validates :notification_level, presence: true
validate :namespace_uniq, if: ->(user) { user.username_changed? }
validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
validate :unique_email, if: ->(user) { user.email_changed? }
@@ -190,6 +191,13 @@ class User < ActiveRecord::Base
# Note: When adding an option, it MUST go on the end of the array.
enum project_view: [:readme, :activity, :files]
+ # Notification level
+ # Note: When adding an option, it MUST go on the end of the array.
+ #
+ # TODO: Add '_prefix: :notification' to enum when update to Rails 5. https://github.com/rails/rails/pull/19813
+ # Because user.notification_disabled? is much better than user.disabled?
+ enum notification_level: [:disabled, :participating, :watch, :global, :mention]
+
alias_attribute :private_token, :authentication_token
delegate :path, to: :namespace, allow_nil: true, prefix: true
@@ -349,10 +357,6 @@ class User < ActiveRecord::Base
"#{self.class.reference_prefix}#{username}"
end
- def notification
- @notification ||= Notification.new(self)
- end
-
def generate_password
if self.force_random_password
self.password = self.password_confirmation = Devise.friendly_token.first(8)
@@ -827,6 +831,10 @@ class User < ActiveRecord::Base
end
end
+ def notification_settings_for(source)
+ notification_settings.find_or_initialize_by(source: source)
+ end
+
private
def projects_union
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index eff0d96f93dd07e0d1ec4336f952d957768b7274..42ec1ac9e1a15ed2b6a76c0cf001fe302e5ec492 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -253,8 +253,8 @@ class NotificationService
def project_watchers(project)
project_members = project_member_notification(project)
- users_with_project_level_global = project_member_notification(project, Notification::N_GLOBAL)
- users_with_group_level_global = group_member_notification(project, Notification::N_GLOBAL)
+ users_with_project_level_global = project_member_notification(project, :global)
+ users_with_group_level_global = group_member_notification(project, :global)
users = users_with_global_level_watch([users_with_project_level_global, users_with_group_level_global].flatten.uniq)
users_with_project_setting = select_project_member_setting(project, users_with_project_level_global, users)
@@ -264,18 +264,16 @@ class NotificationService
end
def project_member_notification(project, notification_level=nil)
- project_members = project.project_members
-
if notification_level
- project_members.where(notification_level: notification_level).pluck(:user_id)
+ project.notification_settings.where(level: NotificationSetting.levels[notification_level]).pluck(:user_id)
else
- project_members.pluck(:user_id)
+ project.notification_settings.pluck(:user_id)
end
end
def group_member_notification(project, notification_level)
if project.group
- project.group.group_members.where(notification_level: notification_level).pluck(:user_id)
+ project.group.notification_settings.where(level: NotificationSetting.levels[notification_level]).pluck(:user_id)
else
[]
end
@@ -284,13 +282,13 @@ class NotificationService
def users_with_global_level_watch(ids)
User.where(
id: ids,
- notification_level: Notification::N_WATCH
+ notification_level: NotificationSetting.levels[:watch]
).pluck(:id)
end
# Build a list of users based on project notifcation settings
def select_project_member_setting(project, global_setting, users_global_level_watch)
- users = project_member_notification(project, Notification::N_WATCH)
+ users = project_member_notification(project, :watch)
# If project setting is global, add to watch list if global setting is watch
global_setting.each do |user_id|
@@ -304,7 +302,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, Notification::N_WATCH)
+ uids = group_member_notification(project, :watch)
# Group setting is watch, add to users list if user is not project member
users = []
@@ -331,40 +329,46 @@ class NotificationService
# Remove users with disabled notifications from array
# Also remove duplications and nil recipients
def reject_muted_users(users, project = nil)
- reject_users(users, :disabled?, project)
+ reject_users(users, :disabled, project)
end
# Remove users with notification level 'Mentioned'
def reject_mention_users(users, project = nil)
- reject_users(users, :mention?, project)
+ reject_users(users, :mention, project)
end
- # Reject users which method_name from notification object returns true.
+ # Reject users which has certain notification level
#
# Example:
- # reject_users(users, :watch?, project)
+ # reject_users(users, :watch, project)
#
- def reject_users(users, method_name, project = nil)
+ def reject_users(users, level, project = nil)
+ level = level.to_s
+
+ unless NotificationSetting.levels.keys.include?(level)
+ raise 'Invalid notification level'
+ end
+
users = users.to_a.compact.uniq
users = users.reject(&:blocked?)
users.reject do |user|
- next user.notification.send(method_name) unless project
+ next user.notification_level == level unless project
- member = project.project_members.find_by(user_id: user.id)
+ setting = user.notification_settings_for(project)
- if !member && project.group
- member = project.group.group_members.find_by(user_id: user.id)
+ if !setting && project.group
+ setting = user.notification_settings_for(project.group)
end
- # reject users who globally set mention notification and has no membership
- next user.notification.send(method_name) unless member
+ # reject users who globally set mention notification and has no setting per project/group
+ next user.notification_level == level unless setting
# reject users who set mention notification in project
- next true if member.notification.send(method_name)
+ next true if setting.level == level
- # reject users who have N_MENTION in project and disabled in global settings
- member.notification.global? && user.notification.send(method_name)
+ # reject users who have mention level in project and disabled in global settings
+ setting.global? && user.notification_level == level
end
end
diff --git a/app/views/profiles/notifications/_group_settings.html.haml b/app/views/profiles/notifications/_group_settings.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..89ae7ffda2bb7eb0db802cc90aeb7f468935eca3
--- /dev/null
+++ b/app/views/profiles/notifications/_group_settings.html.haml
@@ -0,0 +1,13 @@
+%li.notification-list-item
+ %span.notification.fa.fa-holder.append-right-5
+ - if setting.global?
+ = notification_icon(current_user.notification_level)
+ - else
+ = notification_icon(setting.level)
+
+ %span.str-truncated
+ = 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'
diff --git a/app/views/profiles/notifications/_project_settings.html.haml b/app/views/profiles/notifications/_project_settings.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..17c097154da66b771198cd22ff1b079985dfdf50
--- /dev/null
+++ b/app/views/profiles/notifications/_project_settings.html.haml
@@ -0,0 +1,13 @@
+%li.notification-list-item
+ %span.notification.fa.fa-holder.append-right-5
+ - if setting.global?
+ = notification_icon(current_user.notification_level)
+ - else
+ = notification_icon(setting.level)
+
+ %span.str-truncated
+ = 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'
diff --git a/app/views/profiles/notifications/_settings.html.haml b/app/views/profiles/notifications/_settings.html.haml
deleted file mode 100644
index d0d044136f6f7ef07e57ecb797098aa4938ceebe..0000000000000000000000000000000000000000
--- a/app/views/profiles/notifications/_settings.html.haml
+++ /dev/null
@@ -1,17 +0,0 @@
-%li.notification-list-item
- %span.notification.fa.fa-holder.append-right-5
- - if notification.global?
- = notification_icon(@notification)
- - else
- = notification_icon(notification)
-
- %span.str-truncated
- - if membership.kind_of? GroupMember
- = link_to membership.group.name, membership.group
- - else
- = link_to_project(membership.project)
- .pull-right
- = form_tag profile_notifications_path, method: :put, remote: true, class: 'update-notifications' do
- = hidden_field_tag :notification_type, type, id: dom_id(membership, 'notification_type')
- = hidden_field_tag :notification_id, membership.id, id: dom_id(membership, 'notification_id')
- = select_tag :notification_level, options_for_select(Notification.options_with_labels, notification.level), class: 'form-control trigger-submit'
diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml
index 6609295a2a5517fcd4317cfe3b0e7554ccaa63f4..a2a505c082b7c7a20c3e1305094814e0818df5db 100644
--- a/app/views/profiles/notifications/show.html.haml
+++ b/app/views/profiles/notifications/show.html.haml
@@ -1,8 +1,12 @@
- page_title "Notifications"
- header_title page_title, profile_notifications_path
-= form_for @user, url: profile_notifications_path, method: :put, html: { class: 'update-notifications prepend-top-default' } do |f|
- = form_errors(@user)
+%div
+ - if @user.errors.any?
+ %div.alert.alert-danger
+ %ul
+ - @user.errors.full_messages.each do |msg|
+ %li= msg
= hidden_field_tag :notification_type, 'global'
.row
@@ -16,56 +20,55 @@
.col-lg-9
%h5
Global notification settings
- .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'
- .radio
- = f.label :notification_level, value: Notification::N_DISABLED do
- = f.radio_button :notification_level, Notification::N_DISABLED
- .level-title
- Disabled
- %p You will not get any notifications via email
- .radio
- = f.label :notification_level, value: Notification::N_MENTION do
- = f.radio_button :notification_level, Notification::N_MENTION
- .level-title
- On Mention
- %p You will receive notifications only for comments in which you were @mentioned
+ = form_for @user, url: profile_notifications_path, method: :put, html: { class: 'update-notifications prepend-top-default' } do |f|
+ .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'
+ .radio
+ = f.label :notification_level, value: :disabled do
+ = f.radio_button :notification_level, :disabled
+ .level-title
+ Disabled
+ %p You will not get any notifications via email
- .radio
- = f.label :notification_level, value: Notification::N_PARTICIPATING do
- = f.radio_button :notification_level, Notification::N_PARTICIPATING
- .level-title
- Participating
- %p You will only receive notifications from related resources (e.g. from your commits or assigned issues)
+ .radio
+ = f.label :notification_level, value: :mention do
+ = f.radio_button :notification_level, :mention
+ .level-title
+ On Mention
+ %p You will receive notifications only for comments in which you were @mentioned
- .radio
- = f.label :notification_level, value: Notification::N_WATCH do
- = f.radio_button :notification_level, Notification::N_WATCH
- .level-title
- Watch
- %p You will receive notifications for any activity
+ .radio
+ = f.label :notification_level, value: :participating do
+ = f.radio_button :notification_level, :participating
+ .level-title
+ Participating
+ %p You will only receive notifications from related resources (e.g. from your commits or assigned issues)
- .prepend-top-default
- = f.submit 'Update settings', class: "btn btn-create"
+ .radio
+ = f.label :notification_level, value: :watch do
+ = f.radio_button :notification_level, :watch
+ .level-title
+ Watch
+ %p You will receive notifications for any activity
+
+ .prepend-top-default
+ = f.submit 'Update settings', class: "btn btn-create"
%hr
-.col-lg-9.col-lg-push-3
- %h5
- Groups (#{@group_members.count})
- %div
- %ul.bordered-list
- - @group_members.each do |group_member|
- - notification = Notification.new(group_member)
- = render 'settings', type: 'group', membership: group_member, notification: notification
- %h5
- Projects (#{@project_members.count})
- %p.account-well
- To specify the notification level per project of a group you belong to, you need to be a member of the project itself, not only its group.
- .append-bottom-default
- %ul.bordered-list
- - @project_members.each do |project_member|
- - notification = Notification.new(project_member)
- = render 'settings', type: 'project', membership: project_member, notification: notification
+ %h5
+ Groups (#{@group_notifications.count})
+ %div
+ %ul.bordered-list
+ - @group_notifications.each do |setting|
+ = render 'group_settings', setting: setting, group: setting.source
+ %h5
+ Projects (#{@project_notifications.count})
+ %p.account-well
+ To specify the notification level per project of a group you belong to, you need to visit project page and change notification level there.
+ .append-bottom-default
+ %ul.bordered-list
+ - @project_notifications.each do |setting|
+ = render 'project_settings', setting: setting, project: setting.source
diff --git a/app/views/profiles/notifications/update.js.haml b/app/views/profiles/notifications/update.js.haml
deleted file mode 100644
index 84c6ab25599e4356f30a14c33264d884a51b3e07..0000000000000000000000000000000000000000
--- a/app/views/profiles/notifications/update.js.haml
+++ /dev/null
@@ -1,6 +0,0 @@
-- if @saved
- :plain
- new Flash("Notification settings saved", "notice")
-- else
- :plain
- new Flash("Failed to save new settings", "alert")
diff --git a/app/views/projects/buttons/_notifications.html.haml b/app/views/projects/buttons/_notifications.html.haml
index a3786c35a1f93400ec9e8d9d81e2c5116004b116..c1e3e5b73a2c3d945767969881aff32897765bfc 100644
--- a/app/views/projects/buttons/_notifications.html.haml
+++ b/app/views/projects/buttons/_notifications.html.haml
@@ -1,20 +1,11 @@
-- case @membership
-- when ProjectMember
- = form_tag profile_notifications_path, method: :put, remote: true, class: 'inline', id: 'notification-form' do
- = hidden_field_tag :notification_type, 'project'
- = hidden_field_tag :notification_id, @membership.id
- = hidden_field_tag :notification_level
+- 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
%span.dropdown
%a.dropdown-new.btn.notifications-btn#notifications-button{href: '#', "data-toggle" => "dropdown"}
= icon('bell')
- = notification_label(@membership)
+ = notification_title(@notification_setting.level)
= icon('angle-down')
%ul.dropdown-menu.dropdown-menu-right.project-home-dropdown
- - Notification.project_notification_levels.each do |level|
- = notification_list_item(level, @membership)
-
-- when GroupMember
- .btn.disabled.notifications-btn.has-tooltip{title: "To change the notification level, you need to be a member of the project itself, not only its group."}
- = icon('bell')
- = notification_label(@membership)
- = icon('angle-down')
+ - NotificationSetting.levels.each do |level|
+ = notification_list_item(level.first, @notification_setting)
diff --git a/config/routes.rb b/config/routes.rb
index 842fbb99843a92ce465ad7cf9cb14856d0b6c969..48601b7567b08a925279d33ed4df95e2ca617aa6 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -406,6 +406,7 @@ 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
@@ -607,6 +608,7 @@ Rails.application.routes.draw do
resources :forks, only: [:index, :new, :create]
resource :import, only: [:new, :create, :show]
+ resource :notification_setting, only: [:update]
resources :refs, only: [] do
collection do
diff --git a/db/migrate/20160328112808_create_notification_settings.rb b/db/migrate/20160328112808_create_notification_settings.rb
new file mode 100644
index 0000000000000000000000000000000000000000..4755da8b8066a3d498651e617fd4301c4febcad5
--- /dev/null
+++ b/db/migrate/20160328112808_create_notification_settings.rb
@@ -0,0 +1,11 @@
+class CreateNotificationSettings < ActiveRecord::Migration
+ def change
+ create_table :notification_settings do |t|
+ t.references :user, null: false
+ t.references :source, polymorphic: true, null: false
+ t.integer :level, default: 0, null: false
+
+ t.timestamps null: false
+ end
+ end
+end
diff --git a/db/migrate/20160328115649_migrate_new_notification_setting.rb b/db/migrate/20160328115649_migrate_new_notification_setting.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0a110869027a2f95802f41eaaf118ff9e6bc0c81
--- /dev/null
+++ b/db/migrate/20160328115649_migrate_new_notification_setting.rb
@@ -0,0 +1,17 @@
+# This migration will create one row of NotificationSetting for each Member row
+# It can take long time on big instances.
+#
+# This migration can be done online but with following effects:
+# - during migration some users will receive notifications based on their global settings (project/group settings will be ignored)
+# - its possible to get duplicate records for notification settings since we don't create uniq index yet
+#
+class MigrateNewNotificationSetting < ActiveRecord::Migration
+ def up
+ timestamp = Time.now
+ execute "INSERT INTO notification_settings ( user_id, source_id, source_type, level, created_at, updated_at ) SELECT user_id, source_id, source_type, notification_level, '#{timestamp}', '#{timestamp}' FROM members WHERE user_id IS NOT NULL"
+ end
+
+ def down
+ execute "DELETE FROM notification_settings"
+ end
+end
diff --git a/db/migrate/20160328121138_add_notification_setting_index.rb b/db/migrate/20160328121138_add_notification_setting_index.rb
new file mode 100644
index 0000000000000000000000000000000000000000..8aebce0244d68273de80ce36e6333b710628095f
--- /dev/null
+++ b/db/migrate/20160328121138_add_notification_setting_index.rb
@@ -0,0 +1,6 @@
+class AddNotificationSettingIndex < ActiveRecord::Migration
+ def change
+ add_index :notification_settings, :user_id
+ add_index :notification_settings, [:source_id, :source_type]
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index ec235c19131645d54a26a40d451599b2ace8130a..44482de467e379b90e10a6c5bdac648457ed5620 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -637,6 +637,18 @@ ActiveRecord::Schema.define(version: 20160331223143) do
add_index "notes", ["project_id"], name: "index_notes_on_project_id", using: :btree
add_index "notes", ["updated_at"], name: "index_notes_on_updated_at", using: :btree
+ create_table "notification_settings", force: :cascade do |t|
+ t.integer "user_id", null: false
+ t.integer "source_id", null: false
+ t.string "source_type", null: false
+ t.integer "level", default: 0, null: false
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ end
+
+ add_index "notification_settings", ["source_id", "source_type"], name: "index_notification_settings_on_source_id_and_source_type", using: :btree
+ add_index "notification_settings", ["user_id"], name: "index_notification_settings_on_user_id", using: :btree
+
create_table "oauth_access_grants", force: :cascade do |t|
t.integer "resource_owner_id", null: false
t.integer "application_id", null: false
diff --git a/features/profile/notifications.feature b/features/profile/notifications.feature
index 55997d44dec28477bba861b2788c1a893f5a9534..ef8743932f510ab05eeaa0ba7654b7160cedc6c8 100644
--- a/features/profile/notifications.feature
+++ b/features/profile/notifications.feature
@@ -7,3 +7,9 @@ Feature: Profile Notifications
Scenario: I visit notifications tab
When I visit profile notifications page
Then I should see global notifications settings
+
+ @javascript
+ Scenario: I edit Project Notifications
+ Given I visit profile notifications page
+ When I select Mention setting from dropdown
+ Then I should see Notification saved message
diff --git a/features/steps/profile/notifications.rb b/features/steps/profile/notifications.rb
index 447ea6d9d10fd5917772b81bee7f5d1bc6b5e986..a96f35ada51e821a43a26188c4652eb0a737af1f 100644
--- a/features/steps/profile/notifications.rb
+++ b/features/steps/profile/notifications.rb
@@ -9,4 +9,14 @@ class Spinach::Features::ProfileNotifications < Spinach::FeatureSteps
step 'I should see global notifications settings' do
expect(page).to have_content "Notifications"
end
+
+ step 'I select Mention setting from dropdown' do
+ select 'mention', from: 'notification_setting_level'
+ end
+
+ step 'I should see Notification saved message' do
+ page.within '.flash-container' do
+ expect(page).to have_content 'Notification settings saved'
+ end
+ end
end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 939469b3886071523188804674c6a9bcccb1b992..60b9f5e0eceeebbd37278e76cf04fa40fb531c2c 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -263,14 +263,19 @@ module API
expose :id, :path, :kind
end
- class ProjectAccess < Grape::Entity
+ class Member < Grape::Entity
expose :access_level
- expose :notification_level
+ expose :notification_level do |member, options|
+ if member.notification_setting
+ NotificationSetting.levels[member.notification_setting.level]
+ end
+ end
end
- class GroupAccess < Grape::Entity
- expose :access_level
- expose :notification_level
+ class ProjectAccess < Member
+ end
+
+ class GroupAccess < Member
end
class ProjectService < Grape::Entity
diff --git a/spec/controllers/groups/notification_settings_controller_spec.rb b/spec/controllers/groups/notification_settings_controller_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0786e45515a51c637970e54ab0c86233130ec215
--- /dev/null
+++ b/spec/controllers/groups/notification_settings_controller_spec.rb
@@ -0,0 +1,32 @@
+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/projects/notification_settings_controller_spec.rb b/spec/controllers/projects/notification_settings_controller_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..4908b5456487c445625a323df77953b46b7c51b8
--- /dev/null
+++ b/spec/controllers/projects/notification_settings_controller_spec.rb
@@ -0,0 +1,38 @@
+require 'spec_helper'
+
+describe Projects::NotificationSettingsController do
+ let(:project) { create(:empty_project) }
+ let(:user) { create(:user) }
+
+ before do
+ project.team << [user, :developer]
+ end
+
+ describe '#update' do
+ context 'when not authorized' do
+ it 'redirects to sign in page' do
+ put :update,
+ namespace_id: project.namespace.to_param,
+ project_id: project.to_param,
+ notification_setting: { level: :participating }
+
+ expect(response).to redirect_to(new_user_session_path)
+ end
+ end
+
+ context 'when authorized' do
+ before do
+ sign_in(user)
+ end
+
+ it 'returns success' do
+ put :update,
+ namespace_id: project.namespace.to_param,
+ project_id: project.to_param,
+ notification_setting: { level: :participating }
+
+ expect(response.status).to eq 200
+ end
+ end
+ end
+end
diff --git a/spec/helpers/notifications_helper_spec.rb b/spec/helpers/notifications_helper_spec.rb
index f1aba4cfdf3aa3cb7183aaf636f1ac063feb0458..9d5f009ebe1a802b1386282b677af83ccee7fc75 100644
--- a/spec/helpers/notifications_helper_spec.rb
+++ b/spec/helpers/notifications_helper_spec.rb
@@ -2,34 +2,15 @@ require 'spec_helper'
describe NotificationsHelper do
describe 'notification_icon' do
- let(:notification) { double(disabled?: false, participating?: false, watch?: false) }
-
- context "disabled notification" do
- before { allow(notification).to receive(:disabled?).and_return(true) }
-
- it "has a red icon" do
- expect(notification_icon(notification)).to match('class="fa fa-volume-off ns-mute"')
- end
- end
-
- context "participating notification" do
- before { allow(notification).to receive(:participating?).and_return(true) }
-
- it "has a blue icon" do
- expect(notification_icon(notification)).to match('class="fa fa-volume-down ns-part"')
- end
- end
-
- context "watched notification" do
- before { allow(notification).to receive(:watch?).and_return(true) }
-
- it "has a green icon" do
- expect(notification_icon(notification)).to match('class="fa fa-volume-up ns-watch"')
- end
- end
+ it { expect(notification_icon(:disabled)).to match('class="fa fa-microphone-slash fa-fw"') }
+ it { expect(notification_icon(:participating)).to match('class="fa fa-volume-up fa-fw"') }
+ it { expect(notification_icon(:mention)).to match('class="fa fa-at fa-fw"') }
+ it { expect(notification_icon(:global)).to match('class="fa fa-globe fa-fw"') }
+ it { expect(notification_icon(:watch)).to match('class="fa fa-eye fa-fw"') }
+ end
- it "has a blue icon" do
- expect(notification_icon(notification)).to match('class="fa fa-circle-o ns-default"')
- end
+ describe 'notification_title' do
+ it { expect(notification_title(:watch)).to match('Watch') }
+ it { expect(notification_title(:mention)).to match('On mention') }
end
end
diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..295081e9da1646b136a85f08ed6c284738deabdc
--- /dev/null
+++ b/spec/models/notification_setting_spec.rb
@@ -0,0 +1,17 @@
+require 'rails_helper'
+
+RSpec.describe NotificationSetting, type: :model do
+ describe "Associations" do
+ it { is_expected.to belong_to(:user) }
+ it { is_expected.to belong_to(:source) }
+ end
+
+ describe "Validation" do
+ subject { NotificationSetting.new(source_id: 1, source_type: 'Project') }
+
+ it { is_expected.to validate_presence_of(:user) }
+ it { is_expected.to validate_presence_of(:source) }
+ 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/) }
+ end
+end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 0f2aa3ae73ccd959b29abf3b14ee4c8638a60e17..d7c72dc08118672c76097fdaa79e59ad27b7330f 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -88,12 +88,9 @@ describe NotificationService, services: true do
note.project.namespace_id = group.id
note.project.group.add_user(@u_watcher, GroupMember::MASTER)
note.project.save
- user_project = note.project.project_members.find_by_user_id(@u_watcher.id)
- user_project.notification_level = Notification::N_PARTICIPATING
- user_project.save
- group_member = note.project.group.group_members.find_by_user_id(@u_watcher.id)
- group_member.notification_level = Notification::N_GLOBAL
- group_member.save
+
+ @u_watcher.notification_settings_for(note.project).participating!
+ @u_watcher.notification_settings_for(note.project.group).global!
ActionMailer::Base.deliveries.clear
end
@@ -215,7 +212,7 @@ describe NotificationService, services: true do
end
it do
- @u_committer.update_attributes(notification_level: Notification::N_MENTION)
+ @u_committer.update_attributes(notification_level: :mention)
notification.new_note(note)
should_not_email(@u_committer)
end
@@ -246,7 +243,7 @@ describe NotificationService, services: true do
end
it do
- issue.assignee.update_attributes(notification_level: Notification::N_MENTION)
+ issue.assignee.update_attributes(notification_level: :mention)
notification.new_issue(issue, @u_disabled)
should_not_email(issue.assignee)
@@ -596,13 +593,13 @@ describe NotificationService, services: true do
end
def build_team(project)
- @u_watcher = create(:user, notification_level: Notification::N_WATCH)
- @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING)
- @u_participant_mentioned = create(:user, username: 'participant', notification_level: Notification::N_PARTICIPATING)
- @u_disabled = create(:user, notification_level: Notification::N_DISABLED)
- @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_MENTION)
+ @u_watcher = create(:user, notification_level: :watch)
+ @u_participating = create(:user, notification_level: :participating)
+ @u_participant_mentioned = create(:user, username: 'participant', notification_level: :participating)
+ @u_disabled = create(:user, notification_level: :disabled)
+ @u_mentioned = create(:user, username: 'mention', notification_level: :mention)
@u_committer = create(:user, username: 'committer')
- @u_not_mentioned = create(:user, username: 'regular', notification_level: Notification::N_PARTICIPATING)
+ @u_not_mentioned = create(:user, username: 'regular', notification_level: :participating)
@u_outsider_mentioned = create(:user, username: 'outsider')
project.team << [@u_watcher, :master]
@@ -617,8 +614,8 @@ describe NotificationService, services: true do
def add_users_with_subscription(project, issuable)
@subscriber = create :user
@unsubscriber = create :user
- @subscribed_participant = create(:user, username: 'subscribed_participant', notification_level: Notification::N_PARTICIPATING)
- @watcher_and_subscriber = create(:user, notification_level: Notification::N_WATCH)
+ @subscribed_participant = create(:user, username: 'subscribed_participant', notification_level: :participating)
+ @watcher_and_subscriber = create(:user, notification_level: :watch)
project.team << [@subscribed_participant, :master]
project.team << [@subscriber, :master]