[PATCH] Fixed sort column

Tomas Sedovic tsedovic at redhat.com
Fri Dec 16 12:54:22 UTC 2011


On 12/16/2011 10:50 AM, jprovazn at redhat.com wrote:
> From: Jan Provaznik<jprovazn at redhat.com>
>
> https://bugzilla.redhat.com/show_bug.cgi?id=761504
> sort_column method now includes table name, this was problem for
> non-admin users - selects were joined with columns from privilege table
> which contains 'name' column too so there was conflict with default sort
> column 'name'.
>
> Also removed pagination from listing pool families - pagination is obviusly
> not used in a view and privilege target_type changed to 'Pool' when listing
> pool families for a user.
> ---
>   src/app/controllers/application_controller.rb      |    5 ++-
>   .../controllers/hardware_profiles_controller.rb    |    2 +-
>   src/app/controllers/pool_families_controller.rb    |    7 +---
>   src/features/pool_family.feature                   |   39 +++++++++++++++-----
>   src/features/step_definitions/pool_family_steps.rb |    5 +++
>   src/spec/factories/permission.rb                   |    6 +++
>   src/spec/factories/user.rb                         |    3 ++
>   7 files changed, 49 insertions(+), 18 deletions(-)
>
> diff --git a/src/app/controllers/application_controller.rb b/src/app/controllers/application_controller.rb
> index 053a87b..7e8394c 100644
> --- a/src/app/controllers/application_controller.rb
> +++ b/src/app/controllers/application_controller.rb
> @@ -272,8 +272,9 @@ class ApplicationController<  ActionController::Base
>       end
>     end
>
> -  def sort_column(model, default="name")
> -    model.column_names.include?(params[:order_field]) ? params[:order_field] : default
> +  def sort_column(model, default = nil)
> +    return params[:order_field] if model.column_names.include?(params[:order_field])
> +    return default || "#{model.quoted_table_name}.name"
>     end
>
>     def sort_direction
> diff --git a/src/app/controllers/hardware_profiles_controller.rb b/src/app/controllers/hardware_profiles_controller.rb
> index 8773d6a..2d408bf 100644
> --- a/src/app/controllers/hardware_profiles_controller.rb
> +++ b/src/app/controllers/hardware_profiles_controller.rb
> @@ -205,7 +205,7 @@ class HardwareProfilesController<  ApplicationController
>
>     def load_hardware_profiles
>       sort_order = sort_direction
> -    sort_field = sort_column(HardwareProfile)
> +    sort_field = sort_column(HardwareProfile, 'name')
>       if sort_field == "name"
>         @hardware_profiles = HardwareProfile.list_for_user(current_user, Privilege::VIEW).where('provider_id IS NULL', {}).apply_filters(:preset_filter_id =>  params[:hardware_profiles_preset_filter], :search_filter =>  params[:hardware_profiles_search]).order("hardware_profiles.name #{sort_direction}")
>       else
> diff --git a/src/app/controllers/pool_families_controller.rb b/src/app/controllers/pool_families_controller.rb
> index f400a1d..93dce2e 100644
> --- a/src/app/controllers/pool_families_controller.rb
> +++ b/src/app/controllers/pool_families_controller.rb
> @@ -19,14 +19,13 @@ require 'will_paginate/array'
>   class PoolFamiliesController<  ApplicationController
>     before_filter :require_user
>     before_filter :set_params_and_header, :only =>  [:index, :show]
> -  before_filter :load_pool_families, :only =>[:show]
> +  before_filter :load_pool_families, :only =>[:index, :show]
>     before_filter :load_tab_captions_and_details_tab, :only =>  [:show]
>
>     def index
>       clear_breadcrumbs
>       save_breadcrumb(pool_families_path)
>       set_admin_environments_tabs 'pool_families'
> -    load_pool_families
>       respond_to do |format|
>         format.html
>         format.js { render :partial =>  'list' }
> @@ -213,9 +212,7 @@ class PoolFamiliesController<  ApplicationController
>     end
>
>     def load_pool_families
> -    @pool_families = PoolFamily.list_for_user(current_user, Privilege::VIEW).paginate(
> -      :page =>  params[:page] || 1,
> -      :order =>  (sort_column(PoolFamily) + ' ' + sort_direction))
> +    @pool_families = PoolFamily.list_for_user(current_user, Privilege::VIEW, Pool).order(sort_column(PoolFamily) + ' ' + sort_direction)
>     end
>
>     def load_pool_family_tabs
> diff --git a/src/features/pool_family.feature b/src/features/pool_family.feature
> index 5e96ca3..3d3d1d0 100644
> --- a/src/features/pool_family.feature
> +++ b/src/features/pool_family.feature
> @@ -4,12 +4,12 @@ Feature: Pool Families
>     I want to manage pool families
>
>     Background:
> -    Given I am an authorised user
> -    And I am logged in
> -    And I'm connected to the warehouse
> +    Given I'm connected to the warehouse
>
>     Scenario: List pool families
> -    Given I am on the homepage
> +    Given I am an authorised user
> +    And I am logged in
> +    And I am on the homepage
>       And there are these pool families:
>       | name      |
>       | pool_family1 |
> @@ -21,14 +21,25 @@ Feature: Pool Families
>       | pool_family2 |
>       | pool_family3 |
>
> +  Scenario: List pool families as unprivileged user
> +    Given I am logged in
> +    And I can view pool family "testpoolfamily"
> +    When I go to the pool families page
> +    Then I should see "Pool Families"
> +    And I should see "testpoolfamily"
> +
>     Scenario: Show pool family details
> -    Given there is a pool family named "testpoolfamily"
> +    Given I am an authorised user
> +    And I am logged in
> +    And there is a pool family named "testpoolfamily"
>       And I am on the pool families page
>       When I follow "testpoolfamily"
>       Then I should see "Pools"
>
>     Scenario: Create a new Pool family
> -    Given I am on the pool families page
> +    Given I am an authorised user
> +    And I am logged in
> +    And I am on the pool families page
>       And there is not a pool family named "testpoolfamily"
>       When I follow "new_pool_family_button"
>       Then I should be on the new pool family page
> @@ -40,7 +51,9 @@ Feature: Pool Families
>       And I should have a pool family named "testpoolfamily"
>
>     Scenario: Delete a pool family
> -    Given I am on the homepage
> +    Given I am an authorised user
> +    And I am logged in
> +    And I am on the homepage
>       And there is a pool family named "poolfamily1"
>       When I go to the pool families page
>       And I follow "poolfamily1"
> @@ -48,7 +61,9 @@ Feature: Pool Families
>       Then there should not exist a pool family named "poolfamily1"
>
>     Scenario: Disallow deletion of default pool family
> -    Given I am on the pool families page
> +    Given I am an authorised user
> +    And I am logged in
> +    And I am on the pool families page
>       Then I should see "default"
>       When I follow "default"
>       And I press "delete_pool_family_button"
> @@ -82,7 +97,9 @@ Feature: Pool Families
>   #    And I should see "third_family"
>
>     Scenario: Add provider account to pool family
> -    Given there is a pool family named "testpoolfamily"
> +    Given I am an authorised user
> +    And I am logged in
> +    And there is a pool family named "testpoolfamily"
>       And there is a provider named "testprovider"
>       And there is a provider account named "testaccount"
>       And I am on the pool family provider accounts page
> @@ -94,7 +111,9 @@ Feature: Pool Families
>       And I should see "testaccount"
>
>     Scenario: Remove provider account from pool family
> -    Given there is a pool family named "testpoolfamily"
> +    Given I am an authorised user
> +    And I am logged in
> +    And there is a pool family named "testpoolfamily"
>       And there is a provider named "testprovider"
>       And there is a provider account named "testaccount"
>       And there is a provider account "testaccount" related to pool family "testpoolfamily"
> diff --git a/src/features/step_definitions/pool_family_steps.rb b/src/features/step_definitions/pool_family_steps.rb
> index 70bb207..b2239dd 100644
> --- a/src/features/step_definitions/pool_family_steps.rb
> +++ b/src/features/step_definitions/pool_family_steps.rb
> @@ -66,3 +66,8 @@ Then /^there should not exist a provider account assigned to "([^"]*)"$/ do |nam
>     @pool_family = PoolFamily.find_by_name(name)
>     @pool_family.provider_accounts.count == 0
>   end
> +
> +Given /^I can view pool family "([^"]*)"$/ do |arg1|
> +  pool_family = PoolFamily.find_by_name(arg1)  || FactoryGirl.create(:pool_family, :name =>  arg1)
> +  perm = FactoryGirl.create(:pool_family_user_permission, :permission_object =>  pool_family, :user =>  @user)
> +end
> diff --git a/src/spec/factories/permission.rb b/src/spec/factories/permission.rb
> index d69930a..1a1f025 100644
> --- a/src/spec/factories/permission.rb
> +++ b/src/spec/factories/permission.rb
> @@ -50,4 +50,10 @@ FactoryGirl.define do
>       user { |r| r.association(:pool_user2) }
>     end
>
> +  factory :pool_family_user_permission, :parent =>  :permission do
> +    role { |r| Role.first(:conditions =>  ['name = ?', 'Pool Family User']) || FactoryGirl.create(:role, :name =>  'Pool Family User') }
> +    permission_object { |r| r.association(:pool_family) }
> +    user { |r| r.association(:pool_family_user) }
> +  end
> +
>   end
> diff --git a/src/spec/factories/user.rb b/src/spec/factories/user.rb
> index b5e1369..5ca512d 100644
> --- a/src/spec/factories/user.rb
> +++ b/src/spec/factories/user.rb
> @@ -58,4 +58,7 @@ FactoryGirl.define do
>       sequence(:login) { |n| "pool_user2#{n}" }
>     end
>
> +  factory :pool_family_user, :parent =>  :user do
> +    sequence(:login) { |n| "pool_family_user#{n}" }
> +  end
>   end

ACK

Thomas
-- 
No trees were killed to send this message, but a large number of 
electrons were terribly inconvenienced.



More information about the aeolus-devel mailing list