]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5056 Display debt information from rules instead of characteristics
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 11 Mar 2014 17:07:22 +0000 (18:07 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 11 Mar 2014 17:08:06 +0000 (18:08 +0100)
sonar-core/src/main/java/org/sonar/core/technicaldebt/DefaultTechnicalDebtManager.java
sonar-plugin-api/src/main/java/org/sonar/api/rules/Rule.java
sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/Characteristic.java
sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java
sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb
sonar-server/src/main/webapp/WEB-INF/app/views/issue/_rule.html.erb
sonar-server/src/test/java/org/sonar/server/issue/ws/IssueShowWsHandlerTest.java

index 80ed004da3ac5bc38584b26515f295310d7e6554..b9d6d6b6d9a286e8d0425171c4cb771801409d7a 100644 (file)
@@ -72,6 +72,10 @@ public class DefaultTechnicalDebtManager implements TechnicalDebtManager {
     return null;
   }
 
+  /**
+   * @deprecated since 4.3. Always return null
+   */
+  @Deprecated
   @CheckForNull
   public Characteristic findRequirementByRule(Rule rule) {
     return null;
index d30275399ea6779f70bb2d814c1d36057fbd77c5..39453f963ff39a9d51dda44f7e402a4bf6cfa507 100644 (file)
@@ -36,7 +36,10 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import javax.persistence.*;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+import java.util.Set;
 
 @Entity
 @Table(name = "rules")
@@ -117,6 +120,12 @@ public class Rule {
   @JoinColumn(name = "parent_id", updatable = true, nullable = true)
   private Rule parent = null;
 
+  @Column(name = "characteristic_id", updatable = true, nullable = true)
+  private Integer characteristicId;
+
+  @Column(name = "default_characteristic_id", updatable = true, nullable = true)
+  private Integer defaultCharacteristicId;
+
   @org.hibernate.annotations.Cascade({org.hibernate.annotations.CascadeType.ALL, org.hibernate.annotations.CascadeType.DELETE_ORPHAN})
   @OneToMany(mappedBy = "rule")
   private List<RuleParam> params = new ArrayList<RuleParam>();
@@ -439,6 +448,47 @@ public class Rule {
     this.tags = tags;
   }
 
+  /**
+   * For internal use only.
+   *
+   * @since 4.3
+   */
+  @CheckForNull
+  public Integer getCharacteristicId() {
+    return characteristicId;
+  }
+
+  /**
+   * For internal use only.
+   *
+   * @since 4.3
+   */
+  public Rule setCharacteristicId(@Nullable Integer characteristicId) {
+    this.characteristicId = characteristicId;
+    return this;
+  }
+
+
+  /**
+   * For internal use only.
+   *
+   * @since 4.3
+   */
+  @CheckForNull
+  public Integer getDefaultCharacteristicId() {
+    return defaultCharacteristicId;
+  }
+
+  /**
+   * For internal use only.
+   *
+   * @since 4.3
+   */
+  public Rule setDefaultCharacteristicId(@Nullable Integer defaultCharacteristicId) {
+    this.defaultCharacteristicId = defaultCharacteristicId;
+    return this;
+  }
+
   @Override
   public boolean equals(Object obj) {
     if (!(obj instanceof Rule)) {
index 47dcf172d711d446e92ae34c533c5028474193f0..c39eba4108a9ddc6d334a374bb04417adc50671b 100644 (file)
@@ -42,11 +42,23 @@ public interface Characteristic {
   @CheckForNull
   Integer parentId();
 
+  /**
+   * @deprecated since 4.3. return null
+   */
+  @Deprecated
   @CheckForNull
   Integer rootId();
 
+  /**
+   * @deprecated since 4.3. return null
+   */
+  @Deprecated
   RuleKey ruleKey();
 
+  /**
+   * @deprecated since 4.3. return null
+   */
+  @Deprecated
   String function();
 
   /**
index 7800bd090b31defb1f0ff05a2695028f55764a9e..b3a1316ae42c76eb0c62fda2b58ff1d2db76d05f 100644 (file)
@@ -189,13 +189,12 @@ public class IssueShowWsHandler implements RequestHandler {
   }
 
   private void addCharacteristics(IssueQueryResult result, DefaultIssue issue, JsonWriter json) {
-    Characteristic requirement = technicalDebtManager.findRequirementByRule(result.rule(issue));
-    // Requirement can be null if it has been disabled
-    if (requirement != null) {
-      Characteristic characteristic = findCharacteristicById(requirement.rootId());
+    Integer subCharacteristicId = result.rule(issue).getCharacteristicId() != null ? result.rule(issue).getCharacteristicId() : result.rule(issue).getDefaultCharacteristicId();
+    Characteristic subCharacteristic = findCharacteristicById(subCharacteristicId);
+    if (subCharacteristic != null) {
+      json.prop("subCharacteristic", subCharacteristic.name());
+      Characteristic characteristic = findCharacteristicById(subCharacteristic.parentId());
       json.prop("characteristic", characteristic != null ? characteristic.name() : null);
-      Characteristic subCharacteristic = findCharacteristicById(requirement.parentId());
-      json.prop("subCharacteristic", subCharacteristic != null ? subCharacteristic.name() : null);
     }
   }
 
index ba3bd551ade6fdd8f2dc3b72cec321ab14425db2..2df6b11d23dc4cd6b5e64974a6356be9be7e9150 100644 (file)
@@ -187,11 +187,10 @@ class IssueController < ApplicationController
     require_parameters :id
     rule_key = params[:id].split(':')
     @rule = Rule.first(:conditions => ['plugin_name=? and plugin_rule_key=?', rule_key[0], rule_key[1]])
-    @requirement = Internal.debt.findRequirementByRuleId(@rule.id)
-    # Requirement can be null if it's disabled or if there's no requirement on this rule
-    if @requirement
-      @characteristic = Internal.debt.findCharacteristic(@requirement.parentId)
-      @root_characteristic = Internal.debt.findCharacteristic(@requirement.rootId)
+    characteristic_id = @rule.characteristic_id || @rule.default_characteristic_id
+    if characteristic_id
+      @characteristic = Internal.debt.findCharacteristic(characteristic_id)
+      @root_characteristic = Internal.debt.findCharacteristic(@characteristic.parentId())
     end
     render :partial => 'issue/rule'
   end
index 3c7b697508a2c9231282ec0e6b150dd018059a5d..f466660d508f4dcbd5c9bbf80ea6372bb3590663 100644 (file)
@@ -17,7 +17,7 @@
 <p class="note">
   <span class="spacer-right"><%= h @rule.plugin_name -%>:<%= h @rule.plugin_rule_key -%></span>
   <%= image_tag 'sep12.png', :class => 'spacer-right' -%>
-  <% if @requirement %>
+  <% if @characteristic %>
     <%= @root_characteristic.name -%>&nbsp;&gt;&nbsp;<%= @characteristic.name -%>
   <% else %>
     <%= message 'issue.technical_debt_deleted' %>
index 5e7113c43db96cf3232a4c885ca48eae0ae9654a..b27ded9403b7eced401cfae55d55c5f8c68de8dc 100644 (file)
@@ -38,7 +38,6 @@ import org.sonar.api.issue.internal.FieldDiffs;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rules.Rule;
 import org.sonar.api.server.ws.WsTester;
-import org.sonar.api.technicaldebt.server.Characteristic;
 import org.sonar.api.technicaldebt.server.internal.DefaultCharacteristic;
 import org.sonar.api.user.User;
 import org.sonar.api.utils.DateUtils;
@@ -319,16 +318,45 @@ public class IssueShowWsHandlerTest {
   }
 
   @Test
-  public void show_issue_with_characteristics() throws Exception {
+  public void show_issue_with_user_characteristics() throws Exception {
     Issue issue = createStandardIssue().setDebt(Duration.create(7260L));
     issues.add(issue);
 
-    Characteristic requirement = new DefaultCharacteristic().setId(5).setParentId(2).setRootId(1);
-    Characteristic characteristic = new DefaultCharacteristic().setId(1).setName("Maintainability");
-    Characteristic subCharacteristic = new DefaultCharacteristic().setId(2).setName("Readability");
-    when(technicalDebtManager.findRequirementByRule(result.rule(issue))).thenReturn(requirement);
-    when(technicalDebtManager.findCharacteristicById(1)).thenReturn(characteristic);
-    when(technicalDebtManager.findCharacteristicById(2)).thenReturn(subCharacteristic);
+    result.rule(issue).setCharacteristicId(2);
+    when(technicalDebtManager.findCharacteristicById(1)).thenReturn(new DefaultCharacteristic().setId(1).setName("Maintainability"));
+    when(technicalDebtManager.findCharacteristicById(2)).thenReturn(new DefaultCharacteristic().setId(2).setName("Readability").setParentId(1));
+
+    MockUserSession.set();
+    WsTester.TestRequest request = tester.newRequest("show").setParam("key", issue.key());
+    request.execute().assertJson(getClass(), "show_issue_with_characteristics.json");
+  }
+
+  @Test
+  public void show_issue_with_default_characteristics() throws Exception {
+    Issue issue = createStandardIssue().setDebt(Duration.create(7260L));
+    issues.add(issue);
+
+    result.rule(issue).setDefaultCharacteristicId(2);
+    when(technicalDebtManager.findCharacteristicById(1)).thenReturn(new DefaultCharacteristic().setId(1).setName("Maintainability"));
+    when(technicalDebtManager.findCharacteristicById(2)).thenReturn(new DefaultCharacteristic().setId(2).setName("Readability").setParentId(1));
+
+    MockUserSession.set();
+    WsTester.TestRequest request = tester.newRequest("show").setParam("key", issue.key());
+    request.execute().assertJson(getClass(), "show_issue_with_characteristics.json");
+  }
+
+  @Test
+  public void use_user_characteristic_if_both_characteristic_ids_are_defined() throws Exception {
+    Issue issue = createStandardIssue().setDebt(Duration.create(7260L));
+    issues.add(issue);
+
+    result.rule(issue).setCharacteristicId(2);
+    when(technicalDebtManager.findCharacteristicById(1)).thenReturn(new DefaultCharacteristic().setId(1).setName("Maintainability"));
+    when(technicalDebtManager.findCharacteristicById(2)).thenReturn(new DefaultCharacteristic().setId(2).setName("Readability").setParentId(1));
+
+    result.rule(issue).setDefaultCharacteristicId(20);
+    when(technicalDebtManager.findCharacteristicById(10)).thenReturn(new DefaultCharacteristic().setId(10).setName("Default Maintainability"));
+    when(technicalDebtManager.findCharacteristicById(20)).thenReturn(new DefaultCharacteristic().setId(20).setName("Default Readability").setParentId(10));
 
     MockUserSession.set();
     WsTester.TestRequest request = tester.newRequest("show").setParam("key", issue.key());