Contrib.social

Cannot detect if entity is new when the ID is set on creation

Open on Drupal.org →
Created on 3 March 2024, 10 months ago
Updated 30 March 2024, 9 months ago

Problem/Motivation

I have a case with a custom field item list class where I want to detect if the entity is new inside ::setValue() and the entity ID is set on creation

class MyFieldItemList extends FieldItemList {
  public function setValue($values, $notify = TRUE): void {
    if (!$this->getEntity()->isNew()) {
       // some logic
    }
    parent::setValue($values, $notify);
  }
}

If I create the entity in this way:

$entity = $storage->create(
  'id' => 123,
  'field_with_custom_item_list_class' => 'something',
)->save();

then $this->getEntity()->isNew() returns FALSE. Even I try:

$entity = $storage->create(
  'enforceIsNew' => TRUE,
  'id' => 123,
  'field_with_custom_item_list_class' => 'something',
)->save();

the result is the same.

The problem seems to be in ContentEntityStorageBase::create() where $entity->enforceIsNew(); acts too late. Other weird thing is that, in ContentEntityStorageBase::doCreate() the entity class is instantiated with an empty array as values so that when enforceIsNew is set as value, is just lost

Steps to reproduce

See test coverage from MR

Proposed resolution

Trigger $entity->enforceIsNew() a little earlier.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Entity  →

Last updated about 17 hours ago

Created by

🇷🇴Romania claudiu.cristea Arad 🇷🇴

Live updates comments and jobs are added and updated live.
  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @claudiu.cristea
  • Merge request !6872Cannot detect if entity is new when the ID is set on creation → (Open) created by claudiu.cristea
  • Pipeline finished with Success
    10 months ago
    Total: 489s
    #109675
  • Pipeline finished with Failed
    10 months ago
    #109702
  • Pipeline finished with Failed
    10 months ago
    Total: 163s
    #109706
  • Pipeline finished with Success
    10 months ago
    Total: 495s
    #109709
  • Status changed to Needs review 10 months ago
  • Comment 10 months ago →
    🇷🇴Romania claudiu.cristea Arad 🇷🇴
  • Comment 10 months ago →
    🇷🇴Romania claudiu.cristea Arad 🇷🇴
  • Status changed to RTBC 10 months ago
  • Comment 10 months ago →
    🇺🇸United States smustgrave

    Change seems simple enough. And didn't break anything that I can tell.

    Test coverage is perfectly there and shows the issue.

    -'The Name (new)'
+'The Name'
/builds/issue/drupal-3425226/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/builds/issue/drupal-3425226/vendor/phpunit/phpunit/src/Framework/Constraint/IsIdentical.php:79
/builds/issue/drupal-3425226/core/tests/Drupal/KernelTests/Core/Entity/ContentEntityStorageBaseTest.php:79
/builds/issue/drupal-3425226/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!
Tests: 5, Assertions: 25, Failures: 1.
  • Status changed to Needs work 9 months ago
  • Comment 9 months ago →
    🇬🇧United Kingdom alexpott 🇪🇺🌍

    I've add a comment to the MR - I think we should be a little bit more defensive in the code here.

    I also think we should have change record for this.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024