Skip to content

Persist gift subscription delivery data - #29849

Open
kevinansfield wants to merge 7 commits into
mainfrom
agent/ber-3851-delivery-persistence
Open

Persist gift subscription delivery data#29849
kevinansfield wants to merge 7 commits into
mainfrom
agent/ber-3851-delivery-persistence

Conversation

@kevinansfield

@kevinansfield kevinansfield commented Aug 10, 2026

Copy link
Copy Markdown
Member

ref https://linear.app/ghost/issue/BER-3851/

Summary

  • store the single delivery snapshot directly on each gift
  • default existing and omitted purchases to link delivery
  • add schema, model, codec, and migration support for delivery state and latest provider outcome
  • keep email delivery unreachable at this layer

Stack

2 of 6. Stacked on #29848; the transactional email transport PR is next.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The PR adds gift delivery, recipient, scheduling, retry, provider tracking, and delivery outcome fields. It updates database migrations, schema indexes, TypeScript schemas, gift construction, model defaults, and database codecs. It also adds delivery terminology and updates the schema integrity hash.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description accurately summarizes persistent gift delivery state, defaults, schema support, and the PR stack.
Title check ✅ Passed The title clearly and concisely identifies the main change: persisting gift subscription delivery data.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/ber-3851-delivery-persistence

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the migration [pull request] Includes migration for review label Aug 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

It looks like this PR contains a migration 👀
Here's the checklist for reviewing migrations:

General requirements

  • ⚠️ Tested performance on staging database servers, as performance on local machines is not comparable to a production environment
  • Satisfies idempotency requirement (both up() and down())
  • Does not reference models
  • Filename is in the correct format (and correctly ordered)
  • Targets the next minor version
  • All code paths have appropriate log messages
  • Uses the correct utils
  • Contains a minimal changeset
  • Does not mix DDL/DML operations
  • Tested in MySQL and SQLite

Schema changes

  • Both schema change and related migration have been implemented
  • For index changes: has been performance tested for large tables
  • For new tables/columns: fields use the appropriate predefined field lengths
  • For new tables/columns: field names follow the appropriate conventions
  • Does not drop a non-alpha table outside of a major version

Data changes

  • Mass updates/inserts are batched appropriately
  • Does not loop over large tables/datasets
  • Defends against missing or invalid data
  • For settings updates: follows the appropriate guidelines

@nx-cloud

nx-cloud Bot commented Aug 10, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit dcb0dbd

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 3m 4s View ↗
nx run ghost:test:integration ✅ Succeeded 3m 13s View ↗
nx run ghost:test:legacy ✅ Succeeded 2m 51s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 47s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 32s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 21s View ↗
nx run-many -t lint -p ghost,ghost-monorepo ✅ Succeeded 18s View ↗
nx run @tryghost/admin:build ✅ Succeeded 8s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded <1s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-08-11 16:55:24 UTC

@kevinansfield
kevinansfield marked this pull request as ready for review August 10, 2026 15:10
@kevinansfield
kevinansfield force-pushed the agent/ber-3851-delivery-persistence branch from 133ccca to d4dc1d9 Compare August 10, 2026 15:12

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 133ccca423

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ghost/core/core/server/services/gifts/gift.ts Outdated
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 34.35897% with 128 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.38%. Comparing base (b5293ba) to head (dcb0dbd).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...t/core/core/server/services/gifts/gift-delivery.ts 0.00% 43 Missing ⚠️
.../core/server/services/gifts/gift-delivery-codec.ts 0.00% 42 Missing ⚠️
...core/server/services/gifts/gift-delivery-schema.ts 0.00% 29 Missing ⚠️
...st/core/core/server/services/gifts/gift-service.ts 16.66% 5 Missing ⚠️
ghost/core/core/server/models/gift-delivery.js 84.00% 4 Missing ⚠️
ghost/core/core/server/services/gifts/gift.ts 85.71% 3 Missing and 1 partial ⚠️
ghost/core/core/server/models/gift.js 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29849      +/-   ##
==========================================
- Coverage   75.47%   75.38%   -0.10%     
==========================================
  Files        1606     1610       +4     
  Lines      140753   140938     +185     
  Branches    17458    17455       -3     
==========================================
+ Hits       106237   106247      +10     
- Misses      33439    33613     +174     
- Partials     1077     1078       +1     
Flag Coverage Δ
e2e-tests 77.52% <34.35%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Base automatically changed from agent/ber-3851-gift-endpoint-consolidation to main August 10, 2026 17:30
@kevinansfield
kevinansfield force-pushed the agent/ber-3851-delivery-persistence branch from d4dc1d9 to 09af726 Compare August 10, 2026 17:30

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/core/server/services/gifts/gift.ts`:
- Around line 149-162: Update checkRedeemable() to reject gifts whose deliverAt
is in the future before applying the existing redemption checks, while allowing
past and current delivery dates. Return a distinct non-redeemable reason for
delayed availability, and add tests covering past, current, and future deliverAt
values.
🪄 Autofix

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: 24204e7f-0bc1-46e5-8b44-c9d77f61d794

📥 Commits

Reviewing files that changed from the base of the PR and between af0da3d and 09af726.

📒 Files selected for processing (9)
  • ghost/core/core/server/data/migrations/versions/6.57/2026-08-05-15-03-16-add-gift-delivery-fields.js
  • ghost/core/core/server/data/migrations/versions/6.57/2026-08-05-15-15-28-add-gifts-email-provider-message-id-index.js
  • ghost/core/core/server/data/schema/schema.js
  • ghost/core/core/server/models/gift.js
  • ghost/core/core/server/services/gifts/CONTEXT.md
  • ghost/core/core/server/services/gifts/gift-codec.ts
  • ghost/core/core/server/services/gifts/gift-schema.ts
  • ghost/core/core/server/services/gifts/gift.ts
  • ghost/core/test/unit/server/data/schema/integrity.test.js

Comment thread ghost/core/core/server/services/gifts/gift.ts Outdated
@kevinansfield
kevinansfield force-pushed the agent/ber-3851-delivery-persistence branch 2 times, most recently from 7368ac9 to f3e4da1 Compare August 11, 2026 12:09
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions
Gift subscriptions need one authoritative delivery snapshot on the gift before transport, scheduling, and checkout behavior can be introduced independently.
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions

The 6.57 migration folder has already shipped, so newly added migrations must target the next minor to run for existing installations.
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions

The field stores a provider failure payload rather than general diagnostics, so the narrower error name matches its actual contract and existing terminology.
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions

SetOptional expresses the constructor's defaulted fields directly and avoids maintaining the same key list in both Omit and Partial Pick.
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions

Gift.fromPurchase can rely on the constructor's delivery defaults, leaving only the lifecycle state that is specific to a new purchase.
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions

Constructor and purchase inputs should inherit optional fields from Zod defaults instead of maintaining separate property lists that can drift from the schema.
ref https://linear.app/ghost/issue/BER-3851/establish-immediate-email-delivery-for-gift-subscriptions

Gift delivery and redemption have independent lifecycles, so email-only transport state now lives in a one-to-one delivery record while availability remains part of the gift entitlement.
@kevinansfield
kevinansfield force-pushed the agent/ber-3851-delivery-persistence branch from f3e4da1 to dcb0dbd Compare August 11, 2026 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

migration [pull request] Includes migration for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant