Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix for Issue #23 (unable to find custom sections) #50

Merged
merged 2 commits into from

2 participants

@scrozier

(This is my first pull request, so please be gentle with me!)

It looks like this was just an oversight, neglecting to put the exception_notifier directory at the end of the specification for the custom section path.

I have tested it in my application and it is working.

If you'd like me to write a test for this specifically, let me know.

@smartinez87
Owner

Yes, please add a test for this and I'll merge it.

@scrozier

OK, the root cause of this issue turned out to be a little different than I thought at first.

It looks like when ExceptionNotifier::Notifier is loaded, Rails.root has not yet been defined (maybe a Rails 2 -> Rails 3 change?). Consequently, the app level location for views (where new view partials are to be found) is not correctly appended to view_paths.

I tried a couple of approaches, but wound up appending the app level location to view_paths in ExceptionNotifier::Notifier#exception_notification, essentially just before the view is actually needed.

I added a test (and revised a related one) that fails on the current master branch and succeeds on the code represented by this pull request.

Hope this is satisfactory!

@smartinez87
Owner

Yes, I don't quite like appending to view_path in the very method, but as of now I don't see any other better solution to it.
Thanks for the patch!

@smartinez87 smartinez87 merged commit 20122d2 into smartinez87:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 5, 2012
  1. @scrozier
Commits on Jan 6, 2012
  1. @scrozier
This page is out of date. Refresh to see the latest.
View
3  lib/exception_notifier/notifier.rb
@@ -6,7 +6,6 @@ class Notifier < ActionMailer::Base
self.mailer_name = 'exception_notifier'
#Append application view path to the ExceptionNotifier lookup context.
- self.append_view_path Rails.root.nil? ? "app/views" : "#{Rails.root}/app/views" if defined?(Rails)
self.append_view_path "#{File.dirname(__FILE__)}/views"
class << self
@@ -57,6 +56,8 @@ def method_missing(*args, &block)
end
def exception_notification(env, exception)
+ self.append_view_path Rails.root.nil? ? "app/views" : "#{Rails.root}/app/views" if defined?(Rails)
+
@env = env
@exception = exception
@options = (env['exception_notifier.options'] || {}).reverse_merge(self.class.default_options)
View
1  test/dummy/app/views/exception_notifier/_new_section.text.erb
@@ -0,0 +1 @@
+* New section for testing
View
3  test/dummy/config/environment.rb
@@ -4,7 +4,8 @@
Dummy::Application.config.middleware.use ExceptionNotifier,
:email_prefix => "[Dummy ERROR] ",
:sender_address => %{"Dummy Notifier" <dummynotifier@example.com>},
- :exception_recipients => %w{dummyexceptions@example.com}
+ :exception_recipients => %w{dummyexceptions@example.com},
+ :sections => ['new_section', 'request', 'session', 'environment', 'backtrace']
# Initialize the rails application
Dummy::Application.initialize!
View
4 test/dummy/test/functional/posts_controller_test.rb
@@ -38,6 +38,10 @@ class PostsControllerTest < ActionController::TestCase
test "mail should contain timestamp of exception in body" do
assert @mail.body.include? "Timestamp : #{Time.now}"
end
+
+ test "mail should contain the newly defined section" do
+ assert @mail.body.include? "* New section for testing"
+ end
test "should filter sensible data" do
assert @mail.body.include? "secret\"=>\"[FILTERED]"
View
5 test/exception_notification_test.rb
@@ -14,7 +14,9 @@ class ExceptionNotificationTest < ActiveSupport::TestCase
end
test "should have default sections" do
- assert ExceptionNotifier::Notifier.default_sections == %w(request session environment backtrace)
+ for section in %w(request session environment backtrace)
+ assert ExceptionNotifier::Notifier.default_sections.include? section
+ end
end
test "should have default background sections" do
@@ -28,4 +30,5 @@ class ExceptionNotificationTest < ActiveSupport::TestCase
test "should have ignored crawler by default" do
assert ExceptionNotifier.default_ignore_crawlers == []
end
+
end
Something went wrong with that request. Please try again.