Commit aeff506a authored by Robert Speicher's avatar Robert Speicher

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

Bring back option to be notified of own activity

See merge request !10032
parents 8bddf23e c8ad3346
...@@ -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
...@@ -22,6 +22,7 @@ class User < ActiveRecord::Base ...@@ -22,6 +22,7 @@ class User < ActiveRecord::Base
default_value_for :hide_no_ssh_key, false default_value_for :hide_no_ssh_key, false
default_value_for :hide_no_password, false default_value_for :hide_no_password, false
default_value_for :project_view, :files default_value_for :project_view, :files
default_value_for :notified_of_own_activity, false
attr_encrypted :otp_secret, attr_encrypted :otp_secret,
key: Gitlab::Application.secrets.otp_key_base, key: Gitlab::Application.secrets.otp_key_base,
......
...@@ -38,7 +38,7 @@ class NotificationRecipientService ...@@ -38,7 +38,7 @@ class NotificationRecipientService
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
...@@ -47,7 +47,7 @@ class NotificationRecipientService ...@@ -47,7 +47,7 @@ class NotificationRecipientService
recipients = add_labels_subscribers([], target, labels: labels) recipients = add_labels_subscribers([], 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
...@@ -88,7 +88,7 @@ class NotificationRecipientService ...@@ -88,7 +88,7 @@ class NotificationRecipientService
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.uniq recipients.uniq
end end
......
...@@ -280,8 +280,9 @@ class NotificationService ...@@ -280,8 +280,9 @@ class NotificationService
recipients ||= NotificationRecipientService.new(pipeline.project).build_recipients( recipients ||= NotificationRecipientService.new(pipeline.project).build_recipients(
pipeline, pipeline,
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
......
...@@ -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: 10032
author: Richard Macklin
class ReaddNotifiedOfOwnActivityToUsers < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
def change
add_column :users, :notified_of_own_activity, :boolean
end
end
...@@ -1236,6 +1236,7 @@ ActiveRecord::Schema.define(version: 20170317203554) do ...@@ -1236,6 +1236,7 @@ ActiveRecord::Schema.define(version: 20170317203554) do
t.string "organization" t.string "organization"
t.boolean "authorized_projects_populated" t.boolean "authorized_projects_populated"
t.boolean "ghost" t.boolean "ghost"
t.boolean "notified_of_own_activity"
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.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)
...@@ -845,6 +882,20 @@ describe NotificationService, services: true do ...@@ -845,6 +882,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)
...@@ -1040,6 +1091,14 @@ describe NotificationService, services: true do ...@@ -1040,6 +1091,14 @@ describe NotificationService, services: true do
should_not_email(@u_watcher) should_not_email(@u_watcher)
end end
it "notifies the merger when the pipeline succeeds is false but they've opted into notifications about their activity" do
merge_request.merge_when_pipeline_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