Re: [PATCH conductor] Fix BZ 678071

[Francesco Vollero]

Ack'd

Regards,

Francesco Vollero

----- Messaggio originale -----
Da: [e-mail]
A: [e-mail]
Inviato: Martedì, 22 marzo 2011 16:22:13
Oggetto: [PATCH conductor] Fix BZ 678071

From: Tomas Sedovic

https://bugzilla.redhat.com/show_bug.cgi?id=678071

Added appropriate messages when deleting a provider account.
---
 .../admin/provider_accounts_controller.rb          |   23 ++++++++++++++++---
 src/config/locales/en.yml                          |    6 +++++
 src/features/provider_account.feature              |    8 +++++++
 .../step_definitions/provider_account_steps.rb     |   12 ++++++++++
 4 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/src/app/controllers/admin/provider_accounts_controller.rb b/src/app/controllers/admin/provider_accounts_controller.rb
index 6affc73..ceffe17 100644
--- a/src/app/controllers/admin/provider_accounts_controller.rb
+++ b/src/app/controllers/admin/provider_accounts_controller.rb
@@ -119,13 +119,28 @@ class Admin::ProviderAccountsController < ApplicationController
   end

def multi_destroy
-    if (not params[:accounts_selected]) or (params[:accounts_selected].length == 0)
+    if params[:accounts_selected].blank?
       flash[:notice] = "You must select some accounts first."
-    else
-      ProviderAccount.find(params[:accounts_selected]).each do |account|
-        account.destroy if check_privilege(Privilege::MODIFY, account)
+      redirect_to admin_provider_accounts_url and return
+    end
+
+    succeeded = []
+    failed = []
+    ProviderAccount.find(params[:accounts_selected]).each do |account|
+      if check_privilege(Privilege::MODIFY, account) && account.destroyable?
+        account.destroy
+        succeeded << account.label
+      else
+        failed << account.label
       end
     end
+
+    unless succeeded.empty?
+      flash[:notice] = t 'provider_accounts.index.account_deleted', :count => succeeded.length, :list => succeeded.join(', ')
+    end
+    unless failed.empty?
+      flash[:error] = t 'provider_accounts.index.account_not_deleted', :count => failed.length, :list => failed.join(', ')
+    end
     redirect_to admin_provider_accounts_url
   end

diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 233cd14..51555a4 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -119,6 +119,12 @@ en:
   provider_accounts:
     index:
       provider_accounts: Provider Accounts
+      account_deleted:
+        one: "Account %{list} was deleted."
+        other: "Accounts %{list} were deleted."
+      account_not_deleted:
+        one: "Account %{list} was not deleted. There are instances associated with it."
+        other: "Accounts %{list} were not deleted. They have instances associated with them."
     new:
       new_provider_account: New Account
       required_field: Required field.
diff --git a/src/features/provider_account.feature b/src/features/provider_account.feature
index af866e7..e59dab9 100644
--- a/src/features/provider_account.feature
+++ b/src/features/provider_account.feature
@@ -58,10 +58,18 @@ Feature: Manage Provider Accounts
   Scenario: Delete a provider account
     Given there is a provider named "testprovider"
     And there is a provider account named "testaccount"
+    And the account has an instance associated with it
     And I am on the admin provider accounts page
     When I check the "testaccount" account
     And I press "Delete"
     Then I should be on the admin provider accounts page
+    And I should see "was not deleted"
+    And there should be 1 provider account
+    When I delete all instances from the account
+    And I check the "testaccount" account
+    And I press "Delete"
+    Then I should be on the admin provider accounts page
+    And I should see "was deleted"
     And there should be no provider accounts

Scenario: Search for Provider Accounts
diff --git a/src/features/step_definitions/provider_account_steps.rb b/src/features/step_definitions/provider_account_steps.rb
index 86a6d31..561314a 100644
--- a/src/features/step_definitions/provider_account_steps.rb
+++ b/src/features/step_definitions/provider_account_steps.rb
@@ -1,7 +1,19 @@
+Given /^the account has an instance associated with it$/ do
+  Factory :instance, :provider_account => @provider_account
+end
+
+When /^I delete all instances from the account$/ do
+  @provider_account.instances.each { |i| i.destroy }
+end
+
 Then /^there should be no provider accounts$/ do
   ProviderAccount.all.should be_empty
 end

+Then /^there should be (\d+) provider accounts?$/ do |count|
+  ProviderAccount.all.length.should == count.to_i
+end
+
 Given /^there are no provider accounts$/ do
   ProviderAccount.all.should be_empty
 end