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

Use the Field Defaults project. See #46.

Find sane way to assign default value to field item.

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

Support from Acquia helps fund testing for Drupal Acquia logo

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

// 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
FileSize
3.85 KB

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

andypost’s picture

Status: Needs work » Needs review
FileSize
2.63 KB

There's first bird of changes to simplify that #2446511: Add a "preconfigured field options" concept in Field UI

Patch just checks how many tests would fail

Status: Needs review » Needs work

The last submitted patch, 21: 1919834-comment-defauls-21.patch, failed testing.

andypost’s picture

That still does not work

andypost’s picture

andypost’s picture

Version: 8.0.x-dev » 8.1.x-dev

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andypost’s picture

andypost’s picture

Component: comment.module » field_ui.module

proper component

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jibran’s picture

Status: Needs work » Postponed

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andypost’s picture

Status: Postponed » Needs work
Issue tags: +Needs reroll, +Needs tests

This is no longer postponed, I think this change could be accepted in minor version as bugfix

jofitz’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
3.9 KB

Re-rolled.

Status: Needs review » Needs work

The last submitted patch, 34: 1919834-34.patch, failed testing. View results

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andypost’s picture

It still needs work and comment_field_config_create() also would be great to have in field class

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

dqd’s picture

If I understand the issue and the goal correctly (partly too cryptic), I am still not convinced of that a newly created field in a existing bundle with already existing content should write the "default value" (like setup in the field creation process) to all already existing rows without asking yet. Unless this is planned to be optional (checkbox etc.) I think this can become a dangerous approach?

Apart from that, there is a contrib module for that: https://www.drupal.org/project/field_defaults

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

Issue summary: View changes
Status: Needs work » Closed (works as designed)
Issue tags: -API clean-up, -Field API, -Entity Field API, -DX (Developer Experience), -Needs tests +Bug Smash Initiative

Discussed in #bugsmash with andypost, catch, berdir and Lendude and it was agreed that this is working as designed.

Anyone wanting this functionality should use the Field Defaults project. If you think what that module does should be in core, open a new Feature Request to discuss why it is needed, the pros and cons etc. Also, add this issue as a related issue.

Thanks!