Overview

Ensure Pages work with Metatag, which normally requires a site builder to add a "metatag" field to a bundle.

Proposed resolution

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

User interface changes

Command icon Show commands

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

mglaman created an issue. See original summary.

mglaman’s picture

Component: Data model » Page
wim leers’s picture

Title: Page has Metatag integration » [PP-1] Page has Metatag integration
Issue summary: View changes
Status: Active » Postponed
lauriii’s picture

Title: [PP-1] Page has Metatag integration » Page has Metatag integration
Issue summary: View changes
lauriii’s picture

Status: Postponed » Active
mglaman’s picture

Assigned: Unassigned » mglaman

mglaman’s picture

Status: Active » Needs work
wim leers’s picture

Looking great already! Did a pre-emptive review 🤓

mglaman’s picture

Assigned: mglaman » Unassigned
Status: Needs work » Needs review

Should be set, now.

mglaman’s picture

attilatilman’s picture

Pipeline failure is not because of the MR changes, it is a different issue. MR LGTM!

wim leers’s picture

Status: Needs review » Postponed (maintainer needs more info)
Issue tags: +Needs documentation updates

I'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:

* @see https://git.drupalcode.org/project/metatag/-/blob/2.0.x/src/Plugin/Field/FieldType/MetatagFieldItem.php

in \Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem.

So why is the Metatag module not running into the same problem? 🤔

wim leers’s picture

Transplanted the relevant bits from #13 to the core bug report: #3487533-8: Cannot modify a table which uses JSON type.

wim leers’s picture

Assigned: Unassigned » mglaman
Status: Postponed (maintainer needs more info) » Needs work

This is looking great! 👏

Concerns:

  1. developing XB now requires a changed setup: a core patch now is required 😬 — also not clear why the Metatag module doesn't have the same problem?! See #14.
  2. the ::setProvider() magic trick that @larowlan approved needs some docs, because it is not at all obvious nor common (but neat!)
  3. Default config for Metatag: how do we avoid the need for the XB module to need to provide an update path now?
mglaman’s picture

re #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.

mglaman’s picture

The MR has been updated to use the approach found in the json_field module, where SQLite uses the "text" type, avoiding the core patch.

mglaman’s picture

Assigned: mglaman » Unassigned
Status: Needs work » Needs review
wim leers’s picture

Assigned: Unassigned » tedbow
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs documentation updates
Related issues: +#3450957: Prevent modules from being uninstalled if they provide field types used in an Experience Builder field

Now only needs final sign-off from @tedbow WRT for this change:

-          'sqlite_type' => 'json',
+          // @todo Change back to 'json' once https://www.drupal.org/i/3487533 is resolved.
+          'sqlite_type' => 'text',

(Due to @tedbow's work on #3450957: Prevent modules from being uninstalled if they provide field types used in an Experience Builder field.)

tedbow made their first commit to this issue’s fork.

  • tedbow committed 8cbf94aa on 0.x authored by mglaman
    Issue #3487077 by mglaman, wim leers: Page has Metatag integration
    
tedbow’s picture

Status: Reviewed & tested by the community » Fixed

🎉

wim leers’s picture

Assigned: tedbow » Unassigned

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.