[SONAR-1973] Update code to use the permanent ID of violations, set

resource on review and update review date when comment added

10 files changed, 474 insertions, 458 deletions

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 6f491007194..5cbf398eae5 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
@@ -1,334 +1,339 @@
-#
-# Sonar, entreprise quality control tool.
-# Copyright (C) 2008-2011 SonarSource
-# mailto:contact AT sonarsource DOT com
-#
-# Sonar is free software; you can redistribute it and/or
-# modify it under the terms of the GNU Lesser General Public
-# License as published by the Free Software Foundation; either
-# version 3 of the License, or (at your option) any later version.
-#
-# Sonar is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-# Lesser General Public License for more details.
-#
-# You should have received a copy of the GNU Lesser General Public
-# License along with Sonar; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
-#
-class ResourceController < ApplicationController
-
-  SECTION=Navigation::SECTION_RESOURCE
-  helper DashboardHelper
-  
-  def index
-    @resource = Project.by_key(params[:id])
-
-    if (@resource && has_role?(:user, @resource))
-      params[:layout]='false'
-      @snapshot=@resource.last_snapshot
-
-      load_extensions()
-
-      if @extension
-        if (@extension.getId()=='violations')
-          render_violations()
-        elsif (@extension.getId()=='coverage')
-          render_coverage()
-        elsif (@extension.getId()=='source')
-          render_source()
-        else
-          render_extension()
-        end
-      else
-        render_nothing()
-      end
-    else
-      access_denied
-    end
-  end
-
-  private
-
-  def load_extensions
-    @extensions=[]
-    java_facade.getResourceTabs(@resource.scope, @resource.qualifier, @resource.language).each do |tab|
-      tab.getUserRoles().each do |role|
-        if has_role?(role, @resource)
-          @extensions<<tab
-          break
-        end
-      end
-    end
-
-    if !params[:tab].blank?
-      @extension=@extensions.find{|extension| extension.getId()==params[:tab]}
-
-    elsif !params[:metric].blank?
-      metric=Metric.by_key(params[:metric])
-      @extension=@extensions.find{|extension| extension.getDefaultTabForMetrics().include?(metric.key)}
-
-    end
-    @extension=@extensions.find{|extension| extension.isDefaultTab()} if @extension==nil
-  end
-
-  def load_sources
-    @period = params[:period].to_i unless params[:period].blank?
-    @expanded=(params[:expand]=='true')
-
-    if @snapshot.source
-      source_lines=@snapshot.source.syntax_highlighted_lines()
-      init_scm()
-
-      @lines=[]
-      source_lines.each_with_index do |source, index|
-        line=Line.new(source)
-        @lines<<line
-
-        line.revision=@revisions_by_line[index+1]
-        line.author=@authors_by_line[index+1]
-
-        date_string=@dates_by_line[index+1]
-        line.datetime=(date_string ? Java::OrgSonarApiUtils::DateUtils.parseDateTime(date_string): nil)
-      end
-    end
-  end
-
-  def init_scm
-    @scm_available=(@snapshot.measure('last_commit_datetimes_by_line')!=nil)
-    @authors_by_line=load_distribution('authors_by_line')
-    @revisions_by_line=load_distribution('revisions_by_line')
-    @dates_by_line=load_distribution('last_commit_datetimes_by_line')
-  end
-
-  def load_distribution(metric_key)
-    m=@snapshot.measure(metric_key)
-    m ? m.data_as_line_distribution() : {}
-  end
-
-  def render_coverage
-    load_sources()
-    @display_coverage=true
-    @expandable=(@lines!=nil)
-    if @lines
-      @hits_by_line=load_distribution('coverage_line_hits_data')
-      @conditions_by_line=load_distribution('conditions_by_line')
-      @covered_conditions_by_line=load_distribution('covered_conditions_by_line')
-
-      @hits_by_line.each_pair do |line_id,hits|
-        line=@lines[line_id-1]
-        if line
-          line.hits=hits.to_i
-          line.conditions=@conditions_by_line[line_id].to_i
-          line.covered_conditions=@covered_conditions_by_line[line_id].to_i
-        end
-      end
-
-      if @snapshot.measure('conditions_by_line').nil?
-        # TODO remove this code when branch_coverage_hits_data is fully removed from CoreMetrics
-        deprecated_branches_by_line=load_distribution('branch_coverage_hits_data')
-        deprecated_branches_by_line.each_pair do |line_id,label|
-          line=@lines[line_id-1]
-          if line
-            line.deprecated_conditions_label=label
-          end
-        end
-      end
-
-      to=(@period ? Java::JavaUtil::Date.new(@snapshot.period_datetime(@period).to_f * 1000) : nil)
-      metric=Metric.by_key(params[:coverage_filter]||params[:metric])
-      @coverage_filter=(metric ? metric.key : 'coverage')
-      @filtered=true
-      if ('lines_to_cover'==@coverage_filter || 'coverage'==@coverage_filter || 'line_coverage'==@coverage_filter ||
-          'new_lines_to_cover'==@coverage_filter || 'new_coverage'==@coverage_filter || 'new_line_coverage'==@coverage_filter)
-        @coverage_filter='lines_to_cover'
-        filter_lines{|line| line.hits && line.after(to)}
-
-      elsif 'uncovered_lines'==@coverage_filter || 'new_uncovered_lines'==@coverage_filter
-        @coverage_filter='uncovered_lines'
-        filter_lines{|line| line.hits && line.hits==0 && line.after(to)}
-
-      elsif 'conditions_to_cover'==@coverage_filter || 'branch_coverage'==@coverage_filter ||
-            'new_conditions_to_cover'==@coverage_filter || 'new_branch_coverage'==@coverage_filter
-        @coverage_filter='conditions_to_cover'
-        filter_lines{|line| line.conditions && line.conditions>0 && line.after(to)}
-
-      elsif 'uncovered_conditions'==@coverage_filter || 'new_uncovered_conditions'==@coverage_filter
-        @coverage_filter='uncovered_conditions'
-        filter_lines{|line| line.conditions && line.covered_conditions && line.covered_conditions<line.conditions && line.after(to)}
-      end
-    end
-    render :action => 'index', :layout => !request.xhr?
-  end
-
-  
-  
-  def render_violations
-    load_sources()
-    @display_violations=true
-    @global_violations=[]
-    @expandable=(@lines!=nil)
-    @filtered=!@expanded
-
-    conditions='snapshot_id=?'
-    values=[@snapshot.id]
-    unless params[:rule].blank?
-      severity=Sonar::RulePriority.id(params[:rule])
-      if severity
-        conditions += ' AND failure_level=?'
-        values<<severity
-      else
-        rule=Rule.by_key_or_id(params[:rule])
-        conditions += ' AND rule_id=?'
-        values<<(rule ? rule.id : -1)
-      end
-    end
-
-    if @period
-      date=@snapshot.period_datetime(@period)
-      if date
-        conditions+=' AND created_at>?'
-        values<<date.advance(:minutes => 1)
-      else
-        conditions+=' AND id=-1'
-      end
-    end
-
-    RuleFailure.find(:all, :include => ['rule', 'reviews' ], :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)
-      else
-        @global_violations<<violation
-      end
-    end
-
-    if !@expanded && @lines
-      filter_lines{|line| line.violations?}
-    end
-    render :action => 'index', :layout => !request.xhr?
-  end
-  
-  
-  def render_source
-    load_sources()
-    filter_lines_by_date()
-    render :action => 'index', :layout => !request.xhr?
-  end
-
-  
-  def filter_lines_by_date
-    if @period
-      @filtered=true
-      to=Java::JavaUtil::Date.new(@snapshot.period_datetime(@period).to_f * 1000)
-      if to
-        @lines.each do |line|
-          line.flag_as_hidden() if !line.after(to)
-        end
-      end
-    end
-  end
-
-  def filter_lines(&block)
-    @lines.each_with_index do |line,index|
-      if yield(line)
-        for i in index-4...index
-          @lines[i].flag_as_highlight_context() if i>=0
-        end
-        line.flag_as_highlighted()
-        for i in index+1..index+4
-          @lines[i].flag_as_highlight_context() if i<@lines.size
-        end
-      else
-        line.flag_as_hidden()
-      end
-    end
-  end
-
-  class Line
-    attr_accessor :source, :revision, :author, :datetime, :violations, :hits, :conditions, :covered_conditions, :hidden, :highlighted, :deprecated_conditions_label
-
-    def initialize(source)
-      @source=source
-    end
-
-    def add_violation(violation)
-      @violations||=[]
-      @violations<<violation
-      @visible=true
-    end
-
-    def violations?
-      @violations && @violations.size>0
-    end
-
-    def violation_severity
-      if @violations && @violations.size>0
-        @violations[0].failure_level
-      else
-        nil
-      end
-    end
-
-    def after(date)
-      if date && @datetime
-        @datetime.after(date)
-      else
-        true
-      end
-    end
-
-    def flag_as_highlighted
-      @highlighted=true
-      @hidden=false
-    end
-
-    def flag_as_highlight_context
-      # do not force if highlighted has already been set to true
-      @highlighted=false if @highlighted.nil?
-      @hidden=false
-    end
-
-    def flag_as_hidden
-      # do not force if it has already been flagged as visible
-      if @hidden.nil?
-        @hidden=true
-        @highlighted=false
-      end
-    end
-
-    def hidden?
-      @hidden==true
-    end
-
-    def highlighted?
-      # highlighted if the @highlighted has not been set or has been set to true
-      !hidden? && @highlighted!=false
-    end
-
-    def deprecated_conditions_label=(label)
-      if label
-        @deprecated_conditions_label=label
-          if label=='0%'
-            @conditions=2
-            @covered_conditions=0
-          elsif label=='100%'
-            @conditions=2
-            @covered_conditions=2
-          else
-            @conditions=2
-            @covered_conditions=1
-        end
-      end
-    end
-  end
-
-  def render_extension()
-    render :action => 'extension', :layout => !request.xhr?
-  end
-
-  def render_nothing()
-    render :action => 'nothing', :layout => !request.xhr?
+#

