#3478537: Introduce an XB `PageTemplate` config entity added the ComponentTreeMeetRequirements validation constraint. It was designed to also support imposing the absence/presence of Blocks-as-Components, prior to #3475584: Add support for Blocks as Components landing.
And #3475584 has since landed, but due to MR size it was decided to land it an in incomplete state.
This is critical for data integrity, and should be rectified ASAP. It will also help #3479643: Introduce a `Pattern` config entity.
// @todo Remove this in Blocks-as-XB-Components: https://www.drupal.org/project/experience_builder/issues/3475584
if (str_starts_with($missing_value, 'block.') || str_starts_with($missing_value, 'Drupal\\')) {
continue;
}
in \Drupal\experience_builder\Plugin\Validation\Constraint\ComponentTreeMeetsRequirementsConstraintValidator::validate().
3475584 describe what should be tested.
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | Screenshot 2024-11-14 at 2.49.16 PM.png | 322.45 KB | wim leers |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
wim leersFrom 108 failures down to 100.
Expect this to move towards green quickly now 🤓
Comment #4
wim leersDown to 96.
Comment #5
wim leersNeeds review from @longwave and @f.mazeikis.
Is IMHO RTBC-worthy and mergeable.
If need be, I could split off the bit to a separate issue + MR, but it's a single commit: https://git.drupalcode.org/project/experience_builder/-/merge_requests/3... — although that uses PHP Fibers 😄
Comment #6
wim leersDamn that last bit 😬
Comment #8
wim leers@f.mazeikis FYI I’m working on !395 as we speak — I assigned it to me 50 mins ago.
Fortunately I made the exact same changes locally, but with an extra comment explaining it 😊
I’m currently investigating why:
config:search.settingsappears only in D11, not in D10X-Drupal-Cache-Max-Ageis-1on D11, but60on D10Comment #9
wim leers⚠️ I cannot reproduce #8.2 locally: it passes locally with
60!But … I've been testing using commit
48b0aea1d573d9e01752f9909ec8cd3a1b7e3378of the11.xbranch, which dates back to October 2.I already know what causes
max-age: 0, and it's XB that ups this to 60 seconds: it's\Drupal\user\Plugin\Block\UserLoginBlockwhen rendered byFormBuilderfor an authenticated user. Just pushed https://git.drupalcode.org/project/experience_builder/-/merge_requests/3... to make that very explicit in the test coverage.Intermediary conclusion: something changed since then. I bet this is thanks to #2578855: Form tokens are now rendered lazily, allow forms to opt in to be cacheable — see the change record.
Comment #10
wim leers#9's hypothesis is confirmed, although it seems somewhat accidental? 😅 See #2578855-165: Form tokens are now rendered lazily, allow forms to opt in to be cacheable, where I ask @catch about this.
Documented in https://git.drupalcode.org/project/experience_builder/-/merge_requests/3...
Comment #11
wim leers#8.1 is now also no longer a mystery: https://git.drupalcode.org/project/experience_builder/-/merge_requests/3...
Comment #12
wim leersLeft even more "Dear reviewer, here's helpful info"-style guiding comments on the MR, prefixed with
💁.MR is now actually green and definitely ready for review.
While I still want @f.mazeikis' +1, I'm assigning to @longwave because he worked on the cacheability aspects of
ApiComponentsControllerin #3484671: Add support for block derivatives, which this is keeping, but #3484671 is largely removing 😬 That's why I'd like this to land first, because it proves much more is possible, before we temporarily remove much of it in #3484671…Comment #13
wim leersSlightly clarifying title.
Comment #14
f.mazeikis commentedReviewed and approved. It seems MR still needs approval from @tedbow or @effulgentsia.
Comment #15
longwaveRTBC+1 - all points noted by Wim in the self-review make sense, some tiny little bugs fixed too!
Comment #16
longwaveComment #17
wim leersApprovals across the board, with one exception: technically I still need a +1 from @tedbow or @effulgentsia:
… but that's for one single tiny addition to test coverage: https://git.drupalcode.org/project/experience_builder/-/merge_requests/3... — I'm confident that is not contentious.
So moving ahead and merging.
Comment #19
wim leers