Merge pull request #216 from abf/rosa-build:fix_activity_for_pull_requests

fix dublication messages after creation pull request
This commit is contained in:
avokhmin 2013-07-11 19:35:31 +04:00
commit 332168bf5e
7 changed files with 111 additions and 9 deletions

View File

@ -55,6 +55,7 @@ class Api::V1::PullRequestsController < Api::V1::BaseController
render_validation_error(@pull, "#{@pull.class.name} has not been created") && return unless @pull.valid?
@pull.save # set pull id
@pull.reload
@pull.check(false) # don't make event transaction
if @pull.already?
@pull.destroy

View File

@ -45,6 +45,7 @@ class Projects::PullRequestsController < Projects::BaseController
if @pull.valid? # FIXME more clean/clever logics
@pull.save # set pull id
@pull.reload
@pull.check(false) # don't make event transaction
if @pull.already?
@pull.destroy

View File

@ -5,11 +5,11 @@ module Modules::Observers::ActivityFeed::Issue
included do
after_commit :new_issue_notifications, :on => :create
after_commit :send_assign_notifications, :on => :create
after_commit :send_assign_notifications, :on => :create, :if => Proc.new { |i| i.assignee }
after_commit -> { send_assign_notifications(:update) }, :on => :update
after_commit :send_hooks, :on => :create
after_commit -> { send_hooks(:update) }, :on => :update, :if => :status_changed?
after_commit -> { send_hooks(:update) }, :on => :update, :if => Proc.new { |i| i.previous_changes['status'].present? }
end
private
@ -18,7 +18,7 @@ module Modules::Observers::ActivityFeed::Issue
collect_recipients.each do |recipient|
next if user_id == recipient.id
if recipient.notifier.can_notify && recipient.notifier.new_issue && assignee_id != recipient.id
UserMailer.new_issue_notification(self, recipient).deliver
UserMailer.new_issue_notification(self, recipient).deliver
end
ActivityFeed.create(
:user => recipient,
@ -35,12 +35,11 @@ module Modules::Observers::ActivityFeed::Issue
}
)
end
# dont remove outdated issues link
Comment.create_link_on_issues_from_item(self)
end
def send_assign_notifications(action = :create)
if assignee_id && assignee_id_changed?
if(action == :create && assignee_id) || previous_changes['assignee_id'].present?
if assignee.notifier.issue_assign && assignee.notifier.can_notify
UserMailer.issue_assign_notification(self, assignee).deliver
end
@ -59,11 +58,10 @@ module Modules::Observers::ActivityFeed::Issue
)
end
# dont remove outdated issues link
Comment.create_link_on_issues_from_item(self)
Comment.create_link_on_issues_from_item(self) if previous_changes['title'].present? || previous_changes['body'].present?
end
def send_hooks(action = :create)
project.hooks.each{ |h| h.receive_issues(self, action) }
end
end
end

View File

@ -262,6 +262,48 @@ describe Api::V1::PullRequestsController do
end
end
context 'send email messages' do
before(:each) do
@project_reader = FactoryGirl.create :user
@project.relations.create!(:actor_type => 'User', :actor_id => @project_reader.id, :role => 'reader')
@project_admin = FactoryGirl.create :user
@project.relations.create!(:actor_type => 'User', :actor_id => @project_admin.id, :role => 'admin')
@project_writer = FactoryGirl.create :user
@project.relations.create!(:actor_type => 'User', :actor_id => @project_writer.id, :role => 'writer')
http_login(@project_writer)
ActionMailer::Base.deliveries = []
end
it 'should send two email messages to project admins' do
post :create, @create_params
@project.pull_requests.last.issue.send(:new_issue_notifications)
@project.pull_requests.last.issue.send(:send_assign_notifications)
ActionMailer::Base.deliveries.count.should == 2
end
it 'should send two email messages to admins and one to assignee' do
post :create, @create_params.deep_merge(:pull_request => {:assignee_id => @project_reader.id})
@project.pull_requests.last.issue.send(:new_issue_notifications)
@project.pull_requests.last.issue.send(:send_assign_notifications)
ActionMailer::Base.deliveries.count.should == 3
end
it 'should send email message to new assignee' do
put :update, @update_params.deep_merge(:pull_request => {:assignee_id => @project_reader.id})
@project.pull_requests.last.issue.send(:send_assign_notifications)
ActionMailer::Base.deliveries.count.should == 1
end
it 'should not duplicate email message' do
post :create, @create_params.deep_merge(:pull_request => {:assignee_id => @project_admin.id})
@project.pull_requests.last.issue.send(:new_issue_notifications)
@project.pull_requests.last.issue.send(:send_assign_notifications)
ActionMailer::Base.deliveries.count.should == 2 # send only to admins
ActionMailer::Base.deliveries.first.to != ActionMailer::Base.deliveries.last.to
end
end
after(:all) do
User.destroy_all
Platform.destroy_all

