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
In Layout Builder we have
* context_definitions = {
* "entity" = @ContextDefinition("entity"),
* "view_mode" = @ContextDefinition("string", required = FALSE),
* }
with an accompanying plugin alter hook that does
$definitions['overrides']->getContextDefinition('entity')
->addConstraint('EntityHasField', OverridesSectionStorage::FIELD_NAME);
Ideally we would be able to do something like
* context_definitions = {
* "entity" = @ContextDefinition("entity")->addConstraint('EntityHasField', OverridesSectionStorage::FIELD_NAME),
* "view_mode" = @ContextDefinition("string", required = FALSE),
* }
or even
* context_definitions = {
* "entity" = @ContextDefinition("entity", ['EntityHasField' => OverridesSectionStorage::FIELD_NAME]),
* "view_mode" = @ContextDefinition("string", required = FALSE),
* }
Proposed resolution
Add handling for constraints to \Drupal\Core\Annotation\ContextDefinition
allowing annotations like this:
* context_definitions = {
* "entity" = @ContextDefinition("entity", constraints = {
* "EntityHasField" = \Drupal\layout_builder\Plugin\SectionStorage\OverridesSectionStorage::FIELD_NAME,
* }),
* "view_mode" = @ContextDefinition("string", required = FALSE),
* }
Remaining tasks
Write tests
User interface changes
N/A
API changes
N/A
Data model changes
N/A
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#14 | 3016420-constraints-14.patch | 4.63 KB | tim.plunkett |
#13 | 3016420-constraints-13.patch | 4.57 KB | tim.plunkett |
#7 | 3016420-constraints-7-interdiff.txt | 1.82 KB | tim.plunkett |
#7 | 3016420-constraints-7.patch | 4.59 KB | tim.plunkett |
#4 | 3016420-constraints-4-PASS.patch | 2.77 KB | tim.plunkett |
Comments
Comment #2
tim.plunkettWith this patch, this works
Because only annotation namespaces are loaded, the fully-qualified class name is need for that constant, even though its for the file we're in.
Comment #3
EclipseGc CreditAttribution: EclipseGc at Acquia commentedyeah, I totally love that. The FQCN was to be expected. We've done this rodeo before heh.
Eclipse
Comment #4
tim.plunkettFAIL patch is the interdiff
Comment #6
EclipseGc CreditAttribution: EclipseGc at Acquia commentedComment #7
tim.plunkettThe code we wanted this for landed, so rerolled to include that.
Comment #8
EclipseGc CreditAttribution: EclipseGc at Acquia commentedWhat an enormous improvement.
Comment #9
tim.plunkettIt's a shame that the FQCN has to be repeated even within this file (instead of
static::FIELD_NAME
or something), but that's to be expected with the way that annotations are parsed.Comment #10
tim.plunkettComment #11
alexpottWe should have a change record to tell developers they can do this.
Comment #12
alexpottI was trying to think of a release note snippet but I'm not sure that
is worth it :) it is a rehash of the title.
Comment #13
tim.plunkettNW for the CR, but NR for a reroll after #2976148: Layout-based entity rendering should delegate to the correct section storage instead of hardcoding to either defaults or overrides
Comment #14
tim.plunkettReroll, no changes.
Comment #15
tim.plunkettAdded https://www.drupal.org/node/3029856
Comment #16
phenaproximaThat's a really nice DX improvement! Change record makes perfect sense.
Comment #17
alexpottCommitted and pushed 2dfe7b6d36 to 8.8.x and e716af79b9 to 8.7.x. Thanks!
Comment #20
alexpottComment #21
alexpottOopps yeah no release notes.
Comment #23
bkosborneIt's too late now, but one downside to the approach taken is that it's not possible to add multiple constraints of the same type in the annotation. I guess using the plugin alter hook would still work though, and it's probably a bit of an edge case (though one that I hit).
Comment #24
bkosborneHmmm I assumed I could alter block plugin defintions, but I don't see a hook for that anywhere. I just found this 7 year old issue where it was apparently added as a way to alter any plugin definition, but that code isn't there any more... https://www.drupal.org/project/drupal/issues/1705702
Comment #25
bkosborneOoookay, sorry for the noise everyone. Looks like it's actually a limitation of ContextDefinition. The constraints are stored in an array keyed by constraint name, so it wouldn't be possible to have multiple constraints of the same name anyway.