On 10/11/10 15:06 +0000, [email protected] wrote:

Hi Martyn,

Just brief inline comments ;-)

>From: Martyn Taylor <[email protected]>
>
>---
> 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
>[email protected]
>https://fedorahosted.org/mailman/listinfo/deltacloud-devel

-- 
--------------------------------------------------------
Michal Fojtik, [email protected]
Deltacloud API: http://deltacloud.org
--------------------------------------------------------
_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel

Reply via email to