From 07ff874f572a947d7730787492a604dd3f44d496 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Fri, 29 Apr 2016 18:06:22 -0300 Subject: Let users set notification levels in projects which they are not members --- app/controllers/projects_controller.rb | 4 +-- app/models/user.rb | 9 +++++- .../notification_settings_controller_spec.rb | 14 +++++++++ spec/controllers/projects_controller_spec.rb | 35 +++++++++++++++++++++- .../services/merge_requests/update_service_spec.rb | 1 + spec/services/notification_service_spec.rb | 8 ++++- 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 -- cgit v1.2.1