Re: [PATCH] Prohibit deletion of default pool_family

On 03/21/2011 09:49 PM, Matt Wagner wrote:
> .../controllers/admin/pool_families_controller.rb  |   11 ++++++++++-
>   src/features/pool_family.feature                   |    8 ++++++++
>   2 files changed, 18 insertions(+), 1 deletions(-)
>
> diff --git a/src/app/controllers/admin/pool_families_controller.rb b/src/app/controllers/admin/pool_families_controller.rb
> index 42bfb04..ac7dd68 100644
> --- a/src/app/controllers/admin/pool_families_controller.rb
> +++ b/src/app/controllers/admin/pool_families_controller.rb
> @@ -53,7 +53,16 @@ class Admin::PoolFamiliesController<  ApplicationController
>     end
> def multi_destroy
> PoolFamily.destroy(params[:pool_family_selected])
> default_pool_family = PoolFamily.default
> PoolFamily.find(params[:pool_family_selected]).each do |pool_family|
>  # Don't allow deleting the default PoolFamily
>  if pool_family == default_pool_family
>    flash[:notice] = "The default pool family cannot be deleted"
>  else
>    # TODO - Once PoolFamily supports permissions, this should have a permissions check on this
>    pool_family.destroy
>  end
> end
>  redirect_to admin_pool_families_path
> end
>
> diff --git a/src/features/pool_family.feature b/src/features/pool_family.feature
> index 441e927..e37f22e 100644
> --- a/src/features/pool_family.feature
> +++ b/src/features/pool_family.feature
> @@ -44,3 +44,11 @@ Feature: Pool Families
>       And I check "poolfamily1" pool family
>       And I press "Delete"
>       Then there should not exist a pool family named "poolfamily1"
> Scenario: Disallow deletion of default pool family
> Given I am on the admin pool families page
> Then I should see "default"
> When I check "default" pool family
> And I press "Delete"
> Then I should see "The default pool family cannot be deleted"
> Then I should see "default"

Your solution is functionally, but it have a problem .. it will protect 
against destroy only when destroy's call will be called from the UI 
through poolfam controller's action multi_destroy, what is not enough.

Please rework it and send again.

--
Jozef Zigmund