SONAR-1974 improve UI+error handling

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
index 3f59f2c24ac472d2bd26a3ce8df29f0f3d42a8f3..d86a01317897d6a1801193c00b74ec402f2b48a3 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
@@ -137,7 +137,13 @@ class ApplicationController < ActionController::Base
     # Ruby on Rails has a single logger "rails", so it's not possible to distinguish profiling logs
     # from error logs. For this reason a standard SLF4J logger is used instead of logger.error().
     java_facade.logError("Fail to render: #{request.url}\n#{Api::Utils.exception_message(error)}")
-    render :file => "#{Rails.public_path}/500.html", :status => 500
+
+    if request.xhr?
+      message = error.respond_to?('message') ? error.message : error.to_s
+      render :text => message, :status => 500
+    else
+      render :file => "#{Rails.public_path}/500.html", :status => 500
+    end
   end
 
 end

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
index 9ff6c07732ed6919c888e2ee65772afb8f55f2b9..4f53d3f3a8ca0eb2cc4d81ddfcddca33795e31dc 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
@@ -24,7 +24,8 @@ class ResourceController < ApplicationController
   include REXML
 
   SECTION=Navigation::SECTION_RESOURCE
-  helper :dashboard, SourceHelper
+  helper :dashboard
+  helper SourceHelper
 
   verify :method => :post, :only => [:create_violation]
 
@@ -39,7 +40,7 @@ class ResourceController < ApplicationController
     load_extensions()
 
     if @extension
-      if (@extension.getId()=='violations')
+      if @extension.getId()=='violations'
         render_violations()
       elsif (@extension.getId()=='coverage')
         render_coverage()
@@ -67,28 +68,38 @@ class ResourceController < ApplicationController
   end
 
   # Ajax request to display a form to create a review anywhere in source code
-  #
-  #== Parameters
-  #
-  # * 'resource'
-  # * 'line'
   def show_create_violation_form
     @line = params[:line].to_i
     @colspan = params[:colspan].to_i
+    @from = params[:from]
     render :partial => 'resource/create_violation_form'
   end
 
   def create_violation
     resource = Project.by_key(params[:resource])
     access_denied unless resource && current_user
