diff options
16 files changed, 310 insertions, 26 deletions
diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java index f0fe71402de..922e3154e2d 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java @@ -61,12 +61,20 @@ public class CloseReviewsDecorator implements Decorator { if (currentSnapshot != null) { int resourceId = currentSnapshot.getResourceId(); int snapshotId = currentSnapshot.getId(); - Query query = databaseSession.createNativeQuery(generateUpdateOnResourceSqlRequest(resourceId, snapshotId)); + + // Close reviews for which violations have been fixed + Query query = databaseSession.createNativeQuery(generateUpdateToCloseReviewsForFixedViolation(resourceId, snapshotId)); int rowUpdated = query.executeUpdate(); LOG.debug("- {} reviews set to 'closed' on resource #{}", rowUpdated, resourceId); + // Reopen reviews that had been set to resolved but for which the violation is still here + query = databaseSession.createNativeQuery(generateUpdateToReopenResolvedReviewsForNonFixedViolation(resourceId)); + rowUpdated = query.executeUpdate(); + LOG.debug("- {} reviews set to 'reopened' on resource #{}", rowUpdated, resourceId); + + // And close reviews that relate to resources that have been deleted or renamed if (ResourceUtils.isRootProject(resource)) { - query = databaseSession.createNativeQuery(generateUpdateOnProjectSqlRequest(resourceId, currentSnapshot.getId())); + query = databaseSession.createNativeQuery(generateUpdateToCloseReviewsForDeletedResources(resourceId, currentSnapshot.getId())); query.setParameter(1, Boolean.TRUE); rowUpdated = query.executeUpdate(); LOG.debug("- {} reviews set to 'closed' on project #{}", rowUpdated, resourceId); @@ -76,14 +84,18 @@ public class CloseReviewsDecorator implements Decorator { } } - protected String generateUpdateOnResourceSqlRequest(int resourceId, int snapshotId) { - return "UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP WHERE status='OPEN' AND resource_id = " + resourceId + protected String generateUpdateToCloseReviewsForFixedViolation(int resourceId, int snapshotId) { + return "UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP WHERE status!='CLOSED' AND 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)"; } - protected String generateUpdateOnProjectSqlRequest(int projectId, int projectSnapshotId) { - return "UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP WHERE status='OPEN' AND project_id=" + projectId + protected String generateUpdateToReopenResolvedReviewsForNonFixedViolation(int resourceId) { + return "UPDATE reviews SET status='REOPENED', updated_at=CURRENT_TIMESTAMP WHERE status='RESOLVED' AND resource_id = " + resourceId; + } + + protected String generateUpdateToCloseReviewsForDeletedResources(int projectId, int projectSnapshotId) { + return "UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP WHERE status!='CLOSED' AND project_id=" + projectId + " AND resource_id IN ( SELECT prev.project_id FROM snapshots prev WHERE prev.root_project_id=" + projectId + " AND prev.islast=? AND NOT EXISTS ( SELECT cur.id FROM snapshots cur WHERE cur.root_snapshot_id=" + projectSnapshotId + " AND cur.created_at > prev.created_at AND cur.root_project_id=" + projectId + " AND cur.project_id=prev.project_id ) )"; diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java index 2ea8a7c1de4..17ff7a09293 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest.java @@ -49,12 +49,12 @@ public class CloseReviewsDecoratorTest extends DatabaseTestCase { setupData("fixture"); CloseReviewsDecorator reviewsDecorator = new CloseReviewsDecorator(null, null); - String sqlRequest = reviewsDecorator.generateUpdateOnResourceSqlRequest(666, 222); + String sqlRequest = reviewsDecorator.generateUpdateToCloseReviewsForFixedViolation(666, 222); Statement stmt = getConnection().createStatement(); int count = stmt.executeUpdate(sqlRequest); - assertThat(count, is(1)); + assertThat(count, is(3)); assertTables("shouldCloseReviewWithoutCorrespondingViolation", new String[] { "reviews" }, new String[] { "updated_at" }); try { @@ -64,4 +64,29 @@ public class CloseReviewsDecoratorTest extends DatabaseTestCase { // "updated_at" column must be different, so the comparison should raise this exception } } + + @Test + public void shouldReopenResolvedReviewWithNonFixedViolation() throws Exception { + setupData("fixture"); + + CloseReviewsDecorator reviewsDecorator = new CloseReviewsDecorator(null, null); + + // First we close the reviews for which the violations have been fixed (this is because we use the same "fixture"...) + getConnection().createStatement().executeUpdate(reviewsDecorator.generateUpdateToCloseReviewsForFixedViolation(666, 222)); + + // And now we reopen the reviews that still have a violation + String sqlRequest = reviewsDecorator.generateUpdateToReopenResolvedReviewsForNonFixedViolation(666); + Statement stmt = getConnection().createStatement(); + int count = stmt.executeUpdate(sqlRequest); + + assertThat(count, is(1)); + assertTables("shouldReopenResolvedReviewWithNonFixedViolation", new String[] { "reviews" }, new String[] { "updated_at" }); + + try { + assertTables("shouldReopenResolvedReviewWithNonFixedViolation", new String[] { "reviews" }); + fail("'updated_at' columns are identical whereas they should be different."); + } catch (ComparisonFailure e) { + // "updated_at" column must be different, so the comparison should raise this exception + } + } } diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml index 2f432d7f745..5c1b54ac173 100644 --- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml +++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/fixture.xml @@ -1,5 +1,6 @@ <dataset> + <!-- Component 555 --> <snapshots id="11" project_id="555" @@ -8,6 +9,7 @@ id="111" project_id="555" status="P" islast="false"/> + <!-- Component 666 --> <snapshots id="22" project_id="666" @@ -17,6 +19,7 @@ project_id="666" status="P" islast="false"/> + <!-- Violations on previous analysis --> <rule_failures id="1" permanent_id="1" @@ -32,17 +35,22 @@ permanent_id="3" snapshot_id="22" rule_id="1" failure_level="1"/> + <!-- Violations on new analysis --> + <!-- Violation #1 still exists --> <rule_failures id="4" permanent_id="1" snapshot_id="111" rule_id="1" failure_level="1"/> + <!-- Violation #2 has been fixed --> + <!-- Violation #3 still exists --> <rule_failures id="5" permanent_id="3" snapshot_id="222" rule_id="1" failure_level="1"/> + <!-- Existing reviews --> <reviews id="1" status="OPEN" @@ -63,5 +71,25 @@ status="CLOSED" rule_failure_permanent_id="2" resource_id="666"/> + <reviews + id="5" + status="REOPENED" + rule_failure_permanent_id="3" + resource_id="666"/> + <reviews + id="6" + status="RESOLVED" + rule_failure_permanent_id="3" + resource_id="666"/> + <reviews + id="7" + status="REOPENED" + rule_failure_permanent_id="2" + resource_id="666"/> + <reviews + id="8" + status="RESOLVED" + rule_failure_permanent_id="2" + resource_id="666"/> </dataset>
\ No newline at end of file diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml index 7b3d60ec42c..f74c2d97a73 100644 --- a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml +++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldCloseReviewWithoutCorrespondingViolation-result.xml @@ -24,5 +24,29 @@ rule_failure_permanent_id="2" resource_id="666" created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="5" + status="REOPENED" + rule_failure_permanent_id="3" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="6" + status="RESOLVED" + rule_failure_permanent_id="3" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="7" + status="CLOSED" + rule_failure_permanent_id="2" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="8" + status="CLOSED" + rule_failure_permanent_id="2" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> </dataset>
\ No newline at end of file diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldReopenResolvedReviewWithNonFixedViolation-result.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldReopenResolvedReviewWithNonFixedViolation-result.xml new file mode 100644 index 00000000000..0397576d6b3 --- /dev/null +++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/CloseReviewsDecoratorTest/shouldReopenResolvedReviewWithNonFixedViolation-result.xml @@ -0,0 +1,52 @@ +<dataset> + + <reviews + id="1" + status="OPEN" + rule_failure_permanent_id="1" + resource_id="555" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="2" + status="CLOSED" + rule_failure_permanent_id="2" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="3" + status="OPEN" + rule_failure_permanent_id="3" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="4" + status="CLOSED" + rule_failure_permanent_id="2" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="5" + status="REOPENED" + rule_failure_permanent_id="3" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="6" + status="REOPENED" + rule_failure_permanent_id="3" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="7" + status="CLOSED" + rule_failure_permanent_id="2" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + <reviews + id="8" + status="CLOSED" + rule_failure_permanent_id="2" + resource_id="666" + created_at="[null]" updated_at="[null]" user_id="[null]" assignee_id="[null]" title="[null]" false_positive="[null]" severity="[null]" resource_line="[null]" project_id="[null]"/> + +</dataset>
\ No newline at end of file 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 9882beb6ceb..e45d559f3dc 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 @@ -106,6 +106,7 @@ class Api::ReviewsController < Api::ApiController # - '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. # # Example : # - PUT "/api/reviews/?id=1&false_positive=true&new_text=Because @@ -113,6 +114,7 @@ class Api::ReviewsController < Api::ApiController # - 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 begin @@ -122,6 +124,7 @@ class Api::ReviewsController < Api::ApiController 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] @@ -150,6 +153,10 @@ class Api::ReviewsController < Api::ApiController 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 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 01dec9ea605..892dc6f1ba5 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 @@ -23,8 +23,8 @@ class ReviewsController < ApplicationController SECTION=Navigation::SECTION_HOME verify :method => :post, - :only => [:assign, :comment_form, :flag_as_false_positive, - :violation_assign, :violation_flag_as_false_positive,:violation_save_comment, :violation_delete_comment], + :only => [:assign, :flag_as_false_positive, :save_comment, :delete_comment, :change_status, + :violation_assign, :violation_flag_as_false_positive,:violation_save_comment, :violation_delete_comment, :violation_change_status], :redirect_to => {:action => :error_not_post} helper SourceHelper, UsersHelper @@ -148,6 +148,24 @@ class ReviewsController < ApplicationController render :partial => "reviews/view" end + # POST + def change_status + @review = Review.find(params[:id], :include => ['project']) + unless has_rights_to_modify?(@review.project) + render :text => "<b>Cannot create the comment</b> : access denied." + return + end + + if @review.isResolved? + @review.reopen + else + # for the moment, if a review is not open, it can only be "RESOLVED" + @review.resolve + end + + render :partial => "reviews/view" + end + # # @@ -261,6 +279,28 @@ class ReviewsController < ApplicationController end render :partial => "resource/violation", :locals => { :violation => violation } end + + # POST + def violation_change_status + violation = RuleFailure.find(params[:id], :include => 'snapshot') + unless has_rights_to_modify?(violation.snapshot) + render :text => "<b>Cannot delete the comment</b> : access denied." + return + end + sanitize_violation(violation) + + if violation.review + review = violation.review + if review.isResolved? + review.reopen + else + # for the moment, if a review is not open, it can only be "RESOLVED" + review.resolve + end + end + + render :partial => "resource/violation", :locals => { :violation => violation } + end @@ -272,7 +312,7 @@ class ReviewsController < ApplicationController @assignee_id = params[:assignee_id] || default_user @author_id = params[:author_id] || '' @severities = filter_any(params[:severities]) || [''] - @statuses = filter_any(params[:statuses]) || [Review::STATUS_OPEN] + @statuses = filter_any(params[:statuses]) || [Review::STATUS_OPEN, Review::STATUS_REOPENED] @projects = filter_any(params[:projects]) || [''] @false_positives = params[:false_positives] || 'without' @id = params[:review_id] || '' 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 638270e1476..74ad759c008 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 @@ -32,6 +32,8 @@ class Review < ActiveRecord::Base before_save :assign_project STATUS_OPEN = 'OPEN' + STATUS_RESOLVED = 'RESOLVED' + STATUS_REOPENED = 'REOPENED' STATUS_CLOSED = 'CLOSED' def on_project? @@ -114,6 +116,28 @@ class Review < ActiveRecord::Base end end + def isResolved? + status == STATUS_RESOLVED + end + + def isClosed? + status == STATUS_CLOSED + end + + def isReopened? + status == STATUS_REOPENED + end + + def reopen + self.status = STATUS_REOPENED + self.save! + end + + def resolve + self.status = STATUS_RESOLVED + self.save! + end + # diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_violation.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_violation.html.erb index ef561c7e4a7..a9427f39804 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_violation.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_violation.html.erb @@ -9,7 +9,7 @@ <%= image_tag("sep12.png") -%> - <span class="rulename"> + <span class="rulename" <%= 'style="text-decoration: line-through"' if violation.review && violation.review.isResolved? %> > <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> @@ -25,6 +25,12 @@ <span class="falsePositive">False-Positive</span> <% end %> + <% if violation.review && violation.review.isReopened? %> + <%= image_tag("sep12.png") -%> + + <span class="review-reopened">Reoponed</span> + + <% end %> <% if violation.review && violation.review.assignee_id %> <%= image_tag("sep12.png") -%> @@ -40,11 +46,19 @@ unless violation.switched_off? %> - <%= 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();$('commentActions" + violation.id.to_s + "').hide();$('assignee_id').focus();" -%> - <% + <%= link_to_remote (violation.review.isResolved? ? "Reopen" : "Resolve"), + :url => { :controller => "reviews", :action => "violation_change_status", :id => violation.id}, + :update => "vId" + violation.id.to_s, + :confirm => violation.review.isResolved? ? "Do you want to reopen this review?" : "Do you want to resolve this review?" -%> + + <% unless violation.review && violation.review.isResolved? %> + + <%= link_to_remote (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();$('commentActions" + violation.id.to_s + "').hide();$('assignee_id').focus();" -%> + <% + end end else %> @@ -56,10 +70,12 @@ <% end %> + <% unless violation.review && violation.review.isResolved? %> <%= link_to_remote (violation.switched_off? ? "Unflag as false-positive" : "Flag as false-positive"), :url => { :controller => "reviews", :action => "violation_false_positive_form", :id => violation.id, :false_positive => !violation.switched_off? }, :update => "reviewForm" + violation.id.to_s, :complete => "$('reviewForm" + violation.id.to_s + "').show();$('commentText" + violation.id.to_s + "').focus();$('vActions" + violation.id.to_s + "').hide();$('commentActions" + violation.id.to_s + "').hide();" -%> + <% end %> </span> <% end %> @@ -98,7 +114,7 @@ <%= 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 ?" -%> + :confirm => "Do you want to delete this comment?" -%> <% end %> <% end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_review.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_review.html.erb index 2865a586090..094b60f560f 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_review.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_review.html.erb @@ -8,7 +8,7 @@ <% end %> <% - if current_user && review.status != "CLOSED" && review.rule_failure + if current_user && !review.isClosed? && review.rule_failure violation_switched_off = review.rule_failure.switched_off? %> <span class="actions" id="rActions"> @@ -18,17 +18,26 @@ <% if !violation_switched_off %> - <%= link_to_remote (review.assignee_id ? "Reassign" : "Assign"), - :url => { :controller => "reviews", :action => "assign_form", :review_id => review.id}, - :update => "assignForm", - :complete => "$('rActions').hide(); $('editActions').hide(); $('assignee_id').focus();" -%> - - + <%= link_to_remote (review.isResolved? ? "Reopen" : "Resolve"), + :url => { :controller => "reviews", :action => "change_status", :id => review.id}, + :update => "review", + :confirm => review.isResolved? ? "Do you want to reopen this review?" : "Do you want to resolve this review?" -%> + + <% unless review.isResolved? %> + <%= link_to_remote (review.assignee_id ? "Reassign" : "Assign"), + :url => { :controller => "reviews", :action => "assign_form", :review_id => review.id}, + :update => "assignForm", + :complete => "$('rActions').hide(); $('editActions').hide(); $('assignee_id').focus();" -%> + + + <% end %> <% end %> - <%= link_to_remote (violation_switched_off ? "Unflag as false-positive" : "Flag as false-positive"), + <% unless review.isResolved? %> + <%= link_to_remote (violation_switched_off ? "Unflag as false-positive" : "Flag as false-positive"), :url => { :controller => "reviews", :action => "false_positive_form", :id => review.id, :false_positive => !violation_switched_off }, :update => "reviewForm", :complete => "$('reviewForm').show(); $('rActions').hide(); $('editActions').hide(); $('commentText').focus();" -%> + <% end %> </span> <% end %> diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/index.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/index.html.erb index a470efb0b65..de38cf0ef36 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/index.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/index.html.erb @@ -37,6 +37,8 @@ function launchSearch(columnName, link) { <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_REOPENED -%>" class="status_reopened" <%= 'selected' if @statuses.include?(Review::STATUS_REOPENED) -%>>Reopened</option> + <option value="<%= Review::STATUS_RESOLVED -%>" class="status_resolved" <%= 'selected' if @statuses.include?(Review::STATUS_RESOLVED) -%>>Resolved</option> <option value="<%= Review::STATUS_CLOSED -%>" class="status_closed" <%= 'selected' if @statuses.include?(Review::STATUS_CLOSED) -%>>Closed</option> </select> </td> diff --git a/sonar-server/src/main/webapp/images/status/REOPENED.png b/sonar-server/src/main/webapp/images/status/REOPENED.png Binary files differnew file mode 100644 index 00000000000..8c0dad70d83 --- /dev/null +++ b/sonar-server/src/main/webapp/images/status/REOPENED.png diff --git a/sonar-server/src/main/webapp/images/status/RESOLVED.png b/sonar-server/src/main/webapp/images/status/RESOLVED.png Binary files differnew file mode 100644 index 00000000000..4e30efa07a4 --- /dev/null +++ b/sonar-server/src/main/webapp/images/status/RESOLVED.png diff --git a/sonar-server/src/main/webapp/stylesheets/style.css b/sonar-server/src/main/webapp/stylesheets/style.css index 4d42adbf1c0..f2ec4559808 100644 --- a/sonar-server/src/main/webapp/stylesheets/style.css +++ b/sonar-server/src/main/webapp/stylesheets/style.css @@ -687,6 +687,11 @@ div.vtitle{ padding-left: 5px; padding-right: 5px; } +.review-reopened { + background-color: #FDC596; + padding-left: 5px; + padding-right: 5px; +} div.vtitle a.action { color: #777; } @@ -1195,6 +1200,12 @@ select.withIcons option { option.status_open { background-image: url('../images/status/OPEN.png'); } +option.status_reopened { + background-image: url('../images/status/REOPENED.png'); +} +option.status_resolved { + background-image: url('../images/status/RESOLVED.png'); +} option.status_closed { background-image: url('../images/status/CLOSED.png'); } 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 244ef0b5b4d..262cc9e4bfe 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 @@ -29,6 +29,7 @@ public class ReviewUpdateQuery extends UpdateQuery<Review> { private String newText; private String assignee; private Boolean falsePositive; + private String status; public ReviewUpdateQuery() { } @@ -96,6 +97,23 @@ public class ReviewUpdateQuery extends UpdateQuery<Review> { return query; } + /** + * 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; + } + public Long getReviewId() { return reviewId; } @@ -141,6 +159,15 @@ public class ReviewUpdateQuery extends UpdateQuery<Review> { return this; } + public String getStatus() { + return status; + } + + public ReviewUpdateQuery setStatus(String status) { + this.status = status; + return this; + } + @Override public String getUrl() { StringBuilder url = new StringBuilder(); @@ -152,6 +179,7 @@ public class ReviewUpdateQuery extends UpdateQuery<Review> { appendUrlParameter(url, "new_text", getNewText()); appendUrlParameter(url, "assignee", getAssignee()); appendUrlParameter(url, "false_positive", getFalsePositive()); + appendUrlParameter(url, "status", getStatus()); return url.toString(); } 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 ee28a908ab6..10620d92b82 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 @@ -51,4 +51,10 @@ public class ReviewUpdateQueryTest extends QueryTestCase { assertThat(query.getUrl(), is("/api/reviews/?id=13&new_text=Hello+World%21&false_positive=true&")); } + @Test + public void testChangeStatus() { + ReviewUpdateQuery query = ReviewUpdateQuery.changeStatusQuery(13L, "RESOLVED"); + assertThat(query.getUrl(), is("/api/reviews/?id=13&status=RESOLVED&")); + } + }
\ No newline at end of file |
