Skip to content

fix: fire created_volume PostHog event before token generation#2283

Merged
dobrac merged 3 commits intomainfrom
joe/posthog-volume-events-v2
Apr 2, 2026
Merged

fix: fire created_volume PostHog event before token generation#2283
dobrac merged 3 commits intomainfrom
joe/posthog-volume-events-v2

Conversation

@joe-lombrozo-s-bot
Copy link
Copy Markdown
Contributor

Moves the created_volume PostHog event to fire immediately after tx.Commit succeeds, before generateVolumeContentToken. This ensures the analytics event is always recorded when a volume is created, even if token generation subsequently fails.

Addresses review comment on #2282.

Move the PostHog event to immediately after tx.Commit succeeds,
before generateVolumeContentToken. This ensures the event is always
fired when a volume is created, even if token generation fails.

Addresses review comment on #2282.
@djeebus djeebus marked this pull request as ready for review April 1, 2026 16:48
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — clean reorder that guarantees the analytics event fires on successful commit, regardless of downstream token generation failure.

Extended reasoning...

Overview

This PR modifies a single file (volume_create.go) by moving the created_volume PostHog event call from after generateVolumeContentToken to immediately after tx.Commit succeeds. No logic changes, no new code paths — pure reorder.

Security Risks

None. The change only affects analytics event ordering and does not touch auth, permissions, token generation logic, or any external-facing behavior.

Level of Scrutiny

Low. This is a mechanical reorder addressing a reviewer comment from #2282. The intent is clear and the implementation is correct: analytics should record that a volume was created when it is actually created (after commit), not contingent on a subsequent token generation step that can fail independently.

Other Factors

The inline bug comment flags a pre-existing issue (missing IdentifyAnalyticsTeam and GetPackageToPosthogProperties) that was introduced in #2282, not by this PR. This PR neither worsens nor fixes that gap — it is orthogonal to the reorder. That issue warrants a follow-up fix in a separate PR.

@dobrac dobrac merged commit 992b45d into main Apr 2, 2026
35 checks passed
@dobrac dobrac deleted the joe/posthog-volume-events-v2 branch April 2, 2026 08:29
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.

4 participants