Commit 39ead205 authored by Felipe Artur's avatar Felipe Artur

Remove notification level fild from users, improve migrations and specs

parent 8f6d43e0
......@@ -17,21 +17,18 @@ class Profiles::NotificationsController < Profiles::ApplicationController
end
def user_params
params.require(:user).permit(:notification_email, :notification_level)
params.require(:user).permit(:notification_email)
end
def notification_params
params.require(:notification_level)
def global_notification_setting_params
params.require(:global_notification_setting).permit(:level)
end
private
def update_notification_settings
return true unless notification_params
return true unless global_notification_setting_params
notification_setting = current_user.global_notification_setting
notification_setting.level = notification_params
notification_setting.save
current_user.global_notification_setting.update_attributes(global_notification_setting_params)
end
end
......@@ -71,26 +71,13 @@ module NotificationsHelper
html << content_tag(:div, class: "radio") do
content_tag(:label, { value: level }) do
radio_button_tag(:notification_level, level, @global_notification_setting.level.to_sym == level) +
radio_button_tag(:"global_notification_setting[level]", level, @global_notification_setting.level.to_sym == level) +
content_tag(:div, level.to_s.capitalize, class: "level-title") +
content_tag(:p, notification_level_description(level))
content_tag(:p, notification_description(level))
end
end
end
html.html_safe
end
def notification_level_description(level)
case level
when :disabled
"You will not get any notifications via email"
when :mention
"You will receive notifications only for comments in which you were @mentioned"
when :participating
"You will only receive notifications from related resources (e.g. from your commits or assigned issues)"
when :watch
"You will receive notifications for any activity"
end
end
end
......@@ -797,9 +797,12 @@ class User < ActiveRecord::Base
# Lazy load global notification setting
# Initializes User setting with Participating level if setting not persisted
def global_notification_setting
setting = notification_settings.find_or_initialize_by(source: nil)
setting.level = NotificationSetting.levels[DEFAULT_NOTIFICATION_LEVEL] unless setting.persisted?
setting
return @global_notification_setting if defined?(@global_notification_setting)
@global_notification_setting = notification_settings.find_or_initialize_by(source: nil)
@global_notification_setting.update_attributes(level: NotificationSetting.levels[DEFAULT_NOTIFICATION_LEVEL]) unless @global_notification_setting.persisted?
@global_notification_setting
end
def assigned_open_merge_request_count(force: false)
......
......@@ -353,7 +353,9 @@ class NotificationService
users = users.reject(&:blocked?)
users.reject do |user|
next user.global_notification_setting.level == level unless project
global_notification_setting = user.global_notification_setting
next global_notification_setting.level == level unless project
setting = user.notification_settings_for(project)
......@@ -362,13 +364,13 @@ class NotificationService
end
# reject users who globally set mention notification and has no setting per project/group
next user.global_notification_setting.level == level unless setting
next global_notification_setting.level == level unless setting
# reject users who set mention notification in project
next true if setting.level == level
# reject users who have mention level in project and disabled in global settings
setting.global? && user.global_notification_setting.level == level
setting.global? && global_notification_setting.level == level
end
end
......
class MigrateUsersNotificationLevel < ActiveRecord::Migration
# Migrates only users which changes theier default notification level :participating
# creating a new record on notification settins table
def up
changed_users = exec_query(%Q{
SELECT id, notification_level
FROM users
WHERE notification_level != 1
})
changed_users.each do |row|
uid = row['id']
u_notification_level = row['notification_level']
execute(%Q{
INSERT INTO notification_settings
(user_id, level, created_at, updated_at)
VALUES
(#{uid}, #{u_notification_level}, now(), now())
})
end
end
end
......@@ -2,6 +2,10 @@ class RemoveNotificationSettingNotNullConstraints < ActiveRecord::Migration
def up
change_column :notification_settings, :source_type, :string, null: true
change_column :notification_settings, :source_id, :integer, null: true
change_column :users, :notification_level, :integer, null: true
end
def down
change_column :notification_settings, :source_type, :string, null: false
change_column :notification_settings, :source_id, :integer, null: false
end
end
class MigrateUsersNotificationLevel < ActiveRecord::Migration
# Migrates only users who changed their default notification level :participating
# creating a new record on notification settings table
def up
execute(%Q{
INSERT INTO notification_settings
(user_id, level, created_at, updated_at)
(SELECT id, notification_level, created_at, updated_at FROM users WHERE notification_level != 1)
})
end
# Migrates from notification settings back to user notification_level
# If no value is found the default level of 1 will be used
def down
execute(%Q{
UPDATE users u SET
notification_level = COALESCE((SELECT level FROM notification_settings WHERE user_id = u.id AND source_type IS NULL), 1)
})
end
end
class RemoveNotificationLevelFromUsers < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
def change
remove_column :users, :notification_level, :integer
end
end
......@@ -85,13 +85,14 @@ describe NotificationService, services: true do
context 'participating' do
context 'by note' do
before do
ActionMailer::Base.deliveries.clear
note.author = @u_lazy_participant
note.save
notification.new_note(note)
end
it { should_email(@u_lazy_participant) }
it { should_not_email(@u_lazy_participant) }
end
end
end
......@@ -953,8 +954,8 @@ describe NotificationService, services: true do
def add_users_with_subscription(project, issuable)
@subscriber = create :user
@unsubscriber = create :user
@subscribed_participant = create(:user, username: 'subscribed_participant', notification_level: :participating)
@watcher_and_subscriber = create(:user, notification_level: :watch)
@subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating)
@watcher_and_subscriber = create_global_setting_for(create(:user), :watch)
project.team << [@subscribed_participant, :master]
project.team << [@subscriber, :master]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment