From 3f1c6bb54f96dacd32f86321c7ba89c5dd843d57 Mon Sep 17 00:00:00 2001 From: simonbrandhof Date: Wed, 7 Dec 2011 18:57:05 +0100 Subject: [PATCH] SONAR-1974 add a page to manage manual rules Manual rules allow users to create their own violations directly from UI. These rules can also be defined 'on the fly' while creating violations. --- .../resources/org/sonar/l10n/core.properties | 1 + .../sonar/server/startup/RegisterRules.java | 4 +- .../app/controllers/api/reviews_controller.rb | 4 +- .../controllers/manual_rules_controller.rb | 71 +++++++++++++++++++ .../app/controllers/resource_controller.rb | 6 +- .../main/webapp/WEB-INF/app/models/review.rb | 16 ----- .../main/webapp/WEB-INF/app/models/rule.rb | 71 +++++++++++++------ .../webapp/WEB-INF/app/models/rule_failure.rb | 15 ---- .../app/views/layouts/_layout.html.erb | 1 + .../app/views/manual_rules/index.html.erb | 64 +++++++++++++++++ .../WEB-INF/app/views/metrics/index.html.erb | 10 ++- .../resource/_create_violation_form.html.erb | 12 +--- .../server/startup/RegisterRulesTest.java | 6 +- ...es.xml => shouldNotDisableManualRules.xml} | 2 +- 14 files changed, 204 insertions(+), 79 deletions(-) create mode 100644 sonar-server/src/main/webapp/WEB-INF/app/controllers/manual_rules_controller.rb create mode 100644 sonar-server/src/main/webapp/WEB-INF/app/views/manual_rules/index.html.erb rename sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/{shouldNotDisableReviewRules.xml => shouldNotDisableManualRules.xml} (84%) 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 b40853ec03d..bac0836f526 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 @@ -281,6 +281,7 @@ filters.page=Filters global_roles.page=Global Roles manual_metrics.page=Manual Metrics manual_measures.page=Manual Measures +manual_rules.page=Manual Rules my_profile.page=My Profile project_roles.page=Project Roles project_settings.page=Settings diff --git a/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java b/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java index 005775ea877..4cbfa583f93 100644 --- a/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java +++ b/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java @@ -84,8 +84,8 @@ public final class RegisterRules { } private void disableAllRules(DatabaseSession session) { - // the hardcoded repository "review" is used for manual violations - session.createQuery("UPDATE " + Rule.class.getSimpleName() + " SET enabled=false WHERE parent IS NULL AND pluginName<>'review'").executeUpdate(); + // the hardcoded repository "manual" is used for manual violations + session.createQuery("UPDATE " + Rule.class.getSimpleName() + " SET enabled=false WHERE parent IS NULL AND pluginName<>'manual'").executeUpdate(); } private void registerRepository(RuleRepository repository, DatabaseSession session) { 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 1c8a80f3bf5..59c69bdbd40 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 @@ -110,8 +110,8 @@ class Api::ReviewsController < Api::ApiController 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[:rule_name]) - violation = RuleFailure.create_manual!(resource, rule, params) + rule = Rule.find_or_create_manual_rule(params[:rule_name]) + violation = rule.create_violation!(resource, 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/manual_rules_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/manual_rules_controller.rb new file mode 100644 index 00000000000..9f489df1d70 --- /dev/null +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/manual_rules_controller.rb @@ -0,0 +1,71 @@ +# +# Sonar, entreprise quality control tool. +# Copyright (C) 2008-2011 SonarSource +# mailto:contact AT sonarsource DOT com +# +# Sonar is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 3 of the License, or (at your option) any later version. +# +# Sonar is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with Sonar; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 +# +class ManualRulesController < ApplicationController + + before_filter :admin_required + verify :method => :post, :only => [:create], :redirect_to => {:action => :index} + verify :method => :delete, :only => [:delete], :redirect_to => {:action => :index} + + SECTION=Navigation::SECTION_CONFIGURATION + + def index + @rules = Rule.manual_rules() + @rule=Rule.new + render :action => 'index' + end + + def edit + @rules = Rule.manual_rules() + @rule=Rule.manual_rule(params['id'].to_i) + bad_request('Missing rule id') unless @rule + render :action => 'index' + end + + def create + access_denied unless is_admin? + begin + if params[:id].to_i>0 + # Update rule + rule=Rule.manual_rule(params['id'].to_i) + bad_request('Unknown rule') unless rule + + else + # Create rule + rule=Rule.find_or_create_manual_rule(params[:name]) + end + rule.description=params[:description] + rule.save! + rescue Exception => e + flash[:error]= e.message + end + redirect_to :action => 'index' + end + + def delete + access_denied unless is_admin? + rule=Rule.manual_rule(params['id'].to_i) + bad_request('Missing rule id') unless rule + rule.enabled=false + unless rule.save + flash[:error]=rule.errors.to_s + end + redirect_to :action => 'index' + end +end \ No newline at end of file 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 35eaf9bb45f..b891e9ab304 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,7 +72,7 @@ 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') + @rules = Rule.manual_rules @html_id="#{params[:resource]}_#{@line}" render :partial => 'resource/create_violation_form' end @@ -88,8 +88,8 @@ class ResourceController < ApplicationController bad_request(message('code_viewer.create_violation.missing_severity')) if params[:severity].blank? Review.transaction do - rule = Review.find_or_create_rule(rule_id_or_name) - violation = RuleFailure.create_manual!(resource, rule, params) + rule = Rule.find_or_create_manual_rule(rule_id_or_name) + violation = rule.create_violation!(resource, params) violation.create_review!( :assignee => current_user, :user => 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 c4c87bbc44d..ce5e94d0176 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 @@ -39,8 +39,6 @@ class Review < ActiveRecord::Base RESOLUTION_FALSE_POSITIVE = 'FALSE-POSITIVE' RESOLUTION_FIXED = 'FIXED' - RULE_REPOSITORY_KEY = 'review' - def on_project? resource_id==project_id end @@ -410,20 +408,6 @@ class Review < ActiveRecord::Base json end - - 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 - # # # PRIVATE METHODS diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb index af391f2c97e..8ebc14b6f45 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb @@ -19,7 +19,10 @@ # class Rule < ActiveRecord::Base - validates_presence_of :name, :plugin_rule_key, :plugin_name + MANUAL_REPOSITORY_KEY = 'manual' + + validates_presence_of :name, :plugin_name + validates_presence_of :plugin_rule_key, :if => 'name.present?' has_many :rules_parameters has_many :rule_failures @@ -66,11 +69,11 @@ class Rule < ActiveRecord::Base def name @l10n_name ||= - begin - result = Java::OrgSonarServerUi::JRubyFacade.getInstance().getRuleName(I18n.locale, repository_key, plugin_rule_key) - result = read_attribute(:name) unless result - result - end + begin + result = Java::OrgSonarServerUi::JRubyFacade.getInstance().getRuleName(I18n.locale, repository_key, plugin_rule_key) + result = read_attribute(:name) unless result + result + end end def name=(value) @@ -79,11 +82,11 @@ class Rule < ActiveRecord::Base def description @l10n_description ||= - begin - result = Java::OrgSonarServerUi::JRubyFacade.getInstance().getRuleDescription(I18n.locale, repository_key, plugin_rule_key) - result = read_attribute(:description) unless result - result - end + begin + result = Java::OrgSonarServerUi::JRubyFacade.getInstance().getRuleDescription(I18n.locale, repository_key, plugin_rule_key) + result = read_attribute(:description) unless result + result + end end def description=(value) @@ -122,6 +125,43 @@ class Rule < ActiveRecord::Base rule end + def self.manual_rules + Rule.find(:all, :conditions => ['enabled=? and plugin_name=?', true, MANUAL_REPOSITORY_KEY], :order => 'name') + end + + def self.manual_rule(id) + Rule.find(:first, :conditions => ['enabled=? and plugin_name=? and id=?', true, MANUAL_REPOSITORY_KEY, id]) + end + + def self.find_or_create_manual_rule(rule_id_or_name) + if Api::Utils.is_integer?(rule_id_or_name) + rule = Rule.find(:first, :conditions => {:enabled => true, :plugin_name => MANUAL_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 => MANUAL_REPOSITORY_KEY, :plugin_rule_key => key}) + unless rule + rule = Rule.create!(:enabled => true, :plugin_name => MANUAL_REPOSITORY_KEY, :plugin_rule_key => key, :name => rule_id_or_name) + end + end + rule + end + + def create_violation!(resource, options={}) + line = options['line'] + checksum = nil + level = Sonar::RulePriority.id(options['severity']||Severity::MAJOR) + RuleFailure.create!( + :snapshot => resource.last_snapshot, + :rule => self, + :failure_level => level, + :message => options['message'], + :cost => (options['cost'] ? options['cost'].to_f : nil), + :switched_off => false, + :line => line, + :checksum => checksum) + end + + def to_hash_json(profile) json = {'title' => name, 'key' => key, 'plugin' => plugin_name, 'config_key' => config_key} json['description'] = description if description @@ -277,13 +317,4 @@ class Rule < ActiveRecord::Base end rules end - - def self.find_or_create_for_review(name) - key = name.strip.downcase.sub(/\s+/, '_') - rule = find(:first, :conditions => {:enabled => true, :plugin_name => REVIEW_REPOSITORY, :plugin_rule_key => key}) - unless rule - rule = create!(:enabled => true, :plugin_name => REVIEW_REPOSITORY, :plugin_rule_key => key, :name => name) - end - rule - end end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb index 3bbdda0517d..2dcabc50107 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb @@ -130,21 +130,6 @@ class RuleFailure < ActiveRecord::Base self.review.save! end - def self.create_manual!(resource, rule, options={}) - line = options['line'] - checksum = nil - level = Sonar::RulePriority.id(options['severity']||Severity::MAJOR) - RuleFailure.create!( - :snapshot => resource.last_snapshot, - :rule => rule, - :failure_level => level, - :message => options['message'], - :cost => (options['cost'] ? options['cost'].to_f : nil), - :switched_off => false, - :line => line, - :checksum => checksum) - end - private def update_permanent_id if self.permanent_id.nil? && self.id diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/layouts/_layout.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/layouts/_layout.html.erb index a30afbef6c4..d3f06427235 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/layouts/_layout.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/layouts/_layout.html.erb @@ -78,6 +78,7 @@ <% if is_admin? %>
  • <%= message('event_categories.page') -%>
  • <%= message('manual_metrics.page') -%>
    • +
  • <%= message('manual_rules.page') -%>
  • <%= message('default_filters.page') -%>
  • <%= message('default_dashboards.page') -%>
    • <% controller.java_facade.getPages(Navigation::SECTION_CONFIGURATION, nil,nil, nil).each do |page| %> diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/manual_rules/index.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/manual_rules/index.html.erb new file mode 100644 index 00000000000..79b2e2fe174 --- /dev/null +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/manual_rules/index.html.erb @@ -0,0 +1,64 @@ + + + + <% if is_admin? %> + + + <% end %> + +
    + + + + + + + + + <% if @rules.empty? %> + + + + <% end %> + <% @rules.each do |rule| %> + + + + + <% end %> + +
    NameOperations
    <%= message('no_results') -%>
    + <%= h rule.name -%> +    + <%= h rule.description -%> + + <%= link_to 'Edit', {:action => 'edit', :id => rule.id}, {:id => "edit_#{u(rule.key)}"} %> + <%= link_to 'Delete', {:action => 'delete', :id => rule.id}, {:confirm => message('are_you_sure'), :id => "delete_#{u(rule.key)}", :method => 'delete'} %> +
    +     		+
    + + + + + + + + + + + + +
    + Name:
    +
    + Ex. : Performance +
    + Description:
    + +
    +
    + +
    +
    +
    diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/metrics/index.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/metrics/index.html.erb index a4cc8514682..9de32a8aa63 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/metrics/index.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/metrics/index.html.erb @@ -9,7 +9,7 @@ Description Domain Type - Operations + Operations @@ -20,14 +20,12 @@ <%= h metric.description -%> <%= metric.domain -%> <%= metric.value_type_name -%> - + <% if is_admin? && metric.updatable_online? %> - <%= button_to 'Edit', {:action => 'index', :id => metric.id}, {:class => 'action', :id => "edit_#{h(metric.short_name)}", :method => 'get'} %> + <%= link_to 'Edit', {:action => 'index', :id => metric.id}, {:class => 'action', :id => "edit_#{h(metric.short_name)}", :method => 'get'} %> <% end %> - - <% if is_admin? && metric.updatable_online? %> - <%= button_to 'Delete', {:action => 'delete_from_web', :id => metric.id}, {:confirm => "Warning : all the measures will be deleted.", :class => 'action', :id => "delete_#{h(metric.short_name)}"} %> + <%= link_to 'Delete', {:action => 'delete_from_web', :id => metric.id}, {:confirm => "Warning : all the measures will be deleted.", :id => "delete_#{h(metric.short_name)}"} %> <% 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 b041f146ae9..80003e74fcb 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 @@ -33,17 +33,7 @@
    - - - - - - -
    - - - <%= render :partial => 'markdown/help' -%> -
    + <%= message('cancel') -%> diff --git a/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java b/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java index 226e382556d..5eeadc156c2 100644 --- a/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java +++ b/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java @@ -164,9 +164,9 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase { } @Test - public void shouldNotDisableReviewRules() { - // the hardcoded repository "review" is used for manual violations - setupData("shouldNotDisableReviewRules"); + public void shouldNotDisableManualRules() { + // the hardcoded repository "manual" is used for manual violations + setupData("shouldNotDisableManualRules"); RegisterRules task = new RegisterRules(getSessionFactory(), new RuleRepository[]{new FakeRepository()}); task.start(); diff --git a/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/shouldNotDisableReviewRules.xml b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/shouldNotDisableManualRules.xml similarity index 84% rename from sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/shouldNotDisableReviewRules.xml rename to sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/shouldNotDisableManualRules.xml index 2f279b4d3a7..8eb56a1b61a 100644 --- a/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/shouldNotDisableReviewRules.xml +++ b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/shouldNotDisableManualRules.xml @@ -1,6 +1,6 @@ -