From: Tomas Sedovic <tsedovic(a)redhat.com>
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 | 98 ++++++++++++--------
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, 73 insertions(+), 47 deletions(-)
diff --git a/src/app/controllers/cloud_accounts_controller.rb
b/src/app/controllers/cloud_accounts_controller.rb
index c8e94fe..a9b6189 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,35 +75,51 @@ class CloudAccountsController < ApplicationController
end
def update_accounts
- if params[:update_cloud_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)
+ if not params[:update_cloud_accounts]
+ redirect_to :controller => 'provider', :action => 'accounts',
:id => @provider.id
+ end
- # blank password means the user didn't change it -- don't update it then
- if attributes.has_key? :password and String(attributes[:password]).empty?
- attributes.delete :password
- end
- @cloud_account.quota.maximum_running_instances =
quota_from_string(params[:quota][id][:maximum_running_instances])
- private_cert = attributes[:x509_cert_priv_file]
- if private_cert.nil?
- attributes.delete :x509_cert_priv
- else
- attributes[:x509_cert_priv] = private_cert.read
- attributes.delete :x509_cert_priv_file
- end
- public_cert = attributes[:x509_cert_pub_file]
- if public_cert.nil?
- attributes.delete :x509_cert_pub
- else
- attributes[:x509_cert_pub] = public_cert.read
- attributes.delete :x509_cert_pub_file
- end
- @cloud_account.update_attributes!(attributes)
- @cloud_account.quota.save!
+ success = true
+ @provider.cloud_accounts.each do |cloud_account|
+ attributes = params[:cloud_accounts][cloud_account.id.to_s]
+
+ # blank password means the user didn't change it -- don't update it then
+ if attributes.has_key? :password and String(attributes[:password]).empty?
+ attributes.delete :password
+ end
+ 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
+ attributes[:x509_cert_priv] = private_cert.read
+ attributes.delete :x509_cert_priv_file
end
+ public_cert = attributes[:x509_cert_pub_file]
+ if public_cert.nil?
+ attributes.delete :x509_cert_pub
+ else
+ attributes[:x509_cert_pub] = public_cert.read
+ attributes.delete :x509_cert_pub_file
+ end
+
+ 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[: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 8a8c36a..11b5ba0 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_for @provider, :url => {:controller => 'cloud_accounts', :action
=> 'update_accounts'}, :multipart => true do |form|
= 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