Comments below On Nov 8, 2010, at 2:36 PM, [email protected] wrote:
> From: Martyn Taylor <[email protected]> > > --- > src/app/controllers/provider_controller.rb | 27 +++++++++------ > src/app/models/provider.rb | 16 ++++++++- > src/app/views/provider/_providers.haml | 2 + > src/features/provider.feature | 19 ++++++++++- > src/features/step_definitions/provider_steps.rb | 41 +++++++++++++++++++++- > src/features/support/custom.rb | 12 +++--- > src/spec/factories/replicated_image.rb | 5 +++ > 7 files changed, 101 insertions(+), 21 deletions(-) > create mode 100644 src/spec/factories/replicated_image.rb > > diff --git a/src/app/controllers/provider_controller.rb > b/src/app/controllers/provider_controller.rb > index da5c9bb..8d6ff1f 100644 > --- a/src/app/controllers/provider_controller.rb > +++ b/src/app/controllers/provider_controller.rb > @@ -99,19 +99,24 @@ class ProviderController < ApplicationController > end > > def destroy > - if request.post? > - @provider = Provider.find(params[:provider][:id]) > - require_privilege(Privilege::PROVIDER_MODIFY, p) > - if @provider.destroy and @provider.destroyed? > - redirect_to :action => "index" > - else > - flash[:error] = { > - :summary => "Failed to delete Provider", > - :failures => @provider.errors.full_messages, > - } > - render :action => 'show' > + if request.post? || request.delete? > + @provider = Provider.find(params[:id]) > + require_privilege(Privilege::PROVIDER_MODIFY, @provider) > + if @provider.destroyable? > + if @provider.destroy and @provider.destroyed? > + redirect_to :action => "index" > + flash[:notice] = "Provider Deleted" > + return > + end > end > + > + flash[:error] = { > + :summary => "Failed to delete Provider", > + :failures => @provider.errors.full_messages, > + } > + render :action => 'show' > end > + render :action => 'show' > end > > def hardware_profiles > diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb > index 6ec3cc3..c4450b6 100644 > --- a/src/app/models/provider.rb > +++ b/src/app/models/provider.rb > @@ -38,7 +38,14 @@ class Provider < ActiveRecord::Base > :include => [:role], > :order => "permissions.id ASC" > > - before_destroy :destroyable? > + before_destroy { |provider| ReplicatedImage.destroy_all "provider_id = > #{provider.id}" } > + > + before_destroy { |provider| HardwareProfile.destroy_all "provider_id = > #{provider.id}" } > + > + before_destroy { |provider| Realm.destroy_all "provider_id = > #{provider.id}" } > + > + before_destroy { |provider| CloudAccount.destroy_all "provider_id = > #{provider.id}" } > + I have tested this and you don't have to do before_destroy with all this code because every has_many relation has :dependent => :destroy which will do what you do here. > > # there is a destroy dependency for a cloud accounts association, > # but a cloud account is silently not destroyed when there is > @@ -126,4 +133,11 @@ class Provider < ActiveRecord::Base > def valid_framework? > connect.nil? ? false : true > end > + > + def delete_associated_objects > + replicated_images.each |ri| ri.destoy > + hardware_profiles.each |hwp| hwp.destroy > + realms.each |r| r.destroy > + cloud_accounts.each |ca| ca.destroy > + end Didn't find any use of this method. Any reason to have it? > end > diff --git a/src/app/views/provider/_providers.haml > b/src/app/views/provider/_providers.haml > index f051692..f4eeef9 100644 > --- a/src/app/views/provider/_providers.haml > +++ b/src/app/views/provider/_providers.haml > @@ -11,3 +11,5 @@ > = submit_tag t(:edit), :disabled => ('disabled' unless @provider and > ['show', 'accounts'].include? controller.action_name) > - form_tag({:controller => 'provider', :action => 'new'}, {:method => :get > , :class => 'buttononly'}) do > %input{ :type => 'submit', :value => t(:add), :disabled => ('disabled' > unless @providers.length == 0) } > + - form_tag({:controller => 'provider', :action => 'destroy', :id => > @provider}, {:method => :post , :class => 'buttononly'}) do > + = submit_tag 'delete', :disabled => ('disabled' unless @provider and > ['show', 'accounts'].include? controller.action_name) > \ No newline at end of file > diff --git a/src/features/provider.feature b/src/features/provider.feature > index c332667..12047bb 100644 > --- a/src/features/provider.feature > +++ b/src/features/provider.feature > @@ -85,4 +85,21 @@ Feature: Manage Providers > And I fill in "cloud_account[password]" with "incorrect_password" > And I fill in "cloud_account[account_number]" with "12345678" > And I press "test_account" > - Then I should see "Test Connection Failed: Invalid Account Details" > \ No newline at end of file > + Then I should see "Test Connection Failed: Invalid Account Details" > + > + Scenario: Delete a provider > + Given I am on the homepage > + And there is a provider named "provider1" > + And this provider has 5 replicated images > + And this provider has 5 hardware profiles > + And this provider has a realm > + And this provider has a cloud account > + When I go to the providers page > + And I follow "provider1" > + And I press "delete" > + Then I should see "Provider Deleted" > + And there should not exist a provider named "provider1" > + And there should not be any replicated images > + And there should not be any hardware profiles > + And there should not be a cloud account > + And there should not be a realm > \ No newline at end of file > diff --git a/src/features/step_definitions/provider_steps.rb > b/src/features/step_definitions/provider_steps.rb > index e593d43..abc4662 100644 > --- a/src/features/step_definitions/provider_steps.rb > +++ b/src/features/step_definitions/provider_steps.rb > @@ -1,7 +1,12 @@ > -Given /^there is not a provider named "([^\"]*)"$/ do |name| > +Given /^there should not exist a provider named "([^\"]*)"$/ do |name| > Provider.find_by_name(name).should be_nil > end > > +Given /^there is not a provider named "([^"]*)"$/ do |name| > + provider = Provider.find_by_name(name) > + if provider then provider.destroy end > +end > + > Given /^there is a provider named "([^\"]*)"$/ do |name| > @provider = Factory(:mock_provider, :name => name) > end > @@ -20,9 +25,41 @@ When /^I delete provider$/ do > click_button "Delete provider" > end > > - > Given /^there are these providers:$/ do |table| > table.hashes.each do |hash| > Factory(:mock_provider, :name => hash['name']) > end > end > + > +Given /^this provider has (\d+) replicated images$/ do |number| > + number.to_i.times { |i| Factory(:replicated_image, :provider => @provider) > } > +end > + > +Given /^this provider has (\d+) hardware profiles$/ do |number| > + number.to_i.times { |i| Factory(:mock_hwp1, :provider => @provider) } > +end > + > + > +Given /^this provider has a realm$/ do > + Factory(:realm, :provider => @provider) > +end > + > +Given /^this provider has a cloud account$/ do > + Factory(:mock_cloud_account, :provider => @provider) > +end > + > +Then /^there should not be any replicated images$/ do > + ReplicatedImage.find(:all, :conditions => { :provider_id => @provider.id} > ).size.should == 0 > +end > + > +Then /^there should not be any hardware profiles$/ do > + HardwareProfile.find(:all, :conditions => { :provider_id => @provider.id} > ).size.should == 0 > +end > + > +Then /^there should not be a cloud account$/ do > + CloudAccount.find(:all, :conditions => { :provider_id => @provider.id} > ).size.should == 0 > +end > + > +Then /^there should not be a realm$/ do > + Realm.find(:all, :conditions => { :provider_id => @provider.id} > ).size.should == 0 > +end > diff --git a/src/features/support/custom.rb b/src/features/support/custom.rb > index 6e4e1ef..b5440ae 100644 > --- a/src/features/support/custom.rb > +++ b/src/features/support/custom.rb > @@ -34,12 +34,12 @@ CloudAccount.class_eval do > def generate_cloud_account_key > end > > - def instance_key > - @key = mock('Key', :null_object => true) > - @key.stub!(:pem).and_return("PEM") > - @key.stub!(:id).and_return("1_user") > - @key > - end > +# def instance_key > +# @key = mock('Key', :null_object => true) > +# @key.stub!(:pem).and_return("PEM") > +# @key.stub!(:id).and_return("1_user") > +# @key > +# end > end > > RepositoryManager.class_eval do > diff --git a/src/spec/factories/replicated_image.rb > b/src/spec/factories/replicated_image.rb > new file mode 100644 > index 0000000..f989380 > --- /dev/null > +++ b/src/spec/factories/replicated_image.rb > @@ -0,0 +1,5 @@ > +Factory.define :replicated_image do |ri| > + ri.association :image > + ri.association :provider > + ri.sequence(:provider_image_key) { |n| "provider_image_key#(n)" } > +end > \ No newline at end of file > -- > 1.7.2.3 Also this patch breaks unit tests for spec/model/provider_spec.rb with: 'Provider (using stubbed out connect method) should not destroy provider if deletion of associated cloud account fails' FAILED expected true to be false ./spec/models/provider_spec.rb:81: -- Ladislav _______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
