[PATCH cloud engine 1/3] BZ640257: wrong tooltips in Provider Accounts page

[Tomas Sedovic]

From: Tomas Sedovic <tsedovic(a)redhat.com&gt;

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

The "EC2 Access Key" and "Secret Acces Key" fields had an
untintelligible
`title` attribute because the i18n engine was looking for a dictionary fields
that were missing.
---
 src/app/views/provider/accounts.haml |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/app/views/provider/accounts.haml b/src/app/views/provider/accounts.haml
index 87d85f1..8a8c36a 100644
--- a/src/app/views/provider/accounts.haml
+++ b/src/app/views/provider/accounts.haml
@@ -31,8 +31,8 @@
         - disabled = 'disabled' unless has_account_modify?(@provider)
         - cloud_account_id = "cloud_accounts[#{acct.id}]"
         = text_field cloud_account_id, :label, :title => t('.account_name'),
:value => acct.label, :disabled => disabled, :class => "grid_4 alpha"
-        = text_field cloud_account_id, :username, :title => t('.access_key'),
:value => acct.username, :disabled => disabled, :class => "grid_3"
-        = password_field cloud_account_id, :password, :title =>
t('.secret_access_key'), :disabled => disabled, :class =>
"grid_3"
+        = text_field cloud_account_id, :username, :title => t('.user_name'),
:value => acct.username, :disabled => disabled, :class => "grid_3"
+        = password_field cloud_account_id, :password, :title =>
t('.password'), :disabled => disabled, :class => "grid_3"
         = error_message_on :maximum_running_instances, 'Maximum Running Instances
'
         = text_field "quota[#{acct.id}]", :maximum_running_instances, :title
=> t('.quota_instances'), :value =>
(acct.quota.maximum_running_instances.nil? ? "unlimited" :
acct.quota.maximum_running_instances), :disabled => disabled, :id =>
"quota_instances#{acct.id}", :class => "grid_3 omega"
       %fieldset.nomargin.clearfix
@@ -85,8 +85,8 @@
         %span.required *
     %fieldset.nomargin.clearfix
       = text_field :cloud_account, :label, :title => t('.account_name'),
:class => "grid_4 alpha"
-      = text_field :cloud_account, :username, :title => t('.access_key'),
:class => "grid_3"
-      = password_field :cloud_account, :password, :title =>
t('.secret_access_key'), :class => "grid_3"
+      = text_field :cloud_account, :username, :title => t('.user_name'),
:class => "grid_3"
+      = password_field :cloud_account, :password, :title => t('.password'),
:class => "grid_3"
       = text_field "quota", :maximum_running_instances, :title =>
t('.quota_instances'), :value =>  "unlimited",:id =>
"quota_instances", :class => "grid_3 omega"
     %fieldset.nomargin.clearfix
       .grid_3.prefix_10.alpha.omega
-- 
1.7.2.3

[PATCH cloud engine 2/3] BZ640257: "Reset" button on the Cloud Account page

[Tomas Sedovic]

From: Tomas Sedovic <tsedovic(a)redhat.com&gt;

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

Previously, pressing the Reset button saved the entered input instead of
resetting the previous values.
---
 src/app/views/provider/accounts.haml |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/app/views/provider/accounts.haml b/src/app/views/provider/accounts.haml
index 8a8c36a..3efee3d 100644
--- a/src/app/views/provider/accounts.haml
+++ b/src/app/views/provider/accounts.haml
@@ -63,7 +63,7 @@
           %a.button{ :href => remove_path, :name =>
"remove_acc_#{acct.id}", :class => "tiny #{'disabled' unless
remove_path}" }
             = t(:remove)
       %fieldset.clearfix.gap
-        = submit_tag t(:reset), :name => 'reset_form', :class =>
"dialogbutton" unless @provider.cloud_accounts.empty?
+        = link_to t(:reset), {:controller => 'provider', :action =>
'accounts', :id => @provider.id}, :class => "button dialogbutton"
unless @provider.cloud_accounts.empty?
         = submit_tag t(:save), :name => 'update_cloud_accounts', :class =>
"dialogbutton" unless @provider.cloud_accounts.empty?
 
   %h2
-- 
1.7.2.3

[PATCH cloud engine 3/3] BZ640257: Required fields in cloud-accounts page

[Tomas Sedovic]

From: Tomas Sedovic <tsedovic(a)redhat.com&gt;

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

