From c2c7014e017a36e2819335653f5d3fc04cc2c054 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Fri, 6 May 2016 16:40:27 -0300 Subject: [PATCH] Improve documentation and add changelog --- app/controllers/projects_controller.rb | 6 +---- app/models/user.rb | 7 +++-- spec/services/notification_service_spec.rb | 30 +++++++++++++++++++++- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 55632308b4f..6d2b6b60f13 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -101,11 +101,7 @@ class ProjectsController < Projects::ApplicationController respond_to do |format| format.html do - if current_user - if can?(current_user, :read_project, @project) - @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/app/models/user.rb b/app/models/user.rb index cc312a291c6..09fb6c978f3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -777,11 +777,10 @@ class User < ActiveRecord::Base end def notification_settings_for(source) - notification_setting = 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? + if source.is_a?(Project) && !source.team.member?(id) && !notification_setting.persisted? + notification_setting.level = :disabled end notification_setting diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index d557b2e65b8..6a84dcfa014 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -106,6 +106,35 @@ describe NotificationService, services: true do should_not_email(@u_disabled) end end + + context 'when user is not project member' do + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public) } + let(:issue) { create(:issue, project: project, assignee: create(:user)) } + let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: 'anything') } + + before { ActionMailer::Base.deliveries.clear } + + context "and has notification setting" do + before do + setting = user.notification_settings_for(project) + setting.level = :watch + setting.save + end + + it "sends user email" do + notification.new_note(note) + should_email(user) + end + end + + context "and does note have notification setting" do + it "does not send email" do + notification.new_note(note) + should_not_email(user) + end + end + end end context 'confidential issue note' do @@ -147,7 +176,6 @@ describe NotificationService, services: true do before do build_team(note.project) note.project.team << [[note.author, note.noteable.author, note.noteable.assignee], :master] - ActionMailer::Base.deliveries.clear end