[PATCH conductor] BZ#798287 remove unused Settings pages

Mo Morsi mmorsi at redhat.com
Thu Mar 1 20:02:08 UTC 2012


Applied and seems to work as intended so ACK w/ some caveats below.

On 02/28/2012 10:22 AM, tsedovic at redhat.com wrote:
> From: Tomas Sedovic <tomas at sedovic.cz>
>
> ---
>  src/app/views/settings/general_settings.html.haml |   50 ----------
>  src/app/views/settings/index.html.haml            |   34 -------
>  src/app/views/settings/self_service.html.haml     |  108 +--------------------
>  src/config/locales/en.yml                         |   27 +-----
>  src/config/routes.rb                              |    7 --
>  5 files changed, 2 insertions(+), 224 deletions(-)
>  delete mode 100644 src/app/views/settings/general_settings.html.haml
>
> diff --git a/src/app/views/settings/general_settings.html.haml b/src/app/views/settings/general_settings.html.haml
> deleted file mode 100644
> index 5ed6e87..0000000
> --- a/src/app/views/settings/general_settings.html.haml
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -%header.admin-page-header
> -  %h1{:class => controller.controller_name}= t'settings.general_settings.general_settings'
> -  #obj_actions.button-group
> -
> -%section.admin-content-section.settings
> -  %header
> -    %h2= t'settings.settings'
> -
> -  .content
> -    %form{:class => 'container_16'}
> -      .grid_16
> -        %h1= t'settings.general_settings.portal'
> -        %fieldset.clearfix
> -          %label.grid_5.alpha= t 'settings.general_settings.brand_icon'
> -          .grid_6
> -            %input{:type => "file", :disabled => true}
> -        %fieldset.clearfix
> -          %label.grid_5.alpha= t 'settings.general_settings.tag_line'
> -          %textarea.grid_6.disabled{:disabled => true}
> -        %fieldset.clearfix
> -          .grid_5.alpha.disabled.ra
> -            %input{:type => "checkbox", :disabled => true}
> -            %label= t 'settings.general_settings.allow_self_service'
> -          .grid_6.disabled
> -            %select{:disabled => true}
> -              %option= t'settings.general_settings.select_treatment'
> -        %fieldset.clearfix
> -          %label.alpha.grid_5= t'settings.general_settings.primary_url_port'
> -          %input.grid_6{:value => "3000", :disabled => true}
> -        %h1=t 'settings.general_settings.content'
> -        %fieldset.clearfix
> -          %label.grid_5.alpha=t 'settings.general_settings.entitlement_mode'
> -          .grid_5
> -            %ul.block
> -              %li
> -                %input{:type => "radio", :name => "entitlement", :disabled => true}
> -                %label.disabled=t 'settings.general_settings.user_brings_subscriptions'
> -              %li
> -                %input{:type => "radio", :name => "entitlement", :disabled => true}
> -                %label.disabled= t'settings.general_settings.system_subscription'
> -        %fieldset.clearfix
> -          %label.grid_5.alpha= t'settings.general_settings.certificate'
> -          .grid_6
> -            %input{:type => "file", :disabled => true}
> -        %fieldset.clearfix
> -          %label.alpha.grid_5=t 'settings.general_settings.content_manager_url'
> -          %input.grid_6{:disabled => true}
> -        %fieldset.clearfix
> -          %button.formbutton{:disabled => true}=t 'reset'
> -          %button.formbutton{:disabled => true}=t 'save'

I'm assuming we legitimately want to remove
/conductor/settings/general_settings action from the application.

In this case, shouldn't we also remove the 'general_settings' method in
the settings controller?