+# Sonar, entreprise quality control tool.

+# Copyright (C) 2008-2011 SonarSource

+# mailto:contact AT sonarsource DOT com

+#

+# Sonar is free software; you can redistribute it and/or

+# modify it under the terms of the GNU Lesser General Public

+# License as published by the Free Software Foundation; either

+# version 3 of the License, or (at your option) any later version.

+#

+# Sonar is distributed in the hope that it will be useful,

+# but WITHOUT ANY WARRANTY; without even the implied warranty of

+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

+# Lesser General Public License for more details.

+#

+# You should have received a copy of the GNU Lesser General Public

+# License along with Sonar; if not, write to the Free Software

+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02

+#

+class ResourceController < ApplicationController

+

+  SECTION=Navigation::SECTION_RESOURCE

+  helper DashboardHelper

+  

+  def index

+    @resource = Project.by_key(params[:id])

+

+    if (@resource && has_role?(:user, @resource))

+      params[:layout]='false'

+      @snapshot=@resource.last_snapshot

+

+      load_extensions()

+

+      if @extension

+        if (@extension.getId()=='violations')

+          render_violations()

+        elsif (@extension.getId()=='coverage')

+          render_coverage()

+        elsif (@extension.getId()=='source')

+          render_source()

+        else

+          render_extension()

