From a865a6e9cc94b606dc75951c8cfd96656ba0a3a6 Mon Sep 17 00:00:00 2001
From: João M. M. da Silva + Diego Araújo <jaodsilv@linux.ime.usp.br>
Date: Tue, 21 Aug 2012 18:11:32 -0300
Subject: [PATCH] [Mezuro] Handling exceptions in models, controllers and views

---
 plugins/mezuro/controllers/mezuro_plugin_myprofile_controller.rb          | 29 ++++++++++++++++++++++++++---
 plugins/mezuro/controllers/mezuro_plugin_profile_controller.rb            | 61 +++++++++++++++++++++++++++++++++++++++++++------------------
 plugins/mezuro/lib/mezuro_plugin/configuration_content.rb                 |  7 ++++++-
 plugins/mezuro/lib/mezuro_plugin/project_content.rb                       |  8 ++++++--
 plugins/mezuro/test/functional/mezuro_plugin_myprofile_controller_test.rb |  2 +-
 plugins/mezuro/test/functional/mezuro_plugin_profile_controller_test.rb   |  7 -------
 plugins/mezuro/views/cms/mezuro_plugin/_configuration_content.html.erb    | 10 +++-------
 plugins/mezuro/views/cms/mezuro_plugin/_project_content.html.erb          | 12 +++++++-----
 plugins/mezuro/views/mezuro_plugin_profile/error_page.html.erb            |  1 -
 9 files changed, 92 insertions(+), 45 deletions(-)

diff --git a/plugins/mezuro/controllers/mezuro_plugin_myprofile_controller.rb b/plugins/mezuro/controllers/mezuro_plugin_myprofile_controller.rb
index e6087ab..ac3042c 100644
--- a/plugins/mezuro/controllers/mezuro_plugin_myprofile_controller.rb
+++ b/plugins/mezuro/controllers/mezuro_plugin_myprofile_controller.rb
@@ -4,7 +4,7 @@ class MezuroPluginMyprofileController < ProfileController
 
   rescue_from Exception do |exception|
     message = URI.escape(CGI.escape(exception.message),'.')
-    redirect_to "/myprofile/#{profile.identifier}/plugin/mezuro/error_page?message=#{message}"
+    redirect_to_error_page message
   end
 
   def error_page
@@ -31,6 +31,7 @@ class MezuroPluginMyprofileController < ProfileController
   def new_compound_metric_configuration
     @configuration_content = profile.articles.find(params[:id])
     @metric_configurations = @configuration_content.metric_configurations
+    look_for_configuration_content_errors
   end
 
   def edit_metric_configuration
@@ -45,7 +46,7 @@ class MezuroPluginMyprofileController < ProfileController
     @metric_configurations = @configuration_content.metric_configurations
     @metric = @metric_configuration.metric
   end
-  
+
   def create_metric_configuration
     id = params[:id]
     metric_name = params[:metric_configuration][:metric][:name]
@@ -56,7 +57,9 @@ class MezuroPluginMyprofileController < ProfileController
   def create_compound_metric_configuration
     id = params[:id]
     metric_name = params[:metric_configuration][:metric][:name]
-    Kalibro::MetricConfiguration.new(params[:metric_configuration]).save
+    metric_configuration = Kalibro::MetricConfiguration.new(params[:metric_configuration])
+    metric_configuration.save
+    look_for_model_error metric_configuration
     redirect_to "/myprofile/#{profile.identifier}/plugin/mezuro/edit_compound_metric_configuration?id=#{id}&metric_name=#{metric_name.gsub(/\s/, '+')}"
   end
 
@@ -65,6 +68,7 @@ class MezuroPluginMyprofileController < ProfileController
     metric_name = params[:metric_configuration][:metric][:name]
     metric_configuration = Kalibro::MetricConfiguration.find_by_configuration_name_and_metric_name(@configuration_content.name, metric_name)
     metric_configuration.update_attributes params[:metric_configuration]
