[PATCH aggregator] BZ#640192 Fixed Validation on Quota Fields

[Martyn Taylor]

From: martyntaylor <mtaylor(a)redhat.com&gt;

---
 src/app/controllers/settings_controller.rb      |    8 +++-
 src/app/models/quota.rb                         |   10 +++--
 src/app/views/settings/self_service.haml        |    2 +-
 src/features/settings.feature                   |   44 +++++++++++++++++++++++
 src/features/step_definitions/settings_steps.rb |   10 +++++
 src/features/support/paths.rb                   |    6 +++
 6 files changed, 73 insertions(+), 7 deletions(-)
 create mode 100644 src/features/settings.feature
 create mode 100644 src/features/step_definitions/settings_steps.rb

diff --git a/src/app/controllers/settings_controller.rb
b/src/app/controllers/settings_controller.rb
index b040f7f..ae20e12 100644
--- a/src/app/controllers/settings_controller.rb
+++ b/src/app/controllers/settings_controller.rb
@@ -53,8 +53,12 @@ class SettingsController < ApplicationController
    KEYS.each do |key|
      if params[key]
        if key == SELF_SERVICE_DEFAULT_QUOTA
-         self_service_default_quota = MetadataObject.lookup(key)
-         self_service_default_quota.update_attributes(params[key])
+         @self_service_default_quota = MetadataObject.lookup(key)
+         if !(a)self_service_default_quota.update_attributes(params[key])
+           flash[:notice] = "Could not update the default quota"
+           render :self_service
+           return
+         end
        elsif key == SELF_SERVICE_DEFAULT_POOL
          if Pool.exists?(params[key])
            MetadataObject.set(key, Pool.find(params[key]))
diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
index 83c20c4..cfc4555 100644
--- a/src/app/models/quota.rb
+++ b/src/app/models/quota.rb
@@ -28,14 +28,16 @@ class Quota < ActiveRecord::Base
   validates_numericality_of :maximum_total_instances,
                             :greater_than_or_equal_to => 0,
                             :less_than_or_equal_to => 2147483647,
-                            :integer_only => true,
-                            :allow_nil => true
+                            :only_integer => true,
+                            :allow_nil => true,
+                            :message => "must be a positive whole number less
than 2147483647"
 
   validates_numericality_of :maximum_running_instances,
                             :greater_than_or_equal_to => 0,
                             :less_than_or_equal_to => 2147483647,
-                            :integer_only => true,
-                            :allow_nil => true
+                            :only_integer => true,
+                            :allow_nil => true,
+                            :message => "must be a positive whole number less
than 2147483647"
 
   QuotaResource = Struct.new(:name, :used, :max, :available, :unit)
 
diff --git a/src/app/views/settings/self_service.haml
b/src/app/views/settings/self_service.haml
index c1da0ae..8fd9950 100644
--- a/src/app/views/settings/self_service.haml
+++ b/src/app/views/settings/self_service.haml
@@ -8,7 +8,6 @@
 
 .grid_13
   = error_messages_for @parent_type
-  = error_messages_for 'self_service_default_quota'
   %h2
     = t('.self_service_default')
   - form_for @self_service_default_quota, :url => { :action => 'update' }
do |form|
@@ -53,6 +52,7 @@
         = t('.quota') + ":"
       = text_field :self_service_default_quota, :maximum_running_instances, :class =>
'grid_5'
       .grid_2.la (instances)
+      = form.error_message_on :maximum_running_instances, 'Maximum Running Instances
'
     %h3 POOLS
     %fieldset.clearfix
       %label.grid_2.alpha Permissions:
