tests for defaulting, validation of config entries

app/controllers/admin/settings_controller.rb

@@ -7,7 +7,7 @@ class Admin::SettingsController < ApplicationController
     :denied_message => 'You must have admin privileges to edit settings.'
 
   def index
-
+    redirect_to admin_configuration_url
   end
 
   def show
@@ -25,7 +25,7 @@ def edit
   end
 
   def update  
-    @setting.update_attributes!(:value, params[:setting][:value])
+    @setting.value = params[:config][:value]
     respond_to do |format|
       format.html { render :action => 'show' }
       format.js { render :layout => false, :action => 'show' }

app/models/radiant/config.rb

@@ -57,6 +57,7 @@ class Config < ActiveRecord::Base
     set_table_name "config"
     after_save :update_cache
     cattr_accessor :definitions
+    @@definitions = {}
 
     class ConfigError < RuntimeError; end
 
@@ -185,17 +186,28 @@ def namespace(prefix, options = {}, &block)
       #     end
       #   end
       #
+      # It's also possible to reuse a definition by passing it to define:
+      #
+      #   choose_layout = Radiant::Config::Definition.new(:select_from => lambda {Layout.all.map{|l| [l.name, l.d]}})
+      #   define "my.layout", choose_layout
+      #   define "your.layout", choose_layout
+      #
+      # but that's only used in testing at the moment.
+      #
       def define(key, options={})
-        key = [options[:prefix], key].join('.') if options[:prefix]
-        definitions[key] = Radiant::Config::Definition.new(options)
-        self[key] ||= options[:default]
+        if options.is_a? Radiant::Config::Definition
+          definitions[key] = options
+        else
+          key = [options[:prefix], key].join('.') if options[:prefix]
+          definitions[key] = Radiant::Config::Definition.new(options)
+        end
+        self[key] ||= definitions[key].default
       end
 
       # We makes sure that core settings.rb files are reloaded in dev mode by calling initialize_definitions
       # whenever read_configuration_files is called (as it will be whenever an extension reloads).
       #
       def initialize_definitions
-        @@definitions = {}
         read_configuration_file(RAILS_ROOT + '/config/settings.rb')
         read_configuration_file(RADIANT_ROOT + '/config/settings.rb') unless RADIANT_ROOT == RAILS_ROOT
       end
@@ -222,7 +234,8 @@ def value=(param)
         else
           self[:value] = newvalue
         end
-        self.save!
+        raise "ActiveRecord::RecordInvalid" unless self.valid?
+        self.save
       end
       self[:value]
     end
@@ -241,8 +254,6 @@ def value=(param)
     def value
       if boolean?
         checked?
-      elsif definition.type == :integer
-        self[:value].to_i
       else
         self[:value]
       end
@@ -284,7 +295,7 @@ def update_cache
       Radiant::Config.initialize_cache
     end
 
-    delegate :default, :type, :allow_blank?, :hidden?, :settable?, :selection, :to => :definition
+    delegate :default, :type, :allow_blank?, :hidden?, :settable?, :selection, :notes, :to => :definition
 
     def validate
       definition.validate(self)

app/views/admin/settings/edit.html.haml

@@ -0,0 +1 @@
+= edit_setting(@setting)

app/views/admin/settings/show.html.haml

@@ -0,0 +1,2 @@
+%p.inline
+  = show_setting(@setting)

db/schema.rb

@@ -69,10 +69,10 @@
     t.integer  "lock_version",                 :default => 0
   end
 
-  add_index "pages", ["class_name"], :name => "altered_pages_class_name"
-  add_index "pages", ["parent_id"], :name => "altered_pages_parent_id"
-  add_index "pages", ["slug", "parent_id"], :name => "altered_pages_child_slug"
-  add_index "pages", ["virtual", "status_id"], :name => "altered_pages_published"
+  add_index "pages", ["class_name"], :name => "pages_class_name"
+  add_index "pages", ["parent_id"], :name => "pages_parent_id"
+  add_index "pages", ["slug", "parent_id"], :name => "pages_child_slug"
+  add_index "pages", ["virtual", "status_id"], :name => "pages_published"
 
   create_table "sessions", :force => true do |t|
     t.string   "session_id"

features/admin/configuration.feature

@@ -0,0 +1,27 @@
+Feature: Rich configuration
+  In order to control site delivery
+  an administrator
+  wants to change the configuration of the site
+
+  Background:
+    Given I am logged in as "admin"
+    And I go to the "configuration" admin page
+
+  Scenario: Reviewing configuration
+    Then I should see a "Site configuration" heading
+		And I should see "Site title"
+		And I should see "Site domain"
+		And I should see the site title
+		And I should see the site domain    
+
+  Scenario: Editing configuration
+		When I press "Edit settings"
+		Then I should see a "Configuration" heading
+		And I should see "Site title"
+		And I should see "Timezone name"
+		When I fill in "Site title" with "Something else"
+		And I select "London" from "Timezone name"
+		And I press "Save Changes"
+		Then I should see "Site configuration"
+		And I should see "Something Else"
+

features/support/paths.rb

@@ -41,6 +41,8 @@ def path_to(page_name, format=nil)
       admin_snippets_path(:format => format)
     when /users/i
       admin_users_path(:format => format)
+    when /configuration/i
+      admin_configuration_path(:format => format)
     when /extensions/i
       admin_extensions_path(:format => format)
     when /export/i

lib/radiant/config/definition.rb

@@ -1,7 +1,7 @@
 module Radiant
   class Config
     class Definition
-      attr_reader :default, :type, :label, :notes, :validate_with, :select_from, :allow_blank, :allow_display, :allow_change, :error_message
+      attr_reader :default, :type, :label, :notes, :validate_with, :select_from, :allow_blank, :allow_display, :allow_change
 
       # Configuration 'definitions' are metadata held in memory that add restriction and description to individual config entries.
       #
@@ -22,7 +22,7 @@ class Definition
       # See the method documentation in Radiant::Config for options and conventions.
       #
       def initialize(options={})
-        [:default, :type, :label, :validate_with, :select_from, :allow_blank, :allow_change, :allow_display, :error_message].each do |attribute|
+        [:default, :type, :label, :notes, :validate_with, :select_from, :allow_blank, :allow_change, :allow_display].each do |attribute|
           instance_variable_set "@#{attribute}".to_sym, options[attribute]
         end
       end
@@ -82,17 +82,19 @@ def selected(value)
       # * if :allow_blank has been set to false, we test that the value is not blank
       #
       def validate(setting)
-        if validate_with.is_a? Proc
-          setting.errors.add :value, error_message unless validate_with.call(setting.value)
-        end
-        if !allow_blank?
+        if allow_blank?
+          return if setting.value.blank?
+        else
           setting.errors.add :value, "must not be blank" if setting.value.blank?
         end
+        if validate_with.is_a? Proc
+          validate_with.call(setting)
+        end
         if selector?
           setting.errors.add :value, "must choose one of the permitted values" unless selectable?(setting.value)
         end
         if integer?
-          setting.errors.add :value, "must be a number" unless setting.value =~ /\A-?\d*\Z/
+          Integer(setting.value) rescue setting.errors.add :value, "must be a number"
         end
       end