Client requests component props form with incomplete parameters early in the rendering process

Is the warning happening on this line?

$component_machine_name = json_decode($this->getRequest()->get('tree'), TRUE)['type'];

If so, what is the value of $this->getRequest()->get('tree') when the warning happens?

When this happens tree is the string null. The culprit is DummyPropsEditForm:

    const tree = findNodeInObjectTree(layout, selectedComponent);
    const query = new URLSearchParams({
      tree: JSON.stringify(tree),
      props: JSON.stringify(preparedModel),
      selected: selectedComponent,
      latestUndoRedoActionId,
    });
    setDynamicStaticCardQueryString(`?${query.toString()}`);

findNodeInObjectTree can be null if we didn't find the selected component; on page load this appears to happen sometimes while things are still being loaded into place; the actual tree (layout) has no children at this point. In manual testing there always seemed to be a valid request immediately afterwards, so we maybe can just skip the case where tree is null and not trigger the query?

Unsure if or how to write a test for this.

1. I think the fix should be different: the UI should inform the user that the requested component instance longer exists. While the current MR may subdue the symptoms, it does so by using the ostrich technique 😅
2. Why isn't the OpenAPI request validation not detecting this? 🤔 Ah … because /openapi.yml is incomplete! AFAICT this addition should surface the problem right away:
   

    openapi.yml | 6 ++++++
    1 file changed, 6 insertions(+)
   
   diff --git a/openapi.yml b/openapi.yml
   index 6b5c2af8..bfba2303 100644
   --- a/openapi.yml
   +++ b/openapi.yml
   @@ -149,6 +149,12 @@ paths:
        parameters:
          - $ref: '#/components/parameters/entityTypeId'
          - $ref: '#/components/parameters/entityId'
   +      # A JSON blob representing the edited component subtree (a single node, really).
   +      - in: query
   +        name: tree
   +        required: true
   +        schema:
   +          type: string
        get:
          responses:
            '200':
   
   

   👆 Additions like that are why I asked 📌 Add missing API paths + request bodies to `openapi.yml` Active to be opened for.

I wanted to capture what's happening in the issue title. We're looking at a race condition issue in which early on in the rendering process we attempt to compose the required parameters to retrieve the component props form, but we don't yet have all the data available to do so, then we proceed to send the request anyway. We need to wait until the layout is fetched.

This was already diagnosed in #6. I think all we need is to detect that the data is not yet available even sooner. I proposed the solution in a thread on the MR.

@wim leers, are you happy wit this? 😊

Looking at the fix, this code a few lines down seems somewhat related as well:

    const node = findNodeByUuid(layout, selectedComponent);
    const selectedComponentType = node ? (node.type as string) : 'noop';

noop should never be set now we have the additional guard?

#13: I think we can still keep that in case findNodeByUuid can't return the node for some reason.

#12: No, because /openapi.yml has not been updated … but that's what 📌 Add missing API paths + request bodies to `openapi.yml` Active exists for so 👍

@balintbrews Thoughts about #13?

Of course, I didn't mean to ignore the required /openapi.yml changes, but saw that we have another issue for that. 🙂

Replied to #13 just moments before your comment.

Re #14 but wondering if instead of this special "noop" string (which I'm not sure is handled anywhere and perhaps just to satisfy TypeScript?) we should display an error to the user, in the case that the layout does have children, but we didn't find the child we are looking for?

I don't think that that belongs to this issue. Here is my reasoning.

We have error boundaries in the app which will catch that problem if it happens. In this case, the one placed in ContextualPanel.tsx will display an error if anything throws in that component subtree. Whether an error will actually be thrown is outside of the scope of this issue: it's not the root cause of the reported problem. The problem is caused by a race condition, which is now fixed.

I'm going to merge this given it's a small change and the DummpyPropsEdit file the change is in is not intended to be final anyway.

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