Closed (fixed)
Project:
Experience Builder
Version:
0.x-dev
Component:
Page
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
12 Nov 2024 at 20:51 UTC
Updated:
9 Dec 2024 at 15:34 UTC
Jump to comment: Most recent
Ensure Pages work with Metatag, which normally requires a site builder to add a "metatag" field to a bundle.
See #3482259: Landing page integration: new content entity type for unstructured content
Add conditional metatag basefield if the metatag module is installed, and default global config for xb_page
Start 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:
Comments
Comment #2
mglamanComment #3
wim leersComment #4
lauriiiComment #5
lauriiiComment #6
mglamanComment #8
mglamanComment #9
wim leersLooking great already! Did a pre-emptive review 🤓
Comment #10
mglamanShould be set, now.
Comment #11
mglamanThis has uncovered #3487533: Cannot modify a table which uses JSON type
Comment #12
attilatilman commentedPipeline failure is not because of the MR changes, it is a different issue. MR LGTM!
Comment #13
wim leersI'm concerned about applying #3487533: Cannot modify a table which uses JSON type — that's the first time XB is literally relying on a core patch. We've managed to avoid that so far.
The XB field type itself also uses JSON storage. See
\Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::schema().AHHHHHH — that is literally the code that revealed that core bug! 🤯😱
But I made this use exactly the same pattern that Metatag uses, hence this bit:
in
\Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem.So why is the Metatag module not running into the same problem? 🤔
Comment #14
wim leersTransplanted the relevant bits from #13 to the core bug report: #3487533-8: Cannot modify a table which uses JSON type.
Comment #15
wim leersThis is looking great! 👏
Concerns:
::setProvider()magic trick that @larowlan approved needs some docs, because it is not at all obvious nor common (but neat!)Comment #16
mglamanre #15
1. tests can be run with mysql locally, and it's one test that fails on sqlite without the patch
3. blame Drupal core for not providing enough ways to govern things via code and magically through config objects 😬, I'll try to find a way though.
Comment #17
mglamanThe MR has been updated to use the approach found in the
json_fieldmodule, where SQLite uses the "text" type, avoiding the core patch.Comment #18
mglamanComment #19
wim leersNow only needs final sign-off from @tedbow WRT for this change:
(Due to @tedbow's work on #3450957: Prevent modules from being uninstalled if they provide field types used in an Experience Builder field.)
Comment #22
tedbow🎉
Comment #23
wim leers