Follow-up to #2490966: [Meta] Replace deprecated usage of entity_create with a direct call to the entity type class

Problem/Motivation

According to #2346261: Deprecate entity_create() in favor of a <EntityType>::create($values) or \Drupal::entityManager()->getStorage($entity_type)->create($values), entity_create() function is going to be deprecated so we shouldn't use it anymore. When the entity type is known we should directly call <EntityType>::create(). What to do when the entity type is not known or is variable is upon discussions.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task
Issue priority Normal because it's just about code cleanup and good practices
Prioritized changes The main goal of this issue is DX, performance and removing code already deprecated for 8.0.0. (Direct calls to EntityType::create are better than generic calls to entity_create for readability)
Disruption This change is not disruptive at all as it only replaces deprecated functions call by their exact equivalent.

Proposed resolution

Replace the deprecated call to entity_create() by a proper call to <EntityType>::create().

Before:

entity_create('field_config', $field_values)->save();

After:

use Drupal\field\Entity\FieldConfig;
FieldConfig::create($field_values)->save();

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Create a patch Instructions Done
Manually test the patch Novice Instructions
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards Instructions

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#14 2641584-14.patch113.61 KBheykarthikwithu
#11 field_config-2641584-11.patch101.36 KBlegovaer
#7 field_config-2641584-7.patch102.31 KBMac_Weber
#4 field_config-2641584-4.patch102.3 KBMac_Weber
#2 field_config-2641584-2.patch106.77 KBMac_Weber
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mac_Weber created an issue. See original summary.

Mac_Weber’s picture

Mac_Weber CreditAttribution: Mac_Weber as a volunteer commented
FileSize
field_config-2641584-2.patch106.77 KB

Status: Needs review » Needs work

The last submitted patch, 2: field_config-2641584-2.patch, failed testing.

Mac_Weber’s picture

Mac_Weber CreditAttribution: Mac_Weber as a volunteer commented
Status: Needs work » Needs review
FileSize
field_config-2641584-4.patch102.3 KB

Fixed class aliasing

swentel’s picture

swentel CreditAttribution: swentel commented

Doesn't apply anymore (probably after #2604220: PHP notice for single value image field configured with a default image (no image present) and a hidden label), should be easy to fix.

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Needs review » Needs work
Mac_Weber’s picture

Mac_Weber CreditAttribution: Mac_Weber as a volunteer commented
Status: Needs work » Needs review
FileSize
field_config-2641584-7.patch102.31 KB
Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano at Druid commented
Status: Needs review » Needs work

Tests must use $this->container->get('entity_type.manager')->getStorage('field_config')->create(). This type of dependency injection is not possible in procedural code, so that's why we use FieldConfig::create() there.

chx’s picture

chx CreditAttribution: chx at Smartsheet commented
Status: Needs work » Needs review

Erm, nope. That's not what the @deprecated section of entity_create() says. If you disagree with that documentation and the decision behind it , bring it up with the entity maintainers but not in this issue or family of issues.

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer and at Acquia commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

This patch needs reroll due to conflicts with #2641596: Replace deprecated usage of entity_create('node') with a direct call to Node::create()

legovaer’s picture

legovaer
he/him
Primary language Dutch
Location Duffel
CreditAttribution: legovaer as a volunteer commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
field_config-2641584-11.patch101.36 KB
  • Re-rolled #7
  • Re-ordered some of the "use" statements in order to have them alphabetical

Status: Needs review » Needs work

The last submitted patch, 11: field_config-2641584-11.patch, failed testing.

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Assigned: Unassigned » heykarthikwithu
heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Assigned: heykarthikwithu » Unassigned
Status: Needs work » Needs review
FileSize
2641584-14.patch113.61 KB

As per #10.

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Issue tags: +@deprecated, +deprecated
legovaer’s picture

legovaer
he/him
Primary language Dutch
Location Duffel
CreditAttribution: legovaer as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Patch applies, successfully replaces entity_create('field_config').

RTBC.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x, thanks!

  • catch committed 8e4fe5c on 8.1.x 
    Issue #2641584 by Mac_Weber, heykarthikwithu, legovaer: Replace...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.