]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-2327 first step to refactor reviews page
authorsimonbrandhof <simon.brandhof@gmail.com>
Wed, 20 Apr 2011 21:48:39 +0000 (23:48 +0200)
committersimonbrandhof <simon.brandhof@gmail.com>
Thu, 21 Apr 2011 16:29:45 +0000 (18:29 +0200)
SONAR-2327 add link to review

SONAR-2327 render review results in a table

sonar-server/src/main/webapp/WEB-INF/app/controllers/reviews_controller.rb
sonar-server/src/main/webapp/WEB-INF/app/helpers/reviews_helper.rb
sonar-server/src/main/webapp/WEB-INF/app/models/project.rb
sonar-server/src/main/webapp/WEB-INF/app/models/review.rb
sonar-server/src/main/webapp/WEB-INF/app/models/severity.rb [new file with mode: 0644]
sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_show.html.erb [new file with mode: 0644]
sonar-server/src/main/webapp/WEB-INF/app/views/reviews/index.html.erb
sonar-server/src/main/webapp/WEB-INF/db/migrate/191_create_review.rb
sonar-server/src/main/webapp/images/status/closed.png [new file with mode: 0644]
sonar-server/src/main/webapp/images/status/open.png [new file with mode: 0644]
sonar-server/src/main/webapp/stylesheets/style.css

index dd98d5a3ecc486cbf5c12aba238bef94f397346c..55bd4c558967449706b84462771c3fac0d9f1fa3 100644 (file)
@@ -26,11 +26,12 @@ class ReviewsController < ApplicationController
 
   def index
     init_params
-
-    @reviews = []
-    unless params.blank?
-      find_reviews_for_user_query
-    end
+    search_reviews
+  end
+  
+  def show
+    @review=Review.find(params[:id], :include => ['resource', 'project'])
+    render :partial => 'reviews/show'
   end
 
   def list
@@ -202,15 +203,13 @@ class ReviewsController < ApplicationController
 
   def init_params
     @user_names = [["Any", ""]]
-    default_user = [""]
-    if current_user
-      default_user = [current_user.id]
-    end
+    default_user = (current_user ? [current_user.id.to_s] : [''])
     add_all_users @user_names
-    @review_authors = filter_any(params[:review_authors]) || default_user
-    @comment_authors = filter_any(params[:comment_authors]) || default_user
-    @severities = filter_any(params[:severities]) || [""]
-    @statuses = filter_any(params[:statuses]) || ["open"]
+    @authors = filter_any(params[:authors]) || ['']
+    @assignees = filter_any(params[:assignees]) || default_user
+    @severities = filter_any(params[:severities]) || ['']
+    @statuses = filter_any(params[:statuses]) || [Review::STATUS_OPEN]
+    @projects = filter_any(params[:projects]) || ['']
   end
   
   def add_all_users ( user_options )
@@ -231,28 +230,28 @@ class ReviewsController < ApplicationController
     array
   end
 
-  def find_reviews_for_user_query
+  def search_reviews
     conditions=[]
     values={}
 
     unless @statuses == [""]
-      conditions << "reviews.status in (:statuses)"
+      conditions << "status in (:statuses)"
       values[:statuses]=@statuses
     end
     unless @severities == [""]
-      conditions << "reviews.severity in (:severities)"
+      conditions << "severity in (:severities)"
       values[:severities]=@severities
     end
-    unless @review_authors == [""]
-      conditions << "reviews.user_id in (:review_authors)"
-      values[:review_authors]=@review_authors
+    unless @authors == [""]
+      conditions << "user_id in (:authors)"
+      values[:authors]=@authors
     end
-    unless @comment_authors == [""]
-      conditions << "review_comments.user_id in (:comment_authors)"
-      values[:comment_authors]=@comment_authors
+    unless @assignees == [""]
+      conditions << "assignee_id in (:assignees)"
+      values[:assignees]=@assignees
     end
 
-    @reviews = Review.find( :all, :order => "created_at DESC", :joins => :review_comments, :conditions => [ conditions.join(" AND "), values] ).uniq
+    @reviews = Review.find( :all, :order => "created_at DESC", :conditions => [ conditions.join(" AND "), values] ).uniq
   end
 
   def find_reviews_for_rule_failure ( rule_failure_permanent_id )
index c615c99a82f319af8bf616c12b49886932401973..7c51816f33348c7b6ef87fc64bd01e14b3607d2c 100644 (file)
 #
 module ReviewsHelper
   
-  def self.severity_options_with_any
-    severity_ops = []
-    severity_ops << ["Any", ""]
-    Review.severity_options.each do |severity|
-      severity_ops << severity
+  def options_for_project_select
+    options=[['Any', '']]
+    projects=Project.find(:all, :select => 'id,name', :conditions => ['enabled=? AND scope=? AND qualifier IN (?)', true, 'PRJ', ['TRK', 'VW','SVW']], :order => 'name ASC')
+    projects.each do |project|
+      options<<[project.name, project.id]
     end
-    return severity_ops
+    options_for_select(options, @projects)
   end
   
-  def self.status_options_with_any
-    status_ops = []
-    status_ops << ["Any", ""]
-    Review.status_options.each do |status|
-      status_ops << status
-    end
-    return status_ops
-  end
-
 end
index d1fd310a0861e31574e1ebfbc2d0a4c1a6237e26..8489caea88b6c13e2834b35639f7a7e615d1469c 100644 (file)
@@ -28,7 +28,7 @@ class Project < ActiveRecord::Base
   belongs_to :profile, :class_name => 'Profile', :foreign_key => 'profile_id'
   has_many :user_roles, :foreign_key => 'resource_id'
   has_many :group_roles, :foreign_key => 'resource_id'
-
+  belongs_to :root, :class_name => 'Project', :foreign_key => 'root_id'
 
   def self.by_key(k)
     begin
@@ -38,7 +38,11 @@ class Project < ActiveRecord::Base
       Project.find(:first, :conditions => {:kee => k})
     end
   end
-
+  
+  def project
+    root||self
+  end
+  
   def last_snapshot
     @last_snapshot ||=
       begin
index 8cd7857e7efd67bab7f99ba17a0bca24db071d10..3849b2ec04b791529b723ea80dd941f6543a58cc 100644 (file)
@@ -21,27 +21,25 @@ class Review < ActiveRecord::Base
   belongs_to :user
   belongs_to :assignee, :class_name => "User", :foreign_key => "assignee_id"
   belongs_to :resource, :class_name => "Project", :foreign_key => "resource_id"
+  belongs_to :project, :class_name => "Project", :foreign_key => "project_id"
   has_many :review_comments, :order => "created_at", :dependent => :destroy
+  belongs_to :rule_failure, :foreign_key => 'rule_failure_permanent_id'
+  
   validates_presence_of :user, :message => "can't be empty"
   validates_presence_of :title, :message => "can't be empty"
   validates_presence_of :review_type, :message => "can't be empty"
   validates_presence_of :status, :message => "can't be empty"
-
-  SEVERITY_INFO = "INFO"
-  SEVERITY_MINOR = "MINOR"
-  SEVERITY_MAJOR = "MAJOR"
-  SEVERITY_CRITICAL = "CRITICAL"
-  SEVERITY_BLOCKER = "BLOCKER"
+  
+  before_save :assign_project
   
   TYPE_COMMENTS = "comments"
   TYPE_FALSE_POSITIVE = "f-positive"
   
   STATUS_OPEN = "open"
   STATUS_CLOSED = "closed"
-
-
+  
   def self.default_severity
-    return SEVERITY_MAJOR
+    return Severity::MAJOR
   end
   
   def self.default_type
@@ -51,22 +49,16 @@ class Review < ActiveRecord::Base
   def self.default_status
     return STATUS_OPEN
   end
-  
-  def self.severity_options
-    severity_ops = []
-    severity_ops << ["Info", SEVERITY_INFO]
-    severity_ops << ["Minor", SEVERITY_MINOR]
-    severity_ops << ["Major", SEVERITY_MAJOR]
-    severity_ops << ["Critical", SEVERITY_CRITICAL]
-    severity_ops << ["Blocker", SEVERITY_BLOCKER]
-    return severity_ops
+    
+  def source?
+    resource!=nil && resource.last_snapshot!=nil && resource.last_snapshot.source!=nil
   end
+
+  private
   
-  def self.status_options
-    status_ops = []
-    status_ops << ["Open", STATUS_OPEN]
-    status_ops << ["Closed", STATUS_CLOSED]
-    return status_ops
+  def assign_project
+    if self.project.nil? && self.resource
+      self.project=self.resource.project
+    end
   end
-
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/severity.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/severity.rb
new file mode 100644 (file)
index 0000000..f62e477
--- /dev/null
@@ -0,0 +1,30 @@
+#
+# Sonar, entreprise quality control tool.
+# Copyright (C) 2008-2011 SonarSource
+# mailto:contact AT sonarsource DOT com
+#
+# Sonar is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 3 of the License, or (at your option) any later version.
+#
+# Sonar is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with {library}; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+#
+
+class Severity
+
+  INFO = "INFO"
+  MINOR = "MINOR"
+  MAJOR = "MAJOR"
+  CRITICAL = "CRITICAL"
+  BLOCKER = "BLOCKER"
+  
+  SEVERITIES=[INFO,MINOR,MAJOR,CRITICAL,BLOCKER]
+end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_show.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/reviews/_show.html.erb
new file mode 100644 (file)
index 0000000..499fd21
--- /dev/null
@@ -0,0 +1,13 @@
+<div>
+       <a href="#" onclick="backReviews()">Back to results</a>
+</div>
+
+<h1><%= h(@review.project.long_name) -%> | <%= h(@review.resource.long_name) -%></h1>
+
+<% if @review.source? %>
+
+<% end %>
+
+<%= @review.title -%>
+
+
index 9394ad52c41a588506431a0a9e1c31cb1206a8f1..a2a06f63c8d45c4e7e07ee74e16382af082a985f 100644 (file)
-<h1>Reviews</h1>
+<script>
+function onReviewLoading() {
+       $('reviews-search').hide();
+       $('review').hide();
+       $('review-loading').show();
+}
+function onReviewLoaded() {
+       $('reviews-search').hide();
+       $('review-loading').hide();
+       $('review').show();
+}
+function backReviews() {
+       $('review').hide();
+       $('review-loading').hide();
+    $('reviews-search').show();        
+}
+</script>
 
-<div class="tabs-panel marginbottom10 background-gray">
+<div id="reviews-search">
+  <h1>Reviews</h1>
   <% form_tag({:action => 'index'}, {:method => 'get'}) do %>
-   <table class="spaced" id="search_table">
+   <table id="reviews-form" class="search-filter-box">
+        <thead>
+               <tr><th colspan="6"></th></tr>
+       </thead>
+       <tbody>
      <tr>
-       <td class="left" valign="top" width="1%" nowrap>
-         <span class="note">Created by</span><br/>
-         <%= select_tag "review_authors", options_for_select(@user_names, @review_authors), :multiple => true, :size => 6 %>
+          <td width="1%" nowrap>
+         <span class="note">Status</span><br/>
+                <select size="6" name="statuses[]" multiple="multiple" id="statuses" class="with-icons">
+                       <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_CLOSED -%>" class="status_closed" <%= 'selected' if @statuses.include?(Review::STATUS_CLOSED) -%>>Closed</option>
        </td>
-       <td class="left" valign="top" width="1%" nowrap>
-         <span class="note">Commented by</span><br/>
-         <%= select_tag "comment_authors", options_for_select(@user_names, @comment_authors), :multiple => true, :size => 6 %>
+
+       <td width="1%" nowrap>
+         <span class="note">Project</span><br/>
+         <%= select_tag "projects", options_for_project_select(), :multiple => true, :size => 6 %>
        </td>
-       <td class="left" valign="top" width="1%" nowrap>
+       <td width="1%" nowrap>
          <span class="note">Severity</span><br/>
-         <%= select_tag "severities", options_for_select(ReviewsHelper.severity_options_with_any, @severities), :multiple => true, :size => 6 %>
+               <select size="6" name="severities[]" multiple="multiple" id="severities" class="with-icons">
+          <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>
        </td>
-       <td class="left" valign="top" width="1%" nowrap>
-         <span class="note">Status</span><br/>
-         <%= select_tag "statuses", options_for_select(ReviewsHelper.status_options_with_any, @statuses), :multiple => true, :size => 6 %>
+    
+       <td width="1%" nowrap>
+         <span class="note">Created by</span><br/>
+         <%= select_tag "authors", options_for_select(@user_names, @authors), :multiple => true, :size => 6 %>
+       </td>
+       <td width="1%" nowrap>
+         <span class="note">Assigned to</span><br/>
+         <%= select_tag "assignees", options_for_select(@user_names, @assignees), :multiple => true, :size => 6 %>
        </td>
-       <td class="left" valign="top" >
+       <td>
          <br/>
          <%= submit_tag "Search", :id => 'submit_search' %>
        </td>
      </tr>
+    </tbody>
    </table>
-  <% end %>
-</div>
+<% end %>
+
 
-<div id="reviewList">
 <% 
