SONAR-2607 Provide email notifications on review changes

* Update CloseReviewsDecorator to send notifications

* Fix permalink in ReviewEmailTemplate

8 files changed, 223 insertions, 85 deletions

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java
index 9bc35bb5f0f..437db024b2a 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java
@@ -19,7 +19,7 @@
  */
 package org.sonar.plugins.core.sensors;
 
-import javax.persistence.Query;
+import java.util.List;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -28,10 +28,15 @@ import org.sonar.api.batch.DecoratorBarriers;
 import org.sonar.api.batch.DecoratorContext;
 import org.sonar.api.batch.DependsUpon;
 import org.sonar.api.database.DatabaseSession;
+import org.sonar.api.database.model.Review;
 import org.sonar.api.database.model.Snapshot;
+import org.sonar.api.database.model.User;
+import org.sonar.api.notifications.Notification;
+import org.sonar.api.notifications.NotificationManager;
 import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
 import org.sonar.api.resources.ResourceUtils;
+import org.sonar.api.security.UserFinder;
 import org.sonar.batch.index.ResourcePersister;
 import org.sonar.core.NotDryRun;
 
@@ -46,10 +51,14 @@ public class CloseReviewsDecorator implements Decorator {
 
   private ResourcePersister resourcePersister;
   private DatabaseSession databaseSession;
+  private NotificationManager notificationManager;
+  private UserFinder userFinder;
 
-  public CloseReviewsDecorator(ResourcePersister resourcePersister, DatabaseSession databaseSession) {
+  public CloseReviewsDecorator(ResourcePersister resourcePersister, DatabaseSession databaseSession, NotificationManager notificationManager, UserFinder userFinder) {
     this.resourcePersister = resourcePersister;
     this.databaseSession = databaseSession;
+    this.notificationManager = notificationManager;
+    this.userFinder = userFinder;
   }
 
   public boolean shouldExecuteOnProject(Project project) {
@@ -62,43 +71,99 @@ public class CloseReviewsDecorator implements Decorator {
       int resourceId = currentSnapshot.getResourceId();
       int snapshotId = currentSnapshot.getId();
 
-      // Close reviews for which violations have been fixed
-      Query query = databaseSession.createNativeQuery(generateUpdateToCloseReviewsForFixedViolation(resourceId, snapshotId));
-      int rowUpdated = query.executeUpdate();
-      LOG.debug("- {} reviews set to 'closed' on resource #{}", rowUpdated, resourceId);
-
-      // Reopen reviews that had been set to resolved but for which the violation is still here
-      query = databaseSession.createNativeQuery(generateUpdateToReopenResolvedReviewsForNonFixedViolation(resourceId));
-      rowUpdated = query.executeUpdate();
-      LOG.debug("- {} reviews set to 'reopened' on resource #{}", rowUpdated, resourceId);
-
-      // And close reviews that relate to resources that have been deleted or renamed
+      closeReviews(resourceId, snapshotId);
+      reopenReviews(resourceId);
       if (ResourceUtils.isRootProject(resource)) {
-        query = databaseSession.createNativeQuery(generateUpdateToCloseReviewsForDeletedResources(resourceId, currentSnapshot.getId()));
-        query.setParameter(1, Boolean.TRUE);
-        rowUpdated = query.executeUpdate();
-        LOG.debug("- {} reviews set to 'closed' on project #{}", rowUpdated, resourceId);
+        closeReviewsForDeletedResources(resourceId, currentSnapshot.getId());
       }
 
       databaseSession.commit();
     }
   }
 
-  protected String generateUpdateToCloseReviewsForFixedViolation(int resourceId, int snapshotId) {
-    return "UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP WHERE status!='CLOSED' AND resource_id = " + resourceId
-        + " AND rule_failure_permanent_id NOT IN " + "(SELECT permanent_id FROM rule_failures WHERE snapshot_id = " + snapshotId
+  /**
+   * Close reviews for which violations have been fixed.
+   */
+  protected int closeReviews(int resourceId, int snapshotId) {
+    String conditions = " WHERE status!='CLOSED' AND resource_id=" + resourceId
+        + " AND rule_failure_permanent_id NOT IN " + "(SELECT permanent_id FROM rule_failures WHERE snapshot_id=" + snapshotId
         + " AND permanent_id IS NOT NULL)";
+    List<Review> reviews = databaseSession.getEntityManager().createNativeQuery("SELECT * FROM reviews " + conditions, Review.class).getResultList();
+    for (Review review : reviews) {
+      notifyClosed(review);
+    }
+    int rowUpdated = databaseSession.createNativeQuery("UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP" + conditions).executeUpdate();
+    LOG.debug("- {} reviews set to 'closed' on resource #{}", rowUpdated, resourceId);
+    return rowUpdated;
   }
 
-  protected String generateUpdateToReopenResolvedReviewsForNonFixedViolation(int resourceId) {
-    return "UPDATE reviews SET status='REOPENED', resolution=NULL, updated_at=CURRENT_TIMESTAMP WHERE status='RESOLVED' AND resource_id = " + resourceId;
+  /**
+   * Reopen reviews that had been set to resolved but for which the violation is still here.
+   */
+  protected int reopenReviews(int resourceId) {
+    String conditions = " WHERE status='RESOLVED' AND resource_id=" + resourceId;
+    List<Review> reviews = databaseSession.getEntityManager().createNativeQuery("SELECT * FROM reviews " + conditions, Review.class).getResultList();
+    for (Review review : reviews) {
+      notifyReopened(review);
+    }
+    int rowUpdated = databaseSession.createNativeQuery("UPDATE reviews SET status='REOPENED', resolution=NULL, updated_at=CURRENT_TIMESTAMP" + conditions).executeUpdate();
+    LOG.debug("- {} reviews set to 'reopened' on resource #{}", rowUpdated, resourceId);
+    return rowUpdated;
   }
 
-  protected String generateUpdateToCloseReviewsForDeletedResources(int projectId, int projectSnapshotId) {
-    return "UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP WHERE status!='CLOSED' AND project_id=" + projectId
+  /**
+   * Close reviews that relate to resources that have been deleted or renamed.
+   */
+  protected int closeReviewsForDeletedResources(int projectId, int projectSnapshotId) {
+    String conditions = " WHERE status!='CLOSED' AND project_id=" + projectId
         + " AND resource_id IN ( SELECT prev.project_id FROM snapshots prev  WHERE prev.root_project_id=" + projectId
-        + " AND prev.islast=? AND NOT EXISTS ( SELECT cur.id FROM snapshots cur WHERE cur.root_snapshot_id=" + projectSnapshotId
+        + " AND prev.islast=TRUE AND NOT EXISTS ( SELECT cur.id FROM snapshots cur WHERE cur.root_snapshot_id=" + projectSnapshotId
         + " AND cur.created_at > prev.created_at AND cur.root_project_id=" + projectId + " AND cur.project_id=prev.project_id ) )";
+    List<Review> reviews = databaseSession.getEntityManager().createNativeQuery("SELECT * FROM reviews " + conditions, Review.class).getResultList();
+    for (Review review : reviews) {
+      notifyClosed(review);
+    }
+    int rowUpdated = databaseSession.createNativeQuery("UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP" + conditions).executeUpdate();
+    LOG.debug("- {} reviews set to 'closed' on project #{}", rowUpdated, projectSnapshotId);
+    return rowUpdated;
+  }
+
+  private String getCreator(Review review) {
+    if (review.getUserId() == null) { // no creator and in fact this should never happen in real-life, however happens during unit tests
+      return null;
+    }
+    User user = userFinder.findById(review.getUserId());
+    return user != null ? user.getLogin() : null;
+  }
+
+  private String getAssignee(Review review) {
+    if (review.getAssigneeId() == null) { // not assigned
+      return null;
+    }
+    User user = userFinder.findById(review.getAssigneeId());
+    return user != null ? user.getLogin() : null;
+  }
+
+  void notifyReopened(Review review) {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", String.valueOf(review.getId()))
+        .setFieldValue("creator", getCreator(review))
+        .setFieldValue("assignee", getAssignee(review))
+        .setFieldValue("old.status", review.getStatus())
+        .setFieldValue("new.status", "REOPENED")
+        .setFieldValue("old.resolution", review.getResolution())
+        .setFieldValue("new.resolution", null);
+    notificationManager.scheduleForSending(notification);
+  }
+
+  void notifyClosed(Review review) {
+    Notification notification = new Notification("review-changed")
+        .setFieldValue("reviewId", String.valueOf(review.getId()))
+        .setFieldValue("creator", getCreator(review))
+        .setFieldValue("assignee", getAssignee(review))
+        .setFieldValue("old.status", review.getStatus())
+        .setFieldValue("new.status", "CLOSED");
+    notificationManager.scheduleForSending(notification);
   }
 
 }
diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java
index 17ff7a09293..19c04f675c8 100644
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java
@@ -23,24 +23,36 @@ import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
-
-import java.sql.Statement;
-
 import junit.framework.ComparisonFailure;
 
+import org.junit.Before;
 import org.junit.Test;
+import org.sonar.api.notifications.Notification;
+import org.sonar.api.notifications.NotificationManager;
 import org.sonar.api.resources.Project;
-import org.sonar.test.persistence.DatabaseTestCase;
+import org.sonar.api.security.UserFinder;
+import org.sonar.jpa.test.AbstractDbUnitTestCase;
 
-public class CloseReviewsDecoratorTest extends DatabaseTestCase {
+public class CloseReviewsDecoratorTest extends AbstractDbUnitTestCase {
+
+  private NotificationManager notificationManager;
+  private CloseReviewsDecorator reviewsDecorator;
+
+  @Before
+  public void setUp() {
+    notificationManager = mock(NotificationManager.class);
+    reviewsDecorator = new CloseReviewsDecorator(null, getSession(), notificationManager, mock(UserFinder.class));
+  }
 
   @Test
   public void testShouldExecuteOnProject() throws Exception {
     Project project = mock(Project.class);
     when(project.isLatestAnalysis()).thenReturn(true);
-    CloseReviewsDecorator reviewsDecorator = new CloseReviewsDecorator(null, null);
     assertTrue(reviewsDecorator.shouldExecuteOnProject(project));
   }
 
@@ -48,17 +60,14 @@ public class CloseReviewsDecoratorTest extends DatabaseTestCase {
   public void shouldCloseReviewWithoutCorrespondingViolation() throws Exception {
     setupData("fixture");
 
-    CloseReviewsDecorator reviewsDecorator = new CloseReviewsDecorator(null, null);
-    String sqlRequest = reviewsDecorator.generateUpdateToCloseReviewsForFixedViolation(666, 222);
-
-    Statement stmt = getConnection().createStatement();
-    int count = stmt.executeUpdate(sqlRequest);
+    int count = reviewsDecorator.closeReviews(666, 222);
 
     assertThat(count, is(3));
-    assertTables("shouldCloseReviewWithoutCorrespondingViolation", new String[] { "reviews" }, new String[] { "updated_at" });
+    verify(notificationManager, times(3)).scheduleForSending(any(Notification.class));
+    checkTables("shouldCloseReviewWithoutCorrespondingViolation", new String[] { "updated_at" }, new String[] { "reviews" });
 
     try {
-      assertTables("shouldCloseReviewWithoutCorrespondingViolation", new String[] { "reviews" });
+      checkTables("shouldCloseReviewWithoutCorrespondingViolation", new String[] { "reviews" });
       fail("'updated_at' columns are identical whereas they should be different.");
     } catch (ComparisonFailure e) {
       // "updated_at" column must be different, so the comparison should raise this exception
@@ -69,21 +78,18 @@ public class CloseReviewsDecoratorTest extends DatabaseTestCase {
   public void shouldReopenResolvedReviewWithNonFixedViolation() throws Exception {
     setupData("fixture");
 
-    CloseReviewsDecorator reviewsDecorator = new CloseReviewsDecorator(null, null);
-
     // First we close the reviews for which the violations have been fixed (this is because we use the same "fixture"...)
-    getConnection().createStatement().executeUpdate(reviewsDecorator.generateUpdateToCloseReviewsForFixedViolation(666, 222));
+    reviewsDecorator.closeReviews(666, 222);
 
     // And now we reopen the reviews that still have a violation
-    String sqlRequest = reviewsDecorator.generateUpdateToReopenResolvedReviewsForNonFixedViolation(666);
-    Statement stmt = getConnection().createStatement();
-    int count = stmt.executeUpdate(sqlRequest);
+    int count = reviewsDecorator.reopenReviews(666);
 
     assertThat(count, is(1));
-    assertTables("shouldReopenResolvedReviewWithNonFixedViolation", new String[] { "reviews" }, new String[] { "updated_at" });
+    verify(notificationManager, times(4)).scheduleForSending(any(Notification.class));
+    checkTables("shouldReopenResolvedReviewWithNonFixedViolation", new String[] { "updated_at" }, new String[] { "reviews" });
 
     try {
-      assertTables("shouldReopenResolvedReviewWithNonFixedViolation", new String[] { "reviews" });
+      checkTables("shouldReopenResolvedReviewWithNonFixedViolation", new String[] { "reviews" });
       fail("'updated_at' columns are identical whereas they should be different.");
     } catch (ComparisonFailure e) {
       // "updated_at" column must be different, so the comparison should raise this exception
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml
index b72092a662a..73e8ebbaab9 100644
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml
@@ -51,13 +51,23 @@
              rule_id="1" failure_level="1"/>
 
   <!-- Existing reviews -->
-  <!-- Note that DbUnit uses the first tag for a table to define the columns to be populated. So that's why "resolution" column here. -->
+  <!--
+  Note that DbUnit uses the first tag for a table to define the columns to be populated.
+  So that's why "resolution", "created_at", "updated_at", "project_id", "resource_line", "severity" and "user_id" columns here.
+  --> 
   <reviews
   			id="1"
   			status="OPEN"
   			rule_failure_permanent_id="1"
   			resolution="[null]"
+            created_at="[null]"
+            updated_at="[null]"
+            project_id="[null]"
+            resource_line="[null]"
+            severity="[null]"
+            user_id="[null]"
   			resource_id="555"/>
+        
   <reviews
   			id="2"
   			status="OPEN"
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml
index 5ba0b972ce9..3c2044866c6 100644
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml
@@ -5,48 +5,48 @@
   			status="OPEN"
   			rule_failure_permanent_id="1"
   			resource_id="555"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="2"
   			status="CLOSED"
   			rule_failure_permanent_id="2"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="3"
   			status="OPEN"
   			rule_failure_permanent_id="3"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="4"
   			status="CLOSED"
   			rule_failure_permanent_id="2"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="5"
   			status="REOPENED"
   			rule_failure_permanent_id="3"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="6"
   			status="RESOLVED"
   			rule_failure_permanent_id="3"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="FIXED" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="FIXED" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="7"
   			status="CLOSED"
   			rule_failure_permanent_id="2"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="8"
   			status="CLOSED"
   			rule_failure_permanent_id="2"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="FIXED" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="FIXED" severity="[null]" resource_line="[null]" project_id="[null]"/>
 
 </dataset>
\ No newline at end of file
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldReopenResolvedReviewWithNonFixedViolation-result.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldReopenResolvedReviewWithNonFixedViolation-result.xml
index de7ef1fb28c..750659d46a8 100644
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldReopenResolvedReviewWithNonFixedViolation-result.xml
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldReopenResolvedReviewWithNonFixedViolation-result.xml
@@ -5,48 +5,48 @@
   			status="OPEN"
   			rule_failure_permanent_id="1"
   			resource_id="555"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="2"
   			status="CLOSED"
   			rule_failure_permanent_id="2"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="3"
   			status="OPEN"
   			rule_failure_permanent_id="3"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="4"
   			status="CLOSED"
   			rule_failure_permanent_id="2"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="5"
   			status="REOPENED"
   			rule_failure_permanent_id="3"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="6"
   			status="REOPENED"
   			rule_failure_permanent_id="3"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="7"
   			status="CLOSED"
   			rule_failure_permanent_id="2"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/>
   <reviews
   			id="8"
   			status="CLOSED"
   			rule_failure_permanent_id="2"
   			resource_id="666"
-  			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="FIXED" severity="[null]" resource_line="[null]" project_id="[null]"/>
+  			created_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" resolution="FIXED" severity="[null]" resource_line="[null]" project_id="[null]"/>
 
 </dataset>
\ No newline at end of file
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
index b91d00a90cf..94e6786f8a4 100644
--- 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
@@ -97,7 +97,7 @@ public class ReviewEmailTemplate extends EmailTemplate {
   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');
+        .append("See it in Sonar: ").append(configuration.getServerBaseURL()).append("/review/view/").append(reviewId).append('\n');
   }
 
   /**
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
index 3b65b418f60..6b64545eb5a 100644
--- 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
@@ -58,7 +58,7 @@ public class ReviewEmailTemplateTest {
    *   This is my first comment
    * 
    * --
-   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * See it in Sonar: http://nemo.sonarsource.org/review/view/1
    * </pre>
    */
   @Test
@@ -72,7 +72,7 @@ public class ReviewEmailTemplateTest {
     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"));
+    assertThat(message.getMessage(), is("Comment:\n  This is my first comment\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/view/1\n"));
   }
 
   /**
@@ -86,7 +86,7 @@ public class ReviewEmailTemplateTest {
    *   This is my first comment
    * 
    * --
-   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * See it in Sonar: http://nemo.sonarsource.org/review/view/1
    * </pre>
    */
   @Test
@@ -100,7 +100,7 @@ public class ReviewEmailTemplateTest {
     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"));
+    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/view/1\n"));
   }
 
   /**
@@ -112,7 +112,7 @@ public class ReviewEmailTemplateTest {
    *   This is deleted comment
    *   
    * --
-   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * See it in Sonar: http://nemo.sonarsource.org/review/view/1
    * </pre>
    */
   @Test
@@ -126,7 +126,7 @@ public class ReviewEmailTemplateTest {
     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"));
+    assertThat(message.getMessage(), is("Comment deleted, was:\n  This is deleted comment\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/view/1\n"));
   }
 
   /**
@@ -137,7 +137,7 @@ public class ReviewEmailTemplateTest {
    * Assignee: Evgeny Mandrikov
    * 
    * --
-   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * See it in Sonar: http://nemo.sonarsource.org/review/view/1
    * </pre>
    */
   @Test
@@ -151,7 +151,7 @@ public class ReviewEmailTemplateTest {
     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"));
+    assertThat(message.getMessage(), is("Assignee: Evgeny Mandrikov\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/view/1\n"));
   }
 
   /**
@@ -162,7 +162,7 @@ public class ReviewEmailTemplateTest {
    * Assignee: Simon Brandhof (was Evgeny Mandrikov)
    * 
    * --
-   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * See it in Sonar: http://nemo.sonarsource.org/review/view/1
    * </pre>
    */
   @Test
@@ -176,7 +176,7 @@ public class ReviewEmailTemplateTest {
     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"));
+    assertThat(message.getMessage(), is("Assignee: Simon Brandhof (was Evgeny Mandrikov)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/view/1\n"));
   }
 
   /**
@@ -187,7 +187,7 @@ public class ReviewEmailTemplateTest {
    * Assignee: (was Simon Brandhof)
    * 
    * --
-   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * See it in Sonar: http://nemo.sonarsource.org/review/view/1
    * </pre>
    */
   @Test
@@ -201,7 +201,7 @@ public class ReviewEmailTemplateTest {
     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"));
+    assertThat(message.getMessage(), is("Assignee:  (was Simon Brandhof)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/view/1\n"));
   }
 
   /**
@@ -212,7 +212,7 @@ public class ReviewEmailTemplateTest {
    * Status: CLOSED (was OPEN)
    * 
    * --
-   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * See it in Sonar: http://nemo.sonarsource.org/review/view/1
    * </pre>
    */
   @Test
@@ -225,7 +225,7 @@ public class ReviewEmailTemplateTest {
     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"));
+    assertThat(message.getMessage(), is("Status: CLOSED (was OPEN)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/view/1\n"));
   }
 
   /**
@@ -237,7 +237,7 @@ public class ReviewEmailTemplateTest {
    * Resolution: (was FIXED)
    * 
    * --
-   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * See it in Sonar: http://nemo.sonarsource.org/review/view/1
    * </pre>
    */
   @Test
@@ -252,7 +252,7 @@ public class ReviewEmailTemplateTest {
     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"));
+    assertThat(message.getMessage(), is("Status: REOPENED (was RESOLVED)\nResolution:  (was FIXED)\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/view/1\n"));
   }
 
   /**
@@ -264,7 +264,7 @@ public class ReviewEmailTemplateTest {
    * Resolution: FIXED
    * 
    * --
-   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * See it in Sonar: http://nemo.sonarsource.org/review/view/1
    * </pre>
    */
   @Test
@@ -280,7 +280,7 @@ public class ReviewEmailTemplateTest {
     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"));
+    assertThat(message.getMessage(), is("Status: RESOLVED (was OPEN)\nResolution: FIXED\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/view/1\n"));
   }
 
   /**
@@ -292,7 +292,7 @@ public class ReviewEmailTemplateTest {
    * Resolution: FALSE-POSITIVE
    * 
    * --
-   * See it in Sonar: http://nemo.sonarsource.org/review/1
+   * See it in Sonar: http://nemo.sonarsource.org/review/view/1
    * </pre>
    */
   @Test
@@ -308,7 +308,7 @@ public class ReviewEmailTemplateTest {
     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"));
+    assertThat(message.getMessage(), is("Status: RESOLVED (was REOPENED)\nResolution: FALSE-POSITIVE\n\n--\nSee it in Sonar: http://nemo.sonarsource.org/review/view/1\n"));
   }
 
   @Test
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/database/model/Review.java b/sonar-plugin-api/src/main/java/org/sonar/api/database/model/Review.java
index dc92a9851e6..927e96a4a8e 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/database/model/Review.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/database/model/Review.java
@@ -19,7 +19,16 @@
  */
 package org.sonar.api.database.model;
 
-import javax.persistence.*;
+import java.util.Date;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+import javax.persistence.Table;
+
+import org.apache.commons.lang.builder.ToStringBuilder;
+import org.apache.commons.lang.builder.ToStringStyle;
 
 @Entity
 @Table(name = "reviews")
@@ -39,6 +48,33 @@ public final class Review {
   @Column(name = "title")
   private String title;
 
+  @Column(name = "status")
+  private String status;
+
+  @Column(name = "resolution")
+  private String resolution;
+
+  @Column(name = "rule_failure_permanent_id")
+  private Integer permanentId;
+
+  @Column(name = "project_id")
+  private Integer projectId;
+
+  @Column(name = "resource_id")
+  private Integer resourceId;
+
+  @Column(name = "resource_line")
+  private Integer resourceLine;
+
+  @Column(name = "created_at")
+  private Date createdAt;
+
+  @Column(name = "updated_at")
+  private Date updatedAt;
+
+  @Column(name = "severity")
+  private String severity;
+
   /**
    * @return id of review
    */
@@ -83,4 +119,25 @@ public final class Review {
     return this;
   }
 
+  public String getStatus() {
+    return status;
+  }
+
+  public void setStatus(String status) {
+    this.status = status;
+  }
+
+  public String getResolution() {
+    return resolution;
+  }
+
+  public void setResolution(String resolution) {
+    this.resolution = resolution;
+  }
+
+  @Override
+  public String toString() {
+    return ToStringBuilder.reflectionToString(this, ToStringStyle.SHORT_PREFIX_STYLE);
+  }
+
 }