From 6c263271502929862a6fde7846408c4576640614 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Thu, 16 May 2013 23:05:25 +0200 Subject: [PATCH] SONAR-4304 better form of manual issue --- .../issue/InternalRubyIssueService.java | 18 ++++---- .../app/controllers/api/issues_controller.rb | 2 +- .../app/controllers/issue_controller.rb | 26 ++++++----- .../app/views/issue/_create_form.html.erb | 44 +++++++++++++------ .../src/main/webapp/stylesheets/style.css | 10 ++++- .../org/sonar/wsclient/issue/NewIssue.java | 25 ++++++----- .../sonar/wsclient/issue/NewIssueTest.java | 16 ++++++- 7 files changed, 90 insertions(+), 51 deletions(-) diff --git a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java index 7c67a7467d5..ed12f859991 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java @@ -36,9 +36,9 @@ import org.sonar.core.resource.ResourceDao; import org.sonar.core.resource.ResourceDto; import org.sonar.core.resource.ResourceQuery; import org.sonar.server.platform.UserSession; +import org.sonar.server.util.RubyUtils; import javax.annotation.Nullable; - import java.text.SimpleDateFormat; import java.util.Collection; import java.util.Date; @@ -114,12 +114,10 @@ public class InternalRubyIssueService implements ServerComponent { // TODO verify authorization // TODO check existence of component DefaultIssueBuilder builder = new DefaultIssueBuilder().componentKey(componentKey); - String line = parameters.get("line"); - builder.line(line != null ? Integer.parseInt(line) : null); + builder.line(RubyUtils.toInteger(parameters.get("line"))); builder.message(parameters.get("message")); builder.severity(parameters.get("severity")); - String effortToFix = parameters.get("effortToFix"); - builder.effortToFix(effortToFix != null ? Double.parseDouble(effortToFix) : null); + builder.effortToFix(RubyUtils.toDouble(parameters.get("effortToFix"))); // TODO verify existence of rule builder.ruleKey(RuleKey.parse(parameters.get("rule"))); Issue issue = builder.build(); @@ -245,16 +243,16 @@ public class InternalRubyIssueService implements ServerComponent { } if (!Strings.isNullOrEmpty(projectParam) && !Strings.isNullOrEmpty(name) && !name.equals(oldName) - && actionPlanService.isNameAlreadyUsedForProject(name, projectParam)) { + && actionPlanService.isNameAlreadyUsedForProject(name, projectParam)) { result.addError("issues_action_plans.same_name_in_same_project"); } if (result.ok()) { DefaultActionPlan actionPlan = DefaultActionPlan.create(name) - .setProjectKey(projectParam) - .setDescription(description) - .setUserLogin(UserSession.get().login()) - .setDeadLine(deadLine); + .setProjectKey(projectParam) + .setDescription(description) + .setUserLogin(UserSession.get().login()) + .setDeadLine(deadLine); result.setObject(actionPlan); } return result; diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb index fc481b2394b..8c4b99062a6 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb @@ -205,7 +205,7 @@ class Api::IssuesController < Api::ApiController # -- Optional parameters # 'severity' is in BLOCKER, CRITICAL, ... INFO. Default value is MAJOR. # 'line' starts at 1 - # 'description' is the plain-text description + # 'message' is the markdown message # # -- Example # curl -X POST -v -u admin:admin 'http://localhost:9000/api/issues/create?component=commons-io:commons-io:org.apache.commons.io.filefilter.OrFileFilter&rule=pmd:ConstructorCallsOverridableMethod&line=2&severity=BLOCKER' diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb index dc7e0d07fe2..e25ba57f6db 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb @@ -29,29 +29,27 @@ class IssueController < ApplicationController if request.xhr? render :partial => 'issue/issue', :locals => {:issue => @issue_results.issues.get(0)} else + # Used in Eclipse Plugin params[:layout] = 'false' render :action => 'show' end end + # Form used for: assign, comment, transition, change severity and plan def action_form verify_ajax_request require_parameters :id, :issue - action_type = params[:id] - - # not used yet - issue_key = params[:issue] - render :partial => "issue/#{action_type}_form" end def do_action + verify_ajax_request verify_post_request require_parameters :id, :issue - issue_key = params[:issue] action_type = params[:id] + issue_key = params[:issue] if action_type=='comment' Internal.issues.addComment(issue_key, params[:text]) @@ -70,7 +68,7 @@ class IssueController < ApplicationController render :partial => 'issue/issue', :locals => {:issue => @issue_results.issues.get(0)} end - # form to edit comment + # Form used to edit comment def edit_comment_form verify_ajax_request require_parameters :id @@ -80,7 +78,9 @@ class IssueController < ApplicationController render :partial => 'issue/edit_comment_form' end + # Edit and save an existing comment def edit_comment + verify_ajax_request verify_post_request require_parameters :key, :text @@ -91,16 +91,17 @@ class IssueController < ApplicationController render :partial => 'issue/issue', :locals => {:issue => @issue_results.issues.get(0)} end - # modal window to delete comment + # Form in a modal window to delete comment def delete_comment_form verify_ajax_request require_parameters :id - render :partial => 'issue/delete_comment_form' end + # Delete an existing comment def delete_comment verify_post_request + verify_ajax_request require_parameters :id comment = Internal.issues.deleteComment(params[:id]) @@ -109,14 +110,17 @@ class IssueController < ApplicationController render :partial => 'issue/issue', :locals => {:issue => @issue_results.issues.get(0)} end - + # Form used to create a manual issue def create_form verify_ajax_request + require_parameters :component render :partial => 'issue/create_form' end + # Create a manual issue def create verify_post_request + verify_ajax_request require_parameters :rule, :component component_key = params[:component] @@ -131,6 +135,8 @@ class IssueController < ApplicationController render :partial => 'issue/issue', :locals => {:issue => @issue_results.issues.get(0)} end + + # # # ACTIONS FROM THE ISSUES WIDGETS diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_create_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_create_form.html.erb index 3daf08f9cbb..95e12c5d73d 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_create_form.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_create_form.html.erb @@ -16,24 +16,40 @@
- <% unless manual_rules.empty? %> - <%= dropdown_tag 'rule', - options_for_select([[]].concat(manual_rules.map{|rule| [rule.name, rule.key] })), - {:show_search_box => true, :placeholder => 'Select a Rule', :open => true}, - {:html_id => "#{form_id}-rules"} -%> - <% end %> - <% if is_admin %> - Add Rule - <% end %> + + + + + +
+ <% unless manual_rules.empty? %> + <%= dropdown_tag 'rule', + options_for_select([[]].concat(manual_rules.map { |rule| [rule.name, rule.key] })), + {:show_search_box => true, :placeholder => 'Select a Rule'}, + {:html_id => "#{form_id}-rules"} -%> + <% end %> + + <% if is_admin %> + Manage Rules + <% end %> +
+
- - -  <%= link_to_function message('cancel'), 'closeMIF(this)' -%> + + + + + + + +
+ +
+  <%= link_to_function message('cancel'), 'closeMIF(this)' -%> +
-
- <% end %> \ No newline at end of file diff --git a/sonar-server/src/main/webapp/stylesheets/style.css b/sonar-server/src/main/webapp/stylesheets/style.css index 73b2dafc0ce..6f9c6471e23 100644 --- a/sonar-server/src/main/webapp/stylesheets/style.css +++ b/sonar-server/src/main/webapp/stylesheets/style.css @@ -699,7 +699,7 @@ ul.operations li img { } .sources2 tr.row:hover td.plus { - background: url("../images/add.png") no-repeat scroll left 50% #EFEFEF; + background: url("../images/add.png") no-repeat scroll left 0 #EFEFEF; } .sources2 td.plus a { @@ -892,18 +892,24 @@ span.rulename a:hover { vertical-align: text-bottom; } +.code-issue-create-form { + padding-bottom: 10px; +} + .code-global-issues { padding: 10px; } + .code-issues { background-color: #FFF; + padding-bottom: 10px; } .code-issue { background-color: #FFF; margin: 0; font-size: 12px; - padding: 5px 10px; + padding: 10px 10px 0 10px; } .code-issue-name { diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/NewIssue.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/NewIssue.java index f4b43a39385..4b8932c2a11 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/NewIssue.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/NewIssue.java @@ -19,6 +19,7 @@ */ package org.sonar.wsclient.issue; +import javax.annotation.Nullable; import java.util.HashMap; import java.util.Map; @@ -40,7 +41,10 @@ public class NewIssue { return params; } - public NewIssue severity(String s) { + /** + * Optionally set the severity, in INFO, MINOR, MAJOR, CRITICAL or BLOCKER. Default value is MAJOR. + */ + public NewIssue severity(@Nullable String s) { params.put("severity", s); return this; } @@ -50,31 +54,28 @@ public class NewIssue { return this; } + /** + * Rule key prefixed by the repository, for example "checkstyle:AvoidCycle". + */ public NewIssue rule(String s) { params.put("rule", s); return this; } /** - * Optional line + * Optional line, starting from 1. */ - public NewIssue line(int i) { + public NewIssue line(@Nullable Integer i) { params.put("line", i); return this; } - public NewIssue description(String s) { - params.put("desc", s); - return this; - } - - // TODO to be removed - public NewIssue cost(Double d) { - params.put("cost", d); + public NewIssue message(@Nullable String s) { + params.put("message", s); return this; } - public NewIssue effortToFix(double d) { + public NewIssue effortToFix(@Nullable Double d) { params.put("effortToFix", d); return this; } diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/NewIssueTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/NewIssueTest.java index a6e2e9d7d40..75c9d3c3738 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/NewIssueTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/NewIssueTest.java @@ -35,7 +35,7 @@ public class NewIssueTest { NewIssue newIssue = NewIssue.create() .component("Action.java") .effortToFix(4.2) - .description("the desc") + .message("the message") .line(123) .rule("squid:AvoidCycle") .severity("BLOCKER") @@ -44,11 +44,23 @@ public class NewIssueTest { assertThat(newIssue.urlParams()).hasSize(7).includes( entry("component", "Action.java"), entry("effortToFix", 4.2), - entry("desc", "the desc"), + entry("message", "the message"), entry("line", 123), entry("rule", "squid:AvoidCycle"), entry("severity", "BLOCKER"), entry("attr[JIRA]", "FOO-123") ); } + + @Test + public void create_with_only_required_parameters() { + NewIssue newIssue = NewIssue.create() + .component("Action.java") + .rule("squid:AvoidCycle"); + + assertThat(newIssue.urlParams()).hasSize(2).includes( + entry("component", "Action.java"), + entry("rule", "squid:AvoidCycle") + ); + } } -- 2.39.5