diff --git a/src/features/settings.feature b/src/features/settings.feature
new file mode 100644
index 0000000..f41e188
--- /dev/null
+++ b/src/features/settings.feature
@@ -0,0 +1,44 @@
+Feature: Manage System wide Settings
+  In order to manage my cloud engine
+  As a user
+  I want to manage system settings
+
+  Background:
+  Given I am an authorised user
+  And I am logged in
+
+  Scenario: Change the self service default quota
+    Given the default quota is set to 5
+    And I am on the self service settings page
+    When I fill in "self_service_default_quota[maximum_running_instances]" with
"8"
+    And I press "Save"
+    Then I should see "Settings Updated!"
+    And the default quota should be 8
+    And I should be on the self service settings page
+
+  Scenario: Invalid decimal entry for the self service default quota
+    Given the default quota is set to 5
+    And I am on the self service settings page
+    When I fill in "self_service_default_quota[maximum_running_instances]" with
"1.5"
+    And I press "Save"
+    Then I should see "Could not update the default quota"
+    And the default quota should be 5
+    And I should be on the settings update page
+
+  Scenario: Invalid chars entry for the self service default quota
+    Given the default quota is set to 5
+    And I am on the self service settings page
+    When I fill in "self_service_default_quota[maximum_running_instances]" with
"abc"
+    And I press "Save"
+    Then I should see "Could not update the default quota"
+    And the default quota should be 5
+    And I should be on the settings update page
+
+  Scenario: Invalid special chars entry for the self service default quota
+    Given the default quota is set to 5
+    And I am on the self service settings page
+    When I fill in "self_service_default_quota[maximum_running_instances]" with
"^&(*_!"
+    And I press "Save"
+    Then I should see "Could not update the default quota"
+    And the default quota should be 5
+    And I should be on the settings update page
diff --git a/src/features/step_definitions/settings_steps.rb
b/src/features/step_definitions/settings_steps.rb
new file mode 100644
index 0000000..7b1a7d7
--- /dev/null
+++ b/src/features/step_definitions/settings_steps.rb
@@ -0,0 +1,10 @@
+Given /^the default quota is set to (\d+)$/ do |no_instances|
+  @default_quota = MetadataObject.lookup("self_service_default_quota")
+  @default_quota.maximum_running_instances = no_instances
+  @default_quota.save
+end
+
+Then /^the default quota should be (\d+)$/ do |no_instances|
+  @default_quota.reload
+  @default_quota.maximum_running_instances.should == no_instances.to_i
+end
\ No newline at end of file
diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb
index b0a5f7e..fdc63f5 100644
--- a/src/features/support/paths.rb
+++ b/src/features/support/paths.rb
@@ -74,6 +74,12 @@ module NavigationHelpers
     when /the create template page/
       url_for :action => 'create', :controller => 'templates',
:only_path => true
 
+    when /the self service settings page/
+      url_for :action => 'self_service', :controller => 'settings',
:only_path => true
+
+    when /the settings update page/
+      url_for :action => 'update', :controller => 'settings',
:only_path => true
+
     # Add more mappings here.
     # Here is an example that pulls values out of the Regexp:
     #
-- 
1.7.2.3

[Tomas Sedovic]

ACK

