Added gift links /g/ reader route#28793
Open
kevinansfield wants to merge 3 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR implements the frontend reader path for Ghost "gift links." A new Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx build @tryghost/comments-ui |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/signup-form |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/announcement-bar |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/activitypub |
✅ Succeeded | 3s | View ↗ |
nx build @tryghost/portal |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/admin-toolbar |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/sodo-search |
✅ Succeeded | <1s | View ↗ |
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 9m 51s | View ↗ |
Additional runs (15) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-06-22 18:53:06 UTC
kevinansfield
force-pushed
the
ber-3728-gift-links-g-route
branch
from
debcafe to
a906c05
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ghost/core/test/integration/services/gift-links.test.ts (1)
159-161: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd an explicit row-existence assertion before field checks.
Line 159 currently assumes
reloadedexists; asserting that first gives clearer failures and avoids opaque crashes.Suggested patch
const reloaded = await models.Base.knex('gift_links').where({token}).first(); +assert.ok(reloaded, 'gift_links row should exist for issued token'); assert.equal(reloaded.redeemed_count, 2); -assert.notEqual(reloaded.last_redeemed_at, null); +assert.ok(reloaded.last_redeemed_at, 'last_redeemed_at should be set after redemption');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/core/test/integration/services/gift-links.test.ts` around lines 159 - 161, The code queries the gift_links table and immediately accesses properties on the reloaded result without verifying it exists, which can lead to opaque crashes if the query returns no rows. Add an explicit assertion immediately after the knex query (after the line with the where and first call) to check that reloaded is not null or undefined before accessing its redeemed_count and last_redeemed_at properties. This will provide clearer failure messages if the database query unexpectedly returns no results.ghost/core/test/e2e-frontend/gift-links.test.ts (1)
180-191: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winRemove cross-test state dependency in the bot-read assertion.
Line 188 assumes a prior test has already incremented the counter. Capture a baseline in this test and assert it doesn’t change after the bot request.
Suggested patch
it('does not count a read from a bot user-agent', async function () { const botAgent = supertest.agent(configUtils.config.get('url')); + const baseline = await pollRedeemedCount(token, 0, 6); await botAgent .get(`/g/${slug}/?key=${token}`) .set('User-Agent', 'Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)') .expect(200); - // The human test above already brought the count to 1; a bot read must - // not bump it further. - const count = await pollRedeemedCount(token, 2, 6); - assert.equal(count, 1, 'bot read is not counted'); + const count = await pollRedeemedCount(token, baseline, 6); + assert.equal(count, baseline, 'bot read is not counted'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ghost/core/test/e2e-frontend/gift-links.test.ts` around lines 180 - 191, The test 'does not count a read from a bot user-agent' assumes a prior test has already set the redeemed count to 1, creating a cross-test dependency. Instead, capture the baseline redeemed count before making the bot request to `/g/${slug}/?key=${token}`, then call pollRedeemedCount after the bot request and assert that the count remains unchanged from the baseline. This makes the test self-contained and independent of other test execution.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/core/core/frontend/helpers/tpl/gift-toast.hbs`:
- Around line 1-96: The `.gh-gift-toast-icon` class uses a `color-mix()`
declaration for the background property but lacks a fallback for older browser
compatibility. Add an `rgba()` fallback declaration immediately before the
`color-mix()` line to match the existing pattern used in other CSS files
throughout the codebase like transistor.css. The fallback should provide a
similar visual result using `rgba(0, 0, 0, 0.15)` before the `color-mix()`
declaration so browsers that don't support `color-mix()` will still render an
acceptable background color.
---
Nitpick comments:
In `@ghost/core/test/e2e-frontend/gift-links.test.ts`:
- Around line 180-191: The test 'does not count a read from a bot user-agent'
assumes a prior test has already set the redeemed count to 1, creating a
cross-test dependency. Instead, capture the baseline redeemed count before
making the bot request to `/g/${slug}/?key=${token}`, then call
pollRedeemedCount after the bot request and assert that the count remains
unchanged from the baseline. This makes the test self-contained and independent
of other test execution.
In `@ghost/core/test/integration/services/gift-links.test.ts`:
- Around line 159-161: The code queries the gift_links table and immediately
accesses properties on the reloaded result without verifying it exists, which
can lead to opaque crashes if the query returns no rows. Add an explicit
assertion immediately after the knex query (after the line with the where and
first call) to check that reloaded is not null or undefined before accessing its
redeemed_count and last_redeemed_at properties. This will provide clearer
failure messages if the database query unexpectedly returns no results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b99c9579-ead2-4702-8111-bf934bd46385
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
ghost/core/core/frontend/helpers/ghost_foot.jsghost/core/core/frontend/helpers/tpl/gift-toast.hbsghost/core/core/frontend/services/proxy.jsghost/core/core/frontend/services/routing/controllers/gift-links.tsghost/core/core/frontend/services/routing/controllers/index.jsghost/core/core/frontend/services/routing/gift-links-router.tsghost/core/core/frontend/services/routing/router-manager.jsghost/core/core/frontend/web/middleware/frontend-caching.jsghost/core/core/server/api/endpoints/previews.jsghost/core/core/server/services/gift-links/index.tsghost/core/core/server/services/gift-links/is-bot-user-agent.tsghost/core/core/server/services/gift-links/model.tsghost/core/core/server/services/gift-links/queries.tsghost/core/core/server/services/gift-links/read-counter.tsghost/core/core/server/services/gift-links/service.tsghost/core/core/server/services/members/service.jsghost/core/core/server/services/members/synthesize-member.jsghost/core/package.jsonghost/core/test/e2e-frontend/gift-links-toast-override.test.tsghost/core/test/e2e-frontend/gift-links.test.tsghost/core/test/integration/services/gift-links.test.tsghost/core/test/unit/frontend/services/routing/controllers/gift-links.test.tsghost/core/test/unit/server/services/gift-links/is-bot-user-agent.test.tsghost/core/test/unit/server/services/gift-links/read-counter.test.tsghost/core/test/utils/fixtures/themes/gift-toast-override-theme/default.hbsghost/core/test/utils/fixtures/themes/gift-toast-override-theme/index.hbsghost/core/test/utils/fixtures/themes/gift-toast-override-theme/package.jsonghost/core/test/utils/fixtures/themes/gift-toast-override-theme/partials/gift-toast.hbsghost/core/test/utils/fixtures/themes/gift-toast-override-theme/partials/marker.hbsghost/core/test/utils/fixtures/themes/gift-toast-override-theme/post.hbspnpm-workspace.yaml
kevinansfield
force-pushed
the
ber-3728-gift-links-g-route
branch
from
a906c05 to
fa1faa8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ghost/core/test/e2e-frontend/gift-links.test.ts`:
- Around line 157-177: The test assumes the redeemed count is exactly 1 at
specific points, but the same token is reused across multiple tests, making the
baseline count unpredictable and causing test flakiness. Instead of asserting
absolute count values, capture the baseline count before each request and then
assert that the count increases by the expected relative amount (increment by 1
for the first view in the humanAgent section, and by 0 for the repeat view which
should be de-duped). This makes the assertions independent of prior test
execution by comparing against a baseline rather than an absolute value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 271c1ee8-7c91-46ad-bcb1-981857d1e909
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
ghost/core/core/frontend/helpers/ghost_foot.jsghost/core/core/frontend/helpers/tpl/gift-toast.hbsghost/core/core/frontend/services/proxy.jsghost/core/core/frontend/services/routing/controllers/gift-links.tsghost/core/core/frontend/services/routing/controllers/index.jsghost/core/core/frontend/services/routing/gift-links-router.tsghost/core/core/frontend/services/routing/router-manager.jsghost/core/core/frontend/web/middleware/frontend-caching.jsghost/core/core/server/api/endpoints/previews.jsghost/core/core/server/services/gift-links/index.tsghost/core/core/server/services/gift-links/is-bot-user-agent.tsghost/core/core/server/services/gift-links/model.tsghost/core/core/server/services/gift-links/queries.tsghost/core/core/server/services/gift-links/read-counter.tsghost/core/core/server/services/gift-links/service.tsghost/core/core/server/services/members/service.jsghost/core/core/server/services/members/synthesize-member.jsghost/core/package.jsonghost/core/test/e2e-frontend/gift-links-toast-override.test.tsghost/core/test/e2e-frontend/gift-links.test.tsghost/core/test/integration/services/gift-links.test.tsghost/core/test/unit/frontend/services/routing/controllers/gift-links.test.tsghost/core/test/unit/server/services/gift-links/is-bot-user-agent.test.tsghost/core/test/unit/server/services/gift-links/read-counter.test.tspnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (2)
- pnpm-workspace.yaml
- ghost/core/core/frontend/services/routing/controllers/index.js
🚧 Files skipped from review as they are similar to previous changes (20)
- ghost/core/core/frontend/services/proxy.js
- ghost/core/test/unit/server/services/gift-links/is-bot-user-agent.test.ts
- ghost/core/core/server/services/members/synthesize-member.js
- ghost/core/core/frontend/helpers/ghost_foot.js
- ghost/core/core/server/services/members/service.js
- ghost/core/core/frontend/web/middleware/frontend-caching.js
- ghost/core/core/server/services/gift-links/is-bot-user-agent.ts
- ghost/core/package.json
- ghost/core/core/server/services/gift-links/index.ts
- ghost/core/core/server/services/gift-links/queries.ts
- ghost/core/test/integration/services/gift-links.test.ts
- ghost/core/core/server/api/endpoints/previews.js
- ghost/core/test/unit/frontend/services/routing/controllers/gift-links.test.ts
- ghost/core/core/server/services/gift-links/model.ts
- ghost/core/core/frontend/services/routing/controllers/gift-links.ts
- ghost/core/core/frontend/helpers/tpl/gift-toast.hbs
- ghost/core/core/server/services/gift-links/service.ts
- ghost/core/core/frontend/services/routing/gift-links-router.ts
- ghost/core/core/server/services/gift-links/read-counter.ts
- ghost/core/core/frontend/services/routing/router-manager.js
kevinansfield
force-pushed
the
ber-3728-gift-links-g-route
branch
from
fa1faa8 to
fb52efa
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #28793 +/- ##
==========================================
+ Coverage 73.89% 73.97% +0.07%
==========================================
Files 1559 1564 +5
Lines 134722 135261 +539
Branches 16216 16254 +38
==========================================
+ Hits 99555 100060 +505
- Misses 34157 34221 +64
+ Partials 1010 980 -30
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
kevinansfield
force-pushed
the
ber-3728-gift-links-g-route
branch
2 times, most recently
from
f3d5963 to
9925a7c
Compare
ref https://linear.app/ghost/issue/BER-3728 - the public reader path for gift links: /g/<slug>/?key=TOKEN renders a paid post's full content to an anonymous visitor holding a valid token, reworked from the spike (#28629) to keep one owner and reuse the render pipeline - the whole flow lives in a single controller: no site-wide middleware and no res.locals hand-off. getPostByToken now returns just {id, type}, so the happy path is one by-id read through the real public serializer - access is granted the same way /p/ previews grant it — a synthesized all-active-paid-tiers member — rather than threading a gift branch through content-gating, post-gating and the posts-public cache key. The synthesis is extracted to members/synthesize-member.js and previews.js refactored onto it, so the security-sensitive grant has one source of truth - the token is authoritative and the slug cosmetic: a stale slug on a valid token canonicalises to /g/<current-slug>/?key=…; an invalid/missing token 301s the slug to its canonical URL with the key dropped, or 404s (the single redirect-loop guard: never redirect to a target under /g/) - the router mounts in preview's privileged position so /g/ always wins over a routes.yaml collection; frontend-caching marks /g/ no-store so the edge never caches unlocked content; everything gated on labs.giftLinks
ref https://linear.app/ghost/issue/BER-3728 - each successful /g/ read now bumps redeemed_count and last_redeemed_at, giving publishers a leak signal (a link read far more than expected has likely been forwarded) - the counting mechanism is expected to change, so it all lives behind one seam — recordRead(req, res, {token, postId}) — that the controller calls only on the verified render path, never on redirects or 404s - ported the spike's hard-won mechanics: a per-post ghost-gift-seen-<id> cookie (value = token) de-dupes repeat views and survives sessions (1-year maxAge); the ghost- prefix is required so Fastly forwards it (BER-3737); secure is set at the Cookies constructor, not per-.set(), so a TLS-terminating proxy doesn't make the lib throw and skip the count - bot/scanner reads are skipped via isbot, whose word-boundary-aware matching avoids false-positives on device UAs (e.g. CUBOT) and social in-app browsers, where most gift links are actually opened - the write is fire-and-forget so read-counting never blocks or breaks the render
ref https://linear.app/ghost/issue/BER-3728 - a gift read now shows the reader a toast ("<publication> unlocked this post so you can read it for free.") so it's clear they're seeing paid content via a gift, not a paywall bypass - shipped as an overridable `gift-toast` partial (core default in helpers/tpl/gift-toast.hbs), rendered by ghost_foot when the internal `_gift` flag is set — themes can supply their own partials/gift-toast.hbs, the same override mechanism as navigation/pagination - driven by an underscore-prefixed internal `_gift` flag rather than a public `@gift` theme-context contract, so we don't commit to an API that's hard to walk back; flag-gated on labs.giftLinks - the toast string uses the {{t}} helper (translatable) and reads @site.title / @site.accent_color; handlebars auto-escapes both
kevinansfield
force-pushed
the
ber-3728-gift-links-g-route
branch
from
9925a7c to
0c666e9
Compare
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.
ref https://linear.app/ghost/issue/BER-3728
Builds the public reader path for gift links: a
/g/<slug>/?key=TOKENURL renders a paid post's full content to an anonymous visitor holding a valid token. The schema,GiftLinksServiceand admin API are already onmain; this PR adds the/g/reader path, reworked from the spike (#28629).Why a rework, not the spike as-is
The spike worked but its architecture had three problems this PR fixes:
/g/path check that duplicated the router prefix) while slug-match + render lived in the controller — one flow with two owners, handing the grant across the boundary viares.locals.content-gating.js,posts-public.js(cache key),entry-lookup.jsandfrontend-caching.jseach grew a gift-specific branch.How it works now
res.localshand-off. The/g/controller owns the whole flow top-to-bottom.getPostByTokenreturns just{id, type}, so the happy path is a single by-id read through the real public serializer +renderEntry./p/previews grant it — a synthesized all-active-paid-tiers member — so the existingcheckPostAccess/checkGatedBlockAccessreveal member-only content unchanged. Because the posts-public cache key already varies onmember.products, this gets its own key with no anonymous-cache poisoning and nogift:cache-key field. The synthesis is extracted tomembers/synthesize-member.jsandpreviews.jsrefactored onto it, so the security-sensitive grant has one source of truth.content-gating.js,post-gating.jsand the posts-public cache key are not touched./g/<current-slug>/?key=…(keeping the key). An invalid/missing token 301s the slug to its canonical URL with the key dropped, or 404s — with a single redirect-loop guard (never redirect to a target under/g/)./g/always wins over a routes.yaml collection.frontend-cachingmarks/g/no-storeso the edge never caches unlocked content.Read counting
Each successful
/g/read bumpsredeemed_count/last_redeemed_at, giving publishers a leak signal. The whole mechanism lives behind one seam —recordRead(req, res, {token, postId})— called only on the verified render path, never on redirects or 404s, because the mechanism is expected to change. A per-postghost-gift-seen-<id>cookie de-dupes repeat views and survives sessions; theghost-prefix is required so Fastly forwards it;secureis set at theCookiesconstructor so a TLS-terminating proxy can't make the lib throw and skip the count; bot reads are skipped viaisbot. The write is fire-and-forget so counting never blocks or breaks the render.Reader toast
A gift read shows the reader an overridable toast (" unlocked this post so you can read it for free.") so it's clear they're seeing gifted content. It ships as a
gift-toastpartial rendered byghost_footwhen an internal_giftflag is set — themes can supply their ownpartials/gift-toast.hbs, the same override mechanism as navigation/pagination. An underscore-prefixed internal flag is used instead of a public@gifttheme-context contract, so there's no hard-to-walk-back API commitment.Everything is gated on
labs.giftLinks.Notes / follow-ups
recordReadseam.ghost-statssendslocation.hrefincluding?key=) is tracked separately on BER-3728.Delivered as three commits (reader route core → read counting → reader toast).