Merge branch 'issue_12758' into 'master'

Implement custom notification level options

![Screen_Shot_2016-06-17_at_15.31.43](/uploads/3fc47d2f461b3e8b67bb8acaa304cf99/Screen_Shot_2016-06-17_at_15.31.43.png)

![Screenshot_from_2016-06-15_10-52-27](/uploads/88dbdd21d97e80ee772fe08fa0c9b393/Screenshot_from_2016-06-15_10-52-27.png)

part of #12758 

See merge request !4389

6 files changed, 268 insertions, 96 deletions

diff --git a/spec/controllers/groups/notification_settings_controller_spec.rb b/spec/controllers/groups/notification_settings_controller_spec.rb
deleted file mode 100644
index 0786e45515a..00000000000
--- a/spec/controllers/groups/notification_settings_controller_spec.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-require 'spec_helper'
-
-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: :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/notification_settings_controller_spec.rb b/spec/controllers/notification_settings_controller_spec.rb
new file mode 100644
index 00000000000..15d155833b4
--- /dev/null
+++ b/spec/controllers/notification_settings_controller_spec.rb
@@ -0,0 +1,125 @@
+require 'spec_helper'
+
+describe 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
+      it 'redirects to sign in page' do
+        post :create,
+             project: { id: project.id },
+             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,
+             project: { id: project.id },
+             notification_setting: { level: :participating }
+
+        expect(response.status).to eq 200
+      end
+
+      context 'and setting custom notification setting' do
+        let(:custom_events) do
+          events = {}
+
+          NotificationSetting::EMAIL_EVENTS.each do |event|
+            events[event] = "true"
+          end
+        end
+
+        it 'returns success' do
+          post :create,
+               project: { id: project.id },
+               notification_setting: { level: :participating, events: custom_events }
+
+          expect(response.status).to eq 200
+        end
+      end
+    end
+
+    context 'not authorized' do
+      let(:private_project) { create(:project, :private) }
+      before { sign_in(user) }
+
+      it 'returns 404' do
+        post :create,
+             project: { id: private_project.id },
+             notification_setting: { level: :participating }
+
+        expect(response.status).to eq(404)
+      end
+    end
+  end
+
+  describe '#update' do
+    let(:notification_setting) { user.global_notification_setting }
+
+    context 'when not authorized' do
+      it 'redirects to sign in page' do
+        put :update,
+            id: notification_setting,
+            notification_setting: { level: :participating }
+
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+
+    context 'when authorized' do
+      before{ sign_in(user) }
+
+      it 'returns success' do
+        put :update,
+            id: notification_setting,
+            notification_setting: { level: :participating }
+
+        expect(response.status).to eq 200
+      end
+
+      context 'and setting custom notification setting' do
+        let(:custom_events) do
+          events = {}
+
+          NotificationSetting::EMAIL_EVENTS.each do |event|
+            events[event] = "true"
+          end
+        end
+
+        it 'returns success' do
+          put :update,
+              id: notification_setting,
+              notification_setting: { level: :participating, events: custom_events }
+
+          expect(response.status).to eq 200
+        end
+      end
+    end
+
+    context 'not authorized' do
+      let(:other_user) { create(:user) }
+
+      before { sign_in(other_user) }
+
+      it 'returns 404' do
+        put :update,
+            id: notification_setting,
+            notification_setting: { level: :participating }
+
+        expect(response.status).to eq(404)
+      end
+    end
+  end
+end
diff --git a/spec/controllers/projects/notification_settings_controller_spec.rb b/spec/controllers/projects/notification_settings_controller_spec.rb
deleted file mode 100644
index c5d17d97ec9..00000000000
--- a/spec/controllers/projects/notification_settings_controller_spec.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-require 'spec_helper'
-
-describe Projects::NotificationSettingsController do
-  let(:project) { create(:empty_project) }
-  let(:user) { create(:user) }
-
-  before do
-    project.team << [user, :developer]
-  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: :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
-
-    context 'not authorized' do
-      let(:private_project) { create(:project, :private) }
-      before { sign_in(user) }
-
-      it 'returns 404' do
-        put :update,
-            namespace_id: private_project.namespace.to_param,
-            project_id: private_project.to_param,
-            notification_setting: { level: :participating }
-
-        expect(response.status).to eq(404)
-      end
-    end
-  end
-end
diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb
index 4e24e89b008..df336a6effe 100644
--- a/spec/models/notification_setting_spec.rb
+++ b/spec/models/notification_setting_spec.rb
@@ -12,5 +12,30 @@ RSpec.describe NotificationSetting, type: :model do
     it { is_expected.to validate_presence_of(:user) }
     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/) }
