Commit graph

5 commits

Author SHA1 Message Date
Paulo Castellano
ca1e346227
feat: capture ad click IDs for ad-platform conversion attribution (#276)
* feat: capture ad click IDs for Meta/Google/LinkedIn/TikTok/Reddit/Pinterest attribution

Adds gclid, fbclid, li_fat_id, ttclid, rdt_cid, and epik columns to users,
captured the same way UTM parameters already are (query string -> session
-> persisted on signup, surviving the OAuth redirect round-trip via the
new PreservesClickIds trait).

Forwards them as first-touch ($set_once) PostHog person properties in
SyncUser, so PostHog's native ad-platform destinations (Meta Ads
Conversions API, Google Ads Conversions, LinkedIn Ads, TikTok Ads, Reddit
Ads, Pinterest) have first-party click IDs to match conversions back to
the originating ad click.

* refactor: unify PreservesUtmParameters and PreservesClickIds into one trait

Both traits captured a set of query-string keys into the session and
retrieved them at signup, with identical extract/store/retrieve logic and
every call site always using both together — the split added no real
separation, just duplicated the same mechanism twice.

PreservesAttributionParameters replaces both with a single ATTRIBUTION_KEYS
list and one session key. Adding a future ad network's click ID is now one
line in that list instead of a second trait.

* refactor: split UTM_KEYS and CLICK_ID_KEYS into separate constants

Same single trait, single session key, single extract/store/retrieve
mechanism — just two named arrays instead of one merged list, so it's
clear at a glance which key belongs to which category.

* fix: don't truncate ad click IDs to 255 chars, only UTM parameters

Ad platforms explicitly warn against assuming a fixed max length for
click IDs (Google: gclid has already grown from 26 to 100+ chars, and
their docs say never truncate or validate against a fixed length).
Truncating would silently corrupt the value into something that no
longer matches the real click ID, which is worse than not capturing it
at all.

Widens the click-id columns from string (VARCHAR 255) to text — safe to
edit the migration in place since it hasn't shipped to production yet.
UTM parameters still get truncated to 255, since those are ours (our own
campaign URLs) and the column stays VARCHAR(255).

* refactor: use Laravel collection/Str helpers, forward UTMs to PostHog too

- extractAttributionParameters now reads through collect()/Str::limit()
  instead of raw array_filter/array_map/mb_substr; storeAttributionParameters
  drops its now-redundant emptiness check since retrieveAttributionParameters
  already treats "absent" and "present-but-empty" the same via pull()'s
  default.
- SyncUser forwards utm_source/medium/campaign/term/content alongside the
  click ids as first-touch ($set_once) PostHog person properties. UTMs were
  never sent to PostHog before this, on any prior code — now that PostHog is
  the source of truth for ad-platform attribution, it should have the full
  picture, not just click ids.
- Adds the missing GitHub-existing-user click-id session test, mirroring
  the Google one (parity with the existing UTM coverage).

* fix: 3 issues found by review — empty-string leak, duplicated key list, comment style

- extractAttributionParameters no longer keeps an empty-string value (e.g.
  ?utm_source=&gclid=, which some ad/email templates always append even
  for unfilled slots). The refactor to collect()/Str::limit() a few
  commits back dropped the outer array_filter() that used to strip these,
  so they were slipping into User::create() as '' instead of staying
  null. Restored via a trailing ->filter() on the merged result, and
  extended the same protection to click ids (which never had it, even
  before that refactor).
- New App\Support\AttributionKeys centralizes the UTM_KEYS/CLICK_ID_KEYS
  lists that PreservesAttributionParameters and SyncUser each maintained
  independently. SyncUser previously hand-listed the same 11 field names
  as a second array with no shared source of truth — a future ad network
  added to the trait would silently never reach PostHog unless someone
  remembered to update this second copy too.
- Removed the // comment block from the click-id migration explaining the
  text-column rationale — CLAUDE.md's PHP rules reserve inline comments
  for exceptionally complex logic; the rationale already lives in the
  commit message that introduced it.
2026-08-11 15:07:23 -03:00
Paulo Castellano
e3acd1bb4b revert: keep one OAuth callback URL per provider
Drops the dedicated /settings/authentication/providers/{provider}/callback
route added in the previous refactor — registering a second callback URL
in each OAuth app is more ops cost than the trade is worth.

Back to one callback URL per provider, with a small `Auth::check()`
branch in the auth controllers' callbacks. The check is safe because
the redirects that initiate the round-trip enforce the right
middleware (signup/login is `guest`-only, connect is `auth`-only),
so the auth state at callback time matches the flow's intent.
2026-05-04 19:42:13 -03:00
Paulo Castellano
79f800f858 refactor: dedicated callback URL for connect-from-settings flow
Splits the OAuth connect flow off entirely from signup/login so each
route has a single responsibility.

- routes/auth.php returns to its original state — redirect + callback
  both back inside the `guest` group.
- routes/app.php gains a paired callback route at
  /settings/authentication/providers/{provider}/callback.
- AuthenticationController::connectProvider /
  connectProviderCallback override Socialite's redirectUrl so the
  round-trip stays on the connect-flow URL. The Auth::check() branch
  in the auth controllers is gone.

The OAuth apps in Google Cloud and GitHub Developer Settings need the
new callback URL registered alongside the existing one — documented in
.env.example.
2026-05-04 19:30:16 -03:00
Paulo Castellano
d89f6ceede fix: let authenticated users connect Google/GitHub from Settings
The Connect button on /settings/authentication pointed at the
auth.{provider}.redirect routes that live behind `guest` middleware,
so authenticated users were bounced to /app/home before reaching
Socialite. The OAuth callback also needed to handle two flows
(signup/login vs link to current user) but had no branch for the
second case — meaning a different-email GitHub account would have
been registered as a new user, logging the original session out.

Splits the flows by intent:

- New `app.authentication.connect-provider` route in the auth group,
  handled by the settings controller (where it sits next to
  disconnect-provider). Replaces the OAuth signup link as the
  Connect button's target.
- Auth callbacks moved out of the guest group (still one URL per
  provider, since OAuth apps only register one) and gain a single
  Auth::check() branch that calls connectToCurrentUser().
- connectToCurrentUser() rejects if the provider id already belongs
  to a different user; otherwise sets it on the current user and
  redirects back to settings with a flash message.
2026-05-04 19:22:09 -03:00
Paulo Castellano
2e9e0f716b feat: capture signup UTMs/IP and add GitHub OAuth login
Persists marketing attribution and registration metadata for new users
across the three signup paths (email, Google, GitHub):

- 5 utm_* columns + registration_ip on the users table
- PreservesUtmParameters trait stores incoming utm_* query params on
  the register/redirect GET, retrieves them on the POST/callback —
  surviving the OAuth round-trip via session
- request()->ip() captured at the controller layer

Adds GitHub as a second OAuth provider:

- GitHubController mirroring the Google one (now renamed from
  SocialLoginController for symmetry)
- Settings → Authentication can connect/disconnect GitHub like Google
- Single SocialLogin.vue component replaces the per-provider buttons
  on Login/Register, rendering each enabled provider plus a single
  "or continue with" divider

UserFactory gains defaults for the new nullable columns so model
strict-mode access in tests doesn't trip.
2026-05-04 18:42:25 -03:00