[PATCH] BZ#644283 Added validation check on format of pool name

[Martyn Taylor]

From: Martyn Taylor

---
 src/app/controllers/pools_controller.rb |   10 ++++++----
 src/app/models/pool.rb                  |    2 ++
 src/app/views/pools/new.haml            |    6 +++---
 src/features/pool.feature               |    7 +++++++
 4 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/app/controllers/pools_controller.rb b/src/app/controllers/pools_controller.rb
index 1fdc047..2ba6940 100644
--- a/src/app/controllers/pools_controller.rb
+++ b/src/app/controllers/pools_controller.rb
@@ -103,10 +103,12 @@ class PoolsController < ApplicationController
     @pool.quota_id = quota.id

@pool.zone = Zone.default
-    @pool.save!
-
-    flash[:notice] = "Pool added."
-    redirect_to :action => 'show', :id => @pool.id
+    if @pool.save
+      flash[:notice] = "Pool added."
+      redirect_to :action => 'show', :id => @pool.id
+    else
+      render :action => :new
+    end
   end

def manage_pool
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index cb03ddf..597128b 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -34,6 +34,8 @@ class Pool < ActiveRecord::Base
   validates_presence_of :zone
   validates_uniqueness_of :exported_as, :if => :exported_as

+  validates_format_of :name, :with => /^[\w -]*$/n, :message => "must only contain: numbers, letters, spaces, '_' and '-'"
+
   has_many :permissions, :as => :permission_object, :dependent => :destroy,
            :include => [:role],
            :order => "permissions.id ASC"
diff --git a/src/app/views/pools/new.haml b/src/app/views/pools/new.haml
index 3b67b31..037c7c4 100644
--- a/src/app/views/pools/new.haml
+++ b/src/app/views/pools/new.haml
@@ -1,12 +1,12 @@
 .grid_16
-  = error_messages_for 'pool'
   = error_messages_for 'account'
   %h1 Create a new Pool
-  - form_tag :action => 'create' do
+  - form_for @pool, :action => 'create' do |form|
     %h2 Pool
     %fieldset
       %label.grid_2.alpha Name:
-      = text_field :pool, :name, :class => "grid_5"
+      = form.text_field :name, :class => "grid_5"
+      = form.error_message_on :name, 'Name '
       .clear.grid_14.prefix_2.alpha Provide a descriptive name for this pool.

= submit_tag "Save", :class => "submit formbutton"
diff --git a/src/features/pool.feature b/src/features/pool.feature
index 15abe09..b85b930 100644
--- a/src/features/pool.feature
+++ b/src/features/pool.feature
@@ -62,3 +62,10 @@ Feature: Manage Pools
     Then I should see the following:
     | Running Instances | 10           | 8             |
     | Total Instances   | 15           | 15            |
+
+  Scenario: Enter invalid characters into Name field
+    Given I am an authorised user
+    And I am on the new pool page
+    When I fill in "pool[name]" with "@%&*())_@!#!"
+    And I press "Save"
+    Then I should see "Name must only contain: numbers, letters, spaces, '_' and '-'"
\ No newline at end of file

[Michal Fojtik]

On 10/11/10 15:06 +0000, [e-mail] wrote:

Hi Martyn,

Just brief inline comments ;-)
> From: Martyn Taylor
>
> src/app/controllers/pools_controller.rb |   10 ++++++----
> src/app/models/pool.rb                  |    2 ++
> src/app/views/pools/new.haml            |    6 +++---
> src/features/pool.feature               |    7 +++++++
> 4 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/src/app/controllers/pools_controller.rb b/src/app/controllers/pools_controller.rb
> index 1fdc047..2ba6940 100644
> --- a/src/app/controllers/pools_controller.rb
> +++ b/src/app/controllers/pools_controller.rb
> @@ -103,10 +103,12 @@ class PoolsController < ApplicationController
>     @pool.quota_id = quota.id
> @pool.zone = Zone.default
> @pool.save!
> flash[:notice] = "Pool added."
> redirect_to :action => 'show', :id => @pool.id
> if @pool.save
>  flash[:notice] = "Pool added."
>  redirect_to :action => 'show', :id => @pool.id

Better to use pool_url(@pool.id) (route.rb: map.resources :pool)
Btw. there is no need to use '@' IMHO, because you are not passing this
variable to view.
> else
>  render :action => :new
>
> end
> end
>
> def manage_pool
>
> diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
> index cb03ddf..597128b 100644
> --- a/src/app/models/pool.rb
> +++ b/src/app/models/pool.rb
> @@ -34,6 +34,8 @@ class Pool < ActiveRecord::Base
>   validates_presence_of :zone
>   validates_uniqueness_of :exported_as, :if => :exported_as
> validates_format_of :name, :with => /^[\w -]*$/n, :message => "must only contain: numbers, letters, spaces, '_' and '-'"
> has_many :permissions, :as => :permission_object, :dependent => :destroy,
>          :include => [:role],
>          :order => "permissions.id ASC"
>
> diff --git a/src/app/views/pools/new.haml b/src/app/views/pools/new.haml
> index 3b67b31..037c7c4 100644
> --- a/src/app/views/pools/new.haml
> +++ b/src/app/views/pools/new.haml
> @@ -1,12 +1,12 @@
> .grid_16
> = error_messages_for 'pool'
> = error_messages_for 'account'
> %h1 Create a new Pool
> form_tag :action => 'create' do
> form_for @pool, :action => 'create' do |form|

