Multinode treepicker dynamic path only works after refresh

Laantje commented 5 months ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

13.1.0

Bug summary

When using the dynamic path option (in our case: 'Current' -> 'Nearest ancestor or self' on 'SiteRoot' document type which is one level under every root node) on a multinode treepicker datatype, and you create a new node with the same document type that uses the datatype, you have to save & refresh first before it starts working.

The part where you have to save first is understandable, since the node doesn't actually exist in the content tree yet. But the need to refresh after saving is a bit confusing, especially for our customers.

Specifics

Cannot provide an URL, haven't tested in other browsers but I'm using: Chrome Version 122.0.6261.69 (Official Build) (64-bit)

Steps to reproduce

Create a new Document Type
On that Document Type, create a Multinode treepicker with dynamic path enabled (in our case: 'Current' -> 'Nearest ancestor or self' on 'SiteRoot' document type which is one level under every root node)
Under 'Content', create a 'Content Node' with the newely created Document Type
Try to use the Multinode treepicker to fetch the content list of the dynamic root, it probably doesn't work (which makes sense, it shouldn't)
Now save and publish the node
Repeat step 4, it SHOULD work at this point, but it doesn't.
Now refresh the page and try again. At this point it does work correctly.

The extra refresh step is confusing for less technical users.

Expected result / actual result

The multinode treepicker should be listing the correct nodes from the content tree under the desired content after saving.

github-actions[bot] commented 5 months ago

Hi there @Laantje!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
We'll replicate the issue to ensure that the problem is as described.
We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

elit0451 commented 4 months ago

Hi @Laantje 👋

Thanks for reaching out! I am able to reproduce the issue. Here is a video showing it - as you said, only after a refresh, we get the correct data. It seems like we are not refreshing the data on the front end by callingGetApplicationTrees after saving. I will mark this as up for grabs as we would like some help fixing this 🙂

https://github.com/umbraco/Umbraco-CMS/assets/21998037/2e0155ae-3b06-4b12-b73c-189ce15cd5ba

github-actions[bot] commented 4 months ago

Hi @Laantje,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post.

Thanks muchly, from your friendly Umbraco GitHub bot :-)

Luuk1983 commented 3 months ago

I'm not sure if this is related, but I think it's also a 'refresh' issue potentially (Umbraco 13.3.0):

I have a simple dyamic root:

When I create a new node and it isn't saved yet, an error will show when trying select any 'Afdeling' (department):

However, when the node is saved, it works perfectly fine:

This makes the dynamic root utterly useless at this point. Maybe it's related to: https://github.com/umbraco/Umbraco-CMS/pull/15159?

PerplexDaniel commented 2 months ago

I can reproduce Dynamic Root not working properly when creating new nodes at least when using Site as the first step of the query.

The request to umbraco/backoffice/umbracoapi/entity/getDynamicRoot is something like this:

{
    "query": {
        "originAlias": "Site",
        "originKey": null,
        "querySteps": [
            { "alias": "NearestDescendantOrSelf", "anyOfDocTypeKeys": ["<GUID>"] }
        ]
    },
    "parentId": 1150,
    "currentId": 0, // NOTE: This is 0 because it's a new node
    "currentCulture": null,
    "currentSegment": null
}

The server then returns an empty 200 OK response with no data.

It seems that Dynamic Root fails when currentId is 0, which is the case for new nodes. This is unfortunate since the parentId that is passed in can be used as a perfectly acceptable fallback to detect the Site node.

Judging from the code in SiteDynamicRootOriginFinder it will just return null when CurrentKey is null instead of using ParentKey.

The relevant snippet is:

if (query.OriginAlias != SupportedOriginType || query.Context.CurrentKey.HasValue is false)
{
    return null;
}

IEntitySlim? entity = _entityService.Get(query.Context.CurrentKey.Value);
if (entity is null || entity.NodeObjectType != Constants.ObjectTypes.Document)
{
    return null;
}

Considering this is the Site-specific implementation of FindOriginKey() we can simply use either CurrentKey or ParentKey. The only edge case to consider is what happens when we are creating a new Site-level node which contains a property with a Dynamic Root that itself uses Site. Probably that does not work when you are still in the process of creating the node so I think it's expected that will never work anyway.

I'd say the fix is probably simply the following where we try CurrentKey and if that is null, fall back to ParentKey which is apparently never null -- it is defined as Guid rather than Guid?.

if (query.OriginAlias != SupportedOriginType)
{
    return null;
}

// Use either CurrentKey _or_ ParentKey as the entity key
Guid key = query.Context.CurrentKey ?? query.Context.ParentKey;

IEntitySlim? entity = _entityService.Get(key);
if (entity is null || entity.NodeObjectType != Constants.ObjectTypes.Document)
{
    return null;
}

PerplexDaniel commented 2 months ago

For anyone interested I believe this can be worked around temporarily using an AngularJS interceptor until Umbraco has fixed this in a future release.

This interceptor will look at the specific call /umbraco/backoffice/umbracoapi/entity/getDynamicRoot and will simply copy the parentId into the currentId when using a Site query that has a currentId of 0.

Hopefully this goes without saying but use at your own risk, this works for me in Umbraco 13.3.0.

Create directory App_Plugins\DynamicRootPatch

Create file App_Plugins\DynamicRootPatch\package.manifest:

{
"javascript": [ "~/App_Plugins/DynamicRootPatch/dynamic-root-patch.js" ]
}

Create file App_Plugins\DynamicRootPatch\dynamic-root-patch.js:

/*
Fixes Dynamic Root Site query for new nodes.
This patch can be removed when Umbraco has fixed the issue in a future release.
See https://github.com/umbraco/Umbraco-CMS/issues/15770
*/
(function dynamicRootPatch() {
addRequestInterceptor(function (config) {
    if (config != null &&
        config.method === "POST" &&
        config.url === "/umbraco/backoffice/umbracoapi/entity/getDynamicRoot" &&
        config.data != null &&
        config.data.query != null &&
        config.data.query.originAlias === "Site" &&
        config.data.currentId === 0 &&
        config.data.parentId > 0) {
        config.data.currentId = config.data.parentId;
    }

    return config;
});

function addRequestInterceptor(requestFn) {
    angular.module("umbraco").config(function ($httpProvider) {
        $httpProvider.interceptors.push(function () {
            return { request: requestFn }
        });
    });
}
})();

Restart your website.

bielu commented 3 weeks ago

I actually have similar issue but cause by Parent, if node is root node, there is no parent to retrieve so it cannot retrieve parent and it returns empty result, I would love it to actually use current node instead of returning nothing where there is no parent presented. I am assuming it would be as easy as introducing new Main Root query Parent or Self to resolve issue in backwards compatible way. for now i modified interceptor from Daniel:

/*
    Fixes Dynamic Root Site query for new nodes.
    This patch can be removed when Umbraco has fixed the issue in a future release.
    See https://github.com/umbraco/Umbraco-CMS/issues/15770
*/
(function dynamicRootPatch() {
  addRequestInterceptor(function (config) {
    if (config != null &&
      config.method === "POST" &&
      config.url === "/umbraco/backoffice/umbracoapi/entity/getDynamicRoot" &&
      config.data != null &&
      config.data.query != null &&
      config.data.query.originAlias === "Parent" &&
      config.data.parentId == -1) {
      config.data.parentId = config.data.currentId;
    }

    return config;
  });

  function addRequestInterceptor(requestFn) {
    angular.module("umbraco").config(function ($httpProvider) {
      $httpProvider.interceptors.push(function () {
        return { request: requestFn }
      });
    });
  }
})();

umbraco / Umbraco-CMS