From: Tomas Sedovic tsedovic@redhat.com
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
From: Tomas Sedovic tsedovic@redhat.com
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/controllers/cloud_accounts_controller.rb | 48 +++++++++++---------- 1 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/src/app/controllers/cloud_accounts_controller.rb b/src/app/controllers/cloud_accounts_controller.rb index 9d75347..c8e94fe 100644 --- a/src/app/controllers/cloud_accounts_controller.rb +++ b/src/app/controllers/cloud_accounts_controller.rb @@ -71,31 +71,33 @@ 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) + 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)
- # 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 + # 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! end - @cloud_account.update_attributes!(attributes) - @cloud_account.quota.save! end redirect_to :controller => 'provider', :action => 'accounts', :id => params[:provider][:id] end
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 | 97 ++++++++++++-------- 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, 72 insertions(+), 47 deletions(-)
diff --git a/src/app/controllers/cloud_accounts_controller.rb b/src/app/controllers/cloud_accounts_controller.rb index c8e94fe..e1a50c5 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,50 @@ 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) - - # 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! + @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 + + 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 password.blank? + 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] + unless private_cert.blank? + attributes[:x509_cert_priv] = private_cert.read + end + attributes.delete :x509_cert_priv_file + + public_cert = attributes[:x509_cert_pub_file] + unless public_cert.blank? + attributes[:x509_cert_pub] = public_cert.read end + 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[: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..f0eb629 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
deltacloud-devel@lists.fedorahosted.org
-
Tomas Sedovic