Merge remote-tracking branch 'origin/staging' into feat/quickbooks-integration

Author: waleedlatif1

apps/sim/app/(landing)/demo/components/demo-scheduler/demo-scheduler.test.tsx

@@ -5,21 +5,30 @@ import { act } from 'react'
 import { createRoot, type Root } from 'react-dom/client'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockCal, mockCalComponent, mockConsent, mockGetCalApi, mockTrackGoogleEvent } = vi.hoisted(
-  () => ({
-    mockCal: vi.fn(),
-    mockCalComponent: vi.fn(() => null),
-    mockConsent: { marketing: true, measurement: true },
-    mockGetCalApi: vi.fn(),
-    mockTrackGoogleEvent: vi.fn(),
-  })
-)
+const {
+  mockCal,
+  mockCalComponent,
+  mockConsent,
+  mockGetCalApi,
+  mockTrackGoogleAdsConversion,
+  mockTrackGoogleEvent,
+} = vi.hoisted(() => ({
+  mockCal: vi.fn(),
+  mockCalComponent: vi.fn(() => null),
+  mockConsent: { marketing: true, measurement: true },
+  mockGetCalApi: vi.fn(),
+  mockTrackGoogleAdsConversion: vi.fn(),
+  mockTrackGoogleEvent: vi.fn(),
+}))
 
 vi.mock('@calcom/embed-react', () => ({
   default: mockCalComponent,
   getCalApi: mockGetCalApi,
 }))
