Fixed remaining lazyRouting URL 404s in service-layer + serializer call sites#27915
Merged
allouis merged 7 commits intoMay 18, 2026
Merged
Conversation
ref https://linear.app/tryghost/issue/HKG-1738/ batch-sending-service loads the post that gets threaded through the email renderer to build the post URL embedded in the newsletter (via urlService.facade.getUrlForResource). The previous load only included posts_meta+authors; under lazyRouting the URL serializer evaluates each router's NQL filter against the loaded record and tag-filtered routes need 'tags' loaded too, otherwise every newsletter ships with /404/ links for sites using collection routing.
ref https://linear.app/tryghost/issue/HKG-1738/ email-controller loads the post for both preview and send-test endpoints, which then render through the email-renderer (same getUrlForResource path as batch-sending). Add 'tags' to the existing posts_meta+authors load so tag-filtered routes resolve under lazyRouting.
ref https://linear.app/tryghost/issue/HKG-1738/ The /email/<uuid>/ endpoint returns a post that goes through the email-posts output mapper → mappers.posts → url.forPost, which calls urlService.facade.getUrlForResource. Without tags+authors loaded the URL serializer can't evaluate tag- or author-filtered routes under lazyRouting and the rendered page embeds /404/ as the post URL. The fix merges with caller-supplied withRelated rather than overwriting, so ?include= options stay intact.
ref https://linear.app/tryghost/issue/HKG-1738/ getCommentEvents, getClickEvents and getFeedbackEvents all emit a post (or link.post) per row. The activity-feed mappers call url.forPost on that post, routing to urlService.facade.getUrlForResource. Without tags/authors loaded under lazyRouting every row's post URL is /404/ for tag- or author-filtered routes. Add post.tags / post.authors to the three withRelated lists (and link.post.tags / link.post.authors for clicks where the post is one relation deeper).
ref https://linear.app/tryghost/issue/HKG-1738/ getAdminAllComments emits a post per comment row for the admin moderation listing. mappers/comments.js calls url.forPost on the embedded post → urlService.facade.getUrlForResource. Without post.tags / post.authors loaded under lazyRouting each row's post URL resolves to /404/ for tag- or author-filtered routes.
ref https://linear.app/tryghost/issue/HKG-1738/ input/pages.js's defaultRelations had the same shape as the unfixed input/posts.js — a thin column request like ?fields=id,url short-circuits without loading any relations. Under lazyRouting the URL serializer needs tags+authors to evaluate filtered routes; without them a ?fields=url request resolves every URL to /404/. Mirrors the fix in input/posts.js: a forceUrlRelationsWhenLazy helper called from both the admin defaultRelations path and the Content API branch (which uses mapWithRelated rather than defaultRelations).
ref https://linear.app/tryghost/issue/HKG-1738/ The Content API branch of input/posts.js uses mapWithRelated rather than defaultRelations, so the lazyRouting fix in the previous PR (#27908) didn't cover Content API consumers passing ?fields=url. Same /404/ symptom: the URL serializer can't evaluate tag- or author-filtered routes against a thin record. Extracted the conditional into a forceUrlRelationsWhenLazy helper called from both the admin defaultRelations path and the Content API branch, so the two paths stay in sync.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
allouis
merged commit 3d8322d
into
hkg-1738-getUrlForResource-fields
41 of 44 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacks on #27908. After the audit triggered by that PR's review, seven additional call sites of
urlService.facade.getUrlForResource(...)were identified that load posts withouttags/authorsrelations and produce/404/URLs underconfig.lazyRoutingfor any tag- or author-filtered route inroutes.yaml.Each commit is a single-site fix with a TDD-style test that demonstrates the bug before the fix and pins the contract afterward.
Commits (7)
services/email-service/batch-sending-service.js/404/post URLs and broken share linksservices/email-service/email-controller.jsapi/endpoints/email-post.js/email/<uuid>/landing pages render with broken canonical URLservices/members/members-api/repositories/event-repository.js/404/post URL on every rowservices/comments/comments-service.js/404/post URL on every rowapi/endpoints/utils/serializers/input/pages.jspages.browse?fields=urlresolves to/404/(Admin and Content API)api/endpoints/utils/serializers/input/posts.jsContent API branchposts.browse?fields=urlresolves to/404/on the Content APIPattern
Most fixes are one-line
withRelatedextensions: add'tags'(or'post.tags'/'link.post.tags'as appropriate) to the existing relation list. The two serializer fixes (commits 6 and 7) extract a smallforceUrlRelationsWhenLazy(frame)helper that mirrors what #27908 added forinput/posts.js's admin branch.All fixes are gated on the same condition class as #27908:
config.lazyRoutingis on AND the call site is one where the URL serializer needs the relations to evaluate router filters. Eager-mode behaviour is unchanged.Test plan
pnpm test:single test/unit/server/services/email-service/batch-sending-service.test.js(55/55)pnpm test:single test/unit/server/services/email-service/email-controller.test.js(14/14)pnpm test:single test/unit/api/endpoints/email-post.test.js(1/1, new file)pnpm test:single test/unit/server/services/members/members-api/repositories/event-repository.test.js(35/35)pnpm test:single test/unit/server/services/comments/comments-service.test.js(1/1, new file)pnpm test:single test/unit/api/canary/utils/serializers/input/pages.test.js(22/22)pnpm test:single test/unit/api/canary/utils/serializers/input/posts.test.js(29/29 of mine; 2 pre-existing html-to-mobiledoc env failures unrelated)config.lazyRouting: truewith a tag-filtered collection in routes.yaml: send a newsletter / open a comment moderation page / open the admin activity feed → all post URLs should be the real post URL, not/404/