+    look_for_model_error metric_configuration
     redirect_to "/#{profile.identifier}/#{@configuration_content.slug}"
   end
 
@@ -73,6 +77,7 @@ class MezuroPluginMyprofileController < ProfileController
     metric_name = params[:metric_configuration][:metric][:name]
     metric_configuration = Kalibro::MetricConfiguration.find_by_configuration_name_and_metric_name(@configuration_content.name, metric_name)
     metric_configuration.update_attributes params[:metric_configuration]
+    look_for_model_error metric_configuration
     redirect_to "/#{profile.identifier}/#{@configuration_content.slug}"
   end
 
@@ -81,6 +86,7 @@ class MezuroPluginMyprofileController < ProfileController
     metric_name = params[:metric_name]
     metric_configuration = Kalibro::MetricConfiguration.find_by_configuration_name_and_metric_name(configuration_content.name, metric_name)
     metric_configuration.destroy
+    look_for_model_error metric_configuration
     redirect_to "/#{profile.identifier}/#{configuration_content.slug}"
   end
   
@@ -105,6 +111,7 @@ class MezuroPluginMyprofileController < ProfileController
     metric_configuration = Kalibro::MetricConfiguration.find_by_configuration_name_and_metric_name(@configuration_content.name, metric_name)   
     metric_configuration.add_range(@range)
     metric_configuration.save
+    look_for_model_error metric_configuration
   end
   
   def update_range
@@ -115,6 +122,7 @@ class MezuroPluginMyprofileController < ProfileController
     index = metric_configuration.ranges.index{ |range| range.beginning == beginning_id.to_f || beginning_id == "-INF" }
     metric_configuration.ranges[index] = Kalibro::Range.new params[:range]
     metric_configuration.save
+    look_for_model_error metric_configuration
   end
   
   def remove_range
@@ -124,6 +132,7 @@ class MezuroPluginMyprofileController < ProfileController
     metric_configuration = Kalibro::MetricConfiguration.find_by_configuration_name_and_metric_name(configuration_content.name, metric_name)
     metric_configuration.ranges.delete_if { |range| range.beginning == beginning_id.to_f || beginning_id == "-INF" }
     metric_configuration.save
+
     formatted_metric_name = metric_name.gsub(/\s/, '+')
     if metric_configuration.metric.class == Kalibro::CompoundMetric
       redirect_to "/myprofile/#{profile.identifier}/plugin/mezuro/edit_compound_metric_configuration?id=#{configuration_content.id}&metric_name=#{formatted_metric_name}"
@@ -132,4 +141,18 @@ class MezuroPluginMyprofileController < ProfileController
     end
   end
 
+  private
+
+  def redirect_to_error_page(message)
+    redirect_to "/myprofile/#{profile.identifier}/plugin/mezuro/error_page?message=#{message}"
+  end
+
+  def look_for_configuration_content_errors
+    redirect_to_error_page(@configuration_content.errors[:base]) if not @configuration_content.errors.nil?
+  end
+
+  def look_for_model_error(model)
+    redirect_to_error_page(model.errors[0].message) if not model.errors.empty?
+  end
+
 end
diff --git a/plugins/mezuro/controllers/mezuro_plugin_profile_controller.rb b/plugins/mezuro/controllers/mezuro_plugin_profile_controller.rb
index b23accf..f820554 100644
--- a/plugins/mezuro/controllers/mezuro_plugin_profile_controller.rb
+++ b/plugins/mezuro/controllers/mezuro_plugin_profile_controller.rb
@@ -2,11 +2,6 @@ class MezuroPluginProfileController < ProfileController
 
   append_view_path File.join(File.dirname(__FILE__) + '/../views')
 
-  rescue_from Exception do |exception|
-    message = URI.escape(CGI.escape(exception.message),'.')
-    redirect_to_error_page message
-  end
-
   def error_page
     @message = params[:message]
   end
