ENHANCEMENT Ensure DBVarchar scaffolds text field with TextField with appropriate max length

Fixes #1413

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)
     {

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);
     }
 
     /**

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()

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) {
+        // 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
-            return new NullableField(new TextField($this->name, $title));
-        } else {
-            // Automatically determine null (empty string)
-            return parent::scaffoldFormField($title);
+        if (!$this->getNullifyEmpty()) {
+            return NullableField::create($field);
         }
+        return $field;
     }
 }

tests/php/ORM/DBVarcharTest.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace SilverStripe\ORM\Tests;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Forms\NullableField;
+use SilverStripe\Forms\TextField;
+
+class DBVarcharTest extends SapphireTest
+{
+    protected static $extra_dataobjects = [
+        DBVarcharTest\TestObject::class,
+    ];
+
+    public function testScaffold()
+    {
+        $obj = new DBVarcharTest\TestObject();
+        /** @var TextField $field */
+        $field = $obj->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());
+    }
+}

tests/php/ORM/DBVarcharTest/TestObject.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DBVarcharTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+
+class TestObject extends DataObject implements TestOnly
+{
+    private static $table_name = 'DBVarcharTest_TestObject';
+
+    private static $db = [
+        'Title' => 'Varchar(129)',
+        'NullableField' => 'Varchar(111, ["nullifyEmpty" => false])'
+    ];
+}

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')
             ),