Skip to content

Changed email rendering to interpret segments at a single boundary - #29359

Merged
kevinansfield merged 1 commit into
mainfrom
email-audience-descriptor
Jul 16, 2026
Merged

Changed email rendering to interpret segments at a single boundary#29359
kevinansfield merged 1 commit into
mainfrom
email-audience-descriptor

Conversation

@kevinansfield

@kevinansfield kevinansfield commented Jul 15, 2026

Copy link
Copy Markdown
Member

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

The email pipeline identifies each render variant only by its member segment string — persisted on email_batches.member_segment and accepted free-form by the preview APIs. Four consumers were reverse-engineering meaning from that string with their own heuristics: the paywall check (segment.includes(accessFilter)), the data-gh-segment card stripping, the preheader condition, and getExampleMember's enrichment check. The tier-aware segments from #28315 already strained this, and the tier-specific previews in BER-3779 add more segment shapes — every new shape had to be anticipated by all four heuristics or previews and sends silently rendered the wrong variant.

Adding tier-specific previews was hard, so this refactor sets it up to be easy:

  • checkSegmentPostAccess in content-gating answers "can this audience read this post's gated content" by rebuilding the member the segment describes (status + the tiers it names) and reusing checkPostAccess, replacing the substring matching. This also fixes tier segments on paid-visibility posts being wrongly paywalled — the substring check could never match a filter shape it didn't anticipate.
  • describeSegment(post, segment) in the renderer derives the audience facts (status, hasPostAccess) once per render; the paywall, card stripping, and preheader read the descriptor instead of re-parsing the segment.
  • getExampleMember takes the audience status from its callers instead of strict-matching segment strings, so test emails keep personalizing from the real recipient as new segment shapes appear.
  • The preheader now regenerates from the gated html for any audience without post access, not just status:free — previously the no-access variants introduced by 🐛 Fixed paid members on other tiers receiving gated newsletter content #28315 could leak gated content from model plaintext into paid recipients' preheaders.

No behavioural change for any segment the pipeline currently produces; the tier-preview feature lands separately on top.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Email segment access now uses shared content-gating logic to evaluate candidate audiences. Email rendering propagates an audience descriptor through paywall handling, segmented content filtering, template data, and preheader generation. Preview and test-email flows use the derived audience status for example-member construction and send the selected render segment. Unit tests cover tier access, paid visibility, malformed segments, and updated service interactions.

Possibly related PRs

  • TryGhost/Ghost#29059: Modifies overlapping tier-restricted email preview, rendering, paywall, and sending flows.

Suggested reviewers: 9larsons

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: consolidating email segment handling at a single boundary.
Description check ✅ Passed The description is clearly aligned with the changeset and summarizes the same email-rendering refactor.
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 email-audience-descriptor

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.44.1)
ghost/core/test/unit/server/services/email-service/email-renderer.test.js

ast-grep timed out on this file

ghost/core/test/unit/server/services/email-service/email-service.test.js

ast-grep retry budget exhausted before isolating this batch

ghost/core/test/unit/server/services/members/content-gating.test.js

ast-grep retry budget exhausted before isolating this batch


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

@nx-cloud

nx-cloud Bot commented Jul 15, 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 4f682ea

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 41s View ↗
nx run ghost:test:integration ✅ Succeeded 2m 46s View ↗
nx run ghost:test:legacy ✅ Succeeded 3m 3s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 28s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 33s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 18s View ↗
nx run-many -t lint -p ghost,ghost-monorepo ✅ Succeeded 21s View ↗
nx run @tryghost/admin:build ✅ Succeeded 10s 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-07-15 18:02:25 UTC

@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: 122bc422f9

ℹ️ 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/members/content-gating.js Outdated
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.43662% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.05%. Comparing base (710e865) to head (4f682ea).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
...ore/core/server/services/members/content-gating.js 82.27% 14 Missing ⚠️
...ore/server/services/email-service/email-service.js 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29359      +/-   ##
==========================================
- Coverage   74.06%   74.05%   -0.01%     
==========================================
  Files        1578     1578              
  Lines      137430   137609     +179     
  Branches    16652    16665      +13     
