[webui] Refactor the revision view, test it

src/api/app/assets/stylesheets/webui/application/package.css.scss

@@ -31,3 +31,7 @@ table.repostatus td {
   background-color: yellow;
   border: 1px solid black;
 }
+
+.commit_item {
+  margin: 12px 0px; 
+}

src/api/app/controllers/webui/package_controller.rb

@@ -175,15 +175,18 @@ def revisions
       flash[:error] = 'Could not access revisions'
       redirect_to :action => :show, :project => @project.name, :package => @package.name and return
     end
-    @max_revision = @package.rev.to_i
-    @upper_bound = @max_revision
+    @lastrev = @package.rev.to_i
+    @lastrev = params[:rev].to_i if params[:rev]
     if params[:showall]
-      @visible_commits = @max_revision
+      @revisions = (1..@lastrev).to_a.reverse
     else
-      @upper_bound = params[:rev].to_i if params[:rev]
-      @visible_commits = [9, @upper_bound].min # Don't show more than 9 requests
+      if @lastrev < 21
+        @revisions = (1..@lastrev).to_a.reverse
+      else
+        @revisions = []
+        @lastrev.downto(@lastrev-19) { |n| @revisions << n }
+      end
     end
-    @lower_bound = [1, @upper_bound - @visible_commits + 1].max
   end
 
   def submit_request_dialog

src/api/app/views/webui/package/_commit_item.html.erb

@@ -21,15 +21,25 @@
             committed
         <% end %>
     <% end %>
-    <%= fuzzy_time(Time.at(commit['time'].to_i)) %> (revision <%= commit['rev'] %>)<br/>
-    <% unless commit['user'] == '_service' %>
+    <%= fuzzy_time(Time.at(commit['time'].to_i)) %> (revision <%= commit['rev'] %>)
+    <% unless commit['user'] == '_service' or commit['comment'].blank? %>
         <pre class="plain"><%= commit['comment'] %></pre>
     <% end %>
-    <%= link_to(sprite_tag('brick'), {:controller => :package, :action => :rdiff,
-                                      :package => @package, :project => @project,
-                                      :rev => commit['rev'], :linkrev => 'base'}, :style => 'margin-left: 2em') %>
-    <%= link_to('Show diff', :controller => :package, :action => :rdiff,
-                :package => @package, :project => @project, :rev => commit['rev'], :linkrev => 'base') if commit['rev'] != '1' %>
+    <ul class="horizontal-list" style="margin: 5px 0px 0px 20px;">
+    <% if commit['rev'] != '1' %>
+      <li>
+        <%= link_to(sprite_tag('brick'), {:controller => :package, :action => :rdiff,
+                                          :package => @package, :project => @project,
+                                          :rev => commit['rev'], :linkrev => 'base'}) %>
+        <%= link_to('Files changed', :controller => :package, :action => :rdiff,
+                    :package => @package, :project => @project, :rev => commit['rev'], :linkrev => 'base') %>
+      </li>
+    <% end -%>
+      <li>
+        <%= link_to(image_tag('folder_go.png'), :action => 'show', :project => @project, :package => @package, :rev => commit['rev']) %>
+        <%= link_to('Browse Source', :action => 'show', :project => @project, :package => @package, :rev => commit['rev']) %>
+      </li>
+    </ul>
 <% else %>
     <i>Revision <%= rev.to_s %> not found</i>
 <% end %>

src/api/app/views/webui/package/revisions.html.erb

@@ -3,56 +3,19 @@
 
 <%= render :partial => 'tabs' %>
 
-<% if @max_revision %>
-  <h3><%= @pagetitle %></h3>
-  <p><%= button_to_function('Go up', 'go_up()', :id => 'go_up_button', :class => 'hidden') %></p>
+<% if @revisions %>
+  <h3><%= "#{@pagetitle} (#{@lastrev})" %></h3>
   <div id="revisions_view" style="margin-left: 1.2em">
-    <% @upper_bound.downto(@lower_bound) do |rev| %>
-      <div id="commit_item_<%= rev %>">
+    <% @revisions.each do |rev| %>
+      <div id="commit_item_<%= rev %>" class="commit_item">
         <%= render(:partial => 'commit_item', :locals => {:rev => rev }) %>
-        <%= link_to(image_tag('folder_go.png'), :action => 'show', :project => @project, :package => @package, :rev => rev) %>
-        <%= link_to('Show source', :action => 'show', :project => @project, :package => @package, :rev => rev) %>
       </div>
     <% end %>
   </div>
-  <p><%= button_to_function('Go down', 'go_down()', :id => 'go_down_button', :class => 'hidden') %></p>
-  <% if @lower_bound > 1 || @max_revision > @upper_bound %>
+  <% unless params[:showall] %>
     <p><%= link_to('Show all', :action  => 'revisions', :project => @project, :package => @package, :showall => 1) %></p>
   <% end %>
-  <%= javascript_tag do %>
-    <% if @max_revision > @upper_bound %>
-      $('#go_up_button').show();
-    <% end %>
-    <% if @lower_bound > 1 %>
-      $('#go_down_button').show(); 
-    <% end %>
-    var bounds_offset = 0;
-
-    function go_up() {
-      bounds_offset = bounds_offset + 1;
-      var new_upper_bound = <%= @upper_bound %> + bounds_offset;
-      $('#go_down_button').show();
-      if (<%= @max_revision %> == new_upper_bound) { $('#go_up_button').hide(); }
-      $('#revisions_view div:last').remove();
-      $('#revisions_view div:first').before('<div id=\"commit_item_'+new_upper_bound+'\">...</div>');
-
-      var path = "/package/commit?revision="+new_upper_bound+"&package=<%= @package %>&project=<%= @project %>";
-      $("#revisions_view div#commit_item_"+new_upper_bound).load(path);
-    }
-
-    function go_down() {
-      bounds_offset = bounds_offset - 1;
-      var new_lower_bound = <%= @lower_bound %> + bounds_offset;
-
-      $('#go_up_button').show();
-      if (new_lower_bound == 1) { $('#go_down_button').hide(); }
-      $('#revisions_view div:first').remove();
-      $('#revisions_view div:last').after('<div id=\"commit_item_'+new_lower_bound+'\">...</div>');
-
-      var path = "/package/commit?revision="+new_lower_bound+"&package=<%= @package %>&project=<%= @project %>";
-      $("#revisions_view div#commit_item_"+new_lower_bound).load(path);
-    }
-  <% end %>
 <% else %>
-  <i>No commits exists yet.</i>
+  <h3><%= @pagetitle %></h3>
+  <p><i>No commits exists yet.</i></p>
 <% end %>

src/api/config/routes.rb

@@ -90,7 +90,7 @@ def self.matches?(request)
       get 'package/requests/:project/:package' => :requests, as: 'package_requests', constraints: cons
       get 'package/statistics/:project/:package' => :statistics, as: 'package_statistics', constraints: cons
       get 'package/commit/:project/:package' => :commit, as: 'package_commit', constraints: cons
-      get 'package/revisions/:project/:package' => :revisions, constraints: cons
+      get 'package/revisions/:project/:package' => :revisions, constraints: cons, as: 'package_view_revisions'
       get 'package/submit_request_dialog/:project/:package' => :submit_request_dialog, constraints: cons
       post 'package/submit_request/:project/:package' => :submit_request, constraints: cons
       get 'package/add_person/:project/:package' => :add_person, constraints: cons

src/api/test/functional/webui/package_controller_test.rb

@@ -316,4 +316,23 @@ def fill_comment(body = 'Comment Body')
     # restore now
     Suse::Backend.put( '/source/home:dmayr/x11vnc/README?user=king', 'just to delete')
   end
+
+  test "revisions" do
+    visit package_view_revisions_path(project: 'BaseDistro2.0', package: 'pack2')
+    click_link "Revisions"
+    page.must_have_text "Revision Log of pack2 (3)"
+
+    visit package_view_revisions_path(project: 'BaseDistro2.0', package: 'pack2', rev: '2')
+    page.must_have_text "Revision Log of pack2 (2)"
+    click_link "Show all"
+    page.must_have_text "Revision Log of pack2 (3)"
+
+    login_king
+    20.times { |i| put '/source/BaseDistro2.0/pack2/dummy', i.to_s }
+    visit package_view_revisions_path(project: 'BaseDistro2.0', package: 'pack2')
+    page.must_have_text "Revision Log of pack2 (23)"
+    all(:css, 'div.commit_item').count.must_equal 20
+    click_link "Show all"
+    all(:css, 'div.commit_item').count.must_equal 23
+  end
 end