refactor(paypal): extract + unit-test sync mapping; reuse auth helper (R3)

PayPal transaction filter + payments mapping + external-id dedupe were
inline and untested, and auth was a hand-rolled select instead of the
shared helper. Extracted to pure lib/paypal-logic.ts (also fixes a
latent throw on a missing/invalid PayPal date by falling back to now);
route now uses requireMemberWithRole. 7 tests; suite 521. Behavior
otherwise unchanged.

__tests__/paypal-logic.test.ts

@@ -0,0 +1,108 @@
+import { describe, it, expect } from 'vitest'
+import {
+  mapPayPalTransactions,
+  filterUnseenByExternalId,
+  externalIdsOf,
+  type PayPalPaymentRow,
+} from '@/lib/paypal-logic'
+
+const NOW = '2026-01-01T00:00:00.000Z'
+
+describe('mapPayPalTransactions', () => {
+  it('keeps only incoming (amount > 0) and maps fields', () => {
+    const rows = mapPayPalTransactions(
+      [
+        {
+          transaction_info: {
+            transaction_id: 't1',
+            transaction_amount: { value: '25.00' },
+            transaction_subject: 'Dues',
+            transaction_initiation_date: '2026-02-03T10:00:00Z',
+          },
+          payer_info: { email_address: '[email protected]' },
+        },
+        { transaction_info: { transaction_id: 't2', transaction_amount: { value: '-9.00' } } },
+        { transaction_info: { transaction_id: 't3', transaction_amount: { value: '0' } } },
+      ],
+      's1',
+      NOW,
+    )
+    expect(rows).toHaveLength(1)
+    expect(rows[0]).toEqual<PayPalPaymentRow>({
+      space_id: 's1',
+      platform: 'paypal',
+      amount: 25,
+      from_identifier: '[email protected]',
+      from_note: 'Dues',
+      transaction_date: new Date('2026-02-03T10:00:00Z').toISOString(),
+      link_status: 'unlinked',
+      external_id: 't1',
+    })
+  })
+
+  it('falls back through name then a literal, and note then subject then null', () => {
+    const [a, b] = mapPayPalTransactions(
+      [
+        {
+          transaction_info: { transaction_id: 'a', transaction_amount: { value: '1' }, transaction_note: 'n' },
+          payer_info: { payer_name: { alternate_full_name: 'Jane' } },
+        },
+        { transaction_info: { transaction_id: 'b', transaction_amount: { value: '2' } } },
+      ],
+      's1',
+      NOW,
+    )
+    expect(a.from_identifier).toBe('Jane')
+    expect(a.from_note).toBe('n')
+    expect(b.from_identifier).toBe('PayPal User')
+    expect(b.from_note).toBeNull()
+  })
+
+  it('uses nowIso when the date is missing or invalid (never throws)', () => {
+    const [a, b] = mapPayPalTransactions(
+      [
+        { transaction_info: { transaction_id: 'a', transaction_amount: { value: '1' } } },
+        {
+          transaction_info: {
+            transaction_id: 'b',
+            transaction_amount: { value: '1' },
+            transaction_initiation_date: 'not-a-date',
+          },
+        },
+      ],
+      's1',
+      NOW,
+    )
+    expect(a.transaction_date).toBe(NOW)
+    expect(b.transaction_date).toBe(NOW)
+  })
+
+  it('handles empty / nullish input', () => {
+    expect(mapPayPalTransactions([], 's1', NOW)).toEqual([])
+    expect(mapPayPalTransactions(undefined as unknown as [], 's1', NOW)).toEqual([])
+  })
+})
+
+describe('filterUnseenByExternalId', () => {
+  const rows = [
+    { external_id: 'a' },
+    { external_id: 'b' },
+    { external_id: null },
+  ] as PayPalPaymentRow[]
+
+  it('drops already-seen ids, keeps unseen and null-id rows', () => {
+    const out = filterUnseenByExternalId(rows, new Set(['a']))
+    expect(out.map(r => r.external_id)).toEqual(['b', null])
+  })
+  it('keeps everything when nothing seen', () => {
+    expect(filterUnseenByExternalId(rows, new Set())).toHaveLength(3)
+  })
+})
+
+describe('externalIdsOf', () => {
+  it('returns only non-null external ids', () => {
+    expect(
+      externalIdsOf([{ external_id: 'a' }, { external_id: null }, { external_id: 'c' }] as PayPalPaymentRow[]),
+    ).toEqual(['a', 'c'])
+  })
+})

app/api/paypal/sync/route.ts

@@ -2,6 +2,12 @@ import { NextRequest, NextResponse } from 'next/server'
 import { createClient } from '@/lib/supabase/server'
 import { createAdminClient } from '@/lib/supabase/admin'
 import { readSecret } from '@/lib/secrets/vault'
+import { requireMemberWithRole } from '@/lib/auth-helpers'
+import {
+  mapPayPalTransactions,
+  filterUnseenByExternalId,
+  externalIdsOf,
+} from '@/lib/paypal-logic'
 
 // PayPal Transaction Search API (v1)
 // Docs: https://developer.paypal.com/docs/api/transaction-search/v1/
@@ -53,14 +59,13 @@ async function fetchPayPalTransactions(
 
 export async function POST(req: NextRequest) {
   const supabase = await createClient()
-  const { data: { user } } = await supabase.auth.getUser()
-  if (!user) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
-
-  const { data: member } = await supabase
-    .from('space_members').select('space_id, role').eq('user_id', user.id).in('status', ['current', 'unverified', 'late']).single()
-  if (!member || !['admin', 'board', 'treasurer'].includes(member.role)) {
-    return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 })
-  }
+  const auth = await requireMemberWithRole(
+    supabase,
+    ['admin', 'board', 'treasurer'],
+    'Insufficient permissions',
+  )
+  if (!auth.ok) return NextResponse.json({ error: auth.error }, { status: 403 })
+  const { member } = auth
 
   // Get PayPal integration config for this space
   const { data: integration } = await supabase