+        end

+      else

+        render_nothing()

+      end

+    else

+      access_denied

+    end

+  end

+

+  private

+

+  def load_extensions

+    @extensions=[]

+    java_facade.getResourceTabs(@resource.scope, @resource.qualifier, @resource.language).each do |tab|

+      tab.getUserRoles().each do |role|

+        if has_role?(role, @resource)

+          @extensions<<tab

+          break

+        end

+      end

+    end

+

+    if !params[:tab].blank?

+      @extension=@extensions.find{|extension| extension.getId()==params[:tab]}

+

+    elsif !params[:metric].blank?

+      metric=Metric.by_key(params[:metric])

+      @extension=@extensions.find{|extension| extension.getDefaultTabForMetrics().include?(metric.key)}

+

+    end

+    @extension=@extensions.find{|extension| extension.isDefaultTab()} if @extension==nil

+  end

+

+  def load_sources

+    @period = params[:period].to_i unless params[:period].blank?

+    @expanded=(params[:expand]=='true')

+

+    if @snapshot.source

+      source_lines=Java::OrgSonarServerUi::JRubyFacade.new.colorizeCode(@snapshot.source.data, @snapshot.project.language).split("\n")

+      init_scm()

+

+      @lines=[]

+      source_lines.each_with_index do |source, index|

+        line=Line.new(source)

+        @lines<<line

+

+        line.revision=@revisions_by_line[index+1]

+        line.author=@authors_by_line[index+1]

+

+        date_string=@dates_by_line[index+1]

+        line.datetime=(date_string ? Java::OrgSonarApiUtils::DateUtils.parseDateTime(date_string): nil)

+      end

+    end

+  end

+

+  def init_scm

+    @scm_available=(@snapshot.measure('last_commit_datetimes_by_line')!=nil)

+    @authors_by_line=load_distribution('authors_by_line')

+    @revisions_by_line=load_distribution('revisions_by_line')

+    @dates_by_line=load_distribution('last_commit_datetimes_by_line')

+  end

+

+  def load_distribution(metric_key)

+    m=@snapshot.measure(metric_key)

+    m ? m.data_as_line_distribution() : {}

+  end

+

+  def render_coverage

+    load_sources()

+    @display_coverage=true

+    @expandable=(@lines!=nil)

+    if @lines

+      @hits_by_line=load_distribution('coverage_line_hits_data')

+      @conditions_by_line=load_distribution('conditions_by_line')

+      @covered_conditions_by_line=load_distribution('covered_conditions_by_line')

