Repository navigation
Conversation
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Type-Safe Boundaries |  |
The PR adds a new Mailgun boundary read without parsing the response. #fetchEventsFromDomain maps page.items directly to normalizeEvent at mailgun-client.js:283 and :332, and the changed cod… |
Add a Zod schema for the Mailgun event data used by this pipeline, including optional nullable client-info.bot, and parse or safely parse each external event before normalizeEvent reads it. Reject or skip invalid events without accessin… |
-
Check name Status Explanation Linked Issues check Issue #30388is closed and supplies historical context only. No active directly linked issue supplies coding requirements for this pull request.Out of Scope Changes check The changes stay within the described email-analytics scope. They normalize Mailgun bot data, filter only genericopened events in newsletter and automation processors, and add focused unit tests. N…New Files Are Typescript The authoritative PR diff contains no added files. All six changed paths have status M, including the JavaScript files, so the check's failure condition for adding a new.js/.jsx/.cjs/.mjssource …Tooling Ownership PASS. The PR changes only Ghost Core Mailgun/email-analytics implementation files and their adjacent unit tests. No scripts, tools, shared configuration, entrypoints, or moved paths changed. The servi… Title check The title clearly identifies the main change: excluding security-scanner opens from email analytics counts. Description check The description accurately explains Mailgun bot detection, filtering of generic bot opens, retained handling for Apple and Gmail opens, and test coverage. Full details: Out of Scope Changes check
-
Explanation
The changes stay within the described email-analytics scope. They normalize Mailgun bot data, filter only
genericopened events in newsletter and automation processors, and add focused unit tests. No unrelated change is shown.
Full details: New Files Are Typescript
-
Explanation
The authoritative PR diff contains no added files. All six changed paths have status
M, including the JavaScript files, so the check's failure condition for adding a new.js/.jsx/.cjs/.mjssource file is not met.
Full details: Tooling Ownership
-
Explanation
PASS. The PR changes only Ghost Core Mailgun/email-analytics implementation files and their adjacent unit tests. No scripts, tools, shared configuration, entrypoints, or moved paths changed. The service README assigns domain logic to Core services, and the tests remain beside their owners. No tooling ownership or execution problem is introduced.
-
Full details: Type-Safe Boundaries
-
Explanation
The PR adds a new Mailgun boundary read without parsing the response.
#fetchEventsFromDomainmapspage.itemsdirectly tonormalizeEventatmailgun-client.js:283and:332, and the changed code readsevent['client-info'].botat:405-409. Thetypeofcheck validates only the leaf value. No Zod schema or boundary parser validates the Mailgun event orclient-infobefore this read. The downstream bot checks consume the resulting unvalidated field.Resolution
Add a Zod schema for the Mailgun event data used by this pipeline, including optional nullable
client-info.bot, and parse or safely parse each external event beforenormalizeEventreads it. Reject or skip invalid events without accessing unvalidated fields. Use the parsed result or az.infertype for valid events, and preserve the current bot normalization for valid strings.
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- 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.
|
Thanks for this suggestion. I’ve narrowed the change to only skip opens flagged generic (firewall / anti-virus scanners). It’s ready code-wise, but I'm marking it blocked for now because changing how opens are counted shifts open rates for some publishers overnight and given that we want a UI/rollout plan alongside it before merging. We’ll pick it back up once the team working on email analytics has bandwidth! |
ref TryGhost#30388 Corporate firewalls and anti-virus scanners fetch every link and the tracking pixel as soon as an email arrives, inflating open rates for newsletters and automated emails. Mailgun flags these opens with `client-info.bot: generic`, so both analytics processors now skip them. Opens flagged `apple` (Mail Privacy Protection) and `gmail` (image proxy) are still counted as before, because dropping them would silently erase most opens from Apple Mail and Gmail readers.
906f2a4 to
a0a0a8c
Compare
9larsons
changed the title
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:integration |
✅ Succeeded | 1m 53s | View ↗ |
nx run ghost:test:ci:integration |
✅ Succeeded | 1m 50s | View ↗ |
nx run ghost:test:ci:e2e |
✅ Succeeded | 3m 44s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 3m 41s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 1m 46s | View ↗ |
nx run ghost:test:ci:unit |
✅ Succeeded | 1m 39s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 21s | View ↗ |
nx run-many -t test:unit -p ghost |
✅ Succeeded | 40s | 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-27 20:07:42 UTC
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #30625 +/- ##
==========================================
+ Coverage 85.45% 85.56% +0.10%
==========================================
Files 1404 1404
Lines 52544 52546 +2
Branches 9022 9023 +1
==========================================
+ Hits 44903 44962 +59
+ Misses 6540 6499 -41
+ Partials 1101 1085 -16
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:
|
ref #30388
Firewall and anti-virus scanners fetch the tracking pixel when an email arrives, inflating open rates. Mailgun flags these opens with
client-info.bot: generic; newsletter and automation analytics now skip them.apple(Mail Privacy Protection) andgmail(image proxy) opens are still counted, since dropping them would erase most Apple Mail/Gmail opens. Unflagged opens are unchanged; guard tests cover all three cases.