]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-1974 add a page to manage manual rules
authorsimonbrandhof <simon.brandhof@gmail.com>
Wed, 7 Dec 2011 17:57:05 +0000 (18:57 +0100)
committersimonbrandhof <simon.brandhof@gmail.com>
Wed, 7 Dec 2011 17:59:51 +0000 (18:59 +0100)
Manual rules allow users to create their own violations directly from UI. These rules can
also be defined 'on the fly' while creating violations.

15 files changed:
plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties
sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java
sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb
sonar-server/src/main/webapp/WEB-INF/app/controllers/manual_rules_controller.rb [new file with mode: 0644]
sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
sonar-server/src/main/webapp/WEB-INF/app/models/review.rb
sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb
sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb
sonar-server/src/main/webapp/WEB-INF/app/views/layouts/_layout.html.erb
sonar-server/src/main/webapp/WEB-INF/app/views/manual_rules/index.html.erb [new file with mode: 0644]
sonar-server/src/main/webapp/WEB-INF/app/views/metrics/index.html.erb
sonar-server/src/main/webapp/WEB-INF/app/views/resource/_create_violation_form.html.erb
sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java
sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/shouldNotDisableManualRules.xml [new file with mode: 0644]
sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/shouldNotDisableReviewRules.xml [deleted file]

