[PATCH aggregator] Refactor Provider Accounts to have index with list

Redmine: https://morazi.usersys.redhat.com/redmine/issues/10

Ok, hopefully sending this patch last time. It's fixing broken form with cert files for provider accounts.

From: Ladislav Martincik

---
 src/app/controllers/cloud_accounts_controller.rb   |   88 +++++++++++---------
 src/app/controllers/providers_controller.rb        |    9 --
 src/app/models/cloud_account.rb                    |   58 +++++++++----
 src/app/views/cloud_accounts/_form.haml            |   89 ++++++++++++--------
 src/app/views/cloud_accounts/edit.haml             |   23 ++++--
 src/app/views/cloud_accounts/index.haml            |   25 ++++++
 src/app/views/cloud_accounts/new.haml              |   25 ++++--
 src/app/views/layouts/_notification.rhtml          |    3 +
 src/config/locales/en.yml                          |   11 ++-
 src/config/navigation.rb                           |    4 +-
 src/config/routes.rb                               |    5 +-
 src/features/provider.feature                      |    4 +-
 src/features/step_definitions/web_steps.rb         |    6 ++
 .../controllers/cloud_accounts_controller_spec.rb  |   37 +++++++--
 src/spec/controllers/provider_controller_spec.rb   |   12 ---
 src/spec/models/cloud_account_spec.rb              |   15 ++--
 16 files changed, 265 insertions(+), 149 deletions(-)
 create mode 100644 src/app/views/cloud_accounts/index.haml

diff --git a/src/app/controllers/cloud_accounts_controller.rb b/src/app/controllers/cloud_accounts_controller.rb
index e1c9b23..6a2a072 100644
--- a/src/app/controllers/cloud_accounts_controller.rb
+++ b/src/app/controllers/cloud_accounts_controller.rb
@@ -21,6 +21,14 @@

class CloudAccountsController < ApplicationController
   before_filter :require_user
+  before_filter :load_providers
+
+  helper :providers
+
+  def index
+    @provider = Provider.find(params[:provider_id])
+    require_privilege(Privilege::ACCOUNT_VIEW, @provider)
+  end

def new
     @provider = Provider.find(params[:provider_id])
@@ -29,43 +37,38 @@ class CloudAccountsController < ApplicationController
   end

def create
-    @provider = Provider.find(params[:cloud_account][:provider_id])
+    @provider = Provider.find(params[:provider_id])
     require_privilege(Privilege::ACCOUNT_MODIFY,@provider)
-    if params[:cloud_account] && !params[:cloud_account][:x509_cert_priv_file].blank?
-      params[:cloud_account][:x509_cert_priv] = params[:cloud_account][:x509_cert_priv_file].read
-    end
-    params[:cloud_account].delete :x509_cert_priv_file
-    if params[:cloud_account] && !params[:cloud_account][:x509_cert_pub_file].blank?
-      params[:cloud_account][:x509_cert_pub] = params[:cloud_account][:x509_cert_pub_file].read
-    end
-    params[:cloud_account].delete :x509_cert_pub_file
     @cloud_account = CloudAccount.new(params[:cloud_account])
+    @cloud_account.provider = @provider

if params[:test_account]
       test_account(@cloud_account)
-      redirect_to :controller => "providers", :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 => "providers", :action => "accounts", :id => @provider
-      kick_condor
-    else
+      render :action => 'new' and return
+    end
+
+    if @cloud_account.invalid?
       if not @cloud_account.valid_credentials?
-        flash[:notice] = "The entered credential information is incorrect"
+        flash.now[:error] = "The entered credential information is incorrect"
       elsif @cloud_account.errors.on(:username)
-        flash[:notice] = "The access key '#{params[:cloud_account][:username]}' has already been taken."
+        flash.now[:error] = "The access key '#{params[:cloud_account][:username]}' has already been taken."
       else
-        flash[:notice] = "You must fill in all the required fields"
+        flash.now[:error] = "You must fill in all the required fields"
       end
-      redirect_to :controller => "providers", :action => "accounts", :id => @provider, :cloud_account => params[:cloud_account]
+      render :action => 'new' and return
     end
+
+    quota = Quota.new
+    quota.maximum_running_instances = quota_from_string(params[:quota][:maximum_running_instances])
+    quota.save!
+    @cloud_account.quota = quota
+    @cloud_account.zones << Zone.default
+    @cloud_account.save!
+    if @cloud_account.populate_realms
+      flash[:notice] = "Provider account added."
+    end
+    redirect_to provider_accounts_path(@provider)
+    kick_condor
   end

def edit
@@ -119,11 +122,12 @@ class CloudAccountsController < ApplicationController
   end

def update
-    @cloud_account = CloudAccount.find(params[:cloud_account][:id])
-    require_privilege(Privilege::ACCOUNT_MODIFY,@cloud_account.provider)
+    @cloud_account = CloudAccount.find(params[:id])
+    @provider = @cloud_account.provider
+    require_privilege(Privilege::ACCOUNT_MODIFY, @provider)
     if @cloud_account.update_attributes(params[:cloud_account])
       flash[:notice] = "Cloud Account updated!"
-      redirect_to :controller => 'providers', :action => 'accounts', :id => @cloud_account.provider.id
+      redirect_to provider_accounts_path(@provider)
     else
       render :action => :edit
     end
@@ -138,27 +142,27 @@ class CloudAccountsController < ApplicationController
   end

def destroy
-    acct = CloudAccount.find(params[:id])
-    provider = acct.provider
-    require_privilege(Privilege::ACCOUNT_MODIFY,provider)
-    if acct.destroyable?
-      CloudAccount.destroy(params[:id])
+    account = CloudAccount.find(params[:id])
+    provider = account.provider
+    require_privilege(Privilege::ACCOUNT_MODIFY, provider)
+    if account.destroy
       flash[:notice] = "Cloud Account destroyed"
     else
-      flash[:notice] = "Cloud Account could not be destroyed"
+      flash[:error] = "Cloud Account could not be destroyed"
     end
-    redirect_to :controller => 'providers', :action => 'accounts', :id => provider.id
+    redirect_to provider_accounts_path(provider)
   end

def test_account(account)
     if account.valid_credentials?
-      flash[:notice] = "Test Connection Success: Valid Account Details"
+      flash.now[:notice] = "Test Connection Success: Valid Account Details"
     else
-      flash[:notice] = "Test Connection Failed: Invalid Account Details"
+      flash.now[:error] = "Test Connection Failed: Invalid Account Details"
     end
   rescue
-    flash[:notice] = "Test Connection Failed: Could not connect to provider"
+    flash.now[:error] = "Test Connection Failed: Could not connect to provider"
   end
+
   private

def quota_from_string(quota_raw)
@@ -168,4 +172,8 @@ class CloudAccountsController < ApplicationController
       return Integer(quota_raw)
     end
   end
+
+  def load_providers
+    @providers = Provider.list_for_user(@current_user, Privilege::PROVIDER_VIEW)
+  end
 end
diff --git a/src/app/controllers/providers_controller.rb b/src/app/controllers/providers_controller.rb
index 580552e..0754d65 100644
--- a/src/app/controllers/providers_controller.rb
+++ b/src/app/controllers/providers_controller.rb
@@ -120,15 +120,6 @@ class ProvidersController < ApplicationController
     require_privilege(Privilege::PROVIDER_VIEW, @provider)
   end

-  def accounts
-    @provider = Provider.find(:first, :conditions => {:id => params[:id]})
-    require_privilege(Privilege::ACCOUNT_VIEW, @provider)
-    if params[:cloud_account]
-      @cloud_account = CloudAccount.new(params[:cloud_account])
-      @quota = Quota.new(params[:quota])
-    end
-  end
-
   def realms
     @provider = Provider.find(params[:id])
     @realm_names = @provider.realms.collect { |r| r.name }
diff --git a/src/app/models/cloud_account.rb b/src/app/models/cloud_account.rb
index 5673e9a..e3ef0db 100644
--- a/src/app/models/cloud_account.rb
+++ b/src/app/models/cloud_account.rb
@@ -23,34 +23,62 @@ require 'nokogiri'

class CloudAccount < ActiveRecord::Base
   include PermissionedObject
+
+  # Relations
   belongs_to :provider
   belongs_to :quota
   has_many :instances
   has_and_belongs_to_many :zones
+  has_many :permissions, :as => :permission_object, :dependent => :destroy,
+           :include => [:role],
+           :order => "permissions.id ASC"

-  # what form does the account quota take?
+  has_one :instance_key, :dependent => :destroy

-  validates_presence_of :provider_id
+  # Helpers
+  attr_accessor :x509_cert_priv_file, :x509_cert_pub_file

+  # Validations
+  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
+  validate :validate_presence_of_x509_certs
+  validate :validate_credentials
+
+  # We're using this instead of <tt>validates_presence_of</tt> helper because
+  # we want to show errors on different attributes ending with '_file'.
+  def validate_presence_of_x509_certs
+    errors.add(:x509_cert_pub_file, "can't be blank") if x509_cert_pub.blank?
+    errors.add(:x509_cert_priv_file, "can't be blank") if x509_cert_priv.blank?
+  end

-  has_many :permissions, :as => :permission_object, :dependent => :destroy,
-           :include => [:role],
-           :order => "permissions.id ASC"
+  def validate_credentials
+    unless valid_credentials?
+      errors.add(:base, "Login Credentials are Invalid for this Provider")
+    end
+  end

-  has_one :instance_key, :dependent => :destroy
+  # Hooks
   after_create :generate_cloud_account_key
-
-  before_destroy {|entry| entry.destroyable? }
+  before_destroy :destroyable?
+  before_validation :read_x509_files

def destroyable?
-    self.instances.empty?
+    instances.empty?
+  end
+
+  def read_x509_files
+    if x509_cert_pub_file.respond_to?(:read)
+      x509_cert_pub_file.rewind # Sometimes the file has to rewind, becuase something already read from it.
+      self.x509_cert_pub = x509_cert_pub_file.read
+    end
+    if x509_cert_priv_file.respond_to?(:read)
+      x509_cert_priv_file.rewind
+      self.x509_cert_priv = x509_cert_priv_file.read
+    end
   end

def connect
@@ -76,7 +104,7 @@ class CloudAccount < ActiveRecord::Base
   end

def name
-    label.nil? || label == "" ? username : label
+    label.blank? ? username : label
   end

# FIXME: for already-mapped accounts, update rather than add new
@@ -142,11 +170,6 @@ EOT
     return xml.to_s
   end

-  protected
-  def validate
-    errors.add_to_base("Login Credentials are Invalid for this Provider") unless valid_credentials?
-  end
-
   private
   def generate_cloud_account_key
     client = connect
@@ -156,5 +179,4 @@ EOT
     end
   end

-
 end
