From b1461de993daf6d43c8a54482eecacc6bb58df4d Mon Sep 17 00:00:00 2001
From: Riyad Preukschas <riyad@informatik.uni-bremen.de>
Date: Sat, 13 Oct 2012 16:23:12 +0200
Subject: [PATCH] Make Note methods saner

---
 app/helpers/notes_helper.rb     |  4 +---
 app/mailers/notify.rb           |  2 +-
 app/models/note.rb              | 17 +++++++++--------
 app/roles/static_model.rb       |  8 ++++++++
 app/views/notes/_note.html.haml |  6 +++---
 spec/mailers/notify_spec.rb     |  2 +-
 spec/models/note_spec.rb        | 18 ++++++++++++++++--
 7 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 99db8b6f6b7..ffcc7acc8da 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -13,9 +13,7 @@ module NotesHelper
   end
 
   def link_to_commit_diff_line_note(note)
-    return unless note.line_note?
-
-    commit = note.target
+    commit = note.noteable
     diff_index, diff_old_line, diff_new_line = note.line_code.split('_')
 
     link_file = commit.diffs[diff_index.to_i].new_path
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 0afc1d31ef4..9e64b64a86e 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -29,7 +29,7 @@ class Notify < ActionMailer::Base
 
   def note_commit_email(recipient_id, note_id)
     @note = Note.find(note_id)
-    @commit = @note.target
+    @commit = @note.noteable
     @commit = CommitDecorator.decorate(@commit)
     @project = @note.project
     mail(to: recipient(recipient_id), subject: subject("note for commit #{@commit.short_id}", @commit.title))
diff --git a/app/models/note.rb b/app/models/note.rb
index ae51e486675..e2f4a89d7d6 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -48,11 +48,12 @@ class Note < ActiveRecord::Base
     @notify_author ||= false
   end
 
-  def target
-    if commit?
+  # override to return commits, which are not active record
+  def noteable
+    if for_commit?
       project.commit(noteable_id)
     else
-      noteable
+      super
     end
   # Temp fix to prevent app crash
   # if note commit id doesnt exist
@@ -74,22 +75,22 @@ class Note < ActiveRecord::Base
   #   Boolean
   #
   def notify_only_author?(user)
-    commit? && commit_author &&
+    for_commit? && commit_author &&
       commit_author.email != user.email
   end
 
-  def commit?
+  def for_commit?
     noteable_type == "Commit"
   end
 
-  def line_note?
+  def for_diff_line?
     line_code.present?
   end
 
   def commit_author
     @commit_author ||=
-      project.users.find_by_email(target.author_email) ||
-      project.users.find_by_name(target.author_name)
+      project.users.find_by_email(noteable.author_email) ||
+      project.users.find_by_name(noteable.author_name)
   rescue
     nil
   end
diff --git a/app/roles/static_model.rb b/app/roles/static_model.rb
index d67af2439c7..5b64be1f041 100644
--- a/app/roles/static_model.rb
+++ b/app/roles/static_model.rb
@@ -36,4 +36,12 @@ module StaticModel
   def destroyed?
     false
   end
+
+  def ==(other)
+    if other.is_a? StaticModel
+      id == other.id
+    else
+      super
+    end
+  end
 end
diff --git a/app/views/notes/_note.html.haml b/app/views/notes/_note.html.haml
index 0901e4b8302..70baa212d10 100644
--- a/app/views/notes/_note.html.haml
+++ b/app/views/notes/_note.html.haml
@@ -8,10 +8,10 @@
         ago
 
     - unless note_for_main_target?(note)
-      - if note.commit?
+      - if note.for_commit?
         %span.cgray
-          on #{link_to note.target.short_id, project_commit_path(@project, note.target)}
-          = link_to_commit_diff_line_note(note) if note.line_note?
+          on #{link_to note.noteable.short_id, project_commit_path(@project, note.noteable)}
+          = link_to_commit_diff_line_note(note) if note.for_diff_line?
 
     -# only show vote if it's a note for the main target
     - if note_for_main_target?(note)
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 4a9f142e50d..874864a3894 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -235,7 +235,7 @@ describe Notify do
             commit.stub(:safe_message).and_return('some message')
           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) }
 
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 8b622d5b8c5..514b6202b74 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -85,9 +85,19 @@ describe Note do
         noteable_type: "Commit"
     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
       @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
 
@@ -101,7 +111,11 @@ describe Note do
 
     it "should save a valid note" do
       @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