Mohammed Morsi wrote:
> ---
> src/app/controllers/cloud_accounts_controller.rb | 20 ++++++++++++++++++++
> src/app/controllers/provider_controller.rb | 4 ++--
> src/app/views/cloud_accounts/_form.html.erb | 10 ++++++++++
> src/app/views/cloud_accounts/edit.html.erb | 10 ++++++++++
> src/app/views/provider/accounts.html.erb | 4 ++++
> src/app/views/provider/new_account.html.erb | 13 +++----------
> 6 files changed, 49 insertions(+), 12 deletions(-)
> create mode 100644 src/app/views/cloud_accounts/_form.html.erb
> create mode 100644 src/app/views/cloud_accounts/edit.html.erb
>
> diff --git a/src/app/controllers/cloud_accounts_controller.rb
> b/src/app/controllers/cloud_accounts_controller.rb
> index 26c1a63..fb09468 100644
> --- a/src/app/controllers/cloud_accounts_controller.rb
> +++ b/src/app/controllers/cloud_accounts_controller.rb
> @@ -66,5 +66,25 @@ class CloudAccountsController < ApplicationController
> redirect_to :controller => "pool", :action => 'show', :id => @pool.id
> end
>
> + def edit
> + @cloud_account = CloudAccount.find(params[:id])
> + @provider = @cloud_account.provider
> + end
>
We need to check permissions here. For create we do:
require_privilege(Privilege::ACCOUNT_MODIFY,@provider)
That's probably fine here too -- just checking at the provider level. If
we want to check at the account level, we need to modify the create
actions to grant permissions to the user creating the cloud account on
the account itself.
>
> + def update
> + @cloud_account = CloudAccount.find(params[:cloud_account][:id])
> + if @cloud_account.update_attributes(params[:cloud_account])
> + flash[:notice] = "Cloud Account updated!"
> + redirect_to :controller => 'provider', :action => 'accounts', :id =>
> @cloud_account.provider.id
> + else
> + render :action => :edit
> + end
> + end
> +
>
What parameters are we allowing edits on? If we actually change the
username, then it's a new account entirely, and any existing instances
in this account would now be invalid. Should we only allow editing the
password?
In addition, the permission checks above apply.
> + def destroy
> + provider = CloudAccount.find(params[:id]).provider
> + CloudAccount.destroy(params[:id])
> + flash[:notice] = "Cloud Account destroyed"
> + redirect_to :controller => 'provider', :action => 'accounts', :id =>
> provider.id
> + end
>
Should we check for existing instances for this account? Otherwise any
users with instances scheduled here will have their instances suddenly
vanish from the UI.
> end
> diff --git a/src/app/controllers/provider_controller.rb
> b/src/app/controllers/provider_controller.rb
> index 0252a26..23b5ba0 100644
> --- a/src/app/controllers/provider_controller.rb
> +++ b/src/app/controllers/provider_controller.rb
> @@ -74,8 +74,8 @@ class ProviderController < ApplicationController
>
> def create_account
> require_privilege(Privilege::ACCOUNT_MODIFY)
> - @acct = CloudAccount.find_or_create(params[:account])
> - @provider = Provider.find(params[:account][:provider_id])
> + @acct = CloudAccount.find_or_create(params[:cloud_account])
> + @provider = Provider.find(params[:cloud_account][:provider_id])
> @provider.cloud_accounts << @acct
> redirect_to :action => 'accounts', :id => @provider.id
> end
> diff --git a/src/app/views/cloud_accounts/_form.html.erb
> b/src/app/views/cloud_accounts/_form.html.erb
> new file mode 100644
> index 0000000..504b5b7
> --- /dev/null
> +++ b/src/app/views/cloud_accounts/_form.html.erb
> @@ -0,0 +1,10 @@
> +<fieldset>
> +<legend>Account</legend>
> +<%= hidden_field :cloud_account, :id %>
> +<%= hidden_field :cloud_account, :provider_id, :value => @provider.id %>
> +<ul>
> +<li><label>UserName<span>UserName for the account you wish to connect to
> this pool.</span></label><%= text_field :cloud_account, :username %></li>
> +<li><label>Password<span>Password for the account you wish to connect to
> this pool.</span></label><%=password_field :cloud_account, :password %></li>
> +</ul>
> +</fieldset>
> +<%= submit_tag "Save", :class => "submit" %>
> diff --git a/src/app/views/cloud_accounts/edit.html.erb
> b/src/app/views/cloud_accounts/edit.html.erb
> new file mode 100644
> index 0000000..3ae86aa
> --- /dev/null
> +++ b/src/app/views/cloud_accounts/edit.html.erb
> @@ -0,0 +1,10 @@
> +<div class="dcloud_form">
> + <%= error_messages_for 'cloud_account' %>
> +
> + <h2>Edit Cloud Account</h2><br />
> +
> + <% form_for @cloud_account, :url => { :action => 'update' } do |form| -%>
> + <%= render :partial => 'form' %>
> + <% end %>
> +
> +</div>
> diff --git a/src/app/views/provider/accounts.html.erb
> b/src/app/views/provider/accounts.html.erb
> index 595443d..ebde82f 100644
> --- a/src/app/views/provider/accounts.html.erb
> +++ b/src/app/views/provider/accounts.html.erb
> @@ -5,12 +5,16 @@
> <thead>
> <tr>
> <th scope="col">Username</th>
> + <th scope="col"></th>
> + <th scope="col"></th>
> </tr>
> </thead>
> <tbody>
> <%[email protected]_accounts.each {|acct| %>
> <tr>
> <td><%= acct.username %></td>
> + <td><%= link_to "Edit", {:controller => 'cloud_accounts',
> :action => 'edit', :id => acct.id} %></td>
> + <td><%= link_to "Delete", {:controller => 'cloud_accounts',
> :action => 'destroy', :id => acct.id} %></td>
>
We need to only show Edit/Delete links for users with modify permissions
on the provider. In addition if we're not allowing delete for accounts
with instances, then we probably want to hide the 'delete' link for
accounts that have at least one instance.
> </tr>
> <% } %>
> </tbody>
> diff --git a/src/app/views/provider/new_account.html.erb
> b/src/app/views/provider/new_account.html.erb
> index b9ae229..eed474f 100644
> --- a/src/app/views/provider/new_account.html.erb
> +++ b/src/app/views/provider/new_account.html.erb
> @@ -4,15 +4,8 @@
>
> <h2>Create an Account for this Provider</h2><br />
>
> - <% form_tag :action => 'create_account' do -%>
> - <fieldset>
> - <legend>Account</legend>
> - <ul>
> - <li><label>UserName<span>UserName for the account you wish to connect to
> this pool.</span></label><%= text_field :account, :username %></li>
> - <li><label>Password<span>Password for the account you wish to connect to
> this pool.</span></label><%=password_field :account, :password %></li>
> - </ul>
> - <%=hidden_field :account, :provider_id, :value => @provider.id %>
> - </fieldset>
> - <%= submit_tag "Save", :class => "submit" %>
> + <% form_tag :action => 'create_account' do |form| -%>
> + <%= render :partial => 'cloud_accounts/form' %>
> <% end %>
> +
> </div>
>
_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel
