fix: harden transaction status updates and expand email/e2e coverage (#228)

Author: wheeljackz

app/models/enum.ts

@@ -27,9 +27,10 @@ export const TRANSACTION_STATUS = {
   SUBMITTED: 'SUBMITTED',
   VERIFIED: 'VERIFIED',
   REJECTED: 'REJECTED',
-}
+} as const
 
-export type TransactionStatus = 'SUBMITTED' | 'VERIFIED' | 'REJECTED'
+export type TransactionStatus =
+  typeof TRANSACTION_STATUS[keyof typeof TRANSACTION_STATUS]
 
 export const TRANSACTION_METHOD = {
   BANK_TRANSFER: 'BANK_TRANSFER',

app/models/transaction.ts

@@ -102,10 +102,26 @@ export async function getTransactionById(
 export async function updateTransactionStatus(
   id: string,
   notes: string,
-  status: TransactionStatus
+  status: TransactionStatus,
+  allowedCurrentStatuses?: TransactionStatus[]
 ) {
-  return await db.transaction.update({
-    where: { id },
-    data: { notes, status },
-  })
+  try {
+    return await db.transaction.update({
+      where: allowedCurrentStatuses
+        ? { id, status: { in: allowedCurrentStatuses } }
+        : { id },
+      data: { notes, status },
+    })
+  } catch (e: unknown) {
+    // Prisma P2025: record not found (concurrent status change)
+    if (
+      e instanceof Error &&
+      'code' in e &&
+      (e as { code: string }).code === 'P2025'
+    ) {
+      return null
+    }
+
+    throw e
+  }
 }

app/routes/__tests__/dashboard.transactions.$transactionId.$action.test.ts

@@ -0,0 +1,136 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { action } from '../dashboard.transactions.$transactionId.$action'
+import { TRANSACTION_STATUS } from '~/models/enum'
+import { requireUser } from '~/services/auth.server'
+import { getFirstCourse } from '~/models/course'
+import { requireCourseAuthor } from '~/utils/permissions'
+import {
+  getTransactionById,
+  updateTransactionStatus,
+} from '~/models/transaction'
+import {
+  activateSubscription,
+  deactivateSubscription,
+} from '~/models/subscription'
+
+vi.mock('~/services/auth.server', () => ({
+  requireUser: vi.fn(),
+}))
+
+vi.mock('~/models/course', () => ({
+  getFirstCourse: vi.fn(),
+}))
+
+vi.mock('~/utils/permissions', () => ({
+  requireCourseAuthor: vi.fn(),
+}))
+
+vi.mock('~/models/transaction', () => ({
+  getTransactionById: vi.fn(),
+  updateTransactionStatus: vi.fn(),
+}))
+
+vi.mock('~/models/subscription', () => ({
+  activateSubscription: vi.fn(),
+  deactivateSubscription: vi.fn(),
+}))
+
+function buildRequest(status: string, notes = 'Catatan verifikasi') {
+  const formData = new URLSearchParams({ status, notes })
+
+  return new Request(
+    'http://localhost:3000/dashboard/transactions/tx-1/verify',
+    {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
+      body: formData,
+    }
+  )
+}
+
+async function runAction(status: string) {
+  return action({
+    request: buildRequest(status),
+    params: { transactionId: 'tx-1' },
+    context: {},
+  } as never)
+}
+
+describe('dashboard.transactions.$transactionId.$action action', () => {
+  beforeEach(() => {
+    vi.resetAllMocks()
+
+    vi.mocked(requireUser).mockResolvedValue({ id: 'author-1' } as never)
+    vi.mocked(getFirstCourse).mockResolvedValue({ id: 'course-1' } as never)
+    vi.mocked(requireCourseAuthor).mockReturnValue(true)
+    vi.mocked(getTransactionById).mockResolvedValue({
+      id: 'tx-1',
+      status: TRANSACTION_STATUS.SUBMITTED,
+      userId: 'user-1',
+    } as never)
+    vi.mocked(updateTransactionStatus).mockResolvedValue({
+      id: 'tx-1',
+      status: TRANSACTION_STATUS.VERIFIED,
+      userId: 'user-1',
+    } as never)
+    vi.mocked(activateSubscription).mockResolvedValue({ id: 'sub-1' } as never)
+    vi.mocked(deactivateSubscription).mockResolvedValue({
+      id: 'sub-1',
+    } as never)
+  })
+
+  it('rejects tampered status payloads that are outside allowlist', async () => {
+    try {
+      await runAction(TRANSACTION_STATUS.SUBMITTED)
+      throw new Error('Expected action to throw Response')
+    } catch (error) {
+      expect(error).toBeInstanceOf(Response)
+      const response = error as Response
+      expect(response.status).toBe(400)
+      await expect(response.json()).resolves.toBe(
+        'Status transaksi tidak valid.'
+      )
+    }
+
+    expect(updateTransactionStatus).not.toHaveBeenCalled()
+  })
+
+  it('rejects forbidden transition from VERIFIED to REJECTED', async () => {
+    vi.mocked(getTransactionById).mockResolvedValue({
+      id: 'tx-1',
+      status: TRANSACTION_STATUS.VERIFIED,
+      userId: 'user-1',
+    } as never)
+
+    try {
+      await runAction(TRANSACTION_STATUS.REJECTED)
+      throw new Error('Expected action to throw Response')
+    } catch (error) {
+      expect(error).toBeInstanceOf(Response)
+      const response = error as Response
+      expect(response.status).toBe(400)
+      await expect(response.json()).resolves.toBe(
+        'Transaksi yang sudah diverifikasi tidak dapat diubah menjadi ditolak.'
+      )
+    }
+
+    expect(updateTransactionStatus).not.toHaveBeenCalled()
+  })
+
+  it('accepts VERIFIED status and updates transaction', async () => {
+    const response = await runAction(TRANSACTION_STATUS.VERIFIED)
+
+    expect(response).toBeInstanceOf(Response)
+    if (!(response instanceof Response)) throw new Error('Expected Response')
+    expect(response.status).toBe(302)
+    expect(response.headers.get('Location')).toBe(
+      '/dashboard/transactions/tx-1'
+    )
+    expect(updateTransactionStatus).toHaveBeenCalledWith(
+      'tx-1',
+      'Catatan verifikasi',
+      TRANSACTION_STATUS.VERIFIED,
+      [TRANSACTION_STATUS.SUBMITTED, TRANSACTION_STATUS.REJECTED]
+    )
+  })
+})

app/routes/dashboard.transactions.$transactionId.$action.tsx

@@ -15,6 +15,7 @@ import {
   TransactionWithUser,
   updateTransactionStatus,
 } from '~/models/transaction'
+import { canUpdateTransactionStatus } from '~/utils/transaction-status'
 import { printLocaleDateTimeString, printRupiah } from '~/utils/format'
 import { TransactionStatus, TRANSACTION_STATUS } from '~/models/enum'
 import { requireUser } from '~/services/auth.server'
@@ -75,13 +76,42 @@ export const action: ActionFunction = async ({ request, params }) => {
     }
   }
 
+  if (
+    status !== TRANSACTION_STATUS.VERIFIED &&
+    status !== TRANSACTION_STATUS.REJECTED
+  ) {
+    throw json('Status transaksi tidak valid.', { status: 400 })
+  }
+
+  const existingTransaction = await getTransactionById(transactionId)
+  if (!existingTransaction) {
+    return redirect('/dashboard/transactions')
+  }
+
+  if (
+    !canUpdateTransactionStatus(
+      existingTransaction.status as TransactionStatus,
+      status as TransactionStatus
+    )
+  ) {
+    throw json(
+      'Transaksi yang sudah diverifikasi tidak dapat diubah menjadi ditolak.',
+      { status: 400 }
+    )
+  }
+
   const transaction = await updateTransactionStatus(
     transactionId,
     notes,
-    status as TransactionStatus
+    status as TransactionStatus,
+    status === TRANSACTION_STATUS.REJECTED
+      ? [TRANSACTION_STATUS.SUBMITTED]
+      : [TRANSACTION_STATUS.SUBMITTED, TRANSACTION_STATUS.REJECTED]
   )
   if (!transaction) {
-    throw json({ transaction, notes, status }, { status: 500 })
+    throw json('Transaksi gagal diperbarui karena status sudah berubah.', {
+      status: 409,
+    })
   }
 
   if (transaction.status === TRANSACTION_STATUS.VERIFIED) {

app/routes/dashboard.transactions.$transactionId.tsx

@@ -49,11 +49,13 @@ export default function TransactionDetailsPage() {
     <SingleColumnLayout>
       <div className="min-h-full">
         <TransactionDetails transaction={transaction} user={transaction.user}>
-          {/* TODO: Disable rejecting a verified transaction */}
           <TertiaryButtonLink
             to={`reject?${searchParams.toString()}`}
             replace
-            disabled={transaction.status === TRANSACTION_STATUS.REJECTED}
+            disabled={
+              transaction.status === TRANSACTION_STATUS.REJECTED ||
+              transaction.status === TRANSACTION_STATUS.VERIFIED
+            }
           >
             Tolak Transaksi
           </TertiaryButtonLink>

app/services/__tests__/email.server.test.ts

@@ -1,11 +1,19 @@
+import { vi, describe, it, expect, beforeEach } from 'vitest'
 import { User } from '@prisma/client'
 import { sendEmail } from '../email.server'
+import * as emailProvider from '~/services/email-provider.server'
 import { userBuilder } from '~/models/__mocks__/user'
 
 describe('sendEmail', () => {
   const user = userBuilder() as User
 
+  beforeEach(() => {
+    vi.restoreAllMocks()
+  })
+
   it('call emailProvider.sendEmail method', async () => {
+    const spy = vi.spyOn(emailProvider, 'sendEmail')
+
     await sendEmail({
       emailAddress: user.email,
       magicLink: 'http://localhost:3000/magic',
@@ -14,6 +22,14 @@ describe('sendEmail', () => {
       form: new FormData(),
     })
 
-    // TODO: assert emailProvider.sendEmail was called
+    expect(spy).toHaveBeenCalledOnce()
+    expect(spy).toHaveBeenCalledWith(
+      expect.objectContaining({
+        to: user.email,
+        from: expect.any(String),
+        subject: expect.any(String),
+        html: expect.stringContaining('localhost:3000/magic'),
+      })
+    )
   })
 })

app/utils/__tests__/transaction-status.test.ts

@@ -0,0 +1,58 @@
+import { canUpdateTransactionStatus } from '../transaction-status'
+import { TRANSACTION_STATUS } from '~/models/enum'
+
+describe('canUpdateTransactionStatus', () => {
+  it('allows submitted to verified', () => {
+    expect(
+      canUpdateTransactionStatus(
+        TRANSACTION_STATUS.SUBMITTED,
+        TRANSACTION_STATUS.VERIFIED
+      )
+    ).toBe(true)
+  })
+
+  it('allows submitted to rejected', () => {
+    expect(
+      canUpdateTransactionStatus(
+        TRANSACTION_STATUS.SUBMITTED,
+        TRANSACTION_STATUS.REJECTED
+      )
+    ).toBe(true)
+  })
+
+  it('prevents verified from being rejected', () => {
+    expect(
+      canUpdateTransactionStatus(
+        TRANSACTION_STATUS.VERIFIED,
+        TRANSACTION_STATUS.REJECTED
+      )
+    ).toBe(false)
+  })
+
+  it('allows verified to remain verified', () => {
+    expect(
+      canUpdateTransactionStatus(
+        TRANSACTION_STATUS.VERIFIED,
+        TRANSACTION_STATUS.VERIFIED
+      )
+    ).toBe(true)
+  })
+
+  it('allows rejected to be re-verified', () => {
+    expect(
+      canUpdateTransactionStatus(
+        TRANSACTION_STATUS.REJECTED,
+        TRANSACTION_STATUS.VERIFIED
+      )
+    ).toBe(true)
+  })
+
+  it('allows rejected to remain rejected', () => {
+    expect(
+      canUpdateTransactionStatus(
+        TRANSACTION_STATUS.REJECTED,
+        TRANSACTION_STATUS.REJECTED
+      )
+    ).toBe(true)
+  })
+})

app/utils/transaction-status.ts

@@ -0,0 +1,15 @@
+import { TransactionStatus, TRANSACTION_STATUS } from '~/models/enum'
+
+export function canUpdateTransactionStatus(
+  currentStatus: TransactionStatus,
+  destinationStatus: TransactionStatus
+) {
+  if (
+    currentStatus === TRANSACTION_STATUS.VERIFIED &&
+    destinationStatus === TRANSACTION_STATUS.REJECTED
+  ) {
+    return false
+  }
+
+  return true
+}

e2e/profile.spec.ts

@@ -30,7 +30,9 @@ test('Validate phone number when updating data', async ({
     name: /nomor whatsapp/i,
   })
 
-  // Fill phoneNumber with invalid value
+  // Click first to ensure onFocus fires reliably on WebKit (iPhone 11),
+  // then fill with invalid value and blur to trigger client-side validation.
+  await phoneNumber.click()
   await phoneNumber.fill('6512345678')
   await phoneNumber.blur()
 
@@ -64,7 +66,9 @@ test('Validate name when updating data', async ({ page, noscript, screen }) => {
     name: /nama lengkap/i,
   })
 
-  // Clear name and trigger validation
+  // Click first to ensure onFocus fires reliably on WebKit (iPhone 11),
+  // then clear and blur to trigger client-side validation.
+  await name.click()
   await name.fill('')
   await name.blur()