Commit ced5a311fe743b262a8ed936b7cbf52ad8834dba
1 parent
3a3e21a5
Exists in
master
and in
1 other branch
Added ability to tie an app to a GitHub repository. Notices then include links t…
…o the file in the GitHub repo. Conflicts: app/helpers/errs_helper.rb app/models/app.rb app/models/notice.rb app/views/apps/_fields.html.haml app/views/notices/_summary.html.haml spec/models/notice_spec.rb
Showing
8 changed files
with
130 additions
and
4 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, notice | ||
| 12 | + file_name = notice.top_in_app_backtrace_line['file'].split('/').last | ||
| 13 | + file_path = notice.top_in_app_backtrace_line['file'].gsub('[PROJECT_ROOT]', '') | ||
| 14 | + line_number = notice.top_in_app_backtrace_line['number'] | ||
| 15 | + link_to(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
| @@ -79,7 +79,11 @@ class Notice | @@ -79,7 +79,11 @@ class Notice | ||
| 79 | def cache_last_notice_at | 79 | def cache_last_notice_at |
| 80 | err.update_attributes(:last_notice_at => created_at) | 80 | err.update_attributes(:last_notice_at => created_at) |
| 81 | end | 81 | end |
| 82 | - | 82 | + |
| 83 | + def top_in_app_backtrace_line | ||
| 84 | + @top_in_app_backtrace_line ||= self.backtrace.find {|line| line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))} } | ||
| 85 | + end | ||
| 86 | + | ||
| 83 | protected | 87 | protected |
| 84 | 88 | ||
| 85 | def should_notify? | 89 | def should_notify? |
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/_summary.html.haml
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 |