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
There is no longer a way to gain the context of the block/entity within a condition plugin. This has made it impossible for me to upgrade Per User Block to the new condition plugins.
This module is a backport of the functionality that was wrote when we removed it from D8.
Relevant usage is here: http://cgit.drupalcode.org/per_user_block/tree/src/Plugin/Condition/PerU...
Proposed resolution
To be discussed.
Remaining tasks
Agree upon a solution, write patch.
User interface changes
n/a
API changes
n/a
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | interdiff.txt | 1.1 KB | benjy |
| #7 | 2374295-7.patch | 1.29 KB | benjy |
| #5 | 2374295-5.patch | 860 bytes | benjy |
| #2 | 2374295-1.patch | 865 bytes | benjy |
Comments
Comment #1
EclipseGc CreditAttribution: EclipseGc commentedSo, this is really screwy which is why I proposed we accept this regression. If we want to do this for real, you need to document the block as a required context (in the annotation) of this condition. I have a notion of HOW we could do this. After #2339151: Conditions / context system does not allow for multiple configurable contexts, eg. language types lands, the Block entity itself will be passed the context array. We might be able to ad-hoc add the current block to the context stack so that this can be evaluated, but all my efforts are focussed there right now. Once that lands, I'm way more up for trying to address this.
Eclipse
Comment #2
benjy CreditAttribution: benjy commentedAs discussed with EclipseGc in IRC one approach would be as shown in the attached patch (written by EclipseGc). The patch depends on #2339151: Conditions / context system does not allow for multiple configurable contexts, eg. language types so leaving as active for now.
Comment #3
EclipseGc CreditAttribution: EclipseGc commentedShould use Core's version of Context, but otherwise yeah, this would make the current block available to conditions to evaluate it if necessary.
Eclipse
Comment #4
EclipseGc CreditAttribution: EclipseGc commentedAlso, I reserve the right to completely disown this patch if at some point I become convinced this is just insane. We abandoned this particular feature (that this issue is attempting to re-enable) for a purpose, which was that we are pursuing a vastly different block placement model. True, D8 still has a lot of similarities to D7, and we obviously CAN make per-user block visibility work, but I'm not sure we want to. This is totally overridable in contrib, I have yet to be convinced one way or another, but as a technical challenge it was interesting, and there is a solution. I have no clue how we'd migrate this to a more panels-style block rendering style though.
Eclipse
Comment #5
benjy CreditAttribution: benjy commentedOK, I re-rolled the patch and updated to use the core class now #2339151: Conditions / context system does not allow for multiple configurable contexts, eg. language types is in. We don't have anywhere in core that uses this functionality which makes it hard to test from a web test.
Comment #6
tim.plunkettThis should probably be namespaced better, maybe block.entity or block.block_entity?
Also this means that two objects are instantiated with every call to this each time, is it worth storing it somewhere?
Comment #7
benjy CreditAttribution: benjy commentedI know EclipseGc expressed concerns about this issue in general, if we do end up deciding against it, this is how I solved it in contrib: http://cgit.drupalcode.org/per_user_block/commit/?id=654aeb3d05ea0ff2bf3...
Now storing the context in a class property and i namespaced as "block.block_entity". I'm using the tests from per_user_block to test these changes locally but still don't have a test for core.
Comment #8
benjy CreditAttribution: benjy commentedOne other thing that is strange because we have the namespace hardcoded is that to use this in your annotation you have to do this:
Where the key is always block.block_entity but with any other condition, you can use whatever keys you want, even storing the same context in two different keys?
Comment #9
tim.plunkettThe same context cannot be used in multiple keys until #2378585: Multiple context requirements cannot be satisfied by a single value.
And no, you don't need to do that in your annotation, that's why we have context_mapping.
You would just need to resave the block in the UI after changing the context ID, it will then map whatever you have in your annotation to "block.block_entity" for you.
Comment #10
EclipseGc CreditAttribution: EclipseGc commentedYup to everything tim said.
Eclipse
Comment #11
benjy CreditAttribution: benjy commentedI read this in the docs at the top of Drupal\Core\Annotation\ContextDefinition, thanks for pointing me to that issue.
Unfortunately, I can't get the per_user_block tests to pass without that annotation. I get an invalid context error. You could try running the tests from per_user_block yourself locally to see what i mean or I could roll a patch with it included into core?
Comment #12
tim.plunkettAh, @EclipseGc hit this exact problem today:
If you are submitting this through the UI, it will be handled automatically.
But if you are using drupalPlaceBlock, you have to do it manually.
See the bottom of the interdiff in #2339151-165: Conditions / context system does not allow for multiple configurable contexts, eg. language types for an example.
Comment #18
pritam.tiwari CreditAttribution: pritam.tiwari commentedI am also facing the same problem. Not able to get the block details in plugin condition evaluate.