[PATCH 3/3] BZ 801911 added checks before destroying a pool family

Jan Provazník jprovazn at redhat.com
Fri Mar 16 16:02:35 UTC 2012


On 03/14/2012 03:55 PM, Tzu-Mainn Chen wrote:
> ---
>   src/app/controllers/pool_families_controller.rb |    5 ++++-
>   1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/src/app/controllers/pool_families_controller.rb b/src/app/controllers/pool_families_controller.rb
> index 0a4f2d5..ee74d96 100644
> --- a/src/app/controllers/pool_families_controller.rb
> +++ b/src/app/controllers/pool_families_controller.rb
> @@ -95,11 +95,14 @@ class PoolFamiliesController<  ApplicationController
>     def destroy
>       pool_family = PoolFamily.find(params[:id])
>       require_privilege(Privilege::MODIFY, pool_family)
> -    if pool_family.destroy
> +    if pool_family == PoolFamily.default
> +      flash[:error] = t("pool_families.flash.error.default_pool_family_not_deleted")

It's better to do all checks in model (this check is already done in 
before_destroy callback) and just call errors.add(:base, msg) in the 
model. Then in the controller fill in flash[:error] with all object's 
errors.

But your solution works fine so go ahead and push it. Similar code is on 
more places in conductor's code, so we can optimize it post 1.0.

> +    elsif pool_family.destroy
>         flash[:notice] = t "pool_families.flash.notice.deleted"
>       else
>         flash[:error] = t "pool_families.flash.error.not_deleted"
>       end
> +
>       redirect_to pool_families_path
>     end
>

ACK, one note inline



More information about the aeolus-devel mailing list