SONAR-2327 split actions for resource viewer and reviews page

26 files changed, 400 insertions, 567 deletions

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java
index b0827f546f4..4634bcb01fe 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java
@@ -214,7 +214,7 @@ public class CorePlugin extends SonarPlugin {
     extensions.add(NoSonarFilter.class);
     extensions.add(DirectoriesDecorator.class);
     extensions.add(FilesDecorator.class);
-    extensions.add(ReviewsDecorator.class);
+    extensions.add(CloseReviewsDecorator.class);
 
     // time machine
     extensions.add(TendencyDecorator.class);
diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ReviewsDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java
index c4be5daf4f0..e037cd03877 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ReviewsDecorator.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java
@@ -36,14 +36,14 @@ import org.sonar.batch.index.ResourcePersister;
  * Decorator that currently only closes a review when its corresponding violation has been fixed.
  */
 @DependsUpon("ViolationPersisterDecorator")
-public class ReviewsDecorator implements Decorator {
+public class CloseReviewsDecorator implements Decorator {
 
-  private static final Logger LOG = LoggerFactory.getLogger(ReviewsDecorator.class);
+  private static final Logger LOG = LoggerFactory.getLogger(CloseReviewsDecorator.class);
 
   private ResourcePersister resourcePersister;
   private DatabaseSession databaseSession;
 
-  public ReviewsDecorator(ResourcePersister resourcePersister, DatabaseSession databaseSession) {
+  public CloseReviewsDecorator(ResourcePersister resourcePersister, DatabaseSession databaseSession) {
     this.resourcePersister = resourcePersister;
     this.databaseSession = databaseSession;
   }
@@ -64,9 +64,9 @@ public class ReviewsDecorator implements Decorator {
     }
   }
 
-  protected String generateSqlRequest(int resourceId, int snapshotId) {
-    return "UPDATE reviews SET status='closed' " + "WHERE resource_id = " + resourceId + " AND rule_failure_permanent_id NOT IN "
-        + "(SELECT permanent_id FROM rule_failures WHERE snapshot_id = " + snapshotId + ")";
+  String generateSqlRequest(int resourceId, int snapshotId) {
+    return "UPDATE reviews SET status='CLOSED' " + "WHERE 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)";
   }
 
 }
diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ReviewsDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java
index 53c42d1b06a..493352c1147 100644
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ReviewsDecoratorTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java
@@ -27,16 +27,15 @@ import java.sql.Statement;
 import org.junit.Test;
 import org.sonar.test.persistence.DatabaseTestCase;
 
-public class ReviewsDecoratorTest extends DatabaseTestCase {
+public class CloseReviewsDecoratorTest extends DatabaseTestCase {
 
   @Test
   public void shouldCloseReviewWithoutCorrespondingViolation() throws Exception {
     setupData("fixture");
 
-    ReviewsDecorator reviewsDecorator = new ReviewsDecorator(null, null);
+    CloseReviewsDecorator reviewsDecorator = new CloseReviewsDecorator(null, null);
     String sqlRequest = reviewsDecorator.generateSqlRequest(666, 222);
-    System.out.println(sqlRequest);
-
+    
     Statement stmt = getConnection().createStatement();
     int count = stmt.executeUpdate(sqlRequest);
 
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/ReviewsDecoratorTest/fixture.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml
index d57973770cb..6142cbf4f3b 100644
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/ReviewsDecoratorTest/fixture.xml
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml
@@ -45,17 +45,17 @@
 
   <reviews
   			id="1"
-  			status="open"
+  			status="OPEN"
   			rule_failure_permanent_id="1"
   			resource_id="555"/>
   <reviews
   			id="2"
-  			status="open"
+  			status="OPEN"
   			rule_failure_permanent_id="2"
   			resource_id="666"/>
   <reviews
   			id="3"
-  			status="open"
+  			status="OPEN"
   			rule_failure_permanent_id="3"
   			resource_id="666"/>
 
diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/ReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml
index 7d614f3112c..d46429ddf3d 100644
--- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/ReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml
+++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml
@@ -2,19 +2,19 @@
 
   <reviews
   			id="1"
-  			status="open"
+  			status="OPEN"
   			rule_failure_permanent_id="1"
   			resource_id="555"
   			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" review_type="[null]" severity="[null]" resource_line="[null]"/>
   <reviews
   			id="2"
-  			status="closed"
+  			status="CLOSED"
   			rule_failure_permanent_id="2"
   			resource_id="666"
   			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" review_type="[null]" severity="[null]" resource_line="[null]"/>
   <reviews
   			id="3"
-  			status="open"
+  			status="OPEN"
   			rule_failure_permanent_id="3"
   			resource_id="666"
   			created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" review_type="[null]" severity="[null]" resource_line="[null]"/>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
index 4e221556509..72893127e98 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
@@ -49,6 +49,7 @@ class ResourceController < ApplicationController
     end
   end
 
+
   private
 
   def load_extensions
@@ -205,7 +206,7 @@ class ResourceController < ApplicationController
       end
     end
 
-    RuleFailure.find(:all, :include => ['rule', 'reviews' ], :conditions => [conditions] + values, :order => 'failure_level DESC').each do |violation|
+    RuleFailure.find(:all, :include => ['rule', 'review' ], :conditions => [conditions] + values, :order => 'failure_level DESC').each do |violation|
       # sorted by severity => from blocker to info
       if violation.line && violation.line>0 && @lines
         @lines[violation.line-1].add_violation(violation)
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/reviews_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/reviews_controller.rb
index d7d038e98df..ab023eb4311 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/reviews_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/reviews_controller.rb
@@ -22,10 +22,9 @@ class ReviewsController < ApplicationController
 
   SECTION=Navigation::SECTION_HOME
 
-  verify :method => :post, :only => [  :create, :create_comment ], :redirect_to => { :action => :error_not_post }
+  verify :method => :post, :only => [:violation_assign, :violation_flag_as_false_positive,:violation_save_comment, :violation_delete_comment], :redirect_to => {:action => :error_not_post}
+  helper(:reviews,:markdown)
 
-  helper(:reviews, :markdown)
-  
   def index
     init_params
     search_reviews
@@ -36,193 +35,149 @@ class ReviewsController < ApplicationController
     render :partial => 'reviews/show'
   end
 
-  def list
-    reviews = find_reviews_for_rule_failure params[:rule_failure_permanent_id]
-    render :partial => "list", :locals => { :reviews => reviews }
-  end
 
+  #
+  #
+  # ACTIONS FROM VIOLATIONS TAB OF RESOURCE VIEWER
+  #
+  #
+
+  # GET
   def display_violation
-    violation = find_last_rule_failure_with_permanent_id params[:rule_failure_permanent_id]
+    violation = RuleFailure.find(params[:id])
     render :partial => "resource/violation", :locals => { :violation => violation }
   end
 
-  def form
-    rule_failure = find_last_rule_failure_with_permanent_id params[:rule_failure_permanent_id]
-    @review = Review.new
-    @review.rule_failure_permanent_id = rule_failure.permanent_id
-    @review_comment = ReviewComment.new
-    @review_comment.review_text = ""
-    if params[:switch_off]
-      @review.review_type = "f-positive"
-    else
-      @review.review_type = Review.default_type
-    end
-    render :partial => "form"
+  # GET
+  def violation_assign_form
+    @user_options = options_for_users()
+    render :partial => "violation_assign_form"
   end
 
-  def create
-    rule_failure = find_last_rule_failure_with_permanent_id params[:review][:rule_failure_permanent_id]
-    unless has_rights_to_modify? rule_failure
-      render :text => "<b>Cannot create the review</b> : access denied."
+  # POST
+  def violation_assign
+    violation = RuleFailure.find(params[:id])
+    unless current_user
+      render :text => "<b>Cannot edit the review</b> : access denied."
       return
     end
+    sanitize_violation(violation)
+
+    violation.build_review(:user_id => current_user.id)
+    violation.review.assignee = User.find params[:assignee_id]
+    violation.review.save!
+    violation.save
+
+    render :partial => "resource/violation", :locals => { :violation => violation }
+  end
+
+  # GET
+  def violation_false_positive_form
+    render :partial => 'reviews/violation_false_positive_form'
+  end
 
-    @review = Review.new(params[:review])
-    @review.user = current_user
-    if params[:assign_to_me]
-      @review.assignee = current_user
+  # POST
+  def violation_flag_as_false_positive
+    violation=RuleFailure.find params[:id]
+    unless has_rights_to_modify?(violation)
+      render :text => "<b>Cannot switch on the violation</b> : access denied."
+      return
     end
-    @review.title = rule_failure.message
-    @review.status = Review.default_status
-    @review.severity = Sonar::RulePriority.to_s rule_failure.failure_level
-    @review.resource = RuleFailure.find( @review.rule_failure_permanent_id, :include => ['snapshot'] ).snapshot.project
-    @review_comment = ReviewComment.new(params[:review_comment])
-    @review_comment.user = current_user
-    @review.review_comments << @review_comment
-    if @review.valid?
-      if @review.review_type == "f-positive" 
-        if rule_failure.get_open_review
-          current_open_review = rule_failure.get_open_review
-          current_open_review.status = "closed"
-          current_open_review.save
-        end
-        rule_failure.switched_off = true
-        rule_failure.save
+    sanitize_violation(violation)
+    false_positive=(params[:false_positive]=='true')
+    violation.switched_off=false_positive
+    violation.save!
+
+    unless params[:comment].blank?
+      if violation.review.nil?
+        violation.build_review(:user_id => current_user.id)
       end
-      @review.save
-      @violation = rule_failure
+      violation.review.review_type=(false_positive ? Review::TYPE_FALSE_POSITIVE : Review::TYPE_VIOLATION)
+      violation.review.status=(false_positive ? Review::STATUS_CLOSED : Review::STATUS_OPEN)
+      violation.review.save!
+      violation.review.comments.create(:review_text => params[:comment], :user_id => current_user.id)
     end
-    render "create_result"
+
+    render :partial => "resource/violation", :locals => { :violation => violation }
   end
 
-  def form_comment
-    @review_comment = ReviewComment.new
-    @review_comment.user = current_user
-    @review_comment.review_id = params[:review_id]
-    @review_comment.review_text = ""
-    @rule_failure_permanent_id = params[:rule_failure_permanent_id]
-    if params[:update_comment]
-      @update_comment = true
-      @review_comment.review_text = params[:review_text]
+  # GET
+  def violation_comment_form
+    @violation = RuleFailure.find params[:id]
+    if !params[:comment_id].blank? && @violation.review
+      @comment = @violation.review.comments.find(params[:comment_id])
     end
-    render :partial => "form_comment"
+    render :partial => 'reviews/violation_comment_form'
   end
 
-  def create_comment
-    rule_failure = find_last_rule_failure_with_permanent_id params[:rule_failure_permanent_id]
-    unless has_rights_to_modify? rule_failure
+  # POST
+  def violation_save_comment
+    violation = RuleFailure.find params[:id]
+    unless has_rights_to_modify?(violation)
       render :text => "<b>Cannot create the comment</b> : access denied."
       return
     end
+    sanitize_violation(violation)
 
-    @review_comment = ReviewComment.new(params[:review_comment])
-    @review_comment.user = current_user
-    @rule_failure_permanent_id = params[:rule_failure_permanent_id]
-    if @review_comment.valid?
-      @review_comment.save
-      # -- TODO : should create a Review#create_comment and put the following logic in it
-      review = @review_comment.review
-      review.updated_at = @review_comment.created_at
-      review.save
-      # -- End of TODO code --
-      @violation = rule_failure
+    unless violation.review
+      violation.create_review!(
+          :assignee => (params['assign_to_me']=='me' ? current_user : nil),
+          :user => current_user)
     end
-    render "create_comment_result"
-  end
 
-  def update_comment
-    review = Review.find params[:review_comment][:review_id]
-    @review_comment = review.review_comments.last
-    unless current_user && current_user.id == @review_comment.user_id
-      render :text => "<b>Cannot modify the comment</b> : access denied."
-      return
+    if params[:comment_id]
+      comment=violation.review.comments.find(params[:comment_id].to_i)
+      if comment
+        comment.text=params[:text]
+        comment.save!
+      end
+    else
+      violation.review.comments.create!(:user => current_user, :text => params[:text])
     end
 
-    @review_comment.review_text = params[:review_comment][:review_text]
-    @review_comment.created_at = DateTime.now
-    @rule_failure_permanent_id = params[:rule_failure_permanent_id]
-    if @review_comment.valid?
-      @review_comment.save
-      review.updated_at = @review_comment.updated_at
-      review.save
-      @violation = find_last_rule_failure_with_permanent_id review.rule_failure_permanent_id
-    end
-    render "create_comment_result"
-  end
-  
-  def form_assign
-    @user_options = add_all_users []
-    @review_id = params[:review_id]
-    @rule_failure_permanent_id = params[:rule_failure_permanent_id]
-    render :partial => "form_assign"
-  end
-  
-  def assign
-    review = Review.find params[:review_id]
-    unless current_user
-      render :text => "<b>Cannot edit the review</b> : access denied."
-      return
-    end
-    
-    review.assignee = User.find params[:assignee_id]
-    review.save
-    violation = find_last_rule_failure_with_permanent_id review.rule_failure_permanent_id
     render :partial => "resource/violation", :locals => { :violation => violation }
   end
-  
-  def close_review
-    review = Review.find params[:review_id]
-    unless current_user
-      render :text => "<b>Cannot edit the review</b> : access denied."
+
+  # POST
+  def violation_delete_comment
+    violation = RuleFailure.find params[:id]
+    unless has_rights_to_modify?(violation)
+      render :text => "<b>Cannot delete the comment</b> : access denied."
       return
     end
-    
-    review.status = "closed"
-    review.save
-    violation = find_last_rule_failure_with_permanent_id review.rule_failure_permanent_id
-    render :partial => "resource/violation", :locals => { :violation => violation }
-  end
-  
-  def switch_on_violation
-    rule_failure = RuleFailure.find params[:rule_failure_id]
-    unless has_rights_to_modify? rule_failure
-      render :text => "<b>Cannot switch on the violation</b> : access denied."
-      return
+    sanitize_violation(violation)
+    if violation.review
+      comment=violation.review.comments.find(params[:comment_id].to_i)
+      comment.delete if comment
     end
-    
-    rule_failure.switched_off = false
-    rule_failure.save
-    
-    review = rule_failure.get_open_review
-    review.status = "closed"
-    review.save
-    
-    render :partial => "resource/violation", :locals => { :violation => rule_failure }
+    render :partial => "resource/violation", :locals => { :violation => violation }
   end
 
+
+
   ## -------------- PRIVATE -------------- ##
   private
 
   def init_params
-    @user_names = [["Any", ""]]
+    @user_names = [["Any", ""]] + options_for_users
     default_user = (current_user ? [current_user.id.to_s] : [''])
-    add_all_users @user_names
     @authors = filter_any(params[:authors]) || ['']
     @assignees = filter_any(params[:assignees]) || default_user
     @severities = filter_any(params[:severities]) || ['']
     @statuses = filter_any(params[:statuses]) || [Review::STATUS_OPEN]
     @projects = filter_any(params[:projects]) || ['']
   end
-  
-  def add_all_users ( user_options )
+
+  def options_for_users
+    options=[]
     User.find( :all ).each do |user|
-      userName = user.name
+      username = user.name
       if current_user && current_user.id == user.id
-        userName = "Me (" + user.name + ")"
+        username = "Me (" + user.name + ")"
       end
-      user_options << [userName, user.id.to_s]
+      options<<[username, user.id.to_s]
     end
-    return user_options
+    options
   end
 
   def filter_any(array)
@@ -233,8 +188,8 @@ class ReviewsController < ApplicationController
   end
 
   def search_reviews
-    conditions=[]
-    values={}
+    conditions=['review_type<>:not_type']
+    values={:not_type => Review::TYPE_FALSE_POSITIVE}
 
     unless @statuses == [""]
       conditions << "status in (:statuses)"
@@ -246,36 +201,30 @@ class ReviewsController < ApplicationController
     end
     unless @authors == [""]
       conditions << "user_id in (:authors)"
-      values[:authors]=@authors
+      values[:authors]=@authors.map{|s| s.to_i}
     end
     unless @assignees == [""]
       conditions << "assignee_id in (:assignees)"
-      values[:assignees]=@assignees
+      values[:assignees]=@assignees.map{|s| s.to_i}
     end
 
     @reviews = Review.find( :all, :order => "created_at DESC", :conditions => [ conditions.join(" AND "), values] ).uniq
   end
 
-  def find_reviews_for_rule_failure ( rule_failure_permanent_id )
-    return Review.find :all, :conditions => ['rule_failure_permanent_id=?', rule_failure_permanent_id]
-  end
-
-  def find_last_rule_failure_with_permanent_id ( rule_failure_permanent_id )
-    return RuleFailure.last( :all, :conditions => [ "permanent_id = ?", rule_failure_permanent_id ], :include => ['snapshot'] )
-  end
-
-  def has_rights_to_modify? ( rule_failure )
-    return false unless current_user
-    
-    project = rule_failure.snapshot.root_project
-    unless has_role?(:user, project)
-      return false
-    end
-    return true
+  def has_rights_to_modify?(violation)
+    current_user && has_role?(:user, violation.snapshot)
   end
 
   def error_not_post
     render :text => "Create actions must use POST method."
   end
 
+  def sanitize_violation(violation)
+    # the field RULE_FAILURES.PERMANENT_ID is not set when upgrading to version 2.8.
+    # It must be manually set when using a violation created before v2.8.
+    if violation.permanent_id.nil?
+      violation.permanent_id=violation.id
+      violation.save
+    end
+  end
 end
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 0e45f87f94b..b05498c097f 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
@@ -23,32 +23,20 @@ class Review < ActiveRecord::Base
   belongs_to :resource, :class_name => "Project", :foreign_key => "resource_id"
   belongs_to :project, :class_name => "Project", :foreign_key => "project_id"
   has_many :review_comments, :order => "created_at", :dependent => :destroy
+  alias_attribute :comments, :review_comments
   belongs_to :rule_failure, :foreign_key => 'rule_failure_permanent_id'
   
   validates_presence_of :user, :message => "can't be empty"
-  validates_presence_of :title, :message => "can't be empty"
   validates_presence_of :review_type, :message => "can't be empty"
   validates_presence_of :status, :message => "can't be empty"
   
   before_save :assign_project
   
-  TYPE_COMMENTS = "comments"
-  TYPE_FALSE_POSITIVE = "f-positive"
+  TYPE_VIOLATION = 'VIOLATION'
+  TYPE_FALSE_POSITIVE = 'FALSE_POSITIVE'
   
-  STATUS_OPEN = "open"
-  STATUS_CLOSED = "closed"
-  
-  def self.default_severity
-    return Severity::MAJOR
-  end
-  
-  def self.default_type
-    return TYPE_COMMENTS
-  end
-
-  def self.default_status
-    return STATUS_OPEN
-  end
+  STATUS_OPEN = 'OPEN'
+  STATUS_CLOSED = 'CLOSED'
     
   def on_project?
     resource_id==project_id
@@ -61,10 +49,6 @@ class Review < ActiveRecord::Base
       end
   end
   
-  def comments
-    review_comments
-  end
-
   private
   
   def assign_project
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/review_comment.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/review_comment.rb
index c690fd5ff4f..fa427eb43a5 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/review_comment.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/review_comment.rb
@@ -28,7 +28,7 @@ class ReviewComment < ActiveRecord::Base
   private
   
   def comment_should_not_be_empty
-    errors.add("Comment", " cannot be empty") if review_text.strip.blank?
+    errors.add("Comment", " cannot be empty") if review_text.blank?
   end
 
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb
index b284b305bcb..890c4e7dd90 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/rule_failure.rb
@@ -22,17 +22,25 @@ class RuleFailure < ActiveRecord::Base
 
   belongs_to :rule
   belongs_to :snapshot
-  has_many :reviews, :primary_key => "permanent_id", :foreign_key => "rule_failure_permanent_id", :order => "created_at"
+  has_one :review, :primary_key => "permanent_id", :foreign_key => "rule_failure_permanent_id", :order => "created_at"
 
-  def get_open_review
-    reviews.each do |review|
-      if review.status == "open"
-        return review
-      end
-    end
-    return nil
+  def false_positive?
+    switched_off==true
   end
-  
+
+  # first line of message
+  def title
+    @title||=
+        begin
+          if message.blank?
+            rule.name
+          else
+            parts=message.split(/\r?\n|\r/, -1)
+            parts.size==0 ? rule.name : parts[0]
+          end
+        end
+  end
+
   def to_hash_json
     json = {}
     json['message'] = message
@@ -53,8 +61,7 @@ class RuleFailure < ActiveRecord::Base
       :qualifier => snapshot.project.qualifier,
       :language => snapshot.project.language
     }
-    open_review = get_open_review
-    json['review'] = open_review.to_hash_json ( false ) if open_review
+    json['review'] = review.to_hash_json(false) if review
     json
   end
 
@@ -85,4 +92,20 @@ class RuleFailure < ActiveRecord::Base
     datetime.strftime("%Y-%m-%dT%H:%M:%S%z")
   end
 
+  def build_review(options={})
+    if review.nil?
+      self.review=Review.new(
+        {:review_type => Review::TYPE_VIOLATION,
+        :status => Review::STATUS_OPEN,
+        :severity => Sonar::RulePriority.to_s(failure_level),
+        :resource_line => line,
+        :resource => snapshot.resource,
+        :title => title}.merge(options))
+    end
+  end
+
+  def create_review!(options={})
+    build_review(options)
+    self.review.save!
+  end
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_violation.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_violation.html.erb
index 4d52a57dcfe..a6adaa5e811 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_violation.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_violation.html.erb
@@ -1,80 +1,51 @@
-<div id="vId<%= violation.permanent_id -%>">
-<%
-  current_open_review = violation.get_open_review
-  false_positive = (current_open_review && current_open_review.review_type == "f-positive")
-%>
-
+<div id="vId<%= violation.id -%>">
 <div class="violation">
-
   <div class="vtitle">
-    <% if current_open_review %>
-    <div style="float: right"><span class="violation_date">#<%= current_open_review.id.to_s -%></span></div>
+    <% if violation.review %>
+      <div style="float: right"><span class="violation_date">#<%= violation.review.id -%></span></div>
     <% end %>
     
-    <%= image_tag("priority/" + violation.failure_level.to_s + ".png") -%>
+    <%= image_tag("priority/" + violation.failure_level.to_s + '.png') -%>
     &nbsp;
     <%= image_tag("sep12.png") -%>
-    &nbsp;&nbsp;
+    &nbsp;
     <span class="rulename">
       <a onclick="window.open(this.href,'rule','height=800,width=900,scrollbars=1,resizable=1');return false;" href="<%= url_for :controller => 'rules', :action => 'show', :id => violation.rule.key, :layout => 'false' -%>"><%= h(violation.rule.name) -%></a>
     </span>
-    &nbsp;&nbsp;
+    &nbsp;
     <%= image_tag("sep12.png") -%>
-    &nbsp;&nbsp;
-    <%
-	  if violation.created_at
-		duration=Date.today - violation.created_at.to_date
-    %>
-	  <span class="violation_date"><%= duration==0 ? 'today' : "#{duration} days ago" -%></span>
-      &nbsp;&nbsp;
+    &nbsp;
+    <% if violation.created_at %>
+	  <span class="violation_date"><%= distance_of_time_in_words_to_now(violation.created_at) -%></span>
+      &nbsp;
     <% end %>
-    <% if false_positive %>
+    <% if violation.false_positive? %>
       <%= image_tag("sep12.png") -%>
-      &nbsp;&nbsp;
-      False-Positive
-      &nbsp;&nbsp;
+      &nbsp;
+      <span class="falsePositive">False-Positive</span>
+      &nbsp;
     <% end %>
-    <% if current_open_review && current_open_review.assignee %>
+    <% if violation.review && violation.review.assignee_id %>
       <%= image_tag("sep12.png") -%>
-      &nbsp;&nbsp;
-      <%= h(current_open_review.assignee.name) -%>
-      &nbsp;&nbsp;
+      &nbsp;
+      Assigned to: <%= h(violation.review.assignee.name) -%>
+      &nbsp;
     <% end %>
     
-    
     <% if current_user %>
-    <span class="actions" id="vActions<%= violation.permanent_id.to_s -%>">
+    <span class="actions" id="vActions<%= violation.id -%>">
       <%= image_tag("sep12.png") -%>
-      &nbsp;&nbsp;
-      <% if current_open_review && !false_positive %>
-        <%= link_to_remote "Close review",  
-        		:url => { :controller => "reviews", :action => "close_review", :review_id => current_open_review.id, :rule_failure_permanent_id => violation.permanent_id },
-        		:update => "vId" + violation.permanent_id.to_s -%>
-        &nbsp;&nbsp;
-        <%= link_to_remote (current_open_review.assignee == nil ? "Assign" : "Reassign"),  
-        		:url => { :controller => "reviews", :action => "form_assign", :review_id => current_open_review.id, :rule_failure_permanent_id => violation.permanent_id },
-        		:update => "vActions" + violation.permanent_id.to_s,
-        		:complete => "$('vActions" + violation.permanent_id.to_s + "').style.visibility='visible';$('assignee_id').focus();" -%>
-        &nbsp;&nbsp;
-      <% end %>
-      <% unless current_open_review %>
-        <%= link_to_remote "Review",  
-        		:url => { :controller => "reviews", :action => "form", :rule_failure_permanent_id => violation.permanent_id },
-        		:update => "reviewForm" + violation.permanent_id.to_s,
-        		:complete => "$('reviewForm" + violation.permanent_id.to_s + "').style.display='';$('reviewText').focus();" -%>
-        &nbsp;&nbsp;
-      <% end %>
-      <% if violation.switched_off %>
-        <%= link_to_remote "Switch-on",  
-        		:url => { :controller => "reviews", :action => "switch_on_violation", :rule_failure_id => violation.id },
-        		:update => "vId" + violation.permanent_id.to_s -%>
-      <% else %>
-        <%= link_to_remote "Switch-off",  
-        		:url => { :controller => "reviews", :action => "form", :rule_failure_permanent_id => violation.permanent_id, :switch_off => true },
-        		:update => "reviewForm" + violation.permanent_id.to_s,
-        		:complete => "$('reviewForm" + violation.permanent_id.to_s + "').style.display='';$('reviewText').focus();" -%>
-      <% end %>
-      &nbsp;&nbsp;
+      &nbsp;
+        <%= link_to_remote (violation.review && violation.review.assignee_id ? "Reassign" : "Assign"),  
+        		:url => { :controller => "reviews", :action => "violation_assign_form", :violation_id => violation.id},
+        		:update => "vActions" + violation.id.to_s,
+        		:complete => "$('vActions" + violation.id.to_s + "').show();$('assignee_id').focus();" -%>
+        &nbsp;
+
+        <%= link_to_remote (violation.false_positive? ? "Unflag false-positive" : "Flag as false-positive"),
+        		:url => { :controller => "reviews", :action => "violation_false_positive_form", :id => violation.id, :false_positive => !violation.false_positive? },
+        		:update => "reviewForm" + violation.id.to_s,
+        		:complete => "$('vActions" + violation.id.to_s + "').hide();$('reviewForm" + violation.id.to_s + "').show();$('reviewText" + violation.id.to_s + "').focus();" -%>
     </span>
     <% end %>
     
@@ -85,26 +56,30 @@
   </div>
 
   <% 
-    if current_open_review
-      last_comment = current_open_review.review_comments.last
-      current_open_review.review_comments.each do |review_comment|
+    if violation.review
+      violation.review.comments.each_with_index do |review_comment, comment_index|
+      is_last_comment=(comment_index==violation.review.comments.size-1)
   %>
   <div class="discussionComment">
-    <% duration=Date.today - review_comment.created_at.to_date %>
-    <h4><%= image_tag("reviews/comment.png") -%> &nbsp;<b><%= review_comment.user.name -%></b> (<%= duration==0 ? 'today' : "#{duration} days ago" -%>)
-    <% if review_comment == last_comment && current_user && current_user.id == review_comment.user.id %>
+    <h4><%= image_tag("reviews/comment.png") -%> &nbsp;<b><%= review_comment.user.name -%></b> (<%= distance_of_time_in_words_to_now(review_comment.created_at) -%>)
+    <% if is_last_comment && current_user && current_user.id == review_comment.user_id %>
       <span class="actions">
         &nbsp;&nbsp;
         <%= image_tag("sep12.png") -%>
         &nbsp;&nbsp;
         <%= link_to_remote "Edit", 
-      		:url => { :controller => "reviews", :action => "form_comment", :review_id => current_open_review.id, :rule_failure_permanent_id => violation.permanent_id, :review_text => last_comment.review_text, :update_comment => "true" },
-      		:update => "lastComment" + violation.permanent_id.to_s,
-      		:complete => "$('commentText" + violation.permanent_id.to_s + "').focus()" -%>
+      		:url => { :controller => "reviews", :action => "violation_comment_form", :comment_id => review_comment.id, :id => violation.id },
+      		:update => "lastComment" + violation.id.to_s,
+      		:complete => "$('reviewForm#{violation.id}').hide();$('vActions#{violation.id}').hide();$('commentText#{violation.id}').focus();" -%>
+
+        <%= link_to_remote "Delete",
+      		:url => { :controller => "reviews", :action => "violation_delete_comment", :comment_id => review_comment.id, :id => violation.id },
+      		:update => "vId" + violation.id.to_s,
+          :confirm => "Do you want to delete this comment ?" -%>
       </span>
     <% end %>
     </h4> 
-      <% if review_comment == last_comment %>
+      <% if is_last_comment %>
         <div id="lastComment<%= violation.id -%>">
           <%= markdown_to_html(review_comment.text) -%>
         </div>
@@ -117,17 +92,17 @@
     end
   %>
 
-  <div class="comment" id="reviewForm<%= violation.permanent_id.to_s -%>" style="display: none">
-  </div>
-
+  <div class="discussionComment" id="reviewForm<%= violation.id -%>" style="display:none"></div>
 </div>
 
-<% if current_user && current_open_review %>
-<div style="padding: 5px" id="commentAction<%= violation.permanent_id -%>">
-  <%= link_to_remote "Add comment", 
-      		:url => { :controller => "reviews", :action => "form_comment", :review_id => current_open_review.id, :rule_failure_permanent_id => violation.permanent_id },
-      		:update => "reviewForm" + violation.permanent_id.to_s, 
-      		:complete => "$('commentAction" + violation.permanent_id.to_s + "').style.display='none';$('reviewForm" + violation.permanent_id.to_s + "').style.display='';$('commentText" + violation.permanent_id.to_s + "').focus()" -%>
+
+
+<% if current_user %>
+<div style="padding: 5px" id="commentAction<%= violation.id -%>">
+  <%= link_to_remote "Add comment",
+      		:url => { :controller => "reviews", :action => "violation_comment_form", :id => violation.id },
+      		:update => "reviewForm" + violation.id.to_s,
+      		:complete => "$('vActions#{violation.id}').hide();$('commentAction" + violation.id.to_s + "').hide();$('reviewForm" + violation.id.to_s + "').show();$('commentText" + violation.id.to_s + "').focus()" -%>
 </div>
 <% end %>
 
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_form.html.erb
deleted file mode 100644
index 494bc2974fc..00000000000
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_form.html.erb
+++ /dev/null
@@ -1,30 +0,0 @@
-<% form_for :review, @review do |f| %>
-      <%= f.hidden_field :rule_failure_permanent_id -%>
-      <%= f.hidden_field :review_type -%>
-      
-      <% if @review.review_type == "f-positive" %>
-          <b>Why is it a false-positive ?</b>
-      <% end %>
-      <%= text_area :review_comment, :review_text, 
-           :id => "reviewText", :rows => 8, 
-           :style => "width:100%", :onKeyUp => "if (this.value=='') $('submit_btn').disabled='true'; else $('submit_btn').disabled='';" -%>
-      <br/>
-      
-      <div>
-        <%
-          if @review.review_type == "comments"
-            button_text = "Post review"
-          else
-            button_text = "Switch-off violation"
-          end
-        %>
-        <%= submit_to_remote "submit_btn", button_text, :url => { :action => 'create' }, :html => { :id => "submit_btn", :disabled => "true" } -%>
-        &nbsp;&nbsp;
-        <a onclick="$('reviewForm<%= @review.rule_failure_permanent_id.to_s -%>').style.display='none';" href="#">Cancel</a>
-        <% if @review.review_type == "comments" %>
-          &nbsp;&nbsp;
-          <%= check_box_tag "assign_to_me", "me", true -%> Assign to me
-        <% end %>
-      </div>
-      </div class="clear"></div>      
-<% end %>
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_form_assign.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_form_assign.html.erb
deleted file mode 100644
index 4c53bb70131..00000000000
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_form_assign.html.erb
+++ /dev/null
@@ -1,14 +0,0 @@
-<%= image_tag("sep12.png") -%>
-&nbsp;&nbsp;
-<% form_tag :html => {:style => "display:inline"} do %>
-  <%= hidden_field_tag :review_id, @review_id -%>
-  <%= select_tag "assignee_id", options_for_select(@user_options, current_user.id.to_s)  %>
-  &nbsp;&nbsp;
-  <%= submit_to_remote "submit_btn", "Assign", 
-         :url => { :action => 'assign' }, 
-         :update => "vId" + @rule_failure_permanent_id.to_s -%>
-  &nbsp;&nbsp;
-  <%= link_to_remote 'Cancel', 
-         :url => { :action => 'display_violation', :rule_failure_permanent_id => @rule_failure_permanent_id },
-         :update => "vId" + @rule_failure_permanent_id.to_s %> 
-<% end %>
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_form_comment.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_form_comment.html.erb
deleted file mode 100644
index 016e8052e99..00000000000
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_form_comment.html.erb
+++ /dev/null
@@ -1,28 +0,0 @@
-<% form_for :review_comment, @review_comment do |f| %>
-      <%= f.hidden_field :review_id %>
-      <%= f.text_area :review_text, :rows => 8, 
-            :id => "commentText" + @rule_failure_permanent_id.to_s,
-            :style => "width:100%",
-            :onKeyUp => "if (this.value=='') $('submit_btn').disabled='true'; else $('submit_btn').disabled='';" %>
-      <br/>
-      
-      <% if @update_comment %>
-        <%= submit_to_remote 'submit_btn', 'Update comment', 
-                     :url => { :action => 'update_comment', 
-                               :rule_failure_permanent_id => @rule_failure_permanent_id },
-                     :html => { :id => "submit_btn" } %>
-        &nbsp;&nbsp;
-        <%= link_to_remote 'Cancel', 
-                     :url => { :action => 'display_violation', 
-                               :rule_failure_permanent_id => @rule_failure_permanent_id },
-                     :update => "vId" + @rule_failure_permanent_id.to_s %>     
-      <% else %>
-        <%= submit_to_remote 'submit_btn', 'Post comment', 
-                     :url => { :action => 'create_comment', 
-                               :rule_failure_permanent_id => @rule_failure_permanent_id },
-                     :html => { :id => "submit_btn", :disabled => true } %>
-        &nbsp;&nbsp;
-        <a onclick="$('reviewForm<%= @rule_failure_permanent_id.to_s -%>').style.display='none'; $('commentAction<%= @rule_failure_permanent_id.to_s -%>').style.display='';" href="#">Cancel</a>
-      <% end %>
-      
-<% end %>
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_list.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_list.html.erb
deleted file mode 100644
index 2fbbd000238..00000000000
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_list.html.erb
+++ /dev/null
@@ -1,11 +0,0 @@
-<% 
-   unless reviews.blank? 
-     reviews.each do |review|
-%>
-
-  <%= render :partial => "reviews/view", :locals => { :review => review } %>
-
-<% 
-     end
-   end
-%>
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_review.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_review.html.erb
index 970a328bfee..80a5c613d14 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_review.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_review.html.erb
@@ -10,13 +10,13 @@
 	     Status:	
 	   </td>
 	   <td  class="val">
-		<%= image_tag "status/#{review.status}.png" -%> <%= review.status -%>
+		<%= image_tag "status/#{review.status}.png" -%> <%= review.status.capitalize -%>
 	   </td>
 	   <td class="key">
          Severity:
 	   </td>
 	   <td  class="val">
-          <%= image_tag "priority/#{review.severity}.png" -%> <%= review.severity -%>
+          <%= image_tag "priority/#{review.severity}.png" -%> <%= review.severity.capitalize -%>
 	   </td>
 	 </tr>
 	 <tr>
@@ -39,7 +39,7 @@
 	     Rule:	
 	   </td>
 	   <td class="val" colspan="3">
-		  <a onclick="window.open(this.href,'rule','height=800,width=900,scrollbars=1,resizable=1');return false;" href="<%= url_for :controller => 'rules', :action => 'show', :id => review.rule.key, :layout => 'false' -%>"><%= h(review.rule.key) -%></a>
+		  <a onclick="window.open(this.href,'rule','height=800,width=900,scrollbars=1,resizable=1');return false;" href="<%= url_for :controller => 'rules', :action => 'show', :id => review.rule.key, :layout => 'false' -%>"><%= h(review.rule.name) -%></a>
 	   </td>
 	</tr>
 	<% end %>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_show.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_show.html.erb
index 45b8bc3a8af..7e90b13accb 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_show.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_show.html.erb
@@ -1,5 +1,5 @@
 <div class="marginbottom10">
-	<a href="#" onclick="backReviews()">Back to reviews</a>
+	<a href="#" onclick="backReviews()">&laquo; Back to reviews</a>
 </div>
 
 <%= render :partial => 'reviews/review', :locals => {:review => @review} -%>
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_view.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_view.html.erb
deleted file mode 100644
index aa12cbc38fb..00000000000
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_view.html.erb
+++ /dev/null
@@ -1,65 +0,0 @@
-  <div id="review<%= review.id -%>">
-	  <div>
-      	<b>Review #<%= review.id -%> - <%= h(review.title) -%></b>
-      	<br/>
-      	<% if review.assignee %>
-      	<i>Assigned to <%= h(review.assignee.name) -%></i>
-      	<% end %>
-      </div>
-      
-      <div style="margin-top: 10px; margin-left: 20px">
-        <% unless review.review_comments.blank?
-             last_comment = review.review_comments.last
-		     review.review_comments.each do |review_comment|
-		%>
-        <table style="width:100%; margin-bottom: 3px">
-		  <tr style="border: solid 1px grey; background-color: #F7F7F7">
-		    <td style="width:180px; vertical-align:top; padding: 2px 2px 2px 10px; font-weight: bold">
-				<%= image_tag("user.png") -%> <b><%= h(review_comment.user.name) -%></b>
-		    </td>
-		    <td style="vertical-align: top; padding: 2px 10px 2px 2px; color: grey; text-align: right; font-style: italic">
-		    	<%= l review_comment.created_at -%>
-		    </td>
-		  </tr>
-		  <tr style="border: solid 1px grey;">
-		    <td colspan="2" style="padding: 5px;">
-		      <% if review_comment == last_comment %>
-		        <div id="lastComment<%= review.id -%>">
-		          <%= h(review_comment.review_text) -%>
-		        </div>
-		      <% else %>
-		        <%= h(review_comment.review_text) -%>
-		      <% end %>
-		    </td>
-		  </tr>
-		</table>
-		<% 
-		     end
-		   end
-		%>
-      
-        <% if current_user %>
-           <div style="text-align: right; padding: 5px">
-             <% if current_user.id == review.review_comments.last.user_id %>
-               <%= image_tag("pencil.png") -%>
-               <%= link_to_remote "Edit my last comment", 
-                  :url => { :controller => "reviews", :action => "form_comment", 
-                            :review_id => review.id, 
-                            :rule_failure_permanent_id => review.rule_failure_permanent_id,
-                            :review_text => review.review_comments.last.review_text,
-                            :update_comment => "true" },
-                  :update => "lastComment" + review.id.to_s, 
-                  :complete => "$('commentText" + review.id.to_s + "').focus()" -%>
-               &nbsp;
-             <% end %>
-             <%= image_tag("pencil.png") -%>
-             <%= link_to_remote "Add a new comment", 
-        		:url => { :controller => "reviews", :action => "form_comment", :review_id => review.id, :rule_failure_permanent_id => review.rule_failure_permanent_id },
-        		:update => "createComment" + review.id.to_s, 
-        		:complete => "$('commentText" + review.id.to_s + "').focus()" -%>
-           </div>
-           <div id="createComment<%= review.id -%>"></div>
-        <% end %>
-      </div>
-
-  </div>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_assign_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_assign_form.html.erb
new file mode 100644
index 00000000000..2b2b35030bb
--- /dev/null
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_assign_form.html.erb
@@ -0,0 +1,14 @@
+<%= image_tag("sep12.png") -%>
+&nbsp;&nbsp;
+<% form_tag :html => {:style => "display:inline"} do %>
+  <%= hidden_field_tag :id, params[:violation_id] -%>
+  <%= select_tag "assignee_id", options_for_select(@user_options, current_user.id.to_s)  %>
+  &nbsp;&nbsp;
+  <%= submit_to_remote "submit_btn", "Assign", 
+         :url => { :action => 'violation_assign' },
+         :update => "vId" + params[:violation_id] -%>
+  &nbsp;&nbsp;
+  <%= link_to_remote 'Cancel', 
+         :url => { :action => 'display_violation', :id => params[:violation_id] },
+         :update => "vId" + params[:violation_id] %>
+<% end %>
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_comment_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_comment_form.html.erb
new file mode 100644
index 00000000000..18e604906e1
--- /dev/null
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_comment_form.html.erb
@@ -0,0 +1,18 @@
+<%
+  button=(@comment ? 'Update comment' : 'Add comment')
+%>
+
+<form method="POST" action="'violation_save_comment'">
+  <input type="hidden" name="id" value="<%= params[:id] -%>"/>
+  <% if @comment %>
+    <input type="hidden" name="comment_id" value="<%= @comment.id -%>"/>
+  <% end %>
+  <textarea id="commentText<%= params[:id] -%>" rows="8" name="text" style="width: 100%" onkeyup="if (this.value=='') $('submit_btn').disabled='true'; else $('submit_btn').disabled='';"><%= @comment.text if @comment -%></textarea>
+  <%= submit_to_remote "submit_btn", button, :url => { :action => 'violation_save_comment'}, :html => { :id => "submit_btn", :disabled => "true" }, :update => 'vId'+params[:id] -%>
+  <%= link_to_remote 'Cancel', :url => {:action => 'display_violation', :id => params[:id]}, :update => 'vId' + params[:id] -%>
+
+  <% if @violation.review.nil? || @violation.review.comments.size==0 %>
+    &nbsp;&nbsp;
+    <%= check_box_tag "assign_to_me", "me", true -%> <label for="assign_to_me">Assign to me</label>
+  <% end %>
+</form>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_false_positive_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_false_positive_form.html.erb
new file mode 100644
index 00000000000..109f3a292bd
--- /dev/null
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_false_positive_form.html.erb
@@ -0,0 +1,19 @@
+<%
+   if params[:false_positive]=='true'
+     title = "Why is it a false-positive ?"
+     button = "Flag as false-positive"
+   else
+     title = "Why is it not a false-positive anymore ?"
+     button = "Unflag as false-positive"
+   end
+ %>
+<form method="POST" action="violation_flag_as_false_positive">
+  <input type="hidden" name="id" value="<%= params[:id] -%>"/>
+  <% if params[:false_positive]=='true' -%>
+    <input type="hidden" name="false_positive" value="true"/>
+  <% end %>
+  <h3><%= title -%></h3>
+  <textarea rows="8" name="comment" style="width: 100%" onkeyup="if (this.value=='') $('submit_btn').disabled='true'; else $('submit_btn').disabled='';"></textarea>
+  <%= submit_to_remote "submit_btn", button, :url => { :action => 'violation_flag_as_false_positive' }, :html => { :id => "submit_btn", :disabled => "true" }, :update => 'vId'+params[:id] -%>
+  <%= link_to_remote 'Cancel', :url => {:action => 'display_violation', :id => params[:id]}, :update => 'vId' + params[:id] -%>
+</form>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/create_comment_result.js.rjs b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/create_comment_result.js.rjs
deleted file mode 100644
index 3281ad51d3b..00000000000
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/create_comment_result.js.rjs
+++ /dev/null
@@ -1,5 +0,0 @@
-if @violation
-  page.replace "vId" + @violation.permanent_id.to_s, :partial => "resource/violation", :locals => { :violation => @violation }
-else
-  page.replace_html "reviewForm" + @rule_failure_permanent_id.to_s, :partial => "form_comment"
-end
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/create_result.js.rjs b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/create_result.js.rjs
deleted file mode 100644
index 9e735242a22..00000000000
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/create_result.js.rjs
+++ /dev/null
@@ -1,5 +0,0 @@
-if @violation
-  page.replace "vId" + @violation.permanent_id.to_s, :partial => "resource/violation", :locals => { :violation => @violation }
-else 
-  page.replace_html "reviewForm" + @review.rule_failure_permanent_id.to_s, :partial => "form"
-end
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/index.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/index.html.erb
index 67707ac52f6..0000c7478bc 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/index.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/index.html.erb
@@ -1,124 +1,130 @@
 <script>
-function onReviewLoading() {
-	$('reviews-search').hide();
-	$('review').hide();
-	$('review-loading').show();
-}
-function onReviewLoaded() {
-	$('reviews-search').hide();
-	$('review-loading').hide();
-	$('review').show();
-}
-function backReviews() {
-  	$('review').hide();
-	$('review-loading').hide();
-    $('reviews-search').show();	
-}
+  function onReviewLoading() {
+    $('reviews-search').hide();
+    $('review').hide();
+    $('review-loading').show();
+  }
+  function onReviewLoaded() {
+    $('reviews-search').hide();
+    $('review-loading').hide();
+    $('review').show();
+  }
+  function backReviews() {
+    $('review').hide();
+    $('review-loading').hide();
+    $('reviews-search').show();
+  }
 </script>
 
 <div id="reviews-search">
   <h1>Reviews</h1>
   <% form_tag({:action => 'index'}, {:method => 'get'}) do %>
-   <table id="reviews-form" class="searchFilterBox">
-	 <thead>
-		<tr><th colspan="6"></th></tr>
-	</thead>
-	<tbody>
-     <tr>
-	   <td width="1%" nowrap>
-         <span class="note">Status</span><br/>
-		 <select size="6" name="statuses[]" multiple="multiple" id="statuses" class="withIcons">
-			<option <%= 'selected' if @statuses.include?('') -%> value="">Any</option>
-		    <option value="<%= Review::STATUS_OPEN -%>" class="status_open" <%= 'selected' if @statuses.include?(Review::STATUS_OPEN) -%>>Open</option>
+    <table id="reviews-form" class="searchFilterBox">
+      <thead>
+      <tr>
+        <th colspan="6"></th>
+      </tr>
+      </thead>
+      <tbody>
+      <tr>
+        <td width="1%" nowrap>
+          <span class="note">Status</span><br/>
+          <select size="6" name="statuses[]" multiple="multiple" id="statuses" class="withIcons">
+            <option <%= 'selected' if @statuses.include?('') -%> value="">Any</option>
+            <option value="<%= Review::STATUS_OPEN -%>" class="status_open" <%= 'selected' if @statuses.include?(Review::STATUS_OPEN) -%>>Open</option>
             <option value="<%= Review::STATUS_CLOSED -%>" class="status_closed" <%= 'selected' if @statuses.include?(Review::STATUS_CLOSED) -%>>Closed</option>
-       </td>
+          </select>
+        </td>
+
+        <td width="1%" nowrap>
+          <span class="note">Project</span><br/>
+          <select size="6" name="projects[]" multiple="multiple" id="projects">
+            <option <%= 'selected' if @projects.include?('') -%> value="">Any</option>
+            <% projects_for_select.each do |project|
+              name=project.name(true)
+            %>
+              <option value="<%= project.id -%>" title="<%= h(name) -%>" <%= 'selected' if @projects.include?(project.id.to_s) -%>><%= h(truncate(name, :length => 20)) -%></option>
+            <% end %>
+          </select>
+        </td>
+        <td width="1%" nowrap>
+          <span class="note">Severity</span><br/>
+          <select size="6" name="severities[]" multiple="multiple" id="severities" class="withIcons">
+            <option <%= 'selected' if @severities.include?('') -%> value="">Any</option>
+            <option value="<%= Severity::BLOCKER -%>" class="sev_BLOCKER" <%= 'selected' if @severities.include?(Severity::BLOCKER) -%>>Blocker</option>
+            <option value="<%= Severity::CRITICAL -%>" class="sev_CRITICAL" <%= 'selected' if @severities.include?(Severity::CRITICAL) -%>>Critical</option>
+            <option value="<%= Severity::MAJOR -%>" class="sev_MAJOR" <%= 'selected' if @severities.include?(Severity::MAJOR) -%>>Major</option>
+            <option value="<%= Severity::MINOR -%>" class="sev_MINOR" <%= 'selected' if @severities.include?(Severity::MINOR) -%>>Minor</option>
+            <option value="<%= Severity::INFO -%>" class="sev_INFO" <%= 'selected' if @severities.include?(Severity::INFO) -%>>Info</option>
+           </select>
+        </td>
+
+        <td width="1%" nowrap>
+          <span class="note">Created by</span><br/>
+          <%= select_tag "authors", options_for_select(@user_names, @authors), :multiple => true, :size => 6 %>
+        </td>
+        <td width="1%" nowrap>
+          <span class="note">Assigned to</span><br/>
+          <%= select_tag "assignees", options_for_select(@user_names, @assignees), :multiple => true, :size => 6 %>
+        </td>
+        <td>
+          <br/>
+          <%= submit_tag "Search", :id => 'submit_search' %>
+        </td>
+      </tr>
+      </tbody>
+    </table>
+  <% end %>
 
-       <td width="1%" nowrap>
-         <span class="note">Project</span><br/>
-		<select size="6" name="projects[]" multiple="multiple" id="projects">
-			<option <%= 'selected' if @projects.include?('') -%> value="">Any</option>
-			<% projects_for_select.each do |project| 
-				name=project.name(true)
-			%>
-		      <option value="<%= project.id -%>" title="<%= h(name)-%>" <%= 'selected' if @projects.include?(project.id.to_s) -%>><%= h(truncate(name, :length => 20)) -%></option>
-		    <% end %>
-		</select>
-       </td>
-       <td width="1%" nowrap>
-         <span class="note">Severity</span><br/>
-		<select size="6" name="severities[]" multiple="multiple" id="severities" class="withIcons">
-          <option <%= 'selected' if @severities.include?('') -%> value="">Any</option>
-          <option value="<%= Severity::BLOCKER -%>" class="sev_BLOCKER" <%= 'selected' if @severities.include?(Severity::BLOCKER) -%>>Blocker</option>
-          <option value="<%= Severity::CRITICAL -%>" class="sev_CRITICAL" <%= 'selected' if @severities.include?(Severity::CRITICAL) -%>>Critical</option>
-          <option value="<%= Severity::MAJOR -%>" class="sev_MAJOR" <%= 'selected' if @severities.include?(Severity::MAJOR) -%>>Major</option>
-          <option value="<%= Severity::MINOR -%>" class="sev_MINOR" <%= 'selected' if @severities.include?(Severity::MINOR) -%>>Minor</option>
-          <option value="<%= Severity::INFO -%>" class="sev_INFO" <%= 'selected' if @severities.include?(Severity::INFO) -%>>Info</option>
-       </td>
-    
-       <td width="1%" nowrap>
-         <span class="note">Created by</span><br/>
-         <%= select_tag "authors", options_for_select(@user_names, @authors), :multiple => true, :size => 6 %>
-       </td>
-       <td width="1%" nowrap>
-         <span class="note">Assigned to</span><br/>
-         <%= select_tag "assignees", options_for_select(@user_names, @assignees), :multiple => true, :size => 6 %>
-       </td>
-       <td>
-         <br/>
-         <%= submit_tag "Search", :id => 'submit_search' %>
-       </td>
-     </tr>
-    </tbody>
-   </table>
-<% end %>
 
+  <%
+     if @reviews && !@reviews.empty?
+  %>
+    <table id="reviews-list" class="data width100">
+      <thead>
+      <tr>
+        <th width="1%" nowrap>St.</th>
+        <th width="1%">Project</th>
+        <th>Title</th>
+        <th width="1%" nowrap>Se.</th>
+        <th>Assignee</th>
+        <th>Age</th>
 
-<% 
-   if @reviews && !@reviews.empty?
-%>
-  <table id="reviews-list" class="data width100">
-	<thead>
-		<tr>
-			<th width="1%" nowrap>St.</th>
-			<th width="1%">Project</th>
-			<th>Title</th>
-			<th width="1%" nowrap>Se.</th>
-			<th>Assignee</th>
-			<th>Age</th>
-			
-		</tr>
-	</thead>
-	<tfoot>
-		<tr><td colspan="6"><%= @reviews.size -%> results</tr>
-	</tfoot>
-	<tbody>
-<%
-     @reviews.each do |review|
-%>
-      <tr class="<%= cycle('even', 'odd') -%>">
-	    <td><img src="<%= ApplicationController.root_context -%>/images/status/<%= review.status -%>.png"/></td>
-    	<td><%= review.project.name -%><br/><span class="note"><%= review.resource.long_name -%></span></td>
-	    <td>
-	      <%= link_to_remote(h(review.title), :update => 'review', :url => {:action => 'show', :id => review.id}, :loading => 'onReviewLoading()', :complete => "onReviewLoaded()") -%>
-	    </td>
-    	<td><img src="<%= ApplicationController.root_context -%>/images/priority/<%= review.severity -%>.png"/></td>
-		<td><%= review.assignee ? h(review.assignee.name) : '-' -%></td>
-		<td><%= distance_of_time_in_words_to_now(review.created_at) -%></td>
       </tr>
-<% 
+      </thead>
+      <tfoot>
+      <tr>
+        <td colspan="6"><%= @reviews.size -%> results
+      </tr>
+      </tfoot>
+      <tbody>
+      <%
+         @reviews.each do |review|
+      %>
+        <tr class="<%= cycle('even', 'odd') -%>">
+          <td><img src="<%= ApplicationController.root_context -%>/images/status/<%= review.status -%>.png"/></td>
+          <td><%= review.project.name -%>
+            <br/><span class="note"><%= review.resource.long_name -%></span></td>
+          <td>
+            <%= link_to_remote(h(review.title), :update => 'review', :url => {:action => 'show', :id => review.id}, :loading => 'onReviewLoading()', :complete => "onReviewLoaded()") -%>
+          </td>
+          <td><img src="<%= ApplicationController.root_context -%>/images/priority/<%= review.severity -%>.png"/></td>
+          <td><%= review.assignee ? h(review.assignee.name) : '-' -%></td>
+          <td><%= distance_of_time_in_words_to_now(review.created_at) -%></td>
+        </tr>
+      <%
+         end
+      %>
+      </tbody>
+    </table>
+  <%
+     elsif @reviews
+  %>
+    <p>No results</p>
+  <%
      end
-%> 
-   </tbody>
-  </table>
-<%
-   elsif @reviews
-%>
-  <p>No results</p>
-<%	
-   end
-%>
+  %>
 </div>
-	
-<div id="review-loading" style="display: none"><%= image_tag 'loading.gif' -%></div>
-<div id="review" style="display: none"></div>
 
+<div id="review-loading" style="display: none"><%= image_tag 'loading.gif' -%></div>
+<div id="review" style="display: none"></div>
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/db/migrate/191_create_review.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/191_create_review.rb
index e2827654bed..224f95f3804 100644
--- a/sonar-server/src/main/webapp/WEB-INF/db/migrate/191_create_review.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/db/migrate/191_create_review.rb
@@ -30,7 +30,7 @@ class CreateReview < ActiveRecord::Migration
       t.column 'user_id', 						:integer, 	:null => true
       t.column 'assignee_id', 					:integer, 	:null => true
       t.column 'title',		 					:string, 	:null => true,	:limit => 500
-      t.column 'review_type', 					:string, 	:null => true,	:limit => 10
+      t.column 'review_type', 					:string, 	:null => true,	:limit => 15
       t.column 'status', 						:string, 	:null => true,	:limit => 10
       t.column 'severity', 						:string, 	:null => true,	:limit => 10
       t.column 'rule_failure_permanent_id', 	:integer, 	:null => true   
diff --git a/sonar-server/src/main/webapp/stylesheets/style.css b/sonar-server/src/main/webapp/stylesheets/style.css
index ed955431b87..513a1962de2 100644
--- a/sonar-server/src/main/webapp/stylesheets/style.css
+++ b/sonar-server/src/main/webapp/stylesheets/style.css
@@ -687,9 +687,12 @@ div.vtitle{
   margin:0;
   padding:0 10px;
   line-height: 2.2em;
-  text-shadow: 1px 1px 0 #FFF;
+  text-shadow: 0 1px 0 #FFF;
   color:#777
 }
+.falsePositive {
+  background-color: #FFF6BF;
+}
 div.vtitle a.action {
   color: #777;
 }
@@ -1108,10 +1111,10 @@ select.withIcons option {
 	vertical-align: middle;
 }
 option.status_open {
-	background-image: url('../images/status/open.png');
+	background-image: url('../images/status/OPEN.png');
 }
 option.status_closed {
-	background-image: url('../images/status/closed.png');
+	background-image: url('../images/status/CLOSED.png');
 }
 option.sev_INFO {
 	background-image: url('../images/priority/INFO.png');