+

+      @hits_by_line.each_pair do |line_id,hits|

+        line=@lines[line_id-1]

+        if line

+          line.hits=hits.to_i

+          line.conditions=@conditions_by_line[line_id].to_i

+          line.covered_conditions=@covered_conditions_by_line[line_id].to_i

+        end

+      end

+

+      if @snapshot.measure('conditions_by_line').nil?

+        # TODO remove this code when branch_coverage_hits_data is fully removed from CoreMetrics

+        deprecated_branches_by_line=load_distribution('branch_coverage_hits_data')

+        deprecated_branches_by_line.each_pair do |line_id,label|

+          line=@lines[line_id-1]

+          if line

+            line.deprecated_conditions_label=label

+          end

+        end

+      end

+

+      to=(@period ? Java::JavaUtil::Date.new(@snapshot.period_datetime(@period).to_f * 1000) : nil)

+      metric=Metric.by_key(params[:coverage_filter]||params[:metric])

+      @coverage_filter=(metric ? metric.key : 'coverage')

+      @filtered=true

+      if ('lines_to_cover'==@coverage_filter || 'coverage'==@coverage_filter || 'line_coverage'==@coverage_filter ||

+          'new_lines_to_cover'==@coverage_filter || 'new_coverage'==@coverage_filter || 'new_line_coverage'==@coverage_filter)

+        @coverage_filter='lines_to_cover'

+        filter_lines{|line| line.hits && line.after(to)}

+

+      elsif 'uncovered_lines'==@coverage_filter || 'new_uncovered_lines'==@coverage_filter

+        @coverage_filter='uncovered_lines'

+        filter_lines{|line| line.hits && line.hits==0 && line.after(to)}

+

+      elsif 'conditions_to_cover'==@coverage_filter || 'branch_coverage'==@coverage_filter ||

+            'new_conditions_to_cover'==@coverage_filter || 'new_branch_coverage'==@coverage_filter

+        @coverage_filter='conditions_to_cover'

+        filter_lines{|line| line.conditions && line.conditions>0 && line.after(to)}

+

+      elsif 'uncovered_conditions'==@coverage_filter || 'new_uncovered_conditions'==@coverage_filter

+        @coverage_filter='uncovered_conditions'

+        filter_lines{|line| line.conditions && line.covered_conditions && line.covered_conditions<line.conditions && line.after(to)}

+      end

+    end

+    render :action => 'index', :layout => !request.xhr?

+  end

+

+  

+  

+  def render_violations

+    load_sources()

+    @display_violations=true

+    @global_violations=[]

+    @expandable=(@lines!=nil)

+    @filtered=!@expanded

+

+    conditions='snapshot_id=?'

+    values=[@snapshot.id]

+    unless params[:rule].blank?

+      severity=Sonar::RulePriority.id(params[:rule])

+      if severity

+        conditions += ' AND failure_level=?'

+        values<<severity

+      else

+        rule=Rule.by_key_or_id(params[:rule])

+        conditions += ' AND rule_id=?'

+        values<<(rule ? rule.id : -1)

+      end

+    end

+

+    if @period

+      date=@snapshot.period_datetime(@period)

+      if date

+        conditions+=' AND created_at>?'

+        values<<date.advance(:minutes => 1)

+      else

+        conditions+=' AND id=-1'

+      end

+    end

+

+    RuleFailure.find(:all, :include => ['rule', 'reviews' ], :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)

+      else

+        @global_violations<<violation

+      end

+      # if the permanent_id does not exist, set it to the current id

+      unless violation.permanent_id

+        violation.permanent_id = violation.id

+        violation.save

+      end

+    end

+

+    if !@expanded && @lines

+      filter_lines{|line| line.violations?}

+    end

+    render :action => 'index', :layout => !request.xhr?

+  end

+  

+  

+  def render_source

+    load_sources()

+    filter_lines_by_date()

+    render :action => 'index', :layout => !request.xhr?

+  end

+

+  

+  def filter_lines_by_date

+    if @period

+      @filtered=true

+      to=Java::JavaUtil::Date.new(@snapshot.period_datetime(@period).to_f * 1000)

+      if to

+        @lines.each do |line|

+          line.flag_as_hidden() if !line.after(to)

+        end

+      end

+    end

+  end

+

+  def filter_lines(&block)

+    @lines.each_with_index do |line,index|

+      if yield(line)

+        for i in index-4...index

+          @lines[i].flag_as_highlight_context() if i>=0

+        end

+        line.flag_as_highlighted()

+        for i in index+1..index+4

+          @lines[i].flag_as_highlight_context() if i<@lines.size

+        end

+      else

