[SONAR-1973] Add title & assignee on review, improve UI

6 files changed, 134 insertions, 98 deletions

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 cc41f63549a..05151fca017 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
@@ -39,10 +39,13 @@ class ReviewsController < ApplicationController
   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 = params[:rule_failure_permanent_id]

+    @review.rule_failure_permanent_id = rule_failure.permanent_id

     @review.user = current_user

-    @review.severity = Review.default_severity

+    @review.assignee = current_user

+    @user_options = add_all_users []

+    @review.title = rule_failure.message

     @review_comment = ReviewComment.new

     @review_comment.review_text = ""

     render :partial => "form"

@@ -58,7 +61,8 @@ class ReviewsController < ApplicationController
   end

 

   def create

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

+    rule_failure = find_last_rule_failure_with_permanent_id params[:review][:rule_failure_permanent_id]

+    unless has_rights_to_create? rule_failure

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

       return

     end

@@ -67,6 +71,7 @@ class ReviewsController < ApplicationController
     @review.user = current_user

     @review.status = Review.default_status

     @review.review_type = Review.default_type

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

@@ -74,12 +79,15 @@ class ReviewsController < ApplicationController
     if @review.valid?

       @review.save

       @reviews = find_reviews_for_rule_failure @review.rule_failure_permanent_id

+    else

+      @user_options = add_all_users []

     end

     render "create_result"

   end

 

   def create_comment

-    unless has_rights_to_create? params[:rule_failure_permanent_id]

+    rule_failure = find_last_rule_failure_with_permanent_id params[:rule_failure_permanent_id]

+    unless has_rights_to_create? rule_failure

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

       return

     end

@@ -109,14 +117,19 @@ class ReviewsController < ApplicationController
       @user_names << ["Me", current_user.id]

       default_user = [current_user.id]

     end

-    User.find( :all ).each do |user|

-      @user_names << [user.name, user.id.to_s]

-    end

+    add_all_users @user_names

     @review_authors = filter_any(params[:review_authors]) || default_user

     @comment_authors = filter_any(params[:comment_authors]) || default_user

     @severities = filter_any(params[:severities]) || [""]

     @statuses = filter_any(params[:statuses]) || ["open"]

   end

+  

+  def add_all_users ( user_options )

+    User.find( :all ).each do |user|

+      user_options << [user.name, user.id.to_s]

+    end

+    return user_options

+  end

 

   def filter_any(array)

     if array && array.size>1 && array.include?("")

@@ -153,14 +166,14 @@ class ReviewsController < ApplicationController
     return Review.find :all, :conditions => ['rule_failure_permanent_id=?', rule_failure_permanent_id]

   end

 

-  def find_rule_failure_with_permanent_id ( rule_failure_permanent_id )

+  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_create? ( rule_failure_permanent_id )

+  def has_rights_to_create? ( rule_failure )

     return false unless current_user

     

-    project = find_rule_failure_with_permanent_id( rule_failure_permanent_id).snapshot.root_project

+    project = rule_failure.snapshot.root_project

     unless has_role?(:user, project)

       return false

     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 6fc2db26717..9c6cb791852 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
@@ -1,71 +1,73 @@
-#
-# 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 Review < ActiveRecord::Base
-  belongs_to :user
-  belongs_to :rule_failure
-  belongs_to :resource, :class_name => "Project", :foreign_key => "resource_id"
-  has_many :review_comments, :order => "created_at", :dependent => :destroy
-  validates_presence_of :user, :message => "can't be empty"
-  validates_presence_of :review_type, :message => "can't be empty"
-  validates_presence_of :status, :message => "can't be empty"
-
-  SEVERITY_INFO = "info"
-  SEVERITY_MINOR = "minor"
-  SEVERITY_MAJOR = "major"
-  SEVERITY_CRITICAL = "critical"
-  SEVERITY_BLOCKER = "blocker"
-  
-  TYPE_COMMENTS = "comments"
-  TYPE_FALSE_POSITIVE = "f-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
-  
-  def self.severity_options
-    severity_ops = []
-    severity_ops << ["Info", SEVERITY_INFO]
-    severity_ops << ["Minor", SEVERITY_MINOR]
-    severity_ops << ["Major", SEVERITY_MAJOR]
-    severity_ops << ["Critical", SEVERITY_CRITICAL]
-    severity_ops << ["Blocker", SEVERITY_BLOCKER]
-    return severity_ops
-  end
-  
-  def self.status_options
-    status_ops = []
-    status_ops << ["Open", STATUS_OPEN]
-    status_ops << ["Closed", STATUS_CLOSED]
-    return status_ops
-  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 Sonar; if not, write to the Free Software

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

