On 01/31/2011 12:34 PM, jzigmund@redhat.com wrote:
From: Jozef Zigmundjzigmund@redhat.com
Inside this patch I rename model of CloudAccount to ProviderAccount. Also I refactor all codes where this model was called. All supported providers are included in constant hash, that is named PROVIDER_TYPES in Provider model. So now, attribute of class "cloud_type" is renamed to "provider_type" and it must be one of these PROVIDER_TYPES value. I fixed all Rspecs and Cucumbers for provider_account model.
When you'll test this patch won't forget to run "rake db:migrate", because some of the attributes were changed in DB.
diff --git a/src/app/controllers/admin/provider_accounts_controller.rb b/src/app/controllers/admin/provider_accounts_controller.rb index d422a53..40b700b 100644 --- a/src/app/controllers/admin/provider_accounts_controller.rb +++ b/src/app/controllers/admin/provider_accounts_controller.rb @@ -38,71 +38,77 @@ class Admin::ProviderAccountsController< ApplicationController end
def new
- @cloud_account = CloudAccount.new
- @provider_account = ProviderAccount.new @quota = Quota.new @providers = Provider.all
- if @providers.empty?
flash[:error] = "You don't have any provider yet. Please create one!"
- else
- @selected_provider = @providers.first unless @providers.blank?
- end end
Hmm. I think the provider needs to be selected before getting here -- we need to pull this from the 'new provider account' link (which should be an action off the provider). This may be an issue with the existing UI rather than part of this patch though.
def create
- @provider = Provider.find(params[:provider_id])
- require_privilege(Privilege::CREATE, CloudAccount, @provider)
- @selected_provider = @provider = Provider.find(params[:select_provider])
- require_privilege(Privilege::CREATE, ProviderAccount, @provider)
If we use provider_id instead of select_provider, the provider should be set without having to do this as a separate step.
@@ -112,13 +118,30 @@ class Admin::ProviderAccountsController< ApplicationController if (not params[:accounts_selected]) or (params[:accounts_selected].length == 0) flash[:notice] = "You must select some accounts first." else
CloudAccount.find(params[:accounts_selected]).each do |account|
ProviderAccount.find(params[:accounts_selected]).each do |account| account.destroy if check_privilege(Privilege::MODIFY, account) end end redirect_to admin_provider_accounts_url
end
def set_selected_provider
@quota = Quota.new
@provider_account = ProviderAccount.new
respond_to do |format|
format.js {
@providers = Provider.find(:all)
@selected_provider = Provider.find(params[:select_provider])
render :partial => 'provider_selection'
}
format.html {
@providers = Provider.find(:all)
@selected_provider = Provider.find(params[:select_provider])
render :action => 'new', :layout => true
}
end
end
protected
def test_account(account)
diff --git a/src/db/migrate/20110117140824_rename_cloud_account_to_provider_account.rb b/src/db/migrate/20110117140824_rename_cloud_account_to_provider_account.rb new file mode 100644 index 0000000..beaabc5 --- /dev/null +++ b/src/db/migrate/20110117140824_rename_cloud_account_to_provider_account.rb @@ -0,0 +1,11 @@ +class RenameCloudAccountToProviderAccount< ActiveRecord::Migration
- def self.up
- rename_table (:cloud_accounts,:provider_accounts)
- change_column (:providers, :cloud_type, :integer, :null => false)
- end
- def self.down
- change_column (:providers, :cloud_type, :string, :null => false)
- rename_table (:provider_accounts, :cloud_accounts)
- end
+end diff --git a/src/db/seeds.rb b/src/db/seeds.rb index 9a6065c..1ed2329 100644 --- a/src/db/seeds.rb +++ b/src/db/seeds.rb @@ -30,17 +30,17 @@ roles = Quota => [VIEW]}}, Provider => {"Provider Owner" => {Provider => [VIEW, MOD, VPRM,GPRM],
CloudAccount => [VIEW,USE,MOD,CRE,VPRM,GPRM]}},
- CloudAccount =>
{"Provider Account User" => {CloudAccount => [VIEW,USE]},
"Provider Account Owner" => {CloudAccount => [VIEW,USE,MOD, VPRM,GPRM]}},
ProviderAccount => [VIEW,USE,MOD,CRE,VPRM,GPRM]}},
- ProviderAccount =>
{"Provider Account User" => {ProviderAccount => [VIEW,USE]},
Template => {"Template User" => {Template => [VIEW,USE]}, "Template Owner" => {Template => [VIEW,USE,MOD, VPRM,GPRM]}}, BasePermissionObject => {"Provider Creator" => {Provider => [ CRE]}, "Provider Administrator" => {Provider => [VIEW, MOD,CRE,VPRM,GPRM],"Provider Account Owner" => {ProviderAccount => [VIEW,USE,MOD, VPRM,GPRM]}},
CloudAccount => [VIEW,USE,MOD,CRE,VPRM,GPRM]},
ProviderAccount => [VIEW,USE,MOD,CRE,VPRM,GPRM]}, "HWP Administrator" => {HardwareProfile => [ MOD,CRE,VPRM,GPRM]}, "Realm Administrator" => {Realm => [ USE,MOD,CRE,VPRM,GPRM]}, "Pool Creator" => {Pool => [ CRE]},
@@ -50,7 +50,7 @@ roles = PoolFamily => [VIEW, MOD,CRE,VPRM,GPRM]}, "Template Administrator" => {Template => [VIEW,USE,MOD,CRE,VPRM,GPRM]}, "Administrator" => {Provider => [VIEW, MOD,CRE,VPRM,GPRM],
CloudAccount => [VIEW,USE,MOD,CRE,VPRM,GPRM],
ProviderAccount => [VIEW,USE,MOD,CRE,VPRM,GPRM], HardwareProfile => [ MOD,CRE,VPRM,GPRM], Realm => [ USE,MOD,CRE,VPRM,GPRM], User => [VIEW, MOD,CRE],
You'll need to edit the existing Roles and Privileges objects in a migration to make the corresponding change. Without this, running 'rake db:migrate' won't work as the roles and privileges data (previously loaded via seeds.rb) will be wrong.
For new database, seeds.rb will load the proper data, but for migrating an existing detabase, you need to update all roles with a scope of "CloudAccount" to change scope to "ProviderAccount", and all Privileges with a target_type of "CloudAccount" to change target_type to "ProviderAccount"
To be really complete you should probably also look for Permissions with permission_object_type="CloudAccount" and change those records accordingly as well.
diff --git a/src/db/migrate/20110117140824_rename_cloud_account_to_provider_account.rb b/src/db/migrate/20110117140824_rename_cloud_account_to_provider_account.rb new file mode 100644 index 0000000..beaabc5 --- /dev/null +++ b/src/db/migrate/20110117140824_rename_cloud_account_to_provider_account.rb @@ -0,0 +1,11 @@ +class RenameCloudAccountToProviderAccount< ActiveRecord::Migration
- def self.up
- rename_table (:cloud_accounts,:provider_accounts)
- change_column (:providers, :cloud_type, :integer, :null => false)
- end
- def self.down
- change_column (:providers, :cloud_type, :string, :null => false)
- rename_table (:provider_accounts, :cloud_accounts)
- end
This one's not working on postgres:
PGError: ERROR: column "cloud_type" cannot be cast to type integer : ALTER TABLE "providers" ALTER COLUMN "cloud_type" TYPE integer
This worked for me:
diff --git a/src/db/migrate/20110117140824_rename_cloud_account_to_provider_account.rb b/src/db/migrate/20110117140824_rename_cloud_account_to_provider_account.rb index beaabc5..ecd313a 100644 --- a/src/db/migrate/20110117140824_rename_cloud_account_to_provider_account.rb +++ b/src/db/migrate/20110117140824_rename_cloud_account_to_provider_account.rb @@ -1,11 +1,13 @@ class RenameCloudAccountToProviderAccount < ActiveRecord::Migration def self.up rename_table (:cloud_accounts,:provider_accounts) - change_column (:providers, :cloud_type, :integer, :null => false) + remove_column (:providers, :cloud_type) + add_column (:providers, :cloud_type, :integer, :null => false) end
def self.down - change_column (:providers, :cloud_type, :string, :null => false) + remove_column (:providers, :cloud_type) + add_column (:providers, :cloud_type, :string, :null => false) rename_table (:provider_accounts, :cloud_accounts) end end
However -- this will definitely _not_ work if you already have providers defined -- in that case you'd need a temp column and to translate the column from the string value to the proper int value. If we're not doing this, we need to make clear that these migrations are only supported for databases which have not yet defined any providers.
Running cucumber and spec tests (on postgres) I hit the following errors: ]$ cucumber features/template.feature Using the default profile... /usr/lib/ruby/gems/1.8/gems/rest-client-1.6.1/lib/restclient/abstract_response.rb:50: warning: parenthesize argument(s) for future version ..................................................................................................................F---------..................................F------------
(::) failed steps (::)
PGError: ERROR: operator does not exist: character varying = integer LINE 1: SELECT * FROM "images" WHERE ("images"."target" = 1 AND "ima... ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. : SELECT * FROM "images" WHERE ("images"."target" = 1 AND "images"."template_id" = 37) LIMIT 1 (ActiveRecord::StatementInvalid) ./app/models/image.rb:65:in `build' ./features/step_definitions/template_steps.rb:101:in `/^there is Amazon AWS build for this template$/' features/template.feature:109:in `And there is Amazon AWS build for this template'
expected the following element's content to include "Test1":
Red Hat Cloud Engine Red Hat Cloud Engine Hello John Smith Log out Resource ManagementImage FactoryAdministrationDashboardTemplatesAssembliesDeployables Saved searches NAMEOSVERSIONBOOTABLEARCHNo Templates //<![CDATA[ $(document).ready(function () { $('#delete_button').click(function(e) { if ($("#templates_table input[@type=radio]:checked").length == 0) { alert('Please select any template to be deleted before clicking Delete button.'); e.preventDefault(); } else { if (!confirm("Are you sure you want to delete this template?")) { e.preventDefault(); } } }); }); //]]> Log out Copyright © 2010 Red Hat, Inc. (Spec::Expectations::ExpectationNotMetError) ./features/step_definitions/web_steps.rb:155:in `/^(?:|I )should see "([^"]*)"$/' features/template.feature:157:in `Then I should see "Test1"'
Failing Scenarios: cucumber features/template.feature:105 # Scenario: Build template which is already built cucumber features/template.feature:128 # Scenario: Search for templates
11 scenarios (2 failed, 9 passed) 167 steps (2 failed, 21 skipped, 144 passed) 0m21.606s
$ spec spec/models/instance_observer_spec.rb /usr/lib/ruby/gems/1.8/gems/rest-client-1.6.1/lib/restclient/abstract_response.rb:50: warning: parenthesize argument(s) for future version .............F
1) 'InstanceObserver should generate instance key when instance is running' FAILED expected not: == nil, got: nil ./spec/models/instance_observer_spec.rb:169:
Finished in 53.000692 seconds
14 examples, 1 failure