#2144327: Make all field types provide a schema() added a call to getFieldType which doesn't even exist. Then it tries to add an array to a NULL which fatals. There are, obviously, no tests. I haven't written any but now that migrate uses this, that's ample test... Migrate just does this:
$schema = FieldDefinition::create($type)->getColumns();
return $schema[$column];
Also, StringItem tries to get max_length by running $field_definition->getSetting('max_length') but at this moment $field_definition has no settings. If I patch up FieldDefinition::schema to do
$this->setSettings($definition['settings']);
$schema = $class::schema($this);
then it works but it's clearly wrong -- if someone tries to run just FieldDefinition::create('string')->getSetting('max_length') that wouldn't work. Not sure what to do here. TypeData is still very strange to me.
Comments
Comment #1
tim.plunkettIt used to exist, it was renamed in #2143263: Remove "Field" prefix from FieldDefinitionInterface methods.
So, missing test coverage?
Comment #2
chx commentedAbsolutely. The new schema stuff went in weeks after the linked issue. I also noted the lack of test coverage in the issue summary.
Comment #3
chx commentedComment #4
yched commentedFieldDefinition::create('string')->getSetting('max_length') is real, but different issue.
Opened #2175017: FieldDefinition::create() doesn't populate default 'settings' for the field type.
Comment #5
chx commentedWell, it might be but that's essentially what the current code runs.
Comment #6
yched commentedYeah, sorry, this is definitely related - but larger issue, better discussed in a separate task IMO.
Comment #7
amateescu commentedHere's a quick test. I thought the original patch tested this enough through the existing field tests but, obviously, I haven't considered base fields :(
Comment #9
amateescu commentedComment #10
tstoecklerYes, let's get this in please. Patch looks good.
Comment #11
alexpottCommitted fde84e2 and pushed to 8.x. Thanks!