SONAR-4304 better form of manual issue

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 @@
 
     <div class="code-issue">
       <div class="code-issue-name">
-        <% 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 %>
-          <a href="#">Add Rule</a>
-        <% end %>
+        <table class="width100">
+          <tr>
+            <td class="thin spacer-right">
+              <% 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 %>
+            </td>
+            <td align="left">
+              <% if is_admin %>
+                <a href="<%= ApplicationController.root_context -%>/manual_rules/index">Manage Rules</a>
+              <% end %>
+            </td>
+          </tr>
+        </table>
+
       </div>
 
       <div class="code-issue-msg">
-        <textarea rows="4" name="message" class="width100 marginbottom5"></textarea>
-
-        <input type="submit" value="Create" onclick="return submitMIF(this);"> &nbsp;<%= link_to_function message('cancel'), 'closeMIF(this)' -%>
+        <table class="width100">
+          <tr>
+            <td>
+              <textarea rows="4" name="message" class="width100 marginbottom5"></textarea>
+            </td>
+          </tr>
+          <tr>
+            <td>
+              <input type="submit" value="Create" onclick="return submitMIF(this);"> &nbsp;<%= link_to_function message('cancel'), 'closeMIF(this)' -%>
+            </td>
+          </tr>
+        </table>
       </div>
-      <div class="code-issue-form hidden"></div>
     </div>
   </form>
-
 <% 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")
+    );
+  }
 }