From: Tomas Sedovic tsedovic@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 | 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