@@ -14,52 +9,79 @@ class MezuroPluginProfileController < ProfileController
   def project_state
     @content = profile.articles.find(params[:id])
     project = @content.project
-    redirect_to_error_page(project.errors[0].message) if not project.errors.empty?
-    state = project.kalibro_error.nil? ? project.state : "ERROR"
-    render :text => state
+    if project_content_errors?
+      redirect_to_error_page(@content.errors[:base])
+    else
+      state = project.kalibro_error.nil? ? project.state : "ERROR"
+      render :text => state
+    end
   end
 
   def project_error
     @content = profile.articles.find(params[:id])
     @project = @content.project
-    render :partial => 'content_viewer/project_error'
+    if project_content_errors?
+      redirect_to_error_page(@content.errors[:base])
+    else
+      render :partial => 'content_viewer/project_error'
+    end
   end
 
   def project_result
     @content = profile.articles.find(params[:id])
     date = params[:date]
     @project_result = date.nil? ? @content.project_result : @content.project_result_with_date(date)
-    render :partial => 'content_viewer/project_result'
+    if project_content_errors?
+      redirect_to_error_page(@content.errors[:base])
+    else
+      render :partial => 'content_viewer/project_result'
+    end
   end 	
 
   def module_result
     @content = profile.articles.find(params[:id])
     @module_result = @content.module_result(params)
-    render :partial => 'content_viewer/module_result'
+    if project_content_errors?
+      redirect_to_error_page(@content.errors[:base])
+    else
+      render :partial => 'content_viewer/module_result'
+    end
   end
 
   def project_tree
     @content = profile.articles.find(params[:id])
     date = params[:date]
     project_result = date.nil? ? @content.project_result : @content.project_result_with_date(date)
-    @project_name = @content.project.name
-    @source_tree = project_result.node_of(params[:module_name])
-    render :partial =>'content_viewer/source_tree'
+    @project_name = @content.project.name if not @content.project.nil?
+    if project_content_errors?
+      redirect_to_error_page(@content.errors[:base])
+    else
+      @source_tree = project_result.node_of(params[:module_name])
+      render :partial =>'content_viewer/source_tree'
+    end
   end
 
   def module_metrics_history
     metric_name = params[:metric_name]
     @content = profile.articles.find(params[:id])
     module_history = @content.result_history(params[:module_name])
-    @score_history = filtering_metric_history(metric_name, module_history)
-    render :partial => 'content_viewer/score_history'
+    if project_content_errors?
+      redirect_to_error_page(@content.errors[:base])
+    else
+      @score_history = filtering_metric_history(metric_name, module_history)
+      render :partial => 'content_viewer/score_history'
+    end
   end
 
   def module_grade_history
     @content = profile.articles.find(params[:id])
     modules_results = @content.result_history(params[:module_name])
-    @score_history = modules_results.collect { |module_result| module_result.grade }
-    render :partial => 'content_viewer/score_history'
+    if project_content_errors?
+      redirect_to_error_page(@content.errors[:base])
+    else
+      @score_history = modules_results.collect { |module_result| module_result.grade }
+      render :partial => 'content_viewer/score_history'
+    end
   end
   
   private
@@ -82,5 +104,8 @@ class MezuroPluginProfileController < ProfileController
     redirect_to "/profile/#{profile.identifier}/plugin/mezuro/error_page?message=#{message}"
   end
 
+  def project_content_errors?
+    not @content.errors.nil?
+  end
 end
 
diff --git a/plugins/mezuro/lib/mezuro_plugin/configuration_content.rb b/plugins/mezuro/lib/mezuro_plugin/configuration_content.rb
index a10d490..9f29d52 100644
--- a/plugins/mezuro/lib/mezuro_plugin/configuration_content.rb
+++ b/plugins/mezuro/lib/mezuro_plugin/configuration_content.rb
@@ -35,7 +35,12 @@ class MezuroPlugin::ConfigurationContent < Article
   end
 
   def kalibro_configuration_names