+#

+class Review < ActiveRecord::Base

+  belongs_to :user

+  belongs_to :assignee, :class_name => "User", :foreign_key => "assignee_id"

+  belongs_to :resource, :class_name => "Project", :foreign_key => "resource_id"

+  has_many :review_comments, :order => "created_at", :dependent => :destroy

+  validates_presence_of :user, :message => "can't be empty"

+  validates_presence_of :assignee, :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"

+

+  SEVERITY_INFO = "INFO"

+  SEVERITY_MINOR = "MINOR"

+  SEVERITY_MAJOR = "MAJOR"

+  SEVERITY_CRITICAL = "CRITICAL"

+  SEVERITY_BLOCKER = "BLOCKER"

+  

+  TYPE_COMMENTS = "comments"

+  TYPE_FALSE_POSITIVE = "f-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

+  

+  def self.severity_options

+    severity_ops = []

+    severity_ops << ["Info", SEVERITY_INFO]

+    severity_ops << ["Minor", SEVERITY_MINOR]

+    severity_ops << ["Major", SEVERITY_MAJOR]

+    severity_ops << ["Critical", SEVERITY_CRITICAL]

+    severity_ops << ["Blocker", SEVERITY_BLOCKER]

+    return severity_ops

+  end

+  

+  def self.status_options

+    status_ops = []

+    status_ops << ["Open", STATUS_OPEN]

+    status_ops << ["Closed", STATUS_CLOSED]

+    return status_ops

+  end

+

+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
index d31edd0dca2..16e214941c6 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,8 +1,10 @@
-<b>Create a new review</b>

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

       <%= f.hidden_field :rule_failure_permanent_id %>

-      Severity: 

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

+      <b>New review</b>: 

+      <%= f.text_field :title, :size => 100 %>

+      <br/>

+      Assignee: 

+      <%= select_tag "review[assignee_id]", options_for_select(@user_options, @review.assignee.id.to_s)  %>

       <br/>

       <%= text_area :review_comment, :review_text, :id => "reviewText", :rows => 8, :style => "width:100%" %>

       <br/>

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 b0da98bae31..f15ed482af9 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
@@ -4,7 +4,7 @@
             :id => "commentText" + @review_comment.review_id.to_s,

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

       <br/>

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

+      <%= submit_to_remote 'create_btn', 'Post', 

                      :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 },

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 639a3f036fb..41788df91b9 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
@@ -1,33 +1,50 @@
+<%

+  last_comment_user_id = -1

+%>

   <div id="review<%= review.id -%>">

 	  <div>

-      	<b>Review initiated by <%= h(review.user.name) -%></b> - <%= l review.created_at -%>

-        <br/>

-        Status: <%= h(review.status) -%> / Severity: <%= h(review.severity) -%>      

+      	<b>Review #<%= review.id -%> - <%= h(review.title) -%></b>

+      	<br/>

+      	<i>Assigned to <%= h(review.assignee.name) -%></i>

       </div>

       

-      <div style="margin-top: 10px">

-        <table style="width:100%">

+      <div style="margin-top: 10px; margin-left: 20px">

         <% unless review.review_comments.blank? 

 		     review.review_comments.each do |review_comment|

+		       last_comment_user_id = review_comment.user_id

 		%>

-		  <tr>

-		    <td style="width:180px; vertical-align:top; padding: 5px; border:solid 1px grey; background-color: #F7F7F7">

-				<%= image_tag("user.png") -%> <b><%= h(review_comment.user.name) -%></b> »

-				<br/><%= l review_comment.created_at -%>

+        <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: 5px; border:solid 1px grey; background: white">

-		    	<%= h(review_comment.review_text) -%>

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

+		      <%= h(review_comment.review_text) -%>

+		    </td>

+		  </tr>

+		</table>

 		<% 

 		     end

 		   end

 		%>

-		</table>

       

         <% if current_user %>

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

-             <%= link_to_remote "Add a comment", 

+             <% if current_user.id == last_comment_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 },

+                  :update => "createComment" + 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()" -%>

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 467e8c63fcd..a1abf37a7ac 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
@@ -28,6 +28,8 @@ class CreateReview < ActiveRecord::Migration
       t.column 'created_at', 					:datetime

       t.column 'updated_at', 					:datetime

       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 'status', 						:string, 	:null => true,	:limit => 10

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