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

6 files changed, 65 insertions, 6 deletions

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index f4ec60ad2c7..55632308b4f 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -102,9 +102,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
+          if can?(current_user, :read_project, @project)
             @notification_setting = current_user.notification_settings_for(@project)
           end
         end
diff --git a/app/models/user.rb b/app/models/user.rb
index 368a3f3cfba..cc312a291c6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -777,7 +777,14 @@ class User < ActiveRecord::Base
   end
 
   def notification_settings_for(source)
-    notification_settings.find_or_initialize_by(source: source)
+    notification_setting  = notification_settings.find_or_initialize_by({ source: source })
+
+    if source.is_a?(Project)
+      membership = source.team.find_member(self.id)
+      notification_setting.level = :disabled unless membership.present? || notification_setting.persisted?
+    end
+
+    notification_setting
   end
 
   private
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..685fe9ffe0b 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -8,10 +8,43 @@ 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("disabled")
+          end
+        end
+
+        context "and has notification setting" do
+          before do
+            setting = user.notification_settings_for(public_project)
+            setting.level = :global
+            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("global")
+          end
+        end
+      end
+    end
 
     context "rendering default project view" do
       render_views
-
       it "renders the activity view" do
         allow(controller).to receive(:current_user).and_return(user)
         allow(user).to receive(:project_view).and_return('activity')
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 213e8c2eb3a..e872fc0eed2 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -17,6 +17,7 @@ describe MergeRequests::UpdateService, services: true do
   before do
     project.team << [user, :master]
     project.team << [user2, :developer]
+    project.team << [user3, :developer]
   end
 
   describe 'execute' do
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 4bbc4ddc3ab..d557b2e65b8 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -119,7 +119,10 @@ describe NotificationService, services: true do
       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}") }
 
       it 'filters out users that can not read the issue' do
+        project.team << [admin, :master]
+        project.team << [author, :developer]
         project.team << [member, :developer]
+        project.team << [assignee, :developer]
 
         expect(SentNotification).to receive(:record).with(confidential_issue, any_args).exactly(4).times
 
@@ -143,7 +146,8 @@ describe NotificationService, services: true do
 
       before do
         build_team(note.project)
-        note.project.team << [note.author, :master]
+        note.project.team << [[note.author, note.noteable.author, note.noteable.assignee], :master]
+
         ActionMailer::Base.deliveries.clear
       end
 
@@ -260,6 +264,7 @@ describe NotificationService, services: true do
     before do
       build_team(issue.project)
       add_users_with_subscription(issue.project, issue)
+      project.team << [[issue.assignee, issue.author], :developer]
       ActionMailer::Base.deliveries.clear
     end
 
@@ -491,6 +496,7 @@ describe NotificationService, services: true do
     before do
       build_team(merge_request.target_project)
       add_users_with_subscription(merge_request.target_project, merge_request)
+      project.team << [merge_request.assignee, :developer]
       ActionMailer::Base.deliveries.clear
     end