SONAR-11859 Fix issue changelog

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java

@@ -31,6 +31,7 @@ import org.sonar.core.issue.DefaultIssueComment;
 import org.sonar.core.issue.FieldDiffs;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.core.util.Uuids;
+import org.sonar.db.component.KeyType;
 import org.sonar.server.issue.IssueFieldsSetter;
 import org.sonar.server.issue.workflow.IssueWorkflow;
 
@@ -92,10 +93,11 @@ public class IssueLifecycle {
     raw.setFieldChange(changeContext, IssueFieldsSetter.FROM_LONG_BRANCH, fromLongBranchName, analysisMetadataHolder.getBranch().getName());
   }
 
-  public void mergeConfirmedOrResolvedFromShortLivingBranch(DefaultIssue raw, DefaultIssue base, String fromShortBranchName) {
+  public void mergeConfirmedOrResolvedFromShortLivingBranchOrPr(DefaultIssue raw, DefaultIssue base, KeyType branchType, String fromShortBranchNameOrPR) {
     copyAttributesOfIssueFromOtherBranch(raw, base);
-    raw.setFieldChange(changeContext, IssueFieldsSetter.FROM_SHORT_BRANCH, fromShortBranchName,
-      analysisMetadataHolder.isPullRequest() ? analysisMetadataHolder.getPullRequestKey() : analysisMetadataHolder.getBranch().getName());
+    String from = (branchType == KeyType.PULL_REQUEST ? "#" : "") + fromShortBranchNameOrPR;
+    String to = analysisMetadataHolder.isPullRequest() ? ("#" + analysisMetadataHolder.getPullRequestKey()) : analysisMetadataHolder.getBranch().getName();
+    raw.setFieldChange(changeContext, IssueFieldsSetter.FROM_SHORT_BRANCH, from, to);
   }
 
   private void copyAttributesOfIssueFromOtherBranch(DefaultIssue to, DefaultIssue from) {

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/SiblingIssue.java

@@ -25,6 +25,7 @@ import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.core.issue.tracking.Trackable;
+import org.sonar.db.component.KeyType;
 
 @Immutable
 public class SiblingIssue implements Trackable {
@@ -34,17 +35,20 @@ public class SiblingIssue implements Trackable {
   private final String lineHash;
   private final RuleKey ruleKey;
   private final String status;
-  private final String branchName;
+  private final String branchKey;
+  private final KeyType branchType;
   private final Date updateDate;
 
-  public SiblingIssue(String key, @Nullable Integer line, String message, @Nullable String lineHash, RuleKey ruleKey, String status, String branchName, Date updateDate) {
+  public SiblingIssue(String key, @Nullable Integer line, String message, @Nullable String lineHash, RuleKey ruleKey, String status, String branchKey, KeyType branchType,
+    Date updateDate) {
     this.key = key;
     this.line = line;
     this.message = message;
     this.lineHash = lineHash;
     this.ruleKey = ruleKey;
     this.status = status;
-    this.branchName = branchName;
+    this.branchKey = branchKey;
+    this.branchType = branchType;
     this.updateDate = updateDate;
   }
 
@@ -79,8 +83,12 @@ public class SiblingIssue implements Trackable {
     return status;
   }
 
-  public String getBranchName() {
-    return branchName;
+  public String getBranchKey() {
+    return branchKey;
+  }
+
+  public KeyType getBranchType() {
+    return branchType;
   }
 
   @Override

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/SiblingsIssueMerger.java

@@ -55,7 +55,7 @@ public class SiblingsIssueMerger {
 
     for (Map.Entry<DefaultIssue, SiblingIssue> e : matchedRaws.entrySet()) {
       SiblingIssue issue = e.getValue();
-      issueLifecycle.mergeConfirmedOrResolvedFromShortLivingBranch(e.getKey(), defaultIssues.get(issue), issue.getBranchName());
+      issueLifecycle.mergeConfirmedOrResolvedFromShortLivingBranchOrPr(e.getKey(), defaultIssues.get(issue), issue.getBranchType(), issue.getBranchKey());
     }
   }
 }

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/SiblingsIssuesLoader.java

@@ -67,7 +67,7 @@ public class SiblingsIssuesLoader {
   }
 
   private static SiblingIssue toSiblingIssue(ShortBranchIssueDto dto) {
-    return new SiblingIssue(dto.getKey(), dto.getLine(), dto.getMessage(), dto.getChecksum(), dto.getRuleKey(), dto.getStatus(), dto.getBranchName(),
+    return new SiblingIssue(dto.getKey(), dto.getLine(), dto.getMessage(), dto.getChecksum(), dto.getRuleKey(), dto.getStatus(), dto.getBranchKey(), dto.getKeyType(),
       longToDate(dto.getIssueUpdateDate()));
   }
 

server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java

@@ -32,6 +32,8 @@ import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.DefaultIssueComment;
 import org.sonar.core.issue.FieldDiffs;
 import org.sonar.core.issue.IssueChangeContext;
+import org.sonar.db.component.BranchType;
+import org.sonar.db.component.KeyType;
 import org.sonar.db.protobuf.DbCommons;
 import org.sonar.db.protobuf.DbIssues;
 import org.sonar.server.issue.IssueFieldsSetter;
@@ -117,7 +119,7 @@ public class IssueLifecycleTest {
   }
 
   @Test
-  public void mergeIssueFromShortLivingBranch() {
+  public void mergeIssueFromShortLivingBranchIntoLLB() {
     DefaultIssue raw = new DefaultIssue()
       .setKey("raw");
     DefaultIssue fromShort = new DefaultIssue()
@@ -151,7 +153,7 @@ public class IssueLifecycleTest {
     when(branch.getName()).thenReturn("master");
     analysisMetadataHolder.setBranch(branch);
 
-    underTest.mergeConfirmedOrResolvedFromShortLivingBranch(raw, fromShort, "feature/foo");
+    underTest.mergeConfirmedOrResolvedFromShortLivingBranchOrPr(raw, fromShort, KeyType.BRANCH, "feature/foo");
 
     assertThat(raw.resolution()).isEqualTo("resolution");
     assertThat(raw.status()).isEqualTo("status");
@@ -169,6 +171,113 @@ public class IssueLifecycleTest {
     assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_SHORT_BRANCH).newValue()).isEqualTo("master");
   }
 
+  @Test
+  public void mergeIssueFromShortLivingBranchIntoPR() {
+    DefaultIssue raw = new DefaultIssue()
+      .setKey("raw");
+    DefaultIssue fromShort = new DefaultIssue()
+      .setKey("short");
+    fromShort.setResolution("resolution");
+    fromShort.setStatus("status");
+
+    Date commentDate = new Date();
+    fromShort.addComment(new DefaultIssueComment()
+      .setIssueKey("short")
+      .setCreatedAt(commentDate)
+      .setUserUuid("user_uuid")
+      .setMarkdownText("A comment"));
+
+    Date diffDate = new Date();
+    // file diff alone
+    fromShort.addChange(new FieldDiffs()
+      .setCreationDate(diffDate)
+      .setIssueKey("short")
+      .setUserUuid("user_uuid")
+      .setDiff("file", "uuidA1", "uuidB1"));
+    // file diff with another field
+    fromShort.addChange(new FieldDiffs()
+      .setCreationDate(diffDate)
+      .setIssueKey("short")
+      .setUserUuid("user_uuid")
+      .setDiff("severity", "MINOR", "MAJOR")
+      .setDiff("file", "uuidA2", "uuidB2"));
+
+    Branch branch = mock(Branch.class);
+    when(branch.getType()).thenReturn(BranchType.PULL_REQUEST);
+    analysisMetadataHolder.setBranch(branch);
+    analysisMetadataHolder.setPullRequestKey("3");
+
+    underTest.mergeConfirmedOrResolvedFromShortLivingBranchOrPr(raw, fromShort, KeyType.BRANCH, "feature/foo");
+
+    assertThat(raw.resolution()).isEqualTo("resolution");
+    assertThat(raw.status()).isEqualTo("status");
+    assertThat(raw.defaultIssueComments())
+      .extracting(DefaultIssueComment::issueKey, DefaultIssueComment::createdAt, DefaultIssueComment::userUuid, DefaultIssueComment::markdownText)
+      .containsOnly(tuple("raw", commentDate, "user_uuid", "A comment"));
+    assertThat(raw.changes()).hasSize(2);
+    assertThat(raw.changes().get(0).creationDate()).isEqualTo(diffDate);
+    assertThat(raw.changes().get(0).userUuid()).isEqualTo("user_uuid");
+    assertThat(raw.changes().get(0).issueKey()).isEqualTo("raw");
+    assertThat(raw.changes().get(0).diffs()).containsOnlyKeys("severity");
+    assertThat(raw.changes().get(1).userUuid()).isEqualTo("default_user_uuid");
+    assertThat(raw.changes().get(1).diffs()).containsOnlyKeys(IssueFieldsSetter.FROM_SHORT_BRANCH);
+    assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_SHORT_BRANCH).oldValue()).isEqualTo("feature/foo");
+    assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_SHORT_BRANCH).newValue()).isEqualTo("#3");
+  }
+
+  @Test
+  public void mergeIssueFromPRIntoLLB() {
+    DefaultIssue raw = new DefaultIssue()
+      .setKey("raw");
+    DefaultIssue fromShort = new DefaultIssue()
+      .setKey("short");
+    fromShort.setResolution("resolution");
+    fromShort.setStatus("status");
+
+    Date commentDate = new Date();
+    fromShort.addComment(new DefaultIssueComment()
+      .setIssueKey("short")
+      .setCreatedAt(commentDate)
+      .setUserUuid("user_uuid")
+      .setMarkdownText("A comment"));
+
+    Date diffDate = new Date();
+    // file diff alone
+    fromShort.addChange(new FieldDiffs()
+      .setCreationDate(diffDate)
+      .setIssueKey("short")
+      .setUserUuid("user_uuid")
+      .setDiff("file", "uuidA1", "uuidB1"));
+    // file diff with another field
+    fromShort.addChange(new FieldDiffs()
+      .setCreationDate(diffDate)
+      .setIssueKey("short")
+      .setUserUuid("user_uuid")
+      .setDiff("severity", "MINOR", "MAJOR")
+      .setDiff("file", "uuidA2", "uuidB2"));
+
+    Branch branch = mock(Branch.class);
+    when(branch.getName()).thenReturn("master");
+    analysisMetadataHolder.setBranch(branch);
+
+    underTest.mergeConfirmedOrResolvedFromShortLivingBranchOrPr(raw, fromShort, KeyType.PULL_REQUEST, "1");
+
+    assertThat(raw.resolution()).isEqualTo("resolution");
+    assertThat(raw.status()).isEqualTo("status");
+    assertThat(raw.defaultIssueComments())
+      .extracting(DefaultIssueComment::issueKey, DefaultIssueComment::createdAt, DefaultIssueComment::userUuid, DefaultIssueComment::markdownText)
+      .containsOnly(tuple("raw", commentDate, "user_uuid", "A comment"));
+    assertThat(raw.changes()).hasSize(2);
+    assertThat(raw.changes().get(0).creationDate()).isEqualTo(diffDate);
+    assertThat(raw.changes().get(0).userUuid()).isEqualTo("user_uuid");
+    assertThat(raw.changes().get(0).issueKey()).isEqualTo("raw");
+    assertThat(raw.changes().get(0).diffs()).containsOnlyKeys("severity");
+    assertThat(raw.changes().get(1).userUuid()).isEqualTo("default_user_uuid");
+    assertThat(raw.changes().get(1).diffs()).containsOnlyKeys(IssueFieldsSetter.FROM_SHORT_BRANCH);
+    assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_SHORT_BRANCH).oldValue()).isEqualTo("#1");
+    assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_SHORT_BRANCH).newValue()).isEqualTo("master");
+  }
+
   @Test
   public void copiedIssue() {
     DefaultIssue raw = new DefaultIssue()

server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/SiblingsIssueMergerTest.java

@@ -45,6 +45,7 @@ import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
 import org.sonar.db.component.BranchType;
 import org.sonar.db.component.ComponentDto;
+import org.sonar.db.component.KeyType;
 import org.sonar.db.issue.IssueDto;
 import org.sonar.db.issue.IssueTesting;
 import org.sonar.db.rule.RuleDefinitionDto;
@@ -151,7 +152,7 @@ public class SiblingsIssueMergerTest {
     copier.tryMerge(FILE_1, Collections.singleton(newIssue));
 
     ArgumentCaptor<DefaultIssue> issueToMerge = ArgumentCaptor.forClass(DefaultIssue.class);
-    verify(issueLifecycle).mergeConfirmedOrResolvedFromShortLivingBranch(eq(newIssue), issueToMerge.capture(), eq("myBranch1"));
+    verify(issueLifecycle).mergeConfirmedOrResolvedFromShortLivingBranchOrPr(eq(newIssue), issueToMerge.capture(), eq(KeyType.BRANCH), eq("myBranch1"));
 
     assertThat(issueToMerge.getValue().key()).isEqualTo("issue1");
   }
@@ -170,7 +171,7 @@ public class SiblingsIssueMergerTest {
     copier.tryMerge(FILE_1, Collections.singleton(newIssue));
 
     ArgumentCaptor<DefaultIssue> issueToMerge = ArgumentCaptor.forClass(DefaultIssue.class);
-    verify(issueLifecycle).mergeConfirmedOrResolvedFromShortLivingBranch(eq(newIssue), issueToMerge.capture(), eq("myBranch1"));
+    verify(issueLifecycle).mergeConfirmedOrResolvedFromShortLivingBranchOrPr(eq(newIssue), issueToMerge.capture(), eq(KeyType.BRANCH), eq("myBranch1"));
 
     assertThat(issueToMerge.getValue().key()).isEqualTo("issue1");
   }
@@ -192,7 +193,7 @@ public class SiblingsIssueMergerTest {
     copier.tryMerge(FILE_1, Collections.singleton(newIssue));
 
     ArgumentCaptor<DefaultIssue> issueToMerge = ArgumentCaptor.forClass(DefaultIssue.class);
-    verify(issueLifecycle).mergeConfirmedOrResolvedFromShortLivingBranch(eq(newIssue), issueToMerge.capture(), eq("myBranch2"));
+    verify(issueLifecycle).mergeConfirmedOrResolvedFromShortLivingBranchOrPr(eq(newIssue), issueToMerge.capture(), eq(KeyType.BRANCH), eq("myBranch2"));
 
     assertThat(issueToMerge.getValue().key()).isEqualTo("issue2");
     assertThat(issueToMerge.getValue().defaultIssueComments()).isNotEmpty();

server/sonar-db-dao/src/main/java/org/sonar/db/issue/ShortBranchIssueDto.java

@@ -25,6 +25,7 @@ import javax.annotation.Nullable;
 import org.apache.commons.lang.builder.ToStringBuilder;
 import org.apache.commons.lang.builder.ToStringStyle;
 import org.sonar.api.rule.RuleKey;
+import org.sonar.db.component.KeyType;
 
 public final class ShortBranchIssueDto implements Serializable {
 
@@ -38,7 +39,8 @@ public final class ShortBranchIssueDto implements Serializable {
   // joins
   private String ruleKey;
   private String ruleRepo;
-  private String branchName;
+  private String branchKey;
+  private KeyType keyType;
 
   public String getKey() {
     return kee;
@@ -69,12 +71,24 @@ public final class ShortBranchIssueDto implements Serializable {
     return this;
   }
 
-  public String getBranchName() {
-    return branchName;
+  /**
+   * Branch name for SLB, PR key for PR
+   */
+  public String getBranchKey() {
+    return branchKey;
   }
 
-  public ShortBranchIssueDto setBranchName(String s) {
-    this.branchName = s;
+  public ShortBranchIssueDto setBranchKey(String s) {
+    this.branchKey = s;
+    return this;
+  }
+
+  public KeyType getKeyType() {
+    return keyType;
+  }
+
+  public ShortBranchIssueDto setKeyType(KeyType s) {
+    this.keyType = s;
     return this;
   }
 

server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml

@@ -293,7 +293,8 @@
       i.issue_update_date as issueUpdateDate,
       r.plugin_rule_key as ruleKey,
       r.plugin_name as ruleRepo,
-      b.kee as branchName
+      b.kee as branchKey,
+      b.key_type as keyType
     from issues i
     inner join rules r on r.id = i.rule_id
     inner join project_branches b on i.project_uuid = b.uuid

server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDaoTest.java

@@ -37,6 +37,7 @@ import org.sonar.db.RowNotFoundException;
 import org.sonar.db.component.BranchType;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.component.ComponentTesting;
+import org.sonar.db.component.KeyType;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.rule.RuleDefinitionDto;
 import org.sonar.db.rule.RuleDto;
@@ -234,7 +235,8 @@ public class IssueDaoTest {
     assertThat(fp.getChecksum()).isNotEmpty();
     assertThat(fp.getRuleKey()).isNotNull();
     assertThat(fp.getStatus()).isNotNull();
-    assertThat(fp.getBranchName()).isEqualTo("feature/foo");
+    assertThat(fp.getBranchKey()).isEqualTo("feature/foo");
+    assertThat(fp.getKeyType()).isEqualTo(KeyType.BRANCH);
     assertThat(fp.getIssueUpdateDate()).isNotNull();
   }
 

sonar-core/src/main/resources/org/sonar/l10n/core.properties

@@ -696,7 +696,7 @@ issue.changelog.changed_to={0} changed to {1}
 issue.changelog.was=was {0}
 issue.change.file_move=The file has been moved from {0} to {1}
 issue.change.from_long_branch=The issue has been copied from branch '{0}' to branch '{1}'
-issue.change.from_short_branch=The issue has been merged from branch '{0}' into branch '{1}'
+issue.change.from_short_branch=The issue has been merged from '{0}' into '{1}'
 issue.changelog.removed={0} removed
 issue.changelog.field.severity=Severity
 issue.changelog.field.actionPlan=Action Plan