SONAR-1973, SONAR-2327 Fix issues based on Evgeny's feedback

6 files changed, 85 insertions, 56 deletions

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 69848be04e1..c037867e9c3 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
@@ -33,6 +33,19 @@ class ReviewsController < ApplicationController
     search_reviews()
   end
   
+  # Used for the permalink, e.g. http://localhost:9000/reviews/view/1
+  def view
+    @review=Review.find(params[:id], :include => ['resource', 'project'])
+    render 'reviews/_review', :locals => {:review => @review}
+  end
+
+
+  #
+  #
+  # ACTIONS FROM REVIEW SERVICE PAGE
+  #
+  #
+
   def show
     @review=Review.find(params[:id], :include => ['resource', 'project'])
     render :partial => 'reviews/show'
@@ -262,7 +275,7 @@ class ReviewsController < ApplicationController
     @severities = filter_any(params[:severities]) || ['']
     @statuses = filter_any(params[:statuses]) || [Review::STATUS_OPEN]
     @projects = filter_any(params[:projects]) || ['']
-    @id = params[:id] || ""
+    @id = params[:review_id] || ""
   end
 
   def options_for_users
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 aa187b7b5be..bd4e5c8e920 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
@@ -2,7 +2,7 @@
 <div class="violation">
   <div class="vtitle">
     <% if violation.review %>
-      <div style="float: right"><span class="violation_date">#<%= violation.review.id -%></span></div>
+      <div style="float: right"><span class="review_permalink"><%= link_to "#"+violation.review.id.to_s, :controller => "reviews", :action => "view", :id => violation.review.id -%></span></div>
     <% end %>
     
     <%= image_tag("priority/" + violation.failure_level.to_s + '.png') -%>
@@ -76,21 +76,34 @@
   %>
   <div class="discussionComment">
     <h4><%= image_tag("reviews/comment.png") -%> &nbsp;<b><%= review_comment.user.name -%></b> (<%= distance_of_time_in_words_to_now(review_comment.created_at) -%>)
-    <% if is_last_comment && current_user && current_user.id == review_comment.user_id %>
-      <span class="actions">
+    <!-- && current_user.id == review_comment.user_id -->
+    <% if is_last_comment && current_user %>
+      <span class="actions" id="commentActions<%= violation.id -%>">
         &nbsp;&nbsp;
         <%= image_tag("sep12.png") -%>
         &nbsp;&nbsp;
-        <%= link_to_remote "Edit", 
-      		:url => { :controller => "reviews", :action => "violation_comment_form", :comment_id => review_comment.id, :id => violation.id },
-      		:update => "lastComment" + violation.id.to_s,
-      		:complete => "$('reviewForm#{violation.id}').hide();$('vActions#{violation.id}').hide();$('commentAction#{violation.id}').hide();$('commentText#{violation.id}').focus();" -%>
-        <% unless comment_index == 0 %>
+        <%= link_to_remote "Add comment",
+      		:url => { :controller => "reviews", :action => "violation_comment_form", :id => violation.id },
+      		:update => "reviewForm" + violation.id.to_s,
+      		:complete => "$('vActions#{violation.id}').hide();$('commentActions" + violation.id.to_s + "').hide();$('reviewForm" + violation.id.to_s + "').show();$('commentText" + violation.id.to_s + "').focus()" -%>
+        <% 
+          if current_user.id == review_comment.user_id 
+        %>
+          &nbsp;
+          <%= link_to_remote "Edit", 
+            :url => { :controller => "reviews", :action => "violation_comment_form", :comment_id => review_comment.id, :id => violation.id },
+            :update => "lastComment" + violation.id.to_s,
+            :complete => "$('reviewForm#{violation.id}').hide();$('vActions#{violation.id}').hide();$('commentAction#{violation.id}').hide();$('commentText#{violation.id}').focus();" -%>
+          <% unless comment_index == 0 %>
+          &nbsp;
           <%= 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 ?" -%>
-        <% end %>
+          <% end %>
+        <%
+          end 
+        %>
       </span>
     <% end %>
     </h4> 
@@ -110,15 +123,4 @@
   <div class="discussionComment" id="reviewForm<%= violation.id -%>" style="display:none"></div>
 </div>
 
-
-
-<% if current_user && violation.review %>
-<div style="padding: 5px" id="commentAction<%= violation.id -%>">
-  <%= link_to_remote "Add comment",
-      		:url => { :controller => "reviews", :action => "violation_comment_form", :id => violation.id },
-      		:update => "reviewForm" + violation.id.to_s,
-      		:complete => "$('vActions#{violation.id}').hide();$('commentAction" + violation.id.to_s + "').hide();$('reviewForm" + violation.id.to_s + "').show();$('commentText" + violation.id.to_s + "').focus()" -%>
-</div>
-<% end %>
-
 </div>
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/index.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/index.html.erb
index b8e8092b36e..81daaa4ca63 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/index.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/index.html.erb
@@ -105,8 +105,11 @@
       <% end %>
       <td class="lid section"></td>
       <td class="violations">
