Commit 5e966688 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'option-to-be-notified-of-own-activity' into 'master'

Add option to receive email notifications about your own activity

See merge request !8836
parents b35378a9 9493791d
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
bindEvents() { bindEvents() {
$('.js-preferences-form').on('change.preference', 'input[type=radio]', this.submitForm); $('.js-preferences-form').on('change.preference', 'input[type=radio]', this.submitForm);
$('#user_notification_email').on('change', this.submitForm); $('#user_notification_email').on('change', this.submitForm);
$('#user_notified_of_own_activity').on('change', this.submitForm);
$('.update-username').on('ajax:before', this.beforeUpdateUsername); $('.update-username').on('ajax:before', this.beforeUpdateUsername);
$('.update-username').on('ajax:complete', this.afterUpdateUsername); $('.update-username').on('ajax:complete', this.afterUpdateUsername);
$('.update-notifications').on('ajax:success', this.onUpdateNotifs); $('.update-notifications').on('ajax:success', this.onUpdateNotifs);
......
...@@ -17,6 +17,6 @@ class Profiles::NotificationsController < Profiles::ApplicationController ...@@ -17,6 +17,6 @@ class Profiles::NotificationsController < Profiles::ApplicationController
end end
def user_params def user_params
params.require(:user).permit(:notification_email) params.require(:user).permit(:notification_email, :notified_of_own_activity)
end end
end end
...@@ -217,7 +217,7 @@ class NotificationService ...@@ -217,7 +217,7 @@ class NotificationService
recipients = reject_unsubscribed_users(recipients, note.noteable) recipients = reject_unsubscribed_users(recipients, note.noteable)
recipients = reject_users_without_access(recipients, note.noteable) recipients = reject_users_without_access(recipients, note.noteable)
recipients.delete(note.author) recipients.delete(note.author) unless note.author.notified_of_own_activity?
recipients = recipients.uniq recipients = recipients.uniq
notify_method = "note_#{note.to_ability_name}_email".to_sym notify_method = "note_#{note.to_ability_name}_email".to_sym
...@@ -327,8 +327,9 @@ class NotificationService ...@@ -327,8 +327,9 @@ class NotificationService
recipients ||= build_recipients( recipients ||= build_recipients(
pipeline, pipeline,
pipeline.project, pipeline.project,
nil, # The acting user, who won't be added to recipients pipeline.user,
action: pipeline.status).map(&:notification_email) action: pipeline.status,
skip_current_user: false).map(&:notification_email)
if recipients.any? if recipients.any?
mailer.public_send(email_template, pipeline, recipients).deliver_later mailer.public_send(email_template, pipeline, recipients).deliver_later
...@@ -627,7 +628,7 @@ class NotificationService ...@@ -627,7 +628,7 @@ class NotificationService
recipients = reject_unsubscribed_users(recipients, target) recipients = reject_unsubscribed_users(recipients, target)
recipients = reject_users_without_access(recipients, target) recipients = reject_users_without_access(recipients, target)
recipients.delete(current_user) if skip_current_user recipients.delete(current_user) if skip_current_user && !current_user.notified_of_own_activity?
recipients.uniq recipients.uniq
end end
...@@ -636,7 +637,7 @@ class NotificationService ...@@ -636,7 +637,7 @@ class NotificationService
recipients = add_labels_subscribers([], project, target, labels: labels) recipients = add_labels_subscribers([], project, target, labels: labels)
recipients = reject_unsubscribed_users(recipients, target) recipients = reject_unsubscribed_users(recipients, target)
recipients = reject_users_without_access(recipients, target) recipients = reject_users_without_access(recipients, target)
recipients.delete(current_user) recipients.delete(current_user) unless current_user.notified_of_own_activity?
recipients.uniq recipients.uniq
end end
......
...@@ -34,6 +34,11 @@ ...@@ -34,6 +34,11 @@
.clearfix .clearfix
= form_for @user, url: profile_notifications_path, method: :put do |f|
%label{ for: 'user_notified_of_own_activity' }
= f.check_box :notified_of_own_activity
%span Receive notifications about your own activity
%hr %hr
%h5 %h5
Groups (#{@group_notifications.count}) Groups (#{@group_notifications.count})
......
---
title: Add option to receive email notifications about your own activity
merge_request: 8836
author: Richard Macklin
class AddNotifiedOfOwnActivityToUsers < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
def up
add_column_with_default :users, :notified_of_own_activity, :boolean, default: false
end
def down
remove_column :users, :notified_of_own_activity
end
end
...@@ -1279,6 +1279,7 @@ ActiveRecord::Schema.define(version: 20170210075922) do ...@@ -1279,6 +1279,7 @@ ActiveRecord::Schema.define(version: 20170210075922) do
t.string "organization" t.string "organization"
t.string "incoming_email_token" t.string "incoming_email_token"
t.boolean "authorized_projects_populated" t.boolean "authorized_projects_populated"
t.boolean "notified_of_own_activity", default: false, null: false
end end
add_index "users", ["admin"], name: "index_users_on_admin", using: :btree add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
......
require 'spec_helper'
describe Profiles::NotificationsController do
let(:user) do
create(:user) do |user|
user.emails.create(email: 'original@example.com')
user.emails.create(email: 'new@example.com')
user.update(notification_email: 'original@example.com')
user.save!
end
end
describe 'GET show' do
it 'renders' do
sign_in(user)
get :show
expect(response).to render_template :show
end
end
describe 'POST update' do
it 'updates only permitted attributes' do
sign_in(user)
put :update, user: { notification_email: 'new@example.com', notified_of_own_activity: true, admin: true }
user.reload
expect(user.notification_email).to eq('new@example.com')
expect(user.notified_of_own_activity).to eq(true)
expect(user.admin).to eq(false)
expect(controller).to set_flash[:notice].to('Notification settings saved')
end
it 'shows an error message if the params are invalid' do
sign_in(user)
put :update, user: { notification_email: '' }
expect(user.reload.notification_email).to eq('original@example.com')
expect(controller).to set_flash[:alert].to('Failed to save new settings')
end
end
end
require 'spec_helper'
feature 'Profile > Notifications > User changes notified_of_own_activity setting', feature: true, js: true do
let(:user) { create(:user) }
before do
login_as(user)
end
scenario 'User opts into receiving notifications about their own activity' do
visit profile_notifications_path
expect(page).not_to have_checked_field('user[notified_of_own_activity]')
check 'user[notified_of_own_activity]'
expect(page).to have_content('Notification settings saved')
expect(page).to have_checked_field('user[notified_of_own_activity]')
end
scenario 'User opts out of receiving notifications about their own activity' do
user.update!(notified_of_own_activity: true)
visit profile_notifications_path
expect(page).to have_checked_field('user[notified_of_own_activity]')
uncheck 'user[notified_of_own_activity]'
expect(page).to have_content('Notification settings saved')
expect(page).not_to have_checked_field('user[notified_of_own_activity]')
end
end
...@@ -146,6 +146,16 @@ describe NotificationService, services: true do ...@@ -146,6 +146,16 @@ describe NotificationService, services: true do
should_not_email(@u_lazy_participant) should_not_email(@u_lazy_participant)
end end
it "emails the note author if they've opted into notifications about their activity" do
add_users_with_subscription(note.project, issue)
note.author.notified_of_own_activity = true
reset_delivered_emails!
notification.new_note(note)
should_email(note.author)
end
it 'filters out "mentioned in" notes' do it 'filters out "mentioned in" notes' do
mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author) mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author)
...@@ -476,6 +486,20 @@ describe NotificationService, services: true do ...@@ -476,6 +486,20 @@ describe NotificationService, services: true do
should_not_email(issue.assignee) should_not_email(issue.assignee)
end end
it "emails the author if they've opted into notifications about their activity" do
issue.author.notified_of_own_activity = true
notification.new_issue(issue, issue.author)
should_email(issue.author)
end
it "doesn't email the author if they haven't opted into notifications about their activity" do
notification.new_issue(issue, issue.author)
should_not_email(issue.author)
end
it "emails subscribers of the issue's labels" do it "emails subscribers of the issue's labels" do
user_1 = create(:user) user_1 = create(:user)
user_2 = create(:user) user_2 = create(:user)
...@@ -665,6 +689,19 @@ describe NotificationService, services: true do ...@@ -665,6 +689,19 @@ describe NotificationService, services: true do
should_email(subscriber_to_label_2) should_email(subscriber_to_label_2)
end end
it "emails the current user if they've opted into notifications about their activity" do
subscriber_to_label_2.notified_of_own_activity = true
notification.relabeled_issue(issue, [group_label_2, label_2], subscriber_to_label_2)
should_email(subscriber_to_label_2)
end
it "doesn't email the current user if they haven't opted into notifications about their activity" do
notification.relabeled_issue(issue, [group_label_2, label_2], subscriber_to_label_2)
should_not_email(subscriber_to_label_2)
end
it "doesn't send email to anyone but subscribers of the given labels" do it "doesn't send email to anyone but subscribers of the given labels" do
notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled) notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled)
...@@ -818,6 +855,20 @@ describe NotificationService, services: true do ...@@ -818,6 +855,20 @@ describe NotificationService, services: true do
should_not_email(@u_lazy_participant) should_not_email(@u_lazy_participant)
end end
it "emails the author if they've opted into notifications about their activity" do
merge_request.author.notified_of_own_activity = true
notification.new_merge_request(merge_request, merge_request.author)
should_email(merge_request.author)
end
it "doesn't email the author if they haven't opted into notifications about their activity" do
notification.new_merge_request(merge_request, merge_request.author)
should_not_email(merge_request.author)
end
it "emails subscribers of the merge request's labels" do it "emails subscribers of the merge request's labels" do
user_1 = create(:user) user_1 = create(:user)
user_2 = create(:user) user_2 = create(:user)
...@@ -1013,6 +1064,14 @@ describe NotificationService, services: true do ...@@ -1013,6 +1064,14 @@ describe NotificationService, services: true do
should_not_email(@u_watcher) should_not_email(@u_watcher)
end end
it "notifies the merger when merge_when_build_succeeds is false but they've opted into notifications about their activity" do
merge_request.merge_when_build_succeeds = false
@u_watcher.notified_of_own_activity = true
notification.merge_mr(merge_request, @u_watcher)
should_email(@u_watcher)
end
it_behaves_like 'participating notifications' do it_behaves_like 'participating notifications' do
let(:participant) { create(:user, username: 'user-participant') } let(:participant) { create(:user, username: 'user-participant') }
let(:issuable) { merge_request } let(:issuable) { merge_request }
......
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