diff --git a/src/app/views/cloud_accounts/_form.haml b/src/app/views/cloud_accounts/_form.haml
index 32c12a4..192ce05 100644
--- a/src/app/views/cloud_accounts/_form.haml
+++ b/src/app/views/cloud_accounts/_form.haml
@@ -1,37 +1,52 @@
-%fieldset
-  %legend Account
-  = hidden_field :cloud_account,  :id
-  = hidden_field :cloud_account, :provider_id, :value => @provider.id
-  %ul
-    %li
-      %label
-        Label
-        %span user-visible name for the account
-      = text_field :cloud_account, :label
-    - if @cloud_account.id.nil?
-      %li
-        %label
-          UserName
-          %span UserName for the provider account you wish to add.
-        = text_field :cloud_account, :username
-    %li
-      %label
-        Password
-        %span Password for the provider account you wish to add.
-      = password_field :cloud_account, :password
-    %li
-      %label
-        Account number
-        %span EC2 account number.
-      = text_field :cloud_account, :account_number
-    %li
-      %label
-        Account certificate
-        %span EC2 x509 private key
-      = text_area :cloud_account, :x509_cert_priv
-    %li
-      %label
-        Account certificate
-        %span EC2 x509 private key
-      = text_area :cloud_account, :x509_cert_pub
-= submit_tag "Save", :class => "submit"
+= error_messages_for 'cloud_account'
+%fieldset.clearfix.nomargin
+  %label.grid_4.la.alpha
+    = t('.account_name')
+    %span.required *
+  %label.grid_3.la
+    = t('.user_name')
+    %span.required *
+  %label.grid_3.la
+    = t('.password')
+    %span.required *
+  %label.grid_3.la.omega
+    = t('.quota_instances')
+    %span.required *
+%fieldset.nomargin.clearfix
+  = f.text_field :label, :title => t('.account_name'), :class => "grid_4 alpha"
+  = f.text_field :username, :title => t('.user_name'), :class => "grid_3"
+  = f.password_field :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
+    (
+    %button.linkbutton.nospace{ :type => 'button', :onclick => "set_unlimited_quota("quota_instances");" }<>
+      = t('.unlimited_quota')
+    )
+%fieldset.clearfix.nomargin
+  %label.grid_4.la.alpha
+    = t('.account_number')
+    %span.required *
+  %label.grid_3.la
+    = t('.account_private_cert')
+    %span.required *
+  %label.grid_3.la
+    = t('.account_public_cert')
+    %span.required *
+  .grid_3.omega
+%fieldset.clearfix.nomargin
+  = f.text_field :account_number, :title => t('.account_number'), :class => "grid_4 alpha"
+  .grid_3
+    = f.file_field :x509_cert_priv_file, :title => t('.account_private_cert')
+  .grid_3
+    = f.file_field :x509_cert_pub_file, :title => t('.account_public_cert')
+  .grid_3.omega
+    (
+    %button.linkbutton.nospace{ :type => 'submit', :value => t('.test_account'), :name => 'test_account', :id => 'test_account' }<>
+      = t('.test_account')
+    )
+
+:javascript
+  function set_unlimited_quota(elem_id) {
+    $("#" + elem_id)[0].value = "unlimited";
+  }
diff --git a/src/app/views/cloud_accounts/edit.haml b/src/app/views/cloud_accounts/edit.haml
index 9e16553..c97faac 100644
--- a/src/app/views/cloud_accounts/edit.haml
+++ b/src/app/views/cloud_accounts/edit.haml
@@ -1,6 +1,17 @@
-.dcloud_form
-  = error_messages_for 'cloud_account'
-  %h2 Edit Cloud Account
-  %br/
-  - form_for @cloud_account, :url => { :action => 'update' } do |form|
-    = render :partial => 'form'
+= render :partial => 'providers/providers'
+#details.grid_13
+  %nav.subsubnav
+    = render_navigation(:level => 4)
+
+  %h2
+    = t('.edit_provider_account')
+  - form_for @cloud_account, :url => provider_account_path(@provider, @cloud_account), :html => { :method => :put, :multipart => true } do |f|
+    = render :partial => 'form', :locals => { :f => f }
+    %fieldset.clearfix
+      .grid_13.alpha.omega
+        = submit_tag t(:edit), :class => "ra nomargin dialogbutton"
+    %section
+      %p.requirement
+        %span.required *
+        -
+        = t('.required_field')
diff --git a/src/app/views/cloud_accounts/index.haml b/src/app/views/cloud_accounts/index.haml
new file mode 100644
index 0000000..ab2d475
--- /dev/null
+++ b/src/app/views/cloud_accounts/index.haml
@@ -0,0 +1,25 @@
+= render :partial => 'providers/providers'
+#details.grid_13
+  %nav.subsubnav
+    = render_navigation(:level => 4)
+  %h2
+    = t('.provider_accounts')
+  - unless @provider.cloud_accounts.empty?
+    %table
+      %thead
+        %tr
+          %th{:scope => "col"} Label
+          %th{:scope => "col"} Username
+          %th{:scope => "col"} Account Number
+          %th{:scope => "col", :colspan => "2"} Actions
+      %tbody
+        - @provider.cloud_accounts.each do |cloud_account|
+          %tr
+            %td= cloud_account.label
+            %td= cloud_account.username
+            %td= cloud_account.account_number
+            %td= link_to 'Edit', edit_provider_account_path(@provider, cloud_account)
+            %td= link_to 'Delete', destroy_providers_account_path(@provider, cloud_account), :confirm => 'Are you sure?'
+
+  - if @provider.cloud_accounts.empty?
+    = link_to 'Add', new_provider_account_path(@provider.id), :class => 'button'
diff --git a/src/app/views/cloud_accounts/new.haml b/src/app/views/cloud_accounts/new.haml
index 446fd36..96d08df 100644
--- a/src/app/views/cloud_accounts/new.haml
+++ b/src/app/views/cloud_accounts/new.haml
@@ -1,8 +1,17 @@
-.dcloud_form
-  = error_messages_for 'account'
-  %h2 Add a new Account from this Provider
-  %br/
-  - form_for @cloud_account, :url => { :action => "create" } do |f|
-    = f.error_messages
-    = render :partial => "form", :object => f
-    = link_to "Cancel", root_path, :class => 'actionlink'
+= render :partial => 'providers/providers'
+#details.grid_13
+  %nav.subsubnav
+    = render_navigation(:level => 4)
+
+  %h2
+    = t('.new_provider_account')
+  - form_for @cloud_account, :url => provider_accounts_path(@provider), :html => { :multipart => true } do |f|
+    = render :partial => 'form', :locals => { :f => f }
+    %fieldset.clearfix
+      .grid_13.alpha.omega
+        = submit_tag t(:add), :class => "ra nomargin dialogbutton"
+    %section
+      %p.requirement
+        %span.required *
+        -
+        = t('.required_field')
diff --git a/src/app/views/layouts/_notification.rhtml b/src/app/views/layouts/_notification.rhtml
index fdbf642..6c204fd 100644
--- a/src/app/views/layouts/_notification.rhtml
+++ b/src/app/views/layouts/_notification.rhtml
@@ -22,6 +22,9 @@
       </div>
     <% end %>
   <% end %>
+  <% if flash[:error] && flash[:error].kind_of?(String) %>
+    <div class="error"><ul><li><%= flash[:error] %></li></ul></div>
+  <% end %>
   <% if flash[:warning] %>
     <div class="warning"><ul><li><%= flash[:warning] %></li></ul></div>
   <% end %>
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 6539cbd..1be36b6 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -97,16 +97,23 @@ en:
       test_connection: Test Connection
       required_field: Required field.
       caution_image: Caution
-    accounts:
+  cloud_accounts:
+    index:
       provider_accounts: Provider Accounts
+    new:
       new_provider_account: New Account
+      required_field: Required field.
+    edit:
+      edit_provider_account: Edit Cloud Account
+      required_field: Required field.
+    form:
+      provider_accounts: Provider Accounts
       account_name: Account Name
       user_name: EC2 Access Key
       password: Secret Access Key
       quota_instances: Quota Instances
       test_account: Test Account
       unlimited_quota: Unlimited Quota
-      required_field: Required field.
       account_number: AWS Account ID
       account_private_cert: EC2 x509 private key
       account_public_cert: EC2 x509 public key
diff --git a/src/config/navigation.rb b/src/config/navigation.rb
index 3b63df1..95d5e05 100644
--- a/src/config/navigation.rb
+++ b/src/config/navigation.rb
@@ -8,8 +8,8 @@ SimpleNavigation::Configuration.run do |navigation|
     first_level.item :administration, t(:administration), '#', :class => 'administration' do |second_level|
       second_level.item :system_settings, t(:system_settings), :controller => 'settings' do |third_level|
         third_level.item :manage_providers, t(:manage_providers), providers_path do |fourth_level|
-          fourth_level.item :provider_summary, t(:provider_summary), { :controller => 'providers', :action => 'show', :id => (@provider.id if @provider) }, :highlights_on => //providers/[0-9]+/
-          fourth_level.item :provider_accounts, t(:provider_accounts), { :controller => 'providers', :action => 'accounts', :id => (@provider.id if @provider) }, :highlights_on => //providers/accounts/
+          fourth_level.item :provider_summary, t(:provider_summary), { :controller => 'providers', :action => 'show', :id => (@provider.id if @provider) }, :highlights_on => //providers/\d+(/edit)?$/
+          fourth_level.item :provider_accounts, t(:provider_accounts), { :controller => 'cloud_accounts', :action => 'index', :provider_id => (@provider.id if @provider) }, :highlights_on => //providers/\d+/accounts(/(\d+|new))?/
           fourth_level.item :scheduling_policies, t(:scheduling_policies), '#'
           fourth_level.item :services_provided, t(:services_provided), '#'
           fourth_level.item :map_profiles, t(:map_profiles), '#'
diff --git a/src/config/routes.rb b/src/config/routes.rb
index f59a2aa..931c97f 100644
--- a/src/config/routes.rb
+++ b/src/config/routes.rb
@@ -52,7 +52,10 @@ ActionController::Routing::Routes.draw do |map|

# Temporarily disable this route, provider stuff is not restful yet.
   # Will be re-enabled in upcoming patch
-  map.resources :providers
+  map.resources :providers do |provider|
+    provider.resources :accounts, :controller => 'cloud_accounts'
+  end
+  map.destroy_providers_account '/providers/:provider_id/accounts/:id/destroy', :controller => 'cloud_accounts', :action => 'destroy', :conditions => { :method => :get }

# Allow downloading Web Service WSDL as a file with an extension
   # instead of a file named 'wsdl'
diff --git a/src/features/provider.feature b/src/features/provider.feature
index 12047bb..8cf2ccd 100644
--- a/src/features/provider.feature
+++ b/src/features/provider.feature
@@ -65,6 +65,7 @@ Feature: Manage Providers
     When I go to the providers page
     And I follow "provider1"
     And I follow "Provider Accounts"
+    And I follow "Add" within "#details"
     And I fill in "cloud_account[label]" with "MockAccount"
     And I fill in "cloud_account[username]" with "mockuser"
     And I fill in "cloud_account[password]" with "mockpassword"
@@ -80,6 +81,7 @@ Feature: Manage Providers
     When I go to the providers page
     And I follow "provider1"
     And I follow "Provider Accounts"
+    And I follow "Add" within "#details"
     And I fill in "cloud_account[label]" with "IncorrectAccount"
     And I fill in "cloud_account[username]" with "incorrect_user"
     And I fill in "cloud_account[password]" with "incorrect_password"
@@ -102,4 +104,4 @@ Feature: Manage Providers
     And there should not be any replicated images
     And there should not be any hardware profiles
     And there should not be a cloud account