@@ -100,30 +105,7 @@ export async function POST(req: NextRequest) {
       return NextResponse.json({ imported: 0, message: 'No transactions found in the last 30 days' })
     }
 
-    // Map PayPal transactions to our schema
-    const rows = transactions
-      .filter(tx => {
-        const info = tx.transaction_info
-        const amount = parseFloat(info?.transaction_amount?.value ?? '0')
-        return amount > 0 // Only incoming payments
-      })
-      .map(tx => {
-        const info = tx.transaction_info
-        const payer = tx.payer_info
-        const amount = parseFloat(info.transaction_amount?.value ?? '0')
-        const txDate = info.transaction_initiation_date ?? info.transaction_updated_date
-
-        return {
-          space_id: member.space_id,
-          platform: 'paypal' as const,
-          amount,
-          from_identifier: payer?.email_address ?? payer?.payer_name?.alternate_full_name ?? 'PayPal User',
-          from_note: info.transaction_note ?? info.transaction_subject ?? null,
-          transaction_date: new Date(txDate).toISOString(),
-          link_status: 'unlinked' as const,
-          external_id: info.transaction_id,
-        }
-      })
+    const rows = mapPayPalTransactions(transactions, member.space_id, new Date().toISOString())
 
     if (rows.length === 0) {
       return NextResponse.json({ imported: 0, message: 'No incoming payments found' })
@@ -134,14 +116,14 @@ export async function POST(req: NextRequest) {
     // reliable. Instead, look up which PayPal transaction ids already exist
     // *in this space* and insert only the genuinely new ones (keeping
     // external_id so a later re-sync stays idempotent).
-    const externalIds = rows.map(r => r.external_id).filter((x): x is string => !!x)
+    const externalIds = externalIdsOf(rows)
     const { data: existing } = await supabase
       .from('payments')
       .select('external_id')
       .eq('space_id', member.space_id)
       .in('external_id', externalIds.length > 0 ? externalIds : ['__none__'])
     const seen = new Set((existing ?? []).map(e => e.external_id as string))
-    const newRows = rows.filter(r => !r.external_id || !seen.has(r.external_id))
+    const newRows = filterUnseenByExternalId(rows, seen)
 
     if (newRows.length === 0) {
       return NextResponse.json({ imported: 0, message: 'Already up to date' })

lib/paypal-logic.ts

@@ -0,0 +1,85 @@
+// Pure PayPal Transaction Search -> payments mapping + sync dedupe. No I/O,
+// no Supabase, no network: the route does the OAuth + fetch + DB; this module
+// owns "which transactions become rows and which are new" so the money
+// mapping is unit-tested (it was previously inline + untested).
+
+// Structural shape of a PayPal transaction_detail (only the fields we read).
+type PayPalTxn = {
+  transaction_info?: {
+    transaction_id?: string
+    transaction_amount?: { value?: string }
+    transaction_note?: string | null
+    transaction_subject?: string | null
+    transaction_initiation_date?: string
+    transaction_updated_date?: string
+  }
+  payer_info?: {
+    email_address?: string
+    payer_name?: { alternate_full_name?: string }
+  }
+}
+
+export type PayPalPaymentRow = {
+  space_id: string
+  platform: 'paypal'
+  amount: number
+  from_identifier: string
+  from_note: string | null
+  transaction_date: string
+  link_status: 'unlinked'
+  external_id: string | null
+}
+
+function safeIso(value: string | undefined, fallbackIso: string): string {
+  if (!value) return fallbackIso
+  const d = new Date(value)
+  return Number.isNaN(d.getTime()) ? fallbackIso : d.toISOString()
+}
+
+// Incoming payments only (amount > 0), mapped to ledger rows. nowIso is the
+// fallback when a transaction has no/invalid date (PayPal occasionally omits
+// it) so a bad date can't throw and abort the whole sync.
+export function mapPayPalTransactions(
+  transactions: PayPalTxn[],
+  spaceId: string,
+  nowIso: string,
+): PayPalPaymentRow[] {
+  const rows: PayPalPaymentRow[] = []
+  for (const tx of transactions ?? []) {
+    const info = tx.transaction_info
+    const amount = parseFloat(info?.transaction_amount?.value ?? '0')
+    if (!(amount > 0)) continue
+    const payer = tx.payer_info
+    rows.push({
+      space_id: spaceId,
+      platform: 'paypal',
+      amount,
+      from_identifier:
+        payer?.email_address ?? payer?.payer_name?.alternate_full_name ?? 'PayPal User',
+      from_note: info?.transaction_note ?? info?.transaction_subject ?? null,
+      transaction_date: safeIso(
+        info?.transaction_initiation_date ?? info?.transaction_updated_date,
+        nowIso,
+      ),
+      link_status: 'unlinked',
+      external_id: info?.transaction_id ?? null,
+    })
+  }
+  return rows
+}
+
+// payments has no unique constraint on external_id, so dedupe is explicit:
+// keep rows whose external_id is not already present in this space. Rows with
+// no external_id are kept (can't be matched; better a possible dup than a
+// dropped payment).
+export function filterUnseenByExternalId(
+  rows: PayPalPaymentRow[],
+  seenExternalIds: Set<string>,
+): PayPalPaymentRow[] {
+  return rows.filter(r => !r.external_id || !seenExternalIds.has(r.external_id))
+}
+
+// External ids present in a batch, for the "already in this space?" query.
+export function externalIdsOf(rows: PayPalPaymentRow[]): string[] {
+  return rows.map(r => r.external_id).filter((x): x is string => !!x)
+}