From 1993bfe16465b627a03868564243b5f8a6cd838b Mon Sep 17 00:00:00 2001
From: Nathan Broadbent <nathan.f77@gmail.com>
Date: Fri, 9 Sep 2011 19:08:38 +0800
Subject: [PATCH] Refactored file links in backtrace. github_url field is tried first, then falls back to issue trackers such as redmine. Redmine can now also be configured with 2 projects - one for creating tickets, and one for viewing files. (Some organizations do it this way because of permissions issues)

---
 app/helpers/errs_helper.rb                         | 18 +-----------------
 app/helpers/notices_helper.rb                      | 34 +++++++++++++++++++++++++---------
 app/models/app.rb                                  |  5 -----
 app/models/issue_tracker.rb                        |  1 +
 app/models/issue_trackers/redmine_tracker.rb       | 19 +++++++++++++++++--
 app/models/notice.rb                               |  2 +-
 app/views/apps/_fields.html.haml                   | 10 +++-------
 spec/models/issue_trackers/redmine_tracker_spec.rb | 20 ++++++++++++++++++--
 8 files changed, 66 insertions(+), 43 deletions(-)

diff --git a/app/helpers/errs_helper.rb b/app/helpers/errs_helper.rb
index 0905998..44a57f2 100644
--- a/app/helpers/errs_helper.rb
+++ b/app/helpers/errs_helper.rb
@@ -1,5 +1,4 @@
 module ErrsHelper
-
   def last_notice_at err
     err.last_notice_at || err.created_at
   end
@@ -7,20 +6,5 @@ module ErrsHelper
   def err_confirm
     Errbit::Config.confirm_resolve_err === false ? nil : 'Seriously?'
   end
-
-  def link_to_github app, line, text=nil
-    file_name   = line['file'].split('/').last
-    file_path   = line['file'].gsub('[PROJECT_ROOT]', '')
-    line_number = line['number']
-    link_to(text || file_name, "#{app.github_url_to_file(file_path)}#L#{line_number}", :target => '_blank')
-  end
-
-  def link_to_redmine app, line, text=nil
-    file_name   = line['file'].split('/').last
-    file_path   = line['file'].gsub('[PROJECT_ROOT]', '')
-    line_number = line['number']
-    lnk = "%s%s#L%s" % [ app.redmine_url, file_path, line_number]
-    link_to(text || file_name, lnk, :target => '_blank')
-  end
-
 end
+
diff --git a/app/helpers/notices_helper.rb b/app/helpers/notices_helper.rb
index bb34062..bd867f2 100644
--- a/app/helpers/notices_helper.rb
+++ b/app/helpers/notices_helper.rb
@@ -4,16 +4,32 @@ module NoticesHelper
     render :partial => "notices/atom_entry.html.haml", :locals => {:notice => notice}
   end
 
-  def render_line_number(app, line)
-    unless Notice.in_app_backtrace_line?(line)
-      line['number']
-    else
-      case true
-      when app.github_url? then link_to_github(app, line, line['number'])
-      when app.redmine_url? then link_to_redmine(app, line, line['number'])
-      else
-        line['number']
+  def line_number_with_link(app, line)
+    return "&nbsp;".html_safe unless line['number']
+    if Notice.in_app_backtrace_line?(line)
+      return link_to_github(app, line, line['number']) if app.github_url?
+      if app.issue_tracker && app.issue_tracker.respond_to?(:url_to_file)
+        # Return link to file on tracker if issue tracker supports this
+        return link_to_issue_tracker_file(app, line, line['number'])
       end
     end
+    line['number']
+  end
+
+  def filepath_parts(file)
+    [file.split('/').last, file.gsub('[PROJECT_ROOT]', '')]
+  end
+
+  def link_to_github(app, line, text = nil)
+    file_name, file_path = filepath_parts(line['file'])
+    href = "%s#L%s" % [app.github_url_to_file(file_path), line['number']]
+    link_to(text || file_name, href, :target => '_blank')
+  end
+
+  def link_to_issue_tracker_file(app, line, text = nil)
+    file_name, file_path = filepath_parts(line['file'])
+    href = app.issue_tracker.url_to_file(file_path, line['number'])
+    link_to(text || file_name, href, :target => '_blank')
   end
 end
+
diff --git a/app/models/app.rb b/app/models/app.rb
index 8e2742d..230bfbe 100644
--- a/app/models/app.rb
+++ b/app/models/app.rb
@@ -5,7 +5,6 @@ class App
   field :name, :type => String
   field :api_key
   field :github_url
-  field :redmine_url
   field :resolve_errs_on_deploy, :type => Boolean, :default => false
   field :notify_all_users, :type => Boolean, :default => false
   field :notify_on_errs, :type => Boolean, :default => true
@@ -67,10 +66,6 @@ class App
     self.github_url.present?
   end
 
-  def redmine_url?
-    self.redmine_url.present?
-  end
-
   def github_url_to_file(file)
     "#{self.github_url}/blob/master#{file}"
   end
diff --git a/app/models/issue_tracker.rb b/app/models/issue_tracker.rb
index 1392312..0ca8eec 100644
--- a/app/models/issue_tracker.rb
+++ b/app/models/issue_tracker.rb
@@ -8,6 +8,7 @@ class IssueTracker
   embedded_in :app, :inverse_of => :issue_tracker
 
   field :project_id, :type => String
