Commit 7cad597f authored by Stan Hu's avatar Stan Hu

Revert "Merge branch '8836-mr-revert' into 'master'

This reverts commit 68e40bd4, reversing
changes made to 2d1f823b.
parent 4bf4612c
......@@ -25,6 +25,7 @@
bindEvents() {
$('.js-preferences-form').on('change.preference', 'input[type=radio]', 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:complete', this.afterUpdateUsername);
$('.update-notifications').on('ajax:success', this.onUpdateNotifs);
......
......@@ -17,6 +17,6 @@ class Profiles::NotificationsController < Profiles::ApplicationController
end
def user_params
params.require(:user).permit(:notification_email)
params.require(:user).permit(:notification_email, :notified_of_own_activity)
end
end
......@@ -217,7 +217,7 @@ class NotificationService
recipients = reject_unsubscribed_users(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
notify_method = "note_#{note.to_ability_name}_email".to_sym
......@@ -327,8 +327,9 @@ class NotificationService
recipients ||= build_recipients(
pipeline,
pipeline.project,
nil, # The acting user, who won't be added to recipients
action: pipeline.status).map(&:notification_email)
pipeline.user,
action: pipeline.status,
skip_current_user: false).map(&:notification_email)
if recipients.any?
mailer.public_send(email_template, pipeline, recipients).deliver_later
......@@ -627,7 +628,7 @@ class NotificationService
recipients = reject_unsubscribed_users(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
end
......@@ -636,7 +637,7 @@ class NotificationService
recipients = add_labels_subscribers([], project, target, labels: labels)
recipients = reject_unsubscribed_users(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
end
......
......@@ -34,6 +34,11 @@
.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
%h5
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
......@@ -1318,6 +1318,7 @@ ActiveRecord::Schema.define(version: 20170315174634) do
t.string "incoming_email_token"
t.string "organization"
t.boolean "authorized_projects_populated"
t.boolean "notified_of_own_activity", default: false, null: false
t.boolean "ghost"
end
......
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
should_not_email(@u_lazy_participant)
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
mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author)
......@@ -476,6 +486,20 @@ describe NotificationService, services: true do
should_not_email(issue.assignee)
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
user_1 = create(:user)
user_2 = create(:user)
......@@ -665,6 +689,19 @@ describe NotificationService, services: true do
should_email(subscriber_to_label_2)
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
notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled)
......@@ -818,6 +855,20 @@ describe NotificationService, services: true do
should_not_email(@u_lazy_participant)
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
user_1 = create(:user)
user_2 = create(:user)
......@@ -1013,6 +1064,14 @@ describe NotificationService, services: true do
should_not_email(@u_watcher)
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
let(:participant) { create(:user, username: 'user-participant') }
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