feat(web-analytics): Add Web Analytics Digest Email Job#52785
Merged
jordanm-posthog merged 20 commits intomasterfrom Apr 13, 2026
Merged
feat(web-analytics): Add Web Analytics Digest Email Job#52785jordanm-posthog merged 20 commits intomasterfrom
jordanm-posthog merged 20 commits intomasterfrom
Conversation
assign-reviewers-posthog
Bot
requested review from
a team,
ablaszkiewicz,
cat-ph and
hpouillot
Contributor
|
Size Change: +7.2 kB (+0.01%) Total Size: 129 MB
ℹ️ View Unchanged
|
Contributor
Prompt To Fix All With AIThis is a comment left during a code review.
Path: posthog/api/user.py
Line: 290-295
Comment:
**Tuple constant re-created on every loop iteration**
`_dict_notification_keys` is defined inside the `for key, value in notification_settings.items()` loop, so it's re-created as a fresh tuple object on every iteration. Move it outside the loop (or to module level alongside `NOTIFICATION_DEFAULTS`) to avoid the repeated allocation.
```suggestion
_dict_notification_keys = (
"project_weekly_digest_disabled",
"error_tracking_weekly_digest_project_enabled",
"web_analytics_weekly_digest_project_enabled",
)
```
…should be hoisted above the `for` loop (or defined at module level). The body of the `if` block can then remain as-is.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: products/web_analytics/backend/weekly_digest.py
Line: 133-141
Comment:
**Result-processing happens outside the `try/except`, so malformed rows can crash the whole org's digest run**
The `try/except` block above only guards `runner.calculate()`. The list comprehension that accesses `row[1][0]` and `row[2][0]` runs after the guard ends. If any row returned by the runner has `None` (or another non-subscriptable value) for the visitors or pageviews column, a `TypeError` will propagate through `build_team_digest` and into the `for team in all_org_teams` loop in `build_and_send_digest_for_org_op`, halting digest delivery for the entire organisation.
The same pattern exists in `get_top_sources` (line 169). Consider either moving result processing inside the `try/except`, or using a defensive accessor:
```python
return [
{
"host": "",
"path": row[0] or "",
"visitors": (row[1] or (0, 0))[0],
"pageviews": (row[2] or (0, 0))[0],
}
for row in response.results
]
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: products/web_analytics/backend/weekly_digest.py
Line: 59
Comment:
**`previous or 0` masks `None` vs zero for session-based metrics**
For `visitors`, `pageviews`, and `sessions`, `item.previous or 0` silently converts `None` (no previous data) to `0` before it reaches `compute_week_over_week_change`. The function treats both `None` and `0` identically (returns `None`), so the final email output is identical — but the stored `previous` value in the result dict is `0` rather than `None`, making it impossible for any future consumer to distinguish "no previous period data" from "zero traffic last week".
The `bounce_rate` block (line 69) correctly passes `bounce_item.previous` (i.e., `None`) directly. Consider applying the same pattern to the loop metrics for consistency:
```python
current = item.value or 0
previous = item.previous # keep None to distinguish "no data" from zero
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Add Web Analytics Digest Email Job" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
jordanm-posthog
removed request for
a team,
ablaszkiewicz,
cat-ph and
hpouillot
jordanm-posthog
added
the
update-snapshots
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
…hog into jordanm-posthog/addWADigest
Contributor
Visual regression: Storybook UI snapshots updatedChanges: 2 snapshots (2 modified, 0 added, 0 deleted) What this means:
Next steps:
|
…hog into jordanm-posthog/addWADigest
lricoy
requested changes
Apr 1, 2026
Member
lricoy
left a comment
lricoy
left a comment
There was a problem hiding this comment.
Sorry for the likely block but I think the bit for:
Fan out one Dagster op per qualifying org
Makes this more suitable for temporal or celery like the health checks because of the amount of iterations. You could, however, use a dagster asset/materialization to come up to the list to send if you want.
Especially after: https://posthog.slack.com/archives/C092D6DE1L2/p1775063076802709
Contributor
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
Contributor
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (1 modified, 0 added, 0 deleted) What this means:
Next steps:
|
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
…hog into jordanm-posthog/addWADigest
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
Contributor
Visual regression: Storybook UI snapshots updatedChanges: 2 snapshots (2 modified, 0 added, 0 deleted) What this means:
Next steps:
|
…hog into jordanm-posthog/addWADigest
Contributor
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (1 modified, 0 added, 0 deleted) What this means:
Next steps:
|
lricoy
approved these changes
Apr 13, 2026
Member
lricoy
left a comment
lricoy
left a comment
There was a problem hiding this comment.
🚢 it!
Just some nit comments, completely optional
Comment on lines
+201
to
+204
| @activity.defn(name="wa-digest-send-test") | ||
| async def send_test_wa_digest(input: SendTestDigestInput) -> None: | ||
| """Send a single test digest email for one team, bypassing feature flags.""" | ||
| await database_sync_to_async(_send_test_digest, thread_sensitive=False)(input.team_id, input.email, input.force) |
Comment on lines
+237
to
+249
| def auto_select_project_for_user(user, team_traffic_data: dict[int, dict]) -> bool: | ||
| """For first-time users who have no WA digest project settings, auto-select the project with the most visitors. | ||
|
|
||
| Returns True if settings were updated (caller should refresh_from_db). | ||
| """ | ||
| from posthog.tasks.email_utils import auto_select_digest_project | ||
|
|
||
| return auto_select_digest_project( | ||
| user=user, | ||
| team_data=team_traffic_data, | ||
| setting_key="web_analytics_weekly_digest_project_enabled", | ||
| sort_key=lambda d: d.get("visitors", {}).get("current", 0), | ||
| ) |
Member
There was a problem hiding this comment.
Nice! I've also recently included a toggle to check if we should default to include the host on the breakdowns as well instead of just the paths. I think for "All domains" and for this digest, it might be useful
Contributor
Author
There was a problem hiding this comment.
Oh nice, yeah I can fast follow with that!
Contributor
|
No documentation PR created - This feature is gated behind the Once the feature flag is removed and the feature is generally available, documentation will be needed to cover:
Feel free to tag me again when the feature is ready for public release! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Users have no way to get an overview of this site's analytics week to week.
Changes
Adds new email job that queries stats and send the email to opted-in users.
How it works
New Temporal workflow:
wa-digest-get-orgslists all organizations and filters by theweb-analytics-weekly-digestfeature flagwa-digest-build-and-send-for-orgactivity per valid orgPer org:
Per team digest :
WebOverviewQuery: visitors, pageviews, sessions, bounce rate, avg session durationWebStatsTableQuery: top pagesWebStatsTableQuery: top sourcesWebGoalsQuery: goal conversionsPer user:
web_analytics_weekly_digestnotificationUserPermissionsHow did you test this code?
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Publish to changelog?
Docs update