SONAR-2607 Provide email notifications on review changes

* Add email templates.

* Add server component - UserFinder.

11 files changed, 599 insertions, 124 deletions

diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailConfiguration.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailConfiguration.java
index a2c5a458169..63b5f43cf19 100644
--- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailConfiguration.java
+++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailConfiguration.java
@@ -20,6 +20,7 @@
 package org.sonar.plugins.email;
 
 import org.apache.commons.configuration.Configuration;
+import org.sonar.api.CoreProperties;
 import org.sonar.api.ServerExtension;
 
 /**
@@ -78,4 +79,8 @@ public class EmailConfiguration implements ServerExtension {
     return configuration.getString(PREFIX, PREFIX_DEFAULT);
   }
 
+  public String getServerBaseURL() {
+    return configuration.getString(CoreProperties.SERVER_BASE_URL, CoreProperties.SERVER_BASE_URL_DEFAULT_VALUE);
+  }
+
 }
diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailNotificationChannel.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailNotificationChannel.java
index cb31280efdb..4e5c2eed82f 100644
--- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailNotificationChannel.java
+++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailNotificationChannel.java
@@ -19,16 +19,18 @@
  */
 package org.sonar.plugins.email;
 
+import java.net.MalformedURLException;
+import java.net.URL;
+
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.mail.EmailException;
 import org.apache.commons.mail.SimpleEmail;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.sonar.api.database.DatabaseSession;
 import org.sonar.api.database.model.User;
-import org.sonar.api.notifications.NotificationChannel;
 import org.sonar.api.notifications.Notification;
-import org.sonar.jpa.session.DatabaseSessionFactory;
+import org.sonar.api.notifications.NotificationChannel;
+import org.sonar.api.security.UserFinder;
 import org.sonar.plugins.email.api.EmailMessage;
 import org.sonar.plugins.email.api.EmailTemplate;
 
