This caused #731724-334: Convert comment settings into a field to make them work with CMI and non-node entities code all over comment field

Problem
For field instance that created from Field UI module (means with browser) the default_value is not assigned.
There only hook_ENTITY_TYPE_create() hook that allows to inject defaults into saving field instance.
Also when field is added to existing entity no default value is populated so field item needs to override __get() to detect this.

For example comment field implements:
comment_field_instance_config_create() and \Drupal\comment\Plugin\Field\FieldType\CommentItem::__get()

Proposed solution
Find sane way to assign default value to field item.

Possible approaches:
1) implement DataDefinition->setDefaultValue()
2) use TypedDataInterface::applyDefaultValue()

Files: 
CommentFileSizeAuthor
#15 1919834-comment-defaults-15.patch3.85 KBandypost
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,122 pass(es), 7 fail(s), and 2 exception(s).
[ View ]

Comments

yched’s picture

Sorry but I don't understand what's expected here

andypost’s picture

I mean that code needs always check for existence a value in widget and everywhere
but better to populate "default_value" for field instance upon when it got saved first time but I found no way to make it

<?php
// in FieldOverview::submit()

     
$field = array(
       
'field_name' => $values['field_name'],
       
'type' => $values['type'],
       
'translatable' => $values['translatable'],
      );
     
$instance = array(
       
'field_name' => $field['field_name'],
       
'entity_type' => $this->entity_type,
       
'bundle' => $this->bundle,
       
'label' => $values['label'],
       
'widget' => array(
         
'type' => $values['widget_type'],
         
'weight' => $values['weight'],
        ),
      );

     
// Create the field and instance.
     
try {
       
field_create_field($field);
       
field_create_instance($instance);


// and latter for existing field

       
$instance = array(
         
'field_name' => $field['field_name'],
         
'entity_type' => $this->entity_type,
         
'bundle' => $this->bundle,
         
'label' => $values['label'],
         
'widget' => array(
           
'type' => $values['widget_type'],
           
'weight' => $values['weight'],
          ),
        );

        try {
         
field_create_instance($instance);
?>

No ability to alter :(

swentel’s picture

So there's http://api.drupal.org/api/drupal/core%21modules%21field%21field.api.php/... but that's not ideal, maybe we should have a drupal_alter here ?

andypost’s picture

quickly discussed with @swentel in IRC

<swentel> andypost: hook_field_create_instance() maybe ? Just skimmed fast through the issue, pretty busy here.
<swentel> the annoying thing is of course that the instance has been saved at that point :/
<swentel> so there would be two saves, that's kind of stupid
<swentel> so, yeah, not ideal
<swentel> hmm there should be an alter for that maybe
<andypost> swentel, suppose drupal_alter() is enough. But maybe any kind of instance presave planned with field cmi?
<swentel> andypost: there's a presave method in the entity storage controller right ?
* swentel quickly checks
<swentel> hmm apparently not
<andypost> swentel, maybe hook_field_instance_presave() as analog of hook_field_presave()
<swentel> andypost: yeah, something like that could work

Probably we get this hook with field CMI convertion #1735118: Convert Field API to CMI

yched’s picture

Hmm, it seems what you'd want is the field type to be able to specify a "default default value" for new fields of this type ?

andypost’s picture

to specify a default value for field instance

yched’s picture

@andypost : you're being too cryptic.

This is filed as a bug report. Please explain, with sentences, what the bug is.

andypost’s picture

Category:bug» task
Issue tags:+API clean-up

np, I think that code #2 explains that we have no hook_field_instance_presave of drupal_alter() for instance when fiel_ui creates instance

andypost’s picture

For example I'm adding a Comments field so the next step I need to configure it's default value (Open,Closed)
But field ui creates a field and instance for me without default value (Open) so widget, formatter needs always check for isset($values[0]) to not throw notices.
Ability to provide default value on field/instance level will help DX to simplify formatters and widgets. Also we get standard way to store defaults for field instance.

Currently core "datetime" implements own constructor and instance settings value to provide a default value via annotations so:

What is a better way to initialize default value for field instance now?
Will CMI conversion help to set "default_value" or "default_value_function" for Instance? (probably presave hook for Instance entity is enough to control this)

andypost’s picture

Status:Active» Postponed

Discussed in IRC. After #1735118: Convert Field API to CMI we could use any of _presave() hooks on Instance entity_presave()

andypost’s picture

Status:Postponed» Active

This still valid. Tryed after #1777956: Provide a way to define default values for entity fields

--- core/modules/comment/lib/Drupal/comment/Type/CommentItem.php
+++ core/modules/comment/lib/Drupal/comment/Type/CommentItem.php
@@ -32,6 +32,7 @@ public function getPropertyDefinitions() {
       static::$propertyDefinitions['status'] = array(
         'type' => 'integer',
         'label' => t('Comment status value'),
+        'settings' => array('default_value' => COMMENT_OPEN),
       );
     }
     return static::$propertyDefinitions;

No success

swentel’s picture

Status:Active» Closed (duplicate)
swentel’s picture

Issue summary:View changes

Updated issue summary.

andypost’s picture

Category:Task» Bug report
Issue summary:View changes
Status:Closed (duplicate)» Active
Issue tags:+Field API, +Entity Field API

The bug still there and comment module implements comment_field_instance_config_create() hook to init default values.

EDIT another hack lives in \Drupal\comment\Plugin\Field\FieldType\CommentItem::__get()

andypost’s picture

Issue summary:View changes
Issue tags:+DX (Developer Experience)

Updated summary, at least issue should fix comment field.

andypost’s picture

Component:field system» comment.module
Status:Active» Needs review
StatusFileSize
new3.85 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,122 pass(es), 7 fail(s), and 2 exception(s).
[ View ]

Seems field needs item list class for that now

Status:Needs review» Needs work

The last submitted patch, 15: 1919834-comment-defaults-15.patch, failed testing.

andypost’s picture

Looks like migrate applies default values diferently

yched’s picture

  1. +++ b/core/modules/comment/src/Plugin/Field/FieldType/CommentFieldItemList.php
    @@ -0,0 +1,38 @@
    +    if (empty($default_value)) {
    +      $default_value[] = array(

    Minor, but looks a bit weird. If it's empty, we might as well assign its value ($default_value = ...), rather than append to it ($default_value[] = ...)

  2. +++ b/core/modules/comment/src/Plugin/Field/FieldType/CommentFieldItemList.php
    @@ -0,0 +1,38 @@
    +        'status' => CommentItemInterface::OPEN,
    +        'cid' => 0,
    +        'last_comment_timestamp' => 0,
    +        'last_comment_name' => '',
    +        'last_comment_uid' => 0,
    +        'comment_count' => 0,

    Do we really need to specify all of this ? Wouldn't it be enough to only specify 'status' ?

    I mean, when a default value *is* set, it only contains a default value for 'status', and that's what the method returns, right ?
    So it seems when no default value is set, we could simply do $default_value = CommentItemIbterface::OPEN ?

Other than that, approach looks correct.

andypost’s picture

@yched maybe it makes sense to update FieldOverview::submit() to set default values for fields(instances) rather then provide a hacks?

PS comment should be fixed anyway according #2318605: uuid, created, changed, language default value implementations need to be updated

andypost’s picture

@yched now default values has consistent API please suggest a solution here.
Also in related #2401497-23: Field UI creates fields that can never be translated issue we find that forms hardcodes default values