+
+    context "events" do
+      let(:user) { create(:user) }
+      let(:notification_setting) { NotificationSetting.new(source_id: 1, source_type: 'Project', user_id: user.id) }
+
+      before do
+        notification_setting.level = "custom"
+        notification_setting.new_note = "true"
+        notification_setting.new_issue = 1
+        notification_setting.close_issue = "1"
+        notification_setting.merge_merge_request = "t"
+        notification_setting.close_merge_request = "nil"
+        notification_setting.reopen_merge_request = "false"
+        notification_setting.save
+      end
+
+      it "parses boolean before saving" do
+        expect(notification_setting.new_note).to eq(true)
+        expect(notification_setting.new_issue).to eq(true)
+        expect(notification_setting.close_issue).to eq(true)
+        expect(notification_setting.merge_merge_request).to eq(true)
+        expect(notification_setting.close_merge_request).to eq(false)
+        expect(notification_setting.reopen_merge_request).to eq(false)
+      end
+    end
   end
 end
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index f167813e07d..01eb4b44b83 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -428,8 +428,9 @@ describe API::API, api: true  do
 
     describe 'permissions' do
       context 'all projects' do
-        it 'Contains permission information' do
-          project.team << [user, :master]
+        before { project.team << [user, :master] }
+
+        it 'contains permission information' do
           get api("/projects", user)
 
           expect(response.status).to eq(200)
@@ -440,7 +441,7 @@ describe API::API, api: true  do
       end
 
       context 'personal project' do
-        it 'Sets project access and returns 200' do
+        it 'sets project access and returns 200' do
           project.team << [user, :master]
           get api("/projects/#{project.id}", user)
 
@@ -452,9 +453,11 @@ describe API::API, api: true  do
       end
 
       context 'group project' do
+        let(:project2) { create(:project, group: create(:group)) }
+
+        before { project2.group.add_owner(user) }
+
         it 'should set the owner and return 200' do
-          project2 = create(:project, group: create(:group))
-          project2.group.add_owner(user)
           get api("/projects/#{project2.id}", user)
 
           expect(response.status).to eq(200)
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index e871a103d42..776a6ab5edb 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -46,6 +46,8 @@ describe NotificationService, services: true do
         project.team << [issue.assignee, :master]
         project.team << [note.author, :master]
         create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy')
+        update_custom_notification(:new_note, @u_guest_custom, project)
+        update_custom_notification(:new_note, @u_custom_global)
       end
 
       describe :new_note do
@@ -53,7 +55,7 @@ describe NotificationService, services: true do
           add_users_with_subscription(note.project, issue)
 
           # Ensure create SentNotification by noteable = issue 6 times, not noteable = note
-          expect(SentNotification).to receive(:record).with(issue, any_args).exactly(7).times
+          expect(SentNotification).to receive(:record).with(issue, any_args).exactly(8).times
 
           ActionMailer::Base.deliveries.clear
 
@@ -62,10 +64,12 @@ describe NotificationService, services: true do
           should_email(@u_watcher)
           should_email(note.noteable.author)
           should_email(note.noteable.assignee)
+          should_email(@u_custom_global)
           should_email(@u_mentioned)
           should_email(@subscriber)
           should_email(@watcher_and_subscriber)
           should_email(@subscribed_participant)
+          should_not_email(@u_guest_custom)
           should_not_email(@u_guest_watcher)
           should_not_email(note.author)
           should_not_email(@u_participating)
@@ -103,10 +107,12 @@ describe NotificationService, services: true do
         before do
           note.project.namespace_id = group.id
           note.project.group.add_user(@u_watcher, GroupMember::MASTER)
+          note.project.group.add_user(@u_custom_global, GroupMember::MASTER)
           note.project.save
 
           @u_watcher.notification_settings_for(note.project).participating!
           @u_watcher.notification_settings_for(note.project.group).global!
+          update_custom_notification(:new_note, @u_custom_global)
           ActionMailer::Base.deliveries.clear
         end
 
@@ -116,6 +122,8 @@ describe NotificationService, services: true do
           should_email(note.noteable.author)
           should_email(note.noteable.assignee)
           should_email(@u_mentioned)
+          should_email(@u_custom_global)
+          should_not_email(@u_guest_custom)
           should_not_email(@u_guest_watcher)
           should_not_email(@u_watcher)
           should_not_email(note.author)
@@ -136,6 +144,7 @@ describe NotificationService, services: true do
       let(:admin) { create(:admin) }
       let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) }
       let(:note) { create(:note_on_issue, noteable: confidential_issue, project: project, note: "#{author.to_reference} #{assignee.to_reference} #{non_member.to_reference} #{member.to_reference} #{admin.to_reference}") }
+      let(:guest_watcher) { create_user_with_notification(:watch, "guest-watcher-confidential") }
 
       it 'filters out users that can not read the issue' do
         project.team << [member, :developer]