-    And there should not be a realm
\ No newline at end of file
+    And there should not be a realm
diff --git a/src/features/step_definitions/web_steps.rb b/src/features/step_definitions/web_steps.rb
index f270900..e6d6808 100644
--- a/src/features/step_definitions/web_steps.rb
+++ b/src/features/step_definitions/web_steps.rb
@@ -24,6 +24,12 @@ When /^(?:|I )press "([^"]*)"$/ do |button|
   click_button(button)
 end

+When /^I press "([^"]*)" within "([^"]*)"$/ do |button,scope_selector|
+  within(scope_selector) do
+    click_button(button)
+  end
+end
+
 When /^(?:|I )follow "([^"]*)"$/ do |link|
   click_link(link)
 end
diff --git a/src/spec/controllers/cloud_accounts_controller_spec.rb b/src/spec/controllers/cloud_accounts_controller_spec.rb
index f3b866a..072126e 100644
--- a/src/spec/controllers/cloud_accounts_controller_spec.rb
+++ b/src/spec/controllers/cloud_accounts_controller_spec.rb
@@ -14,6 +14,29 @@ describe CloudAccountsController do
     activate_authlogic
   end

+  it "shows provider accounts as list" do
+    UserSession.create(@admin)
+    get :index, :provider_id => @provider.id
+    response.should be_success
+    response.should render_template("index")
+  end
+
+  it "allows test account validity on create when passing test_account param" do
+    UserSession.create(@admin)
+    post :create, :provider_id => @provider.id, :cloud_account => {}, :test_account => true
+    response.should be_success
+    response.should render_template("new")
+    response.flash[:error].should == "Test Connection Failed: Invalid Account Details"
+  end
+
+  it "doesn't allow to save provider's account if not valid credentials" do
+    UserSession.create(@admin)
+    post :create, :provider_id => @provider.id, :cloud_account => {}
+    response.should be_success
+    response.should render_template("new")
+    response.flash[:error].should == "The entered credential information is incorrect"
+  end
+
   it "should permit users with account modify permission to access edit cloud account interface" do
     UserSession.create(@admin)
     get :edit, :id => @cloud_account.id
@@ -25,21 +48,21 @@ describe CloudAccountsController do
     UserSession.create(@admin)

@cloud_account.password = "foobar"
-    @cloud_account.stub!(:valid_credentials).and_return(true)
-    @cloud_account.save
+    @cloud_account.stub!(:valid_credentials?).and_return(true)
+    @cloud_account.save.should be_true

-    post :update, :cloud_account => { :id => @cloud_account.id, :password => 'mockpassword' }
-    response.should redirect_to("http://test.host/providers/accounts/#[e-mail]%7D")
+    post :update, :id => @cloud_account.id, :cloud_account => { :password => 'mockpassword' }
+    response.should redirect_to provider_accounts_path(@provider)
     CloudAccount.find(@cloud_account.id).password.should == "mockpassword"
   end

it "should allow users with account modify permission to delete a cloud account" do
     UserSession.create(@admin)
     lambda do
-      post :destroy, :id => @cloud_account.id
+      get :destroy, :id => @cloud_account.id
     end.should change(CloudAccount, :count).by(-1)
-    response.should redirect_to("http://test.host/providers/accounts/#[e-mail]%7D")
-    CloudAccount.find(:first, :conditions => ['id = ?', @cloud_account.id]).should be_nil
+    response.should redirect_to provider_accounts_path(@provider)
+    CloudAccount.find_by_id(@cloud_account.id).should be_nil
   end

it "should deny access to users without account modify permission" do
diff --git a/src/spec/controllers/provider_controller_spec.rb b/src/spec/controllers/provider_controller_spec.rb
index a1a317a..49a45a8 100644
--- a/src/spec/controllers/provider_controller_spec.rb
+++ b/src/spec/controllers/provider_controller_spec.rb
@@ -10,18 +10,6 @@ describe ProvidersController do
     activate_authlogic
   end

-  it "should provide ui to view accounts" do
-     UserSession.create(@admin)
-     get :accounts, :id => @provider.id
-     response.should be_success
-     response.should render_template("accounts")
-  end
-
-  it "should fail to grant access to account UIs for unauthenticated user" do
-     get :accounts
-     response.should_not be_success
-  end
-
   it "should provide ui to view hardware profiles" do
      UserSession.create(@admin)
      provider = @admin_permission.permission_object
diff --git a/src/spec/models/cloud_account_spec.rb b/src/spec/models/cloud_account_spec.rb
index 4466977..fb04e40 100644
--- a/src/spec/models/cloud_account_spec.rb
+++ b/src/spec/models/cloud_account_spec.rb
@@ -8,15 +8,13 @@ describe CloudAccount do

it "should not be destroyable if it has instances" do
     @cloud_account.instances << Instance.new
-    @cloud_account.destroyable?.should_not be_true
-    @cloud_account.destroy
-    CloudAccount.find(@cloud_account.id).should_not be_nil
-
+    @cloud_account.destroyable?.should be_false
+    @cloud_account.destroy.should be_false

@cloud_account.instances.clear
     @cloud_account.destroyable?.should be_true
-    @cloud_account.destroy
-    CloudAccount.find(:first, :conditions => ['id = ?', @cloud_account.id]).should be_nil
+    @cloud_account.destroy.equal?(@cloud_account).should be_true
+    @cloud_account.should be_frozen
   end

it "should check the validitiy of the cloud account login credentials" do
@@ -51,6 +49,11 @@ describe CloudAccount do
     cloud_account.instance_key.id == "1_user"
   end

+  it "when calling connect and it fails with exception it will return nil" do
+    DeltaCloud.should_receive(:new).and_raise(Exception.new)
+
+    @cloud_account.connect.should be_nil
+  end

it "should generate credentials xml" do
     expected_xml = <<EOT

[Tomas Sedovic]

I'm really sorry, I noticed that the previously set Quota value isn't displayed in the edit Provider Account page. I should have noticed it sooner when I reviewed the previous version of this patch.

Here's how to repro it:
1. on the Provider Accounts page fill in the data
2. set Quota to 10
3. Click Add
4. Click Edit

Expected:
The Quota field will contain the value 10.

Actual:
The Quota field contains the string "unlimited".

