SONAR-2607 Provide email notifications on review changes

* Add email templates.

* Add server component - UserFinder.

20 files changed, 768 insertions, 195 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/CommentOnReviewEmailTemplate.java b/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/ChangesInReviewAssignedToMe.java
index 4f2d4c8ed6b..fc56412f31b 100644
--- 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/ChangesInReviewAssignedToMe.java
@@ -19,26 +19,30 @@
  */
 package org.sonar.plugins.email.reviews;
 
+import org.apache.commons.lang.StringUtils;
 import org.sonar.api.notifications.Notification;
-import org.sonar.plugins.email.api.EmailMessage;
-import org.sonar.plugins.email.api.EmailTemplate;
+import org.sonar.api.notifications.NotificationDispatcher;
 
-public class CommentOnReviewEmailTemplate extends EmailTemplate {
+/**
+ * This dispatcher means: "notify me when when someone changes review assigned to me".
+ * 
+ * @since 2.10
+ */
+public class ChangesInReviewAssignedToMe extends NotificationDispatcher {
 
   @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;
+  public void dispatch(Notification notification, Context context) {
+    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);
+      }
     }
-    return null;
   }
 
 }
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/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"));
+  }
+
+}
diff --git a/sonar-core/src/main/java/org/sonar/core/components/DefaultUserFinder.java b/sonar-core/src/main/java/org/sonar/core/components/DefaultUserFinder.java
new file mode 100644
index 00000000000..648aa104fb5
--- /dev/null
+++ b/sonar-core/src/main/java/org/sonar/core/components/DefaultUserFinder.java
@@ -0,0 +1,43 @@
+/*
+ * 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.core.components;
+
+import org.sonar.api.database.DatabaseSession;
+import org.sonar.api.database.model.User;
+import org.sonar.api.security.UserFinder;
+import org.sonar.jpa.session.DatabaseSessionFactory;
+
+/**
+ * @since 2.10
+ */
+public class DefaultUserFinder implements UserFinder {
+
+  private DatabaseSessionFactory sessionFactory;
+
+  public DefaultUserFinder(DatabaseSessionFactory sessionFactory) {
+    this.sessionFactory = sessionFactory;
+  }
+
+  public User findByLogin(String login) {
+    DatabaseSession session = sessionFactory.getSession();
+    return session.getSingleResult(User.class, "login", login);
+  }
+
+}
diff --git a/sonar-core/src/main/java/org/sonar/jpa/entity/NotificationQueueElement.java b/sonar-core/src/main/java/org/sonar/jpa/entity/NotificationQueueElement.java
index 198cd0871ce..ac80cec95e6 100644
--- a/sonar-core/src/main/java/org/sonar/jpa/entity/NotificationQueueElement.java
+++ b/sonar-core/src/main/java/org/sonar/jpa/entity/NotificationQueueElement.java
@@ -20,6 +20,7 @@
 package org.sonar.jpa.entity;
 
 import org.sonar.api.notifications.Notification;
+import org.sonar.api.utils.SonarException;
 
 import java.io.*;
 import java.util.Date;
@@ -65,7 +66,7 @@ public class NotificationQueueElement {
       objectOutputStream.close();
       this.data = byteArrayOutputStream.toByteArray();
     } catch (IOException e) {
-      throw new RuntimeException(e);
+      throw new SonarException(e);
     }
   }
 
@@ -80,9 +81,9 @@ public class NotificationQueueElement {
       objectInputStream.close();
       return (Notification) result;
     } catch (IOException e) {
-      throw new RuntimeException(e);
+      throw new SonarException(e);
     } catch (ClassNotFoundException e) {
-      throw new RuntimeException(e);
+      throw new SonarException(e);
     }
   }
 
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/notifications/Notification.java b/sonar-plugin-api/src/main/java/org/sonar/api/notifications/Notification.java
index 8ec915cb0d0..5547acae2bc 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/notifications/Notification.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/notifications/Notification.java
@@ -24,7 +24,7 @@ import org.apache.commons.lang.builder.ReflectionToStringBuilder;
 import org.apache.commons.lang.builder.ToStringStyle;
 
 import java.io.Serializable;
