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 #2068325: [META] Convert entity SQL queries to the Entity Query API. See the parent issue for details.
Comment | File | Size | Author |
---|---|---|---|
#14 | 2068343_13.patch | 4.44 KB | slashrsm |
#14 | interdiff.txt | 2.87 KB | slashrsm |
#12 | 2068343_12.patch | 2.89 KB | slashrsm |
#12 | interdiff.txt | 2.72 KB | slashrsm |
#7 | 2068343_7.patch | 1.22 KB | slashrsm |
Comments
Comment #1
slashrsm CreditAttribution: slashrsm commentedWorking on this.
Comment #2
slashrsm CreditAttribution: slashrsm commentedGo for it, testbot!
Comment #4
slashrsm CreditAttribution: slashrsm commentedLet's try what happens w/o change in function system_update_8024().
Comment #5
twistor CreditAttribution: twistor commentedNo need to have $fid be by reference.
No need for empty(). Switch the conditions.
These files should be bulk-loaded before the foreach.
$controller->loadMultiple()
Can we convert update functions to entity query?
Edit: Leaving at NR for testbot.
Comment #7
slashrsm CreditAttribution: slashrsm commentedFixed comments. chx confirmed on IRC that we do not touch update hooks, so I removed changes to user_update_8011().
Comment #8
slashrsm CreditAttribution: slashrsm commented#7: 2068343_7.patch queued for re-testing.
Comment #10
slashrsm CreditAttribution: slashrsm commented#7: 2068343_7.patch queued for re-testing.
Comment #11
jibranThis plugin needs injection @see Plugins can receive injected dependencies from the container
Comment #12
slashrsm CreditAttribution: slashrsm commentedGood point.
Comment #13
dawehnerSeems to be just a copy and paste of some other code.
You missed to document the QueryFactory.
... so we needs tests? This does NOT set $entityQuery so it should not work.
Comment #14
slashrsm CreditAttribution: slashrsm commentedI'm ashamed... However, all comments from #13 should be fixed in attached patch. The only view in core that currently uses this argument plugin is file listing (detailed usage info part). I added some more tests to that, that should also catch non-working Fid plugin.
Comment #15
peximo CreditAttribution: peximo commentedI have tested the patch and it works, also there's no coding standard problems; all seem good for me.
Comment #16
alexpottCommitted 0ff5e6d and pushed to 8.x. Thanks!
Comment #17
slashrsm CreditAttribution: slashrsm commentedYAY!
Comment #19
plach