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.
Problem/Motivation
Currently sorting on load happens only for blocks but taxonomy and contact categories also needs this
Proposed resolution
- Add implementation ConfigStorageController::loadByProperties()
- Add tests
- Revert taxonomy tests
Commit the change
Related Issues
#1799600: Add test of sorting for configuration entities
#1981418: Drop taxonomy_vocabulary_sort() this depends on both issues
Original report by @tim.plunkett
And it needs some unit tests. Working on this.
Comment | File | Size | Author |
---|---|---|---|
#17 | 1893442-props-17.patch | 4.71 KB | andypost |
#14 | interdiff.txt | 1.1 KB | dawehner |
#14 | drupal-1893442-14.patch | 4.67 KB | dawehner |
#8 | 1893442-load-8.patch | 4.6 KB | andypost |
#6 | 1893442-interdiff-6.txt | 1.46 KB | andypost |
Comments
Comment #1
tim.plunkettSee attached.
Comment #2
xjmSome overlap with: #1846454: Add Entity query to Config entities
Comment #3
fubhy CreditAttribution: fubhy commentedI opened a related issue recently as well: #1887058: Remove usages of entity_load_multiple_by_properties() and EntityStorageController::loadByProperties(). I was toying with the idea of removing it altogether... entity_load_multiple_by_properties() as well as StorageController::loadByProperties(). Once that EFQ patch lands they will be mere wrappers for invoking EFQ. And we really should stop causing endless trails of function / method calls / forwards for no good reason.
Comment #4
andypostThere was a test in taxonomy vocabularies for this method, it was commented/removed in #1552396-71: Convert vocabularies into configuration
Comment #5
tim.plunkettThis is done.
Comment #6
andypost+1 to RTBC
Patch just restores removed tests as pointed in #4
Comment #7
andypost#6: 1893442-load-6.patch queued for re-testing.
Comment #8
andypostChasing HEAD
Comment #9
dawehnerDon't we have EQ for config entities as well? Sure it's doing basically the same, but I guess we could use it here.
Comment #10
andypostScope of the issue to move implementation to parent class
@dawehner for #9 I filed follow-up #1981516: Refactor ConfigStorageController to use entity query
Comment #11
dawehnerI don't think it's a problem that we have both tests for vocabulary and config entity in general.
Comment #12
alexpott#8: 1893442-load-8.patch queued for re-testing.
Comment #13
alexpottIt is possible for this test to generate a random fail as randomName() could return the same string. We have to ensure that the style value is different... i.e. set different lengths.
Comment #14
dawehnerHere is a small fix for that, but just by looking at the test I wouldn't assume that you actually need the ID here.
Comment #15
andypostNice catch
Comment #16
alexpottNeeds a re-roll...
Comment #17
andypostre-roll
Comment #18
tim.plunkett#17: 1893442-props-17.patch queued for re-testing.
Comment #19
alexpottCommitted 80da26f and pushed to 8.x. Thanks!
Comment #20.0
(not verified) CreditAttribution: commentedupdated summary