[PATCH conductor] Added check if there is provider account when adding provider account to poolfamily.

Tomas Sedovic tsedovic at redhat.com
Wed Feb 22 09:53:40 UTC 2012


On 02/21/2012 05:36 PM, Tomáš Hrčka wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=795659
> ---
>   src/app/controllers/pool_families_controller.rb |   14 ++++++++++----
>   src/config/locales/en.yml                       |    1 +
>   2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/src/app/controllers/pool_families_controller.rb b/src/app/controllers/pool_families_controller.rb
> index 7e33977..c4c0da8 100644
> --- a/src/app/controllers/pool_families_controller.rb
> +++ b/src/app/controllers/pool_families_controller.rb
> @@ -106,10 +106,16 @@ class PoolFamiliesController<  ApplicationController
>     def add_provider_accounts
>       @pool_family = PoolFamily.find(params[:id])
>       require_privilege(Privilege::MODIFY, @pool_family)
> -    @provider_accounts = ProviderAccount.
> -      list_for_user(current_user, Privilege::USE).
> -      where('id not in (?)', @pool_family.provider_accounts.empty? ?
> -                             0 : @pool_family.provider_accounts.map(&:id))
> +
> +    if ProviderAccount.count == 0
> +      flash[:error] = "#{t('pool_families.flash.error.no_provider_accounts')}"

The `t` function already returns a string, no interpolation is needed:

     flash[:error] = t('pool_families.flash.error.no_provider_accounts')

> +      redirect_to pool_family_path(@pool_family, :details_tab =>  'provider_accounts') and return

Because you leave the function here, the `else` statement below is not 
necessary. You can unindent the block.

> +    else
> +      @provider_accounts = ProviderAccount.
> +        list_for_user(current_user, Privilege::USE).
> +        where('id not in (?)', @pool_family.provider_accounts.empty? ?
> +                               0 : @pool_family.provider_accounts.map(&:id))
> +    end
>
>       load_tab_captions_and_details_tab('provider_accounts')
>
> diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> index addb68e..6640d69 100644
> --- a/src/config/locales/en.yml
> +++ b/src/config/locales/en.yml
> @@ -636,6 +636,7 @@ en:
>           provider_accounts_not_removed: "Could not remove these Provider Accounts"
>           select_to_add_accounts: "You must select at least one Provider Account to add."
>           select_to_remove_accounts: "You must select at least one Provider Account to remove."
> +        no_provider_accounts: "There are no provider accounts in system."

This should read "There are no provider accounts in *the* system.", this 
may be even better: "There are no provider accounts available."

>     catalog_entries:
>       new_catalog_entry: New Deployable
>       index:

ACK with a couple of nits inline. Please fix the I18n stuff, feel free 
to ignore the `else` thingy if you disagree.

Thomas



More information about the aeolus-devel mailing list