On Mon, 2010-09-20 at 19:50 +0200, [email protected] wrote:
> From: Tomas Sedovic <[email protected]>
> 
> This new page is capable of viewing/updating/creating new providers, so the
> funtionality has been moved there.

functionality ^
> ---
>  src/app/controllers/provider_controller.rb |   26 ++++++++++-
>  src/app/views/provider/show.haml           |   73 
> +++++++++++++++++++++++-----
>  src/config/locales/en.yml                  |   13 +++++
>  src/config/navigation.rb                   |    2 +-
>  4 files changed, 100 insertions(+), 14 deletions(-)
> 
Mostly ACK (realized after I started writing this you have resent this
and previous patch, but think it is the same), couple notes here and
inline. When you create a provider and click 'add' - it does add, but
does not render any new information on the page, just displays the same
'new' page, with no feedback, and no information showing for the
provider you just added, either in the form or the list on the left. If
you are not seeing this behavior, it is possible I just need a newer
version of condor, so I would take your word for it, just let me know.

Oh, one major bit I almost missed - this (or another in the series)
seems to majorly break 'rake cucumber', so please take a look at that
before resending series.

> diff --git a/src/app/controllers/provider_controller.rb 
> b/src/app/controllers/provider_controller.rb
> index af8c3f9..1893967 100644
> --- a/src/app/controllers/provider_controller.rb
> +++ b/src/app/controllers/provider_controller.rb
> @@ -27,14 +27,24 @@ class ProviderController < ApplicationController
>    end
>  
>    def show
> -    @provider = Provider.find(:first, :conditions => {:id => params[:id]})
> +    @providers = Provider.list_for_user(@current_user, 
> Privilege::PROVIDER_VIEW)

I notice this ^ line in several methods, it should be moved into it's
own method and set as a before filter for all actions that use it.

