[PATCH] Fixed sort column

jprovazn at redhat.com jprovazn at redhat.com
Fri Dec 16 09:50:47 UTC 2011


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
-- 
1.7.6.4




More information about the aeolus-devel mailing list