Skip to content

fix(arc-auth): source is first-touch, never overwritten on update (po-96l) - #1660

Merged
anuveyatsu merged 1 commit into
mainfrom
po-96l-land
Aug 19, 2026
Merged

fix(arc-auth): source is first-touch, never overwritten on update (po-96l)#1660
anuveyatsu merged 1 commit into
mainfrom
po-96l-land

Conversation

@anuveyatsu

@anuveyatsu anuveyatsu commented Aug 19, 2026

Copy link
Copy Markdown
Member

source was in the CRM upsert payload unconditionally while the email path matches on emails.primaryEmail alone β€” so any repeat signup through a different surface rewrote the acquisition channel, silently: no error, nothing in the logs, row still looks healthy.

Now added to the payload only on the POST (create) branch, never the PATCH (update) branch. First-touch wins: source means how we acquired this person, which cannot change on a later visit.

The no-email fallback match filter is untouched β€” it still keys on and(name.firstName[eq]:…,source[eq]:…), which is po-k6n's fix stopping a CLI sign-in from overwriting a /build row. Filter = how we find the row; payload = what we write.

Reproduced live before the fix

13:38:23.915Z  source = STAGING_TEST     (pre-state set via API)
13:40:42Z      /build signup completed   (magic link, arc_session issued)
13:40:43.754Z  source = PORTALJS_BUILD   ← OVERWRITTEN

Row 5f85a9c5, createdAt unchanged at 09:25:48 β€” a PATCH, not a new row.

Verification

scripts/verify.sh exit 0, 19 commands, stages setup lint typecheck test build. The after-test (same loop, source must remain STAGING_TEST) runs post-deploy.

πŸ€– Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Preserved original first-touch attribution when updating existing CRM contacts.
    • Ensured source information is included only when creating new CRM contacts.
    • Improved handling of source data when CRM integration is disabled.
  • Tests

    • Added regression coverage for CRM updates using PATCH without overwriting source attribution.

source records how a person was acquired and must be set once at create
time only. Including it on every PATCH silently overwrote a person's
original acquisition channel whenever a second surface wrote the same
row. Match filter (email or login+source) is unchanged β€” this only
touches the write payload. (po-96l)
@anuveyatsu
anuveyatsu merged commit 84ff14f into main Aug 19, 2026
9 checks passed
@vercel

vercel Bot commented Aug 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
www.portaljs.com Ready Ready Preview Aug 19, 2026 1:44pm
7 Skipped Deployments
Project Deployment Actions Updated (UTC)
portaljs-alan-turing Ignored Ignored Preview Aug 19, 2026 1:44pm
portaljs-ckan Ignored Ignored Preview Aug 19, 2026 1:44pm
portaljs-ckan-ssg Ignored Ignored Preview Aug 19, 2026 1:44pm
portaljs-fivethirtyeight Ignored Ignored Preview Aug 19, 2026 1:44pm
portaljs-git-example Ignored Ignored Preview Aug 19, 2026 1:44pm
portaljs-learn Ignored Ignored Preview Aug 19, 2026 1:44pm
portaljs-openspending Ignored Ignored Preview Aug 19, 2026 1:44pm

Request Review

@changeset-bot

changeset-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8ccc0b8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@anuveyatsu
anuveyatsu deleted the po-96l-land branch August 19, 2026 13:43
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
βš™οΈ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a00d71e-4b35-4a99-ba02-f6eb8d157d76

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between c8b79f0 and 8ccc0b8.

πŸ“’ Files selected for processing (2)
  • cloud/auth/src/crm.ts
  • cloud/auth/test/crm.test.ts

πŸ“ Walkthrough

Walkthrough

CRM signup payloads now send source only when creating a new person. Existing-person PATCH requests omit source, while disabled-CRM logs still include it.

Changes

CRM attribution preservation

Layer / File(s) Summary
Separate CRM creation and update payloads
cloud/auth/src/crm.ts, cloud/auth/test/crm.test.ts
The CRM payload excludes source by default. New-person POST requests add signup.source. Existing-person PATCH requests omit source. The regression test verifies this behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

✨ Finishing Touches πŸ’‘ 1
πŸ› οΈ Fix failing CI checks πŸ’‘
  • Create stacked PR
  • Commit on current branch
πŸ“ Generate docstrings
  • Create stacked PR
  • Commit on current branch
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch po-96l-land

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.

❀️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant