From ccf5929571dd5bb226d0c0814e1041f5ebd836ea Mon Sep 17 00:00:00 2001 From: simonbrandhof Date: Wed, 7 Dec 2011 16:41:58 +0100 Subject: [PATCH] SONAR-1974 add all the available rules in the form to create violations --- .../resources/org/sonar/l10n/core.properties | 13 +++++ .../app/controllers/api/reviews_controller.rb | 8 +-- .../app/controllers/application_controller.rb | 2 +- .../app/controllers/resource_controller.rb | 12 +++-- .../main/webapp/WEB-INF/app/models/review.rb | 52 ++++++++++--------- .../resource/_create_violation_form.html.erb | 23 ++++++-- .../app/views/resource/_javascript.html.erb | 1 + 7 files changed, 74 insertions(+), 37 deletions(-) diff --git a/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties b/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties index 50de65d3241..b40853ec03d 100644 --- a/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties +++ b/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties @@ -642,6 +642,19 @@ coverage_viewer.on_new_code=On new code coverage_viewer.unit_tests=Unit Tests coverage_viewer.integration_tests=Integration Tests + +#------------------------------------------------------------------------------ +# +# GENERIC CODE VIEWER +# +#------------------------------------------------------------------------------ +code_viewer.create_violation.new_rule=New Rule +code_viewer.create_violation.rules=Rules +code_viewer.create_violation.submit=Create Violation +code_viewer.create_violation.missing_rule=Missing rule +code_viewer.create_violation.missing_message=Missing message +code_viewer.create_violation.missing_severity=Missing severity + #------------------------------------------------------------------------------ # # MANUAL MEASURES diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb index 56e647c117d..1c8a80f3bf5 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb @@ -63,7 +63,7 @@ class Api::ReviewsController < Api::ApiController # * 'violation_id' : the violation on which the review should be created # # To create a violation : - # * 'category' : the name of the rule in the repository "review". If it does not exist then the rule is created. + # * 'rule_name' : the name of the rule in the repository "review". If it does not exist then the rule is created. # * 'resource' : id or key of the resource to review # * 'line' : optional line. It starts from 1. If 0 then no specific line. Default value is 0. # * 'severity' : BLOCKER, CRITICAL, MAJOR, MINOR or INFO. Default value is MAJOR. @@ -77,7 +77,7 @@ class Api::ReviewsController < Api::ApiController # # ==== Examples # - # * Create a manual violation : POST /api/reviews?resource=MyFile&line=18&status=OPEN&category=Performance%20Issue + # * Create a manual violation : POST /api/reviews?resource=MyFile&line=18&status=OPEN&rule_name=Performance%20Issue # * Review an existing violation : POST /api/reviews?violation_id=1&status=OPEN&assignee=admin&comment=Please%20fix%20this # * Flag an existing violation as false-positive : POST /api/reviews/?violation_id=2&status=RESOLVED&resolution=FALSE-POSITIVE&comment=No%20violation%20here # * Resolve an existing violation : POST /api/reviews/?violation_id=3&status=RESOLVED&resolution=FIXED&assignee=admin&comment=This%20violation%20was%20fixed%20by%20me @@ -104,13 +104,13 @@ class Api::ReviewsController < Api::ApiController else # Manually create a violation and review it - bad_request("Missing parameter 'category'") if params[:category].blank? + bad_request("Missing parameter 'rule_name'") if params[:rule_name].blank? bad_request("Missing parameter 'resource'") if params[:resource].blank? resource = Project.by_key(params[:resource]) access_denied unless resource && has_rights_to_modify?(resource) bad_request("Resource does not exist") unless resource.last_snapshot - rule = Review.find_or_create_rule(params[:category]) + rule = Review.find_or_create_rule(params[:rule_name]) violation = RuleFailure.create_manual!(resource, rule, params) violation.create_review!(:assignee => assignee, :user => current_user, :manual_violation => true) end 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 d86a0131789..715375c809c 100644 --- 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 @@ -29,7 +29,7 @@ class ApplicationController < ActionController::Base rescue_from ActionController::UnknownAction, :with => :render_not_found rescue_from ActionController::RoutingError, :with => :render_not_found rescue_from ActionController::UnknownController, :with => :render_not_found - rescue_from ActiveRecord::RecordInvalid, :with => :render_error + rescue_from ActiveRecord::RecordInvalid, :with => :render_bad_request rescue_from ActiveRecord::RecordNotFound, :with => :render_not_found rescue_from Errors::NotFound, :with => :render_not_found rescue_from Errors::AccessDenied, :with => :render_access_denied # See lib/authenticated_system.rb#access_denied() 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 4f53d3f3a8c..35eaf9bb45f 100644 --- 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 @@ -72,6 +72,8 @@ class ResourceController < ApplicationController @line = params[:line].to_i @colspan = params[:colspan].to_i @from = params[:from] + @rules = Rule.find(:all, :conditions => ['enabled=? and plugin_name=?', true, Review::RULE_REPOSITORY_KEY], :order => 'name') + @html_id="#{params[:resource]}_#{@line}" render :partial => 'resource/create_violation_form' end @@ -79,12 +81,14 @@ class ResourceController < ApplicationController resource = Project.by_key(params[:resource]) access_denied unless resource && current_user - bad_request('Empty rule') if params[:category].blank? - bad_request('Empty message') if params[:message].blank? - bad_request('Missing severity') if params[:severity].blank? + rule_id_or_name = params[:rule] + rule_id_or_name = params[:new_rule] if rule_id_or_name.blank? + bad_request(message('code_viewer.create_violation.missing_rule')) if rule_id_or_name.blank? + bad_request(message('code_viewer.create_violation.missing_message')) if params[:message].blank? + bad_request(message('code_viewer.create_violation.missing_severity')) if params[:severity].blank? Review.transaction do - rule = Review.find_or_create_rule(params[:category]) + rule = Review.find_or_create_rule(rule_id_or_name) violation = RuleFailure.create_manual!(resource, rule, params) violation.create_review!( :assignee => current_user, diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb index 9982537c859..c4c87bbc44d 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb @@ -47,18 +47,18 @@ class Review < ActiveRecord::Base def rule @rule ||= - begin - rule_failure ? rule_failure.rule : nil - end + begin + rule_failure ? rule_failure.rule : nil + end end def rule_failure @rule_failure ||= - begin - # We need to manually run this DB request as the real relation Reviews-RuleFailures is 1:n but we want only 1 violation - # (more than 1 violation can have the same "permanent_id" when several analyses are run in a small time frame) - RuleFailure.find(:first, :conditions => {:permanent_id => rule_failure_permanent_id}, :order => 'id desc') - end + begin + # We need to manually run this DB request as the real relation Reviews-RuleFailures is 1:n but we want only 1 violation + # (more than 1 violation can have the same "permanent_id" when several analyses are run in a small time frame) + RuleFailure.find(:first, :conditions => {:permanent_id => rule_failure_permanent_id}, :order => 'id desc') + end end @@ -115,13 +115,13 @@ class Review < ActiveRecord::Base def to_java_map(params = {}) map = java.util.HashMap.new({ - "project" => project.long_name.to_java, - "resource" => resource.long_name.to_java, - "title" => title.to_java, - "creator" => user == nil ? nil : user.login.to_java, - "assignee" => assignee == nil ? nil : assignee.login.to_java, - "status" => status.to_java, - "resolution" => resolution.to_java + "project" => project.long_name.to_java, + "resource" => resource.long_name.to_java, + "title" => title.to_java, + "creator" => user == nil ? nil : user.login.to_java, + "assignee" => assignee == nil ? nil : assignee.login.to_java, + "status" => status.to_java, + "resolution" => resolution.to_java }) params.each_pair do |k, v| map.put(k.to_java, v.to_java) @@ -400,10 +400,10 @@ class Review < ActiveRecord::Base comments = [] review_comments.each do |comment| comments << { - 'id' => comment.id.to_i, - 'author' => comment.user.login, - 'updatedAt' => Api::Utils.format_datetime(comment.updated_at), - 'text' => convert_markdown ? comment.html_text : comment.plain_text + 'id' => comment.id.to_i, + 'author' => comment.user.login, + 'updatedAt' => Api::Utils.format_datetime(comment.updated_at), + 'text' => convert_markdown ? comment.html_text : comment.plain_text } end json['comments'] = comments @@ -411,11 +411,15 @@ class Review < ActiveRecord::Base end - def self.find_or_create_rule(rule_category) - key = rule_category.strip.downcase.sub(/\s+/, '_') - rule = Rule.find(:first, :conditions => {:enabled => true, :plugin_name => RULE_REPOSITORY_KEY, :plugin_rule_key => key}) - unless rule - rule = Rule.create!(:enabled => true, :plugin_name => RULE_REPOSITORY_KEY, :plugin_rule_key => key, :name => rule_category) + def self.find_or_create_rule(rule_id_or_name) + if Api::Utils.is_integer?(rule_id_or_name) + rule = Rule.find(:first, :conditions => {:enabled => true, :plugin_name => RULE_REPOSITORY_KEY, :id => rule_id_or_name.to_i}) + else + key = rule_id_or_name.strip.downcase.sub(/\s+/, '_') + rule = Rule.find(:first, :conditions => {:enabled => true, :plugin_name => RULE_REPOSITORY_KEY, :plugin_rule_key => key}) + unless rule + rule = Rule.create!(:enabled => true, :plugin_name => RULE_REPOSITORY_KEY, :plugin_rule_key => key, :name => rule_id_or_name) + end end rule end 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 7e048bba009..b041f146ae9 100644 --- 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 @@ -18,7 +18,18 @@     - Rule: + + +
@@ -33,10 +44,14 @@ - - Cancel + + + <%= message('cancel') -%>
<% end %> - \ No newline at end of file + + \ 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 b4ce783b1b1..bc1d9cfca3e 100644 --- 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 @@ -9,6 +9,7 @@ { parameters: {resource: resource, line: line, colspan: colspan, from: '<%= from -%>'}, asynchronous:true, + evalScripts: true, insertion: 'after' }); } -- 2.39.5