On 12/11/10 22:24 +0100, Michal Fojtik wrote: >On 10/11/10 15:06 +0000, [email protected] wrote: > >Hi Martyn, > >Just brief inline comments ;-)
ACKing (after IRC conversation ;-) -- Michal > >>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 -- -------------------------------------------------------- Michal Fojtik, [email protected] Deltacloud API: http://deltacloud.org -------------------------------------------------------- _______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
