From a5163c7922f34236fae1446332d74462a2058025 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 20 Jan 2011 20:39:12 -0500 Subject: [PATCH] first pass at changing JS html insertion into helper instead --- README.md | 24 ++++++++---- features/rails_with_js_notifier.feature | 23 +++-------- .../rails_application_steps.rb | 5 --- lib/hoptoad_notifier/configuration.rb | 10 ++--- .../rails/javascript_notifier.rb | 37 ++++++++---------- lib/templates/javascript_notifier.erb | 28 ++++++-------- test/configuration_test.rb | 11 +++++- test/javascript_notifier_test.rb | 38 ------------------- 8 files changed, 65 insertions(+), 111 deletions(-) delete mode 100644 test/javascript_notifier_test.rb diff --git a/README.md b/README.md index 04e06c4..74cd006 100644 --- a/README.md +++ b/README.md @@ -398,15 +398,25 @@ not working properly. Javascript Notifer ------------------ -To automatically include the Javascript node on every page, set the -:js_notifier to true: +To automatically include the Javascript node on every page, use this helper method from your layouts: - HoptoadNotifier.configure do |config| - config.js_notifier = true - end + <%= hoptoad_javascript_notifier %> -It automatically uses the API key, host, and port specified in the -configuration. +It's important to insert this very high in the markup, above all other javascript. Example: + + + + + + <%= hoptoad_javascript_notifier %> + + + + ... + + + +This helper will automatically use the API key, host, and port specified in the configuration. Credits ------- diff --git a/features/rails_with_js_notifier.feature b/features/rails_with_js_notifier.feature index 2db80d6..045d18d 100644 --- a/features/rails_with_js_notifier.feature +++ b/features/rails_with_js_notifier.feature @@ -10,11 +10,10 @@ Feature: Install the Gem in a Rails application and enable the JavaScript notifi When I configure the notifier to use the following configuration lines: """ config.api_key = "myapikey" - config.js_notifier = true """ And I define a response for "TestController#index": """ - render :text => '' + render :inline => '<%= hoptoad_javascript_notifier %>' """ And I route "/test/index" to "test#index" And I perform a request to "http://example.com:123/test/index" @@ -24,10 +23,6 @@ Feature: Install the Gem in a Rails application and enable the JavaScript notifi And the notifier JavaScript should provide the following errorDefaults: | url | component | action | | http://example.com:123/test/index | test | index | - And I should see the following value as the html head: - """ - - """ Scenario: Include the Javascript notifier when enabled using custom configuration settings When I generate a new Rails application @@ -38,23 +33,18 @@ Feature: Install the Gem in a Rails application and enable the JavaScript notifi config.api_key = "myapikey!" config.host = "myhoptoad.com" config.port = 3001 - config.js_notifier = true """ And I define a response for "TestController#index": """ - render :text => "" + render :inline => '<%= hoptoad_javascript_notifier %>' """ And I route "/test/index" to "test#index" And I perform a request to "http://example.com:123/test/index" Then I should see the notifier JavaScript for the following: | api_key | environment | host | | myapikey! | production | myhoptoad.com:3001 | - And I should see the following value as the html head: - """ - - """ - Scenario: Don't include the Javascript notifier by default + Scenario: Dont include the Javascript notifier by default When I generate a new Rails application And I configure the Hoptoad shim And I configure my application to require the "hoptoad_notifier" gem @@ -64,25 +54,24 @@ Feature: Install the Gem in a Rails application and enable the JavaScript notifi """ And I define a response for "TestController#index": """ - render :text => "" + render :inline => "" """ And I route "/test/index" to "test#index" And I perform a request to "http://example.com:123/test/index" Then I should not see notifier JavaScript - Scenario: Don't include the Javascript notifier when enabled in non-public environments + Scenario: Dont include the Javascript notifier when enabled in non-public environments When I generate a new Rails application And I configure the Hoptoad shim And I configure my application to require the "hoptoad_notifier" gem When I configure the notifier to use the following configuration lines: """ config.api_key = "myapikey!" - config.js_notifier = true config.environment_name = 'test' """ And I define a response for "TestController#index": """ - render :text => "" + render :inline => '<%= hoptoad_javascript_notifier %>' """ And I route "/test/index" to "test#index" And I perform a request to "http://example.com:123/test/index" in the "test" environment diff --git a/features/step_definitions/rails_application_steps.rb b/features/step_definitions/rails_application_steps.rb index 552b82f..ea5dda1 100644 --- a/features/step_definitions/rails_application_steps.rb +++ b/features/step_definitions/rails_application_steps.rb @@ -362,11 +362,6 @@ def rails_non_initializer_hoptoad_config_file end end -Given /^I should see the following value as the html head:$/ do |value| - document_body = '' + @terminal.output.split('').last - document_body.should include(value.strip) -end - Then "the notifier JavaScript should provide the following errorDefaults:" do |table| hash = table.hashes.first diff --git a/lib/hoptoad_notifier/configuration.rb b/lib/hoptoad_notifier/configuration.rb index 25c0de1..91f8b5e 100644 --- a/lib/hoptoad_notifier/configuration.rb +++ b/lib/hoptoad_notifier/configuration.rb @@ -8,7 +8,7 @@ class Configuration :ignore_user_agent, :notifier_name, :notifier_url, :notifier_version, :params_filters, :project_root, :port, :protocol, :proxy_host, :proxy_pass, :proxy_port, :proxy_user, :secure, :framework, - :js_notifier, :user_information].freeze + :user_information].freeze # The API key for your project, found on the project edit form. attr_accessor :api_key @@ -63,9 +63,6 @@ class Configuration # +true+ if you want to check for production errors matching development errors, +false+ otherwise. attr_accessor :development_lookup - # +true+ if you want to enable the JavaScript notifier in production environments - attr_accessor :js_notifier - # The name of the environment the application is running in attr_accessor :environment_name @@ -132,7 +129,6 @@ def initialize @ignore_user_agent = [] @development_environments = %w(development test cucumber) @development_lookup = true - @js_notifier = false @notifier_name = 'Hoptoad Notifier' @notifier_version = VERSION @notifier_url = 'http://hoptoadapp.com' @@ -222,6 +218,10 @@ def protocol end end + def js_notifier=(*args) + warn '[HOPTOAD] config.js_notifier has been deprecated and has no effect. You should use <%= hoptoad_javascript_notifier %> directly at the top of your layouts. Be sure to place it before all other javascript.' + end + def environment_filters warn 'config.environment_filters has been deprecated and has no effect.' [] diff --git a/lib/hoptoad_notifier/rails/javascript_notifier.rb b/lib/hoptoad_notifier/rails/javascript_notifier.rb index ab4509f..cc38e40 100644 --- a/lib/hoptoad_notifier/rails/javascript_notifier.rb +++ b/lib/hoptoad_notifier/rails/javascript_notifier.rb @@ -2,44 +2,39 @@ module HoptoadNotifier module Rails module JavascriptNotifier def self.included(base) #:nodoc: - base.send(:after_filter, :insert_hoptoad_javascript_notifier) + base.send :helper_method, :hoptoad_javascript_notifier end private - def insert_hoptoad_javascript_notifier + def hoptoad_javascript_notifier return unless HoptoadNotifier.configuration.public? - return unless HoptoadNotifier.configuration.js_notifier - path = File.join(File.dirname(__FILE__), '..', '..', 'templates', 'javascript_notifier.erb') + path = File.join File.dirname(__FILE__), '..', '..', 'templates', 'javascript_notifier.erb' host = HoptoadNotifier.configuration.host.dup port = HoptoadNotifier.configuration.port host << ":#{port}" unless [80, 443].include?(port) - options = { - :file => path, - :layout => false, - :use_full_path => false, - :locals => { - :host => host, - :api_key => HoptoadNotifier.configuration.api_key, - :environment => HoptoadNotifier.configuration.environment_name + options = { + :file => path, + :layout => false, + :use_full_path => false, + :locals => { + :host => host, + :api_key => HoptoadNotifier.configuration.api_key, + :environment => HoptoadNotifier.configuration.environment_name, + :action_name => action_name, + :controller_name => controller_name, + :url => request.url } } if @template - javascript = @template.render(options) + @template.render(options) else - javascript = render_to_string(options) + render_to_string(options) end - if response.body.respond_to?(:gsub) - response.body = insert_javascript_after_head response.body, javascript - end - end - - def insert_javascript_after_head(body, javascript) - body.gsub /<(head.*?)>/i, "<\\1>\n#{javascript}\n" end end diff --git a/lib/templates/javascript_notifier.erb b/lib/templates/javascript_notifier.erb index 7764c57..30e563e 100644 --- a/lib/templates/javascript_notifier.erb +++ b/lib/templates/javascript_notifier.erb @@ -1,17 +1,13 @@ - +<%= javascript_tag %Q{ + var notifierJsScheme = (("https:" == document.location.protocol) ? "https://" : "http://"); + document.write(unescape("%3Cscript src='" + notifierJsScheme + "#{host}/javascripts/notifier.js' type='text/javascript'%3E%3C/script%3E")); + } +-%> - +<%= javascript_tag %Q{ + Hoptoad.setKey('#{api_key}'); + Hoptoad.setHost('#{host}'); + Hoptoad.setEnvironment('#{environment}'); + Hoptoad.setErrorDefaults({ url: "#{escape_javascript url}", component: "#{controller_name}", action: "#{action_name}" }); + } +-%> diff --git a/test/configuration_test.rb b/test/configuration_test.rb index eb6b2a5..040b47a 100644 --- a/test/configuration_test.rb +++ b/test/configuration_test.rb @@ -29,7 +29,6 @@ class ConfigurationTest < Test::Unit::TestCase HoptoadNotifier::Configuration::IGNORE_DEFAULT assert_config_default :development_lookup, true assert_config_default :framework, 'Standalone' - assert_config_default :js_notifier, false end should "provide default values for secure connections" do @@ -85,7 +84,7 @@ class ConfigurationTest < Test::Unit::TestCase :http_read_timeout, :ignore, :ignore_by_filters, :ignore_user_agent, :notifier_name, :notifier_url, :notifier_version, :params_filters, :project_root, :port, :protocol, :proxy_host, :proxy_pass, :proxy_port, - :proxy_user, :secure, :development_lookup, :js_notifier].each do |option| + :proxy_user, :secure, :development_lookup].each do |option| assert_equal config[option], hash[option], "Wrong value for #{option}" end end @@ -108,6 +107,14 @@ class ConfigurationTest < Test::Unit::TestCase assert_equal [], config.environment_filters end + should "warn when attempting to write js_notifier" do + config = HoptoadNotifier::Configuration.new + config. + expects(:warn). + with(regexp_matches(/deprecated/i)) + config.js_notifier = true + end + should "allow ignored user agents to be appended" do assert_appends_value :ignore_user_agent end diff --git a/test/javascript_notifier_test.rb b/test/javascript_notifier_test.rb deleted file mode 100644 index 93def69..0000000 --- a/test/javascript_notifier_test.rb +++ /dev/null @@ -1,38 +0,0 @@ -require File.dirname(__FILE__) + '/helper' - -require 'hoptoad_notifier/rails/javascript_notifier' - -class JavascriptNotifierTest < Test::Unit::TestCase - - def self.after_filter(arg); end - include ::HoptoadNotifier::Rails::JavascriptNotifier - - should "insert javascript after head" do - input_body =<<-EOS - - EOS - javascript = "js-code" - expected =<<-EOS - -js-code - - EOS - output = send :insert_javascript_after_head, input_body, javascript - assert_equal expected, output - end - - should "insert javascript after head when head has attributes" do - input_body =<<-EOS - - EOS - javascript = "js-code" - expected =<<-EOS - -js-code - - EOS - output = send :insert_javascript_after_head, input_body, javascript - assert_equal expected, output - end - -end