[PP-1] Integrate saving sections with the backend

👍 Thanks!

This issue's scope will be actually integrating the frontend pieces with the new backend endpoint(s)? for saving a section.

Yep, but the endpoints actually already exist, i.e. the following routes:

• experience_builder.api.config.list
• experience_builder.api.config.get
• experience_builder.api.config.patch
• experience_builder.api.config.post
• experience_builder.api.config.delete

… but they currently only support the PageTemplate config entity, because that's the only XB-owned config entity type that implements \Drupal\experience_builder\Entity\XbHttpApiEligibleConfigEntityInterface — Assigned to: f.mazeikis 📌 Introduce a `Pattern` config entity Active will add the Pattern config entity type that also implements that interface 👍

📌 Introduce a `Pattern` config entity Active is in!

It seems that the FE is already connected but I'm getting an error when trying to use this. This probably needs someone on the backend to debug this.

The error with a backtrace:

Error: Call to undefined method Drupal\Core\Config\Schema\Undefined::getIterator() in Drupal\Core\Entity\Plugin\DataType\ConfigEntityAdapter->getIterator() (line 74 of /var/www/html/drupal/core/lib/Drupal/Core/Entity/Plugin/DataType/ConfigEntityAdapter.php).
Backtrace	
#0 /var/www/html/drupal/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php(163): Drupal\Core\Entity\Plugin\DataType\ConfigEntityAdapter->getIterator()
#1 /var/www/html/drupal/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php(106): Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode()
#2 /var/www/html/drupal/core/lib/Drupal/Core/TypedData/Validation/RecursiveValidator.php(93): Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validate()
#3 /var/www/html/drupal/core/lib/Drupal/Core/TypedData/TypedData.php(132): Drupal\Core\TypedData\Validation\RecursiveValidator->validate()
#4 /var/www/html/drupal/modules/custom/experience_builder/src/Controller/ApiConfigControllers.php(106): Drupal\Core\TypedData\TypedData->validate()
#5 [internal function]: Drupal\experience_builder\Controller\ApiConfigControllers->post()
#6 /var/www/html/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array()
#7 /var/www/html/drupal/core/lib/Drupal/Core/Render/Renderer.php(593): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#8 /var/www/html/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(121): Drupal\Core\Render\Renderer->executeInRenderContext()
#9 /var/www/html/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext()
#10 /var/www/html/drupal/vendor/symfony/http-kernel/HttpKernel.php(183): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#11 /var/www/html/drupal/vendor/symfony/http-kernel/HttpKernel.php(76): Symfony\Component\HttpKernel\HttpKernel->handleRaw()
#12 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/Session.php(53): Symfony\Component\HttpKernel\HttpKernel->handle()
#13 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle()
#14 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/ContentLength.php(28): Drupal\Core\StackMiddleware\KernelPreHandle->handle()
#15 /var/www/html/drupal/core/modules/big_pipe/src/StackMiddleware/ContentLength.php(32): Drupal\Core\StackMiddleware\ContentLength->handle()
#16 /var/www/html/drupal/core/modules/page_cache/src/StackMiddleware/PageCache.php(116): Drupal\big_pipe\StackMiddleware\ContentLength->handle()
#17 /var/www/html/drupal/core/modules/page_cache/src/StackMiddleware/PageCache.php(90): Drupal\page_cache\StackMiddleware\PageCache->pass()
#18 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\page_cache\StackMiddleware\PageCache->handle()
#19 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
#20 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php(36): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
#21 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\AjaxPageState->handle()
#22 /var/www/html/drupal/core/lib/Drupal/Core/DrupalKernel.php(709): Drupal\Core\StackMiddleware\StackedHttpKernel->handle()
#23 /var/www/html/drupal/index.php(19): Drupal\Core\DrupalKernel->handle()
#24 {main}

Here's the request:

fetch("https://drupal-dev.ddev.site/xb/api/config/pattern", {
  "headers": {
    "accept": "*/*",
    "accept-language": "en-US,en;q=0.9",
    "content-type": "application/json",
    "priority": "u=1, i",
    "sec-ch-ua": "\"Google Chrome\";v=\"131\", \"Chromium\";v=\"131\", \"Not_A Brand\";v=\"24\"",
    "sec-ch-ua-mobile": "?0",
    "sec-ch-ua-platform": "\"macOS\"",
    "sec-fetch-dest": "empty",
    "sec-fetch-mode": "cors",
    "sec-fetch-site": "same-origin"
  },
  "referrer": "https://drupal-dev.ddev.site/xb/node/1/editor/component/6e4aac04-cb0d-4431-87df-afaec573375a",
  "referrerPolicy": "strict-origin-when-cross-origin",
  "body": "{\"layout\":{\"nodeType\":\"root\",\"uuid\":\"root\",\"children\":[{\"children\":[{\"uuid\":\"6e4aac04-cb0d-4431-87df-afaec573375a-slot-column_one\",\"name\":\"column_one\",\"nodeType\":\"slot\",\"children\":[{\"children\":[],\"nodeType\":\"component\",\"type\":\"sdc.experience_builder.image\",\"uuid\":\"1b4b7c65-b356-4b22-abb9-d1bccaad4135\"}]},{\"uuid\":\"6e4aac04-cb0d-4431-87df-afaec573375a-slot-column_two\",\"name\":\"column_two\",\"nodeType\":\"slot\",\"children\":[{\"children\":[],\"nodeType\":\"component\",\"type\":\"sdc.experience_builder.heading\",\"uuid\":\"b1b626de-6653-453c-aa26-32e2a79c2c0e\"}]}],\"nodeType\":\"component\",\"type\":\"sdc.experience_builder.two_column\",\"uuid\":\"6e4aac04-cb0d-4431-87df-afaec573375a\"}]},\"model\":{\"6e4aac04-cb0d-4431-87df-afaec573375a\":{\"name\":\"Two Column\",\"width\":25},\"b1b626de-6653-453c-aa26-32e2a79c2c0e\":{\"name\":\"Heading\",\"text\":\"A heading element\",\"style\":\"primary\",\"element\":\"h1\"},\"1b4b7c65-b356-4b22-abb9-d1bccaad4135\":{\"name\":\"Image\",\"image\":{\"src\":\"https://placehold.co/600x400.png\",\"alt\":\"Boring placeholder\",\"width\":600,\"height\":400}}},\"name\":\"Two Column section\"}",
  "method": "POST",
  "mode": "cors",
  "credentials": "include"
});

It seems that the FE is already connected

It isn't? 🤔 That's what this issue is supposed to do!

AHHHHHHHH, I see now! @jessebaker added this in 📌 Allow saving component compositions as sections Postponed :

    getSections: builder.query<any, void>({
      query: () => `/xb/api/config/pattern`,
    }),
    saveSection: builder.mutation<
      { html: string },
      { layout: RootNode; model: ComponentModels; name: string }
    >({
      query: (body) => ({
        url: '/xb/api/config/pattern',
        method: 'POST',
        body,
      }),
    }),

and assumed that that the current client-side data model for component trees would automatically and transparently work for Pattern config entities too. That's not the case though, because that data model is known to be in need of evolution: 🐛 Some components cannot be used in XB because UI prevents SDC props being named `name` Active .

Problems with the current front-end code cited in #6:

1. The required label for a section is missing. Or are we saying that they should not have labels? 🤔 If so, happy to remove that from the server-side logic 👍
2. Last paragraph of #6: the shape of the data uses the current client-side data model, which is not necessarily a good thing. See 🐛 Some components cannot be used in XB because UI prevents SDC props being named `name` Active .
3. NO error handling! 😅 The server side can return validation errors!

Now, I did anticipate that second point. That's why there's

    // ⚠️ For now, there's no denormalization. This may change in the future.
    $denormalized = $decoded;

… in \Drupal\experience_builder\Controller\ApiConfigControllers::post() and ::patch().

It seems like the most pragmatic approach is to:

1. not block this on 🐛 Some components cannot be used in XB because UI prevents SDC props being named `name` Active
2. apply the same denormalizations/transformations from the client to the server (some of the work in ✨ AutoSave Entity Revision POC Active will help)
3. Follow-up for the UI to support error responses.

Missing context: I expected 🐛 Some components cannot be used in XB because UI prevents SDC props being named `name` Active to have been completed weeks ago, and that'd have likely removed the need for bringing the same client-to-server-transformation-and-back functionality to this MR that we already have for editing content entities' component trees.

That's why I'm a bit hesitant, and why I'd like explicit confirmation from @lauriii.

The required label for a section is missing. Or are we saying that they should not have labels? 🤔 If so, happy to remove that from the server-side logic 👍

The API should enforce a label. We should prevent saving sections without a label.

Last paragraph of #6: the shape of the data uses the current client-side data model, which is not necessarily a good thing. See #3467954: Evolve XB UI's data model to allow non-SDC components' inputs, DynamicPropSource support, etc..

This is not necessarily urgent. I was hoping we could demo this at DrupalCon in case that this was really close to being done but it seems that it isn't the case. I'd be fine with us postponing this on 🐛 Some components cannot be used in XB because UI prevents SDC props being named `name` Active and re-evaluating closer to 0.2.0 if that hasn't been done.

NO error handling! 😅 The server side can return validation errors!

I wonder if this is covered by the global error handling? When I tested this, I was getting a fairly graceful handling of errors within the modal.

The API should enforce a label.