-   unless @reviews.blank? 
+   if @reviews && !@reviews.empty?
+%>
+  <table id="reviews-list" class="data width100">
+       <thead>
+               <tr>
+                       <th width="1%" nowrap>St.</th>
+                       <th width="1%">Project</th>
+                       <th>Title</th>
+                       <th width="1%" nowrap>Se.</th>
+                       <th>Assignee</th>
+                       <th>Age</th>
+                       
+               </tr>
+       </thead>
+       <tfoot>
+               <tr><td colspan="6"><%= @reviews.size -%> results</tr>
+       </tfoot>
+       <tbody>
+<%
      @reviews.each do |review|
 %>
-
-  <%= render :partial => "reviews/view", :locals => { :review => review } %>
-
+      <tr class="<%= cycle('even', 'odd') -%>">
+           <td><img src="<%= ApplicationController.root_context -%>/images/status/<%= review.status -%>.png"/></td>
+       <td><%= review.project.name -%><br/><span class="note"><%= review.resource.long_name -%></span></td>
+           <td>
+             <%= link_to_remote(h(review.title), :update => 'review', :url => {:action => 'show', :id => review.id}, :loading => 'onReviewLoading()', :complete => "onReviewLoaded()") -%>
+           </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>
+      </tr>
 <% 
      end
+%> 
+   </tbody>
+  </table>
+<%
+   elsif @reviews
+%>
+  <p>No results</p>
+<%     
    end
 %>
-<div>
+</div>
+       
+<div id="review-loading" style="display: none"><%= image_tag 'loading.gif' -%></div>
+<div id="review" style="display: none"></div>
 
index 937c1f60ac99a282a12d9dffc3e68a754725d914..e2827654bed63f2f26546036dfeb2063cfa68189 100644 (file)
@@ -26,7 +26,7 @@ class CreateReview < ActiveRecord::Migration
   def self.up
     create_table 'reviews' do |t|
       t.column 'created_at',                                   :datetime
-      t.column 'updated_at',                                   :datetime
+      t.column 'updated_at',                                   :datetime,  :null => true
       t.column 'user_id',                                              :integer,       :null => true
       t.column 'assignee_id',                                  :integer,       :null => true
       t.column 'title',                                                        :string,        :null => true,  :limit => 500
@@ -34,6 +34,7 @@ class CreateReview < ActiveRecord::Migration
       t.column 'status',                                               :string,        :null => true,  :limit => 10
       t.column 'severity',                                             :string,        :null => true,  :limit => 10
       t.column 'rule_failure_permanent_id',    :integer,       :null => true   
+         t.column 'project_id',                                        :integer,       :null => true
       t.column 'resource_id',                                  :integer,       :null => true   
       t.column 'resource_line',                                :integer,       :null => true      
     end
diff --git a/sonar-server/src/main/webapp/images/status/closed.png b/sonar-server/src/main/webapp/images/status/closed.png
new file mode 100644 (file)
index 0000000..5a2fef4
Binary files /dev/null and b/sonar-server/src/main/webapp/images/status/closed.png differ
diff --git a/sonar-server/src/main/webapp/images/status/open.png b/sonar-server/src/main/webapp/images/status/open.png
new file mode 100644 (file)
index 0000000..7eeb16d
Binary files /dev/null and b/sonar-server/src/main/webapp/images/status/open.png differ
index 9acfbd11d19df45d830fd9621f7a7d851e915f8b..16eb2895a8fa3c341e7a928d83eb2f9887b5a10f 100644 (file)
@@ -1036,7 +1036,44 @@ span.rulename a:hover {
   padding: 10px;
 }
 
-
+.search-filter-box {
+       background-color: #ECECEC;
+       color: #444;
+       border: 1px solid #DDD;
+       margin: 0 0 10px 0;
+}
+.search-filter-box td {
+       padding: 10px;
+       text-align: left;
+       vertical-align: top;
+}
+select.with-icons option {
+       background-repeat: no-repeat;
+       background-position: 2px 0;
+       padding: 0 2px 0 22px;
+       vertical-align: middle;
+}
+option.status_open {
+       background-image: url('../images/status/open.png');
+}
+option.status_closed {
+       background-image: url('../images/status/closed.png');
+}
+option.sev_INFO {
+       background-image: url('../images/priority/INFO.png');
+}
+option.sev_MINOR {
+       background-image: url('../images/priority/MINOR.png');
+}
+option.sev_MAJOR {
+       background-image: url('../images/priority/MAJOR.png');
+}
+option.sev_CRITICAL {
+       background-image: url('../images/priority/CRITICAL.png');
+}
+option.sev_BLOCKER {
+       background-image: url('../images/priority/BLOCKER.png');
+}
 /* ------------------- VARIATIONS ------------------- */
 .var {
   color: #444 !important;