Commit 413778b6458c71706685150f191becee5d398391
1 parent
853c69c4
Exists in
master
and in
4 other branches
Rename NoteObserver methods and clarify things
Showing
2 changed files
with
51 additions
and
27 deletions
Show diff stats
app/observers/note_observer.rb
| 1 | 1 | class NoteObserver < ActiveRecord::Observer |
| 2 | 2 | |
| 3 | 3 | def after_create(note) |
| 4 | + send_notify_mails(note) | |
| 5 | + end | |
| 6 | + | |
| 7 | + protected | |
| 8 | + | |
| 9 | + def send_notify_mails(note) | |
| 4 | 10 | if note.notify |
| 5 | - # Notify whole team except author of note | |
| 6 | - notify_team_of_new_note(note) | |
| 11 | + notify_team(note) | |
| 7 | 12 | elsif note.notify_author |
| 8 | 13 | # Notify only author of resource |
| 9 | 14 | Notify.note_commit_email(note.commit_author.id, note.id).deliver |
| ... | ... | @@ -13,15 +18,15 @@ class NoteObserver < ActiveRecord::Observer |
| 13 | 18 | end |
| 14 | 19 | end |
| 15 | 20 | |
| 16 | - protected | |
| 17 | - | |
| 18 | - def notify_team_of_new_note(note) | |
| 19 | - note_is_on = note.noteable_type || 'Wall' | |
| 20 | - notify_method = 'note_' + note_is_on.underscore + '_email' | |
| 21 | + # Notifies the whole team except the author of note | |
| 22 | + def notify_team(note) | |
| 23 | + # Note: wall posts are not "attached" to anything, so fall back to "Wall" | |
| 24 | + noteable_type = note.noteable_type || "Wall" | |
| 25 | + notify_method = "note_#{noteable_type.underscore}_email".to_sym | |
| 21 | 26 | |
| 22 | 27 | if Notify.respond_to? notify_method |
| 23 | 28 | team_without_note_author(note).map do |u| |
| 24 | - Notify.send(notify_method.to_sym, u.id, note.id).deliver | |
| 29 | + Notify.send(notify_method, u.id, note.id).deliver | |
| 25 | 30 | end |
| 26 | 31 | end |
| 27 | 32 | end | ... | ... |
spec/observers/note_observer_spec.rb
| ... | ... | @@ -3,8 +3,11 @@ require 'spec_helper' |
| 3 | 3 | describe NoteObserver do |
| 4 | 4 | subject { NoteObserver.instance } |
| 5 | 5 | |
| 6 | + let(:team_without_author) { (1..2).map { |n| double :user, id: n } } | |
| 7 | + let(:delivery_success) { double deliver: true } | |
| 8 | + | |
| 6 | 9 | describe '#after_create' do |
| 7 | - let(:note) { double :note, notify: false, notify_author: false } | |
| 10 | + let(:note) { double :note } | |
| 8 | 11 | |
| 9 | 12 | it 'is called after a note is created' do |
| 10 | 13 | subject.should_receive :after_create |
| ... | ... | @@ -14,40 +17,51 @@ describe NoteObserver do |
| 14 | 17 | end |
| 15 | 18 | end |
| 16 | 19 | |
| 20 | + it 'sends out notifications' do | |
| 21 | + subject.should_receive(:send_notify_mails).with(note) | |
| 22 | + | |
| 23 | + subject.after_create(note) | |
| 24 | + end | |
| 25 | + end | |
| 26 | + | |
| 27 | + describe "#send_notify_mails" do | |
| 28 | + let(:note) { double :note, notify: false, notify_author: false } | |
| 29 | + | |
| 17 | 30 | it 'notifies team of new note when flagged to notify' do |
| 18 | 31 | note.stub(:notify).and_return(true) |
| 19 | - subject.should_receive(:notify_team_of_new_note).with(note) | |
| 32 | + subject.should_receive(:notify_team).with(note) | |
| 20 | 33 | |
| 21 | 34 | subject.after_create(note) |
| 22 | 35 | end |
| 36 | + | |
| 23 | 37 | it 'does not notify team of new note when not flagged to notify' do |
| 24 | - subject.should_not_receive(:notify_team_of_new_note).with(note) | |
| 38 | + subject.should_not_receive(:notify_team).with(note) | |
| 25 | 39 | |
| 26 | 40 | subject.after_create(note) |
| 27 | 41 | end |
| 42 | + | |
| 28 | 43 | it 'notifies the author of a commit when flagged to notify the author' do |
| 29 | 44 | note.stub(:notify_author).and_return(true) |
| 30 | 45 | note.stub(:id).and_return(42) |
| 31 | 46 | author = double :user, id: 1 |
| 32 | 47 | note.stub(:commit_author).and_return(author) |
| 33 | - Notify.should_receive(:note_commit_email).and_return(double(deliver: true)) | |
| 48 | + Notify.should_receive(:note_commit_email).and_return(delivery_success) | |
| 34 | 49 | |
| 35 | 50 | subject.after_create(note) |
| 36 | 51 | end |
| 52 | + | |
| 37 | 53 | it 'does not notify the author of a commit when not flagged to notify the author' do |
| 38 | 54 | Notify.should_not_receive(:note_commit_email) |
| 39 | 55 | |
| 40 | 56 | subject.after_create(note) |
| 41 | 57 | end |
| 58 | + | |
| 42 | 59 | it 'does nothing if no notify flags are set' do |
| 43 | 60 | subject.after_create(note).should be_nil |
| 44 | 61 | end |
| 45 | 62 | end |
| 46 | 63 | |
| 47 | - | |
| 48 | - let(:team_without_author) { (1..2).map { |n| double :user, id: n } } | |
| 49 | - | |
| 50 | - describe '#notify_team_of_new_note' do | |
| 64 | + describe '#notify_team' do | |
| 51 | 65 | let(:note) { double :note, id: 1 } |
| 52 | 66 | |
| 53 | 67 | before :each do |
| ... | ... | @@ -57,40 +71,45 @@ describe NoteObserver do |
| 57 | 71 | context 'notifies team of a new note on' do |
| 58 | 72 | it 'a commit' do |
| 59 | 73 | note.stub(:noteable_type).and_return('Commit') |
| 60 | - Notify.should_receive(:note_commit_email).twice.and_return(double(deliver: true)) | |
| 74 | + Notify.should_receive(:note_commit_email).twice.and_return(delivery_success) | |
| 61 | 75 | |
| 62 | - subject.send(:notify_team_of_new_note, note) | |
| 76 | + subject.send(:notify_team, note) | |
| 63 | 77 | end |
| 78 | + | |
| 64 | 79 | it 'an issue' do |
| 65 | 80 | note.stub(:noteable_type).and_return('Issue') |
| 66 | - Notify.should_receive(:note_issue_email).twice.and_return(double(deliver: true)) | |
| 81 | + Notify.should_receive(:note_issue_email).twice.and_return(delivery_success) | |
| 67 | 82 | |
| 68 | - subject.send(:notify_team_of_new_note, note) | |
| 83 | + subject.send(:notify_team, note) | |
| 69 | 84 | end |
| 85 | + | |
| 70 | 86 | it 'a wiki page' do |
| 71 | 87 | note.stub(:noteable_type).and_return('Wiki') |
| 72 | - Notify.should_receive(:note_wiki_email).twice.and_return(double(deliver: true)) | |
| 88 | + Notify.should_receive(:note_wiki_email).twice.and_return(delivery_success) | |
| 73 | 89 | |
| 74 | - subject.send(:notify_team_of_new_note, note) | |
| 90 | + subject.send(:notify_team, note) | |
| 75 | 91 | end |
| 92 | + | |
| 76 | 93 | it 'a merge request' do |
| 77 | 94 | note.stub(:noteable_type).and_return('MergeRequest') |
| 78 | - Notify.should_receive(:note_merge_request_email).twice.and_return(double(deliver: true)) | |
| 95 | + Notify.should_receive(:note_merge_request_email).twice.and_return(delivery_success) | |
| 79 | 96 | |
| 80 | - subject.send(:notify_team_of_new_note, note) | |
| 97 | + subject.send(:notify_team, note) | |
| 81 | 98 | end |
| 99 | + | |
| 82 | 100 | it 'a wall' do |
| 101 | + # Note: wall posts have #noteable_type of nil | |
| 83 | 102 | note.stub(:noteable_type).and_return(nil) |
| 84 | - Notify.should_receive(:note_wall_email).twice.and_return(double(deliver: true)) | |
| 103 | + Notify.should_receive(:note_wall_email).twice.and_return(delivery_success) | |
| 85 | 104 | |
| 86 | - subject.send(:notify_team_of_new_note, note) | |
| 105 | + subject.send(:notify_team, note) | |
| 87 | 106 | end |
| 88 | 107 | end |
| 89 | 108 | |
| 90 | 109 | it 'does nothing for a new note on a snippet' do |
| 91 | 110 | note.stub(:noteable_type).and_return('Snippet') |
| 92 | 111 | |
| 93 | - subject.send(:notify_team_of_new_note, note).should be_nil | |
| 112 | + subject.send(:notify_team, note).should be_nil | |
| 94 | 113 | end |
| 95 | 114 | end |
| 96 | 115 | ... | ... |