Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Part of meta-issue #2225347: [META] Replace calls to ENTITY_TYPE_load() and ENTITY_TYPE_load_multiple() with static method calls
See the detailed explanations and examples there.
Comment | File | Size | Author |
---|---|---|---|
#35 | 2322105-35.patch | 33.38 KB | rpayanm |
Comments
Comment #1
Temoor CreditAttribution: Temoor commentedComment #2
Michael Hodge Jr CreditAttribution: Michael Hodge Jr commentedI've applied the patch, grepped the code, and can confirm everything looks as it should. Setting to RTBC.
Comment #3
Temoor CreditAttribution: Temoor commentedReplaced #machine_name callback in VocabularyForm.
Comment #4
alexpottInject the storage into the plugins
This should be done through dependency injection too. See ActionFormBase for an example.
Comment #5
Upchuk CreditAttribution: Upchuk commentedPatch needs reroll and will attempt to fix the remaining issue raised in #4.
Comment #6
Upchuk CreditAttribution: Upchuk commentedThe entity_reference TermSelection plugin uses ReflectionFactory which does not use the container. Fix proposed in #2107243: Decouple entity reference selection plugins from field definitions
Comment #7
Upchuk CreditAttribution: Upchuk commentedFieldType plugins are using TypedData factories so no dependency injection. See #2053415: Allow typed data plugins to receive injected dependencies
Comment #8
Upchuk CreditAttribution: Upchuk commentedAttaching txt with reroll (please review), an interdiff since the reroll and the patch which contains the storage injections.
The patch is missing the storage injection for the following cases:
FieldType plugin mentioned in #7
TermSelection plugin mentioned in #6
VocabularyVid views argument plugin that I'm not sure it's being used. See #2048733: Taxonomy vocabulary contextual filters are missing
Comment #9
seanBPatch in #8 applied perfectly.
Found a extra call to entity_load('taxonomy_vocabulary') though, in /core/modules/views_ui/admin.inc on line 233.
Patch with that last change is attached.
Comment #11
Upchuk CreditAttribution: Upchuk commentedHey Sean,
Thanks for the patch, however it failed testing.
Can you please resubmit and include an interdiff of the work you put on top of #8 so we can see what you worked on?
I unassigned this issue now so you can assign it to yourself while you do that. After submitting the patch, you can unassign it again.
Thanks!
Comment #12
seanBSorry about that.. I'll do this later today!
Comment #14
seanBHere is the interdiff for #9.
Comment #15
rpayanmComment #18
jamesdixon CreditAttribution: jamesdixon commentedHere's my attempt at a reroll.
There were differences in bringing classes in via use since the last patch at the top of some files that needed resolving.
The patchutils interdiff was giving me the same error as reported here: https://fedorahosted.org/patchutils/ticket/23. Any good workarounds to that?
Comment #19
rpayanmLook fine for me :)
Comment #20
alexpottI think we should be injecting the vocabulary storage for these two.
Comment #21
rosinegrean CreditAttribution: rosinegrean commentedComment #22
skipyT CreditAttribution: skipyT commentedYou should rename this variable to $vocabulary_storage, we are injection here the storage object, not the interface
this one also should be $vocabularyStorage instead of $storage
lets rename this $vocabulary_storage
Comment #23
skipyT CreditAttribution: skipyT commentedOther stuff I missed before:
Here we need a comment like: "The Vocabulary storage."
The comment should be only: The vocabulary storage.
Comment #24
rosinegrean CreditAttribution: rosinegrean commentedComment #25
skipyT CreditAttribution: skipyT commentedComment #26
alexpottin core/modules/system/src/Tests/Entity/EntityCrudHookTest.php needs converting too.
Comment #27
rpayanmComment #28
pcambraRe-roll, this got in the middle: #2372855: Add content & config entity dependencies to views
Comment #29
aspilicious CreditAttribution: aspilicious commentedComment #31
rpayanmrerolled
Comment #33
rpayanmrerolled
Comment #35
rpayanmrerolled
Comment #36
alexpottCommitted 3b53231 and pushed to 8.0.x. Thanks!
Beta evaluation is in the meta issue.
Comment #38
kim.pepperQuick follow up:
This patch added an unknown and unused interface
use Drupal\Core\TypedData\AllowedValuesInterface
Comment #39
larowlangood find
Comment #40
webchickCommitted and pushed follow-up to 8.0.x. Thanks!