yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixes routing issues (#17572)

Browse Source 
* Child of second root should also hide root path and backoffice needs to show all domains for each language.

* Fixes routing issues based on findings https://github.com/umbraco/Umbraco-CMS/pull/17572#issuecomment-2486219574

* Revert "Fixes routing issues based on findings https://github.com/umbraco/Umbraco-CMS/pull/17572#issuecomment-2486219574"

This reverts commit ba7fb5cc904fbe602450ac109e1821282b7f69d6.

* Fix urls of descendants of non-first roots do not show the root urlsegment when HideTopLevel is true
This commit is contained in:
Bjarke Berg
2024-11-26 09:55:51 +01:00
committed by Sven Geusens
parent 37be8120ea
commit 2846325a8a
4 changed files with 222 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

191
src/Umbraco.Core/Services/DocumentUrlService.cs
View File

@@ -393,7 +393,19 @@ public class DocumentUrlService : IDocumentUrlService
return GetTopMostRootKey(isDraft, culture);
}
// Otherwise we have to find the root items (or child of the first root when hideTopLevelNodeFromPath is true) and follow the url segments in them to get to correct document key
// Special case for all top level nodes except the first (that will have /)
if (runnerKey is null && urlSegments.Length == 1 && hideTopLevelNodeFromPath is true)
{
IEnumerable<Guid> rootKeys = GetKeysInRoot(false, isDraft, culture);
Guid? rootKeyWithUrlSegment = GetChildWithUrlSegment(rootKeys, urlSegments.First(), culture, isDraft);
if (rootKeyWithUrlSegment is not null)
{
return rootKeyWithUrlSegment;
}
}
// Otherwise we have to find the root items (or child of the roots when hideTopLevelNodeFromPath is true) and follow the url segments in them to get to correct document key
for (var index = 0; index < urlSegments.Length; index++)
{
var urlSegment = urlSegments[index];
@@ -447,7 +459,7 @@ public class DocumentUrlService : IDocumentUrlService
var cultureOrDefault = string.IsNullOrWhiteSpace(culture) is false ? culture : _languageService.GetDefaultIsoCodeAsync().GetAwaiter().GetResult();
Guid[] ancestorsOrSelfKeysArray = ancestorsOrSelfKeys as Guid[] ?? ancestorsOrSelfKeys.ToArray();
IDictionary<Guid, Domain?> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, ancestorKey =>
ILookup<Guid, Domain?> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToLookup(x => x, ancestorKey =>
{
Attempt<int> idAttempt = _idKeyMap.GetIdForKey(ancestorKey, UmbracoObjectTypes.Document);
@@ -466,13 +478,11 @@ public class DocumentUrlService : IDocumentUrlService
foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray)
{
if (ancestorOrSelfKeyToDomains.TryGetValue(ancestorOrSelfKey, out Domain? domain))
var domains = ancestorOrSelfKeyToDomains[ancestorOrSelfKey].WhereNotNull();
if (domains.Any())
{
if (domain is not null)
{
foundDomain = domain;
break;
}
foundDomain = domains.First();// What todo here that is better?
break;
}
if (_cache.TryGetValue(CreateCacheKey(ancestorOrSelfKey, cultureOrDefault, isDraft), out PublishedDocumentUrlSegment? publishedDocumentUrlSegment))
@@ -528,7 +538,7 @@ public class DocumentUrlService : IDocumentUrlService
var cultures = languages.ToDictionary(x=>x.IsoCode);
Guid[] ancestorsOrSelfKeysArray = ancestorsOrSelfKeys as Guid[] ?? ancestorsOrSelfKeys.ToArray();
Dictionary<Guid, Task<Dictionary<string, Domain>>> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, async ancestorKey =>
Dictionary<Guid, Task<ILookup<string, Domain>>> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, async ancestorKey =>
{
Attempt<int> idAttempt = _idKeyMap.GetIdForKey(ancestorKey, UmbracoObjectTypes.Document);
@@ -537,35 +547,43 @@ public class DocumentUrlService : IDocumentUrlService
return null;
}
IEnumerable<Domain> domains = _domainCacheService.GetAssigned(idAttempt.Result, false);
return domains.ToDictionary(x => x.Culture!);
return domains.ToLookup(x => x.Culture!);
})!;
foreach ((string culture, ILanguage language) in cultures)
{
var urlSegments = new List<string>();
Domain? foundDomain = null;
var urlSegments = new List<string>();
List<Domain?> foundDomains = new List<Domain?>();
var hasUrlInCulture = true;
foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray)
{
if (ancestorOrSelfKeyToDomains.TryGetValue(ancestorOrSelfKey, out Task<Dictionary<string, Domain>>? domainDictionaryTask))
var hasUrlInCulture = true;
foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray)
{
var domainLookup = await ancestorOrSelfKeyToDomains[ancestorOrSelfKey];
if (domainLookup.Any())
{
Dictionary<string, Domain> domainDictionary = await domainDictionaryTask;
if (domainDictionary.TryGetValue(culture, out Domain? domain))
var domains = domainLookup[culture];
foreach (Domain domain in domains)
{
Attempt<Guid> domainKeyAttempt = _idKeyMap.GetKeyForId(domain.ContentId, UmbracoObjectTypes.Document);
Attempt<Guid> domainKeyAttempt =
_idKeyMap.GetKeyForId(domain.ContentId, UmbracoObjectTypes.Document);
if (domainKeyAttempt.Success)
{
if (_publishStatusQueryService.IsDocumentPublished(domainKeyAttempt.Result, culture))
{
foundDomain = domain;
break;
foundDomains.Add(domain);
}
}
}
if (foundDomains.Any())
{
break;
}
}
if (_cache.TryGetValue(CreateCacheKey(ancestorOrSelfKey, culture, false), out PublishedDocumentUrlSegment? publishedDocumentUrlSegment))
if (_cache.TryGetValue(
CreateCacheKey(ancestorOrSelfKey, culture, false),
out PublishedDocumentUrlSegment? publishedDocumentUrlSegment))
{
urlSegments.Add(publishedDocumentUrlSegment.UrlSegment);
}
@@ -573,34 +591,75 @@ public class DocumentUrlService : IDocumentUrlService
{
hasUrlInCulture = false;
}
}
}
//If we did not find a domain and this is not the default language, then the content is not routable
if (foundDomain is null && language.IsDefault is false)
{
continue;
}
//If we did not find a domain and this is not the default language, then the content is not routable
if (foundDomains.Any() is false && language.IsDefault is false)
{
continue;
}
var isRootFirstItem = GetTopMostRootKey(false, culture) == ancestorsOrSelfKeysArray.Last();
var leftToRight = _globalSettings.ForceCombineUrlPathLeftToRight
|| CultureInfo.GetCultureInfo(culture).TextInfo.IsRightToLeft is false;
if (leftToRight)
{
urlSegments.Reverse();
}
var isRootFirstItem = GetTopMostRootKey(false, culture) == ancestorsOrSelfKeysArray.Last();
result.Add(new UrlInfo(
text: GetFullUrl(isRootFirstItem, urlSegments, foundDomain, leftToRight),
isUrl: hasUrlInCulture,
culture: culture
));
var leftToRight = _globalSettings.ForceCombineUrlPathLeftToRight
|| CultureInfo.GetCultureInfo(culture).TextInfo.IsRightToLeft is false;
if (leftToRight)
{
urlSegments.Reverse();
}
// If no domain was found, we need to add a null domain to the list to make sure we check for no domains.
if (foundDomains.Any() is false)
{
foundDomains.Add(null);
}
foreach (Domain? foundDomain in foundDomains)
{
var foundUrl = GetFullUrl(isRootFirstItem, urlSegments, foundDomain, leftToRight);
if (foundDomain is not null)
{
// if we found a domain, it should be safe to show url
result.Add(new UrlInfo(
text: foundUrl,
isUrl: hasUrlInCulture,
culture: culture
));
}
else
{
// otherwise we need to ensure that no other page has the same url
// e.g. a site with two roots that both have a child with the same name
Guid? documentKeyByRoute = GetDocumentKeyByRoute(foundUrl, culture, foundDomain?.ContentId, false);
if (contentKey.Equals(documentKeyByRoute))
{
result.Add(new UrlInfo(
text: foundUrl,
isUrl: hasUrlInCulture,
culture: culture
));
}
else
{
result.Add(new UrlInfo(
text: "Conflict: Other page has the same url",
isUrl: false,
culture: culture
));
}
}
}
}
return result;
}
private string GetFullUrl(bool isRootFirstItem, List<string> segments, Domain? foundDomain, bool leftToRight)
{
var urlSegments = new List<string>(segments);
@@ -610,7 +669,7 @@ public class DocumentUrlService : IDocumentUrlService
return foundDomain.Name.EnsureEndsWith("/") + string.Join('/', urlSegments);
}
var hideTopLevel = _globalSettings.HideTopLevelNodeFromPath && isRootFirstItem;
var hideTopLevel = HideTopLevel(_globalSettings.HideTopLevelNodeFromPath, isRootFirstItem, urlSegments);
if (leftToRight)
{
return '/' + string.Join('/', urlSegments.Skip(hideTopLevel ? 1 : 0));
@@ -624,6 +683,21 @@ public class DocumentUrlService : IDocumentUrlService
return '/' + string.Join('/', urlSegments);
}
private bool HideTopLevel(bool hideTopLevelNodeFromPath, bool isRootFirstItem, List<string> urlSegments)
{
if (hideTopLevelNodeFromPath is false)
{
return false;
}
if(isRootFirstItem is false && urlSegments.Count == 1)
{
return false;
}
return true;
}
public async Task CreateOrUpdateUrlSegmentsWithDescendantsAsync(Guid key)
{
var id = _idKeyMap.GetIdForKey(key, UmbracoObjectTypes.Document).Result;
@@ -646,7 +720,7 @@ public class DocumentUrlService : IDocumentUrlService
}
}
private IEnumerable<Guid> GetKeysInRoot(bool addFirstLevelChildren, bool isDraft, string culture)
private IEnumerable<Guid> GetKeysInRoot(bool considerFirstLevelAsRoot, bool isDraft, string culture)
{
if (_documentNavigationQueryService.TryGetRootKeys(out IEnumerable<Guid> rootKeysEnumerable) is false)
{
@@ -655,13 +729,10 @@ public class DocumentUrlService : IDocumentUrlService
IEnumerable<Guid> rootKeys = rootKeysEnumerable as Guid[] ?? rootKeysEnumerable.ToArray();
foreach (Guid rootKey in rootKeys)
if (considerFirstLevelAsRoot)
{
yield return rootKey;
}
yield return rootKeys.First();
if (addFirstLevelChildren)
{
foreach (Guid rootKey in rootKeys)
{
if (isDraft is false && IsContentPublished(rootKey, culture) is false)
@@ -677,6 +748,13 @@ public class DocumentUrlService : IDocumentUrlService
}
}
}
else
{
foreach (Guid rootKey in rootKeys)
{
yield return rootKey;
}
}
}
@@ -710,11 +788,7 @@ public class DocumentUrlService : IDocumentUrlService
return Enumerable.Empty<Guid>();
}
/// <summary>
/// Gets the top most root key.
/// </summary>
/// <returns>The top most root key.</returns>
private Guid? GetTopMostRootKey(bool isDraft, string culture)
private IEnumerable<Guid> GetRootKeys(bool isDraft, string culture)
{
if (_documentNavigationQueryService.TryGetRootKeys(out IEnumerable<Guid> rootKeys))
{
@@ -722,11 +796,20 @@ public class DocumentUrlService : IDocumentUrlService
{
if (isDraft || IsContentPublished(rootKey, culture))
{
return rootKey;
yield return rootKey;
}
}
}
return null;
}
/// <summary>
/// Gets the top most root key.
/// </summary>
/// <returns>The top most root key.</returns>
private Guid? GetTopMostRootKey(bool isDraft, string culture)
{
return GetRootKeys(isDraft, culture).Cast<Guid?>().FirstOrDefault();
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]

