From: Fabrice Bellingard <bellingard@gmail.com>
Date: Wed, 23 May 2012 12:56:33 +0000 (+0200)
Subject: SONAR-2706 Clean code
X-Git-Tag: 3.1~101
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=4f38ae1ac08c81cef3668879f12506e9155dab95;p=sonarqube.git

SONAR-2706 Clean code
---

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 6dcddaaa072..f0a01936a8d 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
@@ -201,17 +201,14 @@ class ReviewsController < ApplicationController
         :user => current_user)
     end
 
-    if !params[:text].blank? || !params[:review_command_id].blank?
+    unless params[:text].blank?
       if params[:comment_id]
         violation.review.edit_comment(current_user, params[:comment_id].to_i, params[:text])
       else
-        violation.review.create_comment({:user => current_user, :text => params[:text]}, params[:review_command_id])
+        violation.review.create_comment(:user => current_user, :text => params[:text])
       end
     end
 
-    # Needs to reload as the review may have been changed on the Java side by a ReviewAction
-    violation.review.reload
-
     display_violation
   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 25bac3056ec..20e38d3010f 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
@@ -81,19 +81,8 @@ class Review < ActiveRecord::Base
   # params of 'comment_values' are mandatory:
   # - :user
   # - :text
-  # 
-  # Note: 'review_command_id' is optional (=> specifies which command was triggered instead of creating a simple comment)
-  def create_comment(comment_values={}, review_command_id=nil)
-    if review_command_id
-      review_context = Api::ReviewContext.new(:review => self, :user => User.new(:login => comment_values[:user].login), :params => {"comment.text" => comment_values[:text]})
-      Java::OrgSonarServerUi::JRubyFacade.getInstance().executeCommandActions(review_command_id, review_context.to_string_map)
-      # need to reload the comments as the executed action will have created a new one
-      comments.reload
-      comment = comments.last
-    else
-      # simple comment
-      comment = comments.create!(comment_values)
-    end
+  def create_comment(comment_values={})
+    comment = comments.create!(comment_values)
     touch
     notification_manager.notifyChanged(id.to_i, comment.user.login.to_java, to_java_map, to_java_map("comment" => comment.text))
   end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/project_reviews/_view.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/project_reviews/_view.html.erb
index 7f4b77d1e2a..7f52dafd51a 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/project_reviews/_view.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/project_reviews/_view.html.erb
@@ -1,3 +1,7 @@
 <div id="review">
-  <%= render :partial => 'project_reviews/review', :locals => {:review => @review} -%>
+  <%
+    # hack in case 'error_message' is nil (this should disappear when refactoring the '_view' and '_review' partials) 
+    error_message = error_message 
+  %>
+  <%= render :partial => 'project_reviews/review', :locals => {:review => @review, :error_message => error_message} -%>
 </div>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_comment_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_comment_form.html.erb
index 1f8c3c71696..152fa5baccc 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_comment_form.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_violation_comment_form.html.erb
@@ -1,9 +1,5 @@
 <%
-   if @review_command
-     button_label = Review.command_label(@review_command)
-   else
-     button_label=(@comment ? message('reviews.update_comment_submit') : message('reviews.comment_submit'))
-   end
+   button_label=(@comment ? message('reviews.update_comment_submit') : message('reviews.comment_submit'))
 %>
 <form method="POST"
       onsubmit="new Ajax.Updater('vId<%= params[:id] -%>', '<%= url_for :action => 'violation_save_comment' -%>', {asynchronous:true, evalScripts:true, parameters:Form.serialize(this)});return false;">
@@ -11,17 +7,12 @@
   <% if @comment %>
     <input type="hidden" name="comment_id" value="<%= @comment.id -%>"/>
   <% end %>
-  <% if @review_command %>
-    <input type="hidden" name="review_command_id" value="<%= @review_command.getId() -%>"/>
-  <% end %>
 
   <table class="width100">
     <tr>
       <td style="vertical-align:top">
         <textarea id="commentText<%= params[:id] -%>"
-                  <% unless @review_command -%>
                   onkeyup="if (this.value=='') $('submit_btn<%= params[:id] -%>').disabled='true'; else $('submit_btn<%= params[:id] -%>').disabled='';"
-                  <% end %>
                   rows="4" name="text" style="width: 100%"><%= @comment.text if @comment -%></textarea>
       </td>
       <td class="sep"></td>
@@ -36,7 +27,7 @@
     <%= submit_to_remote "submit_btn"+params[:id],
                          button_label,
                          :url => {:action => 'violation_save_comment'},
-                         :html => {:id => "submit_btn"+params[:id]}.merge(@review_command ? {} : {:disabled => "true"}),
+                         :html => {:id => "submit_btn"+params[:id], :disabled => "true"},
                          :before => "$('loading-#{params[:id]}').show();",
                          :update => 'vId'+params[:id] -%>
     &nbsp;