From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sat, 23 Oct 2010 11:07:04 +0000 (+0000)
Subject: Refactor: merged error rendering methods.
X-Git-Tag: 1.1.0~249
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=7824eca77561dea52960b3d094389bd4e85eeccf;p=redmine.git

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 360d09189..d7b1add85 100644
--- 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
index 43f487d10..000000000
--- 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
index 753e716c6..000000000
--- 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
index 000000000..35d908645
--- /dev/null
+++ b/app/views/common/error.html.erb
@@ -0,0 +1,6 @@
+<h2><%=h @status %></h2>
+
+<p id="errorExplanation"><%=h @message %></p>
+<p><a href="javascript:history.back()">Back</a></p>
+
+<% html_title @status %>
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 59b16c32c..48ab8cef6 100644
--- 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 9a2761021..eea1a03b5 100644
--- 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 = {})