Commit e84d90c1 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge pull request #1692 from riyad/saner-note-methods

Small Note code cleanup
parents bd5dbe14 b1461de9
...@@ -13,9 +13,7 @@ module NotesHelper ...@@ -13,9 +13,7 @@ module NotesHelper
end end
def link_to_commit_diff_line_note(note) def link_to_commit_diff_line_note(note)
return unless note.line_note? commit = note.noteable
commit = note.target
diff_index, diff_old_line, diff_new_line = note.line_code.split('_') diff_index, diff_old_line, diff_new_line = note.line_code.split('_')
link_file = commit.diffs[diff_index.to_i].new_path link_file = commit.diffs[diff_index.to_i].new_path
......
...@@ -29,7 +29,7 @@ class Notify < ActionMailer::Base ...@@ -29,7 +29,7 @@ class Notify < ActionMailer::Base
def note_commit_email(recipient_id, note_id) def note_commit_email(recipient_id, note_id)
@note = Note.find(note_id) @note = Note.find(note_id)
@commit = @note.target @commit = @note.noteable
@commit = CommitDecorator.decorate(@commit) @commit = CommitDecorator.decorate(@commit)
@project = @note.project @project = @note.project
mail(to: recipient(recipient_id), subject: subject("note for commit #{@commit.short_id}", @commit.title)) mail(to: recipient(recipient_id), subject: subject("note for commit #{@commit.short_id}", @commit.title))
......
...@@ -48,11 +48,12 @@ class Note < ActiveRecord::Base ...@@ -48,11 +48,12 @@ class Note < ActiveRecord::Base
@notify_author ||= false @notify_author ||= false
end end
def target # override to return commits, which are not active record
if commit? def noteable
if for_commit?
project.commit(noteable_id) project.commit(noteable_id)
else else
noteable super
end end
# Temp fix to prevent app crash # Temp fix to prevent app crash
# if note commit id doesnt exist # if note commit id doesnt exist
...@@ -74,22 +75,22 @@ class Note < ActiveRecord::Base ...@@ -74,22 +75,22 @@ class Note < ActiveRecord::Base
# Boolean # Boolean
# #
def notify_only_author?(user) def notify_only_author?(user)
commit? && commit_author && for_commit? && commit_author &&
commit_author.email != user.email commit_author.email != user.email
end end
def commit? def for_commit?
noteable_type == "Commit" noteable_type == "Commit"
end end
def line_note? def for_diff_line?
line_code.present? line_code.present?
end end
def commit_author def commit_author
@commit_author ||= @commit_author ||=
project.users.find_by_email(target.author_email) || project.users.find_by_email(noteable.author_email) ||
project.users.find_by_name(target.author_name) project.users.find_by_name(noteable.author_name)
rescue rescue
nil nil
end end
......
...@@ -36,4 +36,12 @@ module StaticModel ...@@ -36,4 +36,12 @@ module StaticModel
def destroyed? def destroyed?
false false
end end
def ==(other)
if other.is_a? StaticModel
id == other.id
else
super
end
end
end end
...@@ -8,10 +8,10 @@ ...@@ -8,10 +8,10 @@
ago ago
- unless note_for_main_target?(note) - unless note_for_main_target?(note)
- if note.commit? - if note.for_commit?
%span.cgray %span.cgray
on #{link_to note.target.short_id, project_commit_path(@project, note.target)} on #{link_to note.noteable.short_id, project_commit_path(@project, note.noteable)}
= link_to_commit_diff_line_note(note) if note.line_note? = link_to_commit_diff_line_note(note) if note.for_diff_line?
-# only show vote if it's a note for the main target -# only show vote if it's a note for the main target
- if note_for_main_target?(note) - if note_for_main_target?(note)
......
...@@ -235,7 +235,7 @@ describe Notify do ...@@ -235,7 +235,7 @@ describe Notify do
commit.stub(:safe_message).and_return('some message') commit.stub(:safe_message).and_return('some message')
end end
end end
before(:each) { note.stub(:target).and_return(commit) } before(:each) { note.stub(:noteable).and_return(commit) }
subject { Notify.note_commit_email(recipient.id, note.id) } subject { Notify.note_commit_email(recipient.id, note.id) }
......
...@@ -85,9 +85,19 @@ describe Note do ...@@ -85,9 +85,19 @@ describe Note do
noteable_type: "Commit" noteable_type: "Commit"
end end
it "should be accessible through #noteable" do
@note.noteable_id.should == commit.id
@note.noteable.should be_a(Commit)
@note.noteable.should == commit
end
it "should save a valid note" do it "should save a valid note" do
@note.noteable_id.should == commit.id @note.noteable_id.should == commit.id
@note.target.id.should == commit.id @note.noteable == commit
end
it "should be recognized by #for_commit?" do
@note.should be_for_commit
end end
end end
...@@ -101,7 +111,11 @@ describe Note do ...@@ -101,7 +111,11 @@ describe Note do
it "should save a valid note" do it "should save a valid note" do
@note.noteable_id.should == commit.id @note.noteable_id.should == commit.id
@note.target.id.should == commit.id @note.noteable.id.should == commit.id
end
it "should be recognized by #for_diff_line?" do
@note.should be_for_diff_line
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment