SONAR-2327 split actions for resource viewer and reviews page

This commit is contained in:
simonbrandhof 2011-04-25 23:12:50 +02:00
parent e174628696
commit dbc988359b
26 changed files with 420 additions and 587 deletions

View File

@ -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);

View File

@ -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)";
}
}

View File

@ -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);

View File

@ -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"/>

View File

@ -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]"/>

View File

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

View File

@ -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."
return
end
@review = Review.new(params[:review])
@review.user = current_user
if params[:assign_to_me]
@review.assignee = current_user
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
end
@review.save
@violation = rule_failure
end
render "create_result"
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]
end
render :partial => "form_comment"
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
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_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
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
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]
# POST
def violation_assign
violation = RuleFailure.find(params[: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
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
def close_review
review = Review.find params[:review_id]
unless current_user
render :text => "<b>Cannot edit the review</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 }
# GET
def violation_false_positive_form
render :partial => 'reviews/violation_false_positive_form'
end
def switch_on_violation
rule_failure = RuleFailure.find params[:rule_failure_id]
unless has_rights_to_modify? rule_failure
# 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
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 }
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
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 :partial => "resource/violation", :locals => { :violation => violation }
end
# 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 => 'reviews/violation_comment_form'
end
# 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)
unless violation.review
violation.create_review!(
:assignee => (params['assign_to_me']=='me' ? current_user : nil),
:user => current_user)
end
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
render :partial => "resource/violation", :locals => { :violation => violation }
end
# 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
sanitize_violation(violation)
if violation.review
comment=violation.review.comments.find(params[:comment_id].to_i)
comment.delete if comment
end
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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -1,11 +0,0 @@
<%
unless reviews.blank?
reviews.each do |review|
%>
<%= render :partial => "reviews/view", :locals => { :review => review } %>
<%
end
end
%>

View File

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

View File

@ -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} -%>

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -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>
<option value="<%= Review::STATUS_CLOSED -%>" class="status_closed" <%= 'selected' if @statuses.include?(Review::STATUS_CLOSED) -%>>Closed</option>
</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>
</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>
</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>
<table id="reviews-form" class="searchFilterBox">
<thead>
<tr>
<th colspan="6"></th>
</tr>
<%
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>
</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>
</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 %>
<%
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>
<%
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>

View File

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

View File

@ -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');