View File

@ -282,4 +282,40 @@ describe Projects::PullRequestsController do
it_should_behave_like 'user without pull request update rights'
end
context 'send email messages' do
before(:each) do
@project_reader = FactoryGirl.create :user
@project.relations.create!(:actor_type => 'User', :actor_id => @project_reader.id, :role => 'reader')
@project_admin = FactoryGirl.create :user
@project.relations.create!(:actor_type => 'User', :actor_id => @project_admin.id, :role => 'admin')
@project_writer = FactoryGirl.create :user
@project.relations.create!(:actor_type => 'User', :actor_id => @project_writer.id, :role => 'writer')
set_session_for(@project_writer)
ActionMailer::Base.deliveries = []
end
it 'should send two email messages to project admins' do
post :create, @create_params
@project.pull_requests.last.issue.send(:new_issue_notifications)
@project.pull_requests.last.issue.send(:send_assign_notifications)
ActionMailer::Base.deliveries.count.should == 2
end
it 'should send two email messages to admins and one to assignee' do
post :create, @create_params.deep_merge(:issue => {:assignee_id => @project_reader.id})
@project.pull_requests.last.issue.send(:new_issue_notifications)
@project.pull_requests.last.issue.send(:send_assign_notifications)
ActionMailer::Base.deliveries.count.should == 3
end
it 'should not duplicate email message' do
post :create, @create_params.deep_merge(:issue => {:assignee_id => @project_admin.id})
@project.pull_requests.last.issue.send(:new_issue_notifications)
@project.pull_requests.last.issue.send(:send_assign_notifications)
ActionMailer::Base.deliveries.count.should == 2 # send only to admins
ActionMailer::Base.deliveries.first.to != ActionMailer::Base.deliveries.last.to
end
end
end

View File

@ -33,6 +33,31 @@ describe Issue do
create_issue(@user)
ActionMailer::Base.deliveries.count.should == 0
end
it 'should create automatic comment from another issue' do
create_issue(@user)
another_issue = FactoryGirl.create(:issue, :project => @project, :title => "[##{@issue.serial_id}]")
Comment.where(:automatic => true, :commentable_type => 'Issue',
:created_from_issue_id => another_issue.id).count.should == 1
end
it 'should create automatic comment after updating another issue body' do
create_issue(@user)
another_issue = FactoryGirl.create(:issue, :project => @project)
another_issue.update_attribute(:title, "[##{@issue.serial_id}]")
another_issue.send(:send_assign_notifications)
Comment.where(:automatic => true, :commentable_type => 'Issue',
:created_from_issue_id => another_issue.id).count.should == 1
end
it 'should send email message to new assignee' do
create_issue(@user)
ActionMailer::Base.deliveries = []
@issue.update_attribute :assignee_id, @user.id
@issue.send(:send_assign_notifications, :update)
ActionMailer::Base.deliveries.count.should == 1
end
end
context 'for member-group' do

View File

@ -15,7 +15,6 @@ def set_data_for_pull
end
describe PullRequest do
context 'for owner user' do
before do
stub_symlink_methods