-import java.util.Map;
+import java.util.HashMap;
 
 /**
  * @since 2.10
@@ -33,7 +33,7 @@ public class Notification implements Serializable {
 
   private String type;
 
-  private Map<String, String> fields = Maps.newHashMap();
+  private HashMap<String, String> fields = Maps.newHashMap(); // NOSONAR false-positive due to serialization : usage of HashMap instead of Map
 
   public Notification(String type) {
     this.type = type;
diff --git a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewAssignedToMe.java b/sonar-plugin-api/src/main/java/org/sonar/api/security/UserFinder.java
index 95051533e11..3f283ff504e 100644
--- a/plugins/sonar-email-plugin/src/main/java/org/sonar/plugins/email/reviews/CommentOnReviewAssignedToMe.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/security/UserFinder.java
@@ -17,23 +17,16 @@
  * 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;
+package org.sonar.api.security;
 
-import org.sonar.api.notifications.Notification;
-import org.sonar.api.notifications.NotificationDispatcher;
+import org.sonar.api.ServerComponent;
+import org.sonar.api.database.model.User;
 
 /**
- * This dispatcher means: "notify me when when someone comments on review assigned to me". 
- * 
  * @since 2.10
  */
-public class CommentOnReviewAssignedToMe extends NotificationDispatcher {
+public interface UserFinder extends ServerComponent {
 
-  @Override
-  public void dispatch(Notification notification, Context context) {
-    if ("review".equals(notification.getType())) {
-      context.addUser(notification.getFieldValue("assignee"));
-    }
-  }
+  User findByLogin(String login);
 
 }
diff --git a/sonar-server/src/main/java/org/sonar/server/notifications/NotificationService.java b/sonar-server/src/main/java/org/sonar/server/notifications/NotificationService.java
index b88cf62146b..b408a05bfd7 100644
--- a/sonar-server/src/main/java/org/sonar/server/notifications/NotificationService.java
+++ b/sonar-server/src/main/java/org/sonar/server/notifications/NotificationService.java
@@ -136,9 +136,9 @@ public class NotificationService implements ServerComponent {
     }
     for (Map.Entry<String, Collection<NotificationChannel>> entry : recipients.asMap().entrySet()) {
       String username = entry.getKey();
-      Collection<NotificationChannel> channels = entry.getValue();
-      LOG.info("For user {} via {}", username, channels);
-      for (NotificationChannel channel : channels) {
+      Collection<NotificationChannel> userChannels = entry.getValue();
+      LOG.info("For user {} via {}", username, userChannels);
+      for (NotificationChannel channel : userChannels) {
         channel.deliver(notification, username);
       }
     }
diff --git a/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java b/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java
index e8200bd4b06..a89a5667acf 100644
--- a/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java
+++ b/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java
@@ -19,49 +19,69 @@
  */
 package org.sonar.server.notifications.reviews;
 
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.commons.lang.StringUtils;
 import org.sonar.api.ServerComponent;
-import org.sonar.api.database.model.Review;
-import org.sonar.api.database.model.User;
 import org.sonar.api.notifications.Notification;
 import org.sonar.api.notifications.NotificationManager;
-import org.sonar.jpa.session.DatabaseSessionFactory;
+
+import com.google.common.collect.Sets;
 
 /**
  * @since 2.10
  */
 public class ReviewsNotificationManager implements ServerComponent {
 
-  private DatabaseSessionFactory sessionFactory;
   private NotificationManager notificationManager;
 
-  public ReviewsNotificationManager(DatabaseSessionFactory sessionFactory, NotificationManager notificationManager) {
-    this.sessionFactory = sessionFactory;
+  public ReviewsNotificationManager(NotificationManager notificationManager) {
     this.notificationManager = notificationManager;
   }
 
   /**
-   * Visibility has been relaxed for tests.
+   * @param reviewId id of review, which was modified
+   * @param author author of change (username)
+   * @param creator author of review (username)
+   * @param assignee current assignee (username)
+   * @param oldComment old text of comment
+   * @param comment new text of comment
    */
-  User getUserById(Integer id) {
-    return sessionFactory.getSession().getEntity(User.class, id);
+  public void notifyCommentChanged(Long reviewId, String author, String creator, String assignee, String oldComment, String newComment) {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", String.valueOf(reviewId))
+        .setFieldValue("author", author)
+        .setFieldValue("creator", creator)
+        .setFieldValue("assignee", assignee)
+        .setFieldValue("old.comment", oldComment)
+        .setFieldValue("new.comment", newComment);
+    notificationManager.scheduleForSending(notification);
   }
 
   /**
-   * Visibility has been relaxed for tests.
+   * @param reviewId reviewId id of review, which was modified
+   * @param author author of change (username)
+   * @param oldValues map of old values
+   * @param newValues map of new values
    */
-  Review getReviewById(Long id) {
-    return sessionFactory.getSession().getEntity(Review.class, id);
-  }
-
-  public void notifyCommentAdded(Long reviewId, Integer userId, String comment) {
-    Review review = getReviewById(reviewId);
-    User author = getUserById(userId);
-
-    Notification notification = new Notification("review");
-    notification // FIXME include info about review
-        .setFieldValue("author", author.getLogin())
-        .setFieldValue("comment", comment);
-
+  public void notifyChanged(Long reviewId, String author, Map<String, String> oldValues, Map<String, String> newValues) {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", author)
+        .setFieldValue("author", author)
+        .setFieldValue("creator", newValues.get("creator"))
+        .setFieldValue("assignee", newValues.get("assignee"));
+    Set<String> fields = Sets.newHashSet();
+    fields.addAll(oldValues.keySet());
+    fields.addAll(newValues.keySet());
+    for (String field : fields) {
+      String oldValue = oldValues.get(field);
+      String newValue = newValues.get(field);
+      if ( !StringUtils.equals(oldValue, newValue)) {
+        notification.setFieldValue("new." + field, newValue);
+        notification.setFieldValue("old." + field, oldValue);
+      }
+    }
     notificationManager.scheduleForSending(notification);
   }
 
diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
index df234f0c490..7b5ef8fac7f 100644
--- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
+++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
@@ -40,6 +40,7 @@ import org.sonar.api.utils.TimeProfiler;
 import org.sonar.core.components.DefaultMetricFinder;
 import org.sonar.core.components.DefaultModelFinder;
 import org.sonar.core.components.DefaultRuleFinder;
+import org.sonar.core.components.DefaultUserFinder;
 import org.sonar.core.notifications.DefaultNotificationManager;
 import org.sonar.jpa.dao.DaoFacade;
 import org.sonar.jpa.dao.MeasuresDao;
@@ -185,6 +186,7 @@ public final class Platform {
     servicesContainer.as(Characteristics.CACHE).addComponent(ProfilesConsole.class);
     servicesContainer.as(Characteristics.CACHE).addComponent(RulesConsole.class);
     servicesContainer.as(Characteristics.CACHE).addComponent(JRubyI18n.class);
+    servicesContainer.as(Characteristics.CACHE).addComponent(DefaultUserFinder.class);
 
     // Notifications
     servicesContainer.as(Characteristics.CACHE).addComponent(NotificationService.class);
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb
index 94df7aed6d6..d4fe68c3808 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb
@@ -54,15 +54,14 @@ class Review < ActiveRecord::Base
   # REVIEW CORE METHODS
   #
   #
-  
+
   # params are mandatory:
   # - :user
   # - :text
   def create_comment(params={})
     comment = comments.create!(params)
     touch
-
-    Java::OrgSonarServerUi::JRubyFacade.getInstance().getReviewsNotificationManager().notifyCommentAdded(id.to_i, comment.user.id.to_i, comment.text.to_java)
+    # TODO notification_manager.notifyCommentChanged(id.to_i, current_user.login.to_java, self.user.login.to_java, self.assignee.login.to_java, nil, comment.text.to_java)
   end
 
   def edit_comment(comment_id, comment_text)
@@ -76,25 +75,61 @@ class Review < ActiveRecord::Base
 
   def edit_last_comment(comment_text)
     comment=comments.last
+    old_comment_text=comment.text
     comment.text=comment_text
     comment.save!
     touch
+    # TODO notification_manager.notifyCommentChanged(id.to_i, current_user.login.to_java, self.user.login.to_java, self.assignee.login.to_java, old_comment.to_java, comment.text.to_java)
   end
-  
+
   def delete_comment(comment_id)
     comment=comments.find(comment_id)
     comments.pop
     if comment
+      old_comment_text=comment.text
       comment.delete
       touch
+      # TODO notification_manager.notifyCommentChanged(id.to_i, current_user.login.to_java, self.user.login.to_java, self.assignee.login.to_java, old_comment_text.to_java, nil)
     end
   end
-  
+
+  def notification_manager
+    Java::OrgSonarServerUi::JRubyFacade.getInstance().getReviewsNotificationManager()
+  end
+
+  def to_java_map()
+    map = java.util.HashMap.new({
+      :creator => user.login.to_java,
+      :assignee => assignee == nil ? nil : assignee.login.to_java,
+      :status => status.to_java,
+      :resolution => resolution.to_java
+    })
+    map
+  end
+
   def reassign(user)
+    old = self.to_java_map
     self.assignee = user
     self.save!
+    # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
   end
-  
+
+  def reopen
+    old = self.to_java_map
+    self.status = STATUS_REOPENED
+    self.resolution = nil
+    self.save!
+    # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
+  end
+
+  def resolve
+    old = self.to_java_map
+    self.status = STATUS_RESOLVED
+    self.resolution = 'FIXED'
+    self.save!
+    # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
+  end
+
   # params are mandatory:
   # - :user (mandatory)
   # - :text (mandatory)
@@ -112,13 +147,15 @@ class Review < ActiveRecord::Base
         end
       end
       create_comment(:user => params[:user], :text => params[:text])
+      old = self.to_java_map
       self.assignee = nil
       self.status = is_false_positive ? STATUS_RESOLVED : STATUS_REOPENED
       self.resolution = is_false_positive ? 'FALSE-POSITIVE' : nil
       self.save!
+      # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
     end
   end
-  
+
   def false_positive
     resolution == 'FALSE-POSITIVE'
   end
@@ -143,20 +180,6 @@ class Review < ActiveRecord::Base
     status == STATUS_OPEN
   end
   
-  def reopen
-    self.status = STATUS_REOPENED
-    self.resolution = nil
-    self.save!
-  end
-  
-  def resolve
-    self.status = STATUS_RESOLVED
-    self.resolution = 'FIXED'
-    self.save!
-  end
-  
-  
-  
   #
   #
   # SEARCH METHODS
diff --git a/sonar-server/src/test/java/org/sonar/server/notifications/reviews/ReviewsNotificationManagerTest.java b/sonar-server/src/test/java/org/sonar/server/notifications/reviews/ReviewsNotificationManagerTest.java
index 8e0eaba6953..187d3e965b3 100644
--- a/sonar-server/src/test/java/org/sonar/server/notifications/reviews/ReviewsNotificationManagerTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/notifications/reviews/ReviewsNotificationManagerTest.java
@@ -19,39 +19,31 @@
  */
 package org.sonar.server.notifications.reviews;
 
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertThat;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
 
 import org.junit.Before;
-import org.junit.Test;
-import org.sonar.api.database.model.Review;
-import org.sonar.api.database.model.User;
-import org.sonar.jpa.test.AbstractDbUnitTestCase;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.sonar.api.notifications.Notification;
+import org.sonar.api.notifications.NotificationManager;
 
-public class ReviewsNotificationManagerTest extends AbstractDbUnitTestCase {
+public class ReviewsNotificationManagerTest { // FIXME implement
 
+  private Notification notification;
   private ReviewsNotificationManager manager;
 
   @Before
   public void setUp() {
-    setupData(getClass().getResourceAsStream("fixture.xml"));
-    manager = new ReviewsNotificationManager(getSessionFactory(), null);
-  }
-
-  @Test
-  public void shouldGetReviewById() {
-    Review review = manager.getReviewById(3L);
-    assertThat(review.getUserId(), is(1));
-    assertThat(review.getAssigneeId(), is(2));
-    assertThat(review.getTitle(), is("Review #3"));
-  }
-
-  @Test
-  public void shouldGetUserById() {
-    User user = manager.getUserById(1);
-    assertThat(user.getLogin(), is("simon"));
-    assertThat(user.getName(), is("Simon Brandhof"));
-    assertThat(user.getEmail(), is("simon.brandhof@sonarsource.com"));
+    NotificationManager delegate = mock(NotificationManager.class);
+    doAnswer(new Answer() {
+      public Object answer(InvocationOnMock invocation) throws Throwable {
+        notification = (Notification) invocation.getArguments()[0];
+        return null;
+      }
+    }).when(delegate).scheduleForSending(any(Notification.class));
+    manager = new ReviewsNotificationManager(delegate);
   }
 
 }
diff --git a/sonar-server/src/test/resources/org/sonar/server/notifications/reviews/fixture.xml b/sonar-server/src/test/resources/org/sonar/server/notifications/reviews/fixture.xml
deleted file mode 100644
index d9a64d90337..00000000000
--- a/sonar-server/src/test/resources/org/sonar/server/notifications/reviews/fixture.xml
+++ /dev/null
@@ -1,15 +0,0 @@
-<dataset>
-
-  <users id="1" login="simon" name="Simon Brandhof" email="simon.brandhof@sonarsource.com" />
-  <users id="2" login="godin" name="Evgeny Mandrikov" email="evgeny.mandrikov@sonarsource.com" />
-
-  <!-- Review created by Simon and assigned to him -->
-  <reviews id="1" user_id="1" assignee_id="1" title="Review #1" />
-  
-  <!-- Review created by Evgeny and not assigned -->
-  <reviews id="2" user_id="2" assignee_id="[null]" title="Review #2" />
-  
-  <!-- Review created by Simon and assigned to Evgeny -->
-  <reviews id="3" user_id="1" assignee_id="2" title="Review #3" />
-
-</dataset>
\ No newline at end of file