From 31b0e53015e38e51d9c02cca85c9279600b1bf85 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 28 Mar 2016 13:41:00 +0200
Subject: [PATCH 01/34] Introduce NotificationSetting model
It will hold notification setting per group or per project. It will
allow get rid of notification level stored in Member model
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/models/notification_setting.rb | 14 ++++++++++++++
...20160328112808_create_notification_settings.rb | 12 ++++++++++++
db/schema.rb | 15 ++++++++++++---
spec/models/notification_setting_spec.rb | 15 +++++++++++++++
4 files changed, 53 insertions(+), 3 deletions(-)
create mode 100644 app/models/notification_setting.rb
create mode 100644 db/migrate/20160328112808_create_notification_settings.rb
create mode 100644 spec/models/notification_setting_spec.rb
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
new file mode 100644
index 000000000000..0dce146b7a9e
--- /dev/null
+++ b/app/models/notification_setting.rb
@@ -0,0 +1,14 @@
+class NotificationSetting < ActiveRecord::Base
+ 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 }
+ # Notification level
+ # Note: When adding an option, it MUST go on the end of the array.
+ enum level: [:disabled, :participating, :watch, :global, :mention]
+end
diff --git a/db/migrate/20160328112808_create_notification_settings.rb b/db/migrate/20160328112808_create_notification_settings.rb
new file mode 100644
index 000000000000..88652821ac38
--- /dev/null
+++ b/db/migrate/20160328112808_create_notification_settings.rb
@@ -0,0 +1,12 @@
+class CreateNotificationSettings < ActiveRecord::Migration
+ def change
+ create_table :notification_settings do |t|
+ t.integer :user_id
+ t.integer :level
+ t.integer :source_id
+ t.string :source_type
+
+ t.timestamps null: false
+ end
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index dce2bfe62cab..a9a68e723ab2 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20160320204112) do
+ActiveRecord::Schema.define(version: 20160328112808) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -417,9 +417,9 @@
t.string "state"
t.integer "iid"
t.integer "updated_by_id"
- t.integer "moved_to_id"
- t.boolean "confidential", default: false
+ t.boolean "confidential", default: false
t.datetime "deleted_at"
+ t.integer "moved_to_id"
end
add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
@@ -638,6 +638,15 @@
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"
+ t.integer "level"
+ t.integer "source_id"
+ t.string "source_type"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ end
+
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/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb
new file mode 100644
index 000000000000..f9d668ed75b4
--- /dev/null
+++ b/spec/models/notification_setting_spec.rb
@@ -0,0 +1,15 @@
+require 'rails_helper'
+
+RSpec.describe NotificationSetting, type: :model do
+ describe "Associations" do
+ it { is_expected.to belong_to(:user) }
+ end
+
+ describe "Validation" do
+ subject { NotificationSetting.new }
+
+ it { is_expected.to validate_presence_of(:user) }
+ it { is_expected.to validate_presence_of(:source) }
+ it { is_expected.to validate_presence_of(:level) }
+ end
+end
--
GitLab
From 73c5a3410596165244bfa3d2f657c313ec1c558c Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 28 Mar 2016 14:27:30 +0200
Subject: [PATCH 02/34] Migrate notification setting from members table
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
...160328115649_migrate_new_notification_setting.rb | 13 +++++++++++++
...20160328121138_add_notification_setting_index.rb | 6 ++++++
db/schema.rb | 5 ++++-
3 files changed, 23 insertions(+), 1 deletion(-)
create mode 100644 db/migrate/20160328115649_migrate_new_notification_setting.rb
create mode 100644 db/migrate/20160328121138_add_notification_setting_index.rb
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 000000000000..331c35535f29
--- /dev/null
+++ b/db/migrate/20160328115649_migrate_new_notification_setting.rb
@@ -0,0 +1,13 @@
+# This migration will create one row of NotificationSetting for each Member row
+# It can take long time on big instances. Its unclear yet if this migration can be done online.
+# This comment should be updated by @dzaporozhets before 8.7 release. If not - please ask him to do so.
+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"
+ end
+
+ def down
+ NotificationSetting.delete_all
+ 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 000000000000..8aebce0244d6
--- /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 a9a68e723ab2..29639abb6fcf 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20160328112808) do
+ActiveRecord::Schema.define(version: 20160328121138) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -647,6 +647,9 @@
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
--
GitLab
From 359157c097993a9b917ca590e128e85cf358d95d Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 28 Mar 2016 18:17:42 +0200
Subject: [PATCH 03/34] Introduce NotificationSetting to user interface
* visiting project will create notification setting if missing
* change notification setting per project even without membership
* use notification settings instead of membership on profile page
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/assets/javascripts/project.js.coffee | 8 +-
.../profiles/notifications_controller.rb | 14 ++--
app/controllers/projects_controller.rb | 17 ++++-
app/helpers/notifications_helper.rb | 74 +++++++++++--------
app/models/notification.rb | 4 +-
app/models/notification_setting.rb | 7 ++
app/models/user.rb | 7 +-
.../notifications/_settings.html.haml | 18 ++---
.../profiles/notifications/show.html.haml | 30 ++++----
.../projects/buttons/_notifications.html.haml | 20 ++---
10 files changed, 106 insertions(+), 93 deletions(-)
diff --git a/app/assets/javascripts/project.js.coffee b/app/assets/javascripts/project.js.coffee
index 87d313ed67c2..f171442d05ad 100644
--- a/app/assets/javascripts/project.js.coffee
+++ b/app/assets/javascripts/project.js.coffee
@@ -37,15 +37,9 @@ class @Project
$('.update-notification').on 'click', (e) ->
e.preventDefault()
notification_level = $(@).data 'notification-level'
+ label = $(@).data 'notification-title'
$('#notification_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'
diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb
index 1fd1d6882dfe..6ca7537300fe 100644
--- a/app/controllers/profiles/notifications_controller.rb
+++ b/app/controllers/profiles/notifications_controller.rb
@@ -2,8 +2,8 @@ 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
@@ -11,14 +11,10 @@ def update
@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
+ notification_setting = current_user.notification_settings.find(params[:notification_id])
+ notification_setting.level = params[:notification_level]
+ notification_setting.save
end
respond_to do |format|
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 928817ba811e..77122f59128b 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -98,14 +98,23 @@ def show
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.find_or_initialize_by(source: @project)
+
+ unless @notification_setting.persisted?
+ @notification_setting.set_defaults
+ @notification_setting.save
+ end
+ 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 499c655d2bf2..a0e91a63d2d4 100644
--- a/app/helpers/notifications_helper.rb
+++ b/app/helpers/notifications_helper.rb
@@ -1,48 +1,60 @@
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
+ def notification_icon(level)
+ icon("#{notification_icon_class(level)} fw")
+ end
+
+ def notification_title(level)
+ case level.to_sym
+ when :disabled
+ 'Disabled'
+ when :participating
+ 'Participate'
+ when :watch
+ 'Watch'
+ when :mention
+ 'On mention'
+ when :global
+ 'Global'
end
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)
+ def notification_list_item(level, setting)
+ title = notification_title(level)
+
+ data = {
+ notification_level: level,
+ notification_title: title
+ }
+
+ content_tag(:li, class: active_level_for(setting, level)) do
+ link_to '#', class: 'update-notification', data: data do
+ icon("#{notification_icon_class(level)} fw", text: title)
end
end
end
- def notification_label(user_membership)
- Notification.new(user_membership).to_s
+ def notification_label(setting)
+ notification_title(setting.level)
end
- def active_level_for(user_membership, level)
- 'active' if user_membership.notification_level == level
+ def active_level_for(setting, level)
+ 'active' if setting.level == level
end
end
diff --git a/app/models/notification.rb b/app/models/notification.rb
index 171b8df45c2f..379f041969b3 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -57,7 +57,7 @@ def mention?
def level
target.notification_level
end
-
+
def to_s
case level
when N_DISABLED
@@ -71,7 +71,7 @@ def to_s
when N_GLOBAL
'Global'
else
- # do nothing
+ # do nothing
end
end
end
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
index 0dce146b7a9e..287862a01bc7 100644
--- a/app/models/notification_setting.rb
+++ b/app/models/notification_setting.rb
@@ -11,4 +11,11 @@ class NotificationSetting < ActiveRecord::Base
# Notification level
# Note: When adding an option, it MUST go on the end of the array.
enum level: [:disabled, :participating, :watch, :global, :mention]
+
+ scope :for_groups, -> { where(source_type: 'Namespace') }
+ scope :for_projects, -> { where(source_type: 'Project') }
+
+ def set_defaults
+ self.level = :global
+ end
end
diff --git a/app/models/user.rb b/app/models/user.rb
index 128ddc2a694e..59493e6f90c7 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,10 @@ 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.
+ enum notification_level: [:disabled, :participating, :watch, :global, :mention]
+
alias_attribute :private_token, :authentication_token
delegate :path, to: :namespace, allow_nil: true, prefix: true
diff --git a/app/views/profiles/notifications/_settings.html.haml b/app/views/profiles/notifications/_settings.html.haml
index d0d044136f6f..c32de0b9925a 100644
--- a/app/views/profiles/notifications/_settings.html.haml
+++ b/app/views/profiles/notifications/_settings.html.haml
@@ -1,17 +1,17 @@
%li.notification-list-item
%span.notification.fa.fa-holder.append-right-5
- - if notification.global?
- = notification_icon(@notification)
+ - if setting.global?
+ = notification_icon(current_user.notification_level)
- else
- = notification_icon(notification)
+ = notification_icon(setting.level)
%span.str-truncated
- - if membership.kind_of? GroupMember
- = link_to membership.group.name, membership.group
+ - if setting.source.kind_of? Project
+ = link_to_project(setting.source)
- else
- = link_to_project(membership.project)
+ = link_to setting.source.name, group_path(setting.source)
.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'
+ = hidden_field_tag :notification_id, setting.id
+ = hidden_field_tag :notification_level, setting.level
+ = select_tag :notification_level, options_for_select(User.notification_levels.keys, setting.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 de80abd7f4da..f6900f61b2d0 100644
--- a/app/views/profiles/notifications/show.html.haml
+++ b/app/views/profiles/notifications/show.html.haml
@@ -26,29 +26,29 @@
.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
+ = 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_MENTION do
- = f.radio_button :notification_level, Notification::N_MENTION
+ = 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_PARTICIPATING do
- = f.radio_button :notification_level, Notification::N_PARTICIPATING
+ = 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)
.radio
- = f.label :notification_level, value: Notification::N_WATCH do
- = f.radio_button :notification_level, Notification::N_WATCH
+ = f.label :notification_level, value: :watch do
+ = f.radio_button :notification_level, :watch
.level-title
Watch
%p You will receive notifications for any activity
@@ -57,18 +57,16 @@
= f.submit 'Update settings', class: "btn btn-create"
%hr
%h5
- Groups (#{@group_members.count})
+ Groups (#{@group_notifications.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
+ - @group_notifications.each do |setting|
+ = render 'settings', setting: setting
%h5
- Projects (#{@project_members.count})
+ Projects (#{@project_notifications.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
+ - @project_notifications.each do |setting|
+ = render 'settings', setting: setting
diff --git a/app/views/projects/buttons/_notifications.html.haml b/app/views/projects/buttons/_notifications.html.haml
index a3786c35a1f9..4b8a10f08197 100644
--- a/app/views/projects/buttons/_notifications.html.haml
+++ b/app/views/projects/buttons/_notifications.html.haml
@@ -1,20 +1,12 @@
-- case @membership
-- when ProjectMember
+- if @notification_setting
= 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
+ = hidden_field_tag :notification_id, @notification_setting.id
+ = hidden_field_tag :notification_level, @notification_setting.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)
--
GitLab
From 7ea1bcab45697556d4ffd79ab680872ed823d4a3 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 28 Mar 2016 18:25:57 +0200
Subject: [PATCH 04/34] Create notification setting when membership created
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/models/member.rb | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/app/models/member.rb b/app/models/member.rb
index ca08007b7eb9..177f37c3bbdc 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -56,6 +56,7 @@ 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?
@@ -160,6 +161,15 @@ def resend_invite
send_invite
end
+ def create_notification_setting
+ notification_setting = user.notification_settings.find_or_initialize_by(source: source)
+
+ unless notification_setting.persisted?
+ notification_setting.set_defaults
+ notification_setting.save
+ end
+ end
+
private
def send_invite
--
GitLab
From b8f38437900cdddac9d19d5c48a2a8e5bb037f41 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 28 Mar 2016 20:31:36 +0200
Subject: [PATCH 05/34] Update NotificationService to use NotificationSettings
instead of membership
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/models/concerns/notifiable.rb | 15 ----------
app/models/group.rb | 1 +
app/models/member.rb | 5 +++-
app/models/notification.rb | 22 ++------------
app/models/project.rb | 1 +
app/models/user.rb | 3 ++
app/services/notification_service.rb | 34 ++++++++++------------
spec/services/notification_service_spec.rb | 26 ++++++++---------
8 files changed, 40 insertions(+), 67 deletions(-)
delete mode 100644 app/models/concerns/notifiable.rb
diff --git a/app/models/concerns/notifiable.rb b/app/models/concerns/notifiable.rb
deleted file mode 100644
index d7dcd97911dd..000000000000
--- 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 b332601c59b7..9a04ac70d350 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 177f37c3bbdc..e665ba6fb756 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
@@ -170,6 +169,10 @@ def create_notification_setting
end
end
+ def notification
+ @notification ||= user.notification_settings.find_by(source: source)
+ end
+
private
def send_invite
diff --git a/app/models/notification.rb b/app/models/notification.rb
index 379f041969b3..8a90b456cc26 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -30,30 +30,12 @@ def project_notification_levels
end
end
+ delegate :disabled?, :participating?, :watch?, :global?, :mention?, to: :target
+
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
diff --git a/app/models/project.rb b/app/models/project.rb
index 2285063ab507..2f9621809b62 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -154,6 +154,7 @@ def update_forks_visibility_level
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 59493e6f90c7..f556dc5903d8 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -193,6 +193,9 @@ class User < ActiveRecord::Base
# 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
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index eff0d96f93dd..9628843c230f 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -253,8 +253,8 @@ def issue_moved(issue, new_issue, current_user)
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 @@ def project_watchers(project)
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 @@ def group_member_notification(project, notification_level)
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 @@ def select_project_member_setting(project, global_setting, users_global_level_wa
# 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 = []
@@ -351,20 +349,20 @@ def reject_users(users, method_name, project = nil)
users.reject do |user|
next user.notification.send(method_name) unless project
- member = project.project_members.find_by(user_id: user.id)
+ setting = user.notification_settings.find_by(source: project)
- if !member && project.group
- member = project.group.group_members.find_by(user_id: user.id)
+ if !setting && project.group
+ setting = user.notification_settings.find_by(source: 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.send(method_name) unless setting
# reject users who set mention notification in project
- next true if member.notification.send(method_name)
+ next true if setting.send(method_name)
- # 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.send(method_name)
end
end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 0f2aa3ae73cc..c01851a8a24f 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -89,11 +89,11 @@
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.notification.level = :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
+ group_member.notification.level = :global
+ group_member.notification.save
ActionMailer::Base.deliveries.clear
end
@@ -215,7 +215,7 @@
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 +246,7 @@
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 +596,13 @@
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 +617,8 @@ def build_team(project)
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]
--
GitLab
From 4ca73f56cb59b86f25b55ff02800571fb82c742f Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 28 Mar 2016 23:22:28 +0200
Subject: [PATCH 06/34] Small refactoring and cleanup of notification logic
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/helpers/notifications_helper.rb | 12 +-----
app/models/member.rb | 2 +
app/models/members/group_member.rb | 1 -
app/models/members/project_member.rb | 1 -
app/models/notification.rb | 46 ----------------------
spec/models/notification_setting_spec.rb | 1 +
spec/services/notification_service_spec.rb | 9 ++---
7 files changed, 8 insertions(+), 64 deletions(-)
diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb
index a0e91a63d2d4..8816cc5d1649 100644
--- a/app/helpers/notifications_helper.rb
+++ b/app/helpers/notifications_helper.rb
@@ -22,16 +22,12 @@ def notification_icon(level)
def notification_title(level)
case level.to_sym
- when :disabled
- 'Disabled'
when :participating
'Participate'
- when :watch
- 'Watch'
when :mention
'On mention'
- when :global
- 'Global'
+ else
+ level.to_s.titlecase
end
end
@@ -50,10 +46,6 @@ def notification_list_item(level, setting)
end
end
- def notification_label(setting)
- notification_title(setting.level)
- end
-
def active_level_for(setting, level)
'active' if setting.level == level
end
diff --git a/app/models/member.rb b/app/models/member.rb
index e665ba6fb756..799f28c3fdf3 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -62,6 +62,8 @@ class Member < ActiveRecord::Base
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)
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index 65d2ea00570d..9fb474a1a93d 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 560d1690e143..07ddb02ae9dd 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
index 8a90b456cc26..3805bde88b0d 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -1,35 +1,6 @@
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
-
delegate :disabled?, :participating?, :watch?, :global?, :mention?, to: :target
def initialize(target)
@@ -39,21 +10,4 @@ def initialize(target)
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/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb
index f9d668ed75b4..f31b2a3cd6f2 100644
--- a/spec/models/notification_setting_spec.rb
+++ b/spec/models/notification_setting_spec.rb
@@ -3,6 +3,7 @@
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
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index c01851a8a24f..c4d52584a4b7 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -88,12 +88,9 @@
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 = :participating
- user_project.save
- group_member = note.project.group.group_members.find_by_user_id(@u_watcher.id)
- group_member.notification.level = :global
- group_member.notification.save
+
+ @u_watcher.notification_settings.find_by(source: note.project).participating!
+ @u_watcher.notification_settings.find_by(source: note.project.group).global!
ActionMailer::Base.deliveries.clear
end
--
GitLab
From 855b2820c16c6e569d5c38b7def8ead18c86cecd Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 28 Mar 2016 23:39:49 +0200
Subject: [PATCH 07/34] Improve db migrations for notification settings
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
...60328112808_create_notification_settings.rb | 7 +++----
...8115649_migrate_new_notification_setting.rb | 2 +-
db/schema.rb | 18 +++++++++---------
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/db/migrate/20160328112808_create_notification_settings.rb b/db/migrate/20160328112808_create_notification_settings.rb
index 88652821ac38..4755da8b8066 100644
--- a/db/migrate/20160328112808_create_notification_settings.rb
+++ b/db/migrate/20160328112808_create_notification_settings.rb
@@ -1,10 +1,9 @@
class CreateNotificationSettings < ActiveRecord::Migration
def change
create_table :notification_settings do |t|
- t.integer :user_id
- t.integer :level
- t.integer :source_id
- t.string :source_type
+ t.references :user, null: false
+ t.references :source, polymorphic: true, null: false
+ t.integer :level, default: 0, null: false
t.timestamps null: false
end
diff --git a/db/migrate/20160328115649_migrate_new_notification_setting.rb b/db/migrate/20160328115649_migrate_new_notification_setting.rb
index 331c35535f29..aff866b5f46f 100644
--- a/db/migrate/20160328115649_migrate_new_notification_setting.rb
+++ b/db/migrate/20160328115649_migrate_new_notification_setting.rb
@@ -4,7 +4,7 @@
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"
+ 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
diff --git a/db/schema.rb b/db/schema.rb
index 29639abb6fcf..e946ecd3f2b3 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -639,12 +639,12 @@
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"
- t.integer "level"
- t.integer "source_id"
- t.string "source_type"
- t.datetime "created_at", null: false
- t.datetime "updated_at", null: false
+ 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
@@ -798,9 +798,9 @@
t.string "type"
t.string "title"
t.integer "project_id"
- t.datetime "created_at", null: false
- t.datetime "updated_at", null: false
- t.boolean "active", null: false
+ t.datetime "created_at"
+ t.datetime "updated_at"
+ t.boolean "active", default: false, null: false
t.text "properties"
t.boolean "template", default: false
t.boolean "push_events", default: true
--
GitLab
From 08b3d7f6ef23b7a8a83c7e71e2d04f6416e73406 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Mar 2016 13:08:30 +0200
Subject: [PATCH 08/34] Refactor notification helper and fix notification
service
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/helpers/notifications_helper.rb | 12 +++-----
app/services/notification_service.rb | 2 +-
spec/helpers/notifications_helper_spec.rb | 37 ++++++-----------------
3 files changed, 14 insertions(+), 37 deletions(-)
diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb
index 8816cc5d1649..54ab9179efc0 100644
--- a/app/helpers/notifications_helper.rb
+++ b/app/helpers/notifications_helper.rb
@@ -16,8 +16,8 @@ def notification_icon_class(level)
end
end
- def notification_icon(level)
- icon("#{notification_icon_class(level)} fw")
+ def notification_icon(level, text = nil)
+ icon("#{notification_icon_class(level)} fw", text: text)
end
def notification_title(level)
@@ -39,14 +39,10 @@ def notification_list_item(level, setting)
notification_title: title
}
- content_tag(:li, class: active_level_for(setting, level)) do
+ content_tag(:li, class: ('active' if setting.level == level)) do
link_to '#', class: 'update-notification', data: data do
- icon("#{notification_icon_class(level)} fw", text: title)
+ notification_icon(level, title)
end
end
end
-
- def active_level_for(setting, level)
- 'active' if setting.level == level
- end
end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 9628843c230f..23f211dfcd2a 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -352,7 +352,7 @@ def reject_users(users, method_name, project = nil)
setting = user.notification_settings.find_by(source: project)
if !setting && project.group
- setting = user.notification_settings.find_by(source: group)
+ setting = user.notification_settings.find_by(source: project.group)
end
# reject users who globally set mention notification and has no setting per project/group
diff --git a/spec/helpers/notifications_helper_spec.rb b/spec/helpers/notifications_helper_spec.rb
index f1aba4cfdf3a..9d5f009ebe1a 100644
--- a/spec/helpers/notifications_helper_spec.rb
+++ b/spec/helpers/notifications_helper_spec.rb
@@ -2,34 +2,15 @@
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
--
GitLab
From 86418c475be1b2a37e89682bc87055b7372bbcfb Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Mar 2016 13:37:43 +0200
Subject: [PATCH 09/34] Remove useless Notification model
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/models/member.rb | 4 ++--
app/models/notification.rb | 13 -------------
app/models/user.rb | 4 ----
app/services/notification_service.rb | 24 +++++++++++++++---------
4 files changed, 17 insertions(+), 28 deletions(-)
delete mode 100644 app/models/notification.rb
diff --git a/app/models/member.rb b/app/models/member.rb
index 799f28c3fdf3..cbcc54c02506 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -171,8 +171,8 @@ def create_notification_setting
end
end
- def notification
- @notification ||= user.notification_settings.find_by(source: source)
+ def notification_setting
+ @notification_setting ||= user.notification_settings.find_by(source: source)
end
private
diff --git a/app/models/notification.rb b/app/models/notification.rb
deleted file mode 100644
index 3805bde88b0d..000000000000
--- a/app/models/notification.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-class Notification
- attr_accessor :target
-
- delegate :disabled?, :participating?, :watch?, :global?, :mention?, to: :target
-
- def initialize(target)
- @target = target
- end
-
- def level
- target.notification_level
- end
-end
diff --git a/app/models/user.rb b/app/models/user.rb
index f556dc5903d8..af6b86bfa703 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -357,10 +357,6 @@ def to_reference(_from_project = nil)
"#{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)
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 23f211dfcd2a..0928dda349ef 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -329,25 +329,31 @@ def add_project_watchers(recipients, project)
# 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
setting = user.notification_settings.find_by(source: project)
@@ -356,13 +362,13 @@ def reject_users(users, method_name, project = nil)
end
# reject users who globally set mention notification and has no setting per project/group
- next user.notification.send(method_name) unless setting
+ next user.notification_level == level unless setting
# reject users who set mention notification in project
- next true if setting.send(method_name)
+ next true if setting.level == level
# reject users who have mention level in project and disabled in global settings
- setting.global? && user.notification.send(method_name)
+ setting.global? && user.notification_level == level
end
end
--
GitLab
From 630c86a7a36cee36ed6b9c93644a6cb51e2b3b23 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Mar 2016 13:42:48 +0200
Subject: [PATCH 10/34] Add spec for user_id uniq in NotificationSetting model
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
spec/models/notification_setting_spec.rb | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb
index f31b2a3cd6f2..295081e9da16 100644
--- a/spec/models/notification_setting_spec.rb
+++ b/spec/models/notification_setting_spec.rb
@@ -7,10 +7,11 @@
end
describe "Validation" do
- subject { NotificationSetting.new }
+ 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
--
GitLab
From 71e7b398431506c8bac2e8e6014b0f3891a41f95 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Mar 2016 13:52:42 +0200
Subject: [PATCH 11/34] Refactor creating notification setting with defaults
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/controllers/projects_controller.rb | 7 +------
app/models/member.rb | 7 +------
app/models/notification_setting.rb | 11 +++++++++++
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 77122f59128b..e2dc6309d263 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -102,12 +102,7 @@ def show
@membership = @project.team.find_member(current_user.id)
if @membership
- @notification_setting = current_user.notification_settings.find_or_initialize_by(source: @project)
-
- unless @notification_setting.persisted?
- @notification_setting.set_defaults
- @notification_setting.save
- end
+ @notification_setting = current_user.notification_settings.find_or_create_for(@project)
end
end
diff --git a/app/models/member.rb b/app/models/member.rb
index cbcc54c02506..747d0f16d8d9 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -163,12 +163,7 @@ def resend_invite
end
def create_notification_setting
- notification_setting = user.notification_settings.find_or_initialize_by(source: source)
-
- unless notification_setting.persisted?
- notification_setting.set_defaults
- notification_setting.save
- end
+ user.notification_setting.find_or_create_for(source)
end
def notification_setting
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
index 287862a01bc7..13a8995b036c 100644
--- a/app/models/notification_setting.rb
+++ b/app/models/notification_setting.rb
@@ -15,6 +15,17 @@ class NotificationSetting < ActiveRecord::Base
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.set_defaults
+ setting.save
+ end
+
+ setting
+ end
+
def set_defaults
self.level = :global
end
--
GitLab
From f8f68d6b8c5b5d85b4798257ae3ae4bf4ec8fadc Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Mar 2016 14:03:23 +0200
Subject: [PATCH 12/34] Fix few bugs related to recent notifications
refactoring
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/controllers/profiles/notifications_controller.rb | 1 -
app/models/member.rb | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb
index 6ca7537300fe..0cca5d1e3307 100644
--- a/app/controllers/profiles/notifications_controller.rb
+++ b/app/controllers/profiles/notifications_controller.rb
@@ -1,7 +1,6 @@
class Profiles::NotificationsController < Profiles::ApplicationController
def show
@user = current_user
- @notification = current_user.notification
@group_notifications = current_user.notification_settings.for_groups
@project_notifications = current_user.notification_settings.for_projects
end
diff --git a/app/models/member.rb b/app/models/member.rb
index 747d0f16d8d9..7d5af1d5c8af 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -163,7 +163,7 @@ def resend_invite
end
def create_notification_setting
- user.notification_setting.find_or_create_for(source)
+ user.notification_settings.find_or_create_for(source)
end
def notification_setting
--
GitLab
From 5583197e091e8f75ad9c99a1bbc46e6a0b7279d4 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Mar 2016 17:42:38 +0200
Subject: [PATCH 13/34] Create NotificationSettings object only when user
change value in dropdown
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/assets/javascripts/project.js.coffee | 9 +++++++-
.../notification_settings_controller.rb | 22 +++++++++++++++++++
app/controllers/projects_controller.rb | 3 ++-
.../projects/buttons/_notifications.html.haml | 5 ++---
config/routes.rb | 1 +
5 files changed, 35 insertions(+), 5 deletions(-)
create mode 100644 app/controllers/projects/notification_settings_controller.rb
diff --git a/app/assets/javascripts/project.js.coffee b/app/assets/javascripts/project.js.coffee
index f171442d05ad..07be85a32a5c 100644
--- a/app/assets/javascripts/project.js.coffee
+++ b/app/assets/javascripts/project.js.coffee
@@ -38,12 +38,19 @@ class @Project
e.preventDefault()
notification_level = $(@).data 'notification-level'
label = $(@).data 'notification-title'
- $('#notification_level').val(notification_level)
+ $('#notification_setting_level').val(notification_level)
$('#notification-form').submit()
$('#notifications-button').empty().append("<i class='fa fa-bell'></i>" + label + "<i class='fa fa-angle-down'></i>")
$(@).parents('ul').find('li.active').removeClass 'active'
$(@).parent().addClass 'active'
+ $('#notification-form').on 'ajax:success', (e, data) ->
+ if data.saved
+ new Flash("Notification settings saved", "notice")
+ else
+ new Flash("Failed to save new settings", "alert")
+
+
@projectSelectDropdown()
projectSelectDropdown: ->
diff --git a/app/controllers/projects/notification_settings_controller.rb b/app/controllers/projects/notification_settings_controller.rb
new file mode 100644
index 000000000000..3ecf63d107f0
--- /dev/null
+++ b/app/controllers/projects/notification_settings_controller.rb
@@ -0,0 +1,22 @@
+class Projects::NotificationSettingsController < Projects::ApplicationController
+ def create
+ notification_setting = project.notification_settings.new(notification_setting_params)
+ notification_setting.user = current_user
+ saved = notification_setting.save
+
+ render json: { saved: saved }
+ end
+
+ def update
+ notification_setting = project.notification_settings.where(user_id: current_user).find(params[:id])
+ 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 e2dc6309d263..ec5318f2d2cf 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -102,7 +102,8 @@ def show
@membership = @project.team.find_member(current_user.id)
if @membership
- @notification_setting = current_user.notification_settings.find_or_create_for(@project)
+ @notification_setting = current_user.notification_settings.find_or_initialize_by(source: @project)
+ @notification_setting.set_defaults unless @notification_setting.persisted?
end
end
diff --git a/app/views/projects/buttons/_notifications.html.haml b/app/views/projects/buttons/_notifications.html.haml
index 4b8a10f08197..2b9d8f2ac816 100644
--- a/app/views/projects/buttons/_notifications.html.haml
+++ b/app/views/projects/buttons/_notifications.html.haml
@@ -1,7 +1,6 @@
- if @notification_setting
- = form_tag profile_notifications_path, method: :put, remote: true, class: 'inline', id: 'notification-form' do
- = hidden_field_tag :notification_id, @notification_setting.id
- = hidden_field_tag :notification_level, @notification_setting.level
+ = form_for [@project.namespace.becomes(Namespace), @project, @notification_setting], 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')
diff --git a/config/routes.rb b/config/routes.rb
index 6bf22fb44567..c665cefbb57d 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -606,6 +606,7 @@
resources :forks, only: [:index, :new, :create]
resource :import, only: [:new, :create, :show]
+ resources :notification_settings, only: [:create, :update]
resources :refs, only: [] do
collection do
--
GitLab
From 729fe42bff474535c9eebb0b73974a79756372b8 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Mar 2016 17:56:12 +0200
Subject: [PATCH 14/34] Improve project notification settings explanation
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/views/profiles/notifications/show.html.haml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml
index f6900f61b2d0..e9c8ae285447 100644
--- a/app/views/profiles/notifications/show.html.haml
+++ b/app/views/profiles/notifications/show.html.haml
@@ -65,7 +65,7 @@
%h5
Projects (#{@project_notifications.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.
+ 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|
--
GitLab
From 26631f9981a826ebe4aeba726e9be2b813d2c5c5 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Mar 2016 18:59:03 +0200
Subject: [PATCH 15/34] Change how notification settings in profile are
rendered and updated
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
app/assets/javascripts/profile.js.coffee | 7 +-
.../notification_settings_controller.rb | 14 ++++
.../profiles/notifications_controller.rb | 28 ++------
.../notifications/_group_settings.html.haml | 13 ++++
.../notifications/_project_settings.html.haml | 13 ++++
.../notifications/_settings.html.haml | 17 -----
.../profiles/notifications/show.html.haml | 72 ++++++++++---------
.../profiles/notifications/update.js.haml | 6 --
config/routes.rb | 1 +
9 files changed, 89 insertions(+), 82 deletions(-)
create mode 100644 app/controllers/groups/notification_settings_controller.rb
create mode 100644 app/views/profiles/notifications/_group_settings.html.haml
create mode 100644 app/views/profiles/notifications/_project_settings.html.haml
delete mode 100644 app/views/profiles/notifications/_settings.html.haml
delete mode 100644 app/views/profiles/notifications/update.js.haml
diff --git a/app/assets/javascripts/profile.js.coffee b/app/assets/javascripts/profile.js.coffee
index ae87c6c4e40b..f4a2562885df 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/controllers/groups/notification_settings_controller.rb b/app/controllers/groups/notification_settings_controller.rb
new file mode 100644
index 000000000000..78e43c83abab
--- /dev/null
+++ b/app/controllers/groups/notification_settings_controller.rb
@@ -0,0 +1,14 @@
+class Groups::NotificationSettingsController < Groups::ApplicationController
+ def update
+ notification_setting = group.notification_settings.where(user_id: current_user).find(params[:id])
+ 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 0cca5d1e3307..18ee55c839a6 100644
--- a/app/controllers/profiles/notifications_controller.rb
+++ b/app/controllers/profiles/notifications_controller.rb
@@ -6,29 +6,13 @@ def show
end
def update
- type = params[:notification_type]
-
- @saved = if type == 'global'
- current_user.update_attributes(user_params)
- else
- notification_setting = current_user.notification_settings.find(params[:notification_id])
- notification_setting.level = params[:notification_level]
- notification_setting.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/views/profiles/notifications/_group_settings.html.haml b/app/views/profiles/notifications/_group_settings.html.haml
new file mode 100644
index 000000000000..89ae7ffda2bb
--- /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 000000000000..17c097154da6
--- /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 c32de0b9925a..000000000000
--- 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 setting.global?
- = notification_icon(current_user.notification_level)
- - else
- = notification_icon(setting.level)
-
- %span.str-truncated
- - if setting.source.kind_of? Project
- = link_to_project(setting.source)
- - else
- = link_to setting.source.name, group_path(setting.source)
- .pull-right
- = form_tag profile_notifications_path, method: :put, remote: true, class: 'update-notifications' do
- = hidden_field_tag :notification_id, setting.id
- = hidden_field_tag :notification_level, setting.level
- = select_tag :notification_level, options_for_select(User.notification_levels.keys, setting.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 e9c8ae285447..a2a505c082b7 100644
--- a/app/views/profiles/notifications/show.html.haml
+++ b/app/views/profiles/notifications/show.html.haml
@@ -1,8 +1,8 @@
- 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|
- -if @user.errors.any?
+%div
+ - if @user.errors.any?
%div.alert.alert-danger
%ul
- @user.errors.full_messages.each do |msg|
@@ -20,48 +20,50 @@
.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: :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: :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
+ = 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: :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)
+ .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: :watch do
- = f.radio_button :notification_level, :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
%h5
Groups (#{@group_notifications.count})
%div
%ul.bordered-list
- @group_notifications.each do |setting|
- = render 'settings', setting: setting
+ = render 'group_settings', setting: setting, group: setting.source
%h5
Projects (#{@project_notifications.count})
%p.account-well
@@ -69,4 +71,4 @@
.append-bottom-default
%ul.bordered-list
- @project_notifications.each do |setting|
- = render 'settings', setting: 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 84c6ab25599e..000000000000
--- 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/config/routes.rb b/config/routes.rb
index c665cefbb57d..7f03fbf6af91 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -406,6 +406,7 @@
resource :avatar, only: [:destroy]
resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
+ resources :notification_settings, only: [:update]
end
end
--
GitLab
From 949431fa02da18257c8b7c7a24c03faa04c02c5e Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Mar 2016 22:19:00 +0200
Subject: [PATCH 16/34] Update API to use notification_level from
notification_setting
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
lib/api/entities.rb | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index f686c568bee3..f414c1f98855 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -255,14 +255,19 @@ class Namespace < Grape::Entity
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
--
GitLab
From 49f9873ce2a88fb38f23f7eb09471e8b58aebe1d Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Tue, 29 Mar 2016 22:20:59 +0200
Subject: [PATCH 17/34] Add changelog item
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG b/CHANGELOG
index 2cd06d90257d..88edf1e8484a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -12,6 +12,7 @@ v 8.7.0 (unreleased)
- Implement 'Groups View' as an option for dashboard preferences !3379 (Elias W.)
- 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
v 8.6.2 (unreleased)
- Comments on confidential issues don't show up in activity feed to non-members
--
GitLab
From 065faac3a390f29b57db5261e9eab4efa076554c Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Wed, 30 Mar 2016 10:32:19 +0200
Subject: [PATCH 18/34] Test changing notification settings per project fron
notificaitons page
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
features/profile/notifications.feature | 6 ++++++
features/steps/profile/notifications.rb | 10 ++++++++++
2 files changed, 16 insertions(+)
diff --git a/features/profile/notifications.feature b/features/profile/notifications.feature
index 55997d44dec2..ef8743932f51 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 447ea6d9d10f..a96f35ada51e 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
--
GitLab
From 1a293a43847b30d4357116f830a1b22d370c4a6f Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Wed, 30 Mar 2016 10:38:03 +0200
Subject: [PATCH 19/34] Update migration comment
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
.../20160328115649_migrate_new_notification_setting.rb | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/db/migrate/20160328115649_migrate_new_notification_setting.rb b/db/migrate/20160328115649_migrate_new_notification_setting.rb
index aff866b5f46f..6a68890f5b15 100644
--- a/db/migrate/20160328115649_migrate_new_notification_setting.rb
+++ b/db/migrate/20160328115649_migrate_new_notification_setting.rb
@@ -1,6 +1,10 @@
# This migration will create one row of NotificationSetting for each Member row
-# It can take long time on big instances. Its unclear yet if this migration can be done online.
-# This comment should be updated by @dzaporozhets before 8.7 release. If not - please ask him to do so.
+# 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
--
GitLab
From d11288be9580633baa3b21b4c70a5fc01e53d094 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 8 Apr 2016 16:49:01 -0300
Subject: [PATCH 20/34] Use query instead of model on migrations
---
db/migrate/20160328115649_migrate_new_notification_setting.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/db/migrate/20160328115649_migrate_new_notification_setting.rb b/db/migrate/20160328115649_migrate_new_notification_setting.rb
index 6a68890f5b15..0a110869027a 100644
--- a/db/migrate/20160328115649_migrate_new_notification_setting.rb
+++ b/db/migrate/20160328115649_migrate_new_notification_setting.rb
@@ -12,6 +12,6 @@ def up
end
def down
- NotificationSetting.delete_all
+ execute "DELETE FROM notification_settings"
end
end
--
GitLab
From 127119f2c4db9038a7f34d1cc73ae1ed19cf0b8d Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 8 Apr 2016 16:59:06 -0300
Subject: [PATCH 21/34] Simplify query to retrieve NotificationSetting on
controllers
---
app/controllers/groups/notification_settings_controller.rb | 2 +-
app/controllers/projects/notification_settings_controller.rb | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/controllers/groups/notification_settings_controller.rb b/app/controllers/groups/notification_settings_controller.rb
index 78e43c83abab..20405a051906 100644
--- a/app/controllers/groups/notification_settings_controller.rb
+++ b/app/controllers/groups/notification_settings_controller.rb
@@ -1,6 +1,6 @@
class Groups::NotificationSettingsController < Groups::ApplicationController
def update
- notification_setting = group.notification_settings.where(user_id: current_user).find(params[:id])
+ notification_setting = group.notification_settings.find_by(user_id: current_user)
saved = notification_setting.update_attributes(notification_setting_params)
render json: { saved: saved }
diff --git a/app/controllers/projects/notification_settings_controller.rb b/app/controllers/projects/notification_settings_controller.rb
index 3ecf63d107f0..da9034380afe 100644
--- a/app/controllers/projects/notification_settings_controller.rb
+++ b/app/controllers/projects/notification_settings_controller.rb
@@ -8,7 +8,7 @@ def create
end
def update
- notification_setting = project.notification_settings.where(user_id: current_user).find(params[:id])
+ notification_setting = project.notification_settings.find_by(user_id: current_user)
saved = notification_setting.update_attributes(notification_setting_params)
render json: { saved: saved }
--
GitLab
From 069724cef5873b83720004772d1e874030cc9fff Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 8 Apr 2016 17:11:13 -0300
Subject: [PATCH 22/34] Use singular resource for NotificationSetting
Since a user cannot have multiple NotificationSettings records for one
group/project we can use singular resource.
---
config/routes.rb | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/config/routes.rb b/config/routes.rb
index fade07c05005..552385110dd2 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -406,7 +406,7 @@
resource :avatar, only: [:destroy]
resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
- resources :notification_settings, only: [:update]
+ resource :notification_setting, only: [:update]
end
end
@@ -608,7 +608,7 @@
resources :forks, only: [:index, :new, :create]
resource :import, only: [:new, :create, :show]
- resources :notification_settings, only: [:create, :update]
+ resource :notification_setting, only: [:create, :update]
resources :refs, only: [] do
collection do
--
GitLab
From ee497599cc69b126cc2e4a929f1799d3d3eb989d Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 8 Apr 2016 17:24:27 -0300
Subject: [PATCH 23/34] Use default_value_for to set default
NotificationSetting#level
---
app/controllers/projects_controller.rb | 1 -
app/models/notification_setting.rb | 14 ++++++--------
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 41a4c41cf805..39f436f6f4e4 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -106,7 +106,6 @@ def show
if @membership
@notification_setting = current_user.notification_settings.find_or_initialize_by(source: @project)
- @notification_setting.set_defaults unless @notification_setting.persisted?
end
end
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
index 13a8995b036c..d89194b5a128 100644
--- a/app/models/notification_setting.rb
+++ b/app/models/notification_setting.rb
@@ -1,4 +1,10 @@
class NotificationSetting < ActiveRecord::Base
+ # Notification level
+ # Note: When adding an option, it MUST go on the end of the array.
+ enum level: [:disabled, :participating, :watch, :global, :mention]
+
+ default_value_for :level, NotificationSetting.levels[:global]
+
belongs_to :user
belongs_to :source, polymorphic: true
@@ -8,9 +14,6 @@ class NotificationSetting < ActiveRecord::Base
validates :user_id, uniqueness: { scope: [:source_type, :source_id],
message: "already exists in source",
allow_nil: true }
- # Notification level
- # Note: When adding an option, it MUST go on the end of the array.
- enum level: [:disabled, :participating, :watch, :global, :mention]
scope :for_groups, -> { where(source_type: 'Namespace') }
scope :for_projects, -> { where(source_type: 'Project') }
@@ -19,14 +22,9 @@ def self.find_or_create_for(source)
setting = find_or_initialize_by(source: source)
unless setting.persisted?
- setting.set_defaults
setting.save
end
setting
end
-
- def set_defaults
- self.level = :global
- end
end
--
GitLab
From 635b65d1206293d0b92322df18d458447ee73d5a Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 8 Apr 2016 17:35:21 -0300
Subject: [PATCH 24/34] Add method to return the user notification setting for
a group, or a project
---
app/controllers/projects_controller.rb | 2 +-
app/models/user.rb | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 39f436f6f4e4..3768efe142aa 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -105,7 +105,7 @@ def show
@membership = @project.team.find_member(current_user.id)
if @membership
- @notification_setting = current_user.notification_settings.find_or_initialize_by(source: @project)
+ @notification_setting = current_user.notification_settings_for(@project)
end
end
diff --git a/app/models/user.rb b/app/models/user.rb
index f68379fd0504..031315debd75 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -831,6 +831,10 @@ def ci_authorized_runners
end
end
+ def notification_settings_for(source)
+ notification_settings.find_or_initialize_by(source: source)
+ end
+
private
def projects_union
--
GitLab
From e3d4ebdd543f17d91e5c67be2f30bb97c687b448 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 7 Apr 2016 16:40:51 +0200
Subject: [PATCH 25/34] Update gitlab-shell to 2.7.2
---
GITLAB_SHELL_VERSION | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION
index 24ba9a38de68..37c2961c2430 100644
--- a/GITLAB_SHELL_VERSION
+++ b/GITLAB_SHELL_VERSION
@@ -1 +1 @@
-2.7.0
+2.7.2
--
GitLab
From 47c8b7f3037c3e464e34d6f978a27e591f09e687 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 8 Apr 2016 17:44:55 -0300
Subject: [PATCH 26/34] Fix CHANGELOG
---
CHANGELOG | 4 ----
1 file changed, 4 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index c072cada732c..eae6cb907003 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -25,10 +25,6 @@ v 8.7.0 (unreleased)
- 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
-
-v 8.6.2 (unreleased)
- - Comments on confidential issues don't show up in activity feed to non-members
- - Fix NoMethodError when visiting CI root path at `/ci`
- Fix creation of merge requests for orphaned branches (Stan Hu)
- Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla)
- Remove "Congratulations!" tweet button on newly-created project. (Connor Shea)
--
GitLab
From 99067a505ca62dc069189118d7d4c4e91de83917 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 8 Apr 2016 18:06:36 -0300
Subject: [PATCH 27/34] Fix schema.rb
---
db/schema.rb | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/db/schema.rb b/db/schema.rb
index 4c7673511fbf..a66274dc5a1b 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20160331133914) do
+ActiveRecord::Schema.define(version: 20160331223143) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -799,9 +799,9 @@
t.string "type"
t.string "title"
t.integer "project_id"
- t.datetime "created_at"
- t.datetime "updated_at"
- t.boolean "active", default: false, null: false
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ t.boolean "active", null: false
t.text "properties"
t.boolean "template", default: false
t.boolean "push_events", default: true
--
GitLab
From 73fdd4b83d76998fef9770dbeaf05981d4500b8c Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 11 Apr 2016 10:23:40 -0300
Subject: [PATCH 28/34] Use Hash instead of Array on NotificationSetting#level
enum
---
app/models/notification_setting.rb | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
index d89194b5a128..5001738f411f 100644
--- a/app/models/notification_setting.rb
+++ b/app/models/notification_setting.rb
@@ -1,7 +1,5 @@
class NotificationSetting < ActiveRecord::Base
- # Notification level
- # Note: When adding an option, it MUST go on the end of the array.
- enum level: [:disabled, :participating, :watch, :global, :mention]
+ enum level: { disabled: 0, participating: 1, watch: 2, global: 3, mention: 4 }
default_value_for :level, NotificationSetting.levels[:global]
--
GitLab
From 93a10f17e0c84074580eaf1b101af2a0fffd19ed Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 11 Apr 2016 18:23:12 -0300
Subject: [PATCH 29/34] Reuse `User#notification_settings_for` when it's
possible
---
app/controllers/groups/notification_settings_controller.rb | 2 +-
.../projects/notification_settings_controller.rb | 7 +++----
app/models/member.rb | 2 +-
app/services/notification_service.rb | 4 ++--
spec/services/notification_service_spec.rb | 4 ++--
5 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/app/controllers/groups/notification_settings_controller.rb b/app/controllers/groups/notification_settings_controller.rb
index 20405a051906..1b46f26a3781 100644
--- a/app/controllers/groups/notification_settings_controller.rb
+++ b/app/controllers/groups/notification_settings_controller.rb
@@ -1,6 +1,6 @@
class Groups::NotificationSettingsController < Groups::ApplicationController
def update
- notification_setting = group.notification_settings.find_by(user_id: current_user)
+ notification_setting = current_user.notification_settings_for(group)
saved = notification_setting.update_attributes(notification_setting_params)
render json: { saved: saved }
diff --git a/app/controllers/projects/notification_settings_controller.rb b/app/controllers/projects/notification_settings_controller.rb
index da9034380afe..90d294a46246 100644
--- a/app/controllers/projects/notification_settings_controller.rb
+++ b/app/controllers/projects/notification_settings_controller.rb
@@ -1,14 +1,13 @@
class Projects::NotificationSettingsController < Projects::ApplicationController
def create
- notification_setting = project.notification_settings.new(notification_setting_params)
- notification_setting.user = current_user
- saved = notification_setting.save
+ notification_setting = current_user.notification_settings_for(project)
+ saved = notification_setting.update_attributes(notification_setting_params)
render json: { saved: saved }
end
def update
- notification_setting = project.notification_settings.find_by(user_id: current_user)
+ notification_setting = current_user.notification_settings_for(project)
saved = notification_setting.update_attributes(notification_setting_params)
render json: { saved: saved }
diff --git a/app/models/member.rb b/app/models/member.rb
index 7d5af1d5c8af..60efafef2117 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -167,7 +167,7 @@ def create_notification_setting
end
def notification_setting
- @notification_setting ||= user.notification_settings.find_by(source: source)
+ @notification_setting ||= user.notification_settings_for(source)
end
private
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 0928dda349ef..42ec1ac9e1a1 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -355,10 +355,10 @@ def reject_users(users, level, project = nil)
users.reject do |user|
next user.notification_level == level unless project
- setting = user.notification_settings.find_by(source: project)
+ setting = user.notification_settings_for(project)
if !setting && project.group
- setting = user.notification_settings.find_by(source: project.group)
+ setting = user.notification_settings_for(project.group)
end
# reject users who globally set mention notification and has no setting per project/group
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index c4d52584a4b7..d7c72dc08118 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -89,8 +89,8 @@
note.project.group.add_user(@u_watcher, GroupMember::MASTER)
note.project.save
- @u_watcher.notification_settings.find_by(source: note.project).participating!
- @u_watcher.notification_settings.find_by(source: note.project.group).global!
+ @u_watcher.notification_settings_for(note.project).participating!
+ @u_watcher.notification_settings_for(note.project.group).global!
ActionMailer::Base.deliveries.clear
end
--
GitLab
From bee28e1785ad7844bd518c19106beee7d8a4c560 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 11 Apr 2016 18:57:18 -0300
Subject: [PATCH 30/34] Requires user to be signed in when changing
notification settings
---
.../notification_settings_controller.rb | 2 ++
.../notification_settings_controller.rb | 2 ++
.../notification_settings_controller_spec.rb | 17 ++++++++++
.../notification_settings_controller_spec.rb | 31 +++++++++++++++++++
4 files changed, 52 insertions(+)
create mode 100644 spec/controllers/groups/notification_settings_controller_spec.rb
create mode 100644 spec/controllers/projects/notification_settings_controller_spec.rb
diff --git a/app/controllers/groups/notification_settings_controller.rb b/app/controllers/groups/notification_settings_controller.rb
index 1b46f26a3781..de13b16ccf21 100644
--- a/app/controllers/groups/notification_settings_controller.rb
+++ b/app/controllers/groups/notification_settings_controller.rb
@@ -1,4 +1,6 @@
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)
diff --git a/app/controllers/projects/notification_settings_controller.rb b/app/controllers/projects/notification_settings_controller.rb
index 90d294a46246..e536725c5b11 100644
--- a/app/controllers/projects/notification_settings_controller.rb
+++ b/app/controllers/projects/notification_settings_controller.rb
@@ -1,4 +1,6 @@
class Projects::NotificationSettingsController < Projects::ApplicationController
+ before_action :authenticate_user!
+
def create
notification_setting = current_user.notification_settings_for(project)
saved = notification_setting.update_attributes(notification_setting_params)
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 000000000000..3572535d61c1
--- /dev/null
+++ b/spec/controllers/groups/notification_settings_controller_spec.rb
@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe Groups::NotificationSettingsController do
+ let(:group) { create(:group) }
+
+ 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: NotificationSetting.levels[:participating] }
+
+ expect(response).to redirect_to(new_user_session_path)
+ 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 000000000000..7e32a75b8125
--- /dev/null
+++ b/spec/controllers/projects/notification_settings_controller_spec.rb
@@ -0,0 +1,31 @@
+require 'spec_helper'
+
+describe Projects::NotificationSettingsController do
+ let(:project) { create(:empty_project) }
+
+ describe '#create' do
+ context 'when not authorized' do
+ it 'redirects to sign in page' do
+ post :create,
+ namespace_id: project.namespace.to_param,
+ project_id: project.to_param,
+ notification_setting: { level: NotificationSetting.levels[:participating] }
+
+ expect(response).to redirect_to(new_user_session_path)
+ end
+ end
+ 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: NotificationSetting.levels[:participating] }
+
+ expect(response).to redirect_to(new_user_session_path)
+ end
+ end
+ end
+end
--
GitLab
From fe58c1f13cc0758bbbd8f85b8794b458b3a72b55 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 11 Apr 2016 19:29:31 -0300
Subject: [PATCH 31/34] Fix partial for update project notifications
---
app/views/projects/buttons/_notifications.html.haml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/views/projects/buttons/_notifications.html.haml b/app/views/projects/buttons/_notifications.html.haml
index 2b9d8f2ac816..49f541399f24 100644
--- a/app/views/projects/buttons/_notifications.html.haml
+++ b/app/views/projects/buttons/_notifications.html.haml
@@ -1,5 +1,5 @@
- if @notification_setting
- = form_for [@project.namespace.becomes(Namespace), @project, @notification_setting], remote: true, html: { class: 'inline', id: 'notification-form' } do |f|
+ = form_for @notification_setting, url: namespace_project_notification_setting_path(@project.namespace.becomes(Namespace), @project), 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"}
--
GitLab
From ef22b76b732c2bf4ce52b8a73570ac2921f9caa4 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 11 Apr 2016 19:33:26 -0300
Subject: [PATCH 32/34] Simplify Projects::NotificationSettingsController
---
.../projects/notification_settings_controller.rb | 7 -------
app/views/projects/buttons/_notifications.html.haml | 2 +-
config/routes.rb | 2 +-
3 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/app/controllers/projects/notification_settings_controller.rb b/app/controllers/projects/notification_settings_controller.rb
index e536725c5b11..7d81cc03c73f 100644
--- a/app/controllers/projects/notification_settings_controller.rb
+++ b/app/controllers/projects/notification_settings_controller.rb
@@ -1,13 +1,6 @@
class Projects::NotificationSettingsController < Projects::ApplicationController
before_action :authenticate_user!
- def create
- notification_setting = current_user.notification_settings_for(project)
- saved = notification_setting.update_attributes(notification_setting_params)
-
- render json: { saved: saved }
- end
-
def update
notification_setting = current_user.notification_settings_for(project)
saved = notification_setting.update_attributes(notification_setting_params)
diff --git a/app/views/projects/buttons/_notifications.html.haml b/app/views/projects/buttons/_notifications.html.haml
index 49f541399f24..c1e3e5b73a2c 100644
--- a/app/views/projects/buttons/_notifications.html.haml
+++ b/app/views/projects/buttons/_notifications.html.haml
@@ -1,5 +1,5 @@
- if @notification_setting
- = form_for @notification_setting, url: namespace_project_notification_setting_path(@project.namespace.becomes(Namespace), @project), remote: true, html: { class: 'inline', id: 'notification-form' } do |f|
+ = 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"}
diff --git a/config/routes.rb b/config/routes.rb
index 552385110dd2..48601b7567b0 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -608,7 +608,7 @@
resources :forks, only: [:index, :new, :create]
resource :import, only: [:new, :create, :show]
- resource :notification_setting, only: [:create, :update]
+ resource :notification_setting, only: [:update]
resources :refs, only: [] do
collection do
--
GitLab
From aabb466e5b35477b39cc57642083df361cd5d112 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 11 Apr 2016 19:54:13 -0300
Subject: [PATCH 33/34] Improve specs for group/project notification controller
---
.../notification_settings_controller_spec.rb | 17 +++++++-
.../notification_settings_controller_spec.rb | 39 ++++++++++++++++++-
2 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/spec/controllers/groups/notification_settings_controller_spec.rb b/spec/controllers/groups/notification_settings_controller_spec.rb
index 3572535d61c1..0786e45515a5 100644
--- a/spec/controllers/groups/notification_settings_controller_spec.rb
+++ b/spec/controllers/groups/notification_settings_controller_spec.rb
@@ -2,16 +2,31 @@
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: NotificationSetting.levels[:participating] }
+ 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
index 7e32a75b8125..385877a26df6 100644
--- a/spec/controllers/projects/notification_settings_controller_spec.rb
+++ b/spec/controllers/projects/notification_settings_controller_spec.rb
@@ -2,6 +2,11 @@
describe Projects::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
@@ -9,11 +14,26 @@
post :create,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
- notification_setting: { level: NotificationSetting.levels[:participating] }
+ 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,
+ namespace_id: project.namespace.to_param,
+ project_id: project.to_param,
+ notification_setting: { level: :participating }
+
+ expect(response.status).to eq 200
+ end
+ end
end
describe '#update' do
@@ -22,10 +42,25 @@
put :update,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
- notification_setting: { level: NotificationSetting.levels[:participating] }
+ 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
--
GitLab
From 61a62e00e3b08e6ed962b029564e3a2446e169fd Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Tue, 12 Apr 2016 12:57:39 -0300
Subject: [PATCH 34/34] Fix specs for Projects::NotificationSettingsController
---
.../notification_settings_controller_spec.rb | 28 -------------------
1 file changed, 28 deletions(-)
diff --git a/spec/controllers/projects/notification_settings_controller_spec.rb b/spec/controllers/projects/notification_settings_controller_spec.rb
index 385877a26df6..4908b5456487 100644
--- a/spec/controllers/projects/notification_settings_controller_spec.rb
+++ b/spec/controllers/projects/notification_settings_controller_spec.rb
@@ -8,34 +8,6 @@
project.team << [user, :developer]
end
- describe '#create' do
- context 'when not authorized' do
- it 'redirects to sign in page' do
- post :create,
- 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
- post :create,
- namespace_id: project.namespace.to_param,
- project_id: project.to_param,
- notification_setting: { level: :participating }
-
- expect(response.status).to eq 200
- end
- end
- end
-
describe '#update' do
context 'when not authorized' do
it 'redirects to sign in page' do
--
GitLab