Added logic to show/hide automation email opens/clicks#29497
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughEmail automation analytics now reads application-level open and click tracking settings. Performance charts and KPIs render tracked values or accessible Off states, with disabled rings dimmed and their chart values suppressed. Email node footers apply the same tracking behavior to opened and clicked metrics. Editor tests now provide controlled app settings, shared email statistics, and coverage for enabled, disabled, and unresolved tracking configurations. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin:test:acceptance |
✅ Succeeded | 6m 46s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin |
✅ Succeeded | 1m 37s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 21s | View ↗ |
nx run-many -t lint -p @tryghost/admin,ghost-mo... |
✅ Succeeded | 58s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 1s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 16s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-22 12:55:38 UTC
55ee804 to
0a7c872
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/admin/src/automations/components/canvas/email-performance-section.tsx (1)
143-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated tracking-settings read into a shared hook. Both files independently re-implement the identical
useAppContext()read +appSettings?.analytics ?? {}destructuring with the same defaults; a single shared hook (e.g.useEmailTrackingSettings()) would remove the duplication and centralize the "unresolved settings → untracked" fallback behavior in one place.
apps/admin/src/automations/components/canvas/email-performance-section.tsx#L143-L146: replace the localuseAppContext()/destructuring with a shareduseEmailTrackingSettings()hook returning{emailTrackOpens, emailTrackClicks}.apps/admin/src/automations/components/canvas/nodes.tsx#L182-L183: replace the identical block with the same shared hook.🤖 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 `@apps/admin/src/automations/components/canvas/email-performance-section.tsx` around lines 143 - 146, The tracking-settings lookup is duplicated across EmailPerformanceSection and nodes.tsx. Create a shared useEmailTrackingSettings hook that reads appSettings analytics through useAppContext, applies false defaults for unresolved settings, and returns emailTrackOpens and emailTrackClicks; replace the local reads in apps/admin/src/automations/components/canvas/email-performance-section.tsx lines 143-146 and apps/admin/src/automations/components/canvas/nodes.tsx lines 182-183 with this hook.apps/admin/src/automations/editor.test.tsx (1)
473-483: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extending the "unresolved app settings" test to the sidebar too.
This test only asserts the footer shows
Offfor both metrics whenmockAppSettingsisundefined; it doesn't open the sidebar or check the ringdata-trackedattributes the way the parameterized test above does. Given this is the edge case for the whole feature (settings not yet loaded), covering the sidebar here would close a coverage gap symmetric with theit.eachtest.🤖 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 `@apps/admin/src/automations/editor.test.tsx` around lines 473 - 483, Extend the “treats unresolved app settings as untracked” test to open the relevant sidebar and assert both metric ring data-tracked attributes indicate untracked, matching the sidebar assertions in the nearby parameterized test while preserving the existing footer checks.
🤖 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 `@apps/admin/src/automations/components/canvas/off-value.tsx`:
- Line 4: Replace the hardcoded TRACKING_OFF_MESSAGE and visible “Off” label in
the off-value component with the project’s translation hook, and add the
corresponding Admin UI translation keys and English values to
packages/i18n/locales/en/ghost.json. Ensure both rendered strings use translated
values for the active admin locale.
---
Nitpick comments:
In `@apps/admin/src/automations/components/canvas/email-performance-section.tsx`:
- Around line 143-146: The tracking-settings lookup is duplicated across
EmailPerformanceSection and nodes.tsx. Create a shared useEmailTrackingSettings
hook that reads appSettings analytics through useAppContext, applies false
defaults for unresolved settings, and returns emailTrackOpens and
emailTrackClicks; replace the local reads in
apps/admin/src/automations/components/canvas/email-performance-section.tsx lines
143-146 and apps/admin/src/automations/components/canvas/nodes.tsx lines 182-183
with this hook.
In `@apps/admin/src/automations/editor.test.tsx`:
- Around line 473-483: Extend the “treats unresolved app settings as untracked”
test to open the relevant sidebar and assert both metric ring data-tracked
attributes indicate untracked, matching the sidebar assertions in the nearby
parameterized test while preserving the existing footer checks.
🪄 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: a1de8bfd-f343-405a-9d70-13bc34e4d2d6
📒 Files selected for processing (4)
apps/admin/src/automations/components/canvas/email-performance-section.tsxapps/admin/src/automations/components/canvas/nodes.tsxapps/admin/src/automations/components/canvas/off-value.tsxapps/admin/src/automations/editor.test.tsx
0a7c872 to
afb45a2
Compare
afb45a2 to
e7762eb
Compare
EvanHahn
left a comment
EvanHahn
left a comment
There was a problem hiding this comment.
Didn't pull this down. Code LGTM.
2 participants
closes NY-1466
When "Email opens" or "Email clicks" is disabled in Settings → Analytics, automation email metrics show a muted "Off" instead of a number — on the canvas step footer and in the step sidebar's Email performance panel.
Metrics stay in place rather than hiding, and "Off" is distinct from "--" (tracking on, no data yet). Untracked sidebar tiles get a hover card explaining why, their chart rings fade to an empty track, and the status is exposed to screen readers. Display-only — re-enabling tracking restores the numbers immediately.
Gated behind the
automationAnalyticslabs flag. To test: open an automation with a Send email step and toggle the two settings.Note that there's no actual click data yet, so you'll only see "Off" or "--".
demo:
ghost-automation-tracking-off-states-2026-07-21-1258-nocaptions-share.mp4