From 324bdad48c7ad3c3faa75388e22a34dfdf7ae4b9 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 25 Oct 2017 09:30:04 +1300 Subject: [PATCH] ENHANCEMENT Ensure DBVarchar scaffolds text field with TextField with appropriate max length Fixes #1413 --- src/Forms/TextField.php | 7 ++--- src/ORM/FieldType/DBField.php | 4 +-- src/ORM/FieldType/DBHTMLVarchar.php | 4 +-- src/ORM/FieldType/DBVarchar.php | 28 ++++++++++++-------- tests/php/ORM/DBVarcharTest.php | 30 ++++++++++++++++++++++ tests/php/ORM/DBVarcharTest/TestObject.php | 16 ++++++++++++ tests/php/ORM/Search/SearchContextTest.php | 3 ++- 7 files changed, 73 insertions(+), 19 deletions(-) create mode 100644 tests/php/ORM/DBVarcharTest.php create mode 100644 tests/php/ORM/DBVarcharTest/TestObject.php diff --git a/src/Forms/TextField.php b/src/Forms/TextField.php index 95cda9cbf..5f00ff0a8 100644 --- a/src/Forms/TextField.php +++ b/src/Forms/TextField.php @@ -22,7 +22,9 @@ class TextField extends FormField * @param string $name * @param null|string $title * @param string $value - * @param null|int $maxLength + * @param null|int $maxLength Max characters to allow for this field. If this value is stored + * against a DB field with a fixed size it's recommended to set an appropriate max length + * matching this size. * @param null|Form $form */ public function __construct($name, $title = null, $value = '', $maxLength = null, $form = null) @@ -40,8 +42,7 @@ class TextField extends FormField /** * @param int $maxLength - * - * @return static + * @return $this */ public function setMaxLength($maxLength) { diff --git a/src/ORM/FieldType/DBField.php b/src/ORM/FieldType/DBField.php index 53dfd3669..e8d04b650 100644 --- a/src/ORM/FieldType/DBField.php +++ b/src/ORM/FieldType/DBField.php @@ -548,9 +548,7 @@ abstract class DBField extends ViewableData implements DBIndexable */ public function scaffoldFormField($title = null, $params = null) { - $field = new TextField($this->name, $title); - - return $field; + return TextField::create($this->name, $title); } /** diff --git a/src/ORM/FieldType/DBHTMLVarchar.php b/src/ORM/FieldType/DBHTMLVarchar.php index 561ad4c4c..5343790b3 100644 --- a/src/ORM/FieldType/DBHTMLVarchar.php +++ b/src/ORM/FieldType/DBHTMLVarchar.php @@ -124,12 +124,12 @@ class DBHTMLVarchar extends DBVarchar public function scaffoldFormField($title = null, $params = null) { - return new HTMLEditorField($this->name, $title, 1); + return HTMLEditorField::create($this->name, $title, 1); } public function scaffoldSearchField($title = null) { - return new TextField($this->name, $title); + return TextField::create($this->name, $title); } public function getSchemaValue() diff --git a/src/ORM/FieldType/DBVarchar.php b/src/ORM/FieldType/DBVarchar.php index b2518e37c..99ef68757 100644 --- a/src/ORM/FieldType/DBVarchar.php +++ b/src/ORM/FieldType/DBVarchar.php @@ -2,10 +2,11 @@ namespace SilverStripe\ORM\FieldType; -use SilverStripe\ORM\DB; use SilverStripe\Core\Config\Config; -use SilverStripe\Forms\TextField; use SilverStripe\Forms\NullableField; +use SilverStripe\Forms\TextField; +use SilverStripe\ORM\Connect\MySQLDatabase; +use SilverStripe\ORM\DB; /** * Class Varchar represents a variable-length string of up to 255 characters, designed to store raw text @@ -22,6 +23,11 @@ class DBVarchar extends DBString "URL" => "Text", ); + /** + * Max size of this field + * + * @var int + */ protected $size; /** @@ -58,8 +64,8 @@ class DBVarchar extends DBString */ public function requireField() { - $charset = Config::inst()->get('SilverStripe\ORM\Connect\MySQLDatabase', 'charset'); - $collation = Config::inst()->get('SilverStripe\ORM\Connect\MySQLDatabase', 'collation'); + $charset = Config::inst()->get(MySQLDatabase::class, 'charset'); + $collation = Config::inst()->get(MySQLDatabase::class, 'collation'); $parts = array( 'datatype'=>'varchar', @@ -117,12 +123,14 @@ class DBVarchar extends DBString public function scaffoldFormField($title = null, $params = null) { - if (!$this->nullifyEmpty) { - // Allow the user to select if it's null instead of automatically assuming empty string is - return new NullableField(new TextField($this->name, $title)); - } else { - // Automatically determine null (empty string) - return parent::scaffoldFormField($title); + // Set field with appropriate size + $field = TextField::create($this->name, $title); + $field->setMaxLength($this->getSize()); + + // Allow the user to select if it's null instead of automatically assuming empty string is + if (!$this->getNullifyEmpty()) { + return NullableField::create($field); } + return $field; } } diff --git a/tests/php/ORM/DBVarcharTest.php b/tests/php/ORM/DBVarcharTest.php new file mode 100644 index 000000000..377f6277e --- /dev/null +++ b/tests/php/ORM/DBVarcharTest.php @@ -0,0 +1,30 @@ +dbObject('Title')->scaffoldFormField(); + $this->assertInstanceOf(TextField::class, $field); + $this->assertEquals(129, $field->getMaxLength()); + + /** @var NullableField $nullable */ + $nullable = $obj->dbObject('NullableField')->scaffoldFormField(); + $this->assertInstanceOf(NullableField::class, $nullable); + $innerField = $nullable->valueField; + $this->assertInstanceOf(TextField::class, $innerField); + $this->assertEquals(111, $innerField->getMaxLength()); + } +} diff --git a/tests/php/ORM/DBVarcharTest/TestObject.php b/tests/php/ORM/DBVarcharTest/TestObject.php new file mode 100644 index 000000000..288073dfd --- /dev/null +++ b/tests/php/ORM/DBVarcharTest/TestObject.php @@ -0,0 +1,16 @@ + 'Varchar(129)', + 'NullableField' => 'Varchar(111, ["nullifyEmpty" => false])' + ]; +} diff --git a/tests/php/ORM/Search/SearchContextTest.php b/tests/php/ORM/Search/SearchContextTest.php index 7f9d73a04..8ef02851f 100644 --- a/tests/php/ORM/Search/SearchContextTest.php +++ b/tests/php/ORM/Search/SearchContextTest.php @@ -106,7 +106,8 @@ class SearchContextTest extends SapphireTest $context = $company->getDefaultSearchContext(); $this->assertEquals( new FieldList( - new TextField("Name", 'Name'), + (new TextField("Name", 'Name')) + ->setMaxLength(255), new TextareaField("Industry", 'Industry'), new NumericField("AnnualProfit", 'The Almighty Annual Profit') ),