@@ -149,6 +158,7 @@ describe NotificationService, services: true do
 
         should_not_email(non_member)
         should_not_email(guest)
+        should_not_email(guest_watcher)
         should_email(author)
         should_email(assignee)
         should_email(member)
@@ -223,6 +233,9 @@ describe NotificationService, services: true do
             should_email(member)
           end
 
+          # it emails custom global users on mention
+          should_email(@u_custom_global)
+
           should_email(@u_guest_watcher)
           should_email(note.noteable.author)
           should_not_email(note.author)
@@ -241,13 +254,16 @@ describe NotificationService, services: true do
         build_team(note.project)
         ActionMailer::Base.deliveries.clear
         allow_any_instance_of(Commit).to receive(:author).and_return(@u_committer)
+        update_custom_notification(:new_note, @u_guest_custom, project)
+        update_custom_notification(:new_note, @u_custom_global)
       end
 
       describe '#new_note, #perform_enqueued_jobs' do
         it do
           notification.new_note(note)
-
           should_email(@u_guest_watcher)
+          should_email(@u_custom_global)
+          should_email(@u_guest_custom)
           should_email(@u_committer)
           should_email(@u_watcher)
           should_not_email(@u_mentioned)
@@ -288,6 +304,8 @@ describe NotificationService, services: true do
       build_team(issue.project)
       add_users_with_subscription(issue.project, issue)
       ActionMailer::Base.deliveries.clear
+      update_custom_notification(:new_issue, @u_guest_custom, project)
+      update_custom_notification(:new_issue, @u_custom_global)
     end
 
     describe '#new_issue' do
@@ -297,6 +315,8 @@ describe NotificationService, services: true do
         should_email(issue.assignee)
         should_email(@u_watcher)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
+        should_email(@u_custom_global)
         should_email(@u_participant_mentioned)
         should_not_email(@u_mentioned)
         should_not_email(@u_participating)
@@ -345,6 +365,7 @@ describe NotificationService, services: true do
 
           notification.new_issue(confidential_issue, @u_disabled)
 
+          should_not_email(@u_guest_watcher)
           should_not_email(non_member)
           should_not_email(author)
           should_not_email(guest)
@@ -356,12 +377,20 @@ describe NotificationService, services: true do
     end
 
     describe '#reassigned_issue' do
+
+      before do
+        update_custom_notification(:reassign_issue, @u_guest_custom, project)
+        update_custom_notification(:reassign_issue, @u_custom_global)
+      end
+
       it 'emails new assignee' do
         notification.reassigned_issue(issue, @u_disabled)
 
         should_email(issue.assignee)
         should_email(@u_watcher)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
+        should_email(@u_custom_global)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(@unsubscriber)
@@ -378,8 +407,10 @@ describe NotificationService, services: true do
         should_email(@u_mentioned)
         should_email(@u_watcher)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
+        should_email(@u_custom_global)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -394,8 +425,10 @@ describe NotificationService, services: true do
         should_email(issue.assignee)
         should_email(@u_watcher)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
+        should_email(@u_custom_global)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -410,8 +443,10 @@ describe NotificationService, services: true do
         should_email(issue.assignee)
         should_email(@u_watcher)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
+        should_email(@u_custom_global)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -425,8 +460,10 @@ describe NotificationService, services: true do
         expect(issue.assignee).to be @u_mentioned
         should_email(@u_watcher)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
+        should_email(@u_custom_global)
         should_not_email(issue.assignee)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
@@ -529,6 +566,12 @@ describe NotificationService, services: true do
     end
 
     describe '#close_issue' do
+
+      before do
+        update_custom_notification(:close_issue, @u_guest_custom, project)
+        update_custom_notification(:close_issue, @u_custom_global)
+      end
+
       it 'should sent email to issue assignee and issue author' do
         notification.close_issue(issue, @u_disabled)
 
@@ -536,6 +579,8 @@ describe NotificationService, services: true do
         should_email(issue.author)
         should_email(@u_watcher)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
+        should_email(@u_custom_global)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
@@ -575,6 +620,11 @@ describe NotificationService, services: true do
     end
 
     describe '#reopen_issue' do
+      before do
+        update_custom_notification(:reopen_issue, @u_guest_custom, project)
+        update_custom_notification(:reopen_issue, @u_custom_global)
+      end
+
       it 'should send email to issue assignee and issue author' do
         notification.reopen_issue(issue, @u_disabled)
 
@@ -582,6 +632,8 @@ describe NotificationService, services: true do
         should_email(issue.author)
         should_email(@u_watcher)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
+        should_email(@u_custom_global)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
@@ -631,6 +683,11 @@ describe NotificationService, services: true do
     end
 
     describe '#new_merge_request' do