> diff --git a/src/app/views/settings/index.html.haml b/src/app/views/settings/index.html.haml
> index bf94499..7facec7 100644
> --- a/src/app/views/settings/index.html.haml
> +++ b/src/app/views/settings/index.html.haml
> @@ -4,14 +4,6 @@
>  %section.admin-content-section.settings
>    .content
>      %ul.tiles{:class => 'container_16'}
> -      -#%li#services.grid_8.disabled
> -        = image_tag "blnk.png", :class => "icon", :alt => ""
> -        %h2
> -          = t('.define_services')
> -        %p
> -          = t('.define_services_desc')
> -        %button{:disabled => "disabled"}
> -          = t(:continue)
>        - if check_privilege(Privilege::MODIFY)
>          %li#permissions.grid_8
>            = image_tag "blnk.png", :class => "icon", :alt => ""
> @@ -20,30 +12,4 @@
>            %p
>              = t('.permissions_desc')
>            = link_to t(:continue), self_service_settings_path, :class => 'button'
> -      -#%li#settings.grid_8
> -        = image_tag "blnk.png", :class => "icon", :alt => ""
> -        %h2
> -          = t('.general_settings')
> -        %p
> -          = t('.general_settings_desc')
> -        = link_to t(:continue), general_settings_settings_path, :class => 'button'
> -      -#%li#locales.grid_8.disabled
> -        = image_tag "blnk.png", :class => "icon", :alt => ""
> -        %h2
> -          = t('.define_locales')
> -        %p
> -          = t('.define_locales_desc')
> -        %button{:disabled => "disabled"}
> -          = t(:continue)
>        .clear
> -
> -  :javascript
> -    $(document).ready(function () {
> -      var $tiles = $(".tiles li:not(.disabled)").addClass('pointercursor');
> -      $tiles.click(function (e) {
> -        $e = $(e.target);
> -        if (!$e.is('.button')) {
> -          window.location = $('.button',this).attr('href');
> -        }
> -      });
> -    });

Seems to just remove bits that were commented out, which is always a
good thing (history will always be in the git repo)


> diff --git a/src/app/views/settings/self_service.html.haml b/src/app/views/settings/self_service.html.haml
> index 2dedda4..5cd97ef 100644
> --- a/src/app/views/settings/self_service.html.haml
> +++ b/src/app/views/settings/self_service.html.haml
> @@ -17,115 +17,9 @@
>        %fieldset
>          = hidden_field :self_service_default_quota,  :id
>          %p
> -        %label
> -          =t 'instances.instances.other'
> -          = t('.quota') + ":"
> +        %label= t('.instances_quota')
>          = text_field :self_service_default_quota, :maximum_running_instances, :class => 'grid_5'
>        %fieldset.options
>          #obj_actions.button-group
>            = link_to t('reset'), self_service_settings_path, :class => 'button pill'
>            = submit_tag t(:save), :class => "button pill"
> -      -#%fieldset.clearfix
> -        %label.grid_2.alpha Permissions:
> -        .grid_4 My personal Instances
> -        .grid_4 Full Instance Permission
> -        .grid_3.omega
> -          (
> -          %a{:href=>'#'}> Edit
> -          )
> -          (
> -          %a{:href=>'#'}> Remove
> -          )
> -      -#%fieldset.clearfix.gap
> -        .grid_4.prefix_2.alpha
> -          %select.disabled{:disabled => true}
> -            %option
> -              Category of Instances
> -        .grid_4
> -          %ul.block
> -            %li
> -              %input{ :type => "radio", :name => "permission", :disabled => true}
> -              %label.disabled Control Instance - Can't Share
> -            %li
> -              %input{ :type => "radio", :name => "permission", :disabled => true}
> -              %label.disabled Control Instance - Can Share
> -            %li
> -              %input{ :type => "radio", :name => "permission", :disabled => true}
> -              %label.disabled Full Instance Permission
> -            %li
> -              %input{ :type => "radio", :name => "permission", :disabled => true}
> -              %label.disabled Custom ...
> -        .grid_3.omega
> -          (
> -          %a{:href=>'#'}> Add
> -          )
> -      -#%h3 POOLS
> -      -#%fieldset.clearfix
> -        %label.grid_2.alpha Permissions:
> -        .grid_4 General Usage
> -        .grid_4 Regular Pool User
> -        .grid_3.omega
> -          (
> -          %a{:href=>'#'}> Edit
> -          )
> -          (
> -          %a{:href=>'#'}> Remove
> -          )
> -      -#%fieldset.clearfix.gap
> -        .grid_4.prefix_2.alpha
> -          %select.disabled{:disabled => true}
> -            %option
> -              Category of Pools
> -        .grid_4
> -          %ul.block
> -            %li
> -              %input{ :type => "radio", :name => "pooltype", :disabled => true}
> -              %label.disabled Limited Pool User
> -            %li
> -              %input{ :type => "radio", :name => "pooltype", :disabled => true}
> -              %label.disabled Regular Pool User
> -            %li
> -              %input{ :type => "radio", :name => "pooltype", :disabled => true}
> -              %label.disabled Advanced Pool User
> -            %li
> -              %input{ :type => "radio", :name => "pooltype", :disabled => true}
> -              %label.disabled Custom ...
> -        .grid_3.omega
> -          (
> -          %a{:href=>'#'}> Add
> -          )
> -      -#%h3 TEMPLATES
> -      -#%fieldset.clearfix
> -        %label.grid_2.alpha Permissions:
> -        .grid_4 General Usage
> -        .grid_4 Basic Template User
> -        .grid_3.omega
> -          (
> -          %a{:href=>'#'}> Edit
> -          )
> -          (
> -          %a{:href=>'#'}> Remove
> -          )
> -      -#%fieldset.clearfix.gap
> -        .grid_4.prefix_2.alpha
> -          %select.disabled{:disabled => true}
> -            %option
> -              Category of Templates
> -        .grid_4
> -          %ul.block
> -            %li
> -              %input{ :type => "radio", :name => "pooltype", :disabled => true}
> -              %label.disabled Basic Template User
> -            %li
> -              %input{ :type => "radio", :name => "pooltype", :disabled => true}
> -              %label.disabled General Template User
> -            %li
> -              %input{ :type => "radio", :name => "pooltype", :disabled => true}
> -              %label.disabled Advanced Template User
> -            %li
> -              %input{ :type => "radio", :name => "pooltype", :disabled => true}
> -              %label.disabled Custom ...
> -        .grid_3.omega
> -          (
> -          %a{:href=>'#'}> Add
> -          )

