Skip to content

Commit 2e4940b

Browse files
marciwCopilotMpdreamz
authored
Fix concatenation bug in "Report a docs issue" URLs (#3046)
* Fix bug that duplicates docs segment in URLs * Add regression tests for report issue URL and breadcrumb URL path prefix duplication Agent-Logs-Url: https://github.com/elastic/docs-builder/sessions/e2174a2c-285c-407b-984e-41a8e2d61217 Co-authored-by: Mpdreamz <245275+Mpdreamz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Mpdreamz <245275+Mpdreamz@users.noreply.github.com>
1 parent 08d43d9 commit 2e4940b

2 files changed

Lines changed: 108 additions & 2 deletions

File tree

src/Elastic.Markdown/HtmlWriter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ private async Task<RenderResult> RenderLayout(MarkdownFile markdown, MarkdownDoc
8888
Uri? reportLinkParameter = null;
8989
if (DocumentationSet.Context.CanonicalBaseUrl is not null)
9090
{
91-
reportLinkParameter = new Uri(DocumentationSet.Context.CanonicalBaseUrl, UrlPath.JoinUrl(DocumentationSet.Context.UrlPathPrefix ?? string.Empty, current.Url));
91+
reportLinkParameter = new Uri(DocumentationSet.Context.CanonicalBaseUrl, current.Url);
9292
}
9393
var reportUrl = $"https://github.com/elastic/docs-content/issues/new?template=issue-report.yaml&link={reportLinkParameter}&labels=source:web";
9494

@@ -195,7 +195,7 @@ private BreadcrumbsList CreateStructuredBreadcrumbsData(MarkdownFile markdown, I
195195
{
196196
Position = position++,
197197
Name = parent.NavigationTitle,
198-
Item = new Uri(DocumentationSet.Context.CanonicalBaseUrl ?? new Uri("http://localhost"), UrlPath.JoinUrl(DocumentationSet.Context.UrlPathPrefix ?? string.Empty, parent.Url)).ToString()
198+
Item = new Uri(DocumentationSet.Context.CanonicalBaseUrl ?? new Uri("http://localhost"), parent.Url).ToString()
199199
}));
200200
// Add current page
201201
breadcrumbItems.Add(new BreadcrumbListItem
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Licensed to Elasticsearch B.V under one or more agreements.
2+
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
3+
// See the LICENSE file in the project root for more information
4+
5+
using System.IO.Abstractions.TestingHelpers;
6+
using AwesomeAssertions;
7+
using Elastic.Documentation;
8+
using Elastic.Documentation.Configuration;
9+
using Elastic.Documentation.Configuration.Builder;
10+
using Elastic.Documentation.Navigation;
11+
using Elastic.Markdown.IO;
12+
using Nullean.ScopedFileSystem;
13+
14+
namespace Elastic.Markdown.Tests.DocSet;
15+
16+
/// <summary>
17+
/// Tests that the "Report a docs issue" URL and breadcrumb structured-data URLs
18+
/// are built correctly and never contain a duplicated path prefix (e.g. /docs/docs/).
19+
///
20+
/// Regression guard for: https://github.com/elastic/docs-content/issues/5788
21+
/// The bug was that HtmlWriter called UrlPath.JoinUrl(UrlPathPrefix, current.Url),
22+
/// but current.Url already contains the prefix because navigation sets PathPrefix = UrlPathPrefix.
23+
/// </summary>
24+
public class ReportIssueUrlTests : IAsyncLifetime
25+
{
26+
private static readonly Uri CanonicalBaseUrl = new("https://www.elastic.co/");
27+
private const string UrlPathPrefix = "docs";
28+
29+
private DocumentationSet Set { get; }
30+
private DocumentationGenerator Generator { get; }
31+
32+
public ReportIssueUrlTests(ITestOutputHelper output)
33+
{
34+
var loggerFactory = new TestLoggerFactory(output);
35+
var mockWriteFs = new MockFileSystem(new MockFileSystemOptions
36+
{
37+
CurrentDirectory = Paths.WorkingDirectoryRoot.FullName
38+
});
39+
var readFileSystem = FileSystemFactory.RealRead;
40+
var writeFileSystem = FileSystemFactory.ScopeCurrentWorkingDirectory(mockWriteFs);
41+
var collector = new TestDiagnosticsCollector(output);
42+
var configurationContext = TestHelpers.CreateConfigurationContext(readFileSystem);
43+
44+
var context = new BuildContext(collector, readFileSystem, writeFileSystem, configurationContext, ExportOptions.Default)
45+
{
46+
Force = false,
47+
UrlPathPrefix = UrlPathPrefix,
48+
CanonicalBaseUrl = CanonicalBaseUrl
49+
};
50+
51+
Set = new DocumentationSet(context, loggerFactory, new TestCrossLinkResolver());
52+
Generator = new DocumentationGenerator(Set, loggerFactory);
53+
}
54+
55+
public async ValueTask InitializeAsync() => await Generator.ResolveDirectoryTree(default);
56+
57+
public ValueTask DisposeAsync()
58+
{
59+
GC.SuppressFinalize(this);
60+
return ValueTask.CompletedTask;
61+
}
62+
63+
[Fact]
64+
public void NavigationItemUrls_WithUrlPathPrefix_AlreadyIncludeThePrefix()
65+
{
66+
// When UrlPathPrefix = "docs", the navigation PathPrefix is "/docs",
67+
// so every navigation item URL should already start with "/docs/".
68+
var navUrl = Set.Navigation.Url;
69+
70+
navUrl.Should().StartWith($"/{UrlPathPrefix}");
71+
}
72+
73+
[Fact]
74+
public void ReportIssueUrl_WithUrlPathPrefix_DoesNotDuplicatePrefix()
75+
{
76+
// Simulate exactly what HtmlWriter does to build the report link parameter:
77+
// reportLinkParameter = new Uri(CanonicalBaseUrl, current.Url)
78+
// current.Url already contains "/docs/..." because navigation incorporates PathPrefix.
79+
// Prepending UrlPathPrefix a second time would produce "/docs/docs/…" (the old bug).
80+
var currentUrl = Set.Navigation.Url;
81+
var reportLinkParameter = new Uri(CanonicalBaseUrl, currentUrl);
82+
83+
reportLinkParameter.AbsoluteUri.Should().NotContain($"/{UrlPathPrefix}/{UrlPathPrefix}/");
84+
reportLinkParameter.AbsoluteUri.Should().StartWith($"{CanonicalBaseUrl.AbsoluteUri.TrimEnd('/')}/{UrlPathPrefix}");
85+
}
86+
87+
[Fact]
88+
public void BreadcrumbUrl_WithUrlPathPrefix_DoesNotDuplicatePrefix()
89+
{
90+
// Simulate what HtmlWriter does for breadcrumb structured data:
91+
// Item = new Uri(CanonicalBaseUrl ?? localhost, parent.Url).ToString()
92+
// Same bug would have applied here if the old JoinUrl call had been used.
93+
INavigationTraversable traversable = Set;
94+
var nestedFile = Set.MarkdownFiles
95+
.First(f => traversable.GetParentsOfMarkdownFile(f).Length > 0);
96+
var parents = traversable.GetParentsOfMarkdownFile(nestedFile);
97+
98+
parents.Should().NotBeEmpty();
99+
foreach (var parent in parents)
100+
{
101+
var breadcrumbUrl = new Uri(CanonicalBaseUrl, parent.Url).ToString();
102+
103+
breadcrumbUrl.Should().NotContain($"/{UrlPathPrefix}/{UrlPathPrefix}/");
104+
}
105+
}
106+
}

0 commit comments

Comments
 (0)