+        line.flag_as_hidden()

+      end

+    end

+  end

+

+  class Line

+    attr_accessor :source, :revision, :author, :datetime, :violations, :hits, :conditions, :covered_conditions, :hidden, :highlighted, :deprecated_conditions_label

+

+    def initialize(source)

+      @source=source

+    end

+

+    def add_violation(violation)

+      @violations||=[]

+      @violations<<violation

+      @visible=true

+    end

+

+    def violations?

+      @violations && @violations.size>0

+    end

+

+    def violation_severity

+      if @violations && @violations.size>0

+        @violations[0].failure_level

+      else

+        nil

+      end

+    end

+

+    def after(date)

+      if date && @datetime

+        @datetime.after(date)

+      else

+        true

+      end

+    end

+

+    def flag_as_highlighted

+      @highlighted=true

+      @hidden=false

+    end

+

+    def flag_as_highlight_context

+      # do not force if highlighted has already been set to true

+      @highlighted=false if @highlighted.nil?

+      @hidden=false

+    end

+

+    def flag_as_hidden

+      # do not force if it has already been flagged as visible

+      if @hidden.nil?

+        @hidden=true

+        @highlighted=false

+      end

+    end

+

+    def hidden?

+      @hidden==true

+    end

+

+    def highlighted?

+      # highlighted if the @highlighted has not been set or has been set to true

+      !hidden? && @highlighted!=false

+    end

+

+    def deprecated_conditions_label=(label)

+      if label

+        @deprecated_conditions_label=label

+          if label=='0%'

+            @conditions=2

+            @covered_conditions=0

+          elsif label=='100%'

+            @conditions=2

+            @covered_conditions=2

+          else

+            @conditions=2

+            @covered_conditions=1

+        end

+      end

+    end

+  end

+

+  def render_extension()

+    render :action => 'extension', :layout => !request.xhr?

+  end

+

+  def render_nothing()

+    render :action => 'nothing', :layout => !request.xhr?

   end
 end
\ No newline at end of file
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 14d23c6a505..cc41f63549a 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
@@ -34,13 +34,13 @@ class ReviewsController < ApplicationController
   end

 

   def list

-    reviews = find_reviews_for_rule_failure params[:rule_failure_id]

+    reviews = find_reviews_for_rule_failure params[:rule_failure_permanent_id]

     render :partial => "list", :locals => { :reviews => reviews }

   end

 

   def form

     @review = Review.new

-    @review.rule_failure_id = params[:violation_id]

+    @review.rule_failure_permanent_id = params[:rule_failure_permanent_id]

     @review.user = current_user

     @review.severity = Review.default_severity

     @review_comment = ReviewComment.new

@@ -53,12 +53,12 @@ class ReviewsController < ApplicationController
     @review_comment.user = current_user

     @review_comment.review_id = params[:review_id]

     @review_comment.review_text = ""

-    @rule_failure_id = params[:rule_failure_id]

+    @rule_failure_permanent_id = params[:rule_failure_permanent_id]

     render :partial => "form_comment"

   end

 

   def create

-    unless has_rights_to_create? params[:review][:rule_failure_id]

+    unless has_rights_to_create? params[:review][:rule_failure_permanent_id]

       render :text => "<b>Cannot create the review</b> : access denied."

       return

     end

@@ -67,28 +67,34 @@ class ReviewsController < ApplicationController
     @review.user = current_user

     @review.status = Review.default_status

     @review.review_type = Review.default_type

+    @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?

       @review.save

-      @reviews = find_reviews_for_rule_failure @review.rule_failure_id

+      @reviews = find_reviews_for_rule_failure @review.rule_failure_permanent_id

     end

     render "create_result"

   end

 

   def create_comment

-    unless has_rights_to_create? params[:rule_failure_id]

+    unless has_rights_to_create? params[:rule_failure_permanent_id]

       render :text => "<b>Cannot create the comment</b> : access denied."

       return

     end

 

     @review_comment = ReviewComment.new(params[:review_comment])

     @review_comment.user = current_user

-    @rule_failure_id = params[:rule_failure_id]

+    @rule_failure_permanent_id = params[:rule_failure_permanent_id]

     if @review_comment.valid?

       @review_comment.save

-      @reviews = find_reviews_for_rule_failure @rule_failure_id

+      # -- 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 --

+      @reviews = find_reviews_for_rule_failure @rule_failure_permanent_id

     end

     render "create_comment_result"

   end

@@ -143,14 +149,18 @@ class ReviewsController < ApplicationController
     @reviews = Review.find( :all, :order => "created_at DESC", :joins => :review_comments, :conditions => [ conditions.join(" AND "), values] ).uniq

   end

 

-  def find_reviews_for_rule_failure ( rule_failure_id )

-    return Review.find :all, :conditions => ['rule_failure_id=?', rule_failure_id]

+  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 has_rights_to_create? ( rule_failure_id )

-    return false unless current_user

+  def find_rule_failure_with_permanent_id ( rule_failure_permanent_id )

+    return RuleFailure.last( :all, :conditions => [ "permanent_id = ?", rule_failure_permanent_id ], :include => ['snapshot'] )

+  end

 

-    project = RuleFailure.find( rule_failure_id, :include => ['snapshot'] ).snapshot.root_project

+  def has_rights_to_create? ( rule_failure_permanent_id )

+    return false unless current_user

+    

+    project = find_rule_failure_with_permanent_id( rule_failure_permanent_id).snapshot.root_project

     unless has_role?(:user, project)

       return false

     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 3cac05b8463..27e1ec25401 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
@@ -1,75 +1,75 @@
-#
-# Sonar, entreprise quality control tool.
-# Copyright (C) 2008-2011 SonarSource
-# mailto:contact AT sonarsource DOT com
-#
-# Sonar is free software; you can redistribute it and/or
-# modify it under the terms of the GNU Lesser General Public
-# License as published by the Free Software Foundation; either
-# version 3 of the License, or (at your option) any later version.
-#
-# Sonar is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-# Lesser General Public License for more details.
-#
-# You should have received a copy of the GNU Lesser General Public
-# License along with {library}; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
-#
-
-class RuleFailure < ActiveRecord::Base
-
-  belongs_to :rule
-  belongs_to :snapshot
-  has_many :reviews, :order => "created_at"
-
-  def to_hash_json
-    json = {}
-    json['message'] = message
-    json['line'] = line if line
-    json['priority'] = Sonar::RulePriority.to_s(failure_level).upcase
-    if created_at
-      json['createdAt'] = format_datetime(created_at)
-    end
-    json['rule'] = {
-      :key => rule.key,
-      :name => rule.name
-    }
-    json['resource'] = {
-      :key => snapshot.project.key,
-      :name => snapshot.project.name,
-      :scope => snapshot.project.scope,
-      :qualifier => snapshot.project.qualifier,
-      :language => snapshot.project.language
-    }
-    json
-  end
-
-  def to_xml(xml=Builder::XmlMarkup.new(:indent => 0))
-    xml.violation do
-      xml.message(message)
-      xml.line(line) if line
-      xml.priority(Sonar::RulePriority.to_s(failure_level))
-      if created_at
-        xml.createdAt(format_datetime(created_at))
-      end
-      xml.rule do
-        xml.key(rule.key)
-        xml.name(rule.name)
-      end
-      xml.resource do
-        xml.key(snapshot.project.key)
-        xml.name(snapshot.project.name)
-        xml.scope(snapshot.project.scope)
-        xml.qualifier(snapshot.project.qualifier)
-        xml.language(snapshot.project.language)
-      end
-    end
-  end
-
-  def format_datetime(datetime)
-    datetime.strftime("%Y-%m-%dT%H:%M:%S%z")
-  end
-
-end
+#

+# Sonar, entreprise quality control tool.

+# Copyright (C) 2008-2011 SonarSource

+# mailto:contact AT sonarsource DOT com

+#

+# Sonar is free software; you can redistribute it and/or

+# modify it under the terms of the GNU Lesser General Public

+# License as published by the Free Software Foundation; either

+# version 3 of the License, or (at your option) any later version.

+#

+# Sonar is distributed in the hope that it will be useful,

+# but WITHOUT ANY WARRANTY; without even the implied warranty of

+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

+# Lesser General Public License for more details.

+#

+# You should have received a copy of the GNU Lesser General Public

+# License along with {library}; if not, write to the Free Software

+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02

+#

+

+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"

+

+  def to_hash_json

+    json = {}

+    json['message'] = message

+    json['line'] = line if line

+    json['priority'] = Sonar::RulePriority.to_s(failure_level).upcase

+    if created_at

+      json['createdAt'] = format_datetime(created_at)

+    end

+    json['rule'] = {

+      :key => rule.key,

+      :name => rule.name

+    }

+    json['resource'] = {

+      :key => snapshot.project.key,

+      :name => snapshot.project.name,

+      :scope => snapshot.project.scope,

+      :qualifier => snapshot.project.qualifier,

+      :language => snapshot.project.language

+    }

+    json

+  end

+

+  def to_xml(xml=Builder::XmlMarkup.new(:indent => 0))

+    xml.violation do

+      xml.message(message)

+      xml.line(line) if line

+      xml.priority(Sonar::RulePriority.to_s(failure_level))