Again +1 to removing old commented code.


> diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> index c6129ad..ae1738b 100644
> --- a/src/config/locales/en.yml
> +++ b/src/config/locales/en.yml
> @@ -940,38 +940,13 @@ en:
>    settings:
>      settings: Settings
>      index:
> -      general_settings: General Settings
> -      general_settings_desc: Enable Self-service Account Registration, Setup Custom Branding, Manage Content Sources
> -      define_locales: Define Locales
> -      define_locales_desc: View, edit and define the names of locales that Aeolus Conductor will expose to its users. The meaning of the names will be defined by being mapped to the geographical locations offered by individual providers.
> -      manage_users: Manage Users
> -      manage_users_desc: View, edit, create and delete user accounts on Aeolus Conductor. Adjust quotas and privileges for users.
> -      define_hardware_profiles: Define Hardware Profiles
> -      define_hardware_profiles_desc: View, edit and define the names of the hardware profiles that Aeolus Conductor will expose to its users. The meaning of the names will be defined by being mapped to capabilities offered by individual providers.
> -      manage_providers: Manage Providers
> -      manage_providers_desc: View, edit and add providers of virtual compute resources to Aeolus Conductor. View, edit and add Aeolus Conductor accounts for these providers. Map provider-specific concepts to Aeolus Conductor.
> -      define_services:  Define Services
> -      define_services_desc: View, edit and define services that Aeolus Conductor will offer. These services will be mapped to individual providers based on their capabilities.
>        permissions: Permissions
>        permissions_desc: Modify self-service default quota.
>      self_service:
>        self_service_default: 'User Treatment: Self-service default'
>        account: Account
> +      instances_quota: "Instances Quota:"
>        self_service_settings: "Self-service Settings"
> -    general_settings:
> -       general_settings: General Settings
> -       portal: Portal
> -       brand_icon: "Brand Icon:"
> -       tag_line: "Tag Line:"
> -       allow_self_service: "Allow Self Service Account Creation:"
> -       select_treatment: Select User Treatment
> -       primary_url_port: "Primary URL Port:"
> -       content: Content
> -       entitlement_mode: "Entitlement Mode:"
> -       user_brings_subscriptions: User Brings Subscriptions
> -       system_subscription: System Provides Subscription
> -       certificate: "Certitifcate:"
> -       content_manager_url: "Content Manager URL:"
>      flash:
>        warning:
>          not_updated_quota: "Could not update the default quota"

Should we be updating all the locales at the same time? Grepping for the
strings you removed, I noticed they still appear in other locales and it
might make sense to sync up changes to each of those.

You're also adding a new string here, does the project have a
requirement for doing so that they are added to all locales?

> diff --git a/src/config/routes.rb b/src/config/routes.rb
> index d24f039..7cef94d 100644
> --- a/src/config/routes.rb
> +++ b/src/config/routes.rb
> @@ -170,13 +170,6 @@ Conductor::Application.routes.draw do
>      delete 'multi_destroy', :on => :collection
>    end
>  
> -  resources :settings do
> -    collection do
> -      get 'self_service'
> -      get 'general_settings'
> -    end
> -  end
> -
>    resources :pool_families do
>      collection do
>        delete 'multi_destroy'

I notice you remove the self_service and general settings routes but
don't remove the controller actions. self_service seems to still work
but navigating to /conductor/settings/general_settings now results in a
rails missing templates error.

  -Mo




More information about the aeolus-devel mailing list