From ced5a311fe743b262a8ed936b7cbf52ad8834dba Mon Sep 17 00:00:00 2001 From: Barry Hess Date: Mon, 27 Jun 2011 16:08:03 -0500 Subject: [PATCH] Added ability to tie an app to a GitHub repository. Notices then include links to the file in the GitHub repo. --- app/helpers/errs_helper.rb | 8 ++++++++ app/models/app.rb | 22 ++++++++++++++++++++-- app/models/notice.rb | 6 +++++- app/views/apps/_fields.html.haml | 4 ++++ app/views/apps/show.html.haml | 10 ++++++++++ app/views/notices/_summary.html.haml | 4 ++++ spec/models/app_spec.rb | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ spec/models/notice_spec.rb | 31 ++++++++++++++++++++++++++++++- 8 files changed, 130 insertions(+), 4 deletions(-) diff --git a/app/helpers/errs_helper.rb b/app/helpers/errs_helper.rb index a84e12d..d772739 100644 --- a/app/helpers/errs_helper.rb +++ b/app/helpers/errs_helper.rb @@ -7,4 +7,12 @@ module ErrsHelper def err_confirm Errbit::Config.confirm_resolve_err === false ? nil : 'Seriously?' end + + def link_to_github app, notice + file_name = notice.top_in_app_backtrace_line['file'].split('/').last + file_path = notice.top_in_app_backtrace_line['file'].gsub('[PROJECT_ROOT]', '') + line_number = notice.top_in_app_backtrace_line['number'] + link_to(file_name, "#{app.github_url_to_file(file_path)}#L#{line_number}", :target => '_blank') + end + end \ No newline at end of file diff --git a/app/models/app.rb b/app/models/app.rb index 4771706..ed11466 100644 --- a/app/models/app.rb +++ b/app/models/app.rb @@ -4,6 +4,7 @@ class App field :name, :type => String field :api_key + field :github_url field :resolve_errs_on_deploy, :type => Boolean, :default => false field :notify_on_errs, :type => Boolean, :default => true field :notify_on_deploys, :type => Boolean, :default => true @@ -23,7 +24,8 @@ class App references_many :errs, :dependent => :destroy before_validation :generate_api_key, :on => :create - + before_save :normalize_github_url + validates_presence_of :name, :api_key validates_uniqueness_of :name, :allow_blank => true validates_uniqueness_of :api_key, :allow_blank => true @@ -58,7 +60,15 @@ class App !(self[:notify_on_deploys] == false) end alias :notify_on_deploys? :notify_on_deploys - + + def github_url? + self.github_url.present? + end + + def github_url_to_file(file) + "#{self.github_url}/blob/master#{file}" + end + protected def generate_api_key @@ -73,4 +83,12 @@ class App end if issue_tracker.errors end end + + def normalize_github_url + return if self.github_url.blank? + self.github_url.gsub!(%r{^http://|git@}, 'https://') + self.github_url.gsub!(/github\.com:/, 'github.com/') + self.github_url.gsub!(/\.git$/, '') + end + end diff --git a/app/models/notice.rb b/app/models/notice.rb index 8c3fc9c..5696b45 100644 --- a/app/models/notice.rb +++ b/app/models/notice.rb @@ -79,7 +79,11 @@ class Notice def cache_last_notice_at err.update_attributes(:last_notice_at => created_at) end - + + def top_in_app_backtrace_line + @top_in_app_backtrace_line ||= self.backtrace.find {|line| line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))} } + end + protected def should_notify? diff --git a/app/views/apps/_fields.html.haml b/app/views/apps/_fields.html.haml index 43a2267..ef5601c 100644 --- a/app/views/apps/_fields.html.haml +++ b/app/views/apps/_fields.html.haml @@ -8,6 +8,10 @@ = f.check_box :notify_on_errs = f.label :notify_on_errs, 'Notify on errors' +%div + = f.label :github_url + = f.text_field :github_url + %div.checkbox = f.check_box :resolve_errs_on_deploy = f.label :resolve_errs_on_deploy, 'Resolve errs on deploy' diff --git a/app/views/apps/show.html.haml b/app/views/apps/show.html.haml index 200ff75..2a46297 100644 --- a/app/views/apps/show.html.haml +++ b/app/views/apps/show.html.haml @@ -27,6 +27,16 @@ %td %em Sadly, no one is watching this app +- if @app.github_url? + %h3 Repository + %table.repository + %thead + %tr + %th GitHub + %tbody + %tr + %td= link_to(@app.github_url, @app.github_url, :target => '_blank') + %h3 Latest Deploys - if @deploys.any? %table.deploys diff --git a/app/views/notices/_summary.html.haml b/app/views/notices/_summary.html.haml index 904b486..22ae1aa 100644 --- a/app/views/notices/_summary.html.haml +++ b/app/views/notices/_summary.html.haml @@ -19,3 +19,7 @@ %tr %th Browser %td= user_agent_graph(notice.err) + - if @app.github_url? + %tr + %th GitHub + %td= link_to_github(@app, notice) diff --git a/spec/models/app_spec.rb b/spec/models/app_spec.rb index 8233101..90ed46f 100644 --- a/spec/models/app_spec.rb +++ b/spec/models/app_spec.rb @@ -36,6 +36,55 @@ describe App do app = Factory(:app) app.api_key.should match(/^[a-f0-9]{32}$/) end + + it 'is fine with blank github urls' do + app = Factory.build(:app, :github_url => "") + app.save + app.github_url.should == "" + end + + it 'does not touch https github urls' do + app = Factory.build(:app, :github_url => "https://github.com/jdpace/errbit") + app.save + app.github_url.should == "https://github.com/jdpace/errbit" + end + + it 'normalizes http github urls' do + app = Factory.build(:app, :github_url => "http://github.com/jdpace/errbit") + app.save + app.github_url.should == "https://github.com/jdpace/errbit" + end + + it 'normalizes public git repo as a github url' do + app = Factory.build(:app, :github_url => "https://github.com/jdpace/errbit.git") + app.save + app.github_url.should == "https://github.com/jdpace/errbit" + end + + it 'normalizes private git repo as a github url' do + app = Factory.build(:app, :github_url => "git@github.com:jdpace/errbit.git") + app.save + app.github_url.should == "https://github.com/jdpace/errbit" + end + end + + context '#github_url_to_file' do + it 'resolves to full path to file' do + app = Factory(:app, :github_url => "https://github.com/jdpace/errbit") + app.github_url_to_file('/path/to/file').should == "https://github.com/jdpace/errbit/blob/master/path/to/file" + end + end + + context '#github_url?' do + it 'is true when there is a github_url' do + app = Factory(:app, :github_url => "https://github.com/jdpace/errbit") + app.github_url?.should be_true + end + + it 'is false when no github_url' do + app = Factory(:app) + app.github_url?.should be_false + end end end diff --git a/spec/models/notice_spec.rb b/spec/models/notice_spec.rb index 521bdf4..d0352da 100644 --- a/spec/models/notice_spec.rb +++ b/spec/models/notice_spec.rb @@ -21,7 +21,36 @@ describe Notice do notice.errors[:notifier].should include("can't be blank") end end - + + context '#top_in_app_backtrace_line' do + before do + backtrace = [{ + 'number' => rand(999), + 'file' => '[GEM_ROOT]/gems/actionpack-3.0.4/lib/action_controller/metal/rescue.rb', + 'method' => ActiveSupport.methods.shuffle.first + }, { + 'number' => rand(999), + 'file' => '[PROJECT_ROOT]/vendor/plugins/seamless_database_pool/lib/seamless_database_pool/controller_filter.rb', + 'method' => ActiveSupport.methods.shuffle.first + }, { + 'number' => rand(999), + 'file' => '[PROJECT_ROOT]/lib/set_headers.rb', + 'method' => ActiveSupport.methods.shuffle.first + }, { + 'number' => rand(999), + 'file' => '[PROJECT_ROOT]/lib/detect_api.rb', + 'method' => ActiveSupport.methods.shuffle.first + }] + + @notice = Factory(:notice, :backtrace => backtrace) + end + + it 'finds the correct line' do + line = @notice.top_in_app_backtrace_line + line['file'].should == '[PROJECT_ROOT]/lib/set_headers.rb' + end + end + context '#from_xml' do before do @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read -- libgit2 0.21.2