==========================================
+ Hits       101781   101905     +124     
- Misses      34604    34657      +53     
- Partials     1045     1047       +2     
Flag Coverage Δ
e2e-tests 76.13% <89.43%> (-0.01%) ⬇️

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.

@kevinansfield
kevinansfield force-pushed the email-audience-descriptor branch from 122bc42 to 7802e56 Compare July 15, 2026 17:19
ref https://linear.app/ghost/issue/BER-3779/

- segment strings are the only audience identity the email pipeline has
  (persisted on email_batches, free-form via the preview APIs), and their
  meaning was reverse-engineered in four places: the paywall check, the
  data-gh-segment card stripping, the preheader condition, and
  getExampleMember
- the tier-aware segments from #28315 made that scattering fragile and
  the tier-specific previews in BER-3779 add more segment shapes, so
  audience facts are now derived once per render (describeSegment) and
  read as data everywhere below
- access is answered by rebuilding the member a segment describes and
  reusing checkPostAccess (new checkSegmentPostAccess in content-gating)
  instead of substring-matching filter strings, which wrongly paywalled
  tier segments on paid-visibility posts and couldn't handle segment
  shapes it didn't anticipate
- the preheader now regenerates from the gated html for any audience
  without post access, not just status:free, so gated content no longer
  leaks into the preheader of the no-access variants from #28315
@kevinansfield
kevinansfield force-pushed the email-audience-descriptor branch from 7802e56 to 4f682ea Compare July 15, 2026 17:52

@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.

🧹 Nitpick comments (1)
ghost/core/test/unit/server/services/email-service/email-renderer.test.js (1)

2419-2463: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the paid-but-denied preheader path.

The bronze fixture omits plaintext, so it cannot exercise preheader regeneration for an audience with hasPostAccess: false. Add plaintext containing the gated phrase; the existing Line 2457 assertion can then catch a preheader leak too.

Suggested test fixture addition
                 if (key === 'title') {
                     return 'Test Post';
                 }
+                if (key === 'plaintext') {
+                    return 'some text for both finishing part only for members';
+                }

This covers the PR objective to prevent gated content appearing in preheaders.

🤖 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/unit/server/services/email-service/email-renderer.test.js`
around lines 2419 - 2463, Update the post fixture in the tier-gated rendering
test to include plaintext containing the gated phrase, so the bronze audience
exercises preheader regeneration with denied access. Keep the existing bronze
assertions unchanged, allowing them to verify the phrase is absent from both
rendered HTML and preheader output.
🤖 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.

Nitpick comments:
In `@ghost/core/test/unit/server/services/email-service/email-renderer.test.js`:
- Around line 2419-2463: Update the post fixture in the tier-gated rendering
test to include plaintext containing the gated phrase, so the bronze audience
exercises preheader regeneration with denied access. Keep the existing bronze
assertions unchanged, allowing them to verify the phrase is absent from both
rendered HTML and preheader output.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ff2ddbb4-450c-4e2a-8d74-5c239e3be2bc

📥 Commits

Reviewing files that changed from the base of the PR and between 7802e56 and 4f682ea.

📒 Files selected for processing (6)
  • ghost/core/core/server/services/email-service/email-renderer.js
  • ghost/core/core/server/services/email-service/email-service.js
  • ghost/core/core/server/services/members/content-gating.js
  • ghost/core/test/unit/server/services/email-service/email-renderer.test.js
  • ghost/core/test/unit/server/services/email-service/email-service.test.js
  • ghost/core/test/unit/server/services/members/content-gating.test.js
🚧 Files skipped from review as they are similar to previous changes (4)
  • ghost/core/test/unit/server/services/email-service/email-service.test.js
  • ghost/core/test/unit/server/services/members/content-gating.test.js
  • ghost/core/core/server/services/members/content-gating.js
  • ghost/core/core/server/services/email-service/email-service.js

@kevinansfield
kevinansfield merged commit 3f2c6b0 into main Jul 16, 2026
45 checks passed
@kevinansfield
kevinansfield deleted the email-audience-descriptor branch July 16, 2026 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant