SONAR-4283 Add notification on new comment, and change emails subject

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplate.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplate.java
index a24c2439b6d0d97fb0a2cfbc8780344efce8992b..7750c8947ba898f320d555166f1e36e8f8350128 100644 (file)
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplate.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplate.java
@@ -53,10 +53,12 @@ public class IssueChangesEmailTemplate extends EmailTemplate {
     String author = notif.getFieldValue("changeAuthor");
 
     StringBuilder sb = new StringBuilder();
-    appendField(sb, "Project", null, notif.getFieldValue("projectName"));
+    String projectName = notif.getFieldValue("projectName");
+    appendField(sb, "Project", null, projectName);
     appendField(sb, "Component", null, StringUtils.defaultString(notif.getFieldValue("componentName"), notif.getFieldValue("componentKey")));
     appendField(sb, "Rule", null, notif.getFieldValue("ruleName"));
     appendField(sb, "Message", null, notif.getFieldValue("message"));
+    appendField(sb, "Comment", null, notif.getFieldValue("comment"));
     sb.append('\n');
 
     appendField(sb, "Assignee", notif.getFieldValue("old.assignee"), notif.getFieldValue("new.assignee"));
@@ -69,7 +71,7 @@ public class IssueChangesEmailTemplate extends EmailTemplate {
 
     EmailMessage message = new EmailMessage()
       .setMessageId("issue-changes/" + issueKey)
-      .setSubject("Issue #" + issueKey)
+      .setSubject("Project "+ projectName +", change on issue #" + issueKey)
       .setMessage(sb.toString());
     if (author != null) {
       message.setFrom(getUserFullName(author));

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/NewIssuesEmailTemplate.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/NewIssuesEmailTemplate.java
index 9678c5d542fca00a37ac6999f2c8e6648b7bd2c1..1199da9ff11372dbf077025adf189663918bb06a 100644 (file)
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/NewIssuesEmailTemplate.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/notification/NewIssuesEmailTemplate.java
@@ -56,7 +56,7 @@ public class NewIssuesEmailTemplate extends EmailTemplate {
 
     EmailMessage message = new EmailMessage()
       .setMessageId("new-issues/" + notification.getFieldValue("projectKey"))
-      .setSubject("New issues for project " + projectName)
+      .setSubject("Project " + projectName + ", new issues")
       .setMessage(sb.toString());
 
     return message;

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 5f27346a6ae4f755652b76f7e29b57835a9285b8..72291d3a6be822ad27fde22fa96cf5546e7e3665 100644 (file)
--- 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
@@ -1591,6 +1591,7 @@ server_id_configuration.does_not_match_organisation_pattern=Organisation does no
 #
 #------------------------------------------------------------------------------
 notification.channel.EmailNotificationChannel=Email
+notification.dispatcher.information=Subscribe to following channels to be notified when the related events occur. A notification is never sent to the author of the event.
 notification.dispatcher.ChangesOnMyIssue=Changes in issues assigned to me or reported by me
 notification.dispatcher.NewIssues=New issues
 notification.dispatcher.NewAlerts=New alerts

diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest.java
index c84b80cf7890ec6c7addfa3257a7affeaa553c2e..a3452e482832c57e6161f509bc799e906c22b65c 100644 (file)
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/IssueChangesEmailTemplateTest.java
@@ -64,6 +64,7 @@ public class IssueChangesEmailTemplateTest {
       .setFieldValue("key", "ABCDE")
       .setFieldValue("ruleName", "Avoid Cycles")
       .setFieldValue("message", "Has 3 cycles")
+      .setFieldValue("comment", "How to fix it?")
       .setFieldValue("old.assignee", "simon")
       .setFieldValue("new.assignee", "louis")
       .setFieldValue("new.resolution", "FALSE-POSITIVE")
@@ -71,9 +72,10 @@ public class IssueChangesEmailTemplateTest {
 
     EmailMessage message = template.format(notification);
     assertThat(message.getMessageId()).isEqualTo("issue-changes/ABCDE");
-    assertThat(message.getSubject()).isEqualTo("Issue #ABCDE");
+    assertThat(message.getSubject()).isEqualTo("Project Struts, change on issue #ABCDE");
     assertThat(message.getMessage()).contains("Rule: Avoid Cycles");
     assertThat(message.getMessage()).contains("Message: Has 3 cycles");
+    assertThat(message.getMessage()).contains("Comment: How to fix it?");
     assertThat(message.getMessage()).contains("Assignee: louis (was simon)");
     assertThat(message.getMessage()).contains("Resolution: FALSE-POSITIVE");
     assertThat(message.getMessage()).contains("See it in SonarQube: http://nemo.sonarsource.org/issue/show/ABCDE");

diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/NewIssuesEmailTemplateTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/NewIssuesEmailTemplateTest.java
index 0f862b1325923d83cdc7c98205db5f072feec64d..af3cc7e4430b04a5a974d9f04cf3c79da24eeced 100644 (file)
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/NewIssuesEmailTemplateTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/notification/NewIssuesEmailTemplateTest.java
@@ -49,7 +49,7 @@ public class NewIssuesEmailTemplateTest {
 
   /**
    * <pre>
-   * Subject: New issues for project Foo
+   * Subject: Project Struts, new issues
    * From: Sonar
    *
    * Project: Foo
@@ -68,7 +68,7 @@ public class NewIssuesEmailTemplateTest {
 
     EmailMessage message = template.format(notification);
     assertThat(message.getMessageId()).isEqualTo("new-issues/org.apache:struts");
-    assertThat(message.getSubject()).isEqualTo("New issues for project Struts");
+    assertThat(message.getSubject()).isEqualTo("Project Struts, new issues");
     assertThat(message.getMessage()).isEqualTo("" +
       "Project: Struts\n" +
       "32 new issues\n" +

diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java
index 0fc2debfc0984990cc0a9b3b89ee6d02a5597ef0..b7cc1fc55044700944988f65c57fe9c2c681f0de 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueNotifications.java
@@ -32,6 +32,7 @@ import org.sonar.core.i18n.RuleI18nManager;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
+
 import java.util.Locale;
 import java.util.Map;
 
@@ -67,28 +68,46 @@ public class IssueNotifications implements BatchComponent, ServerComponent {
   @CheckForNull
   public Notification sendChanges(DefaultIssue issue, IssueChangeContext context, Rule rule, Component project, @Nullable Component component) {
     if (issue.diffs() != null) {
-      Notification notification = newNotification(project, "issue-changes");
-      notification.setFieldValue("key", issue.key());
-      notification.setFieldValue("changeAuthor", context.login());
-      notification.setFieldValue("reporter", issue.reporter());
-      notification.setFieldValue("assignee", issue.assignee());
-      notification.setFieldValue("message", issue.message());
-      notification.setFieldValue("ruleName", ruleName(rule));
-      notification.setFieldValue("componentKey", issue.componentKey());
-      if (component != null) {
-        notification.setFieldValue("componentName", component.name());
-      }
+      Notification notification = createNotification(issue, context, rule, project, component, null);
+      notificationsManager.scheduleForSending(notification);
+      return notification;
+    }
+    return null;
+  }
 
-      for (Map.Entry<String, FieldDiffs.Diff> entry : issue.diffs().diffs().entrySet()) {
+  @CheckForNull
+  public Notification sendChanges(DefaultIssue issue, IssueChangeContext context, IssueQueryResult queryResult, @Nullable String comment) {
+    Notification notification = createNotification(issue, context, queryResult.rule(issue), queryResult.project(issue), queryResult.component(issue), comment);
+    notificationsManager.scheduleForSending(notification);
+    return notification;
+  }
+
+  private Notification createNotification(DefaultIssue issue, IssueChangeContext context, Rule rule, Component project, @Nullable Component component, @Nullable String comment){
+    Notification notification = newNotification(project, "issue-changes");
+    notification.setFieldValue("key", issue.key());
+    notification.setFieldValue("changeAuthor", context.login());
+    notification.setFieldValue("reporter", issue.reporter());
+    notification.setFieldValue("assignee", issue.assignee());
+    notification.setFieldValue("message", issue.message());
+    notification.setFieldValue("ruleName", ruleName(rule));
+    notification.setFieldValue("componentKey", issue.componentKey());
+    if (component != null) {
+      notification.setFieldValue("componentName", component.name());
+    }
+    if (comment != null) {
+      notification.setFieldValue("comment", comment);
+    }
+
+    FieldDiffs diffs = issue.diffs();
+    if (diffs != null) {
+      for (Map.Entry<String, FieldDiffs.Diff> entry : diffs.diffs().entrySet()) {
         String type = entry.getKey();
         FieldDiffs.Diff diff = entry.getValue();
         notification.setFieldValue("old." + type, diff.oldValue() != null ? diff.oldValue().toString() : null);
         notification.setFieldValue("new." + type, diff.newValue() != null ? diff.newValue().toString() : null);
       }
-      notificationsManager.scheduleForSending(notification);
-      return notification;
     }
-    return null;
+    return notification;
   }
 
   private String ruleName(@Nullable Rule rule) {

diff --git a/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java b/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java
index 34789bb72ce2f0c7b37cf1fa1cbce5c201e033f3..af263e33b44de32a281a0853e84b25bbdb35aa1f 100644 (file)
--- a/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/IssueNotificationsTest.java
@@ -89,4 +89,24 @@ public class IssueNotificationsTest {
     assertThat(notification.getFieldValue("new.status")).isEqualTo("RESOLVED");
     Mockito.verify(manager).scheduleForSending(notification);
   }
+
+  @Test
+  public void sendChangesWithComment() throws Exception {
+    IssueChangeContext context = IssueChangeContext.createScan(new Date());
+    DefaultIssue issue = new DefaultIssue()
+      .setMessage("the message")
+      .setKey("ABCDE")
+      .setAssignee("freddy")
+      .setComponentKey("struts:Action")
+      .setProjectKey("struts");
+    DefaultIssueQueryResult queryResult = new DefaultIssueQueryResult(Arrays.<Issue>asList(issue));
+    queryResult.addProjects(Arrays.<Component>asList(new Project("struts")));
+
+    Notification notification = issueNotifications.sendChanges(issue, context, queryResult, "I don't know how to fix it?");
+
+    assertThat(notification.getFieldValue("message")).isEqualTo("the message");
+    assertThat(notification.getFieldValue("key")).isEqualTo("ABCDE");
+    assertThat(notification.getFieldValue("comment")).isEqualTo("I don't know how to fix it?");
+    Mockito.verify(manager).scheduleForSending(notification);
+  }
 }

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java
index 8a5180285144582d5d34e5419a827f7c80c90972..684817d0cffd8ecabd0abdbb798bbeedc33862bb 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java
@@ -23,16 +23,16 @@ import com.google.common.base.Objects;
 import com.google.common.base.Strings;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.issue.IssueComment;
+import org.sonar.api.issue.IssueQuery;
+import org.sonar.api.issue.IssueQueryResult;
 import org.sonar.api.web.UserRole;
-import org.sonar.core.issue.DefaultIssue;
-import org.sonar.core.issue.DefaultIssueComment;
-import org.sonar.core.issue.IssueChangeContext;
-import org.sonar.core.issue.IssueUpdater;
+import org.sonar.core.issue.*;
 import org.sonar.core.issue.db.IssueChangeDao;
 import org.sonar.core.issue.db.IssueChangeDto;
 import org.sonar.core.issue.db.IssueStorage;
 import org.sonar.server.user.UserSession;
 
+import java.util.Arrays;
 import java.util.Date;
 
 public class IssueCommentService implements ServerComponent {
@@ -41,12 +41,14 @@ public class IssueCommentService implements ServerComponent {
   private final IssueChangeDao changeDao;
   private final IssueStorage storage;
   private final DefaultIssueFinder finder;
+  private final IssueNotifications issueNotifications;
 
-  public IssueCommentService(IssueUpdater updater, IssueChangeDao changeDao, IssueStorage storage, DefaultIssueFinder finder) {
+  public IssueCommentService(IssueUpdater updater, IssueChangeDao changeDao, IssueStorage storage, DefaultIssueFinder finder, IssueNotifications issueNotifications) {
     this.updater = updater;
     this.changeDao = changeDao;
     this.storage = storage;
     this.finder = finder;
+    this.issueNotifications = issueNotifications;
   }
 
   public IssueComment findComment(String commentKey) {
@@ -55,11 +57,14 @@ public class IssueCommentService implements ServerComponent {
 
   public IssueComment addComment(String issueKey, String text, UserSession userSession) {
     verifyLoggedIn(userSession);
-    DefaultIssue issue = finder.findByKey(issueKey, UserRole.USER);
+
+    IssueQueryResult queryResult = loadIssue(issueKey);
+    DefaultIssue issue = (DefaultIssue) queryResult.first();
 
     IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.login());
     updater.addComment(issue, text, context);
     storage.save(issue);
+    issueNotifications.sendChanges(issue, context, queryResult, text);
     return issue.comments().get(issue.comments().size() - 1);
   }
 
@@ -109,4 +114,9 @@ public class IssueCommentService implements ServerComponent {
       throw new IllegalStateException("User is not logged in");
     }
   }
+
+  public IssueQueryResult loadIssue(String issueKey) {
+    IssueQuery query = IssueQuery.builder().issueKeys(Arrays.asList(issueKey)).requiredRole(UserRole.USER).build();
+    return finder.find(query);
+  }
 }

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/account/_global_notifications.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/account/_global_notifications.html.erb
index dbad8b052d3a8f613c453228888a0798a340a53e..8be9b58846705b1a9a73887ad9f80d097578c7be 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/account/_global_notifications.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/account/_global_notifications.html.erb
@@ -1,25 +1,27 @@
 <h2><%= message('my_profile.overall_notifications.title') -%></h2>
 
-    <table class="form">
-      <tr>
-        <td></td>
-        <% for channel in @channels %>
-          <td class="center"><b><%= message('notification.channel.' + channel.getKey()) -%></b></td>
-        <% end %>
-      </tr>
-      <% for dispatcher in @global_dispatchers %>
-        <tr>
-          <td><%= message('notification.dispatcher.' + dispatcher) -%></td>
-          <%
-               for channel in @channels
-                 notification_id = dispatcher + '.' + channel.getKey()
-                 check_box_id = 'global_notifs[' + notification_id + ']'
-                 check_box_checked = @global_notifications[notification_id]
-          %>
-          <td class="center">
-              <%= check_box_tag check_box_id, 'true', check_box_checked %>
-          </td>
-          <% end %>
-        </tr>
+<span><%= message('notification.dispatcher.information') -%></span>
+<br/><br/>
+<table class="form">
+  <tr>
+    <td></td>
+    <% for channel in @channels %>
+      <td class="center"><b><%= message('notification.channel.' + channel.getKey()) -%></b></td>
+    <% end %>
+  </tr>
+  <% for dispatcher in @global_dispatchers %>
+    <tr>
+      <td><%= message('notification.dispatcher.' + dispatcher) -%></td>
+      <%
+           for channel in @channels
+             notification_id = dispatcher + '.' + channel.getKey()
+             check_box_id = 'global_notifs[' + notification_id + ']'
+             check_box_checked = @global_notifications[notification_id]
+      %>
+      <td class="center">
+          <%= check_box_tag check_box_id, 'true', check_box_checked %>
+      </td>
       <% end %>
-    </table>
+    </tr>
+  <% end %>
+</table>