Merge pull request #12 from xtr-dev/dev

Dev

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@xtr-dev/payload-billing",
-  "version": "0.1.3",
+  "version": "0.1.4",
   "description": "PayloadCMS plugin for billing and payment provider integrations with tracking and local testing",
   "license": "MIT",
   "type": "module",

src/collections/payments.ts

@@ -1,8 +1,8 @@
-import type { AccessArgs, CollectionBeforeChangeHook, CollectionConfig, CollectionSlug, Field } from 'payload'
+import type { AccessArgs, CollectionBeforeChangeHook, CollectionConfig, Field } from 'payload'
 import type { BillingPluginConfig} from '@/plugin/config';
 import { defaults } from '@/plugin/config'
 import { extractSlug } from '@/plugin/utils'
-import { Payment } from '@/plugin/types/payments'
+import type { Payment } from '@/plugin/types/payments'
 import { initProviderPayment } from '@/collections/hooks'
 
 export function createPaymentsCollection(pluginConfig: BillingPluginConfig): CollectionConfig {
@@ -79,7 +79,7 @@ export function createPaymentsCollection(pluginConfig: BillingPluginConfig): Col
       admin: {
         position: 'sidebar',
       },
-      relationTo: extractSlug(pluginConfig.collections?.invoices || defaults.invoicesCollection) as CollectionSlug,
+      relationTo: extractSlug(pluginConfig.collections?.invoices || defaults.invoicesCollection),
     },
     {
       name: 'metadata',
@@ -104,7 +104,16 @@ export function createPaymentsCollection(pluginConfig: BillingPluginConfig): Col
         readOnly: true,
       },
       hasMany: true,