index b40853ec03daf95f4c3ad17d06c4126303d13029..bac0836f52664cbfde48b8e8a452cdf4e069e265 100644 (file)
@@ -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
index 005775ea87766178d64971796bafc7fc6f4b71cc..4cbfa583f9366dfbab2895b26c0b2ccdfc8e90a9 100644 (file)
@@ -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) {
index 1c8a80f3bf5ba33d349ec1d5b8b743ab5f64f039..59c69bdbd406279f0a5a0d4fc8669049954d801f 100644 (file)
@@ -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 (file)
index 0000000..9f489df
--- /dev/null
@@ -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
index 35eaf9bb45f6aec276cf96ec5f1434e7c6e92abb..b891e9ab3048edac9a52ec3c6c7fafe28dbb233f 100644 (file)
@@ -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,
index c4c87bbc44dcdfff20ea8f79e6952aa2017996de..ce5e94d0176dbbec99a0f34236e8af536b296e03 100644 (file)
@@ -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
index af391f2c97ea5eab229ddb31e452a2afb2723072..8ebc14b6f4503b1246bd34bc4b4f025e80e063c3 100644 (file)
 #
 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
index 3bbdda0517d0fd79fd7f404c63c50bf9c8c71128..2dcabc50107454e9baa3406f6ad18143d0f5f0fc 100644 (file)
@@ -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
index a30afbef6c4bbd196ccf00639844810d99cd4e67..d3f06427235601b7f204f7f0e50ddd7c2f7f5806 100644 (file)
@@ -78,6 +78,7 @@
             <% if is_admin? %>
               <li class="<%= 'selected' if controller.controller_path=='event_categories' -%>"><a href="<%= ApplicationController.root_context -%>/event_categories/index"><%= message('event_categories.page') -%></a></li>
               <li class="<%= 'selected' if controller.controller_path=='metrics' -%>"><a href="<%= ApplicationController.root_context -%>/metrics/index"><%= message('manual_metrics.page') -%></a></li>
+              <li class="<%= 'selected' if controller.controller_path=='manual_rules' -%>"><a href="<%= ApplicationController.root_context -%>/manual_rules/index"><%= message('manual_rules.page') -%></a></li>
               <li class="<%= 'selected' if controller.controller_path=='admin_filters' -%>"><a href="<%= ApplicationController.root_context -%>/admin_filters/index"><%= message('default_filters.page') -%></a></li>
               <li class="<%= 'selected' if controller.controller_path=='admin_dashboards' -%>"><a href="<%= ApplicationController.root_context -%>/admin_dashboards/index"><%= message('default_dashboards.page') -%></a></li>
               <% 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 (file)
index 0000000..79b2e2f
--- /dev/null
@@ -0,0 +1,64 @@
+<table width="100%">
+  <tr>
+    <td valign="top">
+      <table class="data width100" id="manual-rules">
+        <thead>
+        <tr>
+          <th>Name</th>
+          <th class="right">Operations</th>
+        </tr>
+        </thead>
+        <tbody>
+        <% if @rules.empty? %>
+          <tr class="even">
+            <td colspan="2"><%= message('no_results') -%></td>
+          </tr>
+        <% end %>
+        <% @rules.each do |rule| %>
+          <tr class="<%= cycle('even', 'odd') -%>">
+            <td class="left" nowrap>
+              <%= h rule.name -%>
+              &nbsp;&nbsp;
+              <span class="note"><%= h rule.description -%></span>
+            </td>
+            <td class="right thin nowrap">
+              <%= 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'} %>
+            </td>
+          </tr>
+        <% end %>
+        </tbody>
+      </table>
+    </td>
+    <% if is_admin? %>
+      <td class="sep"></td>
+      <td valign="top" align="right" width="210">
+        <form action="<%= ApplicationController.root_context -%>/manual_rules/create" method="POST">
+          <table class="admintable" width="100%">
+            <input type="hidden" name="id" value="<%= @rule.id -%>"/>
+
+            <tr>
+              <td class="left" valign="top">
+                Name:<br/><input type="text" name="name" value="<%= h @rule.name -%>"/>
+                <br/>
+                <span class="desc">Ex. : Performance</span>
+              </td>
+            </tr>
+            <tr>
+              <td class="left" valign="top">
+                Description:<br/>
+                <textarea rows="5" cols="25" name="description"><%= h(@rule.description) -%></textarea>
+                <br/>
+              </td>
+            </tr>
+            <tr>
+              <td class="left" valign="top">
+                <input type="submit" value="<%= @rule.id.nil? ? 'Create' : 'Update' -%>"/>
+              </td>
+            </tr>
+          </table>
+        </form>
+      </td>
+    <% end %>
+  </tr>
+</table>
index a4cc8514682543eca4f78662554e2d1ccba5822d..9de32a8aa63ebdb7eb12f29089eeac888bc4b955 100644 (file)
@@ -9,7 +9,7 @@
             <th class="left">Description</th>
             <th class="left">Domain</th>
             <th class="left">Type</th>
-            <th class="left nosort" colspan="2">Operations</th>
+            <th class="left nosort">Operations</th>
           </tr>
         </thead>
         <tbody>
           <td class="left"><%= h metric.description -%></td>
           <td class="left" ><%= metric.domain -%></td>
           <td class="left" ><%= metric.value_type_name -%></td>
-            <td class="left" width="1%" nowrap>
+          <td class="right thin nowrap">
             <% 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 %>
-          </td>
-          <td class="left">
             <% 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 %>
           </td>
         </tr>
index b041f146ae90c883094b4f90d52db983291b3936..80003e74fcb9e7fe8c012a9df34883a925d50727 100644 (file)
         </div>
 
         <div class="discussionComment first">
-          <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>
+          <textarea rows="5" name="message" style="width: 100%"></textarea>
           <div class="error" id="errorViolationForm<%= @line -%>" style="display: none"></div>
           <input type="submit" value="<%= message('code_viewer.create_violation.submit') -%>">
           <a href="#" onclick="return hVF(<%= @line -%>)"><%= message('cancel') -%></a>
index 226e382556d73f2df296e13bfb2d9a72897c3a33..5eeadc156c291d6d319bd1bed8ca5ae6cbfbdec5 100644 (file)
@@ -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/shouldNotDisableManualRules.xml b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/shouldNotDisableManualRules.xml
new file mode 100644 (file)
index 0000000..8eb56a1
--- /dev/null
@@ -0,0 +1,9 @@
+<dataset>
+
+  <rules id="1" plugin_rule_key="PerformanceIssue" plugin_name="manual" plugin_config_key="[null]" name="Performance Issue" description="[null]"
+         enabled="true" priority="[null]" cardinality="SINGLE" parent_id="[null]"/>
+
+  <rules id="2" plugin_rule_key="IllegalExceptionCheck" plugin_name="checkstyle" plugin_config_key="[null]" name="Illegal Exception" description="[null]"
+         enabled="true" priority="4" cardinality="SINGLE" parent_id="[null]"/>
+
+</dataset>
\ No newline at end of file
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/shouldNotDisableReviewRules.xml
deleted file mode 100644 (file)
index 2f279b4..0000000
+++ /dev/null
@@ -1,9 +0,0 @@
-<dataset>
-
-  <rules id="1" plugin_rule_key="PerformanceIssue" plugin_name="review" plugin_config_key="[null]" name="Performance Issue" description="[null]"
-         enabled="true" priority="[null]" cardinality="SINGLE" parent_id="[null]"/>
-
-  <rules id="2" plugin_rule_key="IllegalExceptionCheck" plugin_name="checkstyle" plugin_config_key="[null]" name="Illegal Exception" description="[null]"
-         enabled="true" priority="4" cardinality="SINGLE" parent_id="[null]"/>
-
-</dataset>
\ No newline at end of file