From 70ee50737cb8a03ef51cedaa6158142be93ede49 Mon Sep 17 00:00:00 2001 From: Fabrice Bellingard Date: Thu, 14 Apr 2011 18:17:57 +0200 Subject: [PATCH] [SONAR-1973] Add title & assignee on review, improve UI --- .../app/controllers/reviews_controller.rb | 33 ++-- .../main/webapp/WEB-INF/app/models/review.rb | 144 +++++++++--------- .../WEB-INF/app/views/reviews/_form.html.erb | 8 +- .../app/views/reviews/_form_comment.html.erb | 2 +- .../WEB-INF/app/views/reviews/_view.html.erb | 43 ++++-- .../WEB-INF/db/migrate/191_create_review.rb | 2 + 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 => "Cannot create the review : 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 => "Cannot create the comment : 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 @@ -Create a new review <% 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) %> + New review: + <%= f.text_field :title, :size => 100 %> +
+ Assignee: + <%= select_tag "review[assignee_id]", options_for_select(@user_options, @review.assignee.id.to_s) %>
<%= text_area :review_comment, :review_text, :id => "reviewText", :rows => 8, :style => "width:100%" %>
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%" %>
- <%= 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 +%>
- Review initiated by <%= h(review.user.name) -%> - <%= l review.created_at -%> -
- Status: <%= h(review.status) -%> / Severity: <%= h(review.severity) -%> + Review #<%= review.id -%> - <%= h(review.title) -%> +
+ Assigned to <%= h(review.assignee.name) -%>
-
- +
<% unless review.review_comments.blank? review.review_comments.each do |review_comment| + last_comment_user_id = review_comment.user_id %> -
-
- <%= image_tag("user.png") -%> <%= h(review_comment.user.name) -%> » -
<%= l review_comment.created_at -%> + + + - + + + +
+ <%= image_tag("user.png") -%> <%= h(review_comment.user.name) -%> - <%= h(review_comment.review_text) -%> + + <%= l review_comment.created_at -%>
+ <%= h(review_comment.review_text) -%> +
<% end end %> -
<% if current_user %>
- <%= 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()" -%> +   + <% 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 -- 2.39.5