-      relationTo: extractSlug(pluginConfig.collections?.refunds || defaults.refundsCollection) as CollectionSlug,
+      relationTo: extractSlug(pluginConfig.collections?.refunds || defaults.refundsCollection),
+    },
+    {
+      name: 'version',
+      type: 'number',
+      defaultValue: 1,
+      admin: {
+        hidden: true, // Hide from admin UI to prevent manual tampering
+      },
+      index: true, // Index for optimistic locking performance
     },
   ]
   if (overrides?.fields) {
@@ -127,7 +136,7 @@ export function createPaymentsCollection(pluginConfig: BillingPluginConfig): Col
     fields,
     hooks: {
       beforeChange: [
-        async ({ data, operation, req }) => {
+        async ({ data, operation, req, originalDoc }) => {
           if (operation === 'create') {
             // Validate amount format
             if (data.amount && !Number.isInteger(data.amount)) {
@@ -144,6 +153,15 @@ export function createPaymentsCollection(pluginConfig: BillingPluginConfig): Col
 
             await initProviderPayment(req.payload, data)
           }
+
+          // Auto-increment version for manual updates (not webhook updates)
+          // Webhook updates handle their own versioning in updatePaymentStatus
+          if (operation === 'update' && !data.version) {
+            // If version is not being explicitly set (i.e., manual admin update),
+            // increment it automatically
+            const currentVersion = (originalDoc as Payment)?.version || 1
+            data.version = currentVersion + 1
+          }
         },
       ] satisfies CollectionBeforeChangeHook<Payment>[],
     },

src/plugin/types/payments.ts

@@ -48,6 +48,10 @@ export interface Payment {
     | boolean
     | null;
   refunds?: (number | Refund)[] | null;
+  /**
+   * Version number for optimistic locking (auto-incremented on updates)
+   */
+  version?: number;
   updatedAt: string;
   createdAt: string;
 }

src/plugin/utils.ts

@@ -1,6 +1,15 @@
 import type { CollectionConfig, CollectionSlug, Field } from 'payload'
+import type { Id } from '@/plugin/types'
 
 export type FieldsOverride = (args: { defaultFields: Field[] }) => Field[]
 
 export const extractSlug =
   (arg: string | Partial<CollectionConfig>) => (typeof arg === 'string' ? arg : arg.slug!) as CollectionSlug
+
+/**
+ * Safely cast ID types for PayloadCMS operations
+ * This utility provides a typed way to handle the mismatch between our Id type and PayloadCMS expectations
+ */
+export function toPayloadId(id: Id): any {
+  return id as any
+}

src/providers/stripe.ts

@@ -74,7 +74,7 @@ export const stripeProvider = (stripeConfig: StripeProviderConfig) => {
               // Verify webhook signature and construct event
               let event: Stripe.Event
               try {
-                event = stripe.webhooks.constructEvent(body, signature, stripeConfig.webhookSecret)
+                event = stripe.webhooks.constructEvent(body, signature, stripeConfig.webhookSecret!)
               } catch (err) {
                 return handleWebhookError('Stripe', err, 'Signature verification failed')
               }

src/providers/utils.ts

@@ -1,8 +1,9 @@
 import type { Payload } from 'payload'
 import type { Payment } from '@/plugin/types/payments'
 import type { BillingPluginConfig } from '@/plugin/config'
+import type { ProviderData } from './types'
 import { defaults } from '@/plugin/config'
-import { extractSlug } from '@/plugin/utils'
+import { extractSlug, toPayloadId } from '@/plugin/utils'
 
 /**
  * Common webhook response utilities
@@ -43,61 +44,77 @@ export async function findPaymentByProviderId(
 }
 
 /**
- * Update payment status and provider data with proper optimistic locking
+ * Update payment status and provider data with optimistic locking
  */
 export async function updatePaymentStatus(
   payload: Payload,
   paymentId: string | number,
   status: Payment['status'],
-  providerData: any,
+  providerData: ProviderData<any>,
   pluginConfig: BillingPluginConfig
 ): Promise<boolean> {
   const paymentsCollection = extractSlug(pluginConfig.collections?.payments || defaults.paymentsCollection)
 
-  // Get current payment to check for concurrent modifications
-  const currentPayment = await payload.findByID({
-    collection: paymentsCollection,
-    id: paymentId as any // Cast to avoid type mismatch between Id and PayloadCMS types
-  }) as Payment
-
-  const now = new Date().toISOString()
-
-  // First, try to find payments that match both ID and current updatedAt
-  const conflictCheck = await payload.find({
-    collection: paymentsCollection,
-    where: {
-      id: { equals: paymentId },
-      updatedAt: { equals: currentPayment.updatedAt }
-    }
-  })
-
-  // If no matching payment found, it means it was modified concurrently
-  if (conflictCheck.docs.length === 0) {
-    console.warn(`[Payment Update] Concurrent modification detected for payment ${paymentId}, skipping update`)
-    return false
-  }
-
   try {
-    const result = await payload.update({
+    // First, fetch the current payment to get the current version
+    const currentPayment = await payload.findByID({
       collection: paymentsCollection,
-      id: paymentId as any, // Cast to avoid type mismatch between Id and PayloadCMS types
-      data: {
-        status,
-        providerData: {
-          ...providerData,
-          webhookProcessedAt: now,
-          previousStatus: currentPayment.status
-        }
-      }
-    })
+      id: toPayloadId(paymentId),
+    }) as Payment
 
-    // Verify the update actually happened by checking if updatedAt changed
-    if (result.updatedAt === currentPayment.updatedAt) {
-      console.warn(`[Payment Update] Update may have failed for payment ${paymentId} - updatedAt unchanged`)
+    if (!currentPayment) {
+      console.error(`[Payment Update] Payment ${paymentId} not found`)
       return false
     }
 
-    return true
+    const currentVersion = currentPayment.version || 1
+
+    // Attempt to update with optimistic locking
+    // We'll use a transaction to ensure atomicity
+    const transactionID = await payload.db.beginTransaction()
+
+    if (!transactionID) {
+      console.error(`[Payment Update] Failed to begin transaction`)
+      return false
+    }
+
+    try {
+      // Re-fetch within transaction to ensure consistency
+      const paymentInTransaction = await payload.findByID({
+        collection: paymentsCollection,
+        id: toPayloadId(paymentId),
+        req: { transactionID: transactionID }
+      }) as Payment
+
+      // Check if version still matches
+      if ((paymentInTransaction.version || 1) !== currentVersion) {
+        // Version conflict detected - payment was modified by another process
+        console.warn(`[Payment Update] Version conflict for payment ${paymentId} (expected version: ${currentVersion}, got: ${paymentInTransaction.version})`)
+        await payload.db.rollbackTransaction(transactionID)
+        return false
+      }
+
+      // Update with new version
+      await payload.update({
+        collection: paymentsCollection,
+        id: toPayloadId(paymentId),
+        data: {
+          status,
+          providerData: {
+            ...providerData,
+            webhookProcessedAt: new Date().toISOString()
+          },
+          version: currentVersion + 1
+        },
+        req: { transactionID: transactionID }
+      })
+
+      await payload.db.commitTransaction(transactionID)
+      return true
+    } catch (error) {
+      await payload.db.rollbackTransaction(transactionID)
+      throw error
+    }
   } catch (error) {
     console.error(`[Payment Update] Failed to update payment ${paymentId}:`, error)
     return false
@@ -121,10 +138,10 @@ export async function updateInvoiceOnPaymentSuccess(
 
   await payload.update({
     collection: invoicesCollection,
-    id: invoiceId as any, // Cast to avoid type mismatch between Id and PayloadCMS types
+    id: toPayloadId(invoiceId),
     data: {
       status: 'paid',
-      payment: payment.id
+      payment: toPayloadId(payment.id)
     }
   })
 }
@@ -187,4 +204,4 @@ export function validateProductionUrl(url: string | undefined, urlType: string):
   } catch {
     throw new Error(`${urlType} URL is not a valid URL`)
   }
-}
+}