Commit b1461de993daf6d43c8a54482eecacc6bb58df4d
1 parent
db3d90cb
Exists in
master
and in
4 other branches
Make Note methods saner
Showing
7 changed files
with
39 additions
and
18 deletions
Show diff stats
app/helpers/notes_helper.rb
... | ... | @@ -13,9 +13,7 @@ module NotesHelper |
13 | 13 | end |
14 | 14 | |
15 | 15 | def link_to_commit_diff_line_note(note) |
16 | - return unless note.line_note? | |
17 | - | |
18 | - commit = note.target | |
16 | + commit = note.noteable | |
19 | 17 | diff_index, diff_old_line, diff_new_line = note.line_code.split('_') |
20 | 18 | |
21 | 19 | link_file = commit.diffs[diff_index.to_i].new_path | ... | ... |
app/mailers/notify.rb
... | ... | @@ -29,7 +29,7 @@ class Notify < ActionMailer::Base |
29 | 29 | |
30 | 30 | def note_commit_email(recipient_id, note_id) |
31 | 31 | @note = Note.find(note_id) |
32 | - @commit = @note.target | |
32 | + @commit = @note.noteable | |
33 | 33 | @commit = CommitDecorator.decorate(@commit) |
34 | 34 | @project = @note.project |
35 | 35 | mail(to: recipient(recipient_id), subject: subject("note for commit #{@commit.short_id}", @commit.title)) | ... | ... |
app/models/note.rb
... | ... | @@ -48,11 +48,12 @@ class Note < ActiveRecord::Base |
48 | 48 | @notify_author ||= false |
49 | 49 | end |
50 | 50 | |
51 | - def target | |
52 | - if commit? | |
51 | + # override to return commits, which are not active record | |
52 | + def noteable | |
53 | + if for_commit? | |
53 | 54 | project.commit(noteable_id) |
54 | 55 | else |
55 | - noteable | |
56 | + super | |
56 | 57 | end |
57 | 58 | # Temp fix to prevent app crash |
58 | 59 | # if note commit id doesnt exist |
... | ... | @@ -74,22 +75,22 @@ class Note < ActiveRecord::Base |
74 | 75 | # Boolean |
75 | 76 | # |
76 | 77 | def notify_only_author?(user) |
77 | - commit? && commit_author && | |
78 | + for_commit? && commit_author && | |
78 | 79 | commit_author.email != user.email |
79 | 80 | end |
80 | 81 | |
81 | - def commit? | |
82 | + def for_commit? | |
82 | 83 | noteable_type == "Commit" |
83 | 84 | end |
84 | 85 | |
85 | - def line_note? | |
86 | + def for_diff_line? | |
86 | 87 | line_code.present? |
87 | 88 | end |
88 | 89 | |
89 | 90 | def commit_author |
90 | 91 | @commit_author ||= |
91 | - project.users.find_by_email(target.author_email) || | |
92 | - project.users.find_by_name(target.author_name) | |
92 | + project.users.find_by_email(noteable.author_email) || | |
93 | + project.users.find_by_name(noteable.author_name) | |
93 | 94 | rescue |
94 | 95 | nil |
95 | 96 | end | ... | ... |
app/roles/static_model.rb
app/views/notes/_note.html.haml
... | ... | @@ -8,10 +8,10 @@ |
8 | 8 | ago |
9 | 9 | |
10 | 10 | - unless note_for_main_target?(note) |
11 | - - if note.commit? | |
11 | + - if note.for_commit? | |
12 | 12 | %span.cgray |
13 | - on #{link_to note.target.short_id, project_commit_path(@project, note.target)} | |
14 | - = link_to_commit_diff_line_note(note) if note.line_note? | |
13 | + on #{link_to note.noteable.short_id, project_commit_path(@project, note.noteable)} | |
14 | + = link_to_commit_diff_line_note(note) if note.for_diff_line? | |
15 | 15 | |
16 | 16 | -# only show vote if it's a note for the main target |
17 | 17 | - if note_for_main_target?(note) | ... | ... |
spec/mailers/notify_spec.rb
... | ... | @@ -235,7 +235,7 @@ describe Notify do |
235 | 235 | commit.stub(:safe_message).and_return('some message') |
236 | 236 | end |
237 | 237 | end |
238 | - before(:each) { note.stub(:target).and_return(commit) } | |
238 | + before(:each) { note.stub(:noteable).and_return(commit) } | |
239 | 239 | |
240 | 240 | subject { Notify.note_commit_email(recipient.id, note.id) } |
241 | 241 | ... | ... |
spec/models/note_spec.rb
... | ... | @@ -85,9 +85,19 @@ describe Note do |
85 | 85 | noteable_type: "Commit" |
86 | 86 | end |
87 | 87 | |
88 | + it "should be accessible through #noteable" do | |
89 | + @note.noteable_id.should == commit.id | |
90 | + @note.noteable.should be_a(Commit) | |
91 | + @note.noteable.should == commit | |
92 | + end | |
93 | + | |
88 | 94 | it "should save a valid note" do |
89 | 95 | @note.noteable_id.should == commit.id |
90 | - @note.target.id.should == commit.id | |
96 | + @note.noteable == commit | |
97 | + end | |
98 | + | |
99 | + it "should be recognized by #for_commit?" do | |
100 | + @note.should be_for_commit | |
91 | 101 | end |
92 | 102 | end |
93 | 103 | |
... | ... | @@ -101,7 +111,11 @@ describe Note do |
101 | 111 | |
102 | 112 | it "should save a valid note" do |
103 | 113 | @note.noteable_id.should == commit.id |
104 | - @note.target.id.should == commit.id | |
114 | + @note.noteable.id.should == commit.id | |
115 | + end | |
116 | + | |
117 | + it "should be recognized by #for_diff_line?" do | |
118 | + @note.should be_for_diff_line | |
105 | 119 | end |
106 | 120 | end |
107 | 121 | ... | ... |