+      if created_at

+        xml.createdAt(format_datetime(created_at))

+      end

+      xml.rule do

+        xml.key(rule.key)

+        xml.name(rule.name)

+      end

+      xml.resource do

+        xml.key(snapshot.project.key)

+        xml.name(snapshot.project.name)

+        xml.scope(snapshot.project.scope)

+        xml.qualifier(snapshot.project.qualifier)

+        xml.language(snapshot.project.language)

+      end

+    end

+  end

+

+  def format_datetime(datetime)

+    datetime.strftime("%Y-%m-%dT%H:%M:%S%z")

+  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 476a0383255..3a599ba11e6 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
@@ -10,10 +10,10 @@
       <td style="width: 30px; vertical-align: top; text-align: center;">

       	<% if current_user && violation.reviews.blank? %>

           <%= link_to_remote image_tag("reviews/+review.png"),  

-        		:url => { :controller => "reviews", :action => "form", :violation_id => violation.id },

-        		:update => "reviewFailure" + violation.id.to_s,

-        		:html => { :id => "createReviewLink" + violation.id.to_s, :title => "Add a review" },

-        		:complete => "$('reviewFailure" + violation.id.to_s + "').style.display='';$('reviewText').focus();" -%>

+        		:url => { :controller => "reviews", :action => "form", :rule_failure_permanent_id => violation.permanent_id },

+        		:update => "reviewFailure" + violation.permanent_id.to_s,

+        		:html => { :id => "createReviewLink" + violation.permanent_id.to_s, :title => "Add a review" },

+        		:complete => "$('reviewFailure" + violation.permanent_id.to_s + "').style.display='';$('reviewText').focus();" -%>

       	<% end %>

       	<% unless violation.reviews.blank? %>

       	  <%= image_tag("reviews/review.png") -%>

@@ -38,7 +38,7 @@
     

     <tr>

       <td colspan="2">

-		<div id="reviewFailure<%= violation.id -%>" style="margin-left: 13px; padding: 10px 5px 5px 10px; border-left: 2px solid #EFEFEF; display:<%= displayReviewFailureDiv -%>">

+		<div id="reviewFailure<%= violation.permanent_id -%>" style="margin-left: 13px; padding: 10px 5px 5px 10px; border-left: 2px solid #EFEFEF; display:<%= displayReviewFailureDiv -%>">

 		  <%= render :partial => "reviews/list", :locals => { :reviews => violation.reviews } %>

 		</div>

       </td>

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
index 375e4fa0873..d31edd0dca2 100644
--- 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
@@ -1,6 +1,6 @@
 <b>Create a new review</b>

 <% form_for :review, @review do |f| %>

-      <%= f.hidden_field :rule_failure_id %>

+      <%= f.hidden_field :rule_failure_permanent_id %>

       Severity: 

       <%= select_tag "review[severity]", options_for_select(Review.severity_options, @review.severity)  %>

       <br/>

@@ -9,7 +9,7 @@
       

       <%= submit_to_remote "create_btn", "Create review", :url => { :action => 'create' } %>

       <input type="button" name="cancel_btn" value="Cancel" 

-             onclick="new Ajax.Updater({success:'reviewFailure<%= @review.rule_failure_id.to_s -%>'}, '<%= ApplicationController.root_context -%>/reviews/list?rule_failure_id=<%= @review.rule_failure_id.to_s -%>', {asynchronous:true, evalScripts:true, parameters:Form.serialize(this.form), }); $('reviewFailure<%= @review.rule_failure_id.to_s -%>').style.display='none';">

+             onclick="new Ajax.Updater({success:'reviewFailure<%= @review.rule_failure_permanent_id.to_s -%>'}, '<%= ApplicationController.root_context -%>/reviews/list?rule_failure_permanent_id=<%= @review.rule_failure_permanent_id.to_s -%>', {asynchronous:true, evalScripts:true, parameters:Form.serialize(this.form), }); $('reviewFailure<%= @review.rule_failure_permanent_id.to_s -%>').style.display='none';">

 

       <%= f.error_messages :header_message => "Can't create the review", :message => "", :header_tag => :h3 %>

       

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
index 6cf80541e2a..b0da98bae31 100644
--- 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
@@ -1,15 +1,15 @@
-<% form_for :review_comment, @review_comment do |f| %>
-      <%= f.hidden_field :review_id %>
-      <%= f.text_area :review_text, :rows => 8, 
-            :id => "commentText" + @review_comment.review_id.to_s,
-            :style => "width:100%" %>
-      <br/>
-      <%= submit_to_remote 'create_btn', 'Add comment', 
-                     :url => { :action => 'create_comment', :rule_failure_id => @rule_failure_id } %>
-      <%= submit_to_remote 'cancel_btn', 'Cancel', 
-                     :url => { :action => 'list', :rule_failure_id => @rule_failure_id.to_s },
-                     :update => { :success => "reviewFailure" + @rule_failure_id.to_s } %>
-      
-      <%= f.error_messages :header_message => "Can't save the comment", :message => "", :header_tag => :h3%>
-      
+<% form_for :review_comment, @review_comment do |f| %>

