Merge branch 'issue_9013' into 'master'

Let users set notification levels in projects which they are not members

Fixes #9013 

See merge request !3986

6 files changed, 81 insertions, 8 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 45a06d1fcd6..211b6fc56ac 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -41,6 +41,7 @@ v 8.8.0 (unreleased)
   - Added button to toggle whitespaces changes on diff view
   - Backport GitHub Enterprise import support from EE
   - Create tags using Rugged for performance reasons. !3745
+  - Allow guests to set notification level in projects
   - API: Expose Issue#user_notes_count. !3126 (Anton Popov)
   - Don't show forks button when user can't view forks
   - Fix atom feed links and rendering
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 6660d99ab09..9697b88c032 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -101,13 +101,7 @@ class ProjectsController < Projects::ApplicationController
 
     respond_to do |format|
       format.html do
-        if current_user
-          @membership = @project.team.find_member(current_user.id)
-
-          if @membership
-            @notification_setting = current_user.notification_settings_for(@project)
-          end
-        end
+        @notification_setting = current_user.notification_settings_for(@project) if current_user
 
         if @project.repository_exists?
           if @project.empty_repo?
diff --git a/doc/workflow/notifications.md b/doc/workflow/notifications.md
index 80817c98d22..cbca94c0b5e 100644
--- a/doc/workflow/notifications.md
+++ b/doc/workflow/notifications.md
@@ -69,7 +69,7 @@ In all of the below cases, the notification will be sent to:
 
     ...with notification level "Participating" or higher
 
-- Watchers: project members with notification level "Watch"
+- Watchers: users with notification level "Watch"
 - Subscribers: anyone who manually subscribed to the issue/merge request
 
 | Event                  | Sent to |
