Problem/Motivation

\Drupal\Core\Field\Plugin\Field\FieldFormatter\StringFormatter doesn't declare the entityManager attribute in the class

Update:
As per suggestion by @alexpott in #4 replace deprecated entity.manager with entity_type.manager service.
Also fix the same for \Drupal\Core\Field\Plugin\Field\FieldFormatter\LanguageFormatter

Proposed resolution

Replace injection of deprecated service entity.manager with entity_type.manager in following classes:
\Drupal\Core\Field\Plugin\Field\FieldFormatter\StringFormatter
\Drupal\Core\Field\Plugin\Field\FieldFormatter\LanguageFormatter

Remaining tasks

Create a patch

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#11 replace-depracated-entitymanager-2981025-11.patch5.79 KBbarthje
#8 interdiff6-8.txt1.73 KBmsankhala
#8 replace-depracated-entitymanager-2981025-8.patch5.78 KBmsankhala
#6 interdiff-2-6.txt5.78 KBmsankhala
#6 replace-depracated-entitymanager-2981025-6.patch5.72 KBmsankhala
#2 entity_manager_attribute-2981025-2.patch688 bytesStockticker
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

penyaskito created an issue. See original summary.

Stockticker’s picture

Stockticker CreditAttribution: Stockticker as a volunteer and at Drupal Ukraine Community commented
Assigned: Unassigned » Stockticker
Status: Active » Needs review
FileSize
entity_manager_attribute-2981025-2.patch688 bytes

Please review.

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Status: Needs review » Reviewed & tested by the community

@Stockticker Thanks for the patch. I can confirm this patch applies cleanly and declares the missing entityManager attribute of the class. Even though EntityManager is deprecated now I think that should be part of another issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work

Nice find!

There's an additional problem here. The entity manager has been deprecated. So we're fixing something but then we'll have to change it in the future. Let's inject entity_type.manager into \Drupal\Core\Field\Plugin\Field\FieldFormatter\StringFormatter instead and fix \Drupal\Core\Field\Plugin\Field\FieldFormatter\LanguageFormatter too. And then the new property can be called $entityTypeManager.

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Title: \Drupal\Core\Field\Plugin\Field\FieldFormatter\StringFormatter doesn't declare the entityManager attribute in the class » Replace injection of deprecated entity.manager service with entity_type.manager in \Drupal\Core\Field\Plugin\Field\FieldFormatter\StringFormatter and LanguageFormatter
Issue summary: View changes
msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Status: Needs work » Needs review
FileSize
replace-depracated-entitymanager-2981025-6.patch5.72 KB
interdiff-2-6.txt5.78 KB

@Stockticker Thanks for reporting this issue and initial patch. Here is updated patch.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Needs review » Needs work 
+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/LanguageFormatter.php
@@ -47,13 +47,12 @@ class LanguageFormatter extends StringFormatter {
+   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/StringFormatter.php
@@ -46,13 +53,12 @@ class StringFormatter extends FormatterBase implements ContainerFactoryPluginInt
+   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
    */

Needs a description as per coding standards

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Status: Needs work » Needs review
FileSize
replace-depracated-entitymanager-2981025-8.patch5.78 KB
interdiff6-8.txt1.73 KB

Ahh.. silly mistake. :) Added param description.

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Assigned: Stockticker » Unassigned
barthje’s picture

barthje CreditAttribution: barthje commented
Status: Needs review » Needs work

I noticed you corrected the comments in one place to "the entity type manager" but in StringFormatter you call it "The entity manager" again, it should be the entity type manager.

barthje’s picture

barthje CreditAttribution: barthje commented
Status: Needs work » Needs review
FileSize
replace-depracated-entitymanager-2981025-11.patch5.79 KB
Mirnaxvb’s picture

Mirnaxvb CreditAttribution: Mirnaxvb as a volunteer and at Synetic commented
Status: Needs review » Reviewed & tested by the community

All comments are correct now.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

Committed 62e59a0 and pushed to 8.6.x. Thanks!

  • catch committed 62e59a0 on 8.6.x 
    Issue #2981025 by msankhala, barthje, Stockticker, penyaskito, alexpott...
catch’s picture

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

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito at Lingotek commented

Not sure how we should deal with this. But even if it wasn't declared, people may assume there is a property (dynamic) called
$entityManager. This would be a backward compatibility issue then if we "rename" it to $entityTypeManager.

Status: Fixed » Closed (fixed)

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