+  field :alt_project_id, :type => String # Specify an alternative project id. e.g. for viewing files
   field :api_token, :type => String
   field :account, :type => String
   field :username, :type => String
diff --git a/app/models/issue_trackers/redmine_tracker.rb b/app/models/issue_trackers/redmine_tracker.rb
index 9eeb134..28d626b 100644
--- a/app/models/issue_trackers/redmine_tracker.rb
+++ b/app/models/issue_trackers/redmine_tracker.rb
@@ -8,11 +8,19 @@ class IssueTrackers::RedmineTracker < IssueTracker
     [:api_token, {
       :placeholder => "API Token for your account"
     }],
-    [:project_id, {}]
+    [:project_id, {
+      :label       => "Ticket Project",
+      :placeholder => "Redmine Project where tickets will be created"
+    }],
+    [:alt_project_id, {
+      :optional    => true,
+      :label       => "App Project",
+      :placeholder => "Where app's files & revisions can be viewed. (Leave blank to use the above project by default)"
+    }]
   ]
 
   def check_params
-    if Fields.detect {|f| self[f[0]].blank? }
+    if Fields.detect {|f| self[f[0]].blank? && !f[1][:optional]}
       errors.add :base, 'You must specify your Redmine URL, API token and Project ID'
     end
   end
@@ -31,6 +39,13 @@ class IssueTrackers::RedmineTracker < IssueTracker
     err.update_attribute :issue_link, "#{RedmineClient::Issue.site.to_s.sub(/#{RedmineClient::Issue.site.path}$/, '')}#{RedmineClient::Issue.element_path(issue.id, :project_id => project_id)}".sub(/\.xml\?project_id=#{project_id}$/, "\?project_id=#{project_id}")
   end
 
+  def url_to_file(file_path, line_number = nil)
+    # alt_project_id let's users specify a different project for tickets / app files.
+    project = self.alt_project_id.present? ? self.alt_project_id : self.project_id
+    url = "#{self.account}/projects/#{project}/repository/annotate/#{file_path.sub(/^\//,'')}"
+    line_number ? url << "#L#{line_number}" : url
+  end
+
   def body_template
     @@body_template ||= ERB.new(File.read(Rails.root + "app/views/issue_trackers/textile_body.txt.erb"))
   end
diff --git a/app/models/notice.rb b/app/models/notice.rb
index ccfa2bc..1ee65af 100644
--- a/app/models/notice.rb
+++ b/app/models/notice.rb
@@ -57,7 +57,7 @@ class Notice
     agent_string.blank? ? nil : UserAgent.parse(agent_string)
   end
 
-  def self.in_app_backtrace_line? line
+  def self.in_app_backtrace_line?(line)
     !!(line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))})
   end
 
diff --git a/app/views/apps/_fields.html.haml b/app/views/apps/_fields.html.haml
index 04e0b33..b14a65d 100644
--- a/app/views/apps/_fields.html.haml
+++ b/app/views/apps/_fields.html.haml
@@ -4,13 +4,9 @@
   = f.label :name
   = f.text_field :name
 
-%fieldset
-  %legend Links to Codebase
-  %div
-    = f.label :github_url
-    = f.text_field :github_url
-    = f.label :redmine_url
-    = f.text_field :redmine_url
+%div
+  = f.label :github_url
+  = f.text_field :github_url
 
 %fieldset
   %legend Notifications
diff --git a/spec/models/issue_trackers/redmine_tracker_spec.rb b/spec/models/issue_trackers/redmine_tracker_spec.rb
index 0b9f4b9..fd7b54a 100644
--- a/spec/models/issue_trackers/redmine_tracker_spec.rb
+++ b/spec/models/issue_trackers/redmine_tracker_spec.rb
@@ -2,8 +2,8 @@ require 'spec_helper'
 
 describe RedmineTracker do
   it "should create an issue on Redmine with err params, and set issue link for err" do
-    notice = Factory :notice
-    tracker = Factory :redmine_tracker, :app => notice.err.app, :project_id => 10
+    notice = Factory(:notice)
+    tracker = Factory(:redmine_tracker, :app => notice.err.app, :project_id => 10)
     err = notice.err
     number = 5
     @issue_link = "#{tracker.account}/issues/#{number}.xml?project_id=#{tracker.project_id}"
@@ -22,5 +22,21 @@ describe RedmineTracker do
 
     err.issue_link.should == @issue_link.sub(/\.xml/, '')
   end
+
+  it "should generate a url where a file with line number can be viewed" do
+    t = Factory(:redmine_tracker, :account => 'http://redmine.example.com', :project_id => "errbit")
+    t.url_to_file("/example/file").should ==
+      'http://redmine.example.com/projects/errbit/repository/annotate/example/file'
+    t.url_to_file("/example/file", 25).should ==
+      'http://redmine.example.com/projects/errbit/repository/annotate/example/file#L25'
+  end
+
+  it "should use the alt_project_id to generate a file/linenumber url, if given" do
+    t = Factory(:redmine_tracker, :account => 'http://redmine.example.com',
+                                  :project_id => "errbit",
+                                  :alt_project_id => "actual_project")
+    t.url_to_file("/example/file", 25).should ==
+      'http://redmine.example.com/projects/actual_project/repository/annotate/example/file#L25'
+  end
 end
 
--
libgit2 0.21.2