+      before do
+        update_custom_notification(:new_merge_request, @u_guest_custom, project)
+        update_custom_notification(:new_merge_request, @u_custom_global)
+      end
+
       it do
         notification.new_merge_request(merge_request, @u_disabled)
 
@@ -639,6 +696,8 @@ describe NotificationService, services: true do
         should_email(@watcher_and_subscriber)
         should_email(@u_participant_mentioned)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
+        should_email(@u_custom_global)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
         should_not_email(@u_lazy_participant)
@@ -685,6 +744,11 @@ describe NotificationService, services: true do
     end
 
     describe '#reassigned_merge_request' do
+      before do
+        update_custom_notification(:reassign_merge_request, @u_guest_custom, project)
+        update_custom_notification(:reassign_merge_request, @u_custom_global)
+      end
+
       it do
         notification.reassigned_merge_request(merge_request, merge_request.author)
 
@@ -694,6 +758,8 @@ describe NotificationService, services: true do
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
+        should_email(@u_custom_global)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -761,12 +827,19 @@ describe NotificationService, services: true do
     end
 
     describe '#closed_merge_request' do
+      before do
+        update_custom_notification(:close_merge_request, @u_guest_custom, project)
+        update_custom_notification(:close_merge_request, @u_custom_global)
+      end
+
       it do
         notification.close_mr(merge_request, @u_disabled)
 
         should_email(merge_request.assignee)
         should_email(@u_watcher)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
+        should_email(@u_custom_global)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
@@ -807,6 +880,12 @@ describe NotificationService, services: true do
     end
 
     describe '#merged_merge_request' do
+
+      before do
+        update_custom_notification(:merge_merge_request, @u_guest_custom, project)
+        update_custom_notification(:merge_merge_request, @u_custom_global)
+      end
+
       it do
         notification.merge_mr(merge_request, @u_disabled)
 
@@ -816,6 +895,8 @@ describe NotificationService, services: true do
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
         should_email(@u_guest_watcher)
+        should_email(@u_custom_global)
+        should_email(@u_guest_custom)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -853,6 +934,11 @@ describe NotificationService, services: true do
     end
 
     describe '#reopen_merge_request' do
+      before do
+        update_custom_notification(:reopen_merge_request, @u_guest_custom, project)
+        update_custom_notification(:reopen_merge_request, @u_custom_global)
+      end
+
       it do
         notification.reopen_mr(merge_request, @u_disabled)
 
@@ -862,6 +948,8 @@ describe NotificationService, services: true do
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
         should_email(@u_guest_watcher)
+        should_email(@u_guest_custom)
+        should_email(@u_custom_global)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -914,7 +1002,9 @@ describe NotificationService, services: true do
         should_email(@u_watcher)
         should_email(@u_participating)
         should_email(@u_lazy_participant)
+        should_email(@u_custom_global)
         should_not_email(@u_guest_watcher)
+        should_not_email(@u_guest_custom)
         should_not_email(@u_disabled)
       end
     end
@@ -929,13 +1019,15 @@ describe NotificationService, services: true do
     @u_committer             = create(:user, username: 'committer')
     @u_not_mentioned         = create_global_setting_for(create(:user, username: 'regular'), :participating)
     @u_outsider_mentioned    = create(:user, username: 'outsider')
+    @u_custom_global         = create_global_setting_for(create(:user, username: 'custom_global'), :custom)
 
     # User to be participant by default
     # This user does not contain any record in notification settings table
     # It should be treated with a :participating notification_level
     @u_lazy_participant      = create(:user, username: 'lazy-participant')
 
-    create_guest_watcher
+    @u_guest_watcher = create_user_with_notification(:watch, 'guest_watching')
+    @u_guest_custom = create_user_with_notification(:custom, 'guest_custom')
 
     project.team << [@u_watcher, :master]
     project.team << [@u_participating, :master]
@@ -945,6 +1037,7 @@ describe NotificationService, services: true do
     project.team << [@u_committer, :master]
     project.team << [@u_not_mentioned, :master]
     project.team << [@u_lazy_participant, :master]
+    project.team << [@u_custom_global, :master]
   end
 
   def create_global_setting_for(user, level)
@@ -955,10 +1048,20 @@ describe NotificationService, services: true do
     user
   end
 
-  def create_guest_watcher
-    @u_guest_watcher = create(:user, username: 'guest_watching')
-    setting = @u_guest_watcher.notification_settings_for(project)
-    setting.level = :watch
+  def create_user_with_notification(level, username)
+    user = create(:user, username: username)
+    setting = user.notification_settings_for(project)
+    setting.level = level
+    setting.save
+
+    user
+  end
+
+  # Create custom notifications
+  # When resource is nil it means global notification
+  def update_custom_notification(event, user, resource = nil)
+    setting = user.notification_settings_for(resource)
+    setting.events[event] = true
     setting.save
   end