-    rule = Review.find_or_create_rule(params[:category])
-    violation = RuleFailure.create_manual!(resource, rule, params)
-    violation.create_review!(
+
+    bad_request('Empty rule') if params[:category].blank?
+    bad_request('Empty message') if params[:message].blank?
+    bad_request('Missing severity') if params[:severity].blank?
+
+    Review.transaction do
+      rule = Review.find_or_create_rule(params[:category])
+      violation = RuleFailure.create_manual!(resource, rule, params)
+      violation.create_review!(
         :assignee => current_user,
         :user => current_user,
         :status => Review::STATUS_OPEN,
         :manual_violation => true)
-    redirect_to :action => 'index', :id => resource.id
+    end
+
+    if params[:from]=='drilldown'
+      render :js => "d(#{resource.id})"
+    else
+      render :update do |page|
+        page.redirect_to :controller => 'resource', :action => 'index', :id => resource.key, :tab => 'violations'
+      end
+    end
   end
 
   private
@@ -108,10 +119,10 @@ class ResourceController < ApplicationController
       end
     end
 
-    if !params[:tab].blank?
+    if params[:tab].present?
       @extension=@extensions.find { |extension| extension.getId()==params[:tab] }
 
-    elsif !params[:metric].blank?
+      elsif !params[:metric].blank?
       metric=Metric.by_key(params[:metric])
       @extension=@extensions.find { |extension| extension.getDefaultTabForMetrics().include?(metric.key) }
     end

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/helpers/users_helper.rb b/sonar-server/src/main/webapp/WEB-INF/app/helpers/users_helper.rb
index f5c2843f20438f5ec8269008691e0f02624cefe4..eb93c36390ff001a55952f590f552405e444b8b3 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/helpers/users_helper.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/helpers/users_helper.rb
@@ -129,7 +129,7 @@ module UsersHelper
     param_id_value = param_value
     
     unless param_id_value.blank?
-      user = User.find(:all, :conditions => [ "login = ?", param_id_value ]).first
+      user = User.find(:first, :conditions => [ "login = ?", param_id_value ])
       param_displayed_value = user.name if user
       param_displayed_value += " (#{message('me').downcase})" if user && current_user && current_user.login == param_id_value
     end

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/api/utils.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/api/utils.rb
index 87ca9d867158b45350c5dd6c091ecfb7a11802d8..16ff81939ba79748f90b44675f0dfa90af89bc16 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/api/utils.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/api/utils.rb
@@ -68,7 +68,7 @@ class Api::Utils
   end
 
   def self.exception_message(exception)
-    result = (exception.respond_to?(:message) ? "#{exception.message}\n" : "#{message}\n")
+    result = (exception.respond_to?(:message) ? "#{exception.message}\n" : "#{exception}\n")
     if exception.respond_to? :backtrace
       result << "\t" + exception.backtrace.join("\n\t") + "\n"
     end

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/drilldown/measures.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/drilldown/measures.html.erb
index a27ebcc021ec5ec90eab95e25b262b12e026acbe..f127d6e7801796a5c134672e7733b70910f04220 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/drilldown/measures.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/drilldown/measures.html.erb
@@ -1,4 +1,4 @@
-<%= render :partial => 'resource/javascript' -%>
+<%= render :partial => 'resource/javascript', :locals => {:from => 'drilldown'} -%>
 <%= render :partial => 'header' -%>
 
 <% if params[:period] && @snapshot.project_snapshot.periods? %>

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/drilldown/violations.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/drilldown/violations.html.erb
index f5aa3eae43360616fccb811ae8a397955955aff2..cfb793ce4a055fc50f0c9dfb809f9eda1b6b664d 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/drilldown/violations.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/drilldown/violations.html.erb
@@ -1,7 +1,6 @@
-<%= render :partial => 'resource/javascript' -%>
+<%= render :partial => 'resource/javascript', :locals => {:from => 'drilldown'} -%>
 <%= render :partial => 'header' -%>
 
-
 <div id="snapshot_title" class="page_title">
 <h4>
 <%

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/layouts/_head.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/layouts/_head.html.erb
index 9aabe2e583a832d715532283c5d4341028207ec8..637ec62a5614cbf8980391fe026e73b1fe6202cd 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/layouts/_head.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/layouts/_head.html.erb
@@ -31,12 +31,8 @@
 <%= javascript_include_tag 'protovis-sonar' %>
 <%= javascript_include_tag 'duplications' %>
 <% end %>
-<!--[if lte IE 6]>
-<link href="<%= ApplicationController.root_context -%>/ie6/index" media="all" rel="stylesheet" type="text/css" />
-<![endif]-->
-<!--[if lt IE 9]>
-<%= javascript_include_tag 'protovis-msie-shim' -%>
-<![endif]-->
+<!--[if lte IE 6]><link href="<%= ApplicationController.root_context -%>/ie6/index" media="all" rel="stylesheet" type="text/css" /><![endif]-->
+<!--[if lte IE 8]><%= javascript_include_tag 'protovis-msie' -%><![endif]-->
 <link rel="shortcut icon" type="image/x-icon" href="<%= image_path('favicon.ico') -%>" />
 <%
 if @project %>

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_create_violation_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_create_violation_form.html.erb
index b5400413fa9ca56a6663e546bccaac33814351b4..7e048bba0098a910be20c9a1138c42c802f5754a 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_create_violation_form.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_create_violation_form.html.erb
@@ -3,29 +3,40 @@
     <td class="nothing"></td>
   <% end %>
   <td class="violations">
-    <form action="<%= ApplicationController.root_context -%>/resource/create_violation" method="POST">
+    <% form_remote_tag :url => "#{ApplicationController.root_context}/resource/create_violation", :failure => "$('errorViolationForm#{@line}').update(request.responseText);$('errorViolationForm#{@line}').show()" do -%>
       <input type="hidden" name="resource" value="<%= params[:resource] -%>">
       <input type="hidden" name="line" value="<%= @line -%>">
+      <input type="hidden" name="from" value="<%= @from -%>">
 
       <div class="violation">
         <div class="vtitle">
-          <select name="severity">
+          <select name="severity" class="withIcons">
             <% Severity::SEVERITIES.each do |severity| %>
-              <option value="<%= severity -%>" <%= 'selected' if severity==Severity::MAJOR -%>><%= message("severity.#{severity}") -%></option>
+              <option class="sev_<%= severity -%>" value="<%= severity -%>" <%= 'selected' if severity==Severity::MAJOR -%>><%= message("severity.#{severity}") -%></option>
             <% end %>
           </select>
           &nbsp;
-          <img src="/dev/images/sep12.png">
+          <img src="<%= ApplicationController.root_context -%>/images/sep12.png">
           &nbsp;
-          Rule: <input type="text" name="category" size="50" maxlength="50">
+          Rule: <input type="text" name="category" size="80">
         </div>
 
         <div class="discussionComment first">
-          <textarea name="message" class="width100"></textarea>
-          <br/>
+          <table class="width100">
+            <tr>
+              <td style="vertical-align:top">
+                <textarea rows="5" name="message" style="width: 100%"></textarea>
+              </td>
+              <td class="sep"></td>
+              <td style="vertical-align:top;width: 90px">
+                <%= render :partial => 'markdown/help' -%>
+              </td>
+            </tr>
+          </table>
+          <div class="error" id="errorViolationForm<%= @line-%>" style="display: none"></div>
           <input type="submit" value="Create Violation"> <a href="#" onclick="return hVF(<%= @line -%>)">Cancel</a>
         </div>
       </div>
-    </form>
+    <% end %>
   </td>
 </tr>
\ No newline at end of file

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_javascript.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_javascript.html.erb
index 54708ff6c474e324b40c9f9da55843ed9ef64da1..b4ce783b1b15caa53af1cc745c16340275491e97 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_javascript.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_javascript.html.erb
@@ -7,7 +7,7 @@
           'pos' + line,
           '<%= ApplicationController.root_context -%>/resource/show_create_violation_form',
           {
-            parameters: {resource: resource, line: line, colspan: colspan},
+            parameters: {resource: resource, line: line, colspan: colspan, from: '<%= from -%>'},
             asynchronous:true,
             insertion: 'after'
           });

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_tabs.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_tabs.html.erb
index 9c629eaad77b2b368a2c2eeab32dc32d4900588a..f2fb607c2ca1ebf38728dc372d956f49117dfba0 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_tabs.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_tabs.html.erb
@@ -23,12 +23,12 @@
   <ul class="tabs" >
   <% if request.xhr? %>
     <% @extensions.each do |extension| %>
-      <li><a href="#" onclick="loadAjaxTab('<%= @resource.id -%>','<%= extension.getId() -%>',<%= display_title -%>)" class="<%= 'selected' if @extension && @extension.getId()==extension.getId() -%>"><%= message(extension.getId() + '.page', :default => extension.getTitle()) %></a></li>
+      <li><a href="#" onclick="return loadAjaxTab('<%= @resource.id -%>','<%= extension.getId() -%>',<%= display_title -%>)" class="<%= 'selected' if @extension && @extension.getId()==extension.getId() -%>"><%= message(extension.getId() + '.page', :default => extension.getTitle()) %></a></li>
     <% end %>
   <% else %>
     <script>function loadTab(url) {$('resource-loading').show();document.location.href=url;return false;}</script>
     <% @extensions.each do |extension| %>
-      <li><a href="#" onClick="loadTab('<%= url_for(:overwrite_params => {:tab => extension.getId(), :metric => nil}) -%>')" class="<%= 'selected' if @extension && @extension.getId()==extension.getId() -%>"><%= message(extension.getId() + '.page', :default => extension.getTitle()) %></a></li>
+      <li><a href="#" onClick="return loadTab('<%= url_for(:overwrite_params => {:tab => extension.getId(), :metric => nil}) -%>')" class="<%= 'selected' if @extension && @extension.getId()==extension.getId() -%>"><%= message(extension.getId() + '.page', :default => extension.getTitle()) %></a></li>
     <% end %>
   <% end %>
   <li>

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/index.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/index.html.erb
index b4c45d7c70cddeef973adc717e89f1a64ecba427..c42ce265a9c9f844d3193d3b1d7effc452b1a175 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/index.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/index.html.erb
@@ -1,5 +1,5 @@
 <%= render :partial => 'tabs' -%>
-<%= render :partial => 'resource/javascript' -%>
+<%= render :partial => 'resource/javascript', :locals => {:from => 'resource'} -%>
 <%= render :partial => "resource/header_#{@extension.getId()}" -%>
 
 <% if @display_violations && @global_violations && @global_violations.size>0 -%>