SONAR-4170 SONAR-3880 Make it possible to search for 'Removed' rules and to sort the rules by 'Removal date', and by 'Available date'

9 files changed, 124 insertions, 54 deletions

diff --git a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties
index 849d65b72c6..c22e6e18db9 100644
--- a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties
+++ b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties
@@ -52,7 +52,6 @@ descending=Descending
 description=Description
 directories=Directories
 directory=Directory
-disabled=Disabled
 display=Display
 download_verb=Download
 edit=Edit
@@ -104,6 +103,7 @@ projects=Projects
 raw=Raw
 refresh=Refresh
 remove=Remove
+removed=Removed
 rename=Rename
 reset_verb=Reset
 result=Result
@@ -1454,6 +1454,12 @@ rules_configuration.original_severity=Original severity
 rules_configuration.repository=Repository
 rules_configuration.status_filters=Status filters
 rules_configuration.available_rules=Available rules
+rules_configuration.sort_by=Sort by:
+rules_configuration.rule_name=Rule name
+rules_configuration.creation_date=Creation date
+rules_configuration.removal_date=Removal date
+rules_configuration.available_since=Available since
+rules_configuration.removed_since=Removed since
 
 
 #------------------------------------------------------------------------------
diff --git a/sonar-check-api/src/main/java/org/sonar/check/Rule.java b/sonar-check-api/src/main/java/org/sonar/check/Rule.java
index 8ed70868ed4..391ca5a4043 100644
--- a/sonar-check-api/src/main/java/org/sonar/check/Rule.java
+++ b/sonar-check-api/src/main/java/org/sonar/check/Rule.java
@@ -62,7 +62,7 @@ public @interface Rule {
   Cardinality cardinality() default Cardinality.SINGLE;
 
   /**
-   * The rule status. Can be Normal, Beta or Deprecated
+   * The rule status. Can be READY, BETA or DEPRECATED
    * @since 3.6
    */
   Status status() default Status.READY;
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 72da01da89f..4c2bafdc86f 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
@@ -93,6 +93,7 @@ public final class RegisterRules {
     for (Integer deprecatedUserRuleId : deprecatedUserRuleIds) {
       Rule rule = session.getSingleResult(Rule.class, "id", deprecatedUserRuleId);
       rule.setEnabled(false);
+      rule.setUpdatedAt(new Date());
       session.saveWithoutFlush(rule);
     }
 
@@ -100,7 +101,7 @@ public final class RegisterRules {
 
   private void disableAllRules(DatabaseSession session) {
     // 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();
+    session.createQuery("UPDATE " + Rule.class.getSimpleName() + " SET enabled=false, updated_at=current_timestamp WHERE parent IS NULL AND pluginName<>'manual' AND enabled=true").executeUpdate();
   }
 
   private void registerRepository(RuleRepository repository, DatabaseSession session) {
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb
index f982ec8ca31..0a9b2bd51bc 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb
@@ -50,11 +50,14 @@ class RulesConfigurationController < ApplicationController
     @select_activation = [[message('any'), 'any'], [message('active'), STATUS_ACTIVE], [message('inactive'), STATUS_INACTIVE]]
     @select_inheritance = [[message('any'), 'any'], [message('rules_configuration.not_inherited'), 'NOT'], [message('rules_configuration.inherited'), 'INHERITED'],
                            [message('rules_configuration.overrides'), 'OVERRIDES']]
-    @select_status = ANY_SELECTION + [[message('beta'), Rule::STATUS_BETA], [message('deprecated'), Rule::STATUS_DEPRECATED], [message('disabled'), Rule::STATUS_DISABLED]]
+    @select_status = ANY_SELECTION + [[message('beta'), Rule::STATUS_BETA], [message('deprecated'), Rule::STATUS_DEPRECATED], [message('removed'), Rule::STATUS_REMOVED]]
+    @select_sort_by = [[message('rules_configuration.rule_name'), Rule::SORT_BY_RULE_NAME]]
+    @select_sort_by << [message('rules_configuration.creation_date'), Rule::SORT_BY_CREATION_DATE] if !status_include_removed?
+    @select_sort_by << [message('rules_configuration.removal_date'), Rule::SORT_BY_REMOVAL_DATE] if status_include_removed?
 
     @rules = Rule.search(java_facade, {
         :profile => @profile, :activation => @activation, :priorities => @priorities, :inheritance => @inheritance, :status => @status,
-        :plugins => @plugins, :searchtext => @searchtext, :include_parameters_and_notes => true, :language => @profile.language})
+        :plugins => @plugins, :searchtext => @searchtext, :include_parameters_and_notes => true, :language => @profile.language, :sort_by => @sort_by})
 
     unless @searchtext.blank?
       @hidden_inactives = Rule.search(java_facade, {
@@ -373,11 +376,16 @@ class RulesConfigurationController < ApplicationController
   def init_params
     @id = params[:id]
     @priorities = filter_any(params[:priorities]) || ['']
-    @plugins=filter_any(params[:plugins]) || ['']
-    @activation=params[:rule_activation] || STATUS_ACTIVE
-    @inheritance=params[:inheritance] || 'any'
-    @status=params[:status]
-    @searchtext=params[:searchtext]
+    @plugins = filter_any(params[:plugins]) || ['']
+    @activation = params[:rule_activation] || STATUS_ACTIVE
+    @inheritance = params[:inheritance] || 'any'
+    @status = params[:status]
+    @sort_by = params[:sort_by]
+    # Force sort by removal date when status contains REMOVED
+    @sort_by = Rule::SORT_BY_REMOVAL_DATE if status_include_removed?
+    @sort_by = Rule::SORT_BY_RULE_NAME if !status_include_removed? and @sort_by == Rule::SORT_BY_REMOVAL_DATE
+    @sort_by ||= Rule::SORT_BY_RULE_NAME
+    @searchtext = params[:searchtext]
   end
 
   def filter_any(array)
@@ -387,4 +395,8 @@ class RulesConfigurationController < ApplicationController
     array
   end
 
+  def status_include_removed?
+    !params[:status].blank? and params[:status].include?(Rule::STATUS_REMOVED)
+  end
+
 end
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 64782ad1d82..a982ab0a8da 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
@@ -21,9 +21,14 @@ class Rule < ActiveRecord::Base
 
   MANUAL_REPOSITORY_KEY = 'manual'
 
+  STATUS_READY = "READY"
   STATUS_BETA = "BETA"
   STATUS_DEPRECATED = "DEPRECATED"
-  STATUS_DISABLED = "DISABLED"
+  STATUS_REMOVED = "REMOVED"
+
+  SORT_BY_RULE_NAME = "SORT_BY_RULE_NAME"
+  SORT_BY_CREATION_DATE = "SORT_BY_CREATION_DATE"
+  SORT_BY_REMOVAL_DATE = "SORT_BY_REMOVAL_DATE"
 
   validates_presence_of :name, :description, :plugin_name
   validates_presence_of :plugin_rule_key, :if => 'name.present?'
@@ -63,6 +68,10 @@ class Rule < ActiveRecord::Base
     cardinality=='MULTIPLE'
   end
 
+  def ready?
+    enabled && status == STATUS_READY
+  end
+
   def editable?
     !parent_id.nil?
   end
@@ -254,33 +263,32 @@ class Rule < ActiveRecord::Base
   end
 
 
-  # options :language => nil, :plugins => [], :searchtext => '', :profile => nil, :priorities => [], :activation => '', :status => []
+  # options :language => nil, :plugins => [], :searchtext => '', :profile => nil, :priorities => [], :activation => '', :status => [], :sort_by => nil
   def self.search(java_facade, options={})
-    conditions = ['enabled=:enabled']
-    values = {:enabled => true}
+    conditions = []
+    values = {}
 
     status = options[:status]
-    if status && !status.empty?
-      values[:enabled] = !status.include?(STATUS_DISABLED)
+    if status.blank?
+      conditions << ['enabled=:enabled']
+      values[:enabled] = true
+    else
+      if !status.include?(STATUS_REMOVED)
+        conditions << ['enabled=:enabled']
+        values[:enabled] = true
+      else
+        # As we want the enabled and disabled rules, we do not filter on the enabled column
+      end
     end
 
-    plugins=nil
-    if remove_blank(options[:plugins])
-      plugins = options[:plugins]
-      unless options[:language].blank?
-        plugins = plugins & java_facade.getRuleRepositoriesByLanguage(options[:language]).collect { |repo| repo.getKey() }
-      end
-    elsif !options[:language].blank?
-      plugins = java_facade.getRuleRepositoriesByLanguage(options[:language]).collect { |repo| repo.getKey() }
+    unless options[:language].blank?
+      conditions << ['language=:language']
+      values[:language] = options[:language]
     end
 
-    if plugins
-      if plugins.empty?
-        conditions << "plugin_name IS NULL"
-      else
-        conditions << "plugin_name IN (:plugin_names)"
-        values[:plugin_names] = plugins
-      end
+    if remove_blank(options[:plugins])
+      conditions << "plugin_name IN (:plugin_names)"
+      values[:plugin_names] = options[:plugins]
     end
 
     unless options[:searchtext].blank?
@@ -299,7 +307,8 @@ class Rule < ActiveRecord::Base
     end
 
     includes=(options[:include_parameters_and_notes] ? [:rules_parameters, :rule_note] : nil)
-    rules = Rule.all(:include => includes, :conditions => [conditions.join(" AND "), values]).sort
+    rules = Rule.all(:include => includes, :conditions => [conditions.join(" AND "), values])
+    rules = Rule.sort_by(rules, options[:sort_by])
     filter(rules, options)
   end
 
@@ -320,12 +329,11 @@ class Rule < ActiveRecord::Base
 
     # The status cannot be filter in the SQL query because the disabled state is not set in the status column but in the disabled column.
     # For instance, a rule can be disabled and in status BETA in database, but in real it has to be considered only as disabled and must be returned when searching only for DISABLED.
-    if status && !status.empty?
-      status_list = status.split(',')
+    if status
       rules = rules.reject do |rule|
-        include_rule = rule.beta? && !status_list.include?(STATUS_BETA) ||
-           rule.deprecated? && !status_list.include?(STATUS_DEPRECATED) ||
-           rule.disabled? && !status_list.include?(STATUS_DISABLED)
+        include_rule = rule.beta? && status.include?(STATUS_BETA) ||
+           rule.deprecated? && status.include?(STATUS_DEPRECATED) ||
+           rule.disabled? && status.include?(STATUS_REMOVED)
         !include_rule
       end
     end
@@ -365,4 +373,17 @@ class Rule < ActiveRecord::Base
     end
     rules
   end
+
+  def self.sort_by(rules, sort_by)
+    case sort_by
+      when SORT_BY_CREATION_DATE
+        rules = rules.sort_by {|rule| rule.created_at}.reverse
+      when SORT_BY_REMOVAL_DATE
+        rules = rules.sort {|rule1, rule2| (rule1.updated_at && rule2.updated_at) ? rule1.updated_at <=> rule2.updated_at : 0}
+      else
+        rules = rules.sort
+    end
+    rules
+  end
+
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb
index 8b596b6d949..e78a243a253 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb
@@ -28,21 +28,23 @@
   </form>
 </td>
 
-<td class="left">
+<td class="left" colspan="2">
   <div class="h3 rule-title"><%= link_to_function("#{h rule.name}", nil, :class => "") do |page|
     page.toggle "desc_#{rule.id}"
   end
   %></div>
 
-  <div class="rule-status">
-  <% if rule.beta? %>
-    <div class="status-beta"><%= message('beta') %></div>
-  <% elsif rule.deprecated? %>
-    <div class="status-deprecated"><%= message('deprecated') %></div>
-  <% elsif rule.disabled? %>
-    <div class="status-disabled"><%= message('disabled') %></div>
+  <% if !rule.ready? %>
+    <div class="rule-status">
+    <% if rule.beta? %>
+      <div class="status-beta"><%= message('beta') %></div>
+    <% elsif rule.deprecated? %>
+      <div class="status-deprecated"><%= message('deprecated') %></div>
+    <% elsif rule.disabled? %>
+      <div class="status-removed"><%= message('removed') %></div>
+    <% end %>
+    </div>
   <% end %>
-  </div>
 
   <div id="desc_<%= rule.id -%>" class="rule_detail" style="<%= 'display:none' -%>">
     <a name="rule<%= rule.id -%>"></a>
@@ -112,6 +114,11 @@
       <div style="float: left">
         <div style="color: #888" id="rule_repository_<%= rule.id -%>"><%= message('rules_configuration.repository')%> : <%= rule.repository_key %></div>
         <div style="color: #888" id="rule_key_<%= rule.id -%>"><%= message('key')%> : <%= rule.plugin_rule_key %></div>
+        <% if !rule.disabled? %>
+          <div style="color: #888" id="rule_available_since_<%= rule.id -%>"><%= message('rules_configuration.available_since')%> : <%= human_short_date(rule.created_at) %></div>
+        <% else %>
+          <div style="color: #888" id="rule_available_since_<%= rule.id -%>"><%= message('rules_configuration.removed_since')%> : <%= human_short_date(rule.updated_at) %></div>
+        <% end  %>
       </div>
     </div>
   </div>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/index.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/index.html.erb
index 49c7d2377d5..56cad81e684 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/index.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/index.html.erb
@@ -35,8 +35,8 @@
 <% enable_modification = is_admin? %>
 <div class="tabs-panel marginbottom10 background-gray">
 
-<% form_tag({:action => 'index'}, {:method => 'get'}) do %>
-    <% hidden_field_tag "id", @id %>
+<% form_tag({:action => 'index'}, {:id => 'rules-search-form', :method => 'get'}) do %>
+    <%= hidden_field_tag "sort_by", @sort_by, :id => 'sort_by' %>
 
     <div class="table" id="search_table">
       <div class="rule-search top">
@@ -99,19 +99,30 @@
     <% end %>
   </ul>
   <h2>
-     <% if @hidden_actives && @hidden_actives>0 %><span class="small"><a href="<%= url_for params.merge({:rule_activation => ''}) -%>" id="active-rules-link">+<%= message('rules_configuration.x_found_in_active_rules', :params => @hidden_actives) -%></a></span><% end %>
-     <% if @hidden_inactives && @hidden_inactives>0 %><span class="small"><a href="<%= url_for params.merge({:rule_activation => ''}) -%>" id="inactive-rules-link">+<%= message('rules_configuration.x_found_in_inactive_rules', :params => @hidden_inactives) -%></a></span><% end %>
+     <% if @hidden_actives && @hidden_actives>0 %>
+      <span class="small"><a href="<%= url_for params.merge({:rule_activation => ''}) -%>"
+        id="active-rules-link">+<%= message('rules_configuration.x_found_in_active_rules', :params => @hidden_actives) -%></a></span><% end %>
+     <% if @hidden_inactives && @hidden_inactives>0 %>
+      <span class="small"><a href="<%= url_for params.merge({:rule_activation => ''}) -%>"
+        id="inactive-rules-link">+<%= message('rules_configuration.x_found_in_inactive_rules', :params => @hidden_inactives) -%></a></span><% end %>
   </h2>
 </div>
 <table class="table table-bordered width100 spacer-bottom" id="result_table">
   <thead>
     <tr>
-        <th class="left" nowrap><%= message('active') -%>/<%= message('severity') -%></th>
-        <th class="left"><%= message('name') -%> <span style="font-weight: normal">[<%= link_to_function(message('rules_configuration.expand'), "expand_rules()") %> / <%= link_to_function(message('rules_configuration.collapse'), "collapse_rules()") %>]</span></th>
+      <th class="left" nowrap><%= message('active') -%>/<%= message('severity') -%></th>
+      <th class="left"><%= message('name') -%> <span style="font-weight: normal">
+        [<%= link_to_function(message('rules_configuration.expand'), "expand_rules()") %> / <%= link_to_function(message('rules_configuration.collapse'), "collapse_rules()") %>]</span></th>
+      <th class="right thin" nowrap>
+        <% form_tag({:action => 'index'}) do %>
+          <%= message('rules_configuration.sort_by') -%>
+          <%= select_tag "sort_by", options_for_select(@select_sort_by, @sort_by), :id => 'select-rules-sort-by', :onChange => 'submitSortBy();' %>
+        <% end %>
+      </th>
     </tr>
 </thead>
 
-  <%= table_pagination(@pagination, :colspan => 2) { |label, page_id| link_to(label, params.merge({:page => page_id}))} -%>
+<%= table_pagination(@pagination, :colspan => 2) { |label, page_id| link_to(label, params.merge({:page => page_id}))} -%>
 
 <tbody>
 <% if @current_rules.empty? %>
@@ -138,6 +149,11 @@
     $(span_id).replace(text_area);
   }
 
+  function submitSortBy(){
+    $j('#sort_by').val($j('#select-rules-sort-by').val());
+    $j("#rules-search-form").submit();
+  }
+
   $j('#search_plugin').select2({
     width: '120px',
     placeholder: "<%= message('any') -%>"
diff --git a/sonar-server/src/main/webapp/stylesheets/style.css b/sonar-server/src/main/webapp/stylesheets/style.css
index 8646a25211d..32c9f39a2eb 100644
--- a/sonar-server/src/main/webapp/stylesheets/style.css
+++ b/sonar-server/src/main/webapp/stylesheets/style.css
@@ -2419,6 +2419,6 @@ textarea.width100 {
   color: #906947;
 }
 
-.rule-status .status-disabled {
+.rule-status .status-removed {
   color: #904553;
 }
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 6f7b57b7756..06d3e943357 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
@@ -88,6 +88,7 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase {
     assertThat(rules.size(), greaterThan(0));
     for (Rule rule : rules) {
       assertThat(rule.isEnabled(), is(false));
+      assertThat(rule.getUpdatedAt(), notNullValue());
     }
   }
 
@@ -102,6 +103,7 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase {
     Rule deprecated = result.get(0);
     assertThat(deprecated.getKey(), is("deprecated"));
     assertThat(deprecated.isEnabled(), is(false));
+    assertThat(deprecated.getUpdatedAt(), notNullValue());
 
     assertThat(result.get(1).isEnabled(), is(true));
     assertThat(result.get(2).isEnabled(), is(true));
@@ -124,9 +126,11 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase {
 
     Rule rule = getSession().getSingleResult(Rule.class, "id", 1);
     assertThat(rule.isEnabled(), is(false));
+    assertThat(rule.getUpdatedAt(), notNullValue());
 
     rule = getSession().getSingleResult(Rule.class, "id", 2);
     assertThat(rule.isEnabled(), is(false));
+    assertThat(rule.getUpdatedAt(), notNullValue());
   }
 
   @Test
@@ -140,9 +144,11 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase {
     assertThat(rule1.getDescription(), is("Description of One"));
     assertThat(rule1.getSeverity(), is(RulePriority.BLOCKER));
     assertThat(rule1.getConfigKey(), is("config1"));
+    assertThat(rule1.getUpdatedAt(), notNullValue());
 
     Rule rule2 = getSession().getSingleResult(Rule.class, "id", 2);
     assertThat(rule2.getStatus(), is(Status.DEPRECATED.name()));
+    assertThat(rule2.getUpdatedAt(), notNullValue());
   }
 
   @Test
@@ -184,6 +190,7 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase {
 
     Rule rule = getSession().getSingleResult(Rule.class, "id", 2);
     assertThat(rule.isEnabled(), is(false));
+    assertThat(rule.getUpdatedAt(), notNullValue());
   }
 
   @Test