Commit de5ece0388dfc9e0577d8e3241befccae9019acb
Exists in
master
and in
1 other branch
Merge branch 'github_links' of https://github.com/harvesthq/errbit into pull-request-39
Showing
8 changed files
with
132 additions
and
6 deletions
Show diff stats
app/helpers/errs_helper.rb
| @@ -7,4 +7,12 @@ module ErrsHelper | @@ -7,4 +7,12 @@ module ErrsHelper | ||
| 7 | def err_confirm | 7 | def err_confirm |
| 8 | Errbit::Config.confirm_resolve_err === false ? nil : 'Seriously?' | 8 | Errbit::Config.confirm_resolve_err === false ? nil : 'Seriously?' |
| 9 | end | 9 | 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 | + | ||
| 10 | end | 18 | end |
| 11 | \ No newline at end of file | 19 | \ No newline at end of file |
app/models/app.rb
| @@ -4,6 +4,7 @@ class App | @@ -4,6 +4,7 @@ class App | ||
| 4 | 4 | ||
| 5 | field :name, :type => String | 5 | field :name, :type => String |
| 6 | field :api_key | 6 | field :api_key |
| 7 | + field :github_url | ||
| 7 | field :resolve_errs_on_deploy, :type => Boolean, :default => false | 8 | field :resolve_errs_on_deploy, :type => Boolean, :default => false |
| 8 | field :notify_on_errs, :type => Boolean, :default => true | 9 | field :notify_on_errs, :type => Boolean, :default => true |
| 9 | field :notify_on_deploys, :type => Boolean, :default => true | 10 | field :notify_on_deploys, :type => Boolean, :default => true |
| @@ -23,7 +24,8 @@ class App | @@ -23,7 +24,8 @@ class App | ||
| 23 | references_many :errs, :dependent => :destroy | 24 | references_many :errs, :dependent => :destroy |
| 24 | 25 | ||
| 25 | before_validation :generate_api_key, :on => :create | 26 | before_validation :generate_api_key, :on => :create |
| 26 | - | 27 | + before_save :normalize_github_url |
| 28 | + | ||
| 27 | validates_presence_of :name, :api_key | 29 | validates_presence_of :name, :api_key |
| 28 | validates_uniqueness_of :name, :allow_blank => true | 30 | validates_uniqueness_of :name, :allow_blank => true |
| 29 | validates_uniqueness_of :api_key, :allow_blank => true | 31 | validates_uniqueness_of :api_key, :allow_blank => true |
| @@ -58,7 +60,15 @@ class App | @@ -58,7 +60,15 @@ class App | ||
| 58 | !(self[:notify_on_deploys] == false) | 60 | !(self[:notify_on_deploys] == false) |
| 59 | end | 61 | end |
| 60 | alias :notify_on_deploys? :notify_on_deploys | 62 | alias :notify_on_deploys? :notify_on_deploys |
| 61 | - | 63 | + |
| 64 | + def github_url? | ||
| 65 | + self.github_url.present? | ||
| 66 | + end | ||
| 67 | + | ||
| 68 | + def github_url_to_file(file) | ||
| 69 | + "#{self.github_url}/blob/master#{file}" | ||
| 70 | + end | ||
| 71 | + | ||
| 62 | protected | 72 | protected |
| 63 | 73 | ||
| 64 | def generate_api_key | 74 | def generate_api_key |
| @@ -73,4 +83,12 @@ class App | @@ -73,4 +83,12 @@ class App | ||
| 73 | end if issue_tracker.errors | 83 | end if issue_tracker.errors |
| 74 | end | 84 | end |
| 75 | end | 85 | end |
| 86 | + | ||
| 87 | + def normalize_github_url | ||
| 88 | + return if self.github_url.blank? | ||
| 89 | + self.github_url.gsub!(%r{^http://|git@}, 'https://') | ||
| 90 | + self.github_url.gsub!(/github\.com:/, 'github.com/') | ||
| 91 | + self.github_url.gsub!(/\.git$/, '') | ||
| 92 | + end | ||
| 93 | + | ||
| 76 | end | 94 | end |
app/models/notice.rb
| @@ -56,6 +56,10 @@ class Notice | @@ -56,6 +56,10 @@ class Notice | ||
| 56 | agent_string.blank? ? nil : UserAgent.parse(agent_string) | 56 | agent_string.blank? ? nil : UserAgent.parse(agent_string) |
| 57 | end | 57 | end |
| 58 | 58 | ||
| 59 | + def self.in_app_backtrace_line? line | ||
| 60 | + line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))} | ||
| 61 | + end | ||
| 62 | + | ||
| 59 | def request | 63 | def request |
| 60 | read_attribute(:request) || {} | 64 | read_attribute(:request) || {} |
| 61 | end | 65 | end |
| @@ -79,7 +83,11 @@ class Notice | @@ -79,7 +83,11 @@ class Notice | ||
| 79 | def cache_last_notice_at | 83 | def cache_last_notice_at |
| 80 | err.update_attributes(:last_notice_at => created_at) | 84 | err.update_attributes(:last_notice_at => created_at) |
| 81 | end | 85 | end |
| 82 | - | 86 | + |
| 87 | + def top_in_app_backtrace_line | ||
| 88 | + @top_in_app_backtrace_line ||= self.backtrace.find {|line| Notice.in_app_backtrace_line?(line) } | ||
| 89 | + end | ||
| 90 | + | ||
| 83 | protected | 91 | protected |
| 84 | 92 | ||
| 85 | def should_notify? | 93 | def should_notify? |
| @@ -118,4 +126,3 @@ class Notice | @@ -118,4 +126,3 @@ class Notice | ||
| 118 | end | 126 | end |
| 119 | end | 127 | end |
| 120 | end | 128 | end |
| 121 | - |
app/views/apps/_fields.html.haml
| @@ -8,6 +8,10 @@ | @@ -8,6 +8,10 @@ | ||
| 8 | = f.check_box :notify_on_errs | 8 | = f.check_box :notify_on_errs |
| 9 | = f.label :notify_on_errs, 'Notify on errors' | 9 | = f.label :notify_on_errs, 'Notify on errors' |
| 10 | 10 | ||
| 11 | +%div | ||
| 12 | + = f.label :github_url | ||
| 13 | + = f.text_field :github_url | ||
| 14 | + | ||
| 11 | %div.checkbox | 15 | %div.checkbox |
| 12 | = f.check_box :resolve_errs_on_deploy | 16 | = f.check_box :resolve_errs_on_deploy |
| 13 | = f.label :resolve_errs_on_deploy, 'Resolve errs on deploy' | 17 | = f.label :resolve_errs_on_deploy, 'Resolve errs on deploy' |
app/views/apps/show.html.haml
| @@ -27,6 +27,16 @@ | @@ -27,6 +27,16 @@ | ||
| 27 | %td | 27 | %td |
| 28 | %em Sadly, no one is watching this app | 28 | %em Sadly, no one is watching this app |
| 29 | 29 | ||
| 30 | +- if @app.github_url? | ||
| 31 | + %h3 Repository | ||
| 32 | + %table.repository | ||
| 33 | + %thead | ||
| 34 | + %tr | ||
| 35 | + %th GitHub | ||
| 36 | + %tbody | ||
| 37 | + %tr | ||
| 38 | + %td= link_to(@app.github_url, @app.github_url, :target => '_blank') | ||
| 39 | + | ||
| 30 | %h3 Latest Deploys | 40 | %h3 Latest Deploys |
| 31 | - if @deploys.any? | 41 | - if @deploys.any? |
| 32 | %table.deploys | 42 | %table.deploys |
app/views/notices/_backtrace.html.haml
| @@ -4,7 +4,8 @@ | @@ -4,7 +4,8 @@ | ||
| 4 | %th | 4 | %th |
| 5 | %ul.line-numbers | 5 | %ul.line-numbers |
| 6 | - lines.each do |line| | 6 | - lines.each do |line| |
| 7 | - %li= line['number'] | 7 | + %li= (@app.github_url? && Notice.in_app_backtrace_line?(line)) ? link_to_github(@app, line, line['number']) : line['number'] |
| 8 | + | ||
| 8 | %td | 9 | %td |
| 9 | %ul.lines | 10 | %ul.lines |
| 10 | - lines.each do |line| | 11 | - lines.each do |line| |
spec/models/app_spec.rb
| @@ -36,6 +36,55 @@ describe App do | @@ -36,6 +36,55 @@ describe App do | ||
| 36 | app = Factory(:app) | 36 | app = Factory(:app) |
| 37 | app.api_key.should match(/^[a-f0-9]{32}$/) | 37 | app.api_key.should match(/^[a-f0-9]{32}$/) |
| 38 | end | 38 | end |
| 39 | + | ||
| 40 | + it 'is fine with blank github urls' do | ||
| 41 | + app = Factory.build(:app, :github_url => "") | ||
| 42 | + app.save | ||
| 43 | + app.github_url.should == "" | ||
| 44 | + end | ||
| 45 | + | ||
| 46 | + it 'does not touch https github urls' do | ||
| 47 | + app = Factory.build(:app, :github_url => "https://github.com/jdpace/errbit") | ||
| 48 | + app.save | ||
| 49 | + app.github_url.should == "https://github.com/jdpace/errbit" | ||
| 50 | + end | ||
| 51 | + | ||
| 52 | + it 'normalizes http github urls' do | ||
| 53 | + app = Factory.build(:app, :github_url => "http://github.com/jdpace/errbit") | ||
| 54 | + app.save | ||
| 55 | + app.github_url.should == "https://github.com/jdpace/errbit" | ||
| 56 | + end | ||
| 57 | + | ||
| 58 | + it 'normalizes public git repo as a github url' do | ||
| 59 | + app = Factory.build(:app, :github_url => "https://github.com/jdpace/errbit.git") | ||
| 60 | + app.save | ||
| 61 | + app.github_url.should == "https://github.com/jdpace/errbit" | ||
| 62 | + end | ||
| 63 | + | ||
| 64 | + it 'normalizes private git repo as a github url' do | ||
| 65 | + app = Factory.build(:app, :github_url => "git@github.com:jdpace/errbit.git") | ||
| 66 | + app.save | ||
| 67 | + app.github_url.should == "https://github.com/jdpace/errbit" | ||
| 68 | + end | ||
| 69 | + end | ||
| 70 | + | ||
| 71 | + context '#github_url_to_file' do | ||
| 72 | + it 'resolves to full path to file' do | ||
| 73 | + app = Factory(:app, :github_url => "https://github.com/jdpace/errbit") | ||
| 74 | + app.github_url_to_file('/path/to/file').should == "https://github.com/jdpace/errbit/blob/master/path/to/file" | ||
| 75 | + end | ||
| 76 | + end | ||
| 77 | + | ||
| 78 | + context '#github_url?' do | ||
| 79 | + it 'is true when there is a github_url' do | ||
| 80 | + app = Factory(:app, :github_url => "https://github.com/jdpace/errbit") | ||
| 81 | + app.github_url?.should be_true | ||
| 82 | + end | ||
| 83 | + | ||
| 84 | + it 'is false when no github_url' do | ||
| 85 | + app = Factory(:app) | ||
| 86 | + app.github_url?.should be_false | ||
| 87 | + end | ||
| 39 | end | 88 | end |
| 40 | 89 | ||
| 41 | end | 90 | end |
spec/models/notice_spec.rb
| @@ -21,7 +21,36 @@ describe Notice do | @@ -21,7 +21,36 @@ describe Notice do | ||
| 21 | notice.errors[:notifier].should include("can't be blank") | 21 | notice.errors[:notifier].should include("can't be blank") |
| 22 | end | 22 | end |
| 23 | end | 23 | end |
| 24 | - | 24 | + |
| 25 | + context '#top_in_app_backtrace_line' do | ||
| 26 | + before do | ||
| 27 | + backtrace = [{ | ||
| 28 | + 'number' => rand(999), | ||
| 29 | + 'file' => '[GEM_ROOT]/gems/actionpack-3.0.4/lib/action_controller/metal/rescue.rb', | ||
| 30 | + 'method' => ActiveSupport.methods.shuffle.first | ||
| 31 | + }, { | ||
| 32 | + 'number' => rand(999), | ||
| 33 | + 'file' => '[PROJECT_ROOT]/vendor/plugins/seamless_database_pool/lib/seamless_database_pool/controller_filter.rb', | ||
| 34 | + 'method' => ActiveSupport.methods.shuffle.first | ||
| 35 | + }, { | ||
| 36 | + 'number' => rand(999), | ||
| 37 | + 'file' => '[PROJECT_ROOT]/lib/set_headers.rb', | ||
| 38 | + 'method' => ActiveSupport.methods.shuffle.first | ||
| 39 | + }, { | ||
| 40 | + 'number' => rand(999), | ||
| 41 | + 'file' => '[PROJECT_ROOT]/lib/detect_api.rb', | ||
| 42 | + 'method' => ActiveSupport.methods.shuffle.first | ||
| 43 | + }] | ||
| 44 | + | ||
| 45 | + @notice = Factory(:notice, :backtrace => backtrace) | ||
| 46 | + end | ||
| 47 | + | ||
| 48 | + it 'finds the correct line' do | ||
| 49 | + line = @notice.top_in_app_backtrace_line | ||
| 50 | + line['file'].should == '[PROJECT_ROOT]/lib/set_headers.rb' | ||
| 51 | + end | ||
| 52 | + end | ||
| 53 | + | ||
| 25 | context '#from_xml' do | 54 | context '#from_xml' do |
| 26 | before do | 55 | before do |
| 27 | @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | 56 | @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read |