diff --git a/spec/controllers/projects/notification_settings_controller_spec.rb b/spec/controllers/projects/notification_settings_controller_spec.rb
index 4908b545648..c5d17d97ec9 100644
--- a/spec/controllers/projects/notification_settings_controller_spec.rb
+++ b/spec/controllers/projects/notification_settings_controller_spec.rb
@@ -34,5 +34,19 @@ describe Projects::NotificationSettingsController do
         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/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 069cd917e5a..91b46c4d65c 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -8,6 +8,40 @@ describe ProjectsController do
   let(:txt)     { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
 
   describe "GET show" do
+    context "user not project member" do
+      before { sign_in(user) }
+
+      context "user does not have access to project" do
+        let(:private_project) { create(:project, :private) }
+
+        it "does not initialize notification setting" do
+          get :show, namespace_id: private_project.namespace.path, id: private_project.path
+          expect(assigns(:notification_setting)).to be_nil
+        end
+      end
+
+      context "user has access to project" do
+        context "and does not have notification setting" do
+          it "initializes notification as disabled" do
+            get :show, namespace_id: public_project.namespace.path, id: public_project.path
+            expect(assigns(:notification_setting).level).to eq("global")
+          end
+        end
+
+        context "and has notification setting" do
+          before do
+            setting = user.notification_settings_for(public_project)
+            setting.level = :watch
+            setting.save
+          end
+
+          it "shows current notification setting" do
+            get :show, namespace_id: public_project.namespace.path, id: public_project.path
+            expect(assigns(:notification_setting).level).to eq("watch")
+          end
+        end
+      end
+    end
 
     context "rendering default project view" do
       render_views
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 4bbc4ddc3ab..cef5e0d8659 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -66,6 +66,7 @@ describe NotificationService, services: true do
           should_email(@subscriber)
           should_email(@watcher_and_subscriber)
           should_email(@subscribed_participant)
+          should_not_email(@u_guest_watcher)
           should_not_email(note.author)
           should_not_email(@u_participating)
           should_not_email(@u_disabled)
@@ -100,6 +101,7 @@ describe NotificationService, services: true do
           should_email(note.noteable.author)
           should_email(note.noteable.assignee)
           should_email(@u_mentioned)
+          should_not_email(@u_guest_watcher)
           should_not_email(@u_watcher)
           should_not_email(note.author)
           should_not_email(@u_participating)
@@ -160,6 +162,7 @@ describe NotificationService, services: true do
             should_email(member)
           end
 
+          should_email(@u_guest_watcher)
           should_email(note.noteable.author)
           should_email(note.noteable.assignee)
           should_not_email(note.author)
@@ -201,6 +204,7 @@ describe NotificationService, services: true do
             should_email(member)
           end
 
+          should_email(@u_guest_watcher)
           should_email(note.noteable.author)
           should_not_email(note.author)
           should_email(@u_mentioned)
@@ -224,6 +228,7 @@ describe NotificationService, services: true do
         it do
           notification.new_note(note)
 
+          should_email(@u_guest_watcher)
           should_email(@u_committer)
           should_email(@u_watcher)
           should_not_email(@u_mentioned)
@@ -236,6 +241,7 @@ describe NotificationService, services: true do
           note.update_attribute(:note, '@mention referenced')
           notification.new_note(note)
 
+          should_email(@u_guest_watcher)
           should_email(@u_committer)
           should_email(@u_watcher)
           should_email(@u_mentioned)
@@ -269,6 +275,7 @@ describe NotificationService, services: true do
 
         should_email(issue.assignee)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_not_email(@u_mentioned)
         should_not_email(@u_participating)
@@ -328,6 +335,7 @@ describe NotificationService, services: true do
 
         should_email(issue.assignee)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(@unsubscriber)
@@ -342,6 +350,7 @@ describe NotificationService, services: true do
 
         should_email(@u_mentioned)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(@unsubscriber)
@@ -356,6 +365,7 @@ describe NotificationService, services: true do
         expect(issue.assignee).to be @u_mentioned
         should_email(issue.assignee)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(@unsubscriber)
@@ -370,6 +380,7 @@ describe NotificationService, services: true do
         expect(issue.assignee).to be @u_mentioned
         should_email(issue.assignee)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(@unsubscriber)
@@ -383,6 +394,7 @@ describe NotificationService, services: true do
 
         expect(issue.assignee).to be @u_mentioned
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_not_email(issue.assignee)
@@ -411,6 +423,7 @@ describe NotificationService, services: true do
         should_not_email(issue.assignee)
         should_not_email(issue.author)
         should_not_email(@u_watcher)
+        should_not_email(@u_guest_watcher)
         should_not_email(@u_participant_mentioned)
         should_not_email(@subscriber)
         should_not_email(@watcher_and_subscriber)
@@ -459,6 +472,7 @@ describe NotificationService, services: true do
         should_email(issue.assignee)
         should_email(issue.author)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
@@ -475,6 +489,7 @@ describe NotificationService, services: true do
         should_email(issue.assignee)
         should_email(issue.author)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
@@ -502,6 +517,7 @@ describe NotificationService, services: true do
         should_email(@u_watcher)
         should_email(@watcher_and_subscriber)
         should_email(@u_participant_mentioned)
+        should_email(@u_guest_watcher)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
       end
@@ -525,6 +541,7 @@ describe NotificationService, services: true do
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
+        should_email(@u_guest_watcher)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -566,6 +583,7 @@ describe NotificationService, services: true do
 
         should_email(merge_request.assignee)
         should_email(@u_watcher)
+        should_email(@u_guest_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
@@ -584,6 +602,7 @@ describe NotificationService, services: true do
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
+        should_email(@u_guest_watcher)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -599,6 +618,7 @@ describe NotificationService, services: true do
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
         should_email(@watcher_and_subscriber)
+        should_email(@u_guest_watcher)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -620,6 +640,7 @@ describe NotificationService, services: true do
 
         should_email(@u_watcher)
         should_email(@u_participating)
+        should_not_email(@u_guest_watcher)
         should_not_email(@u_disabled)
       end
     end
@@ -635,6 +656,8 @@ describe NotificationService, services: true do
     @u_not_mentioned = create(:user, username: 'regular', notification_level: :participating)
     @u_outsider_mentioned = create(:user, username: 'outsider')
 
+    create_guest_watcher
+
     project.team << [@u_watcher, :master]
     project.team << [@u_participating, :master]
     project.team << [@u_participant_mentioned, :master]
@@ -644,6 +667,13 @@ describe NotificationService, services: true do
     project.team << [@u_not_mentioned, :master]
   end
 
+  def create_guest_watcher
+    @u_guest_watcher = create(:user, username: 'guest_watching')
+    setting = @u_guest_watcher.notification_settings_for(project)
+    setting.level = :watch
+    setting.save
+  end
+
   def add_users_with_subscription(project, issuable)
     @subscriber = create :user
     @unsubscriber = create :user