Combine GetUrl requests when loading a MNTP with many entries

[p-m-j]

When loading MNTP's in the backoffice with existing entities associated, a request was made per entity to umbraco/backoffice/UmbracoApi/Entity/GetUrl

All URLs are now resolved in a single request.

Tested

• Media
• Content (including vary by culture URL changes, items in recycle bin)
• Members (no urls expected to render)

[p-m-j]

Requires both udi and integer id support

[nul800sebastiaan]

I can't see anywhere where we're still handling int ids @p-m-j - I feel like that's been deprecated in v8.0.0 but I am not 100% sure. Maybe @bjarnef can remember for sure, he's been working on Udi support a lot!

[bjarnef]

I think mostly it was because old pickers stored int ids. I haven't checked i all pickers in v9 now stores udi and legacy pickers have been removed?

[nul800sebastiaan]

This is for v8! I can't find code that handles int ids for MNTP any more in v8, nor are the legacy datatypes available any more so I believe it's safe to say that those will not work in v8 and that we don't need to anticipate them.

[bjarnef]

@nul800sebastiaan ahh sorry, just noticed @p-m-j was working on various PRs for v9 (but not this 😅 ).

Regarding udi there are a few places it would be great to support this as well:
https://github.com/umbraco/Umbraco-CMS/pulls?q=is%3Aopen+is%3Apr+author%3Abjarnef+udi

Maybe not for v8.17, but a later release on v8.

[nathanwoulfe]

Might need to consider folks who have come from v7 and may have custom value converters to deal with large amounts of legacy pickers using IDs (aka people like me 🙄).

It's probably an edge case, and should be rectified by migrating the id-based data, but assuming UDI would cause issues for these people...

[p-m-j]

@nathanwoulfe yep that's exactly what we're trying to establish, is it possible to create a v7.5 site with integer based MultiNodeTreePicker (int), take it through 7.6 where there would be both MultiNodeTreePicker (int) and MultiNodeTreePicker2 (udi) onwards to 7.14 followed by 8.x still maintaining integers in the property data and if that is possible, how does it work with the current v8 MultiNodeTreePicker (udi) at present.

The upgrade process from 7.5 to 8.x isn't much fun even with a practically empty site so it's hard to establish whether we still need to support this.

[nathanwoulfe]

Since it is possible to migrate id-based to UDI-based, maybe the solution would be adding a migration so that anyone landing on 8.17 will be doing so without IDs?

[p-m-j]

The confusion here is that since the v8 MultiNodeTreePicker only works with UDIs we're assuming that must have already been handled, see GetReferences in https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Web/PropertyEditors/MultiNodeTreePickerPropertyEditor.cs

If you have a v8 site with a MultiNodeTreePicker that still has comma separated integers in the property data then that would prove we still need to handle somehow (via a migration path or adding additional routes for looking up a url map via integer ids)

[nikolajlauridsen]

Overall looks good to me, only found a couple of minor things 👍

[nikolajlauridsen]

Looks good, just tested it out again and everything works as expected, I'll go ahead and merge 👍

[p-m-j]

Sounds like we did indeed need support for integers #11631

[nul800sebastiaan]

This is marked as breaking as it causes problems as described in #11631 - unfortunately we didn't get a chance to fix it before the 9.1.0 release, but we'll fix this in version 9.1.1.

[marcemarc]

One of the problems is that in V9 assumption that the MultiNodeTreePicker only works with Udis!

This is for v8! I can't find code that handles int ids for MNTP any more in v8, nor are the legacy datatypes available any more so I believe it's safe to say that those will not work in v8 and that we don't need to anticipate them.

In fact... Macro Parameter Editors that use the MultiNodeTreePicker in V9 have been kept as storing integer ids...

... and although it's been suggested to update them to use Udis, it's been felt this could only really be done in V10, to avoid breaking changes! But in the meantime, everything has to take ints into consideration.