Repository navigation
Updated Audience Feedback to boot-owned initialization - #32169
Conversation
|
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:
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly summarizes the main change: Audience Feedback now uses boot-owned initialization. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Type-Safe Boundaries | |
The PR does not introduce an explicit type-safe-boundary failure. The new index.ts wires existing internal modules, models, and services through defineService; it adds no any, unchecked as, `@… |
| New Files Are Typescript | |
The PR adds no new .js, .jsx, .cjs, or .mjs source file. The diff adds ghost/core/server/services/audience-feedback/index.ts and deletes the pre-existing index.js; all other JavaScript files are m… |
| Tooling Ownership | |
All changed paths have appropriate owners. .oxlintrc.json is root lint configuration, and its new service-root entry matches the existing migrated-service rule. Runtime and inventory changes remain … |
| Description check | |
The pull request description directly describes the Audience Feedback migration, boot ordering, dependency injection, retained request-handler receivers, and validation results. These points match the… |
Full details: Type-Safe Boundaries
-
Explanation
The PR does not introduce an explicit type-safe-boundary failure. The new
index.tswires existing internal modules, models, and services throughdefineService; it adds noany, uncheckedas,@ts-nocheck,@ts-ignore, or hand-written type duplicating a Zod schema. The changed HTTP endpoint retains its existing framework validation, and the controller changes only dependency injection and service access. No new external boundary payload is consumed without validation.
Full details: New Files Are Typescript
-
Explanation
The PR adds no new .js, .jsx, .cjs, or .mjs source file. The diff adds
ghost/core/server/services/audience-feedback/index.tsand deletes the pre-existingindex.js; all other JavaScript files are modified, not added.
Full details: Tooling Ownership
-
Explanation
All changed paths have appropriate owners.
.oxlintrc.jsonis root lint configuration, and its new service-root entry matches the existing migrated-service rule. Runtime and inventory changes remain under the owning Ghost Core service and its API/web consumers. Tests remain beside the service. Notools,ghost/scripts,ghost/bin, or newscriptspath changed. The updated callers use the TypeScript service root through.default, and boot awaits initialization before Email uses the service, matching existing Core service conventions.
Full details: Description check
-
Explanation
The pull request description directly describes the Audience Feedback migration, boot ordering, dependency injection, retained request-handler receivers, and validation results. These points match the reported changes.
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:integration |
✅ Succeeded | 1m 56s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 1m 29s | View ↗ |
nx run ghost:test:ci:e2e |
✅ Succeeded | 3m 59s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 42s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 2m 7s | View ↗ |
nx run-many -t test:unit -p ghost,@internal/scr... |
✅ Succeeded | 1m 12s | View ↗ |
nx run ghost:test:ci:unit |
✅ Succeeded | 1m 17s | View ↗ |
nx run-many -t test:types -p ghost,@internal/sc... |
✅ Succeeded | 2s | View ↗ |
Additional runs (6) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-10-26 09:28:02 UTC
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #32169 +/- ##
==========================================
+ Coverage 85.42% 85.59% +0.16%
==========================================
Files 1399 1399
Lines 52415 52414 -1
Branches 9002 9001 -1
==========================================
+ Hits 44775 44863 +88
+ Misses 6544 6468 -76
+ Partials 1096 1083 -13
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:
|
e7d431d to
01bfa1e
Compare
31f2ff0 to
7f49eda
Compare
|
@coderabbitai review |
✅ Action performed
|
01bfa1e to
c3ae8dd
Compare
|
Applied the same boundary-test improvement identified in the Theme Settings review: assert every rejected target alongside rule/source. The Email inventory now also describes awaited Audience Feedback readiness. Native suites pass 39 tests; tooling types/lint and all 87 inventory cases pass. @coderabbitai review |
|
🧠 Learnings used
✅ Action performed
|
|
c3ae8dd to
18ca8c3
Compare
ref https://linear.app/ghost/issue/PLA-546 Boot awaits Member Attribution before Staff, Members and Email retain its collaborators. The ready service owns its existing attribution builder and constructor-wired outbound tagger. Parser URLs remain captured at initialization; tracking and outbound tagging settings remain live per operation. This single migration commit targets main and includes exact allowances for string utilities, referrer parsing, toPlain and link-replacer, with a boundary regression. Source review found helpers operating on supplied data without application-owned resources; sibling imports remain restricted. The exact toPlain allowance also appears in #32169 because both independent migrations need it. Staff, Members and Email inventory descriptions record awaited attribution readiness.
18ca8c3 to
b6b96cb
Compare
ref https://linear.app/ghost/issue/PLA-546 Publish the repository, link service and controller together during boot so Email captures a ready retained collaborator. Keep URL capture and request behavior while wiring permissions from the root. Keep the stateless toPlain helper directly imported under the existing exact-file library allowance.
b6b96cb to
53599b9
Compare
|
I don't know much about this code area and it looks like I was only added automatically, so I'm recusing myself from review. Feel free to add me back. |
ref https://linear.app/ghost/issue/PLA-546
Publish Audience Feedback's repository, link service and controller together through defineService. Boot awaits readiness before Email captures the link service; request handlers retain their receivers. Permissions is injected from the root, while URL capture and feedback behavior stay unchanged.
This is one migration commit on main. It uses main's existing exact toPlain allowance and adds no dependency-policy changes.
Validation:
NX_PARALLEL=1 NX_DAEMON=false NX_TUI=false pnpm checkpassed, using normal Nx caching.NX_PARALLEL=1 NX_DAEMON=false NX_TUI=false pnpm nx run ghost:test:unit test/unit/server/services/audience-feedback test/unit/server/services/email-service/email-service-wrapper.test.js test/unit/server/services/email-service/email-renderer.test.js test/unit/boot test/unit/server/services/service-inventory.test.ts --maxWorkers=1passed 364 tests.pnpm --dir tools/scripts exec node --test test/service-dependencies.test.ts test/oxlint.test.ts test/service-root-shape.test.tspassed 94 tests. Core types, lint, boundaries and the detailed dependency report are clean.NX_PARALLEL=1 NX_DAEMON=false NX_TUI=false pnpm nx run ghost:test:e2e test/e2e-api/members/feedback.test.js --maxWorkers=1passed all 14 HTTP cases.