Okay! Then the client must provide a label. We can't auto-guess a label. But for now I'll go with Section N, with N auto-incrementing. That avoids being blocked on the client side.

I'll do a time-boxed iteration on this to see if I can get the pragmatic solution I outlined in #7 done in an hour.

f.mazeikis → made their first commit to this issue’s fork.

@f.mazeikis told me a few days ago he was working on it — I see that the issue metadata was out of date 😅

I talked with @f.mazeikis about this issue today

I wanted to see if the logic we added in ClientDataToEntityConverter could be used in this case also. When added in [##3489994] it hardcoded assumptions that we would be updating an entity with a ComponentTreeItem which of course in sections we won't be. But we did have most of the logic for converting a `layout` and `model` from the client to a `tree` and `props` that server can save.

So I moved most of this logic to a new ClientServerConversionTrait::convertClientToServer. These leaves both ClientDataToEntityConverter::convert and ApiConfigControllers::decode very thin as they can both rely on the the new convertClientToServer

I talked with @f.mazeikis about we need to figure out if we can centralize the conversion from client to the server and then back. But this at least puts the logic for client to server in 1 place for converting a `layout` and `model` from the client to a `tree` and `props`. there really wasn't any new code in here just moving code around that already went through review

Thanks for helping out here, @tedbow! What you did is exactly what I expected 😊👍

Perhaps we should land @tedbow's !444 MR first, because it simplifies the rest of the work in this issue and reduces the accumulation of conflicts with other MRs? 😊

For that reason, I am working to get the MR to green so I can approve it — at which point either of you feel free to merge it if you like that!

Using this button on a two column component instance containing one component per column:

… and then reloading the page (needs follow-up: the UI needs to refresh the list after creating a section!), it actually works:

(Although you can see that the preview is not yet working.)

IMHO once this passes tests, this MR can be merged (for DrupalCon Singapore demo purposes), and the preview + refreshing of the list can happen in subsequent MRs in either this issue or a follow-up.

Still needs work for updating XbConfigEntityHttpApiTest::testPattern() to send request bodies shaped like the client-side data model.

going to work on this

Thank, @f.mazeikis!

That made the preview look less broken, but not yet actually working:

Self-assigning because @tedbow hasn't been pushing anything. This feels close, and it overlaps significantly with work I helped land on both ApiComponentsController (the default_markup stuff @f.mazeikis just added) and ApiPreviewController.

I am reviewing those bits specifically for their cacheability correctness, and hopefully to get the preview to fully work in a short amount of time :)

Markup-only preview of Sections/Patterns is now working:

Next: CSS + JS.

Now previews are fully operational:

I wrote

IMHO once this passes tests, this MR can be merged (for DrupalCon Singapore demo purposes), and the preview + refreshing of the list can happen in subsequent MRs in either this issue or a follow-up.

… but that's before I saw the extensive changes to ApiConfigControllers, which explain why XbConfigEntityHttpApiTest::testPattern() is not passing. This MR as-is is too incomplete to be mergeable; it'd leave XB in a very confusing state: parts of the UI start working, but the thorough HTTP API test coverage no longer passes because the refactor towards the client-side data model is good, but partial.

RE #22 I did a hands-on review where I pushed commits to show the direction I'd like to see rather than describing it, which would've been far more ambiguous.

I've already ">roughly implemented the decode-vs-denormalize split, the same thing will be needed for ::encode() (a new ::normalize() method etc, with a @todo to convert to NormalizerInterface in a future MR).

Now the MR has largely passing \XbConfigEntityHttpApiTest::testPattern(), but the post() method is not yet calling ::normalize(), ::patch() is not yet calling neither ::denormalize() nor ::normalize() and ::get() and ::list() still need to be updated to call ::normalize().

I think introducing all the DenormalizerInterface + NormalizerInterface infrastructure in this MR will lead us too far. IOW: I think we should write this using method names matching https://symfony.com/doc/current/serializer.html#the-serialization-proces..., per config entity type. That will make it easy to convert to that Symfony infrastructure in a subsequent MR :)

Chatted with @wim leers and created an issue to extract out just the changes for ClientServerConversionTrait and ClientDataToEntityConverter 📌 Provide a utility method to convert client layout and model to server tree and props Active

Good to see this progressing towards a green CI pipeline!

Posted an intermediary review. 🏓 Would love to see this get to an RTBC'able state! 😄

The bit that @f.mazeikis researched I confirmed and extracted into a separate issue+MR that make this MR slightly simpler :) — see 📌 Remove pointless asset library dependencies/overrides on `core/once` Active .

This cannot land until that other issue lands, but there's plenty of other remarks that are not yet addressed — by tackling those, this should become RTBC'able! :)

📌 Remove pointless asset library dependencies/overrides on `core/once` Active is in, this can be simplified now 👍

Great to see this approaching the finish line! 😄🏁