[PATCH conductor 2/3] BZ802772: Add pagination to pools#index and pools#show

Jirka Tomasek jtomasek at redhat.com
Wed Mar 21 09:34:49 UTC 2012


On 03/20/2012 05:48 PM, ifarkas at redhat.com wrote:
> From: Imre Farkas<ifarkas at redhat.com>
>
> https://bugzilla.redhat.com/show_bug.cgi?id=802772
> ---
>   src/app/controllers/deployments_controller.rb    |    7 +++++-
>   src/app/controllers/pools_controller.rb          |   23 ++++++++++++++++-----
>   src/app/models/instance.rb                       |    3 --
>   src/app/models/pool.rb                           |    4 ++-
>   src/app/stylesheets/custom.scss                  |    5 ++++
>   src/app/views/deployments/_list.html.haml        |    5 ++-
>   src/app/views/deployments/_pretty_view.html.haml |    8 +++---
>   src/app/views/layouts/_filter_table.html.haml    |    3 ++
>   src/app/views/pools/_header_index.html.haml      |    2 +-
>   src/app/views/pools/_pretty_list.html.haml       |   10 ++++++--
>   src/app/views/pools/show.html.haml               |    3 +-
>   src/config/initializers/will_paginate.rb         |    2 +
>   src/config/locales/en.yml                        |    3 +-
>   src/public/javascripts/backbone/models.js        |    7 ++++-
>   src/public/javascripts/backbone/routers.js       |    1 +
>   src/public/javascripts/backbone/views.js         |   18 ++++++++++++++++-
>   16 files changed, 78 insertions(+), 26 deletions(-)
>   create mode 100644 src/config/initializers/will_paginate.rb
>
> diff --git a/src/app/controllers/deployments_controller.rb b/src/app/controllers/deployments_controller.rb
> index d65dea3..3842872 100644
> --- a/src/app/controllers/deployments_controller.rb
> +++ b/src/app/controllers/deployments_controller.rb
> @@ -379,7 +379,12 @@ class DeploymentsController<  ApplicationController
>         { :name =>  t("providers.provider"), :sortable =>  false }
>       ]
>       @pools = Pool.list_for_user(current_user, Privilege::CREATE, Deployment)
> -    @deployments = Deployment.includes(:owner).apply_filters(:preset_filter_id =>  params[:deployments_preset_filter], :search_filter =>  params[:deployments_search]).list_for_user(current_user, Privilege::VIEW).where('deployments.pool_id' =>  @pools).order(sort_column(Deployment, "deployments.name") +' '+ sort_direction).paginate(:page =>  params[:page] || 1)
> +    @deployments = Deployment.includes(:owner).
> +                              apply_filters(:preset_filter_id =>  params[:deployments_preset_filter], :search_filter =>  params[:deployments_search]).
> +                              list_for_user(current_user, Privilege::VIEW).
> +                              where('deployments.pool_id' =>  @pools).
> +                              order(sort_column(Deployment, "deployments.name") +' '+ sort_direction).
> +                              paginate(:page =>  params[:page], :per_page =>  PER_PAGE)
>     end
>
>     def count_additional_quota(deployment)
> diff --git a/src/app/controllers/pools_controller.rb b/src/app/controllers/pools_controller.rb
> index 11b86aa..cfefe34 100644
> --- a/src/app/controllers/pools_controller.rb
> +++ b/src/app/controllers/pools_controller.rb
> @@ -47,15 +47,22 @@ class PoolsController<  ApplicationController
>       if filter_view?
>         case @details_tab[:id]
>         when 'pools'
> -        @pools = Pool.apply_filters(:preset_filter_id =>  params[:pools_preset_filter], :search_filter =>  params[:pools_search]).list_for_user(current_user, Privilege::VIEW).list(sort_column(Pool), sort_direction)
> +        @pools = Pool.apply_filters(:preset_filter_id =>  params[:pools_preset_filter], :search_filter =>  params[:pools_search]).
> +                      list_for_user(current_user, Privilege::VIEW).list(sort_column(Pool), sort_direction).
> +                      paginate(:page =>  params[:page], :per_page =>  PER_PAGE)
>         when 'instances'
>           params[:instances_preset_filter] = "" unless params[:instances_preset_filter]
> -        @instances = Instance.apply_filters(:preset_filter_id =>  params[:instances_preset_filter], :search_filter =>  params[:instances_search]).list_for_user(current_user, Privilege::VIEW).list(sort_column(Instance), sort_direction)
> +        @instances = Instance.apply_filters(:preset_filter_id =>  params[:instances_preset_filter], :search_filter =>  params[:instances_search]).
> +                              list_for_user(current_user, Privilege::VIEW).list(sort_column(Instance), sort_direction).
> +                              paginate(:page =>  params[:page], :per_page =>  PER_PAGE)
>         when 'deployments'
> -        @deployments = Deployment.apply_filters(:preset_filter_id =>  params[:deployments_preset_filter], :search_filter =>  params[:deployments_search]).list_for_user(current_user, Privilege::VIEW).list(sort_column(Deployment), sort_direction)
> +        @deployments = Deployment.apply_filters(:preset_filter_id =>  params[:deployments_preset_filter], :search_filter =>  params[:deployments_search]).
> +                                  list_for_user(current_user, Privilege::VIEW).list(sort_column(Deployment), sort_direction).
> +                                  paginate(:page =>  params[:page], :per_page =>  PER_PAGE)
>         end
>       else
> -      @pools = Pool.list_for_user(current_user, Privilege::VIEW).list(sort_column(Pool), sort_direction)
> +      @pools = Pool.list_for_user(current_user, Privilege::VIEW).list(sort_column(Pool), sort_direction).
> +                    paginate(:page =>  params[:page], :per_page =>  PER_PAGE)
>       end
>
>       statistics
> @@ -102,12 +109,16 @@ class PoolsController<  ApplicationController
>
>       details_tab_name = params[:details_tab].blank? ? 'deployments' : params[:details_tab]
>       @details_tab = @tabs.find {|t| t[:id] == details_tab_name} || @tabs.first[:name].downcase
> -    @deployments = @pool.deployments.apply_filters(:preset_filter_id =>  params[:deployments_preset_filter], :search_filter =>  params[:deployments_search]).list_for_user(current_user, Privilege::VIEW) if @details_tab[:id] == 'deployments'
> +    @deployments = @pool.deployments.
> +                         apply_filters(:preset_filter_id =>  params[:deployments_preset_filter], :search_filter =>  params[:deployments_search]).
> +                         list_for_user(current_user, Privilege::VIEW).
> +                         paginate(:page =>  params[:page], :per_page =>  PER_PAGE) if @details_tab[:id] == 'deployments'
>       @view = @details_tab[:view]
>       respond_to do |format|
>         format.html { render :action =>  :show}
>         format.js { render :partial =>  @view }
> -      format.json { render :json =>  @pool.as_json(:with_deployments =>  true, :current_user =>  current_user) }
> +      format.json { render :json =>  @pool.as_json(:with_deployments =>  true, :current_user =>  current_user,
> +                                                  :page =>  params[:page], :per_page =>  PER_PAGE) }
>       end
>     end
>
> diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
> index f074150..09938be 100644
> --- a/src/app/models/instance.rb
> +++ b/src/app/models/instance.rb
> @@ -66,9 +66,6 @@ class Instance<  ActiveRecord::Base
>     end
>     include PermissionedObject
>
> -  cattr_reader :per_page
> -  @@per_page = 15
> -
>     belongs_to :pool
>     belongs_to :pool_family
>     belongs_to :provider_account
> diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
> index 3fce49b..ec89e07 100644
> --- a/src/app/models/pool.rb
> +++ b/src/app/models/pool.rb
> @@ -123,7 +123,9 @@ class Pool<  ActiveRecord::Base
>       })
>
>       if options[:with_deployments]
> -      result[:deployments] = deployments.list_for_user(options[:current_user], Privilege::VIEW).map {|d| d.as_json}
> +      result[:deployments] = deployments.list_for_user(options[:current_user], Privilege::VIEW).
> +                                         paginate(:page =>  options[:page], :per_page =>  options[:per_page]).
> +                                         map {|d| d.as_json}
>       end
>
>       result
> diff --git a/src/app/stylesheets/custom.scss b/src/app/stylesheets/custom.scss
> index 484d3c3..84b4c40 100644
> --- a/src/app/stylesheets/custom.scss
> +++ b/src/app/stylesheets/custom.scss
> @@ -271,3 +271,8 @@ form.generic section.services {
>       }
>     }
>   }
> +
> +.pagination{
> +  margin-top: 10px;
> +  text-align: center;
> +}
> diff --git a/src/app/views/deployments/_list.html.haml b/src/app/views/deployments/_list.html.haml
> index bd47ec9..b123219 100644
> --- a/src/app/views/deployments/_list.html.haml
> +++ b/src/app/views/deployments/_list.html.haml
> @@ -40,10 +40,11 @@
>       %td= link_to deployment.pool.name, pool_path(deployment.pool, :view =>  :filter)
>       %td= owner_name(deployment)
>       %td= link_to(deployment.provider.provider_type.name, deployment.provider) if deployment.provider
> +
>   :javascript
>     Conductor.setupPrettyFilterURL(
> -    '#{url_for(:action =>  :show, :details_tab =>  'deployments', :view =>  'filter' )}',
> -    '#{url_for(:action =>  :show, :details_tab =>  'deployments', :view =>  'pretty' )}'
> +    '#{url_for(:action =>  :show, :details_tab =>  'deployments', :view =>  'filter', :page =>  params[:page])}',
> +    '#{url_for(:action =>  :show, :details_tab =>  'deployments', :view =>  'pretty', :page =>  params[:page])}'
>     );
>
>   %script#deploymentTemplate{ :type =>  'text/x-jquery-tmpl' }
> diff --git a/src/app/views/deployments/_pretty_view.html.haml b/src/app/views/deployments/_pretty_view.html.haml
> index c121973..24940a2 100644
> --- a/src/app/views/deployments/_pretty_view.html.haml
> +++ b/src/app/views/deployments/_pretty_view.html.haml
> @@ -21,12 +21,12 @@
>                   %dd
>                     %p.provider= deployment.provider.provider_type.name unless deployment.provider.blank?
>
> +%div= will_paginate(@deployments)
> +
>   :javascript
>     Conductor.setupPrettyFilterURL(
> -            '#{url_for(:action =>  :show, :details_tab =>  'deployments', :view =>  'filter' )}',
> -            '#{url_for(:action =>  :show, :details_tab =>  'deployments', :view =>  'pretty' )}');
> -
> -
> +            '#{url_for(:action =>  :show, :details_tab =>  'deployments', :view =>  'filter', :page =>  params[:page])}',
> +            '#{url_for(:action =>  :show, :details_tab =>  'deployments', :view =>  'pretty', :page =>  params[:page] )}');
>
>   %script#deploymentTemplate{ :type =>  'text/x-jquery-tmpl' }
>     :plain
> diff --git a/src/app/views/layouts/_filter_table.html.haml b/src/app/views/layouts/_filter_table.html.haml
> index 9240fc5..65798e6 100644
> --- a/src/app/views/layouts/_filter_table.html.haml
> +++ b/src/app/views/layouts/_filter_table.html.haml
> @@ -26,3 +26,6 @@
>     - if content_for? form_footer
>       %ul.obj_actions
>         = yield form_footer
> +
> +- if rows.is_a?(WillPaginate::Collection)
> +  %div= will_paginate(rows)

