Merge pull request thoughtworks#71 from SealedEnvelope/raw-output-in-…

…notifier

Don't html escape build changeset, output and project settings in the build report email.

Happily merged. Thanks for the pull request, and thanks for including a test!

app/views/build_mailer/build_report.html.erb

@@ -3,7 +3,7 @@
 
 CHANGES
 -------
-<%= @build.changeset %>
+<%= raw @build.changeset %>
 
 <% unless @failures_and_errors.empty? -%>
 TEST FAILURES AND ERRORS
@@ -19,7 +19,7 @@ See <%= "#{@build.url}" %> for details.
 
 CHANGES
 -------
-<%= @build.changeset %>
+<%= raw @build.changeset %>
 
 BUILD LOG
 ---------
@@ -28,9 +28,9 @@ The output below has been truncated to <%= number_to_human_size(Configuration.ma
 View <%= link_to "the full log", build_artifact_path(:project => @build.project.name, :build => @build.label, :path => 'build.log') %> for the rest.
 <% end %>
 
-<%= @build.output %>
+<%= raw @build.output %>
 
 PROJECT SETTINGS
 ----------------
-<%= @build.project_settings %>
+<%= raw @build.project_settings %>
 <% end -%>

test/unit/plugins/email_notifier_test.rb

@@ -2,11 +2,12 @@
 
 class EmailNotifierTest < ActiveSupport::TestCase
   include FileSandbox
-  
+
   BUILD_LOG = <<-EOL
     blah blah blah
     something built
     tests passed / failed / etc
+    <this should be plain text & should not be escaped>
   EOL
 
   setup do
@@ -22,10 +23,10 @@ class EmailNotifierTest < ActiveSupport::TestCase
     @notifier = EmailNotifier.new
     @notifier.emails = ["default@gmail.com", "ccrb@thoughtworks.com"]
     @notifier.from = 'cruisecontrol@thoughtworks.com'
-    
+
     @project.add_plugin(@notifier, :test_email_notifier)
   end
-  
+
   teardown do
     teardown_sandbox
   end
@@ -61,7 +62,7 @@ class EmailNotifierTest < ActiveSupport::TestCase
       CruiseControl::Log.expects(:event).never
       BuildMailer.expects(:build_report).never
       @notifier.emails = []
-      @notifier.build_finished(failing_build)      
+      @notifier.build_finished(failing_build)
     end
 
     test "should log a single recipient if an email is sent" do
@@ -83,11 +84,11 @@ class EmailNotifierTest < ActiveSupport::TestCase
       CruiseControl::Log.expects(:event).with("Error sending e-mail - current server settings are :\n  :foo = 5", :error)
       mock_mail = mock("Email")
       mock_mail.expects(:deliver).raises('oh noes!')
-      
+
       BuildMailer.expects(:build_report).returns mock_mail
-      
+
       @notifier.emails = ['foo@crapty.com']
-      
+
       assert_raise_with_message(RuntimeError, 'oh noes!') do
         @notifier.build_finished(failing_build)
       end
@@ -97,7 +98,7 @@ class EmailNotifierTest < ActiveSupport::TestCase
       Configuration.expects(:email_from).returns('central@foo.com')
       @notifier.from = nil
       build = failing_build()
-      
+
       BuildMailer.expects(:build_report).with(build, ['default@gmail.com', 'ccrb@thoughtworks.com'],
                           'central@foo.com', 'myproj build 5 failed', 'The build failed.').returns mock(:deliver => true)
 
@@ -108,7 +109,7 @@ class EmailNotifierTest < ActiveSupport::TestCase
       Configuration.stubs(:dashboard_url).returns("http://www.my.com")
       @notifier.emails = ['foo@happy.com']
       @notifier.build_finished(failing_build)
-      
+
       mail = ActionMailer::Base.deliveries[0]
       assert_match /http:\/\/www.my.com\/builds\/myproj\/5/, mail.body.to_s
     end
@@ -121,10 +122,19 @@ class EmailNotifierTest < ActiveSupport::TestCase
       mail = ActionMailer::Base.deliveries[0]
       assert_match /Note: if you set Configuration\.dashboard_url in site_config\.rb/, mail.body.to_s
     end
+
+    test "should contain unescaped build output in the notification email if the dashboard url is not set" do
+      Configuration.stubs(:dashboard_url).returns(nil)
+      @notifier.emails = ['foo@happy.com']
+      @notifier.build_finished(failing_build)
+
+      mail = ActionMailer::Base.deliveries[0]
+      assert_match /<this should be plain text & should not be escaped>/, mail.body.to_s
+    end
   end
 
   private
-  
+
   def failing_build
     @build.stubs(:failed?).returns(true)
     @build.stubs(:output).returns(BUILD_LOG)