Commit f2f02b6ca346f67f96d076b0cbbad93c11c9d6bc
1 parent
07cd269d
Exists in
master
and in
1 other branch
Add links to source files in notification emails
Showing
5 changed files
with
23 additions
and
9 deletions
Show diff stats
app/mailers/mailer.rb
@@ -4,6 +4,7 @@ require Rails.root.join('config/routes.rb') | @@ -4,6 +4,7 @@ require Rails.root.join('config/routes.rb') | ||
4 | 4 | ||
5 | class Mailer < ActionMailer::Base | 5 | class Mailer < ActionMailer::Base |
6 | helper ApplicationHelper | 6 | helper ApplicationHelper |
7 | + helper BacktraceLineHelper | ||
7 | 8 | ||
8 | default :from => Errbit::Config.email_from | 9 | default :from => Errbit::Config.email_from |
9 | 10 |
app/models/backtrace_line.rb
@@ -15,7 +15,7 @@ class BacktraceLine | @@ -15,7 +15,7 @@ class BacktraceLine | ||
15 | delegate :app, :to => :backtrace | 15 | delegate :app, :to => :backtrace |
16 | 16 | ||
17 | def to_s | 17 | def to_s |
18 | - "#{file}:#{number}" << (column.present? ? ":#{column}" : "") | 18 | + "#{file_relative}:#{number}" << (column.present? ? ":#{column}" : "") |
19 | end | 19 | end |
20 | 20 | ||
21 | def in_app? | 21 | def in_app? |
app/views/mailer/err_notification.html.haml
@@ -28,7 +28,9 @@ | @@ -28,7 +28,9 @@ | ||
28 | %p.monospace | 28 | %p.monospace |
29 | = @notice.where | 29 | = @notice.where |
30 | - @notice.in_app_backtrace_lines.each do |line| | 30 | - @notice.in_app_backtrace_lines.each do |line| |
31 | - %p.backtrace= line | 31 | + %p.backtrace |
32 | + = link_to_source_file(line) do | ||
33 | + = line.to_s | ||
32 | %br | 34 | %br |
33 | %p.heading URL: | 35 | %p.heading URL: |
34 | %p.monospace | 36 | %p.monospace |
@@ -40,6 +42,7 @@ | @@ -40,6 +42,7 @@ | ||
40 | %br | 42 | %br |
41 | %p.heading FULL BACKTRACE: | 43 | %p.heading FULL BACKTRACE: |
42 | - @notice.backtrace_lines.each do |line| | 44 | - @notice.backtrace_lines.each do |line| |
43 | - %p.backtrace= line | 45 | + %p.backtrace |
46 | + = link_to_source_file(line) do | ||
47 | + = line.to_s | ||
44 | %br | 48 | %br |
45 | - |
app/views/notices/_backtrace_line.html.haml
1 | %tr{:class => defined?(row_class) && row_class} | 1 | %tr{:class => defined?(row_class) && row_class} |
2 | %td.line{:class => line.in_app? && 'in-app' } | 2 | %td.line{:class => line.in_app? && 'in-app' } |
3 | = link_to_source_file(line) do | 3 | = link_to_source_file(line) do |
4 | - %span.path>=raw line.decorated_path | 4 | + %span.path>= raw line.decorated_path |
5 | %span.file>= line.file_name | 5 | %span.file>= line.file_name |
6 | - if line.number.present? | 6 | - if line.number.present? |
7 | %span.number>= ":#{line.number}" | 7 | %span.number>= ":#{line.number}" |
spec/mailers/mailer_spec.rb
@@ -6,24 +6,34 @@ describe Mailer do | @@ -6,24 +6,34 @@ describe Mailer do | ||
6 | include EmailSpec::Matchers | 6 | include EmailSpec::Matchers |
7 | 7 | ||
8 | let(:notice) { Fabricate(:notice, :message => "class < ActionController::Base") } | 8 | let(:notice) { Fabricate(:notice, :message => "class < ActionController::Base") } |
9 | - let!(:email) { Mailer.err_notification(notice).deliver } | 9 | + |
10 | + before do | ||
11 | + notice.backtrace.lines.last.update_attributes(:file => "[PROJECT_ROOT]/path/to/file.js") | ||
12 | + notice.app.update_attributes :asset_host => "http://example.com" | ||
13 | + | ||
14 | + @email = Mailer.err_notification(notice).deliver | ||
15 | + end | ||
10 | 16 | ||
11 | it "should send the email" do | 17 | it "should send the email" do |
12 | ActionMailer::Base.deliveries.size.should == 1 | 18 | ActionMailer::Base.deliveries.size.should == 1 |
13 | end | 19 | end |
14 | 20 | ||
15 | it "should html-escape the notice's message for the html part" do | 21 | it "should html-escape the notice's message for the html part" do |
16 | - email.should have_body_text("class < ActionController::Base") | 22 | + @email.should have_body_text("class < ActionController::Base") |
17 | end | 23 | end |
18 | 24 | ||
19 | it "should have inline css" do | 25 | it "should have inline css" do |
20 | - email.should have_body_text('<p class="backtrace" style="') | 26 | + @email.should have_body_text('<p class="backtrace" style="') |
27 | + end | ||
28 | + | ||
29 | + it "should have links to source files" do | ||
30 | + @email.should have_body_text('<a href="http://example.com/path/to/file.js" target="_blank">path/to/file.js') | ||
21 | end | 31 | end |
22 | 32 | ||
23 | context 'with a very long message' do | 33 | context 'with a very long message' do |
24 | let(:notice) { Fabricate(:notice, :message => 6.times.collect{|a| "0123456789" }.join('')) } | 34 | let(:notice) { Fabricate(:notice, :message => 6.times.collect{|a| "0123456789" }.join('')) } |
25 | it "should truncate the long message" do | 35 | it "should truncate the long message" do |
26 | - email.subject.should =~ / \d{47}\.{3}$/ | 36 | + @email.subject.should =~ / \d{47}\.{3}$/ |
27 | end | 37 | end |
28 | end | 38 | end |
29 | end | 39 | end |