----- [e-mail] wrote:
> From: Ladislav Martincik
>
> src/app/controllers/cloud_accounts_controller.rb   |   88
> +++++++++++---------
>  src/app/controllers/providers_controller.rb        |    9 --
>  src/app/models/cloud_account.rb                    |   58
> +++++++++----
>  src/app/views/cloud_accounts/_form.haml            |   89
> ++++++++++++--------
>  src/app/views/cloud_accounts/edit.haml             |   23 ++++--
>  src/app/views/cloud_accounts/index.haml            |   25 ++++++
>  src/app/views/cloud_accounts/new.haml              |   25 ++++--
>  src/app/views/layouts/_notification.rhtml          |    3 +
>  src/config/locales/en.yml                          |   11 ++-
>  src/config/navigation.rb                           |    4 +-
>  src/config/routes.rb                               |    5 +-
>  src/features/provider.feature                      |    4 +-
>  src/features/step_definitions/web_steps.rb         |    6 ++
>  .../controllers/cloud_accounts_controller_spec.rb  |   37 +++++++--
>  src/spec/controllers/provider_controller_spec.rb   |   12 ---
>  src/spec/models/cloud_account_spec.rb              |   15 ++--
>  16 files changed, 265 insertions(+), 149 deletions(-)
>  create mode 100644 src/app/views/cloud_accounts/index.haml
>
> diff --git a/src/app/controllers/cloud_accounts_controller.rb
> b/src/app/controllers/cloud_accounts_controller.rb
> index e1c9b23..6a2a072 100644
> --- a/src/app/controllers/cloud_accounts_controller.rb
> +++ b/src/app/controllers/cloud_accounts_controller.rb
> @@ -21,6 +21,14 @@
>
> class CloudAccountsController < ApplicationController
>    before_filter :require_user
>
> before_filter :load_providers
>
> helper :providers
>
> def index
>
> @provider = Provider.find(params[:provider_id])
>
> require_privilege(Privilege::ACCOUNT_VIEW, @provider)
>
> end
>
> def new
>   @provider = Provider.find(params[:provider_id])
>
> @@ -29,43 +37,38 @@ class CloudAccountsController <
> ApplicationController
>    end
>
> def create
> @provider = Provider.find(params[:cloud_account][:provider_id])
> @provider = Provider.find(params[:provider_id])
> require_privilege(Privilege::ACCOUNT_MODIFY,@provider)
> if params[:cloud_account] &&
>
> !params[:cloud_account][:x509_cert_priv_file].blank?
>  params[:cloud_account][:x509_cert_priv] =
>
> params[:cloud_account][:x509_cert_priv_file].read
> end
> params[:cloud_account].delete :x509_cert_priv_file
> if params[:cloud_account] &&
>
> !params[:cloud_account][:x509_cert_pub_file].blank?
>  params[:cloud_account][:x509_cert_pub] =
>
> params[:cloud_account][:x509_cert_pub_file].read
> end
> params[:cloud_account].delete :x509_cert_pub_file
> @cloud_account = CloudAccount.new(params[:cloud_account])
>
> @cloud_account.provider = @provider
>
> if params[:test_account]
>   test_account(@cloud_account)
>  redirect_to :controller => "providers", :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 => "providers", :action => "accounts",
>
> :id => @provider
>  kick_condor
> else
>  render :action => 'new' and return
> end
> if @cloud_account.invalid?
>   if not @cloud_account.valid_credentials?
>    flash[:notice] = "The entered credential information is
>
> incorrect"
>    flash.now[:error] = "The entered credential information is
>
> incorrect"
>        elsif @cloud_account.errors.on(:username)
>    flash[:notice] = "The access key
>
> '#{params[:cloud_account][:username]}' has already been taken."
>    flash.now[:error] = "The access key
>
> '#{params[:cloud_account][:username]}' has already been taken."
>        else
>    flash[:notice] = "You must fill in all the required fields"
>    flash.now[:error] = "You must fill in all the required
>
> fields"
>        end
>  redirect_to :controller => "providers", :action => "accounts",
>
> :id => @provider, :cloud_account => params[:cloud_account]
>  render :action => 'new' and return
> end
> quota = Quota.new
> quota.maximum_running_instances =
>
> quota_from_string(params[:quota][:maximum_running_instances])
>
> quota.save!
>
> @cloud_account.quota = quota
>
> @cloud_account.zones << Zone.default
>
> @cloud_account.save!
>
> if @cloud_account.populate_realms
>  flash[:notice] = "Provider account added."
>
> end
>
> redirect_to provider_accounts_path(@provider)
>
> kick_condor
> end
>
> def edit
>
> @@ -119,11 +122,12 @@ class CloudAccountsController <
> ApplicationController
>    end
>
> def update
> @cloud_account = CloudAccount.find(params[:cloud_account][:id])
>
> require_privilege(Privilege::ACCOUNT_MODIFY,@cloud_account.provider)
> @cloud_account = CloudAccount.find(params[:id])
> @provider = @cloud_account.provider
> require_privilege(Privilege::ACCOUNT_MODIFY, @provider)
> if @cloud_account.update_attributes(params[:cloud_account])
>   flash[:notice] = "Cloud Account updated!"
>  redirect_to :controller => 'providers', :action => 'accounts',
>
> :id => @cloud_account.provider.id
>  redirect_to provider_accounts_path(@provider)
> else
>      render :action => :edit
>    end
>
> @@ -138,27 +142,27 @@ class CloudAccountsController <
> ApplicationController
>    end
>
> def destroy
> acct = CloudAccount.find(params[:id])
> provider = acct.provider
> require_privilege(Privilege::ACCOUNT_MODIFY,provider)
> if acct.destroyable?
>  CloudAccount.destroy(params[:id])
> account = CloudAccount.find(params[:id])
> provider = account.provider
> require_privilege(Privilege::ACCOUNT_MODIFY, provider)
> if account.destroy
>   flash[:notice] = "Cloud Account destroyed"
> else
>  flash[:notice] = "Cloud Account could not be destroyed"
>  flash[:error] = "Cloud Account could not be destroyed"
> end
> redirect_to :controller => 'providers', :action => 'accounts',
>
> :id => provider.id
>
> redirect_to provider_accounts_path(provider)
> end
>
> def test_account(account)
> if account.valid_credentials?
>  flash[:notice] = "Test Connection Success: Valid Account
>
> Details"
>  flash.now[:notice] = "Test Connection Success: Valid Account
>
> Details"
>      else
>  flash[:notice] = "Test Connection Failed: Invalid Account
>
> Details"
>  flash.now[:error] = "Test Connection Failed: Invalid Account
>
> Details"
>      end
>    rescue
> flash[:notice] = "Test Connection Failed: Could not connect to
>
> provider"
> flash.now[:error] = "Test Connection Failed: Could not connect to
>
> provider"
>    end
>
> private
>
> def quota_from_string(quota_raw)
>
> @@ -168,4 +172,8 @@ class CloudAccountsController <
> ApplicationController
>        return Integer(quota_raw)
>      end
>    end
> def load_providers
> @providers = Provider.list_for_user(@current_user,
>
> Privilege::PROVIDER_VIEW)
> end
>
> end
> diff --git a/src/app/controllers/providers_controller.rb
> b/src/app/controllers/providers_controller.rb
> index 580552e..0754d65 100644
> --- a/src/app/controllers/providers_controller.rb
> +++ b/src/app/controllers/providers_controller.rb
> @@ -120,15 +120,6 @@ class ProvidersController <
> ApplicationController
>      require_privilege(Privilege::PROVIDER_VIEW, @provider)
>    end
> def accounts
> @provider = Provider.find(:first, :conditions => {:id =>
>
> params[:id]})
> require_privilege(Privilege::ACCOUNT_VIEW, @provider)
> if params[:cloud_account]
>  @cloud_account = CloudAccount.new(params[:cloud_account])
>  @quota = Quota.new(params[:quota])
> end
> end
> def realms
>    @provider = Provider.find(params[:id])
>    @realm_names = @provider.realms.collect { |r| r.name }
>
> diff --git a/src/app/models/cloud_account.rb
> b/src/app/models/cloud_account.rb
> index 5673e9a..e3ef0db 100644
> --- a/src/app/models/cloud_account.rb
> +++ b/src/app/models/cloud_account.rb
> @@ -23,34 +23,62 @@ require 'nokogiri'
>
> class CloudAccount < ActiveRecord::Base
>    include PermissionedObject
> # Relations
> belongs_to :provider
> belongs_to :quota
> has_many :instances
> has_and_belongs_to_many :zones
> has_many :permissions, :as => :permission_object, :dependent =>
>
> :destroy,
>       :include => [:role],
>       :order => "permissions.id ASC"
>
> # what form does the account quota take?
> has_one :instance_key, :dependent => :destroy
> validates_presence_of :provider_id
>
> # Helpers
>
> attr_accessor :x509_cert_priv_file, :x509_cert_pub_file
>
> # Validations
>
> 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
> validate :validate_presence_of_x509_certs
> validate :validate_credentials
> # We're using this instead of <tt>validates_presence_of</tt> helper
>
> because
> # we want to show errors on different attributes ending with
>
> '_file'.
> def validate_presence_of_x509_certs
> errors.add(:x509_cert_pub_file, "can't be blank") if
>
> x509_cert_pub.blank?
> errors.add(:x509_cert_priv_file, "can't be blank") if
>
> x509_cert_priv.blank?
> end
> has_many :permissions, :as => :permission_object, :dependent =>
>
> :destroy,
>       :include => [:role],
>       :order => "permissions.id ASC"
> def validate_credentials
> unless valid_credentials?
>  errors.add(:base, "Login Credentials are Invalid for this
>
> Provider")
> end
> end
> has_one :instance_key, :dependent => :destroy
> # Hooks
> after_create :generate_cloud_account_key
> before_destroy {|entry| entry.destroyable? }
>
> before_destroy :destroyable?
>
> before_validation :read_x509_files
>
> def destroyable?
> self.instances.empty?
> instances.empty?
> end
> def read_x509_files
> if x509_cert_pub_file.respond_to?(:read)
>  x509_cert_pub_file.rewind # Sometimes the file has to rewind,
>
> becuase something already read from it.
>  self.x509_cert_pub = x509_cert_pub_file.read
>
> end
>
> if x509_cert_priv_file.respond_to?(:read)
>  x509_cert_priv_file.rewind
>  self.x509_cert_priv = x509_cert_priv_file.read
>
> end
> end
>
> def connect
>
> @@ -76,7 +104,7 @@ class CloudAccount < ActiveRecord::Base
>    end
>
> def name
> label.nil? || label == "" ? username : label
>
> label.blank? ? username : label
> end
>
> # FIXME: for already-mapped accounts, update rather than add new
>
> @@ -142,11 +170,6 @@ EOT
>      return xml.to_s
>    end
> protected
> def validate
> errors.add_to_base("Login Credentials are Invalid for this
>
> Provider") unless valid_credentials?
> end
> private
>  def generate_cloud_account_key
>    client = connect
>
> @@ -156,5 +179,4 @@ EOT
>      end
>    end
>
> end
> diff --git a/src/app/views/cloud_accounts/_form.haml
> b/src/app/views/cloud_accounts/_form.haml
> index 32c12a4..192ce05 100644
> --- a/src/app/views/cloud_accounts/_form.haml
> +++ b/src/app/views/cloud_accounts/_form.haml
> @@ -1,37 +1,52 @@
> -%fieldset
> %legend Account
> = hidden_field :cloud_account,  :id
> = hidden_field :cloud_account, :provider_id, :value =>
>
> @provider.id
> %ul
> %li
>  %label
>    Label
>    %span user-visible name for the account
>  = text_field :cloud_account, :label
> if @cloud_account.id.nil?
>  %li
>    %label
>      UserName
>      %span UserName for the provider account you wish to add.
>    = text_field :cloud_account, :username
> %li
>  %label
>    Password
>    %span Password for the provider account you wish to add.
>  = password_field :cloud_account, :password
> %li
>  %label
>    Account number
>    %span EC2 account number.
>  = text_field :cloud_account, :account_number
> %li
>  %label
>    Account certificate
>    %span EC2 x509 private key
>  = text_area :cloud_account, :x509_cert_priv
> %li
>  %label
>    Account certificate
>    %span EC2 x509 private key
>  = text_area :cloud_account, :x509_cert_pub
>
> -= submit_tag "Save", :class => "submit"
> += error_messages_for 'cloud_account'
> +%fieldset.clearfix.nomargin
> %label.grid_4.la.alpha
> = t('.account_name')
> %span.required *
> %label.grid_3.la
> = t('.user_name')
> %span.required *
> %label.grid_3.la
> = t('.password')
> %span.required *
> %label.grid_3.la.omega
> = t('.quota_instances')
> %span.required *
>
> +%fieldset.nomargin.clearfix
> = f.text_field :label, :title => t('.account_name'), :class =>
>
> "grid_4 alpha"
> = f.text_field :username, :title => t('.user_name'), :class =>
>
> "grid_3"
> = f.password_field :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
> (
> %button.linkbutton.nospace{ :type => 'button', :onclick =>
>
> "set_unlimited_quota("quota_instances");" }<>
>  = t('.unlimited_quota')
> )
>
> +%fieldset.clearfix.nomargin
> %label.grid_4.la.alpha
> = t('.account_number')
> %span.required *
> %label.grid_3.la
> = t('.account_private_cert')
> %span.required *
> %label.grid_3.la
> = t('.account_public_cert')
> %span.required *
> .grid_3.omega
>
> +%fieldset.clearfix.nomargin
> = f.text_field :account_number, :title => t('.account_number'),
>
> :class => "grid_4 alpha"
> .grid_3
> = f.file_field :x509_cert_priv_file, :title =>
>
> t('.account_private_cert')
> .grid_3
> = f.file_field :x509_cert_pub_file, :title =>
>
> t('.account_public_cert')
> .grid_3.omega
> (
> %button.linkbutton.nospace{ :type => 'submit', :value =>
>
> t('.test_account'), :name => 'test_account', :id => 'test_account'
> }<>
>  = t('.test_account')
> )
>
> +:javascript
> function set_unlimited_quota(elem_id) {
> $("#" + elem_id)[0].value = "unlimited";
> }
>
> diff --git a/src/app/views/cloud_accounts/edit.haml
> b/src/app/views/cloud_accounts/edit.haml
> index 9e16553..c97faac 100644
> --- a/src/app/views/cloud_accounts/edit.haml
> +++ b/src/app/views/cloud_accounts/edit.haml
> @@ -1,6 +1,17 @@
> -.dcloud_form
> = error_messages_for 'cloud_account'
> %h2 Edit Cloud Account
> %br/
> form_for @cloud_account, :url => { :action => 'update' } do
>
> |form|
> = render :partial => 'form'
>
> += render :partial => 'providers/providers'
> +#details.grid_13
> %nav.subsubnav
> = render_navigation(:level => 4)
> %h2
> = t('.edit_provider_account')
> form_for @cloud_account, :url => provider_account_path(@provider,
>
> @cloud_account), :html => { :method => :put, :multipart => true } do
> |f|
> = render :partial => 'form', :locals => { :f => f }
> %fieldset.clearfix
>  .grid_13.alpha.omega
>    = submit_tag t(:edit), :class => "ra nomargin dialogbutton"
> %section
>  %p.requirement
>    %span.required *
>    \-
>    = t('.required_field')
>
> diff --git a/src/app/views/cloud_accounts/index.haml
> b/src/app/views/cloud_accounts/index.haml
> new file mode 100644
> index 0000000..ab2d475
> --- /dev/null
> +++ b/src/app/views/cloud_accounts/index.haml
> @@ -0,0 +1,25 @@
> += render :partial => 'providers/providers'
> +#details.grid_13
> %nav.subsubnav
> = render_navigation(:level => 4)
> %h2
> = t('.provider_accounts')
> unless @provider.cloud_accounts.empty?
> %table
>  %thead
>    %tr
>      %th{:scope => "col"} Label
>      %th{:scope => "col"} Username
>      %th{:scope => "col"} Account Number
>      %th{:scope => "col", :colspan => "2"} Actions
>  %tbody
>    - @provider.cloud_accounts.each do |cloud_account|
>      %tr
>        %td= cloud_account.label
>        %td= cloud_account.username
>        %td= cloud_account.account_number
>        %td= link_to 'Edit',
>
> edit_provider_account_path(@provider, cloud_account)
>        %td= link_to 'Delete',
>
> destroy_providers_account_path(@provider, cloud_account), :confirm =>
> 'Are you sure?'
> if @provider.cloud_accounts.empty?
> = link_to 'Add', new_provider_account_path(@provider.id), :class
>
> => 'button'
> diff --git a/src/app/views/cloud_accounts/new.haml
> b/src/app/views/cloud_accounts/new.haml
> index 446fd36..96d08df 100644
> --- a/src/app/views/cloud_accounts/new.haml
> +++ b/src/app/views/cloud_accounts/new.haml
> @@ -1,8 +1,17 @@
> -.dcloud_form
> = error_messages_for 'account'
> %h2 Add a new Account from this Provider
> %br/
> form_for @cloud_account, :url => { :action => "create" } do |f|
> = f.error_messages
> = render :partial => "form", :object => f
> = link_to "Cancel", root_path, :class => 'actionlink'
>
> += render :partial => 'providers/providers'
> +#details.grid_13
> %nav.subsubnav
> = render_navigation(:level => 4)
> %h2
> = t('.new_provider_account')
> form_for @cloud_account, :url =>
>
> provider_accounts_path(@provider), :html => { :multipart => true } do
> |f|
> = render :partial => 'form', :locals => { :f => f }
> %fieldset.clearfix
>  .grid_13.alpha.omega
>    = submit_tag t(:add), :class => "ra nomargin dialogbutton"
> %section
>  %p.requirement
>    %span.required *
>    \-
>    = t('.required_field')
>
> diff --git a/src/app/views/layouts/_notification.rhtml
> b/src/app/views/layouts/_notification.rhtml
> index fdbf642..6c204fd 100644
> --- a/src/app/views/layouts/_notification.rhtml
> +++ b/src/app/views/layouts/_notification.rhtml
> @@ -22,6 +22,9 @@
>        </div>
>      <% end %>
>    <% end %>
> <% if flash[:error] && flash[:error].kind_of?(String) %>
>
> <div class="error"><ul><li><%= flash[:error] %></li></ul></div>
> <% end %>
> <% if flash[:warning] %>
>
>   <div class="warning"><ul><li><%= flash[:warning]
>
> %></li></ul></div>
>    <% end %>
> diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> index 6539cbd..1be36b6 100644
> --- a/src/config/locales/en.yml
> +++ b/src/config/locales/en.yml
> @@ -97,16 +97,23 @@ en:
>        test_connection: Test Connection
>        required_field: Required field.
>        caution_image: Caution
> accounts:
> cloud_accounts:
> index:
>   provider_accounts: Provider Accounts
> new:
>   new_provider_account: New Account
>  required_field: Required field.
> edit:
>  edit_provider_account: Edit Cloud Account
>  required_field: Required field.
> form:
>  provider_accounts: Provider Accounts
>  account_name: Account Name
>  user_name: EC2 Access Key
>  password: Secret Access Key
>  quota_instances: Quota Instances
>  test_account: Test Account
>  unlimited_quota: Unlimited Quota
>  required_field: Required field.
>  account_number: AWS Account ID
>  account_private_cert: EC2 x509 private key
>  account_public_cert: EC2 x509 public key
>
> diff --git a/src/config/navigation.rb b/src/config/navigation.rb
> index 3b63df1..95d5e05 100644
> --- a/src/config/navigation.rb
> +++ b/src/config/navigation.rb
> @@ -8,8 +8,8 @@ SimpleNavigation::Configuration.run do |navigation|
>      first_level.item :administration, t(:administration), '#', :class
> => 'administration' do |second_level|
>        second_level.item :system_settings, t(:system_settings),
> :controller => 'settings' do |third_level|
>          third_level.item :manage_providers, t(:manage_providers),
> providers_path do |fourth_level|
>      fourth_level.item :provider_summary, t(:provider_summary),
>
> { :controller => 'providers', :action => 'show', :id => (@provider.id
> if @provider) }, :highlights_on => //providers/[0-9]+/
>      fourth_level.item :provider_accounts,
>
> t(:provider_accounts), { :controller => 'providers', :action =>
> 'accounts', :id => (@provider.id if @provider) }, :highlights_on =>
> //providers/accounts/
>      fourth_level.item :provider_summary, t(:provider_summary),
>
> { :controller => 'providers', :action => 'show', :id => (@provider.id
> if @provider) }, :highlights_on => //providers/\d+(/edit)?$/
>      fourth_level.item :provider_accounts,
>
> t(:provider_accounts), { :controller => 'cloud_accounts', :action =>
> 'index', :provider_id => (@provider.id if @provider) }, :highlights_on
> => //providers/\d+/accounts(/(\d+|new))?/
>            fourth_level.item :scheduling_policies,
> t(:scheduling_policies), '#'
>            fourth_level.item :services_provided,
> t(:services_provided), '#'
>            fourth_level.item :map_profiles, t(:map_profiles), '#'
> diff --git a/src/config/routes.rb b/src/config/routes.rb
> index f59a2aa..931c97f 100644
> --- a/src/config/routes.rb
> +++ b/src/config/routes.rb
> @@ -52,7 +52,10 @@ ActionController::Routing::Routes.draw do |map|
>
> # Temporarily disable this route, provider stuff is not restful
> yet.
>    # Will be re-enabled in upcoming patch
> map.resources :providers
> map.resources :providers do |provider|
> provider.resources :accounts, :controller => 'cloud_accounts'
> end
> map.destroy_providers_account
>
> '/providers/:provider_id/accounts/:id/destroy', :controller =>
> 'cloud_accounts', :action => 'destroy', :conditions => { :method =>
> :get }
>
> # Allow downloading Web Service WSDL as a file with an extension
>    # instead of a file named 'wsdl'
> diff --git a/src/features/provider.feature
> b/src/features/provider.feature
> index 12047bb..8cf2ccd 100644
> --- a/src/features/provider.feature
> +++ b/src/features/provider.feature
> @@ -65,6 +65,7 @@ Feature: Manage Providers
>      When I go to the providers page
>      And I follow "provider1"
>      And I follow "Provider Accounts"
> And I follow "Add" within "#details"
> And I fill in "cloud_account[label]" with "MockAccount"
> And I fill in "cloud_account[username]" with "mockuser"
> And I fill in "cloud_account[password]" with "mockpassword"
>
> @@ -80,6 +81,7 @@ Feature: Manage Providers
>      When I go to the providers page
>      And I follow "provider1"
>      And I follow "Provider Accounts"
> And I follow "Add" within "#details"
> And I fill in "cloud_account[label]" with "IncorrectAccount"
> And I fill in "cloud_account[username]" with "incorrect_user"
> And I fill in "cloud_account[password]" with
>
> "incorrect_password"
> @@ -102,4 +104,4 @@ Feature: Manage Providers
>      And there should not be any replicated images
>      And there should not be any hardware profiles
>      And there should not be a cloud account
> And there should not be a realm
>
> \ No newline at end of file
> And there should not be a realm
>
> diff --git a/src/features/step_definitions/web_steps.rb
> b/src/features/step_definitions/web_steps.rb
> index f270900..e6d6808 100644
> --- a/src/features/step_definitions/web_steps.rb
> +++ b/src/features/step_definitions/web_steps.rb
> @@ -24,6 +24,12 @@ When /^(?:|I )press "([^"]*)"$/ do |button|
>    click_button(button)
>  end
>
> +When /^I press "([^"]*)" within "([^"]*)"$/ do
> |button,scope_selector|
> within(scope_selector) do
> click_button(button)
> end
>
> +end
>
> When /^(?:|I )follow "([^"]*)"$/ do |link|
>    click_link(link)
>  end
> diff --git a/src/spec/controllers/cloud_accounts_controller_spec.rb
> b/src/spec/controllers/cloud_accounts_controller_spec.rb
> index f3b866a..072126e 100644
> --- a/src/spec/controllers/cloud_accounts_controller_spec.rb
> +++ b/src/spec/controllers/cloud_accounts_controller_spec.rb
> @@ -14,6 +14,29 @@ describe CloudAccountsController do
>      activate_authlogic
>    end
> it "shows provider accounts as list" do
> UserSession.create(@admin)
> get :index, :provider_id => @provider.id
> response.should be_success
> response.should render_template("index")
> end
> it "allows test account validity on create when passing
>
> test_account param" do
> UserSession.create(@admin)
> post :create, :provider_id => @provider.id, :cloud_account => {},
>
> :test_account => true
> response.should be_success
> response.should render_template("new")
> response.flash[:error].should == "Test Connection Failed: Invalid
>
> Account Details"
> end
> it "doesn't allow to save provider's account if not valid
>
> credentials" do
> UserSession.create(@admin)
> post :create, :provider_id => @provider.id, :cloud_account => {}
> response.should be_success
> response.should render_template("new")
> response.flash[:error].should == "The entered credential
>
> information is incorrect"
> end
> it "should permit users with account modify permission to access
>
> edit cloud account interface" do
>      UserSession.create(@admin)
>      get :edit, :id => @cloud_account.id
> @@ -25,21 +48,21 @@ describe CloudAccountsController do
>      UserSession.create(@admin)
>  @cloud_account.password = "foobar"
> @cloud_account.stub!(:valid_credentials).and_return(true)
> @cloud_account.save
> @cloud_account.stub!(:valid_credentials?).and_return(true)
> @cloud_account.save.should be_true
> post :update, :cloud_account => { :id => @cloud_account.id,
>
> :password => 'mockpassword' }
> response.should
>
> redirect_to("http://test.host/providers/accounts/#[e-mail]%7D")
> post :update, :id => @cloud_account.id, :cloud_account => {
>
> :password => 'mockpassword' }
> response.should redirect_to provider_accounts_path(@provider)
> CloudAccount.find(@cloud_account.id).password.should ==
>
> "mockpassword"
>    end
>
> it "should allow users with account modify permission to delete a
> cloud account" do
>      UserSession.create(@admin)
>      lambda do
>  post :destroy, :id => @cloud_account.id
>  get :destroy, :id => @cloud_account.id
> end.should change(CloudAccount, :count).by(-1)
> response.should
>
> redirect_to("http://test.host/providers/accounts/#[e-mail]%7D")
> CloudAccount.find(:first, :conditions => ['id = ?',
>
> @cloud_account.id]).should be_nil
>
> response.should redirect_to provider_accounts_path(@provider)
>
> CloudAccount.find_by_id(@cloud_account.id).should be_nil
> end
>
> it "should deny access to users without account modify permission"
>
> do
> diff --git a/src/spec/controllers/provider_controller_spec.rb
> b/src/spec/controllers/provider_controller_spec.rb
> index a1a317a..49a45a8 100644
> --- a/src/spec/controllers/provider_controller_spec.rb
> +++ b/src/spec/controllers/provider_controller_spec.rb
> @@ -10,18 +10,6 @@ describe ProvidersController do
>      activate_authlogic
>    end
> it "should provide ui to view accounts" do
> UserSession.create(@admin)
> get :accounts, :id => @provider.id
> response.should be_success
> response.should render_template("accounts")
> end
> it "should fail to grant access to account UIs for unauthenticated
>
> user" do
> get :accounts
> response.should_not be_success
> end
> it "should provide ui to view hardware profiles" do
>     UserSession.create(@admin)
>     provider = @admin_permission.permission_object
>
> diff --git a/src/spec/models/cloud_account_spec.rb
> b/src/spec/models/cloud_account_spec.rb
> index 4466977..fb04e40 100644
> --- a/src/spec/models/cloud_account_spec.rb
> +++ b/src/spec/models/cloud_account_spec.rb
> @@ -8,15 +8,13 @@ describe CloudAccount do
>
> it "should not be destroyable if it has instances" do
>      @cloud_account.instances << Instance.new
> @cloud_account.destroyable?.should_not be_true
> @cloud_account.destroy
> CloudAccount.find(@cloud_account.id).should_not be_nil
>
> @cloud_account.destroyable?.should be_false
>
> @cloud_account.destroy.should be_false
>
> @cloud_account.instances.clear
> @cloud_account.destroyable?.should be_true
> @cloud_account.destroy
> CloudAccount.find(:first, :conditions => ['id = ?',
>
> @cloud_account.id]).should be_nil
>
> @cloud_account.destroy.equal?(@cloud_account).should be_true
>
> @cloud_account.should be_frozen
> end
>
> it "should check the validitiy of the cloud account login
>
> credentials" do
> @@ -51,6 +49,11 @@ describe CloudAccount do
>      cloud_account.instance_key.id == "1_user"
>    end
> it "when calling connect and it fails with exception it will return
>
> nil" do
>
> DeltaCloud.should_receive(:new).and_raise(Exception.new)
>
> @cloud_account.connect.should be_nil
>
> end
>
> it "should generate credentials xml" do
>   expected_xml = <<EOT
>
> -- 
> 1.7.3.1
>
> deltacloud-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/deltacloud-devel