will_paginate already creates div.pagination itself so it's not 
necessary to put it into another div, so

= will_paginate(rows)

is enough. Please fix this on all places in the patches where helper is 
called.

> diff --git a/src/app/views/pools/_header_index.html.haml b/src/app/views/pools/_header_index.html.haml
> index cb7f0de..6a1768f 100644
> --- a/src/app/views/pools/_header_index.html.haml
> +++ b/src/app/views/pools/_header_index.html.haml
> @@ -2,7 +2,7 @@
>     %h1.section-index= t 'pools.index.overview'
>     #obj_actions.button-container
>       .button-group
> -      = pretty_filter_toggle(pools_path(:view =>  'pretty'), pools_path(:view =>  'filter'))
> +      = pretty_filter_toggle(pools_path(:view =>  'pretty', :page =>  params[:page]), pools_path(:view =>  'filter', :page =>  params[:page]))
>       .button-group
>         - if check_privilege(Privilege::CREATE, Pool)
>           = link_to t('pools.new_pool'), new_pool_path, { :class =>  'button primary', :id =>  'new_pool_button' }
> diff --git a/src/app/views/pools/_pretty_list.html.haml b/src/app/views/pools/_pretty_list.html.haml
> index 02ef6f2..075de9f 100644
> --- a/src/app/views/pools/_pretty_list.html.haml
> +++ b/src/app/views/pools/_pretty_list.html.haml
> @@ -1,6 +1,6 @@
>   %header
>     %h2=t 'pools.index.your_pools'
> -  %span.label.badge.dark.count= @pools.length
> +  %span.label.badge.dark.count= @pools.total_entries
>
>   .content#pools-list
>     - @pools.each do |pool|
> @@ -25,16 +25,20 @@
>           %a.control{:href =>  '#'}
>             %span Expand/Collapse
>         %div.content
> -        = render :partial =>  'deployments', :locals =>  {:pool =>  pool, :deployments =>  pool.deployments.list_for_user(current_user, Privilege::VIEW).ascending_by_name}
> +        - deployments = pool.deployments.list_for_user(current_user, Privilege::VIEW).ascending_by_name.paginate(:page =>  params[:page], :per_page =>  PER_PAGE)
> +        = render :partial =>  'deployments', :locals =>  {:pool =>  pool, :deployments =>  deployments}
>         %ul.content.actions
>           %li
> -          = link_to ("#{html_escape(pool.name)} " + "#{t'pools.index.pool_details'}" + "&raquo;").html_safe, pool_path(pool), :class =>'pool_details'
> +          - pool_details_label = deployments.total_pages>  1 ? t('pools.index.not_all_deployments_displayed_alert_html', :name =>  pool.name) : t('pools.index.not_all_deployments_displayed_alert_html', :name =>  pool.name)
> +          = link_to(pool_details_label, pool_path(pool), :class =>'pool_details')
>           %li
>             - if check_privilege(Privilege::CREATE, Deployment, pool)
>               = link_to t('deployments.new_deployment'), launch_new_deployments_path(:pool_id =>  pool.id), :class =>  'button primary', :id =>  'new_deployment_button'
>           %li.catalog_link
>             = render :partial =>  'layouts/catalog_dropdown', :locals =>  {:catalogs =>  pool.catalogs}
>
> +  %div= will_paginate(@pools)
> +
>   :javascript
>     $(document).ready(function() {
>       $('div.pool').each(function(){
> diff --git a/src/app/views/pools/show.html.haml b/src/app/views/pools/show.html.haml
> index ad60ca4..b593c7d 100644
> --- a/src/app/views/pools/show.html.haml
> +++ b/src/app/views/pools/show.html.haml
> @@ -16,6 +16,7 @@
>       .section-controls
>         #obj_actions.button-container
>           .button-group
> -          = pretty_filter_toggle(pool_path(@pool, :view =>  'pretty', :details_tab =>  @details_tab[:id]), pool_path(@pool, :view =>  'filter', :details_tab =>  @details_tab[:id]))
> +          = pretty_filter_toggle(pool_path(@pool, :view =>  'pretty', :details_tab =>  @details_tab[:id], :page =>  params[:page]),
> +                                 pool_path(@pool, :view =>  'filter', :details_tab =>  @details_tab[:id], :page =>  params[:page]))
>     .content.collapsible.toggle-view.pools#tab
>       = render :partial =>  @view
> diff --git a/src/config/initializers/will_paginate.rb b/src/config/initializers/will_paginate.rb
> new file mode 100644
> index 0000000..25962eb
> --- /dev/null
> +++ b/src/config/initializers/will_paginate.rb
> @@ -0,0 +1,2 @@
> +## Set per_page globally
> +PER_PAGE = 15
> diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> index aeeb52d..4050e0b 100644
> --- a/src/config/locales/en.yml
> +++ b/src/config/locales/en.yml
> @@ -432,7 +432,8 @@ en:
>         confirm_delete: "Are you sure you want to delete this pool? All related catalogs and deployables will be deleted too!"
>         overview: Overview
>         your_pools: Your Pools
> -      pool_details: Pool details
> +      pool_details_html: "%{name} Pool details&raquo;"
> +      not_all_deployments_displayed_alert_html: "Not all deployments are displayed! Go to %{name} Pool details for more!&raquo;"
>         global_uptime: Global uptime
>         destroy: Destroy
>       images:
> diff --git a/src/public/javascripts/backbone/models.js b/src/public/javascripts/backbone/models.js
> index 1d1c54b..967b106 100644
> --- a/src/public/javascripts/backbone/models.js
> +++ b/src/public/javascripts/backbone/models.js
> @@ -2,12 +2,15 @@ Conductor.Models = Conductor.Models || {}
>
>
>   Conductor.Models.Pool = Backbone.Model.extend({
> -  urlRoot: Conductor.prefixedPath('/pools'),
> -
>     initialize: function() {
>       this.deployments = new Conductor.Models.Deployments().filter(function(attributes) {
>         return attributes.pool.id == this.id;
>       });
> +  },
> +  queryParams: {},
> +  url: function() {
> +    var path = Conductor.prefixedPath('/pools/'  + this.id);
> +    return Conductor.parameterizedPath(path, this.queryParams);
>     }
>   });
>
> diff --git a/src/public/javascripts/backbone/routers.js b/src/public/javascripts/backbone/routers.js
> index 94bed1f..e16941b 100644
> --- a/src/public/javascripts/backbone/routers.js
> +++ b/src/public/javascripts/backbone/routers.js
> @@ -36,6 +36,7 @@ Conductor.Routers.Pools = Backbone.Router.extend({
>       setInterval(function() {
>         var pool = new Conductor.Models.Pool({ id: id });
>         var view = new Conductor.Views.PoolsShow({ model: pool });
> +      pool.queryParams = view.queryParams();
>
>         if(view.currentTab() !== 'deployments') return;
>
> diff --git a/src/public/javascripts/backbone/views.js b/src/public/javascripts/backbone/views.js
> index 5065e1b..9061481 100644
> --- a/src/public/javascripts/backbone/views.js
> +++ b/src/public/javascripts/backbone/views.js
> @@ -42,7 +42,7 @@ Conductor.Views.PoolsIndex = Backbone.View.extend({
>     },
>
>     queryParams: function() {
> -    var paramsToInclude = [this.currentTab() + '_preset_filter', this.currentTab() + '_search'];
> +    var paramsToInclude = [this.currentTab() + '_preset_filter', this.currentTab() + '_search', 'page'];
>       var result = Conductor.extractQueryParams(paramsToInclude);
>
>       result['details_tab'] = this.currentTab();
> @@ -113,6 +113,22 @@ Conductor.Views.PoolsShow = Backbone.View.extend({
>       }
>     },
>
> +  queryParams: function() {
> +    var paramsToInclude = ['deployments_preset_filter', 'deployments_search', 'page'];
> +    var result = Conductor.extractQueryParams(paramsToInclude);
> +
> +    // If there is no URL param for the preset filter, we still need to merge in the preset filter
> +    var filter = 'deployments_preset_filter';
> +    if(result[filter] == undefined) {
> +      var filter_selector = '#deployments_preset_filter:enabled';
> +      if($(filter_selector).val() != undefined) {
> +        result[filter] = $(filter_selector).val();
> +      }
> +    };
> +
> +    return result;
> +  },
> +
>     render: function() {
>       this.$('h1.pools').text(this.model.get('name') + ' Pool');
>

Also it would be good to introduce i18n file for will_paginate so the 
prev and next labels can be translated. I'd recomment to use this file 
https://github.com/tigrish/will-paginate-i18n/blob/master/locales/en-US.yml 
and put it into config/locales/will_paginate/en.yml

ACK to patchset

Jirka
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/aeolus-devel/attachments/20120321/07c70e5f/attachment.html>


More information about the aeolus-devel mailing list