ApiLayoutController::getRegionForComponentInstance doesn't work well with nested components if there are multiple regions.

This fails at ApiLayoutController::267 because $regionForComponentId is null and it's not able to get the region even though content region exists under which this component is placed.

That's

      if ($regionForComponentId !== XbPageVariant::MAIN_CONTENT_REGION) {
        if (!$page_regions[$regionForComponentId]->access('edit')) {
          throw new AccessDeniedHttpException(sprintf('Access denied for region %s', $regionForComponentId));
        }

So … this would mean that $page_regions[$regionForComponentId] resolves to NULL, meaning that $regionForComponentId is NOT one of the existing PageRegion config entities?! 😱

Can you please apply this patch to get a more precise error message?

$ git diff
diff --git a/src/Controller/ApiLayoutController.php b/src/Controller/ApiLayoutController.php
index f34ce46cc..570082478 100644
--- a/src/Controller/ApiLayoutController.php
+++ b/src/Controller/ApiLayoutController.php
@@ -264,6 +264,9 @@ final class ApiLayoutController {
     if (!empty($page_regions)) {
       $regionForComponentId = $this->getRegionForComponentInstance($data['layout'], $componentInstanceUuid);
       if ($regionForComponentId !== XbPageVariant::MAIN_CONTENT_REGION) {
+        if (!array_key_exists($regionForComponentId, $page_regions)) {
+          throw new \LogicException(sprintf("Component instance %s is in a region (%s) for which no PageRegion config entity exists. Existing PageRegions: %s.", $componentInstanceUuid, $regionForComponentId, implode(', ', array_keys($page_regions))));
+        }
         if (!$page_regions[$regionForComponentId]->access('edit')) {
           throw new AccessDeniedHttpException(sprintf('Access denied for region %s', $regionForComponentId));
         }

Oh wait:

because $regionForComponentId is null

If true, this would most likely be a bug in \Drupal\experience_builder\Controller\ApiLayoutController::getRegionForComponentInstance()?

What would be really helpful is you sharing the full request body that triggered that 500 response 🙏 That would allow us to easily write a failing test case to ApiLayoutControllerPatchTest.

I walked through the xdebug of this. The component was in the `content` region. I think the bug is in the `slots` code

          // Maybe it's not a component, but a slot inside a component.
          foreach ($componentData['slots'] as $slotData) {
            foreach ($slotData['components'] as $slotComponentData) {
              if ($slotComponentData['uuid'] === $componentInstanceUuid) {
                return TRUE;
              }
            }
          }

It was nested 2 or 3 levels deep.

Bumping priority and tagging data loss, because AFAICT #3535078: Bug in `generateComponentTreeKeys()` causes XB config entity containing a component tree to loses some component instances upon creation is a duplicate bug report!

Tom, could you take a look at this today? 🙏

This blocks #3535078, see #3535078-9: Bug in `generateComponentTreeKeys()` causes XB config entity containing a component tree to loses some component instances upon creation. More likely: that is a duplicate of this issue: different symptom, same root cause. Still, let's double-check there after this lands 👍

This is ready with all tests green. Let me know if we can create a follow-up ticket for tests or if we should include here the tests

Yes, we do, hence the Needs tests tag 😇

See #9 for a sample payload, but it sounds like you were able to reproduce this yourself already :)

I think this should be a beta blocker, its pretty easy to get this error.

Pushed a failing test and rebased off 0.x

If you checkout 185d8c32 and run ApiLayoutControllerPatchTest it fails.

We already had most of the required structure in XbTestSetup, just needed to do some fernangling to mirror the nesting.

Adding the tag, asking for forgiveness not permission etc

2 questions on the MR — one about a remaining @todo, one about something I don't understand.

Thanks!

Landing #3530258: Add rudimentary conflict prevention to the Config Auto-save endpoint first though.

… but let's land !1204 first, that is much harder to reroll.

Sure! feel free to throw it back to me if we need a reroll here.

I have a follow-up MR ready at https://git.drupalcode.org/project/experience_builder/-/merge_requests/1269.

#3530258: Add rudimentary conflict prevention to the Config Auto-save endpoint is in! Merging this as soon as CI passes.

@penyaskito THAT WAS FAST 😱