On 10/27/2010 11:28 AM, mtaylor(a)redhat.com wrote:
>  From: martyntaylor<mtaylor(a)redhat.com>
>
>  ---
>    src/app/controllers/settings_controller.rb      |    8 +++-
>    src/app/models/quota.rb                         |   10 +++--
>    src/app/views/settings/self_service.haml        |    2 +-
>    src/features/settings.feature                   |   44 +++++++++++++++++++++++
>    src/features/step_definitions/settings_steps.rb |   10 +++++
>    src/features/support/paths.rb                   |    6 +++
>    6 files changed, 73 insertions(+), 7 deletions(-)
>    create mode 100644 src/features/settings.feature
>    create mode 100644 src/features/step_definitions/settings_steps.rb
>
>  diff --git a/src/app/controllers/settings_controller.rb
> b/src/app/controllers/settings_controller.rb
>  index b040f7f..ae20e12 100644
>  --- a/src/app/controllers/settings_controller.rb
>  +++ b/src/app/controllers/settings_controller.rb
>  @@ -53,8 +53,12 @@ class SettingsController<  ApplicationController
>       KEYS.each do |key|
>         if params[key]
>           if key == SELF_SERVICE_DEFAULT_QUOTA
>  -         self_service_default_quota = MetadataObject.lookup(key)
>  -         self_service_default_quota.update_attributes(params[key])
>  +         @self_service_default_quota = MetadataObject.lookup(key)
>  +         if !(a)self_service_default_quota.update_attributes(params[key])
>  +           flash[:notice] = "Could not update the default quota"
>  +           render :self_service
>  +           return
>  +         end
>           elsif key == SELF_SERVICE_DEFAULT_POOL
>             if Pool.exists?(params[key])
>               MetadataObject.set(key, Pool.find(params[key]))
>  diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
>  index 83c20c4..cfc4555 100644
>  --- a/src/app/models/quota.rb
>  +++ b/src/app/models/quota.rb
>  @@ -28,14 +28,16 @@ class Quota<  ActiveRecord::Base
>      validates_numericality_of :maximum_total_instances,
>                                :greater_than_or_equal_to =>  0,
>                                :less_than_or_equal_to =>  2147483647,
>  -                            :integer_only =>  true,
>  -                            :allow_nil =>  true
>  +                            :only_integer =>  true,
>  +                            :allow_nil =>  true,
>  +                            :message =>  "must be a positive whole number less
> than 2147483647"
>
>      validates_numericality_of :maximum_running_instances,
>                                :greater_than_or_equal_to =>  0,
>                                :less_than_or_equal_to =>  2147483647,
>  -                            :integer_only =>  true,
>  -                            :allow_nil =>  true
>  +                            :only_integer =>  true,
>  +                            :allow_nil =>  true,
>  +                            :message =>  "must be a positive whole number less
> than 2147483647"
>
>      QuotaResource = Struct.new(:name, :used, :max, :available, :unit)
>
>  diff --git a/src/app/views/settings/self_service.haml
> b/src/app/views/settings/self_service.haml
>  index c1da0ae..8fd9950 100644
>  --- a/src/app/views/settings/self_service.haml
>  +++ b/src/app/views/settings/self_service.haml
>  @@ -8,7 +8,6 @@
>
>    .grid_13
>      = error_messages_for @parent_type
>  -  = error_messages_for 'self_service_default_quota'
>      %h2
>        = t('.self_service_default')
>      - form_for @self_service_default_quota, :url =>  { :action =>  'update'
> } do |form|
>  @@ -53,6 +52,7 @@
>            = t('.quota') + ":"
>          = text_field :self_service_default_quota, :maximum_running_instances, :class
> =>  'grid_5'
>          .grid_2.la (instances)
>  +      = form.error_message_on :maximum_running_instances, 'Maximum Running Instances
> '
>        %h3 POOLS
>        %fieldset.clearfix
>          %label.grid_2.alpha Permissions:
>  diff --git a/src/features/settings.feature b/src/features/settings.feature
>  new file mode 100644
>  index 0000000..f41e188
>  --- /dev/null
>  +++ b/src/features/settings.feature
>  @@ -0,0 +1,44 @@
>  +Feature: Manage System wide Settings
>  +  In order to manage my cloud engine
>  +  As a user
>  +  I want to manage system settings
>  +
>  +  Background:
>  +  Given I am an authorised user
>  +  And I am logged in
>  +
>  +  Scenario: Change the self service default quota
>  +    Given the default quota is set to 5
>  +    And I am on the self service settings page
>  +    When I fill in "self_service_default_quota[maximum_running_instances]"
> with "8"
>  +    And I press "Save"
>  +    Then I should see "Settings Updated!"
>  +    And the default quota should be 8
>  +    And I should be on the self service settings page
>  +
>  +  Scenario: Invalid decimal entry for the self service default quota
>  +    Given the default quota is set to 5
>  +    And I am on the self service settings page
>  +    When I fill in "self_service_default_quota[maximum_running_instances]"
> with "1.5"
>  +    And I press "Save"
>  +    Then I should see "Could not update the default quota"
>  +    And the default quota should be 5
>  +    And I should be on the settings update page
>  +
>  +  Scenario: Invalid chars entry for the self service default quota
>  +    Given the default quota is set to 5
>  +    And I am on the self service settings page
>  +    When I fill in "self_service_default_quota[maximum_running_instances]"
> with "abc"
>  +    And I press "Save"
>  +    Then I should see "Could not update the default quota"
>  +    And the default quota should be 5
>  +    And I should be on the settings update page
>  +
>  +  Scenario: Invalid special chars entry for the self service default quota
>  +    Given the default quota is set to 5
>  +    And I am on the self service settings page
>  +    When I fill in "self_service_default_quota[maximum_running_instances]"
> with "^&(*_!"
>  +    And I press "Save"
>  +    Then I should see "Could not update the default quota"
>  +    And the default quota should be 5
>  +    And I should be on the settings update page
>  diff --git a/src/features/step_definitions/settings_steps.rb
> b/src/features/step_definitions/settings_steps.rb
>  new file mode 100644
>  index 0000000..7b1a7d7
>  --- /dev/null
>  +++ b/src/features/step_definitions/settings_steps.rb
>  @@ -0,0 +1,10 @@
>  +Given /^the default quota is set to (\d+)$/ do |no_instances|
>  +  @default_quota = MetadataObject.lookup("self_service_default_quota")
>  +  @default_quota.maximum_running_instances = no_instances
>  +  @default_quota.save
>  +end
>  +
>  +Then /^the default quota should be (\d+)$/ do |no_instances|
>  +  @default_quota.reload
>  +  @default_quota.maximum_running_instances.should == no_instances.to_i
>  +end
>  \ No newline at end of file
>  diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb
>  index b0a5f7e..fdc63f5 100644
>  --- a/src/features/support/paths.rb
>  +++ b/src/features/support/paths.rb
>  @@ -74,6 +74,12 @@ module NavigationHelpers
>        when /the create template page/
>          url_for :action =>  'create', :controller =>  'templates',
> :only_path =>  true
>
>  +    when /the self service settings page/
>  +      url_for :action =>  'self_service', :controller => 
> 'settings', :only_path =>  true
>  +
>  +    when /the settings update page/
>  +      url_for :action =>  'update', :controller =>  'settings',
> :only_path =>  true
>  +
>        # Add more mappings here.
>        # Here is an example that pulls values out of the Regexp:
>        #