SONAR-6147 Update message when resolution was empty

diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/IssueChangeNotification.java b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/IssueChangeNotification.java
index 2b7c0e659e68971b6976c86501972600eace396d..0e5cd899c91e6a10cb3409342fc3c1f451cff536 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/notification/IssueChangeNotification.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/notification/IssueChangeNotification.java
@@ -19,11 +19,13 @@
  */
 package org.sonar.server.issue.notification;
 
+import com.google.common.base.Strings;
 import org.sonar.api.component.Component;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.issue.internal.FieldDiffs;
 import org.sonar.api.notifications.Notification;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 
 import java.io.Serializable;
@@ -48,10 +50,8 @@ public class IssueChangeNotification extends Notification {
       for (Map.Entry<String, FieldDiffs.Diff> entry : currentChange.diffs().entrySet()) {
         String type = entry.getKey();
         FieldDiffs.Diff diff = entry.getValue();
-        Serializable newValue = diff.newValue();
-        Serializable oldValue = diff.oldValue();
-        setFieldValue("old." + type, oldValue != null ? oldValue.toString() : null);
-        setFieldValue("new." + type, newValue != null ? newValue.toString() : null);
+        setFieldValue("old." + type, neverEmptySerializableToString(diff.oldValue()));
+        setFieldValue("new." + type, neverEmptySerializableToString(diff.newValue()));
       }
     }
     return this;
@@ -88,4 +88,9 @@ public class IssueChangeNotification extends Notification {
     }
     return this;
   }
+
+  @CheckForNull
+  private static String neverEmptySerializableToString(@Nullable Serializable s) {
+    return s != null ? Strings.emptyToNull(s.toString()) : null;
+  }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangeNotificationTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangeNotificationTest.java
new file mode 100644 (file)
index 0000000..328e494
--- /dev/null
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangeNotificationTest.java
@@ -0,0 +1,120 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * SonarQube 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.
+ *
+ * SonarQube 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 this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.issue.notification;
+
+import org.junit.Test;
+import org.sonar.api.issue.internal.DefaultIssue;
+import org.sonar.api.issue.internal.FieldDiffs;
+import org.sonar.core.component.ComponentDto;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class IssueChangeNotificationTest {
+
+  IssueChangeNotification notification = new IssueChangeNotification();
+
+  @Test
+  public void set_issue() throws Exception {
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCD")
+      .setReporter("arthur")
+      .setAssignee("simon")
+      .setMessage("Remove this useless method")
+      .setComponentKey("MyService")
+      .setCurrentChange(new FieldDiffs().setDiff("resolution", "FALSE-POSITIVE", "FIXED"));
+
+    IssueChangeNotification result = notification.setIssue(issue);
+
+    assertThat(result.getFieldValue("key")).isEqualTo("ABCD");
+    assertThat(result.getFieldValue("reporter")).isEqualTo("arthur");
+    assertThat(result.getFieldValue("assignee")).isEqualTo("simon");
+    assertThat(result.getFieldValue("message")).isEqualTo("Remove this useless method");
+    assertThat(result.getFieldValue("componentKey")).isEqualTo("MyService");
+    assertThat(result.getFieldValue("old.resolution")).isEqualTo("FALSE-POSITIVE");
+    assertThat(result.getFieldValue("new.resolution")).isEqualTo("FIXED");
+  }
+
+  @Test
+  public void set_issue_with_current_change_having_no_old_value() throws Exception {
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCD")
+      .setReporter("arthur")
+      .setAssignee("simon")
+      .setMessage("Remove this useless method")
+      .setComponentKey("MyService");
+
+    IssueChangeNotification result = notification.setIssue(issue.setCurrentChange(new FieldDiffs().setDiff("resolution", null, "FIXED")));
+    assertThat(result.getFieldValue("old.resolution")).isNull();
+    assertThat(result.getFieldValue("new.resolution")).isEqualTo("FIXED");
+
+    result = notification.setIssue(issue.setCurrentChange(new FieldDiffs().setDiff("resolution", "", "FIXED")));
+    assertThat(result.getFieldValue("old.resolution")).isNull();
+    assertThat(result.getFieldValue("new.resolution")).isEqualTo("FIXED");
+  }
+
+  @Test
+  public void set_issue_with_current_change_having_no_new_value() throws Exception {
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("ABCD")
+      .setReporter("arthur")
+      .setAssignee("simon")
+      .setMessage("Remove this useless method")
+      .setComponentKey("MyService");
+
+    IssueChangeNotification result = notification.setIssue(issue.setCurrentChange(new FieldDiffs().setDiff("assignee", "john", null)));
+    assertThat(result.getFieldValue("old.assignee")).isEqualTo("john");
+    assertThat(result.getFieldValue("new.assignee")).isNull();
+
+    result = notification.setIssue(issue.setCurrentChange(new FieldDiffs().setDiff("assignee", "john", "")));
+    assertThat(result.getFieldValue("old.assignee")).isEqualTo("john");
+    assertThat(result.getFieldValue("new.assignee")).isNull();
+  }
+
+  @Test
+  public void set_project() throws Exception {
+    IssueChangeNotification result = notification.setProject(new ComponentDto().setKey("MyService").setLongName("My Service"));
+    assertThat(result.getFieldValue("projectKey")).isEqualTo("MyService");
+    assertThat(result.getFieldValue("projectName")).isEqualTo("My Service");
+  }
+
+  @Test
+  public void set_component() throws Exception {
+    IssueChangeNotification result = notification.setComponent(new ComponentDto().setKey("MyService").setLongName("My Service"));
+    assertThat(result.getFieldValue("componentName")).isEqualTo("My Service");
+  }
+
+  @Test
+  public void set_change_author_login() throws Exception {
+    IssueChangeNotification result = notification.setChangeAuthorLogin("stephane");
+    assertThat(result.getFieldValue("changeAuthor")).isEqualTo("stephane");
+  }
+
+  @Test
+  public void set_rule_name() throws Exception {
+    IssueChangeNotification result = notification.setRuleName("Xoo Rule");
+    assertThat(result.getFieldValue("ruleName")).isEqualTo("Xoo Rule");
+  }
+
+  @Test
+  public void setComment() throws Exception {
+    IssueChangeNotification result = notification.setComment("My comment");
+    assertThat(result.getFieldValue("comment")).isEqualTo("My comment");
+  }
+}

diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest.java
index f7023431f43e483abdf06f74707902ecbd122d07..3c82fb374dc1fd528e0ff49d5975780046368be4 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest.java
@@ -100,6 +100,26 @@ public class IssueChangesEmailTemplateTest {
     assertThat(email.getFrom()).isNull();
   }
 
+  @Test
+  public void email_should_display_resolution_change() throws Exception {
+    Notification notification = generateNotification()
+      .setFieldValue("old.resolution", "FALSE-POSITIVE")
+      .setFieldValue("new.resolution", "FIXED");
+
+    EmailMessage email = template.format(notification);
+    assertThat(email.getMessageId()).isEqualTo("issue-changes/ABCDE");
+    assertThat(email.getSubject()).isEqualTo("Struts, change on issue #ABCDE");
+
+    String message = email.getMessage();
+    String expected = Resources.toString(Resources.getResource(
+      "org/sonar/server/issue/notification/IssueChangesEmailTemplateTest/email_should_display_resolution_change.txt"),
+      Charsets.UTF_8
+      );
+    expected = StringUtils.remove(expected, '\r');
+    assertThat(message).isEqualTo(expected);
+    assertThat(email.getFrom()).isNull();
+  }
+
   @Test
   public void display_component_key_if_no_component_name() throws Exception {
     Notification notification = generateNotification()

diff --git a/server/sonar-server/src/test/resources/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest/email_should_display_resolution_change.txt b/server/sonar-server/src/test/resources/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest/email_should_display_resolution_change.txt
new file mode 100644 (file)
index 0000000..39a302e
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/issue/notification/IssueChangesEmailTemplateTest/email_should_display_resolution_change.txt
@@ -0,0 +1,7 @@
+Action
+Rule: Avoid Cycles
+Message: Has 3 cycles
+
+Resolution: FIXED (was FALSE-POSITIVE)
+
+See it in SonarQube: http://nemo.sonarsource.org/issues/search#issues=ABCDE