diff options
13 files changed, 322 insertions, 532 deletions
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb index e45d559f3dc..a18a26f72bc 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb @@ -23,67 +23,78 @@ require 'json' class Api::ReviewsController < Api::ApiController # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html) - verify :method => :put, :only => [ :update ] + verify :method => :put, :only => [ :add_comment, :reassign, :resolve, :reopen ] verify :method => :post, :only => [ :create ] - verify :method => :delete, :only => [ :destroy ] - + #verify :method => :delete, :only => [ :destroy ] + def index - convert_markdown=(params[:output]=='HTML') reviews=select_authorized(:user, Review.search(params), :project) - - render_reviews(reviews, convert_markdown) + + render_reviews(reviews, params[:output] == 'HTML') end # - # --- Creation of a review --- + # --- Creation of a review --- # # POST /api/reviews # Required parameters: # - 'violation_id' : the violation on which the review shoul be created - # - 'text' : the text of the comment + # - 'status' : the initial status (can be 'OPEN' or 'RESOLVED') + # - 'comment' : the text of the comment # # Optional parameters : - # - 'assignee' : login used to create a review directly assigned - # - 'false_positive' : if "true", creates a false-positive review + # - 'resolution' : if status 'RESOLVED', then resolution must be provided (can be 'FIXED' or 'FALSE-POSITIVE') + # - 'assignee' : login used to create a review directly assigned # # Example : - # - POST "/api/reviews/?violation_id=1&assignee=fabrice&text=Hello%20World! - # - POST "/api/reviews/?violation_id=2&false_positive=true&text=No%20violation%20here + # - POST "/api/reviews/?violation_id=1&status=OPEN&assignee=admin&comment=Please%20fix%20this" + # - POST "/api/reviews/?violation_id=2&status=RESOLVED&resolution=FALSE-POSITIVE&comment=No%20violation%20here" + # - POST "/api/reviews/?violation_id=3&status=RESOLVED&resolution=FIXED&assignee=admin&comment=This%20violation%20was%20fixed%20by%20me" # def create begin # 1- Get some parameters and check some of them convert_markdown=(params[:output]=='HTML') - false_positive = params[:false_positive]=='true' assignee = find_user(params[:assignee]) - text = params[:text] - raise "No 'text' parameter has been provided." unless text && !text.blank? - - # 2- Create the review - if params[:violation_id] - violation = RuleFailure.find(params[:violation_id], :include => ['snapshot', 'review']) + status = params[:status] + resolution = params[:resolution] + comment = params[:comment] || request.raw_post + violation_id = params[:violation_id] + raise "No 'violation_id' parameter has been provided." unless violation_id && !violation_id.blank? + raise "No 'status' parameter has been provided." unless status && !status.blank? + raise "No 'comment' parameter has been provided." unless comment && !comment.blank? + + Review.transaction do + # 2- Create the review + violation = RuleFailure.find(violation_id, :include => ['snapshot', 'review']) unless has_rights_to_modify?(violation.snapshot) access_denied return end - raise "Violation #"+violation.id.to_s+" already has a review." if violation.review + raise "Violation #" + violation.id.to_s + " already has a review." if violation.review sanitize_violation(violation) violation.create_review!(:assignee => assignee, :user => current_user) - else - raise "No 'violation_id' parameter has been provided." - end - - # 3- Create the comment and handle false-positive - review = violation.review - if false_positive - review.set_false_positive(false_positive, :user => current_user, :text => text, :violation_id => params[:violation_id]) - else - review.create_comment(:user => current_user, :text => text) + review = violation.review + + # 3- Set status + if status == Review::STATUS_OPEN + review.create_comment(:user => current_user, :text => comment) + elsif status == Review::STATUS_RESOLVED + if resolution == 'FALSE-POSITIVE' + review.set_false_positive(true, :user => current_user, :text => comment, :violation_id => violation_id) + elsif resolution == 'FIXED' + review.create_comment(:user => current_user, :text => comment) + review.resolve + else + raise "Incorrect resolution." + end + else + raise "Incorrect status." + end + + # 4- And finally send back the review + render_reviews([review], convert_markdown) end - - # 4- And finally send back the review - render_reviews([review], convert_markdown) - rescue ApiException => e render_error(e.msg, e.code) @@ -91,136 +102,160 @@ class Api::ReviewsController < Api::ApiController render_error(e.message, 400) end end - - + # - # --- Update a review = reassign, flag as false positive, add/edit a comment --- + # --- Add comment --- # - # PUT /api/reviews + # PUT /api/reviews/add_comment # Required parameters: # - 'id' : the review id - # - # Optional parameters : - # - 'text' : used to edit the text of the last comment (if the last comment belongs to the current user) - # - 'new_text' : used to add a comment - # - 'assignee' : login used to create a review directly assigned. Use 'none' to unassign. - # - 'false_positive' (true/false) : in conjunction with 'new_text' (mandatory in this case), changes the - # state 'false_positive' of the review - # - 'status' : used to change the status of the review. For the moment, only "RESOLVED" or "REOPENED" are accepted. + # - 'comment' : the text of the comment # # Example : - # - PUT "/api/reviews/?id=1&false_positive=true&new_text=Because - # - PUT "/api/reviews/?id=1&assignee=fabrice - # - PUT "/api/reviews/?id=1&assignee=none - # - PUT "/api/reviews/?id=1&new_text=New%20Comment! - # - PUT "/api/reviews/?id=1&text=Modified%20Comment! - # - PUT "/api/reviews/?id=1&status=RESOLVED - # - def update + # - PUT "/api/reviews/add_comment/1?comment=New%20Comment!" + # + def add_comment begin - # 1- Get some parameters - convert_markdown=(params[:output]=='HTML') - text = params[:text] - new_text = params[:new_text] - assignee = params[:assignee] - false_positive = params[:false_positive] - status = params[:status] - - # 2- Get the review or create one - raise "No 'id' parameter has been provided." unless params[:id] - review = Review.find(params[:id], :include => ['project']) - unless has_rights_to_modify?(review.project) - access_denied - return + review = get_review(params[:id]) + review.transaction do + comment = params[:comment] || request.raw_post + if review.isClosed? + raise "Closed review can not be commented." + end + raise "Comment must be provided." unless comment && !comment.blank? + review.create_comment(:user => current_user, :text => comment) end - - # 3- Modify the review - if !false_positive.blank? - raise "'false_positive' parameter must be either 'true' or 'false'." unless false_positive=='true' || false_positive=='false' - raise "Review 'false_positive' status is already " + false_positive if review.false_positive == false_positive - raise "'new_text' parameter is mandatory with 'false_positive'." if new_text.blank? - review.set_false_positive(false_positive=='true', :user => current_user, :text => new_text) - elsif !assignee.blank? - if (assignee=='none') + render_reviews([review], params[:output] == 'HTML') + rescue ApiException => e + render_error(e.msg, e.code) + rescue Exception => e + render_error(e.message, 400) + end + end + + # + # --- Reassign --- + # + # PUT /api/reviews/reassign + # Required parameters: + # - 'id' : the review id + # - 'assignee' : new assignee + # + # Example : + # - PUT "/api/reviews/reassign/1?assignee=fabrice" + # - PUT "/api/reviews/reassign/1?assignee=" + # + def reassign + begin + review = get_review(params[:id]) + review.transaction do + assignee = params[:assignee] + if !review.isOpen? && !review.isReopened? + raise "Only open review can be reassigned." + end + if assignee.blank? user = nil else user = find_user(assignee) - raise "No user found with the following login: "+assignee unless user + raise "Assignee not found." unless user end review.reassign(user) - elsif !new_text.blank? - review.create_comment(:user => current_user, :text => new_text) - elsif !text.blank? - raise "You don't have the rights to edit the last comment of this review." unless review.comments.last.user == current_user - review.edit_last_comment(text) - elsif !status.blank? - raise "Only 'RESOLVED' or 'REOPENED' can be used as a new status for a review." unless status=='RESOLVED' || status=='REOPENED' - review.status=status - review.save! - else - render_error("The given parameter combination is invalid for updating the review.", 403) - return end - - # 4- And finally send back the review - render_reviews([review], convert_markdown) - + render_reviews([review], params[:output] == 'HTML') rescue ApiException => e render_error(e.msg, e.code) + rescue Exception => e + render_error(e.message, 400) + end + end + # + # --- Resolve --- + # + # PUT /api/reviews/resolve + # Required parameters: + # - 'id' : the review id + # - 'resolution' : can be 'FIXED' or 'FALSE-POSITIVE' + # - 'comment' : the text of the comment + # + # Example : + # - PUT "/api/reviews/resolve/1?resolution=FALSE-POSITIVE&comment=No%20violation%20here" + # - PUT "/api/reviews/resolve/1?resolution=FIXED" + # - PUT "/api/reviews/resolve/1?resolution=FIXED&comment=This%20violation%20was%20fixed%20by%20me" + # + def resolve + begin + review = get_review(params[:id]) + review.transaction do + resolution = params[:resolution] + comment = params[:comment] || request.raw_post + if !review.isOpen? && !review.isReopened? + raise "Only open review can be resolved." + end + if resolution == 'FALSE-POSITIVE' + raise "Comment must be provided." unless comment && !comment.blank? + review.set_false_positive(true, :user => current_user, :text => comment) + elsif resolution == 'FIXED' + review.create_comment(:user => current_user, :text => comment) unless comment.blank? + review.resolve + else + raise "Incorrect resolution." + end + end + render_reviews([review], params[:output] == 'HTML') + rescue ApiException => e + render_error(e.msg, e.code) rescue Exception => e render_error(e.message, 400) end end - - + # - # --- Delete the last comment of a review --- + # --- Reopen --- # - # DELETE /api/reviews + # PUT /api/reviews/reopen # Required parameters: # - 'id' : the review id - # - 'comment_id' : the id of the comment to delete (for the moment, only the last comment can be deleted) + # - 'comment' : the text of the comment # # Example : - # - DELETE "/api/reviews/?id=1&comment=5 + # - PUT "/api/reviews/reopen/1" + # - PUT "/api/reviews/reopen/1?comment=Not%20fixed" # - def destroy + def reopen begin - # 1- Get some parameters - convert_markdown=(params[:output]=='HTML') - comment_id = params[:comment_id] - raise "'comment_id' parameter is missing." unless comment_id - - # 2- Get the review or create one - raise "No 'id' parameter has been provided." unless params[:id] - review = Review.find(params[:id], :include => ['project', 'review_comments']) - unless has_rights_to_modify?(review.project) - access_denied - return + review = get_review(params[:id]) + review.transaction do + comment = params[:comment] || request.raw_post + if !review.isResolved? + raise "Only resolved review can be reopened." + end + if review.resolution == 'FALSE-POSITIVE' + raise "Comment must be provided." unless comment && !comment.blank? + review.set_false_positive(false, :user => current_user, :text => comment) + else + review.reopen + review.create_comment(:user => current_user, :text => comment) unless comment.blank? + end end - - # 3- Delete the last comment if possible - raise "Cannot delete the only existing comment of this review." if review.comments.size == 1 - last_comment = review.comments.last - raise "Only the last comment of a review can be deleted" unless last_comment.id == comment_id - raise "You do not have the rights to edit this comment as it is not yours." unless last_comment.user == current_user - review.delete_comment(last_comment.id) - - # 4- And finally send back the review - render_reviews([review], convert_markdown) - + render_reviews([review], params[:output] == 'HTML') rescue ApiException => e render_error(e.msg, e.code) - rescue Exception => e render_error(e.message, 400) end end - - + + private - + + def get_review(id) + raise "No 'id' parameter has been provided." unless id + review = Review.find(id, :include => ['project']) + raise ApiException.new(401, 'Unauthorized') unless has_rights_to_modify?(review.project) + return review + end + def render_reviews(reviews, convert_markdown) respond_to do |format| format.json { render :json => jsonp(Review.reviews_to_json(reviews, convert_markdown)) } @@ -228,14 +263,14 @@ class Api::ReviewsController < Api::ApiController format.text { render :text => text_not_supported } end end - + def find_user(login) unless login.blank? users = User.find(:all, :conditions => ["login = ?", login]) users[0] if users.size > 0 - end + end end - + def has_rights_to_modify?(object) current_user && has_role?(:user, object) end @@ -248,5 +283,5 @@ class Api::ReviewsController < Api::ApiController violation.save end end - -end
\ No newline at end of file + +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 84726448203..b978aff15c0 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 @@ -137,6 +137,10 @@ class Review < ActiveRecord::Base status == STATUS_REOPENED end + def isOpen? + status == STATUS_OPEN + end + def reopen self.status = STATUS_REOPENED self.resolution = nil @@ -284,8 +288,8 @@ class Review < ActiveRecord::Base xml.author(user.login) xml.assignee(assignee.login) if assignee xml.title(title) - xml.falsePositive(false_positive) xml.status(status) + xml.resolution(resolution) if resolution xml.severity(severity) xml.resource(resource.kee) if resource xml.line(resource_line) if resource_line && resource_line>0 @@ -319,8 +323,8 @@ class Review < ActiveRecord::Base json['author'] = user.login json['assignee'] = assignee.login if assignee json['title'] = title if title - json['falsePositive'] = false_positive json['status'] = status + json['resolution'] = resolution if resolution json['severity'] = severity json['resource'] = resource.kee if resource json['line'] = resource_line if resource_line && resource_line>0 @@ -338,8 +342,6 @@ class Review < ActiveRecord::Base json end - - # # # PRIVATE METHODS diff --git a/sonar-server/src/main/webapp/WEB-INF/config/routes.rb b/sonar-server/src/main/webapp/WEB-INF/config/routes.rb index 6f4aafef2a0..ba3c53ba26b 100644 --- a/sonar-server/src/main/webapp/WEB-INF/config/routes.rb +++ b/sonar-server/src/main/webapp/WEB-INF/config/routes.rb @@ -11,7 +11,12 @@ ActionController::Routing::Routes.draw do |map| api.resources :events, :only => [:index, :show, :create, :destroy] api.resources :user_properties, :only => [:index, :show, :create, :destroy], :requirements => { :id => /.*/ } api.resources :favorites, :only => [:index, :show, :create, :destroy], :requirements => { :id => /.*/ } - api.resources :reviews, :only => [:index, :create, :update, :destroy], :requirements => { :id => /.*/ } + api.resources :reviews, :only => [:index, :show, :create], :member => { + :add_comment => :put, + :reassign => :put, + :resolve => :put, + :reopen => :put + } end map.connect 'api/metrics', :controller => 'api/metrics', :action => 'index', :conditions => { :method => :get } diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Review.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Review.java index 646e2b2a7a1..f399788eef3 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Review.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Review.java @@ -39,109 +39,84 @@ public class Review extends Model { private String severity = null; private String resourceKee = null; private Integer line = null; - private Boolean falsePositive = null; + private String resolution = null; private Long violationId; private List<Review.Comment> comments = new ArrayList<Review.Comment>(); /** - * @return the id + * @return id */ public Long getId() { return id; } - /** - * @param id - * the id to set - */ public Review setId(Long id) { this.id = id; return this; } /** - * @return the createdAt + * @return date of creation */ public Date getCreatedAt() { return createdAt; } - /** - * @param createdAt - * the createdAt to set - */ public Review setCreatedAt(Date createdAt) { this.createdAt = createdAt; return this; } /** - * @return the updatedAt + * @return date of last modification */ public Date getUpdatedAt() { return updatedAt; } - /** - * @param updatedAt - * the updatedAt to set - */ public Review setUpdatedAt(Date updatedAt) { this.updatedAt = updatedAt; return this; } /** - * @return the authorLogin + * @return user that initiated review */ public String getAuthorLogin() { return authorLogin; } - /** - * @param s - * the authorLogin to set - */ public Review setAuthorLogin(String s) { this.authorLogin = s; return this; } /** - * @return the assigneeLogin + * @return assignee */ public String getAssigneeLogin() { return assigneeLogin; } - /** - * @param s - * the assigneeLogin to set - */ public Review setAssigneeLogin(String s) { this.assigneeLogin = s; return this; } /** - * @return the title + * @return title */ public String getTitle() { return title; } - /** - * @param s - * the title to set - */ public Review setTitle(String s) { this.title = s; return this; } /** - * @deprecated since 2.9. Use {@link #getFalsePositive()} instead. - * @return the type + * @deprecated since 2.9. */ @Deprecated public String getType() { @@ -149,81 +124,57 @@ public class Review extends Model { } /** - * @deprecated since 2.9. Use {@link #setFalsePositive(Boolean)} instead. - * @param s - * the type to set + * @deprecated since 2.9. */ @Deprecated public Review setType(String s) { this.type = s; - // the following code is only here to ensure backward compatibility with 2.8 - if ("FALSE_POSITIVE".equals(type)) { - falsePositive = Boolean.TRUE; - } else if ("VIOLATION".equals(type)) { - falsePositive = Boolean.FALSE; - } return this; } /** - * @return the status + * @return status */ public String getStatus() { return status; } - /** - * @param status - * the status to set - */ public Review setStatus(String status) { this.status = status; return this; } /** - * @return the severity + * @return severity */ public String getSeverity() { return severity; } - /** - * @param severity - * the severity to set - */ public Review setSeverity(String severity) { this.severity = severity; return this; } /** - * @return the resourceKee + * @return resourceKee */ public String getResourceKee() { return resourceKee; } - /** - * @param resourceKee - * the resourceKee to set - */ public Review setResourceKee(String resourceKee) { this.resourceKee = resourceKee; return this; } /** - * @return the line + * @return line */ public Integer getLine() { return line; } - /** - * @param line - * the line to set - */ public Review setLine(Integer line) { this.line = line; return this; @@ -231,41 +182,34 @@ public class Review extends Model { /** * @since 2.9 - * @return the falsePositive */ - public Boolean getFalsePositive() { - return falsePositive; + public String getResolution() { + return resolution; } /** * @since 2.9 - * @param falsePositive - * true if false positive */ - public Review setFalsePositive(Boolean falsePositive) { - this.falsePositive = falsePositive; + public Review setResolution(String resolution) { + this.resolution = resolution; return this; } /** * @since 2.9 - * @return the violation id + * @return violation id */ public Long getViolationId() { return violationId; } - /** - * @param id - * the violation id to set - */ public Review setViolationId(Long violationId) { this.violationId = violationId; return this; } /** - * @return the comments + * @return comments */ public List<Review.Comment> getComments() { return comments; @@ -295,28 +239,28 @@ public class Review extends Model { /** * @since 2.9 - * @return the id + * @return id */ public Long getId() { return id; } /** - * @return the authorLogin + * @return user that created this comment */ public String getAuthorLogin() { return authorLogin; } /** - * @return the updatedAt + * @return date of last modification */ public Date getUpdatedAt() { return updatedAt; } /** - * @return the text + * @return text */ public String getText() { return text; diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewCreateQuery.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewCreateQuery.java index f48eb971e19..11fc0b5afa3 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewCreateQuery.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewCreateQuery.java @@ -25,58 +25,14 @@ package org.sonar.wsclient.services; public class ReviewCreateQuery extends CreateQuery<Review> { private Long violationId; - private String text; + private String comment; private String assignee; - private Boolean falsePositive; + private String status; + private String resolution; public ReviewCreateQuery() { } - /** - * Builds a request that will create a simple review on a violation, without any assignee. - * - * @param violationId - * The id of the violation that is reviewed - * @param text - * The comment of the review - */ - public static ReviewCreateQuery createSimpleReviewQuery(Long violationId, String text) { - ReviewCreateQuery query = new ReviewCreateQuery(); - query.setText(text); - query.setViolationId(violationId); - return query; - } - - /** - * Builds a request that will create a simple review on a violation and that will be assigned to the given user. - * - * @param violationId - * The id of the violation that is reviewed - * @param text - * The comment of the review - * @param userLogin - * The login of the user whom this review will be assigned to - */ - public static ReviewCreateQuery createAssignedReviewQuery(Long violationId, String text, String userLogin) { - ReviewCreateQuery query = createSimpleReviewQuery(violationId, text); - query.setAssignee(userLogin); - return query; - } - - /** - * Builds a request that will create a false-positive review on a violation. - * - * @param violationId - * The id of the violation that is reviewed - * @param text - * The comment of the review - */ - public static ReviewCreateQuery createFalsePositiveReviewQuery(Long violationId, String text) { - ReviewCreateQuery query = createSimpleReviewQuery(violationId, text); - query.setFalsePositive(Boolean.TRUE); - return query; - } - public Long getViolationId() { return violationId; } @@ -86,12 +42,12 @@ public class ReviewCreateQuery extends CreateQuery<Review> { return this; } - public String getText() { - return text; + public String getComment() { + return comment; } - public ReviewCreateQuery setText(String text) { - this.text = text; + public ReviewCreateQuery setComment(String comment) { + this.comment = comment; return this; } @@ -104,34 +60,41 @@ public class ReviewCreateQuery extends CreateQuery<Review> { return this; } - public Boolean getFalsePositive() { - return falsePositive; + public String getStatus() { + return status; + } + + public ReviewCreateQuery setStatus(String status) { + this.status = status; + return this; + } + + public String getResolution() { + return resolution; } - public ReviewCreateQuery setFalsePositive(Boolean falsePositive) { - this.falsePositive = falsePositive; + public ReviewCreateQuery setResolution(String resolution) { + this.resolution = resolution; return this; } @Override public String getUrl() { StringBuilder url = new StringBuilder(); - url.append(ReviewQuery.BASE_URL); - url.append("/"); - url.append('?'); + url.append(ReviewQuery.BASE_URL).append('?'); appendUrlParameter(url, "violation_id", getViolationId()); - appendUrlParameter(url, "text", getText()); appendUrlParameter(url, "assignee", getAssignee()); - appendUrlParameter(url, "false_positive", getFalsePositive()); + appendUrlParameter(url, "status", getStatus()); + appendUrlParameter(url, "resolution", getResolution()); return url.toString(); } /** - * Property 'text' is transmitted through request body as content may exceed URL size allowed by the server. + * Property {@link #comment} is transmitted through request body as content may exceed URL size allowed by the server. */ @Override public String getBody() { - return text; + return comment; } @Override diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewDeleteQuery.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewDeleteQuery.java deleted file mode 100644 index 5a217666607..00000000000 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewDeleteQuery.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Sonar, open source software quality management 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 - */ -package org.sonar.wsclient.services; - -/** - * @since 2.9 - */ -public class ReviewDeleteQuery extends DeleteQuery<Review> { - - private Long reviewId; - private Long commentId; - - public ReviewDeleteQuery() { - } - - /** - * Builds a request that will delete the comment of a review. For the moment, only the last comment can be deleted, and only the author of - * this comment is allowed to do so. - * - * @param reviewId - * the id of the review - * @param commentId - * the id of the comment to delete - */ - public static ReviewDeleteQuery deleteCommentQuery(Long reviewId, Long commentId) { - ReviewDeleteQuery query = new ReviewDeleteQuery(); - query.reviewId = reviewId; - query.commentId = commentId; - return query; - } - - public Long getReviewId() { - return reviewId; - } - - public ReviewDeleteQuery setReviewId(Long reviewId) { - this.reviewId = reviewId; - return this; - } - - public Long getCommentId() { - return commentId; - } - - public ReviewDeleteQuery setCommentId(Long commentId) { - this.commentId = commentId; - return this; - } - - @Override - public String getUrl() { - StringBuilder url = new StringBuilder(); - url.append(ReviewQuery.BASE_URL); - url.append("/"); - url.append('?'); - appendUrlParameter(url, "id", getReviewId()); - appendUrlParameter(url, "comment_id", getCommentId()); - return url.toString(); - } -} diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewUpdateQuery.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewUpdateQuery.java index 262cc9e4bfe..62e8cdd583a 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewUpdateQuery.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/ReviewUpdateQuery.java @@ -24,121 +24,63 @@ package org.sonar.wsclient.services; */ public class ReviewUpdateQuery extends UpdateQuery<Review> { - private Long reviewId; - private String text; - private String newText; + private long reviewId; + private String action; + private String comment; private String assignee; - private Boolean falsePositive; - private String status; - - public ReviewUpdateQuery() { - } + private String resolution; /** - * Builds a request that will add a comment on a an existing review. - * - * @param reviewId - * The id of the review - * @param text - * The new comment + * Creates query to add comment to review. */ - public static ReviewUpdateQuery addCommentQuery(Long reviewId, String text) { - ReviewUpdateQuery query = new ReviewUpdateQuery(); - query.setReviewId(reviewId); - query.setNewText(text); - return query; + public static ReviewUpdateQuery addComment(long id, String comment) { + return new ReviewUpdateQuery(id, "add_comment").setComment(comment); } /** - * Builds a request that will update the false-positive status of an existing review. - * - * @param reviewId - * The id of the review - * @param text - * The comment for this modification - * @param falsePositive - * the new false positive status of the review + * Creates query to reassign review. */ - public static ReviewUpdateQuery updateFalsePositiveQuery(Long reviewId, String text, Boolean falsePositive) { - ReviewUpdateQuery query = addCommentQuery(reviewId, text); - query.setFalsePositive(falsePositive); - return query; + public static ReviewUpdateQuery reassign(long id, String assignee) { + return new ReviewUpdateQuery(id, "reassign").setAssignee(assignee); } /** - * Builds a request that will reassign an existing review to the given user. <br/> - * <br/> - * To unassign the review, simply pass "none" for the user login. + * Creates query to resolve review. + * If resolution "FALSE-POSITIVE", then you must provide comment using {@link #setComment(String)}. + * Otherwise comment is optional. * - * @param reviewId - * The id of the review that is reviewed - * @param userLogin - * The login of the user whom this review will be assigned to, or "none" to unassign + * @param resolution + * can be "FIXED" or "FALSE-POSITIVE" */ - public static ReviewUpdateQuery reassignQuery(Long reviewId, String userLogin) { - ReviewUpdateQuery query = new ReviewUpdateQuery(); - query.setReviewId(reviewId); - query.setAssignee(userLogin); - return query; + public static ReviewUpdateQuery resolve(long id, String resolution) { + return new ReviewUpdateQuery(id, "resolve").setResolution(resolution); } /** - * Builds a request that will edit the last comment of a an existing review (if the last comment belongs to the current user). - * - * @param reviewId - * The id of the review - * @param text - * The new text for the last comment + * Creates query to reopen review. + * If review was resolved as "FALSE-POSITIVE", then you must provide comment using {@link #setComment(String)}. + * Otherwise comment is optional. */ - public static ReviewUpdateQuery editLastCommentQuery(Long reviewId, String text) { - ReviewUpdateQuery query = new ReviewUpdateQuery(); - query.setReviewId(reviewId); - query.setText(text); - return query; + public static ReviewUpdateQuery reopen(long id) { + return new ReviewUpdateQuery(id, "reopen"); } - /** - * Builds a request that will change the status of a an existing review.<br/> - * <br/> - * Currently, only "RESOLVED" and "REOPENED" are supported. - * - * @param reviewId - * The id of the review - * @param status - * The new status - */ - public static ReviewUpdateQuery changeStatusQuery(Long reviewId, String status) { - ReviewUpdateQuery query = new ReviewUpdateQuery(); - query.setReviewId(reviewId); - query.setStatus(status); - return query; + private ReviewUpdateQuery(long id, String action) { + this.reviewId = id; + this.action = action; } - public Long getReviewId() { + public long getReviewId() { return reviewId; } - public ReviewUpdateQuery setReviewId(Long reviewId) { - this.reviewId = reviewId; + public ReviewUpdateQuery setComment(String comment) { + this.comment = comment; return this; } - public String getText() { - return text; - } - - public ReviewUpdateQuery setText(String text) { - this.text = text; - return this; - } - - public String getNewText() { - return newText; - } - - public ReviewUpdateQuery setNewText(String text) { - this.newText = text; - return this; + public String getComment() { + return comment; } public String getAssignee() { @@ -150,45 +92,37 @@ public class ReviewUpdateQuery extends UpdateQuery<Review> { return this; } - public Boolean getFalsePositive() { - return falsePositive; + public String getResolution() { + return resolution; } - public ReviewUpdateQuery setFalsePositive(Boolean falsePositive) { - this.falsePositive = falsePositive; - return this; - } - - public String getStatus() { - return status; - } - - public ReviewUpdateQuery setStatus(String status) { - this.status = status; + /** + * @param resolution + * can be "FIXED" or "FALSE-POSITIVE" + */ + public ReviewUpdateQuery setResolution(String resolution) { + this.resolution = resolution; return this; } @Override public String getUrl() { StringBuilder url = new StringBuilder(); - url.append(ReviewQuery.BASE_URL); - url.append("/"); - url.append('?'); - appendUrlParameter(url, "id", getReviewId()); - appendUrlParameter(url, "text", getText()); - appendUrlParameter(url, "new_text", getNewText()); + url.append(ReviewQuery.BASE_URL) + .append('/').append(action) + .append('/').append(reviewId) + .append('?'); appendUrlParameter(url, "assignee", getAssignee()); - appendUrlParameter(url, "false_positive", getFalsePositive()); - appendUrlParameter(url, "status", getStatus()); + appendUrlParameter(url, "resolution", getResolution()); return url.toString(); } /** - * Properties 'text' or 'new_text' are transmitted through request body as content may exceed URL size allowed by the server. + * Property {@link #comment} transmitted through request body as content may exceed URL size allowed by the server. */ @Override public String getBody() { - return text == null ? newText : text; + return comment; } @Override diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/ReviewUnmarshaller.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/ReviewUnmarshaller.java index 3ea023700ea..a757c551a0d 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/ReviewUnmarshaller.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/ReviewUnmarshaller.java @@ -42,7 +42,7 @@ public class ReviewUnmarshaller extends AbstractUnmarshaller<Review> { review.setSeverity(utils.getString(json, "severity")); review.setResourceKee(utils.getString(json, "resource")); review.setLine(utils.getInteger(json, "line")); - review.setFalsePositive(utils.getBoolean(json, "falsePositive")); + review.setResolution(utils.getString(json, "resolution")); review.setViolationId(utils.getLong(json, "violationId")); review.setType(utils.getString(json, "type")); diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewCreateQueryTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewCreateQueryTest.java index fd3ae931612..5559755a1fc 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewCreateQueryTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewCreateQueryTest.java @@ -27,22 +27,34 @@ import org.junit.Test; public class ReviewCreateQueryTest extends QueryTestCase { @Test - public void testCreateSimpleReview() { - ReviewCreateQuery query = ReviewCreateQuery.createSimpleReviewQuery(13L, "Hello World!"); - assertThat(query.getUrl(), is("/api/reviews/?violation_id=13&text=Hello+World%21&")); + public void testCreateReview() { + ReviewCreateQuery query = new ReviewCreateQuery() + .setViolationId(13L) + .setComment("Hello World!"); + assertThat(query.getUrl(), is("/api/reviews?violation_id=13&")); + assertThat(query.getBody(), is("Hello World!")); assertThat(query.getModelClass().getName(), is(Review.class.getName())); } @Test public void testCreateAssignedReview() { - ReviewCreateQuery query = ReviewCreateQuery.createAssignedReviewQuery(13L, "Hello World!", "fabrice"); - assertThat(query.getUrl(), is("/api/reviews/?violation_id=13&text=Hello+World%21&assignee=fabrice&")); + ReviewCreateQuery query = new ReviewCreateQuery() + .setViolationId(13L) + .setAssignee("fabrice") + .setComment("Hello World!"); + assertThat(query.getUrl(), is("/api/reviews?violation_id=13&assignee=fabrice&")); + assertThat(query.getBody(), is("Hello World!")); } @Test - public void testCreateFalsePositiveReview() { - ReviewCreateQuery query = ReviewCreateQuery.createFalsePositiveReviewQuery(13L, "Hello World!"); - assertThat(query.getUrl(), is("/api/reviews/?violation_id=13&text=Hello+World%21&false_positive=true&")); + public void testCreateResolvedReview() { + ReviewCreateQuery query = new ReviewCreateQuery() + .setViolationId(13L) + .setStatus("RESOLVED") + .setResolution("FALSE-POSITIVE") + .setComment("Hello World!"); + assertThat(query.getUrl(), is("/api/reviews?violation_id=13&status=RESOLVED&resolution=FALSE-POSITIVE&")); + assertThat(query.getBody(), is("Hello World!")); } -}
\ No newline at end of file +} diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewDeleteQueryTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewDeleteQueryTest.java deleted file mode 100644 index d78bcf39baf..00000000000 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewDeleteQueryTest.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Sonar, open source software quality management 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 - */ -package org.sonar.wsclient.services; - -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertThat; - -import org.junit.Test; - -public class ReviewDeleteQueryTest extends QueryTestCase { - - @Test - public void testAddComment() { - ReviewDeleteQuery query = ReviewDeleteQuery.deleteCommentQuery(13L, 2L); - assertThat(query.getUrl(), is("/api/reviews/?id=13&comment_id=2&")); - } - -}
\ No newline at end of file diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewUpdateQueryTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewUpdateQueryTest.java index 10620d92b82..bb0d783dd31 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewUpdateQueryTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/services/ReviewUpdateQueryTest.java @@ -19,6 +19,7 @@ */ package org.sonar.wsclient.services; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertThat; @@ -28,33 +29,38 @@ public class ReviewUpdateQueryTest extends QueryTestCase { @Test public void testAddComment() { - ReviewUpdateQuery query = ReviewUpdateQuery.addCommentQuery(13L, "Hello World!"); - assertThat(query.getUrl(), is("/api/reviews/?id=13&new_text=Hello+World%21&")); + ReviewUpdateQuery query = ReviewUpdateQuery.addComment(13, "Hello World!"); + assertThat(query.getUrl(), is("/api/reviews/add_comment/13?")); + assertThat(query.getBody(), is("Hello World!")); assertThat(query.getModelClass().getName(), is(Review.class.getName())); } @Test - public void testEditLastComment() { - ReviewUpdateQuery query = ReviewUpdateQuery.editLastCommentQuery(13L, "Hello World!"); - assertThat(query.getUrl(), is("/api/reviews/?id=13&text=Hello+World%21&")); + public void testReassign() { + ReviewUpdateQuery query = ReviewUpdateQuery.reassign(13, "fabrice"); + assertThat(query.getUrl(), is("/api/reviews/reassign/13?assignee=fabrice&")); + assertThat(query.getBody(), nullValue()); } @Test - public void testReassign() { - ReviewUpdateQuery query = ReviewUpdateQuery.reassignQuery(13L, "fabrice"); - assertThat(query.getUrl(), is("/api/reviews/?id=13&assignee=fabrice&")); + public void testResolveAsFalsePositive() { + ReviewUpdateQuery query = ReviewUpdateQuery.resolve(13, "FALSE-POSITIVE").setComment("Hello World!"); + assertThat(query.getUrl(), is("/api/reviews/resolve/13?resolution=FALSE-POSITIVE&")); + assertThat(query.getBody(), is("Hello World!")); } @Test - public void testUpdateFalsePositive() { - ReviewUpdateQuery query = ReviewUpdateQuery.updateFalsePositiveQuery(13L, "Hello World!", Boolean.TRUE); - assertThat(query.getUrl(), is("/api/reviews/?id=13&new_text=Hello+World%21&false_positive=true&")); + public void testResolveAsFixed() { + ReviewUpdateQuery query = ReviewUpdateQuery.resolve(13, "FIXED"); + assertThat(query.getUrl(), is("/api/reviews/resolve/13?resolution=FIXED&")); + assertThat(query.getBody(), nullValue()); } @Test - public void testChangeStatus() { - ReviewUpdateQuery query = ReviewUpdateQuery.changeStatusQuery(13L, "RESOLVED"); - assertThat(query.getUrl(), is("/api/reviews/?id=13&status=RESOLVED&")); + public void testReopen() { + ReviewUpdateQuery query = ReviewUpdateQuery.reopen(13); + assertThat(query.getUrl(), is("/api/reviews/reopen/13?")); + assertThat(query.getBody(), nullValue()); } -}
\ No newline at end of file +} diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/ReviewUnmarshallerTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/ReviewUnmarshallerTest.java index 9041c11a06a..dc9b512ed90 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/ReviewUnmarshallerTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/ReviewUnmarshallerTest.java @@ -24,12 +24,12 @@ import static org.hamcrest.CoreMatchers.nullValue; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; -import java.util.List; - import org.junit.Test; import org.sonar.wsclient.services.Review; import org.sonar.wsclient.services.Review.Comment; +import java.util.List; + public class ReviewUnmarshallerTest extends UnmarshallerTestCase { @Test @@ -50,8 +50,8 @@ public class ReviewUnmarshallerTest extends UnmarshallerTestCase { assertThat(review.getAuthorLogin(), is("admin")); assertThat(review.getAssigneeLogin(), is("admin")); assertThat(review.getTitle(), is("'static' modifier out of order with the JLS suggestions.")); - assertThat(review.getFalsePositive(), is(Boolean.FALSE)); - assertThat(review.getStatus(), is("OPEN")); + assertThat(review.getStatus(), is("RESOLVED")); + assertThat(review.getResolution(), is("FALSE-POSITIVE")); assertThat(review.getSeverity(), is("MINOR")); assertThat(review.getResourceKee(), is("org.codehaus.sonar:sonar-channel:org.sonar.channel.CodeReaderConfiguration")); assertThat(review.getLine(), is(33)); @@ -66,7 +66,8 @@ public class ReviewUnmarshallerTest extends UnmarshallerTestCase { review = reviews.get(1); assertThat(review.getAssigneeLogin(), nullValue()); - assertThat(review.getFalsePositive(), is(Boolean.TRUE)); + assertThat(review.getStatus(), is("OPEN")); + assertThat(review.getResolution(), nullValue()); } /* diff --git a/sonar-ws-client/src/test/resources/reviews/reviews-2.9.json b/sonar-ws-client/src/test/resources/reviews/reviews-2.9.json index 8b1f11c62a8..919b7ff9d8e 100644 --- a/sonar-ws-client/src/test/resources/reviews/reviews-2.9.json +++ b/sonar-ws-client/src/test/resources/reviews/reviews-2.9.json @@ -1 +1 @@ -[{"id":3,"createdAt":"2011-04-26T15:44:42+0200","updatedAt":"2011-04-26T15:44:42+0200","author":"admin","assignee":"admin","title":"'static' modifier out of order with the JLS suggestions.","falsePositive":false,"status":"OPEN","severity":"MINOR","resource":"org.codehaus.sonar:sonar-channel:org.sonar.channel.CodeReaderConfiguration","line":33,"violationId":1,"comments":[{"id":1,"author":"admin","updatedAt":"2011-04-26T15:44:42+0200","text":"This is a review.<br/>And this is on multiple lines...<br/><br/><code>Wouhou!!!!!</code>"},{"id":2,"author":"admin","updatedAt":"2011-04-26T17:10:19+0200","text":"<em>Bold on multiple line?</em>"},{"id":3,"author":"admin","updatedAt":"2011-04-26T17:11:02+0200","text":"And the bullets:<br/><ul><li>1 bullet</li>\n<li>2 bullets</li></ul>"},{"id":4,"author":"admin","updatedAt":"2011-04-26T17:27:37+0200","text":"Wazzaa"}]},{"id":9,"createdAt":"2011-04-27T14:37:20+0200","updatedAt":"2011-04-27T14:37:20+0200","author":"admin","title":"New exception is thrown in catch block, original stack trace may be lost","falsePositive":true,"status":"OPEN","severity":"MAJOR","resource":"org.codehaus.sonar:sonar-channel:org.sonar.channel.CodeReader","line":84,"violationId":2,"comments":[{"id":5,"author":"admin","updatedAt":"2011-04-27T14:37:20+0200","text":"Wazzaaaa"}]}]
\ No newline at end of file +[{"id":3,"createdAt":"2011-04-26T15:44:42+0200","updatedAt":"2011-04-26T15:44:42+0200","author":"admin","assignee":"admin","title":"'static' modifier out of order with the JLS suggestions.","status":"RESOLVED","resolution":"FALSE-POSITIVE","severity":"MINOR","resource":"org.codehaus.sonar:sonar-channel:org.sonar.channel.CodeReaderConfiguration","line":33,"violationId":1,"comments":[{"id":1,"author":"admin","updatedAt":"2011-04-26T15:44:42+0200","text":"This is a review.<br/>And this is on multiple lines...<br/><br/><code>Wouhou!!!!!</code>"},{"id":2,"author":"admin","updatedAt":"2011-04-26T17:10:19+0200","text":"<em>Bold on multiple line?</em>"},{"id":3,"author":"admin","updatedAt":"2011-04-26T17:11:02+0200","text":"And the bullets:<br/><ul><li>1 bullet</li>\n<li>2 bullets</li></ul>"},{"id":4,"author":"admin","updatedAt":"2011-04-26T17:27:37+0200","text":"Wazzaa"}]},{"id":9,"createdAt":"2011-04-27T14:37:20+0200","updatedAt":"2011-04-27T14:37:20+0200","author":"admin","title":"New exception is thrown in catch block, original stack trace may be lost","status":"OPEN","severity":"MAJOR","resource":"org.codehaus.sonar:sonar-channel:org.sonar.channel.CodeReader","line":84,"violationId":2,"comments":[{"id":5,"author":"admin","updatedAt":"2011-04-27T14:37:20+0200","text":"Wazzaaaa"}]}]
\ No newline at end of file |