Add BaseUrl field to domain settings for headless CMS frontend URL support - #549
Conversation
…pport - Add BaseUrl column to SeoDomainCollectionEntity and SeoDomainCollection model - Create database migration for new column - Add BaseUrlHelper utility to rewrite URLs with configured base URL - Update SitemapGenerator, SitemapIndexGenerator, and SitemapMiddleware to apply BaseUrl - Update RobotsSitemapProvider to use BaseUrl for sitemap URLs in robots.txt - Update TextSeoValueConverter to apply BaseUrl for canonical URLs (%CurrentUrl%) - Add BaseUrlHelper unit tests and RobotsSitemapProvider BaseUrl tests Closes #548
PR Summary by QodoSupport headless frontends via per-domain BaseUrl override in SEO URL generation
AI Description
Diagram
High-Level Assessment
Files changed (17)
|
Code Review by Qodo
1. BaseUrl scheme detection bug
|
| using SeoToolkit.Umbraco.Common.Core.Helpers; | ||
|
|
||
| namespace SeoToolkit.Tests | ||
| { |
There was a problem hiding this comment.
1. Tests added outside seotoolkit.tests/ 📘 Rule violation ▣ Testability
New/updated test code is located under src/SeoToolkit.Tests/... instead of a repository-root SeoToolkit.Tests/ directory. This violates the required test-code placement convention and can make test discovery/organization inconsistent.
Agent Prompt
## Issue description
The PR adds/updates test code under `src/SeoToolkit.Tests/...`, but the compliance requirement mandates that test code resides under a repository-root `SeoToolkit.Tests/` directory.
## Issue Context
This PR adds `BaseUrlHelperTests.cs` (and updates other test files) in the `src/` tree, which is outside the required root-level `SeoToolkit.Tests/` location.
## Fix Focus Areas
- src/SeoToolkit.Tests/SeoToolkit.Tests/BaseUrlHelperTests.cs[1-4]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| var normalizedBaseUrl = baseUrl.TrimEnd('/'); | ||
| if (!normalizedBaseUrl.StartsWith("http", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| normalizedBaseUrl = $"https://{normalizedBaseUrl}"; |
There was a problem hiding this comment.
2. Baseurl scheme detection bug 🐞 Bug ≡ Correctness
BaseUrlHelper.ApplyBaseUrl() treats any baseUrl starting with "http" as already having a URL scheme, so hostnames like "http2.example.com" won’t get an "https://" prefix and will fail absolute URI parsing, causing the configured BaseUrl to be ignored. This breaks the new headless/BaseUrl feature for a non-obvious class of valid domains.
Agent Prompt
## Issue description
`BaseUrlHelper.ApplyBaseUrl` uses `StartsWith("http")` to decide whether to prepend a scheme. This incorrectly classifies hostnames that begin with `http` (e.g., `http2.example.com`) as already schemed, causing `Uri.TryCreate(..., UriKind.Absolute, ...)` to fail and the helper to return the original URL (silently disabling BaseUrl).
## Issue Context
BaseUrl is user-configured and expected to accept values without a scheme (per tests and UI). The current heuristic should detect a *real* scheme, not just an `http` prefix.
## Fix Focus Areas
- src/SeoToolkit.Umbraco.Common.Core/Helpers/BaseUrlHelper.cs[16-22]
### Suggested implementation approach
- Trim whitespace first: `baseUrl.Trim()`.
- Detect scheme by checking for `://` (or `Uri.TryCreate` with `UriKind.Absolute` and validating scheme is http/https).
- If no scheme is present, prepend `https://` and retry parsing.
- Add a regression unit test for `baseUrl = "http2.example.com"` (and optionally leading/trailing whitespace).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| var settings = _sitemapConfigurationService.GetSettings(); | ||
| var seoDomain = seoDomainResolver.ResolveDomain(); | ||
| var baseUrl = seoDomain?.BaseUrl; |
There was a problem hiding this comment.
3. Extra umbracocontext scope 🐞 Bug ➹ Performance
SitemapMiddleware.Invoke() resolves the domain via ISeoDomainResolver before entering its own EnsureUmbracoContext() scope, which acquires an additional UmbracoContextFactory scope on sitemap requests. Depending on Umbraco context-factory semantics, this may cause unnecessary overhead per sitemap request.
Agent Prompt
## Issue description
`SitemapMiddleware.Invoke` calls `seoDomainResolver.ResolveDomain()` before it creates its own UmbracoContext scope, and `SeoDomainResolver` internally calls `EnsureUmbracoContext()` as well. This results in an extra context-factory scope acquisition for sitemap requests.
## Issue Context
SitemapMiddleware already selects the Umbraco `Domain` inside its `EnsureUmbracoContext()` scope and could resolve the matching `SeoDomainCollection` (and BaseUrl) without needing a separate resolver call.
## Fix Focus Areas
- src/SeoToolkit.Umbraco.Sitemap.Core/Middleware/SitemapMiddleware.cs[49-66]
- src/SeoToolkit.Umbraco.Common.Core/Helpers/SeoDomainResolver.cs[25-35]
### Suggested implementation approach
- Move BaseUrl resolution inside the existing `using var ctx = _umbracoContextFactory.EnsureUmbracoContext()`.
- Option A: Inject `ISeoDomainsService` into the middleware and call `GetByDomain(domain.Id)` after `DomainUtilities.SelectDomain(...)`.
- Option B: Add a resolver method that can resolve from an already-available domain id (avoiding `EnsureUmbracoContext()` internally).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…pport (#549) * Initial plan * Add BaseUrl field to domain settings for headless CMS frontend URL support - Add BaseUrl column to SeoDomainCollectionEntity and SeoDomainCollection model - Create database migration for new column - Add BaseUrlHelper utility to rewrite URLs with configured base URL - Update SitemapGenerator, SitemapIndexGenerator, and SitemapMiddleware to apply BaseUrl - Update RobotsSitemapProvider to use BaseUrl for sitemap URLs in robots.txt - Update TextSeoValueConverter to apply BaseUrl for canonical URLs (%CurrentUrl%) - Add BaseUrlHelper unit tests and RobotsSitemapProvider BaseUrl tests Closes #548 * Address review: specify column length (500) for BaseUrl migration and entity * Finish the UI * Fix tests --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: patrickdemooij9 <11466511+patrickdemooij9@users.noreply.github.com> Co-authored-by: patrickdemooij9 <patrickdemooij98@hotmail.com>
Headless CMS setups use the API/CMS domain in sitemap.xml, robots.txt sitemap references, and canonical URLs instead of the actual frontend domain. This adds a per-domain-collection
BaseUrlfield that overrides the host portion of generated URLs.Data layer
BaseUrl(varchar 500, nullable) column toSeoDomainCollectionEntityandSeoDomainCollectionstate-7) adds the column toSeoToolkitDomainCollectionsURL rewriting
BaseUrlHelper.ApplyBaseUrl(url, baseUrl)utility replaces scheme+host while preserving path/queryBaseUrlis empty/null, original URL is returned unchanged (backward compatible)Consumers updated
SitemapGeneratorOptions%CurrentUrl%(canonical)Usage
Set the
BaseUrlfield on a domain collection (e.g.https://sw-unlimited-db.com). All URLs generated for that domain group will use the frontend host:Tests
BaseUrlHelpercovering schemes, ports, paths, edge casesRobotsSitemapProviderBaseUrl behavior