AFAIK, no need for 'action' here. By default Rails will use 'POST create'
as an action.
> %h2 Pool
> %fieldset
>   %label.grid_2.alpha Name:
>  = text_field :pool, :name, :class => "grid_5"
>  = form.text_field :name, :class => "grid_5"
>  = form.error_message_on :name, 'Name '
> .clear.grid_14.prefix_2.alpha Provide a descriptive name for this pool.
>
> = submit_tag "Save", :class => "submit formbutton"
>
> diff --git a/src/features/pool.feature b/src/features/pool.feature
> index 15abe09..b85b930 100644
> --- a/src/features/pool.feature
> +++ b/src/features/pool.feature
> @@ -62,3 +62,10 @@ Feature: Manage Pools
>     Then I should see the following:
>     | Running Instances | 10           | 8             |
>     | Total Instances   | 15           | 15            |
> Scenario: Enter invalid characters into Name field
> Given I am an authorised user
> And I am on the new pool page
> When I fill in "pool[name]" with "@%&*())_@!#!"
> And I press "Save"
> Then I should see "Name must only contain: numbers, letters, spaces, '_' and '-'"
> \ No newline at end of file
>
> 1.7.2.3
>
> deltacloud-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/deltacloud-devel

[Michal Fojtik]

On 12/11/10 22:24 +0100, Michal Fojtik wrote:
> On 10/11/10 15:06 +0000, [e-mail] wrote:
>
> Hi Martyn,
>
> Just brief inline comments ;-)

ACKing (after IRC conversation ;-)

-- Michal
> > From: Martyn Taylor
> >
> > src/app/controllers/pools_controller.rb |   10 ++++++----
> > src/app/models/pool.rb                  |    2 ++
> > src/app/views/pools/new.haml            |    6 +++---
> > src/features/pool.feature               |    7 +++++++
> > 4 files changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/app/controllers/pools_controller.rb b/src/app/controllers/pools_controller.rb
> > index 1fdc047..2ba6940 100644
> > --- a/src/app/controllers/pools_controller.rb
> > +++ b/src/app/controllers/pools_controller.rb
> > @@ -103,10 +103,12 @@ class PoolsController < ApplicationController
> >     @pool.quota_id = quota.id
> > @pool.zone = Zone.default
> > @pool.save!
> > flash[:notice] = "Pool added."
> > redirect_to :action => 'show', :id => @pool.id
> > if @pool.save
> >  flash[:notice] = "Pool added."
> >  redirect_to :action => 'show', :id => @pool.id
>
> Better to use pool_url(@pool.id) (route.rb: map.resources :pool)
> Btw. there is no need to use '@' IMHO, because you are not passing this
> variable to view.
> > else
> >  render :action => :new
> >
> > end
> > end
> >
> > def manage_pool
> >
> > diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
> > index cb03ddf..597128b 100644
> > --- a/src/app/models/pool.rb
> > +++ b/src/app/models/pool.rb
> > @@ -34,6 +34,8 @@ class Pool < ActiveRecord::Base
> >   validates_presence_of :zone
> >   validates_uniqueness_of :exported_as, :if => :exported_as
> > validates_format_of :name, :with => /^[\w -]*$/n, :message => "must only contain: numbers, letters, spaces, '_' and '-'"
> > has_many :permissions, :as => :permission_object, :dependent => :destroy,
> >          :include => [:role],
> >          :order => "permissions.id ASC"
> >
> > diff --git a/src/app/views/pools/new.haml b/src/app/views/pools/new.haml
> > index 3b67b31..037c7c4 100644
> > --- a/src/app/views/pools/new.haml
> > +++ b/src/app/views/pools/new.haml
> > @@ -1,12 +1,12 @@
> > .grid_16
> > = error_messages_for 'pool'
> > = error_messages_for 'account'
> > %h1 Create a new Pool
> > form_tag :action => 'create' do
> > form_for @pool, :action => 'create' do |form|
>
> AFAIK, no need for 'action' here. By default Rails will use 'POST create'
> as an action.
> > %h2 Pool
> > %fieldset
> >   %label.grid_2.alpha Name:
> >  = text_field :pool, :name, :class => "grid_5"
> >  = form.text_field :name, :class => "grid_5"
> >  = form.error_message_on :name, 'Name '
> > .clear.grid_14.prefix_2.alpha Provide a descriptive name for this pool.
> >
> > = submit_tag "Save", :class => "submit formbutton"
> >
> > diff --git a/src/features/pool.feature b/src/features/pool.feature
> > index 15abe09..b85b930 100644
> > --- a/src/features/pool.feature
> > +++ b/src/features/pool.feature
> > @@ -62,3 +62,10 @@ Feature: Manage Pools
> >     Then I should see the following:
> >     | Running Instances | 10           | 8             |
> >     | Total Instances   | 15           | 15            |
> > Scenario: Enter invalid characters into Name field
> > Given I am an authorised user
> > And I am on the new pool page
> > When I fill in "pool[name]" with "@%&*())_@!#!"
> > And I press "Save"
> > Then I should see "Name must only contain: numbers, letters, spaces, '_' and '-'"
> > \ No newline at end of file
> >
> > 1.7.2.3
> >
> > deltacloud-devel mailing list
> > [e-mail]
> > https://fedorahosted.org/mailman/listinfo/deltacloud-devel
> --
>
> Michal Fojtik, [e-mail]
> Deltacloud API: http://deltacloud.org
>
> deltacloud-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/deltacloud-devel