-vi.mock('@/lib/analytics/google', () => ({ trackGoogleEvent: mockTrackGoogleEvent }))
+vi.mock('@/lib/analytics/google', () => ({
+  trackGoogleAdsConversion: mockTrackGoogleAdsConversion,
+  trackGoogleEvent: mockTrackGoogleEvent,
+}))
 vi.mock('@/lib/consent/scripts', () => ({ X_DEMO_BOOKED_EVENT_ID: 'demo-booked' }))
 vi.mock('@/lib/consent/tracking-consent', () => ({
   useTrackingConsent: () => mockConsent,
@@ -37,6 +46,18 @@ const LEAD = {
   notes: 'Company: Analytical Engines\nTopic: Demo',
 }
 
+interface BookingRegistration {
+  action: string
+  callback: () => void
+}
+
+/** The listener the scheduler registered with the Cal.com embed, if any. */
+function bookingRegistration(): BookingRegistration | undefined {
+  return mockCal.mock.calls.find(([method]) => method === 'on')?.[1] as
+    | BookingRegistration
+    | undefined
+}
+
 describe('DemoScheduler', () => {
   let container: HTMLDivElement
   let root: Root
@@ -101,9 +122,7 @@ describe('DemoScheduler', () => {
       await Promise.resolve()
     })
 
-    const registration = mockCal.mock.calls.find(([method]) => method === 'on')?.[1] as
-      | { action: string; callback: () => void }
-      | undefined
+    const registration = bookingRegistration()
     expect(registration?.action).toBe('bookingSuccessfulV2')
 
     registration?.callback()
@@ -112,6 +131,7 @@ describe('DemoScheduler', () => {
       form_name: 'sim_demo',
       booking_status: 'scheduled',
     })
+    expect(mockTrackGoogleAdsConversion).toHaveBeenCalledWith('demo_booked')
     expect(trackXEvent).toHaveBeenCalledWith('event', 'demo-booked', {})
 
     await act(async () => {
@@ -125,6 +145,23 @@ describe('DemoScheduler', () => {
     root = createRoot(container)
   })
 
+  it('sends measurement analytics but no ad conversion without marketing consent', async () => {
+    mockConsent.marketing = false
+    const trackXEvent = vi.fn()
+    window.twq = trackXEvent
+
+    await act(async () => {
+      root.render(<DemoScheduler lead={LEAD} />)
+      await Promise.resolve()
+    })
+
+    bookingRegistration()?.callback()
+
+    expect(mockTrackGoogleEvent).toHaveBeenCalledOnce()
+    expect(mockTrackGoogleAdsConversion).not.toHaveBeenCalled()
+    expect(trackXEvent).not.toHaveBeenCalled()
+  })
+
   it('does not register booking analytics without measurement or marketing consent', async () => {
     mockConsent.marketing = false
     mockConsent.measurement = false
@@ -138,7 +175,7 @@ describe('DemoScheduler', () => {
       hideEventTypeDetails: true,
       styles: { branding: { brandColor: '#6f3dfa' } },
     })
-    expect(mockCal.mock.calls.some(([method]) => method === 'on')).toBe(false)
+    expect(bookingRegistration()).toBeUndefined()
   })
 
   it('preloads the configured booker only once', async () => {

apps/sim/app/(landing)/demo/components/demo-scheduler/demo-scheduler.tsx

@@ -2,7 +2,7 @@
 
 import { useEffect } from 'react'
 import Cal, { getCalApi } from '@calcom/embed-react'
-import { trackGoogleEvent } from '@/lib/analytics/google'
+import { trackGoogleAdsConversion, trackGoogleEvent } from '@/lib/analytics/google'
 import { X_DEMO_BOOKED_EVENT_ID } from '@/lib/consent/scripts'
 import { useTrackingConsent } from '@/lib/consent/tracking-consent'
 import type { DemoLead } from '@/app/(landing)/demo/components/demo-form'
@@ -102,7 +102,10 @@ export function DemoScheduler({ lead }: DemoSchedulerProps) {
           booking_status: 'scheduled',
         })
       }
-      if (marketing) window.twq?.('event', X_DEMO_BOOKED_EVENT_ID, {})
+      if (marketing) {
+        trackGoogleAdsConversion('demo_booked')
+        window.twq?.('event', X_DEMO_BOOKED_EVENT_ID, {})
+      }
     }
     const api = getCalApi({ namespace: CAL_NAMESPACE, embedJsUrl: CAL_EMBED.embedJsUrl })
     api

apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/members/bulk/route.ts

@@ -11,8 +11,8 @@ import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
 import { getSession } from '@/lib/auth'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { PERMISSION_GROUP_MEMBER_CONSTRAINTS } from '@/lib/permission-groups/constraints'
+import { acquirePermissionGroupOrgLock } from '@/lib/permission-groups/locks'
 import {
-  acquirePermissionGroupOrgLock,
   authorizeOrgAccessControl,
   findScopeConflicts,
   formatScopeConflictError,

apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/members/route.ts

@@ -11,10 +11,10 @@ import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
 import { getSession } from '@/lib/auth'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { PERMISSION_GROUP_MEMBER_CONSTRAINTS } from '@/lib/permission-groups/constraints'
+import { acquirePermissionGroupOrgLock } from '@/lib/permission-groups/locks'
 import { isOrganizationMember } from '@/lib/workspaces/permissions/utils'
 import {
   type AllMembersConflict,
-  acquirePermissionGroupOrgLock,
   authorizeOrgAccessControl,
   findAllMembersWorkspaceConflict,
   findScopeConflicts,

apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.ts

@@ -15,9 +15,9 @@ import {
   type PermissionGroupConfig,
   parsePermissionGroupConfig,
 } from '@/lib/permission-groups/fields'
+import { acquirePermissionGroupOrgLock } from '@/lib/permission-groups/locks'
 import {
   type AllMembersConflict,
-  acquirePermissionGroupOrgLock,
   authorizeOrgAccessControl,
   findAllMembersWorkspaceConflict,
   findScopeConflicts,

apps/sim/app/api/organizations/[id]/permission-groups/route.ts

@@ -21,9 +21,9 @@ import {
   type PermissionGroupConfig,
   parsePermissionGroupConfig,
 } from '@/lib/permission-groups/fields'
+import { acquirePermissionGroupOrgLock } from '@/lib/permission-groups/locks'
 import {
   type AllMembersConflict,
-  acquirePermissionGroupOrgLock,
   authorizeOrgAccessControl,
   findAllMembersWorkspaceConflict,
   findWorkspacesNotInOrganization,

apps/sim/app/api/organizations/[id]/permission-groups/utils.ts

@@ -41,36 +41,6 @@ export async function authorizeOrgAccessControl(
   return null
 }
 
-const PERMISSION_GROUP_LOCK_TIMEOUT_MS = 5_000
-
-/**
- * Serialize all permission-group membership and scope writes for an organization
- * via a transaction-scoped Postgres advisory lock. Callers acquire it at the top
- * of the transaction that both checks (`findScopeConflicts`) and mutates, so a
- * concurrent member add or scope change can't commit in the check-to-write
- * window and leave a user governed by two groups on the same workspace.
- *
- * The invariant (one effective group per user per workspace) spans users and
- * groups in ways a unique constraint can't express, and these are low-frequency
- * admin writes, so a single org-scoped lock is simpler and more obviously
- * correct than fine-grained per-user/per-group locks with acquire-ordering.
- *
- * `pg_advisory_xact_lock` auto-releases at transaction end (safe on pooled
- * connections), and `lock_timeout` bounds the wait (raising SQLSTATE 55P03)
- * instead of hanging if a holder is stuck.
- */
-export async function acquirePermissionGroupOrgLock(
-  tx: DbOrTx,
-  organizationId: string
-): Promise<void> {
-  await tx.execute(
-    sql`select set_config('lock_timeout', ${`${PERMISSION_GROUP_LOCK_TIMEOUT_MS}ms`}, true)`
-  )
-  await tx.execute(
-    sql`select pg_advisory_xact_lock(hashtextextended(${`permission_group:${organizationId}`}, 0))`
-  )
-}
-
 /** Load a permission group only if it belongs to the given organization. */
 export async function loadGroupInOrganization(
   groupId: string,

apps/sim/app/api/workspaces/route.ts

@@ -2,6 +2,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
 import { db } from '@sim/db'
 import { type WorkspaceMode, workflow } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
+import { getPostgresErrorCode } from '@sim/utils/errors'
 import { and, eq, isNull } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
 import { listWorkspacesQuerySchema } from '@/lib/api/contracts'
@@ -157,6 +158,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
       workspaceMode: creationPolicy.workspaceMode,
       billedAccountUserId: creationPolicy.billedAccountUserId,
       observedOrganizationId: creationPolicy.observedOrganizationId,
+      governingPermissionGroupOrganizationId: creationPolicy.governingPermissionGroupOrganizationId,
     })
 
     captureServerEvent(
@@ -207,6 +209,20 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
         { status: 409 }
       )
     }
+    /**
+     * A lock timeout is contention, not a fault: creation serializes on the
+     * organization's mutation locks and now also on `permission_group:<org>`,
+     * so a concurrent create or a permission-group admin write can exhaust the
+     * `lock_timeout` and abort this transaction. Answer 503 like the
+     * permission-group routes do, rather than letting it reach the generic 500
+     * below — the caller should retry, and a 500 tells them the opposite.
+     */
+    if (getPostgresErrorCode(error) === '55P03') {
+      return NextResponse.json(
+        { error: 'This organization is being updated by another request. Please try again.' },
+        { status: 503 }
+      )
+    }
     logger.error('Error creating workspace:', error)
     return NextResponse.json({ error: 'Failed to create workspace' }, { status: 500 })
   }
@@ -220,6 +236,7 @@ async function createDefaultWorkspace(
     workspaceMode: WorkspaceMode
     billedAccountUserId: string
     observedOrganizationId: string | null
+    governingPermissionGroupOrganizationId: string | null
   }
 ) {
   const firstName = userName?.split(' ')[0] || null
@@ -231,6 +248,7 @@ async function createDefaultWorkspace(
     workspaceMode: creationPolicy.workspaceMode,
     billedAccountUserId: creationPolicy.billedAccountUserId,
     observedOrganizationId: creationPolicy.observedOrganizationId,
+    governingPermissionGroupOrganizationId: creationPolicy.governingPermissionGroupOrganizationId,
   })
 }
 

apps/sim/lib/analytics/google.test.ts

@@ -0,0 +1,27 @@
+/**
+ * @vitest-environment jsdom
+ */
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import { trackGoogleAdsConversion } from '@/lib/analytics/google'
+import { GOOGLE_ADS_ID } from '@/lib/consent/scripts'
+
+afterEach(() => {
+  window.gtag = undefined
+})
+
+describe('trackGoogleAdsConversion', () => {
+  it('addresses the conversion to the Ads tag and its registered label', () => {
+    const gtag = vi.fn()
+    window.gtag = gtag
+
+    trackGoogleAdsConversion('demo_booked')
+
+    expect(gtag).toHaveBeenCalledWith('event', 'conversion', {
+      send_to: `${GOOGLE_ADS_ID}/Xt8wCK7b1e4cEL_Zk99C`,
+    })
+  })
+
+  it('is a no-op when the Google tag has not loaded', () => {
+    expect(() => trackGoogleAdsConversion('demo_booked')).not.toThrow()
+  })
+})

apps/sim/lib/analytics/google.ts

@@ -1,4 +1,11 @@
-import { GOOGLE_ANALYTICS_ID } from '@/lib/consent/scripts'
+import { GOOGLE_ADS_ID, GOOGLE_ANALYTICS_ID } from '@/lib/consent/scripts'
+
+/** Conversion labels registered in Google Ads, keyed by the action they measure. */
+const GOOGLE_ADS_CONVERSION_LABELS = {
+  demo_booked: 'Xt8wCK7b1e4cEL_Zk99C',
+} as const
+
+export type GoogleAdsConversion = keyof typeof GOOGLE_ADS_CONVERSION_LABELS
 
 interface GoogleAnalyticsEventMap {
   sign_up: { method: string }
@@ -17,6 +24,17 @@ export function trackGoogleEvent<E extends keyof GoogleAnalyticsEventMap>(
   window.gtag?.('event', name, parameters)
 }
 
+/**
+ * Records a Google Ads conversion, addressed as `<tag id>/<conversion label>`.
+ * Call only after the caller has verified marketing consent: without it Consent
+ * Mode keeps `ad_storage` denied and the hit could not be attributed to a click.
+ */
+export function trackGoogleAdsConversion(conversion: GoogleAdsConversion): void {
+  window.gtag?.('event', 'conversion', {
+    send_to: `${GOOGLE_ADS_ID}/${GOOGLE_ADS_CONVERSION_LABELS[conversion]}`,
+  })
+}
+
 export function trackGooglePageView(path: string): void {
   window.gtag?.('event', 'page_view', {
     page_path: path,