Problem/Motivation
HEAD runs a config entity query on block entities and then it filters the results. All this is hardwired into the FullPageVariant. More efficient storages would need to replace FullPageVariant.
Proposed resolution
Provide a standalone service to find visible block entities.
Remaining tasks
User interface changes
API changes
API addition of \Drupal\block\BlockRepository
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | interdiff.txt | 1.72 KB | tim.plunkett |
| #12 | 2367579-blocks-12.patch | 20.22 KB | tim.plunkett |
| #9 | interdiff.txt | 4.5 KB | tim.plunkett |
| #9 | 2367579-blocks-8.patch | 19.97 KB | tim.plunkett |
| #4 | interdiff.txt | 14.99 KB | tim.plunkett |
Comments
Comment #1
dawehnerExtracting that logic out into a separate service is a good idea in general. Whether the block storage is the right place is not clear for me.
Comment #2
tim.plunkettI am also not 100% sure this should be on storage, but it might be okay. For now, just tweaking some things and fixing the tests.
Comment #3
fabianx commentedI think this is a good abstraction, RTBC + 1 from me.
Comment #4
tim.plunkettThis feels much cleaner to me. We have a simple service that combines storage and access in a specific and needed method. It can be swapped out independently of the storage, and is actually then easier to test.
Comment #5
fabianx commentedRTBC!!! - Very nice idea!
Does this still need further tests as stated in issue summary or is this ready?
Comment #6
tim.plunkettYes, it has tests now, and the old ones are restored.
I'd considering this RTBC, and I ran it by @EclipseGc
Comment #7
tim.plunkettComment #8
fabianx commentedRTBC then
Comment #9
tim.plunkett@dawehner pointed out that theme negotiator is being misused here.
Comment #10
fabianx commentedStill RTBC
Comment #12
tim.plunkettOh, that returns an object instead of a string.
Comment #13
tim.plunkett#10 covered the intent of my change, I just did it wrong, so setting back to RTBC.
Comment #14
fabianx commentedAgree with RTBC
Comment #15
alexpottReclassifying this as a bug since having the
FullPageVariantdirectly accessing block and checking access is the presentation layer reaching through the business logic layer and directly into the storage layer. This is a bad idea and as the issue summary points out more efficient storages would have to replace the FullPageVariant.Comment #16
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Also this change makes life easier for contrib and to improve the performance of loading blocks per region. Committed ec7b056 and pushed to 8.0.x. Thanks!