29
src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/views/info/document-workspace-view-info-links.element.ts
View File

@@ -22,7 +22,7 @@ export class UmbDocumentWorkspaceViewInfoLinksElement extends UmbLitElement {
private _variantOptions?: Array<UmbDocumentVariantOptionModel>;
@state()
private _lookup: Record<string, string> = {};
private _lookup: Record<string, string[]> = {};
constructor() {
super();
@@ -56,7 +56,10 @@ export class UmbDocumentWorkspaceViewInfoLinksElement extends UmbLitElement {
if (data?.length) {
data[0].urls.forEach((item) => {
if (item.culture && item.url) {
this._lookup[item.culture] = item.url;
if(this._lookup[item.culture] == null){
this._lookup[item.culture] = [];
}
this._lookup[item.culture].push(item.url);
}
});
this.requestUpdate('_lookup');
@@ -108,18 +111,20 @@ export class UmbDocumentWorkspaceViewInfoLinksElement extends UmbLitElement {
#renderUrl(variantOption: UmbDocumentVariantOptionModel) {
const varies = !!variantOption.culture;
const culture = varies ? variantOption.culture! : variantOption.language.unique;
const url = this._lookup[culture];
const urls = this._lookup[culture];
return when(
url,
urls && urls.length >= 1,
() => html`
<a class="link-item" href=${url} target="_blank">
<span>
<span class="culture">${varies ? culture : nothing}</span>
<span>${url}</span>
</span>
<uui-icon name="icon-out"></uui-icon>
</a>
`,
${urls.map((url) =>
html`
<a class="link-item" href=${url} target="_blank">
<span>
<span class="culture">${varies ? culture : nothing}</span>
<span>${url}</span>
</span>
<uui-icon name="icon-out"></uui-icon>
</a>`
)}`,
() => html`
<div class="link-item">
<span>

39
tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentUrlServiceTest.cs
View File

@@ -213,6 +213,45 @@ public class DocumentUrlServiceTest : UmbracoIntegrationTestWithContent
return DocumentUrlService.GetDocumentKeyByRoute(route, isoCode, null, loadDraft)?.ToString()?.ToUpper();
}
[Test]
public async Task Two_items_in_level_1_with_same_name_will_have_conflicting_routes()
{
// Create a second root
var secondRoot = ContentBuilder.CreateSimpleContent(ContentType, "Second Root", null);
var contentSchedule = ContentScheduleCollection.CreateWithEntry(DateTime.Now.AddMinutes(-5), null);
ContentService.Save(secondRoot, -1, contentSchedule);
// Create a child of second root
var childOfSecondRoot = ContentBuilder.CreateSimpleContent(ContentType, Subpage.Name, secondRoot);
childOfSecondRoot.Key = new Guid("FF6654FB-BC68-4A65-8C6C-135567F50BD6");
ContentService.Save(childOfSecondRoot, -1, contentSchedule);
// Publish both the main root and the second root with descendants
ContentService.PublishBranch(Textpage, true, new[] { "*" });
ContentService.PublishBranch(secondRoot, true, new[] { "*" });
var subPageUrls = await DocumentUrlService.ListUrlsAsync(Subpage.Key);
var childOfSecondRootUrls = await DocumentUrlService.ListUrlsAsync(childOfSecondRoot.Key);
//Assert the url of subpage is correct
Assert.AreEqual(1, subPageUrls.Count());
Assert.IsTrue(subPageUrls.First().IsUrl);
Assert.AreEqual("/text-page-1", subPageUrls.First().Text);
Assert.AreEqual(Subpage.Key, DocumentUrlService.GetDocumentKeyByRoute("/text-page-1", "en-US", null, false));
//Assert the url of child of second root is not exposed
Assert.AreEqual(1, childOfSecondRootUrls.Count());
Assert.IsFalse(childOfSecondRootUrls.First().IsUrl);
//Ensure the url without hide top level is not finding the child of second root
Assert.AreNotEqual(childOfSecondRoot.Key, DocumentUrlService.GetDocumentKeyByRoute("/second-root/text-page-1", "en-US", null, false));
}
//TODO test cases:
// - Find the root, when a domain is set
// - Find a nested child, when a domain is set

29
tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentUrlServiceTest_hidetoplevel_false.cs
View File

@@ -120,4 +120,33 @@ public class DocumentUrlServiceTest_HideTopLevel_False : UmbracoIntegrationTestW
return DocumentUrlService.GetDocumentKeyByRoute(route, isoCode, null, loadDraft)?.ToString()?.ToUpper();
}
[Test]
public async Task Two_items_in_level_1_with_same_name_will_not_have_conflicting_routes()
{
// Create a second root
var secondRoot = ContentBuilder.CreateSimpleContent(ContentType, "Second Root", null);
var contentSchedule = ContentScheduleCollection.CreateWithEntry(DateTime.Now.AddMinutes(-5), null);
ContentService.Save(secondRoot, -1, contentSchedule);
// Create a child of second root
var childOfSecondRoot = ContentBuilder.CreateSimpleContent(ContentType, Subpage.Name, secondRoot);
childOfSecondRoot.Key = new Guid("FF6654FB-BC68-4A65-8C6C-135567F50BD6");
ContentService.Save(childOfSecondRoot, -1, contentSchedule);
// Publish both the main root and the second root with descendants
ContentService.PublishBranch(Textpage, true, new[] { "*" });
ContentService.PublishBranch(secondRoot, true, new[] { "*" });
var subPageUrls = await DocumentUrlService.ListUrlsAsync(Subpage.Key);
var childOfSecondRootUrls = await DocumentUrlService.ListUrlsAsync(childOfSecondRoot.Key);
Assert.AreEqual(1, subPageUrls.Count());
Assert.IsTrue(subPageUrls.First().IsUrl);
Assert.AreEqual("/textpage/text-page-1", subPageUrls.First().Text);
Assert.AreEqual(1, childOfSecondRootUrls.Count());
Assert.IsTrue(childOfSecondRootUrls.First().IsUrl);
Assert.AreEqual("/second-root/text-page-1", childOfSecondRootUrls.First().Text);
}
}