-        <% line.violations.each do |violation| %>
+        <% line.violations.each_with_index do |violation, index| %>
           <%= render :partial => 'violation', :locals => {:violation => violation} -%>
+          <% if index < line.violations.size-1 %>
+            &nbsp;
+          <% end %>
         <% end %>
       </td>
     </tr>
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 2cb37687ff4..ef40b132d6c 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
@@ -95,21 +95,33 @@
 	   <div class="discussionComment">
 		 <h4>
 		   <%= image_tag("reviews/comment.png") -%> &nbsp;<b><%= comment.user.name -%></b> (<%= distance_of_time_in_words_to_now(comment.created_at) -%>)
-           <% if is_last_comment && current_user && current_user.id == comment.user_id %>
+           <% if is_last_comment && current_user %>
            <span class="actions" id="editActions">
              &nbsp;&nbsp;
              <%= image_tag("sep12.png") -%>
              &nbsp;&nbsp;
-             <%= link_to_remote "Edit", 
+             <%= link_to_remote "Add comment",
+      		     :url => { :controller => "reviews", :action => "comment_form", :id => review.id },
+      		     :update => "reviewForm",
+      		     :complete => "$('rActions').hide();$('editActions').hide();$('reviewForm').show();$('commentText').focus();" -%>
+             <%
+               if current_user.id == comment.user_id
+             %>
+               &nbsp;
+               <%= link_to_remote "Edit", 
                  :url => { :controller => "reviews", :action => "comment_form", :comment_id => comment.id, :id => review.id },
       		     :update => "lastComment",
-      		     :complete => "$('rActions').hide();$('commentAction').hide();$('editActions').hide();$('commentText').focus();" -%>
-      		 <% unless comment_index == 0 %>
+      		     :complete => "$('rActions').hide();$('editActions').hide();$('commentText').focus();" -%>
+      		   <% unless comment_index == 0 %>
+               &nbsp;
                <%= link_to_remote "Delete",
       		     :url => { :controller => "reviews", :action => "delete_comment", :comment_id => comment.id, :id => review.id },
       	         :update => "review",
                  :confirm => "Do you want to delete this comment ?" -%>
-             <% end %>
+               <% end %>
+             <% 
+               end 
+             %>
            </span>
            <% end %>
          </h4>
@@ -124,14 +136,6 @@
 	 <% end %>
   </div>
   
-  <% if current_user && review.status != "CLOSED" %>
   <div class="discussionComment" id="reviewForm" style="display:none"></div>
-  <div style="padding: 5px" id="commentAction">
-    <%= link_to_remote "Add comment",
-      		:url => { :controller => "reviews", :action => "comment_form", :id => review.id },
-      		:update => "reviewForm",
-      		:complete => "$('rActions').hide();$('commentAction').hide();$('reviewForm').show();$('commentText').focus();$('editActions').hide();" -%>
-  </div>
-  <% end %>
   
 </div>
\ No newline at end of file
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 ca4b8de4763..d77813855a7 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
@@ -35,6 +35,17 @@
             <option value="<%= Review::STATUS_CLOSED -%>" class="status_closed" <%= 'selected' if @statuses.include?(Review::STATUS_CLOSED) -%>>Closed</option>
           </select>
         </td>
+        <td width="1%" nowrap>
+          <span class="note">Severity</span><br/>
+          <select size="6" name="severities[]" multiple="multiple" id="severities" class="withIcons">
+            <option <%= 'selected' if @severities.include?('') -%> value="">Any</option>
+            <option value="<%= Severity::BLOCKER -%>" class="sev_BLOCKER" <%= 'selected' if @severities.include?(Severity::BLOCKER) -%>>Blocker</option>
+            <option value="<%= Severity::CRITICAL -%>" class="sev_CRITICAL" <%= 'selected' if @severities.include?(Severity::CRITICAL) -%>>Critical</option>
+            <option value="<%= Severity::MAJOR -%>" class="sev_MAJOR" <%= 'selected' if @severities.include?(Severity::MAJOR) -%>>Major</option>
+            <option value="<%= Severity::MINOR -%>" class="sev_MINOR" <%= 'selected' if @severities.include?(Severity::MINOR) -%>>Minor</option>
+            <option value="<%= Severity::INFO -%>" class="sev_INFO" <%= 'selected' if @severities.include?(Severity::INFO) -%>>Info</option>
+           </select>
+        </td>
 
         <td width="1%" nowrap>
           <span class="note">Project</span><br/>
@@ -47,17 +58,6 @@
             <% end %>
           </select>
         </td>
-        <td width="1%" nowrap>
-          <span class="note">Severity</span><br/>
-          <select size="6" name="severities[]" multiple="multiple" id="severities" class="withIcons">
-            <option <%= 'selected' if @severities.include?('') -%> value="">Any</option>
-            <option value="<%= Severity::BLOCKER -%>" class="sev_BLOCKER" <%= 'selected' if @severities.include?(Severity::BLOCKER) -%>>Blocker</option>
-            <option value="<%= Severity::CRITICAL -%>" class="sev_CRITICAL" <%= 'selected' if @severities.include?(Severity::CRITICAL) -%>>Critical</option>
-            <option value="<%= Severity::MAJOR -%>" class="sev_MAJOR" <%= 'selected' if @severities.include?(Severity::MAJOR) -%>>Major</option>
-            <option value="<%= Severity::MINOR -%>" class="sev_MINOR" <%= 'selected' if @severities.include?(Severity::MINOR) -%>>Minor</option>
-            <option value="<%= Severity::INFO -%>" class="sev_INFO" <%= 'selected' if @severities.include?(Severity::INFO) -%>>Info</option>
-           </select>
-        </td>
 
         <td width="1%" nowrap>
           <span class="note">Created by</span><br/>
@@ -69,7 +69,7 @@
         </td>
         <td width="1%" nowrap>
           <span class="note">Id</span><br/>
-          <%= text_field_tag "id", @id, :size => 10 -%>
+          <%= text_field_tag "review_id", @id, :size => 10 -%>
           <br/>
           <br/>
           <br/>
@@ -92,13 +92,13 @@
     <table id="reviews-list" class="data width100">
       <thead>
       <tr>
-        <th width="1%" nowrap>St.</th>
+        <th width="1%" nowrap></th>
+        <th width="1%" nowrap>Id</th>
+        <th width="1%" nowrap></th>
         <th>Title</th>
         <th width="1%">Project</th>
-        <th width="1%" nowrap>Se.</th>
         <th>Assignee</th>
         <th>Age</th>
-        <th width="1%" nowrap>Id</th>
 
       </tr>
       </thead>
@@ -112,18 +112,18 @@
          @reviews.each do |review|
       %>
         <tr class="<%= cycle('even', 'odd') -%>">
-          <td><img src="<%= ApplicationController.root_context -%>/images/status/<%= review.status -%>.png"/></td>
+          <td><img src="<%= ApplicationController.root_context -%>/images/status/<%= review.status -%>.png" title="<%= review.status.capitalize -%>"/></td>
+          <td>
+            <%= link_to_remote( "#"+h(review.id), :update => 'review', :url => {:action => 'show', :id => review.id}, :loading => 'onReviewLoading()', :complete => "onReviewLoaded()") -%>
+          </td>
+          <td><img src="<%= ApplicationController.root_context -%>/images/priority/<%= review.severity -%>.png" title="<%= review.severity.capitalize -%>"/></td>
           <td>
             <%= link_to_remote(h(review.title), :update => 'review', :url => {:action => 'show', :id => review.id}, :loading => 'onReviewLoading()', :complete => "onReviewLoaded()") -%>
           </td>
           <td><%= review.project.name -%>
             <br/><span class="note"><%= review.resource.long_name -%></span></td>
-          <td><img src="<%= ApplicationController.root_context -%>/images/priority/<%= review.severity -%>.png"/></td>
           <td><%= review.assignee ? h(review.assignee.name) : '-' -%></td>
           <td><%= distance_of_time_in_words_to_now(review.created_at) -%></td>
-          <td style="font-weight:bold">
-            <%= link_to_remote( "#"+h(review.id), :update => 'review', :url => {:action => 'show', :id => review.id}, :loading => 'onReviewLoading()', :complete => "onReviewLoaded()") -%>
-          </td>
         </tr>
       <%
          end
@@ -140,4 +140,7 @@
 </div>
 
 <div id="review-loading" style="display: none"><%= image_tag 'loading.gif' -%></div>
-<div id="review" style="display: none"></div>
\ No newline at end of file
+<div id="review" style="display: none"></div>
+<script>
+  $('review_id').focus();
+</script>
\ 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 e209d06550d..b00b47bc7a8 100644
--- a/sonar-server/src/main/webapp/stylesheets/style.css
+++ b/sonar-server/src/main/webapp/stylesheets/style.css
@@ -705,6 +705,10 @@ span.violation_date {
   color:#777;
   font-size:90%;
 }
+span.review_permalink a {
+  color:#777;
+  font-size:90%;
+}
 span.rulename a:hover {
   text-decoration: underline;
 }