Commit 1993bfe16465b627a03868564243b5f8a6cd838b
1 parent
bf166e7c
Exists in
master
and in
1 other branch
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)
Showing
8 changed files
with
66 additions
and
43 deletions
Show diff stats
app/helpers/errs_helper.rb
| 1 | module ErrsHelper | 1 | module ErrsHelper |
| 2 | - | ||
| 3 | def last_notice_at err | 2 | def last_notice_at err |
| 4 | err.last_notice_at || err.created_at | 3 | err.last_notice_at || err.created_at |
| 5 | end | 4 | end |
| @@ -7,20 +6,5 @@ module ErrsHelper | @@ -7,20 +6,5 @@ module ErrsHelper | ||
| 7 | def err_confirm | 6 | def err_confirm |
| 8 | Errbit::Config.confirm_resolve_err === false ? nil : 'Seriously?' | 7 | Errbit::Config.confirm_resolve_err === false ? nil : 'Seriously?' |
| 9 | end | 8 | end |
| 10 | - | ||
| 11 | - def link_to_github app, line, text=nil | ||
| 12 | - file_name = line['file'].split('/').last | ||
| 13 | - file_path = line['file'].gsub('[PROJECT_ROOT]', '') | ||
| 14 | - line_number = line['number'] | ||
| 15 | - link_to(text || file_name, "#{app.github_url_to_file(file_path)}#L#{line_number}", :target => '_blank') | ||
| 16 | - end | ||
| 17 | - | ||
| 18 | - def link_to_redmine app, line, text=nil | ||
| 19 | - file_name = line['file'].split('/').last | ||
| 20 | - file_path = line['file'].gsub('[PROJECT_ROOT]', '') | ||
| 21 | - line_number = line['number'] | ||
| 22 | - lnk = "%s%s#L%s" % [ app.redmine_url, file_path, line_number] | ||
| 23 | - link_to(text || file_name, lnk, :target => '_blank') | ||
| 24 | - end | ||
| 25 | - | ||
| 26 | end | 9 | end |
| 10 | + |
app/helpers/notices_helper.rb
| @@ -4,16 +4,32 @@ module NoticesHelper | @@ -4,16 +4,32 @@ module NoticesHelper | ||
| 4 | render :partial => "notices/atom_entry.html.haml", :locals => {:notice => notice} | 4 | render :partial => "notices/atom_entry.html.haml", :locals => {:notice => notice} |
| 5 | end | 5 | end |
| 6 | 6 | ||
| 7 | - def render_line_number(app, line) | ||
| 8 | - unless Notice.in_app_backtrace_line?(line) | ||
| 9 | - line['number'] | ||
| 10 | - else | ||
| 11 | - case true | ||
| 12 | - when app.github_url? then link_to_github(app, line, line['number']) | ||
| 13 | - when app.redmine_url? then link_to_redmine(app, line, line['number']) | ||
| 14 | - else | ||
| 15 | - line['number'] | 7 | + def line_number_with_link(app, line) |
| 8 | + return " ".html_safe unless line['number'] | ||
| 9 | + if Notice.in_app_backtrace_line?(line) | ||
| 10 | + return link_to_github(app, line, line['number']) if app.github_url? | ||
| 11 | + if app.issue_tracker && app.issue_tracker.respond_to?(:url_to_file) | ||
| 12 | + # Return link to file on tracker if issue tracker supports this | ||
| 13 | + return link_to_issue_tracker_file(app, line, line['number']) | ||
| 16 | end | 14 | end |
| 17 | end | 15 | end |
| 16 | + line['number'] | ||
| 17 | + end | ||
| 18 | + | ||
| 19 | + def filepath_parts(file) | ||
| 20 | + [file.split('/').last, file.gsub('[PROJECT_ROOT]', '')] | ||
| 21 | + end | ||
| 22 | + | ||
| 23 | + def link_to_github(app, line, text = nil) | ||
| 24 | + file_name, file_path = filepath_parts(line['file']) | ||
| 25 | + href = "%s#L%s" % [app.github_url_to_file(file_path), line['number']] | ||
| 26 | + link_to(text || file_name, href, :target => '_blank') | ||
| 27 | + end | ||
| 28 | + | ||
| 29 | + def link_to_issue_tracker_file(app, line, text = nil) | ||
| 30 | + file_name, file_path = filepath_parts(line['file']) | ||
| 31 | + href = app.issue_tracker.url_to_file(file_path, line['number']) | ||
| 32 | + link_to(text || file_name, href, :target => '_blank') | ||
| 18 | end | 33 | end |
| 19 | end | 34 | end |
| 35 | + |
app/models/app.rb
| @@ -5,7 +5,6 @@ class App | @@ -5,7 +5,6 @@ class App | ||
| 5 | field :name, :type => String | 5 | field :name, :type => String |
| 6 | field :api_key | 6 | field :api_key |
| 7 | field :github_url | 7 | field :github_url |
| 8 | - field :redmine_url | ||
| 9 | field :resolve_errs_on_deploy, :type => Boolean, :default => false | 8 | field :resolve_errs_on_deploy, :type => Boolean, :default => false |
| 10 | field :notify_all_users, :type => Boolean, :default => false | 9 | field :notify_all_users, :type => Boolean, :default => false |
| 11 | field :notify_on_errs, :type => Boolean, :default => true | 10 | field :notify_on_errs, :type => Boolean, :default => true |
| @@ -67,10 +66,6 @@ class App | @@ -67,10 +66,6 @@ class App | ||
| 67 | self.github_url.present? | 66 | self.github_url.present? |
| 68 | end | 67 | end |
| 69 | 68 | ||
| 70 | - def redmine_url? | ||
| 71 | - self.redmine_url.present? | ||
| 72 | - end | ||
| 73 | - | ||
| 74 | def github_url_to_file(file) | 69 | def github_url_to_file(file) |
| 75 | "#{self.github_url}/blob/master#{file}" | 70 | "#{self.github_url}/blob/master#{file}" |
| 76 | end | 71 | end |
app/models/issue_tracker.rb
| @@ -8,6 +8,7 @@ class IssueTracker | @@ -8,6 +8,7 @@ class IssueTracker | ||
| 8 | embedded_in :app, :inverse_of => :issue_tracker | 8 | embedded_in :app, :inverse_of => :issue_tracker |
| 9 | 9 | ||
| 10 | field :project_id, :type => String | 10 | field :project_id, :type => String |
| 11 | + field :alt_project_id, :type => String # Specify an alternative project id. e.g. for viewing files | ||
| 11 | field :api_token, :type => String | 12 | field :api_token, :type => String |
| 12 | field :account, :type => String | 13 | field :account, :type => String |
| 13 | field :username, :type => String | 14 | field :username, :type => String |
app/models/issue_trackers/redmine_tracker.rb
| @@ -8,11 +8,19 @@ class IssueTrackers::RedmineTracker < IssueTracker | @@ -8,11 +8,19 @@ class IssueTrackers::RedmineTracker < IssueTracker | ||
| 8 | [:api_token, { | 8 | [:api_token, { |
| 9 | :placeholder => "API Token for your account" | 9 | :placeholder => "API Token for your account" |
| 10 | }], | 10 | }], |
| 11 | - [:project_id, {}] | 11 | + [:project_id, { |
| 12 | + :label => "Ticket Project", | ||
| 13 | + :placeholder => "Redmine Project where tickets will be created" | ||
| 14 | + }], | ||
| 15 | + [:alt_project_id, { | ||
| 16 | + :optional => true, | ||
| 17 | + :label => "App Project", | ||
| 18 | + :placeholder => "Where app's files & revisions can be viewed. (Leave blank to use the above project by default)" | ||
| 19 | + }] | ||
| 12 | ] | 20 | ] |
| 13 | 21 | ||
| 14 | def check_params | 22 | def check_params |
| 15 | - if Fields.detect {|f| self[f[0]].blank? } | 23 | + if Fields.detect {|f| self[f[0]].blank? && !f[1][:optional]} |
| 16 | errors.add :base, 'You must specify your Redmine URL, API token and Project ID' | 24 | errors.add :base, 'You must specify your Redmine URL, API token and Project ID' |
| 17 | end | 25 | end |
| 18 | end | 26 | end |
| @@ -31,6 +39,13 @@ class IssueTrackers::RedmineTracker < IssueTracker | @@ -31,6 +39,13 @@ class IssueTrackers::RedmineTracker < IssueTracker | ||
| 31 | 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}") | 39 | 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}") |
| 32 | end | 40 | end |
| 33 | 41 | ||
| 42 | + def url_to_file(file_path, line_number = nil) | ||
| 43 | + # alt_project_id let's users specify a different project for tickets / app files. | ||
| 44 | + project = self.alt_project_id.present? ? self.alt_project_id : self.project_id | ||
| 45 | + url = "#{self.account}/projects/#{project}/repository/annotate/#{file_path.sub(/^\//,'')}" | ||
| 46 | + line_number ? url << "#L#{line_number}" : url | ||
| 47 | + end | ||
| 48 | + | ||
| 34 | def body_template | 49 | def body_template |
| 35 | @@body_template ||= ERB.new(File.read(Rails.root + "app/views/issue_trackers/textile_body.txt.erb")) | 50 | @@body_template ||= ERB.new(File.read(Rails.root + "app/views/issue_trackers/textile_body.txt.erb")) |
| 36 | end | 51 | end |
app/models/notice.rb
| @@ -57,7 +57,7 @@ class Notice | @@ -57,7 +57,7 @@ class Notice | ||
| 57 | agent_string.blank? ? nil : UserAgent.parse(agent_string) | 57 | agent_string.blank? ? nil : UserAgent.parse(agent_string) |
| 58 | end | 58 | end |
| 59 | 59 | ||
| 60 | - def self.in_app_backtrace_line? line | 60 | + def self.in_app_backtrace_line?(line) |
| 61 | !!(line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))}) | 61 | !!(line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))}) |
| 62 | end | 62 | end |
| 63 | 63 |
app/views/apps/_fields.html.haml
| @@ -4,13 +4,9 @@ | @@ -4,13 +4,9 @@ | ||
| 4 | = f.label :name | 4 | = f.label :name |
| 5 | = f.text_field :name | 5 | = f.text_field :name |
| 6 | 6 | ||
| 7 | -%fieldset | ||
| 8 | - %legend Links to Codebase | ||
| 9 | - %div | ||
| 10 | - = f.label :github_url | ||
| 11 | - = f.text_field :github_url | ||
| 12 | - = f.label :redmine_url | ||
| 13 | - = f.text_field :redmine_url | 7 | +%div |
| 8 | + = f.label :github_url | ||
| 9 | + = f.text_field :github_url | ||
| 14 | 10 | ||
| 15 | %fieldset | 11 | %fieldset |
| 16 | %legend Notifications | 12 | %legend Notifications |
spec/models/issue_trackers/redmine_tracker_spec.rb
| @@ -2,8 +2,8 @@ require 'spec_helper' | @@ -2,8 +2,8 @@ require 'spec_helper' | ||
| 2 | 2 | ||
| 3 | describe RedmineTracker do | 3 | describe RedmineTracker do |
| 4 | it "should create an issue on Redmine with err params, and set issue link for err" do | 4 | it "should create an issue on Redmine with err params, and set issue link for err" do |
| 5 | - notice = Factory :notice | ||
| 6 | - tracker = Factory :redmine_tracker, :app => notice.err.app, :project_id => 10 | 5 | + notice = Factory(:notice) |
| 6 | + tracker = Factory(:redmine_tracker, :app => notice.err.app, :project_id => 10) | ||
| 7 | err = notice.err | 7 | err = notice.err |
| 8 | number = 5 | 8 | number = 5 |
| 9 | @issue_link = "#{tracker.account}/issues/#{number}.xml?project_id=#{tracker.project_id}" | 9 | @issue_link = "#{tracker.account}/issues/#{number}.xml?project_id=#{tracker.project_id}" |
| @@ -22,5 +22,21 @@ describe RedmineTracker do | @@ -22,5 +22,21 @@ describe RedmineTracker do | ||
| 22 | 22 | ||
| 23 | err.issue_link.should == @issue_link.sub(/\.xml/, '') | 23 | err.issue_link.should == @issue_link.sub(/\.xml/, '') |
| 24 | end | 24 | end |
| 25 | + | ||
| 26 | + it "should generate a url where a file with line number can be viewed" do | ||
| 27 | + t = Factory(:redmine_tracker, :account => 'http://redmine.example.com', :project_id => "errbit") | ||
| 28 | + t.url_to_file("/example/file").should == | ||
| 29 | + 'http://redmine.example.com/projects/errbit/repository/annotate/example/file' | ||
| 30 | + t.url_to_file("/example/file", 25).should == | ||
| 31 | + 'http://redmine.example.com/projects/errbit/repository/annotate/example/file#L25' | ||
| 32 | + end | ||
| 33 | + | ||
| 34 | + it "should use the alt_project_id to generate a file/linenumber url, if given" do | ||
| 35 | + t = Factory(:redmine_tracker, :account => 'http://redmine.example.com', | ||
| 36 | + :project_id => "errbit", | ||
| 37 | + :alt_project_id => "actual_project") | ||
| 38 | + t.url_to_file("/example/file", 25).should == | ||
| 39 | + 'http://redmine.example.com/projects/actual_project/repository/annotate/example/file#L25' | ||
| 40 | + end | ||
| 25 | end | 41 | end |
| 26 | 42 |