@@ -52,28 +54,53 @@ public class EmailNotificationChannel extends NotificationChannel {
    */
   private static final int SOCKET_TIMEOUT = 30000;
 
+  /**
+   * Email Header Field: "List-ID".
+   * Value of this field should contain mailing list identifier as specified in <a href="http://tools.ietf.org/html/rfc2919">RFC 2919</a>.
+   */
+  private static String LIST_ID_HEADER = "List-ID";
+
+  /**
+   * Email Header Field: "List-Archive".
+   * Value of this field should contain URL of mailing list archive as specified in <a href="http://tools.ietf.org/html/rfc2369">RFC 2369</a>.
+   */
+  private static String LIST_ARCHIVE_HEADER = "List-Archive";
+
+  /**
+   * Email Header Field: "In-Reply-To".
+   * Value of this field should contain related message identifier as specified in <a href="http://tools.ietf.org/html/rfc2822">RFC 2822</a>.
+   */
+  private static String IN_REPLY_TO_HEADER = "In-Reply-To";
+
+  /**
+   * Email Header Field: "References".
+   * Value of this field should contain related message identifier as specified in <a href="http://tools.ietf.org/html/rfc2822">RFC 2822</a>
+   */
+  private static String REFERENCES_HEADER = "References";
+
   private static final String FROM_NAME_DEFAULT = "Sonar";
   private static final String SUBJECT_DEFAULT = "Notification";
 
   private EmailConfiguration configuration;
   private EmailTemplate[] templates;
-  private DatabaseSessionFactory sessionFactory;
+  private UserFinder userFinder;
 
-  public EmailNotificationChannel(EmailConfiguration configuration, EmailTemplate[] templates, DatabaseSessionFactory sessionFactory) {
+  public EmailNotificationChannel(EmailConfiguration configuration, EmailTemplate[] templates, UserFinder userFinder) {
     this.configuration = configuration;
     this.templates = templates;
-    this.sessionFactory = sessionFactory;
-  }
-
-  private User getUserByLogin(String login) {
-    DatabaseSession session = sessionFactory.getSession();
-    return session.getSingleResult(User.class, "login", login);
+    this.userFinder = userFinder;
   }
 
   @Override
   public void deliver(Notification notification, String username) {
+    User user = userFinder.findByLogin(username);
+    if (StringUtils.isBlank(user.getEmail())) {
+      LOG.warn("Email not defined for user: " + username);
+      return;
+    }
     EmailMessage emailMessage = format(notification, username);
     if (emailMessage != null) {
+      emailMessage.setTo(user.getEmail());
       deliver(emailMessage);
     }
   }
@@ -82,11 +109,10 @@ public class EmailNotificationChannel extends NotificationChannel {
     for (EmailTemplate template : templates) {
       EmailMessage email = template.format(notification);
       if (email != null) {
-        User user = getUserByLogin(username);
-        email.setTo(user.getEmail());
         return email;
       }
     }
+    LOG.warn("Email template not found for notification: {}", notification);
     return null;
   }
 
@@ -107,30 +133,33 @@ public class EmailNotificationChannel extends NotificationChannel {
 
   private void send(EmailMessage emailMessage) throws EmailException {
     LOG.info("Sending email: {}", emailMessage);
-    // TODO
-    String domain = "nemo.sonarsource.org";
-    String listId = "<sonar." + domain + ">";
-    String serverUrl = "http://nemo.sonarsource.org";
+    String host = null;
+    try {
+      host = new URL(configuration.getServerBaseURL()).getHost();
+    } catch (MalformedURLException e) {
+      // ignore
+    }
 
     SimpleEmail email = new SimpleEmail();
-    /*
-     * Set headers for proper threading:
-     * GMail will not group messages, even if they have same subject, but don't have "In-Reply-To" and "References" headers.
-     * TODO investigate threading in other clients like KMail, Thunderbird, Outlook
-     */
-    if (StringUtils.isNotEmpty(emailMessage.getMessageId())) {
-      String messageId = "<" + emailMessage.getMessageId() + "@" + domain + ">";
-      email.addHeader("In-Reply-To", messageId);
-      email.addHeader("References", messageId);
+    if (StringUtils.isNotBlank(host)) {
+      /*
+       * Set headers for proper threading: GMail will not group messages, even if they have same subject, but don't have "In-Reply-To" and
+       * "References" headers. TODO investigate threading in other clients like KMail, Thunderbird, Outlook
+       */
+      if (StringUtils.isNotEmpty(emailMessage.getMessageId())) {
+        String messageId = "<" + emailMessage.getMessageId() + "@" + host + ">";
+        email.addHeader(IN_REPLY_TO_HEADER, messageId);
+        email.addHeader(REFERENCES_HEADER, messageId);
+      }
+      // Set headers for proper filtering
+      email.addHeader(LIST_ID_HEADER, "Sonar <sonar." + host + ">");
+      email.addHeader(LIST_ARCHIVE_HEADER, configuration.getServerBaseURL());
     }
-    // Set headers for proper filtering
-    email.addHeader("List-Id", listId);
-    email.addHeader("List-Archive", serverUrl);
     // Set general information
     email.setFrom(configuration.getFrom(), StringUtils.defaultIfBlank(emailMessage.getFrom(), FROM_NAME_DEFAULT));
     email.addTo(emailMessage.getTo(), " ");
     String subject = StringUtils.defaultIfBlank(StringUtils.trimToEmpty(configuration.getPrefix()) + " ", "")
-          + StringUtils.defaultString(emailMessage.getSubject(), SUBJECT_DEFAULT);
+        + StringUtils.defaultString(emailMessage.getSubject(), SUBJECT_DEFAULT);
     email.setSubject(subject);
     email.setMsg(emailMessage.getMessage());
     // Send
diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailPlugin.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailPlugin.java
index 4da910bb9cc..edee6ea8044 100644
--- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailPlugin.java
+++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/EmailPlugin.java
@@ -20,9 +20,9 @@
 package org.sonar.plugins.email;
 
 import org.sonar.api.SonarPlugin;
-import org.sonar.plugins.email.reviews.CommentOnReviewAssignedToMe;
-import org.sonar.plugins.email.reviews.CommentOnReviewCreatedByMe;
-import org.sonar.plugins.email.reviews.CommentOnReviewEmailTemplate;
+import org.sonar.plugins.email.reviews.ChangesInReviewAssignedToMe;
+import org.sonar.plugins.email.reviews.ChangesInReviewCreatedByMe;
+import org.sonar.plugins.email.reviews.ReviewEmailTemplate;
 
 import java.util.Arrays;
 import java.util.List;
@@ -34,9 +34,9 @@ public class EmailPlugin extends SonarPlugin {
         EmailConfiguration.class,
         EmailNotificationChannel.class,
 
-        CommentOnReviewEmailTemplate.class,
-        CommentOnReviewCreatedByMe.class,
-        CommentOnReviewAssignedToMe.class);
+        ReviewEmailTemplate.class,
+        ChangesInReviewAssignedToMe.class,
+        ChangesInReviewCreatedByMe.class);
   }
 
 }
diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewAssignedToMe.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMe.java
index 95051533e11..fc56412f31b 100644
--- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewAssignedToMe.java
+++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMe.java
@@ -19,20 +19,29 @@
  */
 package org.sonar.plugins.email.reviews;
 
+import org.apache.commons.lang.StringUtils;
 import org.sonar.api.notifications.Notification;
 import org.sonar.api.notifications.NotificationDispatcher;
 
 /**
- * This dispatcher means: "notify me when when someone comments on review assigned to me". 
+ * This dispatcher means: "notify me when when someone changes review assigned to me".
  * 
  * @since 2.10
  */
-public class CommentOnReviewAssignedToMe extends NotificationDispatcher {
+public class ChangesInReviewAssignedToMe extends NotificationDispatcher {
 
   @Override
   public void dispatch(Notification notification, Context context) {
-    if ("review".equals(notification.getType())) {
-      context.addUser(notification.getFieldValue("assignee"));
+    if (StringUtils.startsWith(notification.getType(), "review")) {
+      String author = notification.getFieldValue("author"); // author of change
+      String oldAssignee = notification.getFieldValue("old.assignee"); // previous assignee
+      String assignee = notification.getFieldValue("assignee"); // current assignee
+      if (!StringUtils.equals(author, oldAssignee)) {
+        context.addUser(oldAssignee);
+      }
+      if (!StringUtils.equals(author, assignee)) {
+        context.addUser(assignee);
+      }
     }
   }
 
diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewCreatedByMe.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewCreatedByMe.java
index c91ea5f49be..30c99df3ad2 100644
--- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewCreatedByMe.java
+++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewCreatedByMe.java
@@ -19,20 +19,25 @@
  */
 package org.sonar.plugins.email.reviews;
 
+import org.apache.commons.lang.StringUtils;
 import org.sonar.api.notifications.Notification;
 import org.sonar.api.notifications.NotificationDispatcher;
 
 /**
- * This dispatcher means: "notify me when when someone comments on review created by me".
+ * This dispatcher means: "notify me when when someone changes review created by me".
  * 
  * @since 2.10
  */
-public class CommentOnReviewCreatedByMe extends NotificationDispatcher {
+public class ChangesInReviewCreatedByMe extends NotificationDispatcher {
 
   @Override
   public void dispatch(Notification notification, Context context) {
-    if ("review".equals(notification.getType())) {
-      context.addUser(notification.getFieldValue("creator"));
+    if (StringUtils.startsWith(notification.getType(), "review")) {
+      String author = notification.getFieldValue("author"); // author of change
+      String creator = notification.getFieldValue("creator"); // creator of review
+      if (!StringUtils.equals(author, creator)) {
+        context.addUser(creator);
+      }
     }
   }
 
diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewEmailTemplate.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewEmailTemplate.java
deleted file mode 100644
index 4f2d4c8ed6b..00000000000
--- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewEmailTemplate.java
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * Sonar, open source software quality management tool.
- * Copyright (C) 2008-2011 SonarSource
- * mailto:contact AT sonarsource DOT com
- *
- * Sonar 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.
- *
- * Sonar 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 Sonar; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
- */
-package org.sonar.plugins.email.reviews;
-
-import org.sonar.api.notifications.Notification;
-import org.sonar.plugins.email.api.EmailMessage;
-import org.sonar.plugins.email.api.EmailTemplate;
-
-public class CommentOnReviewEmailTemplate extends EmailTemplate {
-
-  @Override
-  public EmailMessage format(Notification notification) {
-    if ("review".equals(notification.getType())) {
-      String reviewId = notification.getFieldValue("reviewId");
-      String author = notification.getFieldValue("author");
-      String comment = notification.getFieldValue("comment");
-      EmailMessage email = new EmailMessage()
-          .setFrom(author)
-          .setMessageId("review/" + reviewId)
-          .setSubject("Review #" + reviewId)
-          .setMessage(comment);
-      return email;
-    }
-    return null;
-  }
-
-}
diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ReviewEmailTemplate.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ReviewEmailTemplate.java
new file mode 100644
index 00000000000..b91d00a90cf
--- /dev/null
+++ b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ReviewEmailTemplate.java
@@ -0,0 +1,117 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2011 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar 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.
+ *
+ * Sonar 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 Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package org.sonar.plugins.email.reviews;
+
+import org.apache.commons.lang.StringUtils;
+import org.sonar.api.database.model.User;
+import org.sonar.api.notifications.Notification;
+import org.sonar.api.security.UserFinder;
+import org.sonar.plugins.email.EmailConfiguration;
+import org.sonar.plugins.email.api.EmailMessage;
+import org.sonar.plugins.email.api.EmailTemplate;
+
+/**
+ * Creates email message for notification "review-changed".
+ * 
+ * @since 2.10
+ */
+public class ReviewEmailTemplate extends EmailTemplate {
+
+  private EmailConfiguration configuration;
+  private UserFinder userFinder;
+
+  public ReviewEmailTemplate(EmailConfiguration configuration, UserFinder userFinder) {
+    this.configuration = configuration;
+    this.userFinder = userFinder;
+  }
+
+  @Override
+  public EmailMessage format(Notification notification) {
+    if ( !"review-changed".equals(notification.getType())) {
+      return null;
+    }
+    String reviewId = notification.getFieldValue("reviewId");
+    String author = notification.getFieldValue("author");
+    StringBuilder sb = new StringBuilder();
+
+    append(sb, "Status", notification.getFieldValue("old.status"), notification.getFieldValue("new.status"));
+    append(sb, "Resolution", notification.getFieldValue("old.resolution"), notification.getFieldValue("new.resolution"));
+    append(sb, "Assignee", getUserFullName(notification.getFieldValue("old.assignee")), getUserFullName(notification.getFieldValue("new.assignee")));
+    appendComment(sb, notification);
+    appendFooter(sb, notification);
+
+    EmailMessage message = new EmailMessage()
+        .setMessageId("review/" + reviewId)
+        .setSubject("Review #" + reviewId)
+        .setMessage(sb.toString());
+    if (author != null) {
+      message.setFrom(getUserFullName(author));
+    }
+    return message;
+  }
+
+  private void append(StringBuilder sb, String name, String oldValue, String newValue) {
+    if (oldValue != null || newValue != null) {
+      sb.append(name).append(": ");
+      if (newValue != null) {
+        sb.append(newValue);
+      }
+      if (oldValue != null) {
+        sb.append(" (was ").append(oldValue).append(")");
+      }
+      sb.append('\n');
+    }
+  }
+
+  private void appendComment(StringBuilder sb, Notification notification) {
+    String newComment = notification.getFieldValue("new.comment");
+    String oldComment = notification.getFieldValue("old.comment");
+
+    if (newComment != null) { // comment was added or modified
+      sb.append("Comment:\n  ").append(newComment).append('\n');
+      if (oldComment != null) { // comment was modified
+        sb.append("Was:\n  ").append(oldComment).append('\n');
+      }
+    } else if (oldComment != null) { // comment was deleted
+      sb.append("Comment deleted, was:\n  ").append(oldComment).append('\n');
+    }
+  }
+
+  private void appendFooter(StringBuilder sb, Notification notification) {
+    String reviewId = notification.getFieldValue("reviewId");
+    sb.append("\n--\n")
+        .append("See it in Sonar: ").append(configuration.getServerBaseURL()).append("/review/").append(reviewId).append('\n');
+  }
+
+  /**
+   * Visibility has been relaxed for tests.
+   */
+  String getUserFullName(String login) {
+    if (login == null) {
+      return null;
+    }
+    User user = userFinder.findByLogin(login);
+    if (user == null) { // most probably user was deleted
+      return login;
+    }
+    return StringUtils.defaultIfBlank(user.getName(), login);
+  }
+
+}
diff --git a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/EmailNotificationChannelTest.java b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/EmailNotificationChannelTest.java
index a33b50e43ec..1508dc676f8 100644
--- a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/EmailNotificationChannelTest.java
+++ b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/EmailNotificationChannelTest.java
@@ -25,8 +25,9 @@ import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-import com.dumbster.smtp.SimpleSmtpServer;
-import com.dumbster.smtp.SmtpMessage;
+import java.io.IOException;
+import java.net.ServerSocket;
+
 import org.apache.commons.mail.EmailException;
 import org.junit.After;
 import org.junit.Before;
@@ -34,8 +35,8 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 import org.sonar.plugins.email.api.EmailMessage;
 
-import java.io.IOException;
-import java.net.ServerSocket;
+import com.dumbster.smtp.SimpleSmtpServer;
+import com.dumbster.smtp.SmtpMessage;
 
 public class EmailNotificationChannelTest {
 
@@ -125,7 +126,7 @@ public class EmailNotificationChannelTest {
     assertThat(email.getHeaderValue("In-Reply-To"), is("<reviews/view/1@nemo.sonarsource.org>"));
     assertThat(email.getHeaderValue("References"), is("<reviews/view/1@nemo.sonarsource.org>"));
 
-    assertThat(email.getHeaderValue("List-Id"), is("<sonar.nemo.sonarsource.org>"));
+    assertThat(email.getHeaderValue("List-ID"), is("Sonar <sonar.nemo.sonarsource.org>"));
     assertThat(email.getHeaderValue("List-Archive"), is("http://nemo.sonarsource.org"));
 
     assertThat(email.getHeaderValue("From"), is("Full Username <server@nowhere>"));
@@ -149,7 +150,7 @@ public class EmailNotificationChannelTest {
     assertThat(email.getHeaderValue("In-Reply-To"), nullValue());
     assertThat(email.getHeaderValue("References"), nullValue());
 
-    assertThat(email.getHeaderValue("List-Id"), is("<sonar.nemo.sonarsource.org>"));
+    assertThat(email.getHeaderValue("List-ID"), is("Sonar <sonar.nemo.sonarsource.org>"));
     assertThat(email.getHeaderValue("List-Archive"), is("http://nemo.sonarsource.org"));
 
     assertThat(email.getHeaderValue("From"), is("Sonar <server@nowhere>"));
@@ -175,6 +176,7 @@ public class EmailNotificationChannelTest {
     when(configuration.getSmtpPort()).thenReturn(Integer.toString(port));
     when(configuration.getFrom()).thenReturn("server@nowhere");
     when(configuration.getPrefix()).thenReturn("[SONAR]");
+    when(configuration.getServerBaseURL()).thenReturn("http://nemo.sonarsource.org");
   }
 
 }
diff --git a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/review/CommentOnReviewAssignedToMeTest.java b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMeTest.java
index 2c31c1f98bd..d32af196241 100644
--- a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/review/CommentOnReviewAssignedToMeTest.java
+++ b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMeTest.java
@@ -17,42 +17,53 @@
  * License along with Sonar; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
  */
-package org.sonar.plugins.email.review;
+package org.sonar.plugins.email.reviews;
 
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 import org.junit.Before;
 import org.junit.Test;
+import org.sonar.api.notifications.Notification;
 import org.sonar.api.notifications.NotificationDispatcher;
-import org.sonar.plugins.email.reviews.CommentOnReviewAssignedToMe;
 
-public class CommentOnReviewAssignedToMeTest { // FIXME implement me
+public class ChangesInReviewAssignedToMeTest {
 
   private NotificationDispatcher.Context context;
-  private CommentOnReviewAssignedToMe dispatcher;
+  private ChangesInReviewAssignedToMe dispatcher;
 
   @Before
   public void setUp() {
     context = mock(NotificationDispatcher.Context.class);
-    dispatcher = new CommentOnReviewAssignedToMe();
+    dispatcher = new ChangesInReviewAssignedToMe();
   }
 
   @Test
-  public void shouldDispatchToAssignee() {
-    // CommentOnReviewNotification notification = new CommentOnReviewNotification(new Review().setAssigneeId(1), new User(), "comment");
-    // dispatcher.dispatch(notification, context);
-    // verify(context).addUser(1);
-    //
-    // notification = new CommentOnReviewNotification(new Review().setAssigneeId(2), new User(), "comment");
-    // dispatcher.dispatch(notification, context);
-    // verify(context).addUser(2);
+  public void dispatchToOldAndNewAssignee() {
+    Notification notification = new Notification("review-assignee-changed")
+        .setFieldValue("author", "freddy")
+        .setFieldValue("old.assignee", "godin")
+        .setFieldValue("assignee", "simon");
+
+    dispatcher.dispatch(notification, context);
+
+    verify(context).addUser("godin");
+    verify(context).addUser("simon");
+    verifyNoMoreInteractions(context);
   }
 
   @Test
-  public void shouldNotDispatchWhenNotAssigned() {
-    // CommentOnReviewNotification notification = new CommentOnReviewNotification(new Review(), new User(), "comment");
-    // dispatcher.dispatch(notification, context);
-    // verifyNoMoreInteractions(context);
+  public void doNotDispatchToAuthorOfChanges() {
+    Notification notification = new Notification("review-assignee-changed")
+        .setFieldValue("author", "simon")
+        .setFieldValue("old.assignee", "simon")
+        .setFieldValue("assignee", "godin");
+
+    dispatcher.dispatch(notification, context);
+
+    verify(context).addUser("godin");
+    verifyNoMoreInteractions(context);
   }
 
 }
diff --git a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/review/CommentOnReviewCreatedByMeTest.java b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ChangesInReviewCreatedByMeTest.java
index 2d345126418..6b0ea61fcda 100644
--- a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/review/CommentOnReviewCreatedByMeTest.java
+++ b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ChangesInReviewCreatedByMeTest.java
@@ -17,35 +17,49 @@
  * License along with Sonar; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
  */
-package org.sonar.plugins.email.review;
+package org.sonar.plugins.email.reviews;
 
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 import org.junit.Before;
 import org.junit.Test;
+import org.sonar.api.notifications.Notification;
 import org.sonar.api.notifications.NotificationDispatcher;
-import org.sonar.plugins.email.reviews.CommentOnReviewCreatedByMe;
 
-public class CommentOnReviewCreatedByMeTest { // FIXME implement me
+public class ChangesInReviewCreatedByMeTest {
 
   private NotificationDispatcher.Context context;
-  private CommentOnReviewCreatedByMe dispatcher;
+  private ChangesInReviewCreatedByMe dispatcher;
 
   @Before
   public void setUp() {
     context = mock(NotificationDispatcher.Context.class);
-    dispatcher = new CommentOnReviewCreatedByMe();
+    dispatcher = new ChangesInReviewCreatedByMe();
   }
 
   @Test
-  public void shouldDispatchToCreator() {
-    // CommentOnReviewNotification notification = new CommentOnReviewNotification(new Review().setUserId(1), new User(), "comment");
-    // dispatcher.dispatch(notification, context);
-    // verify(context).addUser(1);
-    //
-    // notification = new CommentOnReviewNotification(new Review().setUserId(2), new User(), "comment");
-    // dispatcher.dispatch(notification, context);
-    // verify(context).addUser(2);
+  public void dispatchToCreator() {
+    Notification notification = new Notification("review-comment-added")
+        .setFieldValue("author", "godin")
+        .setFieldValue("creator", "simon");
+
+    dispatcher.dispatch(notification, context);
+
+    verify(context).addUser("simon");
+    verifyNoMoreInteractions(context);
+  }
+
+  @Test
+  public void doNotDispatchToAuthorOfChanges() {
+    Notification notification = new Notification("review-comment-added")
+        .setFieldValue("author", "simon")
+        .setFieldValue("creator", "simon");
+
+    dispatcher.dispatch(notification, context);
+
+    verifyNoMoreInteractions(context);
   }
 
 }
diff --git a/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ReviewEmailTemplateTest.java b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ReviewEmailTemplateTest.java
new file mode 100644
index 00000000000..3b65b418f60
--- /dev/null
+++ b/plugins/sonar-email-plugin/src/test/java/org/sonar/plugins/email/reviews/ReviewEmailTemplateTest.java
@@ -0,0 +1,327 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2011 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar 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.
+ *
+ * Sonar 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 Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package org.sonar.plugins.email.reviews;
+
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.sonar.api.database.model.User;
+import org.sonar.api.notifications.Notification;
+import org.sonar.api.security.UserFinder;
+import org.sonar.plugins.email.EmailConfiguration;
+import org.sonar.plugins.email.api.EmailMessage;
+
+public class ReviewEmailTemplateTest {
+
+  private ReviewEmailTemplate template;
+
+  @Before
+  public void setUp() {
+    EmailConfiguration configuration = mock(EmailConfiguration.class);
+    when(configuration.getServerBaseURL()).thenReturn("http://nemo.sonarsource.org");
+    UserFinder userFinder = mock(UserFinder.class);
+    when(userFinder.findByLogin(eq("freddy.mallet"))).thenReturn(new User().setName("Freddy Mallet"));
+    when(userFinder.findByLogin(eq("simon.brandhof"))).thenReturn(new User().setName("Simon Brandhof"));
+    when(userFinder.findByLogin(eq("evgeny.mandrikov"))).thenReturn(new User().setName("Evgeny Mandrikov"));
+    template = new ReviewEmailTemplate(configuration, userFinder);
+  }
+
+  /**
+   * <pre>
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Comment:
+   *   This is my first comment
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * </pre>
+   */
+  @Test
+  public void shouldFormatCommentAdded() {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", "1")
+        .setFieldValue("author", "freddy.mallet")
+        .setFieldValue("old.comment", null)
+        .setFieldValue("new.comment", "This is my first comment");
+    EmailMessage message = template.format(notification);
+    assertThat(message.getMessageId(), is("review/1"));
+    assertThat(message.getSubject(), is("Review #1"));
+    assertThat(message.getFrom(), is("Freddy Mallet"));
+    assertThat(message.getMessage(), is("Comment:\n  This is my first comment\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n"));
+  }
+
+  /**
+   * <pre>
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Comment:
+   *   This is another comment
+   * Was:
+   *   This is my first comment
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * </pre>
+   */
+  @Test
+  public void shouldFormatCommentEdited() {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", "1")
+        .setFieldValue("author", "freddy.mallet")
+        .setFieldValue("old.comment", "This is my first comment")
+        .setFieldValue("new.comment", "This is another comment");
+    EmailMessage message = template.format(notification);
+    assertThat(message.getMessageId(), is("review/1"));
+    assertThat(message.getSubject(), is("Review #1"));
+    assertThat(message.getFrom(), is("Freddy Mallet"));
+    assertThat(message.getMessage(), is("Comment:\n  This is another comment\nWas:\n  This is my first comment\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n"));
+  }
+
+  /**
+   * <pre>
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Comment deleted, was:
+   *   This is deleted comment
+   *   
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * </pre>
+   */
+  @Test
+  public void shouldFormatCommentDeleted() {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", "1")
+        .setFieldValue("old.comment", "This is deleted comment")
+        .setFieldValue("new.comment", null)
+        .setFieldValue("author", "freddy.mallet");
+    EmailMessage message = template.format(notification);
+    assertThat(message.getMessageId(), is("review/1"));
+    assertThat(message.getSubject(), is("Review #1"));
+    assertThat(message.getFrom(), is("Freddy Mallet"));
+    assertThat(message.getMessage(), is("Comment deleted, was:\n  This is deleted comment\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n"));
+  }
+
+  /**
+   * <pre>
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Assignee: Evgeny Mandrikov
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * </pre>
+   */
+  @Test
+  public void shouldFormatAssigneed() {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", "1")
+        .setFieldValue("author", "freddy.mallet")
+        .setFieldValue("old.assignee", null)
+        .setFieldValue("new.assignee", "evgeny.mandrikov");
+    EmailMessage message = template.format(notification);
+    assertThat(message.getMessageId(), is("review/1"));
+    assertThat(message.getSubject(), is("Review #1"));
+    assertThat(message.getFrom(), is("Freddy Mallet"));
+    assertThat(message.getMessage(), is("Assignee: Evgeny Mandrikov\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n"));
+  }
+
+  /**
+   * <pre>
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Assignee: Simon Brandhof (was Evgeny Mandrikov)
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * </pre>
+   */
+  @Test
+  public void shouldFormatAssigneedToAnotherPerson() {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", "1")
+        .setFieldValue("author", "freddy.mallet")
+        .setFieldValue("old.assignee", "evgeny.mandrikov")
+        .setFieldValue("new.assignee", "simon.brandhof");
+    EmailMessage message = template.format(notification);
+    assertThat(message.getMessageId(), is("review/1"));
+    assertThat(message.getSubject(), is("Review #1"));
+    assertThat(message.getFrom(), is("Freddy Mallet"));
+    assertThat(message.getMessage(), is("Assignee: Simon Brandhof (was Evgeny Mandrikov)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n"));
+  }
+
+  /**
+   * <pre>
+   * Subject: Review #1
+   * From: Freddy Mallet
+   * 
+   * Assignee: (was Simon Brandhof)
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * </pre>
+   */
+  @Test
+  public void shouldFormatUnassigned() {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", "1")
+        .setFieldValue("author", "freddy.mallet")
+        .setFieldValue("old.assignee", "simon.brandhof")
+        .setFieldValue("new.assignee", null);
+    EmailMessage message = template.format(notification);
+    assertThat(message.getMessageId(), is("review/1"));
+    assertThat(message.getSubject(), is("Review #1"));
+    assertThat(message.getFrom(), is("Freddy Mallet"));
+    assertThat(message.getMessage(), is("Assignee:  (was Simon Brandhof)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n"));
+  }
+
+  /**
+   * <pre>
+   * Subject: Review #1
+   * From: Sonar
+   * 
+   * Status: CLOSED (was OPEN)
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * </pre>
+   */
+  @Test
+  public void shouldFormatClosed() {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", "1")
+        .setFieldValue("old.status", "OPEN")
+        .setFieldValue("new.status", "CLOSED");
+    EmailMessage message = template.format(notification);
+    assertThat(message.getMessageId(), is("review/1"));
+    assertThat(message.getSubject(), is("Review #1"));
+    assertThat(message.getFrom(), nullValue());
+    assertThat(message.getMessage(), is("Status: CLOSED (was OPEN)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n"));
+  }
+
+  /**
+   * <pre>
+   * Subject: Review #1
+   * From: Simon Brandhof
+   * 
+   * Status: REOPENED (was RESOLVED)
+   * Resolution: (was FIXED)
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * </pre>
+   */
+  @Test
+  public void shouldFormatReopened() {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", "1")
+        .setFieldValue("old.resolution", "FIXED")
+        .setFieldValue("new.resolution", null)
+        .setFieldValue("old.status", "RESOLVED")
+        .setFieldValue("new.status", "REOPENED");
+    EmailMessage message = template.format(notification);
+    assertThat(message.getMessageId(), is("review/1"));
+    assertThat(message.getSubject(), is("Review #1"));
+    assertThat(message.getFrom(), nullValue());
+    assertThat(message.getMessage(), is("Status: REOPENED (was RESOLVED)\nResolution:  (was FIXED)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n"));
+  }
+
+  /**
+   * <pre>
+   * Subject: Review #1
+   * From: Simon Brandhof
+   * 
+   * Status: RESOLVED (was OPEN)
+   * Resolution: FIXED
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * </pre>
+   */
+  @Test
+  public void shouldFormatResolvedAsFixed() {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", "1")
+        .setFieldValue("author", "simon.brandhof")
+        .setFieldValue("old.status", "OPEN")
+        .setFieldValue("old.resolution", null)
+        .setFieldValue("new.status", "RESOLVED")
+        .setFieldValue("new.resolution", "FIXED");
+    EmailMessage message = template.format(notification);
+    assertThat(message.getMessageId(), is("review/1"));
+    assertThat(message.getSubject(), is("Review #1"));
+    assertThat(message.getFrom(), is("Simon Brandhof"));
+    assertThat(message.getMessage(), is("Status: RESOLVED (was OPEN)\nResolution: FIXED\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n"));
+  }
+
+  /**
+   * <pre>
+   * Subject: Review #1
+   * From: Simon Brandhof
+   * 
+   * Status: RESOLVED (was REOPENED)
+   * Resolution: FALSE-POSITIVE
+   * 
+   * --
+   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * </pre>
+   */
+  @Test
+  public void shouldFormatResolvedAsFalsePositive() {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", "1")
+        .setFieldValue("author", "freddy.mallet")
+        .setFieldValue("old.status", "REOPENED")
+        .setFieldValue("old.resolution", null)
+        .setFieldValue("new.status", "RESOLVED")
+        .setFieldValue("new.resolution", "FALSE-POSITIVE");
+    EmailMessage message = template.format(notification);
+    assertThat(message.getMessageId(), is("review/1"));
+    assertThat(message.getSubject(), is("Review #1"));
+    assertThat(message.getFrom(), is("Freddy Mallet"));
+    assertThat(message.getMessage(), is("Status: RESOLVED (was REOPENED)\nResolution: FALSE-POSITIVE\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/1\n"));
+  }
+
+  @Test
+  public void shouldNotFormat() {
+    Notification notification = new Notification("other");
+    EmailMessage message = template.format(notification);
+    assertThat(message, nullValue());
+  }
+
+  @Test
+  public void shouldReturnFullNameOrLogin() {
+    assertThat(template.getUserFullName("freddy.mallet"), is("Freddy Mallet"));
+    assertThat(template.getUserFullName("deleted"), is("deleted"));
+  }
+
+}