Thank you Tom! Fantastic catch. The code needs more love than I suspected. ;) Great QA.

-- Ladislav

On Nov 26, 2010, at 5:39 PM, Tomas Sedovic wrote:
> I'm really sorry, I noticed that the previously set Quota value isn't displayed in the edit Provider Account page. I should have noticed it sooner when I reviewed the previous version of this patch.
>
> Here's how to repro it:
> on the Provider Accounts page fill in the data
> set Quota to 10
> Click Add
> Click Edit
>
> Expected:
> The Quota field will contain the value 10.
>
> Actual:
> The Quota field contains the string "unlimited".
>
> ----- [e-mail] wrote:
> > From: Ladislav Martincik
> >
> > src/app/controllers/cloud_accounts_controller.rb   |   88
> > +++++++++++---------
> > src/app/controllers/providers_controller.rb        |    9 --
> > src/app/models/cloud_account.rb                    |   58
> > +++++++++----
> > src/app/views/cloud_accounts/_form.haml            |   89
> > ++++++++++++--------
> > src/app/views/cloud_accounts/edit.haml             |   23 ++++--
> > src/app/views/cloud_accounts/index.haml            |   25 ++++++
> > src/app/views/cloud_accounts/new.haml              |   25 ++++--
> > src/app/views/layouts/_notification.rhtml          |    3 +
> > src/config/locales/en.yml                          |   11 ++-
> > src/config/navigation.rb                           |    4 +-
> > src/config/routes.rb                               |    5 +-
> > src/features/provider.feature                      |    4 +-
> > src/features/step_definitions/web_steps.rb         |    6 ++
> > .../controllers/cloud_accounts_controller_spec.rb  |   37 +++++++--
> > src/spec/controllers/provider_controller_spec.rb   |   12 ---
> > src/spec/models/cloud_account_spec.rb              |   15 ++--
> > 16 files changed, 265 insertions(+), 149 deletions(-)
> > create mode 100644 src/app/views/cloud_accounts/index.haml
> >
> > diff --git a/src/app/controllers/cloud_accounts_controller.rb
> > b/src/app/controllers/cloud_accounts_controller.rb
> > index e1c9b23..6a2a072 100644
> > --- a/src/app/controllers/cloud_accounts_controller.rb
> > +++ b/src/app/controllers/cloud_accounts_controller.rb
> > @@ -21,6 +21,14 @@
> >
> > class CloudAccountsController < ApplicationController
> >   before_filter :require_user
> >
> > before_filter :load_providers
> >
> > helper :providers
> >
> > def index
> >
> > @provider = Provider.find(params[:provider_id])
> >
> > require_privilege(Privilege::ACCOUNT_VIEW, @provider)
> >
> > end
> >
> > def new
> >  @provider = Provider.find(params[:provider_id])
> >
> > @@ -29,43 +37,38 @@ class CloudAccountsController <
> > ApplicationController
> >   end
> >
> > def create
> > @provider = Provider.find(params[:cloud_account][:provider_id])
> > @provider = Provider.find(params[:provider_id])
> > require_privilege(Privilege::ACCOUNT_MODIFY,@provider)
> > if params[:cloud_account] &&
> >
> > !params[:cloud_account][:x509_cert_priv_file].blank?
> >  params[:cloud_account][:x509_cert_priv] =
> >
> > params[:cloud_account][:x509_cert_priv_file].read
> > end
> > params[:cloud_account].delete :x509_cert_priv_file
> > if params[:cloud_account] &&
> >
> > !params[:cloud_account][:x509_cert_pub_file].blank?
> >  params[:cloud_account][:x509_cert_pub] =
> >
> > params[:cloud_account][:x509_cert_pub_file].read
> > end
> > params[:cloud_account].delete :x509_cert_pub_file
> > @cloud_account = CloudAccount.new(params[:cloud_account])
> >
> > @cloud_account.provider = @provider
> >
> > if params[:test_account]
> >  test_account(@cloud_account)
> >  redirect_to :controller => "providers", :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 => "providers", :action => "accounts",
> >
> > :id => @provider
> >  kick_condor
> > else
> >  render :action => 'new' and return
> > end
> > if @cloud_account.invalid?
> >  if not @cloud_account.valid_credentials?
> >    flash[:notice] = "The entered credential information is
> >
> > incorrect"
> >    flash.now[:error] = "The entered credential information is
> >
> > incorrect"
> >       elsif @cloud_account.errors.on(:username)
> >    flash[:notice] = "The access key
> >
> > '#{params[:cloud_account][:username]}' has already been taken."
> >    flash.now[:error] = "The access key
> >
> > '#{params[:cloud_account][:username]}' has already been taken."
> >       else
> >    flash[:notice] = "You must fill in all the required fields"
> >    flash.now[:error] = "You must fill in all the required
> >
> > fields"
> >       end
> >  redirect_to :controller => "providers", :action => "accounts",
> >
> > :id => @provider, :cloud_account => params[:cloud_account]
> >  render :action => 'new' and return
> > end
> > quota = Quota.new
> > quota.maximum_running_instances =
> >
> > quota_from_string(params[:quota][:maximum_running_instances])
> >
> > quota.save!
> >
> > @cloud_account.quota = quota
> >
> > @cloud_account.zones << Zone.default
> >
> > @cloud_account.save!
> >
> > if @cloud_account.populate_realms
> >  flash[:notice] = "Provider account added."
> >
> > end
> >
> > redirect_to provider_accounts_path(@provider)
> >
> > kick_condor
> > end
> >
> > def edit
> >
> > @@ -119,11 +122,12 @@ class CloudAccountsController <
> > ApplicationController
> >   end
> >
> > def update
> > @cloud_account = CloudAccount.find(params[:cloud_account][:id])
> >
> > require_privilege(Privilege::ACCOUNT_MODIFY,@cloud_account.provider)
> > @cloud_account = CloudAccount.find(params[:id])
> > @provider = @cloud_account.provider
> > require_privilege(Privilege::ACCOUNT_MODIFY, @provider)
> > if @cloud_account.update_attributes(params[:cloud_account])
> >  flash[:notice] = "Cloud Account updated!"
> >  redirect_to :controller => 'providers', :action => 'accounts',
> >
> > :id => @cloud_account.provider.id
> >  redirect_to provider_accounts_path(@provider)
> > else
> >     render :action => :edit
> >   end
> >
> > @@ -138,27 +142,27 @@ class CloudAccountsController <
> > ApplicationController
> >   end
> >
> > def destroy
> > acct = CloudAccount.find(params[:id])
> > provider = acct.provider
> > require_privilege(Privilege::ACCOUNT_MODIFY,provider)
> > if acct.destroyable?
> >  CloudAccount.destroy(params[:id])
> > account = CloudAccount.find(params[:id])
> > provider = account.provider
> > require_privilege(Privilege::ACCOUNT_MODIFY, provider)
> > if account.destroy
> >  flash[:notice] = "Cloud Account destroyed"
> > else
> >  flash[:notice] = "Cloud Account could not be destroyed"
> >  flash[:error] = "Cloud Account could not be destroyed"
> > end
> > redirect_to :controller => 'providers', :action => 'accounts',
> >
> > :id => provider.id
> >
> > redirect_to provider_accounts_path(provider)
> > end
> >
> > def test_account(account)
> > if account.valid_credentials?
> >  flash[:notice] = "Test Connection Success: Valid Account
> >
> > Details"
> >  flash.now[:notice] = "Test Connection Success: Valid Account
> >
> > Details"
> >     else
> >  flash[:notice] = "Test Connection Failed: Invalid Account
> >
> > Details"
> >  flash.now[:error] = "Test Connection Failed: Invalid Account
> >
> > Details"
> >     end
> >   rescue
> > flash[:notice] = "Test Connection Failed: Could not connect to
> >
> > provider"
> > flash.now[:error] = "Test Connection Failed: Could not connect to
> >
> > provider"
> >   end
> >
> > private
> >
> > def quota_from_string(quota_raw)
> >
> > @@ -168,4 +172,8 @@ class CloudAccountsController <
> > ApplicationController
> >       return Integer(quota_raw)
> >     end
> >   end
> > def load_providers
> > @providers = Provider.list_for_user(@current_user,
> >
> > Privilege::PROVIDER_VIEW)
> > end
> >
> > end
> > diff --git a/src/app/controllers/providers_controller.rb
> > b/src/app/controllers/providers_controller.rb
> > index 580552e..0754d65 100644
> > --- a/src/app/controllers/providers_controller.rb
> > +++ b/src/app/controllers/providers_controller.rb
> > @@ -120,15 +120,6 @@ class ProvidersController <
> > ApplicationController
> >     require_privilege(Privilege::PROVIDER_VIEW, @provider)
> >   end
> > def accounts
> > @provider = Provider.find(:first, :conditions => {:id =>
> >
> > params[:id]})
> > require_privilege(Privilege::ACCOUNT_VIEW, @provider)
> > if params[:cloud_account]
> >  @cloud_account = CloudAccount.new(params[:cloud_account])
> >  @quota = Quota.new(params[:quota])
> > end
> > end
> > def realms
> >   @provider = Provider.find(params[:id])
> >   @realm_names = @provider.realms.collect { |r| r.name }
> >
> > diff --git a/src/app/models/cloud_account.rb
> > b/src/app/models/cloud_account.rb
> > index 5673e9a..e3ef0db 100644
> > --- a/src/app/models/cloud_account.rb
> > +++ b/src/app/models/cloud_account.rb
> > @@ -23,34 +23,62 @@ require 'nokogiri'
> >
> > class CloudAccount < ActiveRecord::Base
> >   include PermissionedObject
> > # Relations
> > belongs_to :provider
> > belongs_to :quota
> > has_many :instances
> > has_and_belongs_to_many :zones
> > has_many :permissions, :as => :permission_object, :dependent =>
> >
> > :destroy,
> >       :include => [:role],
> >       :order => "permissions.id ASC"
> >
> > # what form does the account quota take?
> > has_one :instance_key, :dependent => :destroy
> > validates_presence_of :provider_id
> >
> > # Helpers
> >
> > attr_accessor :x509_cert_priv_file, :x509_cert_pub_file
> >
> > # Validations
> >
> > 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
> > validate :validate_presence_of_x509_certs
> > validate :validate_credentials
> > # We're using this instead of <tt>validates_presence_of</tt> helper
> >
> > because
> > # we want to show errors on different attributes ending with
> >
> > '_file'.
> > def validate_presence_of_x509_certs
> > errors.add(:x509_cert_pub_file, "can't be blank") if
> >
> > x509_cert_pub.blank?
> > errors.add(:x509_cert_priv_file, "can't be blank") if
> >
> > x509_cert_priv.blank?
> > end
> > has_many :permissions, :as => :permission_object, :dependent =>
> >
> > :destroy,
> >       :include => [:role],
> >       :order => "permissions.id ASC"
> > def validate_credentials
> > unless valid_credentials?
> >  errors.add(:base, "Login Credentials are Invalid for this
> >
> > Provider")
> > end
> > end
> > has_one :instance_key, :dependent => :destroy
> > # Hooks
> > after_create :generate_cloud_account_key
> > before_destroy {|entry| entry.destroyable? }
> >
> > before_destroy :destroyable?
> >
> > before_validation :read_x509_files
> >
> > def destroyable?
> > self.instances.empty?
> > instances.empty?
> > end
> > def read_x509_files
> > if x509_cert_pub_file.respond_to?(:read)
> >  x509_cert_pub_file.rewind # Sometimes the file has to rewind,
> >
> > becuase something already read from it.
> >  self.x509_cert_pub = x509_cert_pub_file.read
> >
> > end
> >
> > if x509_cert_priv_file.respond_to?(:read)
> >  x509_cert_priv_file.rewind
> >  self.x509_cert_priv = x509_cert_priv_file.read
> >
> > end
> > end
> >
> > def connect
> >
> > @@ -76,7 +104,7 @@ class CloudAccount < ActiveRecord::Base
> >   end
> >
> > def name
> > label.nil? || label == "" ? username : label
> >
> > label.blank? ? username : label
> > end
> >
> > # FIXME: for already-mapped accounts, update rather than add new
> >
> > @@ -142,11 +170,6 @@ EOT
> >     return xml.to_s
> >   end
> > protected
> > def validate
> > errors.add_to_base("Login Credentials are Invalid for this
> >
> > Provider") unless valid_credentials?
> > end
> > private
> > def generate_cloud_account_key
> >   client = connect
> >
> > @@ -156,5 +179,4 @@ EOT
> >     end
> >   end
> >
> > end
> > diff --git a/src/app/views/cloud_accounts/_form.haml
> > b/src/app/views/cloud_accounts/_form.haml
> > index 32c12a4..192ce05 100644
> > --- a/src/app/views/cloud_accounts/_form.haml
> > +++ b/src/app/views/cloud_accounts/_form.haml
> > @@ -1,37 +1,52 @@
> > -%fieldset
> > %legend Account
> > = hidden_field :cloud_account,  :id
> > = hidden_field :cloud_account, :provider_id, :value =>
> >
> > @provider.id
> > %ul
> > %li
> >  %label
> >    Label
> >    %span user-visible name for the account
> >  = text_field :cloud_account, :label
> > if @cloud_account.id.nil?
> >  %li
> >    %label
> >      UserName
> >      %span UserName for the provider account you wish to add.
> >    = text_field :cloud_account, :username
> > %li
> >  %label
> >    Password
> >    %span Password for the provider account you wish to add.
> >  = password_field :cloud_account, :password
> > %li
> >  %label
> >    Account number
> >    %span EC2 account number.
> >  = text_field :cloud_account, :account_number
> > %li
> >  %label
> >    Account certificate
> >    %span EC2 x509 private key
> >  = text_area :cloud_account, :x509_cert_priv
> > %li
> >  %label
> >    Account certificate
> >    %span EC2 x509 private key
> >  = text_area :cloud_account, :x509_cert_pub
> >
> > -= submit_tag "Save", :class => "submit"
> > += error_messages_for 'cloud_account'
> > +%fieldset.clearfix.nomargin
> > %label.grid_4.la.alpha
> > = t('.account_name')
> > %span.required *
> > %label.grid_3.la
> > = t('.user_name')
> > %span.required *
> > %label.grid_3.la
> > = t('.password')
> > %span.required *
> > %label.grid_3.la.omega
> > = t('.quota_instances')
> > %span.required *
> >
> > +%fieldset.nomargin.clearfix
> > = f.text_field :label, :title => t('.account_name'), :class =>
> >
> > "grid_4 alpha"
> > = f.text_field :username, :title => t('.user_name'), :class =>
> >
> > "grid_3"
> > = f.password_field :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
> > (
> > %button.linkbutton.nospace{ :type => 'button', :onclick =>
> >
> > "set_unlimited_quota("quota_instances");" }<>
> >  = t('.unlimited_quota')
> > )
> >
> > +%fieldset.clearfix.nomargin
> > %label.grid_4.la.alpha
> > = t('.account_number')
> > %span.required *
> > %label.grid_3.la
> > = t('.account_private_cert')
> > %span.required *
> > %label.grid_3.la
> > = t('.account_public_cert')
> > %span.required *
> > .grid_3.omega
> >
> > +%fieldset.clearfix.nomargin
> > = f.text_field :account_number, :title => t('.account_number'),
> >
> > :class => "grid_4 alpha"
> > .grid_3
> > = f.file_field :x509_cert_priv_file, :title =>
> >
> > t('.account_private_cert')
> > .grid_3
> > = f.file_field :x509_cert_pub_file, :title =>
> >
> > t('.account_public_cert')
> > .grid_3.omega
> > (
> > %button.linkbutton.nospace{ :type => 'submit', :value =>
> >
> > t('.test_account'), :name => 'test_account', :id => 'test_account'
> > }<>
> >  = t('.test_account')
> > )
> >
> > +:javascript
> > function set_unlimited_quota(elem_id) {
> > $("#" + elem_id)[0].value = "unlimited";
> > }
> >
> > diff --git a/src/app/views/cloud_accounts/edit.haml
> > b/src/app/views/cloud_accounts/edit.haml
> > index 9e16553..c97faac 100644
> > --- a/src/app/views/cloud_accounts/edit.haml
> > +++ b/src/app/views/cloud_accounts/edit.haml
> > @@ -1,6 +1,17 @@
> > -.dcloud_form
> > = error_messages_for 'cloud_account'
> > %h2 Edit Cloud Account
> > %br/
> > form_for @cloud_account, :url => { :action => 'update' } do
> >
> > |form|
> > = render :partial => 'form'
> >
> > += render :partial => 'providers/providers'
> > +#details.grid_13
> > %nav.subsubnav
> > = render_navigation(:level => 4)
> > %h2
> > = t('.edit_provider_account')
> > form_for @cloud_account, :url => provider_account_path(@provider,
> >
> > @cloud_account), :html => { :method => :put, :multipart => true } do
> > |f|
> > = render :partial => 'form', :locals => { :f => f }
> > %fieldset.clearfix
> >  .grid_13.alpha.omega
> >    = submit_tag t(:edit), :class => "ra nomargin dialogbutton"
> > %section
> >  %p.requirement
> >    %span.required *
> >    \-
> >    = t('.required_field')
> >
> > diff --git a/src/app/views/cloud_accounts/index.haml
> > b/src/app/views/cloud_accounts/index.haml
> > new file mode 100644
> > index 0000000..ab2d475
> > --- /dev/null
> > +++ b/src/app/views/cloud_accounts/index.haml
> > @@ -0,0 +1,25 @@
> > += render :partial => 'providers/providers'
> > +#details.grid_13
> > %nav.subsubnav
> > = render_navigation(:level => 4)
> > %h2
> > = t('.provider_accounts')
> > unless @provider.cloud_accounts.empty?
> > %table
> >  %thead
> >    %tr
> >      %th{:scope => "col"} Label
> >      %th{:scope => "col"} Username
> >      %th{:scope => "col"} Account Number
> >      %th{:scope => "col", :colspan => "2"} Actions
> >  %tbody
> >    - @provider.cloud_accounts.each do |cloud_account|
> >      %tr
> >        %td= cloud_account.label
> >        %td= cloud_account.username
> >        %td= cloud_account.account_number
> >        %td= link_to 'Edit',
> >
> > edit_provider_account_path(@provider, cloud_account)
> >        %td= link_to 'Delete',
> >
> > destroy_providers_account_path(@provider, cloud_account), :confirm =>
> > 'Are you sure?'
> > if @provider.cloud_accounts.empty?
> > = link_to 'Add', new_provider_account_path(@provider.id), :class
> >
> > => 'button'
> > diff --git a/src/app/views/cloud_accounts/new.haml
> > b/src/app/views/cloud_accounts/new.haml
> > index 446fd36..96d08df 100644
> > --- a/src/app/views/cloud_accounts/new.haml
> > +++ b/src/app/views/cloud_accounts/new.haml
> > @@ -1,8 +1,17 @@
> > -.dcloud_form
> > = error_messages_for 'account'
> > %h2 Add a new Account from this Provider
> > %br/
> > form_for @cloud_account, :url => { :action => "create" } do |f|
> > = f.error_messages
> > = render :partial => "form", :object => f
> > = link_to "Cancel", root_path, :class => 'actionlink'
> >
> > += render :partial => 'providers/providers'
> > +#details.grid_13
> > %nav.subsubnav
> > = render_navigation(:level => 4)
> > %h2
> > = t('.new_provider_account')
> > form_for @cloud_account, :url =>
> >
> > provider_accounts_path(@provider), :html => { :multipart => true } do
> > |f|
> > = render :partial => 'form', :locals => { :f => f }
> > %fieldset.clearfix
> >  .grid_13.alpha.omega
> >    = submit_tag t(:add), :class => "ra nomargin dialogbutton"
> > %section
> >  %p.requirement
> >    %span.required *
> >    \-
> >    = t('.required_field')
> >
> > diff --git a/src/app/views/layouts/_notification.rhtml
> > b/src/app/views/layouts/_notification.rhtml
> > index fdbf642..6c204fd 100644
> > --- a/src/app/views/layouts/_notification.rhtml
> > +++ b/src/app/views/layouts/_notification.rhtml
> > @@ -22,6 +22,9 @@
> >       </div>
> >     <% end %>
> >   <% end %>
> > <% if flash[:error] && flash[:error].kind_of?(String) %>
> >
> > <div class="error"><ul><li><%= flash[:error] %></li></ul></div>
> > <% end %>
> > <% if flash[:warning] %>
> >
> >  <div class="warning"><ul><li><%= flash[:warning]
> >
> > %></li></ul></div>
> >   <% end %>
> > diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> > index 6539cbd..1be36b6 100644
> > --- a/src/config/locales/en.yml
> > +++ b/src/config/locales/en.yml
> > @@ -97,16 +97,23 @@ en:
> >       test_connection: Test Connection
> >       required_field: Required field.
> >       caution_image: Caution
> > accounts:
> > cloud_accounts:
> > index:
> >  provider_accounts: Provider Accounts
> > new:
> >  new_provider_account: New Account
> >  required_field: Required field.
> > edit:
> >  edit_provider_account: Edit Cloud Account
> >  required_field: Required field.
> > form:
> >  provider_accounts: Provider Accounts
> > account_name: Account Name
> > user_name: EC2 Access Key
> > password: Secret Access Key
> > quota_instances: Quota Instances
> > test_account: Test Account
> > unlimited_quota: Unlimited Quota
> >  required_field: Required field.
> > account_number: AWS Account ID
> > account_private_cert: EC2 x509 private key
> > account_public_cert: EC2 x509 public key
> >
> > diff --git a/src/config/navigation.rb b/src/config/navigation.rb
> > index 3b63df1..95d5e05 100644
> > --- a/src/config/navigation.rb
> > +++ b/src/config/navigation.rb
> > @@ -8,8 +8,8 @@ SimpleNavigation::Configuration.run do |navigation|
> >     first_level.item :administration, t(:administration), '#', :class
> > => 'administration' do |second_level|
> >       second_level.item :system_settings, t(:system_settings),
> > :controller => 'settings' do |third_level|
> >         third_level.item :manage_providers, t(:manage_providers),
> > providers_path do |fourth_level|
> >      fourth_level.item :provider_summary, t(:provider_summary),
> >
> > { :controller => 'providers', :action => 'show', :id => (@provider.id
> > if @provider) }, :highlights_on => //providers/[0-9]+/
> >      fourth_level.item :provider_accounts,
> >
> > t(:provider_accounts), { :controller => 'providers', :action =>
> > 'accounts', :id => (@provider.id if @provider) }, :highlights_on =>
> > //providers/accounts/
> >      fourth_level.item :provider_summary, t(:provider_summary),
> >
> > { :controller => 'providers', :action => 'show', :id => (@provider.id
> > if @provider) }, :highlights_on => //providers/\d+(/edit)?$/
> >      fourth_level.item :provider_accounts,
> >
> > t(:provider_accounts), { :controller => 'cloud_accounts', :action =>
> > 'index', :provider_id => (@provider.id if @provider) }, :highlights_on
> > => //providers/\d+/accounts(/(\d+|new))?/
> >           fourth_level.item :scheduling_policies,
> > t(:scheduling_policies), '#'
> >           fourth_level.item :services_provided,
> > t(:services_provided), '#'
> >           fourth_level.item :map_profiles, t(:map_profiles), '#'
> > diff --git a/src/config/routes.rb b/src/config/routes.rb
> > index f59a2aa..931c97f 100644
> > --- a/src/config/routes.rb
> > +++ b/src/config/routes.rb
> > @@ -52,7 +52,10 @@ ActionController::Routing::Routes.draw do |map|
> >
> > # Temporarily disable this route, provider stuff is not restful
> > yet.
> >   # Will be re-enabled in upcoming patch
> > map.resources :providers
> > map.resources :providers do |provider|
> > provider.resources :accounts, :controller => 'cloud_accounts'
> > end
> > map.destroy_providers_account
> >
> > '/providers/:provider_id/accounts/:id/destroy', :controller =>
> > 'cloud_accounts', :action => 'destroy', :conditions => { :method =>
> > :get }
> >
> > # Allow downloading Web Service WSDL as a file with an extension
> >   # instead of a file named 'wsdl'
> > diff --git a/src/features/provider.feature
> > b/src/features/provider.feature
> > index 12047bb..8cf2ccd 100644
> > --- a/src/features/provider.feature
> > +++ b/src/features/provider.feature
> > @@ -65,6 +65,7 @@ Feature: Manage Providers
> >     When I go to the providers page
> >     And I follow "provider1"
> >     And I follow "Provider Accounts"
> > And I follow "Add" within "#details"
> > And I fill in "cloud_account[label]" with "MockAccount"
> > And I fill in "cloud_account[username]" with "mockuser"
> > And I fill in "cloud_account[password]" with "mockpassword"
> >
> > @@ -80,6 +81,7 @@ Feature: Manage Providers
> >     When I go to the providers page
> >     And I follow "provider1"
> >     And I follow "Provider Accounts"
> > And I follow "Add" within "#details"
> > And I fill in "cloud_account[label]" with "IncorrectAccount"
> > And I fill in "cloud_account[username]" with "incorrect_user"
> > And I fill in "cloud_account[password]" with
> >
> > "incorrect_password"
> > @@ -102,4 +104,4 @@ Feature: Manage Providers
> >     And there should not be any replicated images
> >     And there should not be any hardware profiles
> >     And there should not be a cloud account
> > And there should not be a realm
> >
> > \ No newline at end of file
> > And there should not be a realm
> >
> > diff --git a/src/features/step_definitions/web_steps.rb
> > b/src/features/step_definitions/web_steps.rb
> > index f270900..e6d6808 100644
> > --- a/src/features/step_definitions/web_steps.rb
> > +++ b/src/features/step_definitions/web_steps.rb
> > @@ -24,6 +24,12 @@ When /^(?:|I )press "([^"]*)"$/ do |button|
> >   click_button(button)
> > end
> >
> > +When /^I press "([^"]*)" within "([^"]*)"$/ do
> > |button,scope_selector|
> > within(scope_selector) do
> > click_button(button)
> > end
> >
> > +end
> >
> > When /^(?:|I )follow "([^"]*)"$/ do |link|
> >   click_link(link)
> > end
> > diff --git a/src/spec/controllers/cloud_accounts_controller_spec.rb
> > b/src/spec/controllers/cloud_accounts_controller_spec.rb
> > index f3b866a..072126e 100644
> > --- a/src/spec/controllers/cloud_accounts_controller_spec.rb
> > +++ b/src/spec/controllers/cloud_accounts_controller_spec.rb
> > @@ -14,6 +14,29 @@ describe CloudAccountsController do
> >     activate_authlogic
> >   end
> > it "shows provider accounts as list" do
> > UserSession.create(@admin)
> > get :index, :provider_id => @provider.id
> > response.should be_success
> > response.should render_template("index")
> > end
> > it "allows test account validity on create when passing
> >
> > test_account param" do
> > UserSession.create(@admin)
> > post :create, :provider_id => @provider.id, :cloud_account => {},
> >
> > :test_account => true
> > response.should be_success
> > response.should render_template("new")
> > response.flash[:error].should == "Test Connection Failed: Invalid
> >
> > Account Details"
> > end
> > it "doesn't allow to save provider's account if not valid
> >
> > credentials" do
> > UserSession.create(@admin)
> > post :create, :provider_id => @provider.id, :cloud_account => {}
> > response.should be_success
> > response.should render_template("new")
> > response.flash[:error].should == "The entered credential
> >
> > information is incorrect"
> > end
> > it "should permit users with account modify permission to access
> >
> > edit cloud account interface" do
> >     UserSession.create(@admin)
> >     get :edit, :id => @cloud_account.id
> > @@ -25,21 +48,21 @@ describe CloudAccountsController do
> >     UserSession.create(@admin)
> > @cloud_account.password = "foobar"
> > @cloud_account.stub!(:valid_credentials).and_return(true)
> > @cloud_account.save
> > @cloud_account.stub!(:valid_credentials?).and_return(true)
> > @cloud_account.save.should be_true
> > post :update, :cloud_account => { :id => @cloud_account.id,
> >
> > :password => 'mockpassword' }
> > response.should
> >
> > redirect_to("http://test.host/providers/accounts/#[e-mail]%7D")
> > post :update, :id => @cloud_account.id, :cloud_account => {
> >
> > :password => 'mockpassword' }
> > response.should redirect_to provider_accounts_path(@provider)
> > CloudAccount.find(@cloud_account.id).password.should ==
> >
> > "mockpassword"
> >   end
> >
> > it "should allow users with account modify permission to delete a
> > cloud account" do
> >     UserSession.create(@admin)
> >     lambda do
> >  post :destroy, :id => @cloud_account.id
> >  get :destroy, :id => @cloud_account.id
> > end.should change(CloudAccount, :count).by(-1)
> > response.should
> >
> > redirect_to("http://test.host/providers/accounts/#[e-mail]%7D")
> > CloudAccount.find(:first, :conditions => ['id = ?',
> >
> > @cloud_account.id]).should be_nil
> >
> > response.should redirect_to provider_accounts_path(@provider)
> >
> > CloudAccount.find_by_id(@cloud_account.id).should be_nil
> > end
> >
> > it "should deny access to users without account modify permission"
> >
> > do
> > diff --git a/src/spec/controllers/provider_controller_spec.rb
> > b/src/spec/controllers/provider_controller_spec.rb
> > index a1a317a..49a45a8 100644
> > --- a/src/spec/controllers/provider_controller_spec.rb
> > +++ b/src/spec/controllers/provider_controller_spec.rb
> > @@ -10,18 +10,6 @@ describe ProvidersController do
> >     activate_authlogic
> >   end
> > it "should provide ui to view accounts" do
> > UserSession.create(@admin)
> > get :accounts, :id => @provider.id
> > response.should be_success
> > response.should render_template("accounts")
> > end
> > it "should fail to grant access to account UIs for unauthenticated
> >
> > user" do
> > get :accounts
> > response.should_not be_success
> > end
> > it "should provide ui to view hardware profiles" do
> >    UserSession.create(@admin)
> >    provider = @admin_permission.permission_object
> >
> > diff --git a/src/spec/models/cloud_account_spec.rb
> > b/src/spec/models/cloud_account_spec.rb
> > index 4466977..fb04e40 100644
> > --- a/src/spec/models/cloud_account_spec.rb
> > +++ b/src/spec/models/cloud_account_spec.rb
> > @@ -8,15 +8,13 @@ describe CloudAccount do
> >
> > it "should not be destroyable if it has instances" do
> >     @cloud_account.instances << Instance.new
> > @cloud_account.destroyable?.should_not be_true
> > @cloud_account.destroy
> > CloudAccount.find(@cloud_account.id).should_not be_nil
> >
> > @cloud_account.destroyable?.should be_false
> >
> > @cloud_account.destroy.should be_false
> >
> > @cloud_account.instances.clear
> > @cloud_account.destroyable?.should be_true
> > @cloud_account.destroy
> > CloudAccount.find(:first, :conditions => ['id = ?',
> >
> > @cloud_account.id]).should be_nil
> >
> > @cloud_account.destroy.equal?(@cloud_account).should be_true
> >
> > @cloud_account.should be_frozen
> > end
> >
> > it "should check the validitiy of the cloud account login
> >
> > credentials" do
> > @@ -51,6 +49,11 @@ describe CloudAccount do
> >     cloud_account.instance_key.id == "1_user"
> >   end
> > it "when calling connect and it fails with exception it will return
> >
> > nil" do
> >
> > DeltaCloud.should_receive(:new).and_raise(Exception.new)
> >
> > @cloud_account.connect.should be_nil
> >
> > end
> >
> > it "should generate credentials xml" do
> >  expected_xml = <<EOT
> >
> > -- 
> > 1.7.3.1
> >
> > deltacloud-devel mailing list
> > [e-mail]
> > https://fedorahosted.org/mailman/listinfo/deltacloud-devel