Refactor: merged error rendering methods.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4286 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 360d091..d7b1add 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -275,39 +275,31 @@ class ApplicationController < ActionController::Base
   
   def render_403(options={})
     @project = nil
-    @message = options[:message] || :notice_not_authorized
-    @message = l(@message) if @message.is_a?(Symbol)
-    respond_to do |format|
-      format.html { render :template => "common/403", :layout => use_layout, :status => 403 }
-      format.atom { head 403 }
-      format.xml { head 403 }
-      format.js { head 403 }
-      format.json { head 403 }
-    end
+    render_error({:message => :notice_not_authorized, :status => 403}.merge(options))
     return false
   end
     
-  def render_404
-    respond_to do |format|
-      format.html { render :template => "common/404", :layout => use_layout, :status => 404 }
-      format.atom { head 404 }
-      format.xml { head 404 }
-      format.js { head 404 }
-      format.json { head 404 }
-    end
+  def render_404(options={})
+    render_error({:message => :notice_file_not_found, :status => 404}.merge(options))
     return false
   end
   
-  def render_error(msg)
+  # Renders an error response
+  def render_error(arg)
+    arg = {:message => arg} unless arg.is_a?(Hash)
+    
+    @message = arg[:message]
+    @message = l(@message) if @message.is_a?(Symbol)
+    @status = arg[:status] || 500
+    
     respond_to do |format|
-      format.html { 
-        flash.now[:error] = msg
-        render :text => '', :layout => use_layout, :status => 500
+      format.html {
+        render :template => 'common/error', :layout => use_layout, :status => @status
       }
-      format.atom { head 500 }
-      format.xml { head 500 }
-      format.js { head 500 }
-      format.json { head 500 }
+      format.atom { head @status }
+      format.xml { head @status }
+      format.js { head @status }
+      format.json { head @status }
     end
   end
 

diff --git a/app/views/common/403.rhtml b/app/views/common/403.rhtml
deleted file mode 100644 (file)
index 43f487d..0000000
--- a/app/views/common/403.rhtml
+++ /dev/null
@@ -1,6 +0,0 @@
-<h2>403</h2>
-
-<p><%=h @message %></p>
-<p><a href="javascript:history.back()">Back</a></p>
-
-<% html_title '403' %>

diff --git a/app/views/common/404.rhtml b/app/views/common/404.rhtml
deleted file mode 100644 (file)
index 753e716..0000000
--- a/app/views/common/404.rhtml
+++ /dev/null
@@ -1,6 +0,0 @@
-<h2>404</h2>
-
-<p><%= l(:notice_file_not_found) %></p>
-<p><a href="javascript:history.back()">Back</a></p>
-
-<% html_title '404' %>

diff --git a/app/views/common/error.html.erb b/app/views/common/error.html.erb
new file mode 100644 (file)
index 0000000..35d9086
--- /dev/null
+++ b/app/views/common/error.html.erb
@@ -0,0 +1,6 @@
+<h2><%=h @status %></h2>\r
+\r
+<p id="errorExplanation"><%=h @message %></p>\r
+<p><a href="javascript:history.back()">Back</a></p>\r
+\r
+<% html_title @status %>\r

diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 59b16c3..48ab8ce 100644 (file)
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -340,9 +340,7 @@ class IssuesControllerTest < ActionController::TestCase
     
     get :new, :project_id => 1
     assert_response 500
-    assert_not_nil flash[:error]
-    assert_tag :tag => 'div', :attributes => { :class => /error/ },
-                              :content => /No default issue/
+    assert_error_tag :content => /No default issue/
   end
   
   def test_get_new_with_no_tracker_should_display_an_error
@@ -351,9 +349,7 @@ class IssuesControllerTest < ActionController::TestCase
     
     get :new, :project_id => 1
     assert_response 500
-    assert_not_nil flash[:error]
-    assert_tag :tag => 'div', :attributes => { :class => /error/ },
-                              :content => /No tracker/
+    assert_error_tag :content => /No tracker/
   end
   
   def test_update_new_form

diff --git a/test/test_helper.rb b/test/test_helper.rb
index 9a27610..eea1a03 100644 (file)
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -113,11 +113,15 @@ class ActiveSupport::TestCase
   def self.repository_configured?(vendor)
     File.directory?(repository_path(vendor))
   end
+  
+  def assert_error_tag(options={})
+    assert_tag({:tag => 'p', :attributes => { :id => 'errorExplanation' }}.merge(options))
+  end
 
   # Shoulda macros
   def self.should_render_404
     should_respond_with :not_found
-    should_render_template 'common/404'
+    should_render_template 'common/error'
   end
 
   def self.should_have_before_filter(expected_method, options = {})