mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(billing): stop showing "View all" when there are no more invoices (#5387)
* fix(billing): stop showing "View all" when there are no more invoices Show fewer invoices (10) and derive hasMore from the finalized-invoice count instead of Stripe's raw has_more, which counted drafts we filter out client-side and could report more invoices than actually exist. * chore(billing): move invoice pagination explanation into TSDoc Extract collectFinalizedInvoices with a TSDoc comment explaining the Stripe has_more/draft-filtering rationale, instead of an inline comment block. * fix(billing): don't hide View all when the page-cap is hit inconclusively Greptile P1: if MAX_STRIPE_PAGES is exhausted while the finalized count sits exactly at MAX_INVOICES and Stripe still has_more, hasMore was silently returned as false. collectFinalizedInvoices now also returns whether Stripe's cursor was still open at exit, and the route ORs that into hasMore so the safety cap can never suppress "View all". Also asserts starting_after cursor propagation across pages and adds a test for the safety-cap-hit case.
This commit is contained in:
@@ -0,0 +1,109 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { createMockRequest, dbChainMock, dbChainMockFns } from '@sim/testing'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { mockGetSession, mockGetStripeClient, mockStripeInvoicesList } = vi.hoisted(() => ({
|
||||
mockGetSession: vi.fn(),
|
||||
mockGetStripeClient: vi.fn(),
|
||||
mockStripeInvoicesList: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@sim/db', () => dbChainMock)
|
||||
|
||||
vi.mock('@/lib/auth', () => ({
|
||||
auth: { api: { getSession: vi.fn() } },
|
||||
getSession: mockGetSession,
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/billing/stripe-client', () => ({
|
||||
getStripeClient: mockGetStripeClient,
|
||||
}))
|
||||
|
||||
import { GET } from '@/app/api/billing/invoices/route'
|
||||
|
||||
function makeInvoice(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: `in_${Math.random().toString(36).slice(2)}`,
|
||||
number: 'INV-1',
|
||||
created: 1700000000,
|
||||
total: 1000,
|
||||
amount_paid: 1000,
|
||||
currency: 'usd',
|
||||
status: 'paid',
|
||||
hosted_invoice_url: 'https://stripe.test/invoice',
|
||||
invoice_pdf: 'https://stripe.test/invoice.pdf',
|
||||
...overrides,
|
||||
}
|
||||
}
|
||||
|
||||
describe('GET /api/billing/invoices', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockGetSession.mockResolvedValue({ user: { id: 'user-1' } })
|
||||
dbChainMockFns.limit.mockResolvedValue([{ customer: 'cus_1' }])
|
||||
mockGetStripeClient.mockReturnValue({ invoices: { list: mockStripeInvoicesList } })
|
||||
})
|
||||
|
||||
it('does not surface hasMore when the trailing raw invoice beyond MAX_INVOICES is a draft', async () => {
|
||||
const finalized = Array.from({ length: 10 }, () => makeInvoice())
|
||||
mockStripeInvoicesList.mockResolvedValueOnce({
|
||||
data: [...finalized, makeInvoice({ status: 'draft' })],
|
||||
has_more: false,
|
||||
})
|
||||
|
||||
const request = createMockRequest('GET')
|
||||
const response = await GET(request)
|
||||
const body = await response.json()
|
||||
|
||||
expect(body.invoices).toHaveLength(10)
|
||||
expect(body.hasMore).toBe(false)
|
||||
})
|
||||
|
||||
it('reports hasMore when there are genuinely more finalized invoices', async () => {
|
||||
const finalized = Array.from({ length: 11 }, () => makeInvoice())
|
||||
mockStripeInvoicesList.mockResolvedValueOnce({ data: finalized, has_more: false })
|
||||
|
||||
const request = createMockRequest('GET')
|
||||
const response = await GET(request)
|
||||
const body = await response.json()
|
||||
|
||||
expect(body.invoices).toHaveLength(10)
|
||||
expect(body.hasMore).toBe(true)
|
||||
})
|
||||
|
||||
it('pages through further drafts to confirm hasMore when the first page is inconclusive', async () => {
|
||||
const firstPage = Array.from({ length: 11 }, () => makeInvoice({ status: 'draft' }))
|
||||
mockStripeInvoicesList
|
||||
.mockResolvedValueOnce({ data: firstPage, has_more: true })
|
||||
.mockResolvedValueOnce({ data: [makeInvoice()], has_more: false })
|
||||
|
||||
const request = createMockRequest('GET')
|
||||
const response = await GET(request)
|
||||
const body = await response.json()
|
||||
|
||||
expect(mockStripeInvoicesList).toHaveBeenCalledTimes(2)
|
||||
expect(mockStripeInvoicesList).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({ starting_after: firstPage.at(-1)?.id })
|
||||
)
|
||||
expect(body.invoices).toHaveLength(1)
|
||||
expect(body.hasMore).toBe(false)
|
||||
})
|
||||
|
||||
it('reports hasMore when the MAX_STRIPE_PAGES safety cap is hit while Stripe still has more', async () => {
|
||||
mockStripeInvoicesList.mockResolvedValue({
|
||||
data: Array.from({ length: 11 }, () => makeInvoice({ status: 'draft' })),
|
||||
has_more: true,
|
||||
})
|
||||
|
||||
const request = createMockRequest('GET')
|
||||
const response = await GET(request)
|
||||
const body = await response.json()
|
||||
|
||||
expect(mockStripeInvoicesList).toHaveBeenCalledTimes(5)
|
||||
expect(body.invoices).toHaveLength(0)
|
||||
expect(body.hasMore).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -3,6 +3,7 @@ import { user } from '@sim/db/schema'
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { eq } from 'drizzle-orm'
|
||||
import { type NextRequest, NextResponse } from 'next/server'
|
||||
import type Stripe from 'stripe'
|
||||
import { getInvoicesContract } from '@/lib/api/contracts/subscription'
|
||||
import { parseRequest } from '@/lib/api/server'
|
||||
import { getSession } from '@/lib/auth'
|
||||
@@ -14,7 +15,65 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
|
||||
const logger = createLogger('BillingInvoices')
|
||||
|
||||
/** Cap the number of invoices returned to the most recent statements. */
|
||||
const MAX_INVOICES = 12
|
||||
const MAX_INVOICES = 10
|
||||
|
||||
/** Stripe page size when scanning for finalized invoices; also bounds the has-more probe. */
|
||||
const STRIPE_PAGE_SIZE = MAX_INVOICES + 1
|
||||
|
||||
/** Safety cap on pagination when a customer has many draft invoices interspersed. */
|
||||
const MAX_STRIPE_PAGES = 5
|
||||
|
||||
interface FinalizedInvoicesPage {
|
||||
invoices: Stripe.Invoice[]
|
||||
/**
|
||||
* Whether Stripe's raw cursor still had more records after the scan
|
||||
* stopped — either because `invoices.length` became conclusive, or the
|
||||
* `MAX_STRIPE_PAGES` safety cap was hit first. Callers must OR this into
|
||||
* `hasMore` so hitting the cap never silently hides a "View all" that a
|
||||
* customer with many consecutive drafts genuinely has.
|
||||
*/
|
||||
stripeHasMore: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
* Pages through a customer's Stripe invoices, keeping only finalized ones,
|
||||
* until either more than `MAX_INVOICES` have been collected or Stripe's list
|
||||
* is exhausted (bounded by `MAX_STRIPE_PAGES`).
|
||||
*
|
||||
* Stripe's raw pagination cursor (`has_more`) counts draft invoices, which
|
||||
* the caller filters out — so a single page can under-report finalized
|
||||
* invoices while `has_more` is still true. Paging until the finalized count
|
||||
* is conclusive is what lets the caller derive an accurate `hasMore` for the
|
||||
* "View all" affordance.
|
||||
*/
|
||||
async function collectFinalizedInvoices(
|
||||
stripe: Stripe,
|
||||
stripeCustomerId: string
|
||||
): Promise<FinalizedInvoicesPage> {
|
||||
const invoices: Stripe.Invoice[] = []
|
||||
let startingAfter: string | undefined
|
||||
let stripeHasMore = true
|
||||
|
||||
for (
|
||||
let page = 0;
|
||||
page < MAX_STRIPE_PAGES && stripeHasMore && invoices.length <= MAX_INVOICES;
|
||||
page++
|
||||
) {
|
||||
const result = await stripe.invoices.list({
|
||||
customer: stripeCustomerId,
|
||||
limit: STRIPE_PAGE_SIZE,
|
||||
starting_after: startingAfter,
|
||||
})
|
||||
|
||||
invoices.push(
|
||||
...result.data.filter((invoice) => invoice.id && invoice.status && invoice.status !== 'draft')
|
||||
)
|
||||
stripeHasMore = result.has_more
|
||||
startingAfter = result.data.at(-1)?.id
|
||||
}
|
||||
|
||||
return { invoices, stripeHasMore }
|
||||
}
|
||||
|
||||
/**
|
||||
* Lists finalized Stripe invoices for the caller's billing customer (personal
|
||||
@@ -69,23 +128,21 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await stripe.invoices.list({ customer: stripeCustomerId, limit: MAX_INVOICES })
|
||||
const finalized = await collectFinalizedInvoices(stripe, stripeCustomerId)
|
||||
const hasMore = finalized.invoices.length > MAX_INVOICES || finalized.stripeHasMore
|
||||
const invoices = finalized.invoices.slice(0, MAX_INVOICES).map((invoice) => ({
|
||||
id: invoice.id as string,
|
||||
number: invoice.number ?? null,
|
||||
created: invoice.created,
|
||||
total: invoice.total,
|
||||
amountPaid: invoice.amount_paid,
|
||||
currency: invoice.currency,
|
||||
status: invoice.status ?? null,
|
||||
hostedInvoiceUrl: invoice.hosted_invoice_url ?? null,
|
||||
invoicePdf: invoice.invoice_pdf ?? null,
|
||||
}))
|
||||
|
||||
const invoices = result.data
|
||||
.filter((invoice) => invoice.id && invoice.status && invoice.status !== 'draft')
|
||||
.map((invoice) => ({
|
||||
id: invoice.id as string,
|
||||
number: invoice.number ?? null,
|
||||
created: invoice.created,
|
||||
total: invoice.total,
|
||||
amountPaid: invoice.amount_paid,
|
||||
currency: invoice.currency,
|
||||
status: invoice.status ?? null,
|
||||
hostedInvoiceUrl: invoice.hosted_invoice_url ?? null,
|
||||
invoicePdf: invoice.invoice_pdf ?? null,
|
||||
}))
|
||||
|
||||
return NextResponse.json({ success: true, invoices, hasMore: result.has_more })
|
||||
return NextResponse.json({ success: true, invoices, hasMore })
|
||||
} catch (error) {
|
||||
logger.error('Failed to list invoices', { error, userId: session.user.id, context })
|
||||
return NextResponse.json({ error: 'Failed to list invoices' }, { status: 500 })
|
||||
|
||||
Reference in New Issue
Block a user