All the fields that are marked as "required" are now being checked and when
the user doesn't put anything in them, the create/edit action fails.
---
 src/app/controllers/cloud_accounts_controller.rb   |   76 ++++++++++++--------
 src/app/models/cloud_account.rb                    |    4 +
 src/app/views/provider/accounts.haml               |    6 +-
 .../20090801045212_create_cloud_accounts.rb        |    8 +-
 src/spec/factories/cloud_account.rb                |    4 +
 5 files changed, 61 insertions(+), 37 deletions(-)

diff --git a/src/app/controllers/cloud_accounts_controller.rb
b/src/app/controllers/cloud_accounts_controller.rb
index 9d75347..c9a0c67 100644
--- a/src/app/controllers/cloud_accounts_controller.rb
+++ b/src/app/controllers/cloud_accounts_controller.rb
@@ -44,23 +44,27 @@ class CloudAccountsController < ApplicationController
     if params[:test_account]
       test_account(@cloud_account)
       redirect_to :controller => "provider", :action =>
"accounts", :id => @provider, :cloud_account => params[:cloud_account]
+    elsif @cloud_account.valid?
+      quota = Quota.new
+      quota.maximum_running_instances =
quota_from_string(params[:quota][:maximum_running_instances])
+      quota.save!
+      @cloud_account.quota_id = quota.id
+      @cloud_account.zones << Zone.default
+      @cloud_account.save!
+      if request.post? && @cloud_account.save &&
@cloud_account.populate_realms
+        flash[:notice] = "Provider account added."
+      end
+      redirect_to :controller => "provider", :action =>
"accounts", :id => @provider
+      kick_condor
     else
-      unless @cloud_account.valid_credentials?
+      if not @cloud_account.valid_credentials?
         flash[:notice] = "The entered credential information is incorrect"
-        redirect_to :controller => "provider", :action =>
"accounts", :id => @provider
+      elsif @cloud_account.errors.on(:username)
+        flash[:notice] = "The access key
'#{params[:cloud_account][:username]}' has already been taken."
       else
-        quota = Quota.new
-        quota.maximum_running_instances =
quota_from_string(params[:quota][:maximum_running_instances])
-        quota.save!
-        @cloud_account.quota_id = quota.id
-        @cloud_account.zones << Zone.default
-        @cloud_account.save!
-        if request.post? && @cloud_account.save &&
@cloud_account.populate_realms
-          flash[:notice] = "Provider account added."
-        end
-        redirect_to :controller => "provider", :action =>
"accounts", :id => @provider
-        kick_condor
+        flash[:notice] = "You must fill in all the required fields"
       end
+      redirect_to :controller => "provider", :action =>
"accounts", :id => @provider, :cloud_account => params[:cloud_account]
     end
   end
 
@@ -71,33 +75,47 @@ class CloudAccountsController < ApplicationController
   end
 
   def update_accounts
-    params[:cloud_accounts].each do |id, attributes|
-      @cloud_account = CloudAccount.find(id)
-      require_privilege(Privilege::ACCOUNT_MODIFY, @cloud_account.provider)
+    @provider = Provider.find(params[:provider][:id])
+    require_privilege(Privilege::ACCOUNT_MODIFY, @provider)
+    @providers = Provider.list_for_user(@current_user, Privilege::PROVIDER_VIEW)
 
+    success = true
+    @provider.cloud_accounts.each do |cloud_account|
+      attributes = params[:cloud_accounts][cloud_account.id.to_s]
+
+      password = attributes[:password]
       # blank password means the user didn't change it -- don't update it then
-      if attributes.has_key? :password and String(attributes[:password]).empty?
+      if password.blank?
         attributes.delete :password
       end
-      @cloud_account.quota.maximum_running_instances =
quota_from_string(params[:quota][id][:maximum_running_instances])
+      cloud_account.quota.maximum_running_instances =
quota_from_string(params[:quota][cloud_account.id.to_s][:maximum_running_instances])
+
       private_cert = attributes[:x509_cert_priv_file]
-      if private_cert.nil?
-        attributes.delete :x509_cert_priv
-      else
+      unless private_cert.blank?
         attributes[:x509_cert_priv] = private_cert.read
-        attributes.delete :x509_cert_priv_file
       end
+      attributes.delete :x509_cert_priv_file
+
       public_cert = attributes[:x509_cert_pub_file]
-      if public_cert.nil?
-        attributes.delete :x509_cert_pub
-      else
+      unless public_cert.blank?
         attributes[:x509_cert_pub] = public_cert.read
-        attributes.delete :x509_cert_pub_file
       end
-      @cloud_account.update_attributes!(attributes)
-      @cloud_account.quota.save!
+      attributes.delete :x509_cert_pub_file
+
+      begin
+        cloud_account.update_attributes!(attributes)
+        cloud_account.quota.save!
+      rescue
+        success = false
+      end
+    end
+    if success
+      flash[:notice] = "Account updated."
+      redirect_to :controller => 'provider', :action => 'accounts',
:id => @provider
+    else
+      flash.now[:notice] = "Error updating the cloud account."
+      render :template => 'provider/accounts'
     end
-    redirect_to :controller => 'provider', :action => 'accounts',
:id => params[:provider][:id]
   end
 
   def update
diff --git a/src/app/models/cloud_account.rb b/src/app/models/cloud_account.rb
index cbe4d9f..5673e9a 100644
--- a/src/app/models/cloud_account.rb
+++ b/src/app/models/cloud_account.rb
@@ -32,9 +32,13 @@ class CloudAccount < ActiveRecord::Base
 
   validates_presence_of :provider_id
 
+  validates_presence_of :label
   validates_presence_of :username
   validates_uniqueness_of :username, :scope => :provider_id
   validates_presence_of :password
+  validates_presence_of :account_number
+  validates_presence_of :x509_cert_pub
+  validates_presence_of :x509_cert_priv
 
   has_many :permissions, :as => :permission_object, :dependent => :destroy,
            :include => [:role],
diff --git a/src/app/views/provider/accounts.haml b/src/app/views/provider/accounts.haml
index 3efee3d..e555717 100644
--- a/src/app/views/provider/accounts.haml
+++ b/src/app/views/provider/accounts.haml
@@ -2,16 +2,14 @@
   function set_unlimited_quota(elem_id) {
     $("#" + elem_id)[0].value = "unlimited";
   }
-= render :partial => 'providers'
+= render :partial => 'provider/providers'
 #details.grid_13
   %nav.subsubnav
     = render_navigation(:level => 4)
 
   %h1
     = t('.provider_accounts')
-  - form_tag({:controller => 'cloud_accounts', :action =>
'update_accounts'}, :multipart => true) do
-
-
+  - form_tag ({:controller => 'cloud_accounts', :action =>
'update_accounts'}), :multipart => true do
     = hidden_field :provider, :id, :value => @provider.id
     - @provider.cloud_accounts.each do |acct|
       %fieldset.clearfix.nomargin
diff --git a/src/db/migrate/20090801045212_create_cloud_accounts.rb
b/src/db/migrate/20090801045212_create_cloud_accounts.rb
index e1f2e7b..042c406 100644
--- a/src/db/migrate/20090801045212_create_cloud_accounts.rb
+++ b/src/db/migrate/20090801045212_create_cloud_accounts.rb
@@ -22,15 +22,15 @@
 class CreateCloudAccounts < ActiveRecord::Migration
   def self.up
     create_table :cloud_accounts do |t|
-      t.string :label
+      t.string :label, :null => false
       t.string :username, :null => false
       t.string :password, :null => false
       t.integer :provider_id, :null => false
       t.integer :quota_id
       t.integer :lock_version, :default => 0
-      t.string  :account_number
-      t.text    :x509_cert_priv
-      t.text    :x509_cert_pub
+      t.string  :account_number, :null => false
+      t.text    :x509_cert_priv, :null => false
+      t.text    :x509_cert_pub, :null => false
       t.timestamps
     end
   end
diff --git a/src/spec/factories/cloud_account.rb b/src/spec/factories/cloud_account.rb
index 27d0a1e..797fa47 100644
--- a/src/spec/factories/cloud_account.rb
+++ b/src/spec/factories/cloud_account.rb
@@ -1,6 +1,10 @@
 Factory.define :cloud_account do |f|
   f.sequence(:username) { |n| "testUser#(n)" }
   f.password "testPassword"
+  f.sequence(:label) { |n| "test label#(n)" }
+  f.account_number "3141"
+  f.x509_cert_priv "x509 private key"
+  f.x509_cert_pub "x509 public key"
   f.association :provider
 end
 
-- 
1.7.2.3

[Jason Guiditta]

On Fri, 2010-10-22 at 21:37 +0200, tsedovic(a)redhat.com wrote:
>  From: Tomas Sedovic <tsedovic(a)redhat.com&gt;
>  
>  https://bugzilla.redhat.com/show_bug.cgi?id=640257
>  
>  The "EC2 Access Key" and "Secret Acces Key" fields had an
> untintelligible
>  `title` attribute because the i18n engine was looking for a dictionary fields
>  that were missing. 
ACK to the series