-    ["None"] + Kalibro::Configuration.all_names.sort
+    begin
+      ["None"] + Kalibro::Configuration.all_names.sort
+    rescue Exception => exception
+      errors.add_to_base(exception.message)
+      ["None"]
+    end
   end
 
   private
diff --git a/plugins/mezuro/lib/mezuro_plugin/project_content.rb b/plugins/mezuro/lib/mezuro_plugin/project_content.rb
index e944728..3d293d6 100644
--- a/plugins/mezuro/lib/mezuro_plugin/project_content.rb
+++ b/plugins/mezuro/lib/mezuro_plugin/project_content.rb
@@ -25,8 +25,8 @@ class MezuroPlugin::ProjectContent < Article
       @project ||= Kalibro::Project.find_by_name(name)
     rescue Exception => error
       errors.add_to_base(error.message)
-      @project
     end
+    @project
   end
 
   def project_result
@@ -35,6 +35,7 @@ class MezuroPlugin::ProjectContent < Article
     rescue Exception => error
       errors.add_to_base(error.message)
     end
+    @project_result
   end
   
   def project_result_with_date(date)
@@ -44,6 +45,7 @@ Kalibro::ProjectResult.first_result_after(name, date)
     rescue Exception => error
       errors.add_to_base(error.message)
     end
+    @project_result
   end
 
   def module_result(attributes)
@@ -52,8 +54,9 @@ Kalibro::ProjectResult.first_result_after(name, date)
     begin
       @module_result ||= Kalibro::ModuleResult.find_by_project_name_and_module_name_and_date(name, module_name, date)
     rescue Exception => error
-      raise error
+      errors.add_to_base(error.message)
     end
+    @module_result
   end
 
   def result_history(module_name)
@@ -74,6 +77,7 @@ Kalibro::ProjectResult.first_result_after(name, date)
       existing = Kalibro::Project.all_names
     rescue Exception => error
       errors.add_to_base(error.message)
+      existing = []
     end
     
     if existing.any?{|existing_name| existing_name.casecmp(name)==0} # existing.include?(name) + case insensitive
diff --git a/plugins/mezuro/test/functional/mezuro_plugin_myprofile_controller_test.rb b/plugins/mezuro/test/functional/mezuro_plugin_myprofile_controller_test.rb
index 0837e3c..f4f38eb 100644
--- a/plugins/mezuro/test/functional/mezuro_plugin_myprofile_controller_test.rb
+++ b/plugins/mezuro/test/functional/mezuro_plugin_myprofile_controller_test.rb
@@ -39,7 +39,7 @@ class MezuroPluginMyprofileControllerTest < ActionController::TestCase
     @range = RangeFixtures.range_excellent
     @range_hash = RangeFixtures.range_excellent_hash
   end
-  
+
   should 'test choose base tool' do
     Kalibro::BaseTool.expects(:request).with("BaseTool", :get_base_tool_names).returns({:base_tool_name => @base_tool.name})
     get :choose_base_tool, :profile => @profile.identifier, :id => @content.id
diff --git a/plugins/mezuro/test/functional/mezuro_plugin_profile_controller_test.rb b/plugins/mezuro/test/functional/mezuro_plugin_profile_controller_test.rb
index 7a78df4..a171fb1 100644
--- a/plugins/mezuro/test/functional/mezuro_plugin_profile_controller_test.rb
+++ b/plugins/mezuro/test/functional/mezuro_plugin_profile_controller_test.rb
@@ -25,13 +25,6 @@ class MezuroPluginProfileControllerTest < ActionController::TestCase
     @content.save
   end
 
-  should 'show an error page if an exception is raised' #do
-#    Kalibro::Project.expects(:request).with("Project", :get_project, :project_name => @project.name).raises(Exception, "Error message")
-#    get :project_state, :profile => @profile.identifier, :id => @content.id
-#    assert_response 302
-#    assert_select('h2', 'An error occured: ')
-#  end
-
   should 'test project state without kalibro_error' do
     Kalibro::Project.expects(:request).with("Project", :get_project, :project_name => @project.name).returns({:project => @project.to_hash})
     get :project_state, :profile => @profile.identifier, :id => @content.id
