Commit 16e67fd8be71cfda57ca19250781a9e0f800d619
Exists in
master
and in
4 other branches
Merge pull request #1364 from AlexDenisov/fix_project_access_notification
Project access notifications fixed
Showing
4 changed files
with
27 additions
and
17 deletions
Show diff stats
Gemfile
Gemfile.lock
| @@ -377,6 +377,7 @@ GEM | @@ -377,6 +377,7 @@ GEM | ||
| 377 | tilt (~> 1.1, != 1.3.0) | 377 | tilt (~> 1.1, != 1.3.0) |
| 378 | sqlite3 (1.3.6) | 378 | sqlite3 (1.3.6) |
| 379 | stamp (0.1.6) | 379 | stamp (0.1.6) |
| 380 | + test_after_commit (0.0.1) | ||
| 380 | therubyracer (0.10.1) | 381 | therubyracer (0.10.1) |
| 381 | libv8 (~> 3.3.10) | 382 | libv8 (~> 3.3.10) |
| 382 | thin (1.3.1) | 383 | thin (1.3.1) |
| @@ -476,6 +477,7 @@ DEPENDENCIES | @@ -476,6 +477,7 @@ DEPENDENCIES | ||
| 476 | spinach-rails | 477 | spinach-rails |
| 477 | sqlite3 | 478 | sqlite3 |
| 478 | stamp | 479 | stamp |
| 480 | + test_after_commit | ||
| 479 | therubyracer | 481 | therubyracer |
| 480 | thin | 482 | thin |
| 481 | uglifier (= 1.0.3) | 483 | uglifier (= 1.0.3) |
app/observers/users_project_observer.rb
| 1 | class UsersProjectObserver < ActiveRecord::Observer | 1 | class UsersProjectObserver < ActiveRecord::Observer |
| 2 | - def after_create(users_project) | 2 | + def after_commit(users_project) |
| 3 | + return if users_project.destroyed? | ||
| 3 | Notify.project_access_granted_email(users_project.id).deliver | 4 | Notify.project_access_granted_email(users_project.id).deliver |
| 5 | + end | ||
| 4 | 6 | ||
| 7 | + def after_create(users_project) | ||
| 5 | Event.create( | 8 | Event.create( |
| 6 | project_id: users_project.project.id, | 9 | project_id: users_project.project.id, |
| 7 | action: Event::Joined, | 10 | action: Event::Joined, |
| @@ -9,10 +12,6 @@ class UsersProjectObserver < ActiveRecord::Observer | @@ -9,10 +12,6 @@ class UsersProjectObserver < ActiveRecord::Observer | ||
| 9 | ) | 12 | ) |
| 10 | end | 13 | end |
| 11 | 14 | ||
| 12 | - def after_update(users_project) | ||
| 13 | - Notify.project_access_granted_email(users_project.id).deliver | ||
| 14 | - end | ||
| 15 | - | ||
| 16 | def after_destroy(users_project) | 15 | def after_destroy(users_project) |
| 17 | Event.create( | 16 | Event.create( |
| 18 | project_id: users_project.project.id, | 17 | project_id: users_project.project.id, |
spec/observers/users_project_observer_spec.rb
| 1 | require 'spec_helper' | 1 | require 'spec_helper' |
| 2 | 2 | ||
| 3 | describe UsersProjectObserver do | 3 | describe UsersProjectObserver do |
| 4 | - let(:users_project) { stub.as_null_object } | 4 | + let(:user) { Factory.create :user } |
| 5 | + let(:project) { Factory.create(:project, | ||
| 6 | + code: "Fuu", | ||
| 7 | + path: "Fuu" ) } | ||
| 8 | + let(:users_project) { Factory.create(:users_project, | ||
| 9 | + project: project, | ||
| 10 | + user: user )} | ||
| 5 | subject { UsersProjectObserver.instance } | 11 | subject { UsersProjectObserver.instance } |
| 6 | 12 | ||
| 7 | - describe "#after_create" do | 13 | + describe "#after_commit" do |
| 8 | it "should called when UsersProject created" do | 14 | it "should called when UsersProject created" do |
| 9 | - subject.should_receive(:after_create) | ||
| 10 | - | 15 | + subject.should_receive(:after_commit).once |
| 11 | UsersProject.observers.enable :users_project_observer do | 16 | UsersProject.observers.enable :users_project_observer do |
| 12 | create(:users_project) | 17 | create(:users_project) |
| 13 | end | 18 | end |
| 14 | end | 19 | end |
| 15 | 20 | ||
| 16 | it "should send email to user" do | 21 | it "should send email to user" do |
| 22 | + Notify.should_receive(:project_access_granted_email).with(users_project.id).and_return(double(deliver: true)) | ||
| 23 | + subject.after_commit(users_project) | ||
| 17 | Event.stub(:create => true) | 24 | Event.stub(:create => true) |
| 18 | - Notify.should_receive(:project_access_granted_email).and_return(stub(deliver: true)) | ||
| 19 | - | ||
| 20 | - subject.after_create(users_project) | ||
| 21 | end | 25 | end |
| 22 | 26 | ||
| 23 | it "should create new event" do | 27 | it "should create new event" do |
| @@ -33,17 +37,21 @@ describe UsersProjectObserver do | @@ -33,17 +37,21 @@ describe UsersProjectObserver do | ||
| 33 | 37 | ||
| 34 | describe "#after_update" do | 38 | describe "#after_update" do |
| 35 | it "should called when UsersProject updated" do | 39 | it "should called when UsersProject updated" do |
| 36 | - subject.should_receive(:after_update) | ||
| 37 | - | 40 | + subject.should_receive(:after_commit).once |
| 38 | UsersProject.observers.enable :users_project_observer do | 41 | UsersProject.observers.enable :users_project_observer do |
| 39 | - create(:users_project).update_attribute(:project_access, 40) | 42 | + create(:users_project).update_attribute(:project_access, UsersProject::MASTER) |
| 40 | end | 43 | end |
| 41 | end | 44 | end |
| 42 | 45 | ||
| 43 | it "should send email to user" do | 46 | it "should send email to user" do |
| 44 | Notify.should_receive(:project_access_granted_email).with(users_project.id).and_return(double(deliver: true)) | 47 | Notify.should_receive(:project_access_granted_email).with(users_project.id).and_return(double(deliver: true)) |
| 45 | - | ||
| 46 | - subject.after_update(users_project) | 48 | + subject.after_commit(users_project) |
| 49 | + end | ||
| 50 | + it "should not called after UsersProject destroyed" do | ||
| 51 | + subject.should_not_receive(:after_commit) | ||
| 52 | + UsersProject.observers.enable :users_project_observer do | ||
| 53 | + users_project.destroy | ||
| 54 | + end | ||
| 47 | end | 55 | end |
| 48 | end | 56 | end |
| 49 | 57 |