From 630bfcc823494b13a2c01ace49ad3471a79e4fc2 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:01:15 +1200 Subject: [PATCH 01/18] MINOR error_reporting() now defaults to E_ALL | E_STRICT which means strict errors are now reported *unless* the site is in live mode (which supresses everything except fatal errors and warnings) --- core/Core.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/core/Core.php b/core/Core.php index fde24dbd5..aaf5a030d 100644 --- a/core/Core.php +++ b/core/Core.php @@ -38,8 +38,9 @@ /////////////////////////////////////////////////////////////////////////////// // ENVIRONMENT CONFIG -if(defined('E_DEPRECATED')) error_reporting(E_ALL & ~(E_STRICT)); -else error_reporting(E_ALL); +// ALL errors are reported, including E_STRICT by default *unless* the site is in +// live mode, where reporting is limited to fatal errors and warnings (see later in this file) +error_reporting(E_ALL | E_STRICT); /** * Include _ss_environment.php files @@ -253,13 +254,11 @@ SS_TemplateLoader::instance()->pushManifest(new SS_TemplateManifest( BASE_PATH, false, isset($_GET['flush']) )); -// If this is a dev site, enable php error reporting -// This is necessary to force developers to acknowledge and fix -// notice level errors (you can override this directive in your _config.php) -if (Director::isLive()) { - if(defined('E_DEPRECATED')) error_reporting(E_ALL & ~(E_DEPRECATED | E_STRICT | E_NOTICE)); - else error_reporting(E_ALL & ~E_NOTICE); +// If in live mode, ensure deprecation, strict and notices are not reported +if(Director::isLive()) { + error_reporting(E_ALL & ~(E_DEPRECATED | E_STRICT | E_NOTICE)); } + /////////////////////////////////////////////////////////////////////////////// // POST-MANIFEST COMMANDS From 4c1aba8542d3b7f69fc850aaf54add7c7d10dd26 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:02:13 +1200 Subject: [PATCH 02/18] BUGFIX Object::get_extensions() is now declared as static, as it was never an instance method --- core/Object.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Object.php b/core/Object.php index 4051da256..22532ea9f 100755 --- a/core/Object.php +++ b/core/Object.php @@ -531,7 +531,7 @@ abstract class Object { * @return array Numeric array of either {@link DataExtension} classnames, * or eval'ed classname strings with constructor arguments. */ - function get_extensions($class, $includeArgumentString = false) { + public static function get_extensions($class, $includeArgumentString = false) { $extensions = Config::inst()->get($class, 'extensions'); if($includeArgumentString) { From a9f95051bc902a6722a84b7a5a10aa9ce11d23c2 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:03:33 +1200 Subject: [PATCH 03/18] BUGFIX File::ini2bytes() is now declared as static, as it was never an instance method --- filesystem/File.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filesystem/File.php b/filesystem/File.php index 9b0ad6458..8e9f4ba5a 100644 --- a/filesystem/File.php +++ b/filesystem/File.php @@ -796,7 +796,7 @@ class File extends DataObject { * @param string $phpIniValue * @return int */ - public function ini2bytes($PHPiniValue) { + public static function ini2bytes($PHPiniValue) { switch(strtolower(substr(trim($PHPiniValue), -1))) { case 'g': $PHPiniValue *= 1024; From 9bd7068394129a4f65cbe8373ab2ac1a32d95fbb Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:04:58 +1200 Subject: [PATCH 04/18] MINOR Fixed class documentation for CompositeDBField --- model/fieldtypes/CompositeDBField.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/model/fieldtypes/CompositeDBField.php b/model/fieldtypes/CompositeDBField.php index b1aa39055..b1d48f57a 100644 --- a/model/fieldtypes/CompositeDBField.php +++ b/model/fieldtypes/CompositeDBField.php @@ -7,7 +7,7 @@ * * Example with a combined street name and number: * -* class Street extends DBFields implements CompositeDBField() { +* class Street extends DBField implements CompositeDBField { * protected $streetNumber; * protected $streetName; * protected $isChanged = false; @@ -42,7 +42,7 @@ * } * * function setValue($value, $record = null, $markChanged=true) { -* if ($value instanceof Street && $value->hasValue()) { +* if ($value instanceof Street && $value->exists()) { * $this->setStreetName($value->getStreetName(), $markChanged); * $this->setStreetNumber($value->getStreetNumber(), $markChanged); * if($markChanged) $this->isChanged = true; @@ -85,7 +85,7 @@ * return $this->isChanged; * } * -* function hasValue() { +* function exists() { * return ($this->getStreetName() || $this->getStreetNumber()); * } * } @@ -179,4 +179,4 @@ interface CompositeDBField { */ function exists(); -} \ No newline at end of file +} From 41433f12115f8732105692138b682ff2f1e4329e Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:06:02 +1200 Subject: [PATCH 05/18] BUGFIX Fixing FulltextSearchable and Hierarchy to conform to the parent DataExtension for E_STRICT compliance. --- model/Hierarchy.php | 2 +- search/FulltextSearchable.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/model/Hierarchy.php b/model/Hierarchy.php index b8e988e3f..78e3f934b 100644 --- a/model/Hierarchy.php +++ b/model/Hierarchy.php @@ -25,7 +25,7 @@ class Hierarchy extends DataExtension { function augmentWrite(&$manipulation) { } - static function add_to_class($class, $extensionClass, $args) { + static function add_to_class($class, $extensionClass, $args = null) { Config::inst()->update($class, 'has_one', array('Parent' => $class)); parent::add_to_class($class, $extensionClass, $args); } diff --git a/search/FulltextSearchable.php b/search/FulltextSearchable.php index a69c13436..d3449c444 100644 --- a/search/FulltextSearchable.php +++ b/search/FulltextSearchable.php @@ -75,7 +75,7 @@ class FulltextSearchable extends DataExtension { parent::__construct(); } - static function add_to_class($class, $extensionClass, $args) { + static function add_to_class($class, $extensionClass, $args = null) { Config::inst()->update($class, 'indexes', array('SearchFields' => array( 'type' => 'fulltext', 'name' => 'SearchFields', @@ -90,7 +90,7 @@ class FulltextSearchable extends DataExtension { * * @return Array */ - function get_searchable_classes() { + static function get_searchable_classes() { return self::$searchable_classes; } From 972dfee5c32655934a9c99336872dea4d0fa9aa9 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:06:47 +1200 Subject: [PATCH 06/18] MINOR Fixing E_STRICT compliance in simpletest --- thirdparty/simpletest/compatibility.php | 4 ++-- thirdparty/simpletest/form.php | 4 ++-- thirdparty/simpletest/page.php | 2 +- thirdparty/simpletest/parser.php | 6 +++--- thirdparty/simpletest/tag.php | 6 +++--- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/thirdparty/simpletest/compatibility.php b/thirdparty/simpletest/compatibility.php index 4e0f78a4b..31f31b7df 100644 --- a/thirdparty/simpletest/compatibility.php +++ b/thirdparty/simpletest/compatibility.php @@ -19,7 +19,7 @@ class SimpleTestCompatibility { * @access public * @static */ - function copy($object) { + static function copy($object) { if (version_compare(phpversion(), '5') >= 0) { eval('$copy = clone $object;'); return $copy; @@ -170,4 +170,4 @@ class SimpleTestCompatibility { } } } -?> \ No newline at end of file +?> diff --git a/thirdparty/simpletest/form.php b/thirdparty/simpletest/form.php index b359552f4..be85e2e01 100644 --- a/thirdparty/simpletest/form.php +++ b/thirdparty/simpletest/form.php @@ -120,7 +120,7 @@ class SimpleForm { $class = $this->_encoding; $encoding = new $class(); for ($i = 0, $count = count($this->_widgets); $i < $count; $i++) { - $this->_widgets[$i]->write($encoding); + $this->_widgets[$i]->write($encoding, 0, 0); } return $encoding; } @@ -305,7 +305,7 @@ class SimpleForm { foreach ($this->_buttons as $button) { if ($selector->isMatch($button)) { $encoding = $this->_encode(); - $button->write($encoding); + $button->write($encoding, 0, 0); if ($additional) { $encoding->merge($additional); } diff --git a/thirdparty/simpletest/page.php b/thirdparty/simpletest/page.php index 6c037626c..54b26acfd 100644 --- a/thirdparty/simpletest/page.php +++ b/thirdparty/simpletest/page.php @@ -980,4 +980,4 @@ class SimplePage { return null; } } -?> + diff --git a/thirdparty/simpletest/parser.php b/thirdparty/simpletest/parser.php index 93f8cf980..35616d717 100644 --- a/thirdparty/simpletest/parser.php +++ b/thirdparty/simpletest/parser.php @@ -690,7 +690,7 @@ class SimpleHtmlSaxParser { * @access public * @static */ - function decodeHtml($html) { + static function decodeHtml($html) { return html_entity_decode($html, ENT_QUOTES); } @@ -703,7 +703,7 @@ class SimpleHtmlSaxParser { * @access public * @static */ - function normalise($html) { + static function normalise($html) { $text = preg_replace('||', '', $html); $text = preg_replace('|]*>.*?|', '', $text); $text = preg_replace('|]*alt\s*=\s*"([^"]*)"[^>]*>|', ' \1 ', $text); @@ -761,4 +761,4 @@ class SimpleSaxListener { function addContent($text) { } } -?> + diff --git a/thirdparty/simpletest/tag.php b/thirdparty/simpletest/tag.php index 7bccae205..b7749db03 100644 --- a/thirdparty/simpletest/tag.php +++ b/thirdparty/simpletest/tag.php @@ -325,7 +325,7 @@ class SimpleWidget extends SimpleTag { * @param SimpleEncoding $encoding Form packet. * @access public */ - function write(&$encoding) { + function write(&$encoding, $x, $y) { if ($this->getName()) { $encoding->add($this->getName(), $this->getValue()); } @@ -680,7 +680,7 @@ class SimpleUploadTag extends SimpleWidget { * @param SimpleEncoding $encoding Form packet. * @access public */ - function write(&$encoding) { + function write(&$encoding, $x, $y) { if (! file_exists($this->getValue())) { return; } @@ -1415,4 +1415,4 @@ class SimpleFrameTag extends SimpleTag { return false; } } -?> \ No newline at end of file + From 9a9cebb3693dd0464ebb0e96e935fb878ebc8ed3 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:07:35 +1200 Subject: [PATCH 07/18] MINOR Versioned::add_to_class() now conforms to parent DataExtension::add_to_class() with $args being optional (default to null). This is for E_STRICT compliance. --- model/Versioned.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/Versioned.php b/model/Versioned.php index dc9a62500..7b6924c1f 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -107,7 +107,7 @@ class Versioned extends DataExtension { 'Version' => 'Int' ); - static function add_to_class($class, $extensionClass, $args) { + static function add_to_class($class, $extensionClass, $args = null) { Config::inst()->update($class, 'has_many', array('Versions' => $class)); parent::add_to_class($class, $extensionClass, $args); } From 6b40377a1c2850e43472cca72f825c3efb752a5f Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:08:22 +1200 Subject: [PATCH 08/18] BUGFIX Time::setValue() now conforms to DBField::setValue() for E_STRICT compliance. --- model/fieldtypes/Time.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/fieldtypes/Time.php b/model/fieldtypes/Time.php index 50b92cf63..a9b85b02d 100644 --- a/model/fieldtypes/Time.php +++ b/model/fieldtypes/Time.php @@ -16,7 +16,7 @@ */ class Time extends DBField { - function setValue($value) { + function setValue($value, $record = null) { if($value) { if(preg_match( '/(\d{1,2})[:.](\d{2})([a|A|p|P|][m|M])/', $value, $match )) $this->TwelveHour( $match ); else $this->value = date('H:i:s', strtotime($value)); From fd3de5158d4231bce15d69d9d6cfca1b0cc4f3f1 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:09:39 +1200 Subject: [PATCH 09/18] BUGFIX Use of Link() in security classes now refers to $this->controller instead of calling the instance method Link statically (which isn't allowed for E_STRICT compliance.) --- security/ChangePasswordForm.php | 2 +- security/MemberLoginForm.php | 6 +++--- security/PermissionCheckboxSetField.php | 2 +- security/Security.php | 4 +++- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/security/ChangePasswordForm.php b/security/ChangePasswordForm.php index 21deb3402..77872cc7b 100644 --- a/security/ChangePasswordForm.php +++ b/security/ChangePasswordForm.php @@ -111,7 +111,7 @@ class ChangePasswordForm extends Form { } else { // Redirect to default location - the login form saying "You are logged in as..." - $redirectURL = HTTP::setGetVar('BackURL', Director::absoluteBaseURL(), Security::Link('login')); + $redirectURL = HTTP::setGetVar('BackURL', Director::absoluteBaseURL(), $this->controller->Link('login')); Director::redirect($redirectURL); } } else { diff --git a/security/MemberLoginForm.php b/security/MemberLoginForm.php index 9675b7c4a..f85263992 100644 --- a/security/MemberLoginForm.php +++ b/security/MemberLoginForm.php @@ -130,14 +130,14 @@ JS if(isset($_REQUEST['BackURL'])) $backURL = $_REQUEST['BackURL']; else $backURL = null; - if($backURL) Session::set('BackURL', $backURL); + if($backURL) Session::set('BackURL', $backURL); if($badLoginURL = Session::get("BadLoginURL")) { $this->controller->redirect($badLoginURL); } else { // Show the right tab on failed login - $loginLink = Director::absoluteURL(Security::Link("login")); - if($backURL) $loginLink .= '?BackURL=' . urlencode($backURL); + $loginLink = Director::absoluteURL($this->controller->Link('login')); + if($backURL) $loginLink .= '?BackURL=' . urlencode($backURL); $this->controller->redirect($loginLink . '#' . $this->FormName() .'_tab'); } } diff --git a/security/PermissionCheckboxSetField.php b/security/PermissionCheckboxSetField.php index db9027068..cad6474a6 100644 --- a/security/PermissionCheckboxSetField.php +++ b/security/PermissionCheckboxSetField.php @@ -281,7 +281,7 @@ class PermissionCheckboxSetField_Readonly extends PermissionCheckboxSetField { protected $readonly = true; - function saveInto($record) { + function saveInto(DataObjectInterface $record) { return false; } } diff --git a/security/Security.php b/security/Security.php index dfa658259..5d69b99ce 100644 --- a/security/Security.php +++ b/security/Security.php @@ -514,7 +514,9 @@ class Security extends Controller { */ public static function getPasswordResetLink($autoLoginHash) { $autoLoginHash = urldecode($autoLoginHash); - return self::Link('changepassword') . "?h=$autoLoginHash"; + $selfControllerClass = __CLASS__; + $selfController = new $selfControllerClass(); + return $selfController->Link('changepassword') . "?h=$autoLoginHash"; } /** From 9cf1686786ab411beddd1a69ee0be2f6a7593c09 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:11:22 +1200 Subject: [PATCH 10/18] MINOR Code formatting fixes for CurrencyField --- forms/CurrencyField.php | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/forms/CurrencyField.php b/forms/CurrencyField.php index 2d2d0ac05..9ef8c48dc 100644 --- a/forms/CurrencyField.php +++ b/forms/CurrencyField.php @@ -37,15 +37,9 @@ class CurrencyField extends TextField { * Create a new class for this field */ function performReadonlyTransformation() { - $field = new CurrencyField_Readonly($this->name, $this->title, $this->value); $field -> addExtraClass($this->extraClass()); return $field; - - /* - $this is-a object and cant be passed as_a string of the first parameter of formfield constructor. - return new CurrencyField_Readonly($this); - */ } function validate($validator) { @@ -71,9 +65,8 @@ class CurrencyField_Readonly extends ReadonlyField{ if($this->value){ $val = $this->dontEscape ? $this->value : Convert::raw2xml($this->value); $val = _t('CurrencyField.CURRENCYSYMBOL', '$') . number_format(preg_replace('/[^0-9.]/',"",$val), 2); - - }else { - $val = ''._t('CurrencyField.CURRENCYSYMBOL', '$').'0.00'; + } else { + $val = ''._t('CurrencyField.CURRENCYSYMBOL', '$').'0.00'; } $valforInput = $this->value ? Convert::raw2att($val) : ""; return "extraClass()."\" id=\"" . $this->id() . "\">$valname."\" value=\"".$valforInput."\" />"; @@ -104,9 +97,8 @@ class CurrencyField_Disabled extends CurrencyField{ if($this->value){ $val = $this->dontEscape ? $this->value : Convert::raw2xml($this->value); $val = _t('CurrencyField.CURRENCYSYMBOL', '$') . number_format(preg_replace('/[^0-9.]/',"",$val), 2); - - }else { - $val = ''._t('CurrencyField.CURRENCYSYMBOL', '$').'0.00'; + } else { + $val = ''._t('CurrencyField.CURRENCYSYMBOL', '$').'0.00'; } $valforInput = $this->value ? Convert::raw2att($val) : ""; return "name."\" value=\"".$valforInput."\" />"; From 42988ecb18397ab5e8ff0abe2c70f979de979633 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:11:53 +1200 Subject: [PATCH 11/18] BUGFIX Argument hinting of FormField instance for TabSet::push() to comply with parent CompositeField::push() for E_STRICT compliance --- forms/TabSet.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/forms/TabSet.php b/forms/TabSet.php index defcfbe52..03ff06689 100644 --- a/forms/TabSet.php +++ b/forms/TabSet.php @@ -129,7 +129,7 @@ class TabSet extends CompositeField { /** * Add a new child field to the end of the set. */ - public function push($field) { + public function push(FormField $field) { parent::push($field); $field->setTabSet($this); } @@ -155,4 +155,4 @@ class TabSet extends CompositeField { public function removeByName( $tabName, $dataFieldOnly = false ) { parent::removeByName( $tabName, $dataFieldOnly ); } -} \ No newline at end of file +} From 859f7add2090e7924aa40b42c8cd50e998189825 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:13:22 +1200 Subject: [PATCH 12/18] MINOR TableField_Item::Fields() now conforms to parent TableListField_Item::Fields() for E_STRICT compliance --- forms/TableField.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/forms/TableField.php b/forms/TableField.php index 0d202ccc8..2ac169eef 100644 --- a/forms/TableField.php +++ b/forms/TableField.php @@ -687,7 +687,7 @@ class TableField_Item extends TableListField_Item { return new FieldList($this->fields); } - function Fields() { + function Fields($xmlSafe = true) { return $this->fields; } From 1e61b76ca85771cd601a99a4e56e3b7b2db00e11 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:14:42 +1200 Subject: [PATCH 13/18] MINOR ConfirmedPasswordField::validate() now passes in $validator argument instead of getting it from the form, which is how other FormFields work like CompositeField --- forms/ConfirmedPasswordField.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/forms/ConfirmedPasswordField.php b/forms/ConfirmedPasswordField.php index 2ee1b49df..d0cd40229 100644 --- a/forms/ConfirmedPasswordField.php +++ b/forms/ConfirmedPasswordField.php @@ -216,8 +216,7 @@ class ConfirmedPasswordField extends FormField { return (!$this->showOnClick || ($this->showOnClick && $isVisible && $isVisible->Value())); } - function validate() { - $validator = $this->form->getValidator(); + function validate($validator) { $name = $this->name; // if field isn't visible, don't validate From 8369cded32f0af05fbd8b4414300195942f183a3 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:17:30 +1200 Subject: [PATCH 14/18] MINOR Code formatting in EmailField --- forms/EmailField.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/forms/EmailField.php b/forms/EmailField.php index 94a6bb13e..4e8ed4bcc 100644 --- a/forms/EmailField.php +++ b/forms/EmailField.php @@ -19,18 +19,17 @@ class EmailField extends TextField { * @param Validator $validator * @return String */ - function validate($validator){ + function validate($validator) { $this->value = trim($this->value); - - $pcrePattern = '^[a-z0-9!#$%&\'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&\'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$'; + $pcrePattern = '^[a-z0-9!#$%&\'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&\'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$'; // PHP uses forward slash (/) to delimit start/end of pattern, so it must be escaped $pregSafePattern = str_replace('/', '\\/', $pcrePattern); if($this->value && !preg_match('/' . $pregSafePattern . '/i', $this->value)){ - $validator->validationError( - $this->name, + $validator->validationError( + $this->name, _t('EmailField.VALIDATION', "Please enter an email address."), "validation" ); From 865cde0c39d75c383e08ee11414b9868ea6c8daa Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:18:55 +1200 Subject: [PATCH 15/18] BUGFIX FormField::name_to_label() is now declared as static as it was never used as an instance method API CHANGE FormField::validate() $validator argument is now required for FormField classes --- forms/FormField.php | 90 +++++++++++++++++++++++---------------------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/forms/FormField.php b/forms/FormField.php index c8cb87d74..2baf6ea3e 100644 --- a/forms/FormField.php +++ b/forms/FormField.php @@ -88,6 +88,30 @@ class FormField extends RequestHandler { */ protected $attributes = array(); + /** + * Takes a fieldname and converts camelcase to spaced + * words. Also resolves combined fieldnames with dot syntax + * to spaced words. + * + * Examples: + * - 'TotalAmount' will return 'Total Amount' + * - 'Organisation.ZipCode' will return 'Organisation Zip Code' + * + * @param string $fieldName + * @return string + */ + public static function name_to_label($fieldName) { + if(strpos($fieldName, '.') !== false) { + $parts = explode('.', $fieldName); + $label = $parts[count($parts)-2] . ' ' . $parts[count($parts)-1]; + } else { + $label = $fieldName; + } + $label = preg_replace("/([a-z]+)([A-Z])/","$1 $2", $label); + + return $label; + } + /** * Create a new field. * @param name The internal field name, passed to forms. @@ -582,8 +606,8 @@ class FormField extends RequestHandler { /** * @return boolean */ - function isDisabled() { - return $this->disabled; + function isDisabled() { + return $this->disabled; } /** @@ -591,8 +615,8 @@ class FormField extends RequestHandler { * to actually transform this instance. * @param $bool boolean Setting "false" has no effect on the field-state. */ - function setDisabled($bool) { - $this->disabled = $bool; + function setDisabled($bool) { + $this->disabled = $bool; return $this; } @@ -607,21 +631,23 @@ class FormField extends RequestHandler { } /** - * Return a disabled version of this field + * Return a disabled version of this field. + * Tries to find a class of the class name of this field suffixed with "_Disabled", + * failing that, finds a method {@link setDisabled()}. + * + * @return FormField */ function performDisabledTransformation() { $clone = clone $this; $disabledClassName = $clone->class . '_Disabled'; - if( ClassInfo::exists( $disabledClassName ) ) - return new $disabledClassName( $this->name, $this->title, $this->value ); - elseif($clone->hasMethod('setDisabled')){ + if(ClassInfo::exists($disabledClassName)) { + return new $disabledClassName($this->name, $this->title, $this->value); + } else { $clone->setDisabled(true); return $clone; - }else{ - return $this->performReadonlyTransformation(); } } - + function transform(FormTransformation $trans) { return $trans->transform($this); } @@ -662,14 +688,16 @@ class FormField extends RequestHandler { if($content || $tag != 'input') return "<$tag$preparedAttributes>$content"; else return "<$tag$preparedAttributes />"; } - + /** - * Validation Functions for each field type by default - * formfield doesnt have a validation function - * - * @todo shouldn't this be an abstract method? + * Abstract method each {@link FormField} subclass must implement, + * determines whether the field is valid or not based on the value. + * @todo Make this abstract. + * + * @param Validator + * @return boolean */ - function validate() { + function validate($validator) { return true; } @@ -690,7 +718,7 @@ class FormField extends RequestHandler { */ function setDescription($description) { $this->description = $description; - return $this; + return $this; } /** @@ -721,31 +749,7 @@ class FormField extends RequestHandler { return $validator->fieldIsRequired($this->name); } } - - /** - * Takes a fieldname and converts camelcase to spaced - * words. Also resolves combined fieldnames with dot syntax - * to spaced words. - * - * Examples: - * - 'TotalAmount' will return 'Total Amount' - * - 'Organisation.ZipCode' will return 'Organisation Zip Code' - * - * @param string $fieldName - * @return string - */ - public function name_to_label($fieldName) { - if(strpos($fieldName, '.') !== false) { - $parts = explode('.', $fieldName); - $label = $parts[count($parts)-2] . ' ' . $parts[count($parts)-1]; - } else { - $label = $fieldName; - } - $label = preg_replace("/([a-z]+)([A-Z])/","$1 $2", $label); - - return $label; - } - + /** * Set the FieldList that contains this field. * From 3c70ea492268cb94f1e643d4202f7c0c6b001492 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:20:18 +1200 Subject: [PATCH 16/18] MINOR Fixing broken test MemberDatetimeOptionsetFieldTest, passing in null for $validator argument --- tests/forms/MemberDatetimeOptionsetFieldTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/forms/MemberDatetimeOptionsetFieldTest.php b/tests/forms/MemberDatetimeOptionsetFieldTest.php index b2a381651..608ddf3b6 100644 --- a/tests/forms/MemberDatetimeOptionsetFieldTest.php +++ b/tests/forms/MemberDatetimeOptionsetFieldTest.php @@ -83,11 +83,11 @@ class MemberDatetimeOptionsetFieldTest extends SapphireTest { function testDateFormValid() { $field = new MemberDatetimeOptionsetField('DateFormat', 'DateFormat'); - $this->assertTrue($field->validate()); + $this->assertTrue($field->validate(null)); $_POST['DateFormat_custom'] = 'dd MM yyyy'; - $this->assertTrue($field->validate()); + $this->assertTrue($field->validate(null)); $_POST['DateFormat_custom'] = 'sdfdsfdfd1244'; - $this->assertFalse($field->validate()); + $this->assertFalse($field->validate(null)); } } @@ -97,4 +97,4 @@ class MemberDatetimeOptionsetFieldTest_Controller extends Controller { return 'test'; } -} \ No newline at end of file +} From 852ffcf492dd82b227a645dcd05e90d0450e7903 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:23:18 +1200 Subject: [PATCH 17/18] MINOR MemberDatetimeOptionsetField::validate() now passes in $validator argument instead of getting it from the form, which is how other FormFields work like CompositeField --- forms/MemberDatetimeOptionsetField.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/forms/MemberDatetimeOptionsetField.php b/forms/MemberDatetimeOptionsetField.php index 052249445..edc7b37bf 100644 --- a/forms/MemberDatetimeOptionsetField.php +++ b/forms/MemberDatetimeOptionsetField.php @@ -89,12 +89,11 @@ class MemberDatetimeOptionsetField extends OptionsetField { } } - function validate() { + function validate($validator) { $value = isset($_POST[$this->name . '_custom']) ? $_POST[$this->name . '_custom'] : null; if(!$value) return true; // no custom value, don't validate // Check that the current date with the date format is valid or not - $validator = $this->form ? $this->form->getValidator() : null; require_once 'Zend/Date.php'; $date = Zend_Date::now()->toString($value); $valid = Zend_Date::isDate($date, $value); @@ -107,4 +106,4 @@ class MemberDatetimeOptionsetField extends OptionsetField { return false; } } -} \ No newline at end of file +} From 2b8e14fdffe51e25dc8a2062416150ffc167f709 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 12 Apr 2012 12:24:06 +1200 Subject: [PATCH 18/18] API CHANGE Fixing CompositeField/SelectionGroup performReadonlyTransformation and performDisabledTransformation to be consistent and conform with parent FormField. $trans argument is no longer allowed, just uses DisabledTransformation or ReadonlyTransformation instead. --- forms/CompositeField.php | 30 ++++++++++++++++-------------- forms/SelectionGroup.php | 21 +-------------------- 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/forms/CompositeField.php b/forms/CompositeField.php index dbb540c4c..2fda6399c 100644 --- a/forms/CompositeField.php +++ b/forms/CompositeField.php @@ -298,39 +298,42 @@ class CompositeField extends FormField { } /** - * Return a readonly version of this field. Keeps the composition but returns readonly - * versions of all the children + * Return a readonly version of this field. Keeps the composition but returns readonly + * versions of all the child {@link FormField} objects. + * + * @return CompositeField */ public function performReadonlyTransformation() { $newChildren = new FieldList(); $clone = clone $this; - foreach($clone->getChildren() as $idx => $child) { + if($clone->getChildren()) foreach($clone->getChildren() as $idx => $child) { if(is_object($child)) $child = $child->transform(new ReadonlyTransformation()); $newChildren->push($child, $idx); } $clone->children = $newChildren; $clone->readonly = true; + return $clone; } /** - * Return a readonly version of this field. Keeps the composition but returns readonly - * versions of all the children + * Return a disabled version of this field. Keeps the composition but returns disabled + * versions of all the child {@link FormField} objects. + * + * @return CompositeField */ - public function performDisabledTransformation($trans) { + public function performDisabledTransformation() { $newChildren = new FieldList(); $clone = clone $this; if($clone->getChildren()) foreach($clone->getChildren() as $idx => $child) { - if(is_object($child)) { - $child = $child->transform($trans); - } + if(is_object($child)) $child = $child->transform(new DisabledTransformation()); $newChildren->push($child, $idx); } $clone->children = $newChildren; $clone->readonly = true; - + return $clone; } @@ -394,15 +397,14 @@ class CompositeField extends FormField { $result .= ""; return $result; } - - function validate($validator){ - + + function validate($validator) { $valid = true; foreach($this->children as $idx => $child){ $valid = ($child && $child->validate($validator) && $valid); } - return $valid; } + } diff --git a/forms/SelectionGroup.php b/forms/SelectionGroup.php index b2aabefdc..bc95395eb 100644 --- a/forms/SelectionGroup.php +++ b/forms/SelectionGroup.php @@ -27,25 +27,6 @@ class SelectionGroup extends CompositeField { Requirements::css(SAPPHIRE_DIR . '/css/SelectionGroup.css'); } - - /** - * Return a readonly version of this field. Keeps the composition but returns readonly - * versions of all the children - */ - public function performDisabledTransformation($trans) { - $newChildren = array(); - $clone = clone $this; - if($clone->children) foreach($clone->getChildren() as $idx => $child) { - if(is_object($child)) { - $child = $child->transform($trans); - } - $newChildren[$idx] = $child; - } - - $clone->setChildren(new FieldList($newChildren)); - $clone->setReadonly(true); - return $clone; - } function FieldSet() { return $this->FieldList(); @@ -76,7 +57,7 @@ class SelectionGroup extends CompositeField { if(is_object($item)) $newItems[] = $item->customise($extra); else $newItems[] = new ArrayData($extra); - $firstSelected = $checked =""; + $firstSelected = $checked =""; } return new ArrayList($newItems); }