diff --git a/plugins/mezuro/views/cms/mezuro_plugin/_configuration_content.html.erb b/plugins/mezuro/views/cms/mezuro_plugin/_configuration_content.html.erb
index 319830f..4635a56 100644
--- a/plugins/mezuro/views/cms/mezuro_plugin/_configuration_content.html.erb
+++ b/plugins/mezuro/views/cms/mezuro_plugin/_configuration_content.html.erb
@@ -1,11 +1,8 @@
 <h1> <%= _(MezuroPlugin::ConfigurationContent.short_description) %> </h1>
 
 <%
-  begin
-    kalibro_configuration = @article.title.nil? ? nil : @article.kalibro_configuration
-  rescue
-    kalibro_configuration = nil
-  end
+  kalibro_configuration = @article.title.nil? ? nil : @article.kalibro_configuration
+  kalibro_configuration_names = @article.kalibro_configuration_names
 %>
 
 <%= error_messages_for 'kalibro_configuration' %>
@@ -13,8 +10,7 @@
 <%= hidden_field_tag 'kalibro_configuration[profile_id]', profile.id %>
 <%= hidden_field_tag 'id', @article.id %>
 
-<% kalibro_configuration_names = @article.kalibro_configuration_names %>
-
+ 
 <% selected = (kalibro_configuration.nil? ? "None" : @article.configuration_to_clone_name) %>
 
 <%= required_fields_message %>
diff --git a/plugins/mezuro/views/cms/mezuro_plugin/_project_content.html.erb b/plugins/mezuro/views/cms/mezuro_plugin/_project_content.html.erb
index 9a7ecda..656cb7f 100644
--- a/plugins/mezuro/views/cms/mezuro_plugin/_project_content.html.erb
+++ b/plugins/mezuro/views/cms/mezuro_plugin/_project_content.html.erb
@@ -1,10 +1,14 @@
 <h1> <%= _(MezuroPlugin::ProjectContent.short_description) %> </h1>
 
 <%
+  @project = @article.title.nil? ? nil : @article.project
   begin
-    @project = @article.title.nil? ? nil : Kalibro::Project.find_by_name(@article.title)
-  rescue
-    @project = nil
+    @repository_types = Kalibro::Repository.repository_types.sort
+    @configuration_names = Kalibro::Configuration.all_names.sort
+  rescue Exception => exception
+    @article.errors.add_to_base(exception.message)
+    @repository_types = []
+    @configuration_names = []
   end
 %>
 
@@ -24,14 +28,12 @@
 
 <%= f.text_field :description %><br/>
 
-<% @repository_types = Kalibro::Repository.repository_types.sort %>
 <% @selected = (@project.nil? ? @repository_types : @project.repository.type) %>
 <%= required labelled_form_field _('Repository type'),
   f.select(:repository_type, @repository_types, {:selected => @selected}) %><br/>
 
 <%= required f.text_field(:repository_url) %><br/>
 
-<% @configuration_names = Kalibro::Configuration.all_names.sort %>
 <% @selected = (@project.nil? ? @configuration_names[0] : @project.configuration_name) %>
 
 <% if !@project.nil? && !@article.id.nil? %>
diff --git a/plugins/mezuro/views/mezuro_plugin_profile/error_page.html.erb b/plugins/mezuro/views/mezuro_plugin_profile/error_page.html.erb
index 1d3c248..089af13 100644
--- a/plugins/mezuro/views/mezuro_plugin_profile/error_page.html.erb
+++ b/plugins/mezuro/views/mezuro_plugin_profile/error_page.html.erb
@@ -1,3 +1,2 @@
 <h2> An error occured: </h2>
 <%= @message %>
-<!-- The <%=h is for escaping the URL properly --!>
--
libgit2 0.21.2