On 04/20/2011 11:20 AM, jtomasek(a)redhat.com wrote:
From: Jiri Tomasek<jtomasek(a)redhat.com>
---
.../controllers/admin/pool_families_controller.rb | 29 ++++++++++++++++++-
src/app/views/admin/pool_families/_list.haml | 2 +-
.../admin/pool_families/_provider_accounts.haml | 25 +++++++++++++++++
src/config/routes.rb | 2 +-
src/features/pool_family.feature | 22 +++++++++++++++
src/features/step_definitions/pool_family_steps.rb | 23 +++++++++++++++-
src/features/support/paths.rb | 3 ++
7 files changed, 101 insertions(+), 5 deletions(-)
diff --git a/src/app/controllers/admin/pool_families_controller.rb
b/src/app/controllers/admin/pool_families_controller.rb
index af53aac..ec21c53 100644
--- a/src/app/controllers/admin/pool_families_controller.rb
+++ b/src/app/controllers/admin/pool_families_controller.rb
@@ -2,6 +2,7 @@ class Admin::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_tab_captions_and_details_tab, :only => [:show]
def index
@search_term = params[:q]
@@ -50,8 +51,7 @@ class Admin::PoolFamiliesController< ApplicationController
def show
@pool_family = PoolFamily.find(params[:id])
@url_params = params.clone
- @tab_captions = ['Properties', 'History', 'Permissions',
'Provider Accounts', 'Pools']
- @details_tab = params[:details_tab].blank? ? 'properties' :
params[:details_tab]
+
respond_to do |format|
format.js do
if @url_params.delete :details_pane
@@ -63,6 +63,15 @@ class Admin::PoolFamiliesController< ApplicationController
end
end
+ def add_provider_account
+ @pool_family = PoolFamily.find(params[:id])
+ @provider_account = ProviderAccount.find(params[:provider_account_id])
+
+ @pool_family.provider_accounts<< @provider_account
+ flash[:notice] = "Provider Account has been added"
+ redirect_to admin_pool_family_path(@pool_family, :details_tab =>
'provider_accounts')
+ end
+
def multi_destroy
deleted = []
not_deleted = []
@@ -82,8 +91,24 @@ class Admin::PoolFamiliesController< ApplicationController
redirect_to admin_pool_families_path
end
+ def multi_destroy_provider_accounts
+ @pool_family = PoolFamily.find(params[:pool_family_id])
+
+ ProviderAccount.find(params[:provider_account_selected]).each do |provider_account|
+ @pool_family.provider_accounts.delete provider_account
+ end
+
+ redirect_to admin_pool_family_path(@pool_family, :details_tab =>
'provider_accounts')
+ end
+
protected
+ def load_tab_captions_and_details_tab
+ @tab_captions = ['Properties', 'History', 'Permissions',
'Provider Accounts', 'Pools']
+ @details_tab = params[:details_tab].blank? ? 'properties' :
params[:details_tab]
+ @provider_accounts_header = [{ :name => "Provider Account", :sort_attr
=> :name}]
+ end
+
def set_params_and_header
@url_params = params.clone
@header = [{ :name => "Name", :sort_attr => :name},
diff --git a/src/app/views/admin/pool_families/_list.haml
b/src/app/views/admin/pool_families/_list.haml
index a824676..6250c66 100644
--- a/src/app/views/admin/pool_families/_list.haml
+++ b/src/app/views/admin/pool_families/_list.haml
@@ -8,7 +8,7 @@
%span> ,
= link_to "None", @url_params.merge(:select => 'none')
- %table#pool_famlies_table
+ %table#pool_families_table
= sortable_table_header @header
- unless @pool_families.blank?
- @pool_families.each do |pool_family|
diff --git a/src/app/views/admin/pool_families/_provider_accounts.haml
b/src/app/views/admin/pool_families/_provider_accounts.haml
index e69de29..358e3d2 100644
--- a/src/app/views/admin/pool_families/_provider_accounts.haml
+++ b/src/app/views/admin/pool_families/_provider_accounts.haml
@@ -0,0 +1,25 @@
+%h3
+ Provider Accounts for
+ = @pool_family.name
+
+- unless @pool_family.provider_accounts.blank?
+ - form_tag do
+ = restful_submit_tag "Remove selected", 'destroy',
multi_destroy_provider_accounts_admin_pool_families_path(:pool_family_id =>
@pool_family.id), 'DELETE', :id => 'delete_button'
+ = #submit_tag "Remove selected"
+
+ %table#provider_accounts_table
+ = sortable_table_header @provider_accounts_header
+ - @pool_family.provider_accounts.each do |provider_account|
+ %tr
+ %td
+ - selected = @url_params[:select_provider_accounts] == 'all'
+ %input{:name => "provider_account_selected[]", :type =>
"checkbox", :value => provider_account.id, :id =>
"provider_account_checkbox_#{provider_account.id}", :checked => selected }
+ = provider_account.name
+
+- unless @pool_family.provider_accounts.count == ProviderAccount.all.count
+ - form_for :pool_family, :url => add_provider_account_admin_pool_families_path(:id
=> @pool_family.id) do |f|
+ - unless @pool_family.provider_accounts.blank?
+ = select_tag :provider_account_id, options_for_select(ProviderAccount.find(:all,
:conditions => [ "id NOT IN (?)", @pool_family.provider_account_ids
]).collect{ |pa| [pa.name, pa.id] })
+ - else
+ = select_tag :provider_account_id, options_for_select(ProviderAccount.all.collect{
|pa| [pa.name, pa.id] })
+ = f.submit "Add to #{(a)pool_family.name}"
diff --git a/src/config/routes.rb b/src/config/routes.rb
index 162260c..6aa9c8c 100644
--- a/src/config/routes.rb
+++ b/src/config/routes.rb
@@ -58,7 +58,7 @@ ActionController::Routing::Routes.draw do |map|
r.resources :roles, :collection => { :multi_destroy => :delete }
r.resources :settings, :collection => { :self_service => :get,
:general_settings => :get }
- r.resources :pool_families, :collection => { :multi_destroy => :delete }
+ r.resources :pool_families, :collection => { :multi_destroy => :delete,
:add_provider_account => :post, :multi_destroy_provider_accounts => :delete }
r.resources :realms, :collection => { :multi_destroy => :delete }
r.resources :realm_mappings, :collection => { :multi_destroy => :delete }
end
diff --git a/src/features/pool_family.feature b/src/features/pool_family.feature
index 6795d42..fd4eb55 100644
--- a/src/features/pool_family.feature
+++ b/src/features/pool_family.feature
@@ -78,3 +78,25 @@ Feature: Pool Families
Then I should see "first_family"
And I should see "second_family"
And I should see "third_family"
+
+ Scenario: Add provider account to pool family
+ Given 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
+ Then I should see "Provider Accounts for"
+ When I select "testaccount" from "provider_account_id"
+ And I press "pool_family_submit"
+ Then there should be 1 provider accounts assigned to "testpoolfamily"
+ And I should see "testaccount"
+
+ Scenario: Remove provider account from pool family
+ Given 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"
+ And I am on the pool family provider accounts page
+ Then I should see "testaccount"
+ When I check "testaccount" provider account
+ And I press "Remove selected"
+ Then there should not exist a provider account assigned to
"testpoolfamily"
diff --git a/src/features/step_definitions/pool_family_steps.rb
b/src/features/step_definitions/pool_family_steps.rb
index 9585a21..716d722 100644
--- a/src/features/step_definitions/pool_family_steps.rb
+++ b/src/features/step_definitions/pool_family_steps.rb
@@ -5,7 +5,7 @@ Given /^there are these pool families:$/ do |table|
end
Given /^there is a pool family named "([^\"]*)"$/ do |name|
- @provider = Factory(:pool_family, :name => name)
+ @pool_family = Factory(:pool_family, :name => name)
end
Given /^there is not a pool family named "([^"]*)"$/ do |name|
@@ -25,3 +25,24 @@ end
Then /^there should not exist a pool family named "([^\"]*)"$/ do
|name|
PoolFamily.find_by_name(name).should be_nil
end
+
+Then /^there should be (\d+) provider accounts assigned to "([^\"]*)"$/
do |count,name|
+ @pool_family = PoolFamily.find_by_name(name)
+ @pool_family.provider_accounts.count == count
+end
+
+Given /^there is a provider account "([^"]*)" related to pool family
"([^"]*)"$/ do |provider_account, pool_family|
+ @pool_family = PoolFamily.find_by_name(pool_family)
+ @provider_account = ProviderAccount.find_by_label(provider_account)
+ @pool_family.provider_accounts<< @provider_account
+end
+
+When /^I check "([^"]*)" provider account$/ do |label|
+ provider_account = ProviderAccount.find_by_label(label)
+ check("provider_account_checkbox_#{provider_account.id}")
+end
+
+Then /^there should not exist a provider account assigned to "([^"]*)"$/
do |name|
+ @pool_family = PoolFamily.find_by_name(name)
+ @pool_family.provider_accounts.count == 0
+end
diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb
index 5af9b34..a313aa0 100644
--- a/src/features/support/paths.rb
+++ b/src/features/support/paths.rb
@@ -83,6 +83,9 @@ module NavigationHelpers
when /the template builds page/
url_for image_factory_template_path(@template, :details_tab =>
'builds')
+ when /the pool family provider accounts page/
+ url_for admin_pool_family_path(@pool_family, :details_tab =>
'provider_accounts')
+
when /the templates page/
templates_path
Hey Jiri,
It looks good, but I'm afraid there's been some misunderstanding
regarding how pool families work.
As far as I'm aware, a provider account must be associated with *exactly
one* pool family at all times.
That means:
1) User should not be able to remove an account from the default pool family
2) When user removes an account from a non-default pool family, it
should be moved to the default one automatically
3) When user adds a provider account to a pool family, it must be
removed from the family it was associated with previously
Ideally, the patch should enforce these rules and have tests for them.
However.
It's entirely possible that my understanding of the pool families is
incorrect. We should clarify this with Scott once he gets online. If
your patch mimics the correct behaviour, I'll ack and push it immediatelly.
Take care,
Thomas