+      <%= f.hidden_field :review_id %>

+      <%= f.text_area :review_text, :rows => 8, 

+            :id => "commentText" + @review_comment.review_id.to_s,

+            :style => "width:100%" %>

+      <br/>

+      <%= submit_to_remote 'create_btn', 'Add comment', 

+                     :url => { :action => 'create_comment', :rule_failure_permanent_id => @rule_failure_permanent_id } %>

+      <%= submit_to_remote 'cancel_btn', 'Cancel', 

+                     :url => { :action => 'list', :rule_failure_permanent_id => @rule_failure_permanent_id.to_s },

+                     :update => { :success => "reviewFailure" + @rule_failure_permanent_id.to_s } %>

+      

+      <%= f.error_messages :header_message => "Can't save the comment", :message => "", :header_tag => :h3%>

+      

 <% end %>
\ 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
index b93300505bd..639a3f036fb 100644
--- 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
@@ -28,7 +28,7 @@
         <% if current_user %>

            <div style="text-align: right; padding: 5px">

              <%= link_to_remote "Add a comment", 

-        		:url => { :controller => "reviews", :action => "form_comment", :review_id => review.id, :rule_failure_id => review.rule_failure_id },

+        		: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>

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
index 0c4c474c601..6eae877ae47 100644
--- 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
@@ -1,5 +1,5 @@
-if @reviews
-  page.replace_html "reviewFailure" + @rule_failure_id.to_s, :partial => "list", :locals => { :reviews => @reviews }
-else
-  page.replace_html "createComment" + @review_comment.review_id.to_s, :partial => "form_comment"
+if @reviews

+  page.replace_html "reviewFailure" + @rule_failure_permanent_id.to_s, :partial => "list", :locals => { :reviews => @reviews }

+else

+  page.replace_html "createComment" + @review_comment.review_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
index aa8ac5ad618..5f802780b34 100644
--- 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
@@ -1,6 +1,6 @@
 if @reviews

-  page.replace_html "createReviewLink" + @review.rule_failure_id.to_s, image_tag("reviews/review.png")

-  page.replace_html "reviewFailure" + @review.rule_failure_id.to_s, :partial => "list", :locals => { :reviews => @reviews }

+  page.replace_html "createReviewLink" + @review.rule_failure_permanent_id.to_s, image_tag("reviews/review.png")

+  page.replace_html "reviewFailure" + @review.rule_failure_permanent_id.to_s, :partial => "list", :locals => { :reviews => @reviews }

 else

-  page.replace_html "reviewFailure" + @review.rule_failure_id.to_s, :partial => "form"

+  page.replace_html "reviewFailure" + @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/db/migrate/191_create_review.rb b/sonar-server/src/main/webapp/WEB-INF/db/migrate/191_create_review.rb
index 4978f21e5c5..467e8c63fcd 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
@@ -25,19 +25,20 @@ class CreateReview < ActiveRecord::Migration
 

   def self.up

     create_table 'reviews' do |t|

-      t.column 'created_at', 		:datetime

-      t.column 'updated_at', 		:datetime

-      t.column 'user_id', 			:integer, 	:null => true

-      t.column 'review_type', 		:string, 	:null => true,	:limit => 10

-      t.column 'status', 			:string, 	:null => true,	:limit => 10

-      t.column 'severity', 			:string, 	:null => true,	:limit => 10

-      t.column 'rule_failure_id', 	:integer, 	:null => true   

-      t.column 'resource_id', 		:integer, 	:null => true   

-      t.column 'resource_line', 	:integer, 	:null => true      

+      t.column 'created_at', 					:datetime

+      t.column 'updated_at', 					:datetime

+      t.column 'user_id', 						:integer, 	:null => true

+      t.column 'review_type', 					:string, 	:null => true,	:limit => 10

+      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   

+      t.column 'resource_id', 					:integer, 	:null => true   

+      t.column 'resource_line', 				:integer, 	:null => true      

     end

     

     create_table 'review_comments' do |t|

       t.column 'created_at', 		:datetime

+      t.column 'updated_at', 		:datetime

       t.column 'review_id', 		:integer

       t.column 'user_id', 			:integer, 	:null => true

       t.column 'review_text', 		:text, 		:null => true