SONAR-2453 Update the way "false-positive" reviews are managed

* The column REVIEWS.FALSE-POSITIVE should be renamed to
  REVIEWS.RESOLUTION, the value of this column should be FALSE-POSITIVE
  for false-positive reviews and FIXED for other RESOLVED reviews.

* The status of a false-positive reviews should be RESOLVED.

4 files changed, 26 insertions, 16 deletions

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 9b886cfbd79..84726448203 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
@@ -110,13 +110,21 @@ class Review < ActiveRecord::Base
         end
       end
       create_comment(:user => params[:user], :text => params[:text])
-      self.false_positive = is_false_positive
       self.assignee = nil
-      self.status = STATUS_OPEN
-      self.save!      
+      self.status = is_false_positive ? STATUS_RESOLVED : STATUS_REOPENED
+      self.resolution = is_false_positive ? 'FALSE-POSITIVE' : nil
+      self.save!
     end
   end
   
+  def false_positive
+    resolution == 'FALSE-POSITIVE'
+  end
+  
+  def can_change_false_positive_flag?
+    (status == STATUS_RESOLVED && resolution == 'FALSE-POSITIVE') || status == STATUS_OPEN || status == STATUS_REOPENED
+  end
+
   def isResolved?
     status == STATUS_RESOLVED
   end
@@ -131,11 +139,13 @@ class Review < ActiveRecord::Base
   
   def reopen
     self.status = STATUS_REOPENED
+    self.resolution = nil
     self.save!
   end
   
   def resolve
     self.status = STATUS_RESOLVED
+    self.resolution = 'FIXED'
     self.save!
   end
   
@@ -155,22 +165,19 @@ class Review < ActiveRecord::Base
     # Following code just for backward compatibility
     review_type = options['review_type']
     if review_type
-      conditions << 'false_positive=:false_positive'
       if review_type == 'FALSE_POSITIVE'
-        values[:false_positive]=true
+        conditions << "resolution='FALSE-POSITIVE'"
       else
-        values[:false_positive]=false
+        conditions << "(resolution<>'FALSE-POSITIVE' OR resolution IS NULL)"
       end
     end
     # --- End of code for backward compatibility code ---
       
     false_positives = options['false_positives']
     if false_positives == "only"
-      conditions << 'false_positive=:false_positive'
-      values[:false_positive]=true
+      conditions << "resolution='FALSE-POSITIVE'"
     elsif false_positives == "without"
-      conditions << 'false_positive=:false_positive'
-      values[:false_positive]=false    
+      conditions << "(resolution<>'FALSE-POSITIVE' OR resolution IS NULL)"
     end
     
     ids=options['ids'].split(',') if options['ids']
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 114032a6472..15b621fec22 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
@@ -70,7 +70,7 @@
         <% end %>
       
         &nbsp;
-        <% unless violation.review && violation.review.isResolved? %>
+        <% if violation.review && violation.review.can_change_false_positive_flag? %>
         <%= link_to_remote (violation.switched_off? ? "Unflag as false-positive" : "Flag as false-positive"),
         		:url => { :controller => "reviews", :action => "violation_false_positive_form", :id => violation.id, :false_positive => !violation.switched_off? },
         		:update => "reviewForm" + violation.id.to_s,
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 45d91944eab..05c7e46353f 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
@@ -32,7 +32,7 @@
 	        &nbsp;
           <% end %>
         <% end %>
-        <% unless review.isResolved? %>
+        <% if review.can_change_false_positive_flag? %>
           <%= link_to_remote (violation_switched_off ? "Unflag as false-positive" : "Flag as false-positive"),
         		:url => { :controller => "reviews", :action => "false_positive_form", :id => review.id, :false_positive => !violation_switched_off },
         		:update => "reviewForm",
diff --git a/sonar-server/src/main/webapp/WEB-INF/db/migrate/201_change_false_positive_on_reviews.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/201_change_false_positive_on_reviews.rb
index d2c2312cf7b..fa0d157e867 100644
--- a/sonar-server/src/main/webapp/WEB-INF/db/migrate/201_change_false_positive_on_reviews.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/db/migrate/201_change_false_positive_on_reviews.rb
@@ -24,14 +24,17 @@
 class ChangeFalsePositiveOnReviews < ActiveRecord::Migration
 
   def self.up
-    add_column 'reviews', 'false_positive', :boolean, :null => true, :default => false
+    add_column 'reviews', 'resolution', :string, :limit => 200, :null => true
     Review.reset_column_information
-    
+
     Review.find(:all).each do |review|
-      review.false_positive= (review.review_type == 'FALSE_POSITIVE')
+      if review.review_type == 'FALSE_POSITIVE'
+        review.status = 'RESOLVED'
+        review.resolution = 'FALSE-POSITIVE'
+      end
       review.save!
     end
-    
+
     remove_column 'reviews', 'review_type'
     Review.reset_column_information
   end