On 01/31/2011 12:34 PM, jzigmund@redhat.com wrote:
From: Jozef Zigmund <jzigmund@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]},
+      "Provider Account Owner" => {ProviderAccount => [VIEW,USE,MOD,    VPRM,GPRM]}},
    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],
-                                   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