> +    @current_provider = Provider.find(:first, :conditions => {:id => 
> params[:id]})
>      require_privilege(Privilege::PROVIDER_VIEW, @provider)
>    end
>  
> +  def edit
> +    @providers = Provider.list_for_user(@current_user, 
> Privilege::PROVIDER_MODIFY)
> +    @current_provider = Provider.find(:first, :conditions => {:id => 
> params[:id]})
> +    require_privilege(Privilege::PROVIDER_MODIFY, @provider)
> +    render :show
> +  end
> +
>    def new
>      require_privilege(Privilege::PROVIDER_MODIFY)
> +    @providers = Provider.list_for_user(@current_user, 
> Privilege::PROVIDER_MODIFY)
>      @provider = Provider.new(params[:provider])
>      condormatic_classads_sync
> +    render :show
>    end
>  
>    def create
> @@ -50,6 +60,20 @@ class ProviderController < ApplicationController
>      condormatic_classads_sync
>    end
>  
> +  def update
> +    require_privilege(Privilege::PROVIDER_MODIFY)
> +    @provider = Provider.find(:first, :conditions => {:id => 
> params[:provider][:id]})
> +    @provider.name = params[:provider][:name]
> +
> +    if @provider.save
> +        flash[:notice] = "Provider updated."
> +        redirect_to :action => "show", :id => @provider
> +    else
> +      render :action => "edit"
> +    end
> +    condormatic_classads_sync
> +  end
> +
>    def destroy
>      if request.post?
>        @provider = Provider.find(params[:provider][:id])
> diff --git a/src/app/views/provider/show.haml 
> b/src/app/views/provider/show.haml
> index b3e0bd7..c5aa916 100644
> --- a/src/app/views/provider/show.haml
> +++ b/src/app/views/provider/show.haml
> @@ -1,12 +1,61 @@
> -- content_for :scripts do
> -  :javascript
> -    $(document).ready(function() {
> -      $("#provider-tabs").tabs();
> -    });
> -#provider-tabs
> -  %ul
> -    %li= link_to "Accounts",  {:action => "accounts", :id => @provider.id, 
> :ajax => true}
> -    %li= link_to "Realms",  {:action => "realms", :id => @provider.id, :ajax 
> => true }
> -    - if has_view_perms?
> -      %li= link_to "User access",  {:controller => "permissions", :action => 
> "list", :provider_id => @provider.id, :ajax => true }
> -    %li= link_to "Settings", {:action => "settings", :id => @provider.id, 
> :ajax => true}
> +- readonly = controller.action_name == 'show' ? true : false
> +#providers_nav.grid_3
> +  %dl
> +    %dt
> +      = t('.providers')
> +    - @providers.each do |provider|
> +      %dd
> +        %span
> +        - selected = 'selected' if @current_provider and (provider.id == 
> @current_provider.id)
> +        %a{ :href => url_for(:controller => 'provider', :action => 'show', 
> :id => provider), :class => selected }
> +          = provider.name
> +    - if controller.action_name == 'show' and @current_provider
> +      - edit_url = url_for(:controller => 'provider', :action => 'edit', :id 
> => @current_provider)
> +    - else
> +      - edit_class = 'disabled'
> +    %a.button{ :href => edit_url, :class => edit_class }<
> +      = t(:edit)
> +    - if @providers.length > 1
> +      - add_class = 'disabled'
> +    - else
> +      - add_url = url_for(:controller => 'provider', :action => 'new')
> +    %a.button{ :href => add_url, :class => add_class}
> +      = t(:add)
> +#details.grid_13
> +  = render_navigation(:level => 4)
> +  - if controller.action_name == 'new'
> +    - form_action = 'create'
> +  - elsif controller.action_name == 'edit'
> +    - form_action = 'update'
> +  - form_tag :controller => :provider, :action => form_action do
> +    %h2.first
> +      = t('.provider_name')
> +      - unless readonly
> +        %span.required
> +          *
> +    %h2.second
> +      = t('.provider_url')
> +      - unless readonly
> +        %span.required
> +          *
> +    .fieldGroup.clearfix
> +      = text_field :provider, :name, :title => t('.provider_name'), :value 
> => (@current_provider.name if @current_provider), :disabled => ('disabled' if 
> controller.action_name == 'show')
> +      = text_field :provider, :url, :title => t('.provider_url'), :class => 
> 'emailinput', :value => (@current_provider.url if @current_provider), 
> :disabled => ('disabled' unless controller.action_name == 'new')
> +      - if controller.action_name == 'edit':
> +        = hidden_field :provider, :id, :value => @current_provider.id
> +      %p
> +        %span
> +          (
> +          %a{ :href => ''}<>
> +            = t('.test_connection')
> +          )
> +    - unless readonly
> +      %p.requirement
> +        %span.required
> +          *
> +        \-
> +        = t('.required_field')
> +    - if controller.action_name == 'edit'
> +      %input{ :type => 'submit', :value => t(:save), :name => 
> 'save_provider', :id => 'save_provider' }
> +    - elsif controller.action_name == 'new'
> +      %input{ :type => 'submit', :value => t(:add), :name => 'add_provider', 
> :id => 'add_provider' }
> diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> index c435f00..8304a20 100644
> --- a/src/config/locales/en.yml
> +++ b/src/config/locales/en.yml
> @@ -39,6 +39,9 @@ en:
>    launch_instance: Launch Instance
>    help: Help
>    continue: Continue
> +  edit: Edit
> +  add: Add
> +  save: Save
>    settings:
>      index:
>        general_settings: General Settings
> @@ -55,3 +58,13 @@ en:
>        define_services_desc: View, edit and define services that DeltaCloud 
> will offer. These services will be mapped to individual providers based on 
> their capabilities.
>        permissions: Permissions
>        permissions_desc: Create and edit User Treatments that are applied to 
> users. Treatments pair together Roles with Categories of Instances, Pools and 
> Templates. Manage the categories, and define and edit Roles. Roles group 
> together sets of permissions that are relevant to Pools, Templates and 
> Instances.
> +  provider:
> +    show:
> +      providers: PROVIDERS
> +      provider_name: Provider Name
> +      provider_url: Provider URL
> +      caution_alt_text: Caution
> +      enter_correct_url_msg: Please enter a correct URL format.
> +      test_connection: Test Connection
> +      required_field: Required field.
> +      caution_image: Caution
> diff --git a/src/config/navigation.rb b/src/config/navigation.rb
> index 17e06ed..662462f 100644
> --- a/src/config/navigation.rb
> +++ b/src/config/navigation.rb
> @@ -8,7 +8,7 @@ SimpleNavigation::Configuration.run do |navigation|
>      first_level.item :administration, t(:administration), '', :class => 
> 'administration' do |second_level|
>        second_level.item :system_settings, t(:system_settings), "/settings" 
> do |third_level|
>          third_level.item :manage_providers, t(:manage_providers), 
> "/provider" do |fourth_level|
> -          fourth_level.item :provider_summary, t(:provider_summary), 
> "/provider/show"
> +          fourth_level.item :provider_summary, t(:provider_summary), 
> "/provider/show", :highlights_on => /\/provider\/(show|edit|new)/
>            fourth_level.item :provider_accounts, t(:provider_accounts), 
> "/provider/accounts"
>          end
>          third_level.item :self_service_settings, t(:self_service_settings), 
> "/settings/self-service"


_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel

Reply via email to