Просмотр исходного кода

Subtract job discounts from net profit and give every job a start date (#454)

Net profit in the revenue and parts reports added up line totals and ignored a discount on the whole job. Each line now carries its share of the discount, on screen and in scheduled reports.

Jobs created by the Lubelog, Invoice Ninja and spreadsheet importers, the onboarding sample data and the manual service record form were written without a start date, so date-ranged reports skipped them. They now start on their service date, and a migration backfills existing rows.
Bernt Christian Egeland 2 недель назад
Родитель
Сommit
d1db0a8f67

+ 11 - 0
prisma/migrations/20261010120000_backfill_service_start/migration.sql

@@ -0,0 +1,11 @@
+-- Jobs brought in by the Lubelog, Invoice Ninja and spreadsheet importers were
+-- written without a start, and every date-ranged report finds a job by its
+-- start, so they were in none of them. The start is the service date, the
+-- same rule the 20260327055410 backfill used for older rows.
+BEGIN;
+
+UPDATE "service_records"
+SET "startDateTime" = "serviceDate"
+WHERE "startDateTime" IS NULL;
+
+COMMIT;

+ 62 - 0
src/__tests__/features/reports/every-job-has-a-start.test.ts

@@ -0,0 +1,62 @@
+/**
+ * Every date-ranged report, and the calendar sync, finds a job by its
+ * `startDateTime`. A job written without one is in none of them: the three
+ * importers did exactly that, and a workshop's imported history was missing
+ * from its revenue, profit, parts and tax reports.
+ *
+ * The importers are whole route handlers over a zip upload, so this guards
+ * the rule at the source instead: every call that creates a service record
+ * has to say when it starts.
+ */
+
+import { readdirSync, readFileSync, statSync } from 'node:fs'
+import path from 'node:path'
+import { describe, expect, it } from 'vitest'
+
+const SRC = path.resolve(__dirname, '../../..')
+const SKIP = new Set(['__tests__', 'generated', 'node_modules'])
+
+function sourceFiles(dir: string): string[] {
+  return readdirSync(dir).flatMap((name) => {
+    if (SKIP.has(name)) return []
+    const full = path.join(dir, name)
+    if (statSync(full).isDirectory()) return sourceFiles(full)
+    return /\.tsx?$/.test(name) ? [full] : []
+  })
+}
+
+/** Each `serviceRecord.create(...)` call in a file, from the name to its closing bracket. */
+function createCalls(source: string): string[] {
+  const calls: string[] = []
+  const opener = /serviceRecord\.create\(/g
+  for (let match = opener.exec(source); match; match = opener.exec(source)) {
+    let depth = 0
+    let end = match.index + match[0].length - 1
+    for (; end < source.length; end++) {
+      if (source[end] === '(') depth++
+      else if (source[end] === ')' && --depth === 0) break
+    }
+    calls.push(source.slice(match.index, end + 1))
+  }
+  return calls
+}
+
+describe('creating a service record', () => {
+  const creators = sourceFiles(SRC)
+    .map((file) => ({ file, calls: createCalls(readFileSync(file, 'utf8')) }))
+    .filter(({ calls }) => calls.length > 0)
+
+  it('finds the places that do it', () => {
+    expect(creators.length).toBeGreaterThan(5)
+  })
+
+  // Per call, not per file: the sample data set a start on two of its three
+  // jobs, and the third was missing from every new workshop's reports.
+  it.each(
+    creators.flatMap(({ file, calls }) =>
+      calls.map((call, index) => [`${path.relative(SRC, file)} #${index + 1}`, call])
+    )
+  )('%s sets startDateTime', (_name, call) => {
+    expect(call).toMatch(/startDateTime/)
+  })
+})

+ 81 - 0
src/__tests__/features/reports/reports-tax.test.ts

@@ -174,6 +174,87 @@ describe('getRevenueReport — net parts/labor calculation', () => {
   })
 })
 
+describe('getRevenueReport — a discount on the job comes off the profit', () => {
+  /** 100 of parts (cost 50) and 200 of labor, 10% off the job, 25% tax. */
+  const discounted = (taxInclusive: boolean) => {
+    const scale = taxInclusive ? 1.25 : 1
+    return {
+      serviceDate: new Date('2026-01-15'),
+      startDateTime: new Date('2026-01-15'),
+      totalAmount: 337.5, // (300 - 30) net + 25% tax
+      cost: 337.5,
+      type: 'repair',
+      taxRate: 25,
+      taxInclusive,
+      subtotal: 300 * scale,
+      discountAmount: 30 * scale,
+      manuallyPaid: false,
+      payments: [],
+      partItems: [{ unitCost: 50, quantity: 1, total: 100 * scale }],
+      laborItems: [{ total: 200 * scale }],
+    }
+  }
+
+  it('spreads the discount over parts and labor in exclusive mode', async () => {
+    setupAuth()
+    vi.mocked(db.serviceRecord.findMany).mockResolvedValue([discounted(false)] as any)
+
+    const result = await getRevenueReport({})
+
+    expect(result.data?.summary.totalPartsRevenue).toBeCloseTo(90)
+    expect(result.data?.summary.totalLaborRevenue).toBeCloseTo(180)
+    expect(result.data?.summary.totalPartsNetProfit).toBeCloseTo(40)
+    // What was earned before tax (270) less what the parts cost (50).
+    expect(result.data?.summary.netProfit).toBeCloseTo(220)
+    expect(result.data?.monthly[0].netProfit).toBeCloseTo(220)
+  })
+
+  it('gives the same profit when the same job is entered tax-inclusive', async () => {
+    setupAuth()
+    vi.mocked(db.serviceRecord.findMany).mockResolvedValue([discounted(true)] as any)
+
+    const result = await getRevenueReport({})
+
+    expect(result.data?.summary.totalPartsRevenue).toBeCloseTo(90)
+    expect(result.data?.summary.totalLaborRevenue).toBeCloseTo(180)
+    expect(result.data?.summary.netProfit).toBeCloseTo(220)
+  })
+
+  it('makes revenue before tax and net profit differ by the parts cost alone', async () => {
+    setupAuth()
+    const record = discounted(false)
+    vi.mocked(db.serviceRecord.findMany).mockResolvedValue([record] as any)
+
+    const result = await getRevenueReport({})
+    const summary = result.data!.summary
+
+    const revenueBeforeTax = record.totalAmount / 1.25
+    expect(summary.netProfit).toBeCloseTo(revenueBeforeTax - summary.totalPartsCost)
+  })
+})
+
+describe('getPartsUsageReport — a discount on the job comes off the part', () => {
+  it('agrees with the revenue report on the same job', async () => {
+    setupAuth()
+    vi.mocked(db.servicePart.findMany).mockResolvedValue([
+      {
+        name: 'Brake disc',
+        partNumber: 'BD-1',
+        quantity: 1,
+        total: 100,
+        unitCost: 50,
+        serviceRecord: { taxRate: 25, taxInclusive: false, subtotal: 300, discountAmount: 30 },
+      },
+    ] as any)
+
+    const result = await getPartsUsageReport({})
+
+    expect(result.data?.totalPartsRevenue).toBeCloseTo(90)
+    expect(result.data?.totalPartsNetProfit).toBeCloseTo(40)
+    expect(result.data?.parts[0].netProfit).toBeCloseTo(40)
+  })
+})
+
 // ---------------------------------------------------------------------------
 // getTaxReport
 // ---------------------------------------------------------------------------

+ 43 - 1
src/__tests__/lib/tax.test.ts

@@ -1,7 +1,7 @@
 import { describe, it, expect } from 'vitest'
 import { formatCurrency } from '@/lib/format'
 import { roundAsPrinted, roundMoney } from '@/lib/money'
-import { calculateTotals, discountAmountFor, netLineTotal } from '@/lib/tax'
+import { calculateTotals, discountAmountFor, earnedLineTotal, netLineTotal } from '@/lib/tax'
 
 describe('calculateTotals', () => {
   describe('exclusive mode', () => {
@@ -555,3 +555,45 @@ describe('totals that add up to the cent', () => {
     expect(wrong).toEqual([])
   })
 })
+
+describe('earnedLineTotal', () => {
+  const exclusive = { taxRate: 25, taxInclusive: false }
+  const inclusive = { taxRate: 25, taxInclusive: true }
+
+  it('is the net line total when the job has no discount', () => {
+    expect(earnedLineTotal(100, { ...exclusive, subtotal: 300, discountAmount: 0 })).toBe(100)
+    expect(earnedLineTotal(125, { ...inclusive, subtotal: 375, discountAmount: 0 })).toBeCloseTo(
+      100
+    )
+  })
+
+  it("takes the line's share of the discount off", () => {
+    // 10% off a 300 job: a 100 line earned 90.
+    expect(earnedLineTotal(100, { ...exclusive, subtotal: 300, discountAmount: 30 })).toBeCloseTo(
+      90
+    )
+  })
+
+  it('gives the same figure in inclusive mode, where line and discount are both gross', () => {
+    // The same job entered gross: lines 125 + 250, discount 37.50 gross.
+    expect(earnedLineTotal(125, { ...inclusive, subtotal: 375, discountAmount: 37.5 })).toBeCloseTo(
+      90
+    )
+  })
+
+  it('adds up across the lines to the subtotal less the discount', () => {
+    const record = { ...exclusive, subtotal: 300, discountAmount: 45 }
+    const earned = [100, 120, 80].reduce((sum, line) => sum + earnedLineTotal(line, record), 0)
+    expect(earned).toBeCloseTo(255)
+  })
+
+  it('never goes below zero when the stored discount exceeds the subtotal', () => {
+    expect(earnedLineTotal(100, { ...exclusive, subtotal: 100, discountAmount: 150 })).toBe(0)
+  })
+
+  it('leaves the line alone when the record carries no subtotal or discount', () => {
+    expect(earnedLineTotal(100, exclusive)).toBe(100)
+    expect(earnedLineTotal(100, { ...exclusive, subtotal: 0, discountAmount: 20 })).toBe(100)
+    expect(earnedLineTotal(100, { ...exclusive, subtotal: null, discountAmount: null })).toBe(100)
+  })
+})

+ 5 - 1
src/app/api/protected/backup/import-invoice-ninja/route.ts

@@ -407,6 +407,7 @@ export async function POST(request: NextRequest) {
 
           const partsTotal = partItems.reduce((sum, li) => sum + (li.line_total || 0), 0)
           const laborTotal = laborItems.reduce((sum, li) => sum + (li.line_total || 0), 0)
+          const serviceDate = toSafeWorkshopDate(invoice.date, timeZone) ?? new Date()
 
           const record = await tx.serviceRecord.create({
             data: {
@@ -417,7 +418,10 @@ export async function POST(request: NextRequest) {
               type: 'repair',
               status: 'completed',
               cost: amount,
-              serviceDate: toSafeWorkshopDate(invoice.date, timeZone) ?? new Date(),
+              serviceDate,
+              // Reports and the calendar find a job by when it started; a
+              // job without a start is in none of them.
+              startDateTime: serviceDate,
               invoiceNumber,
               subtotal: partsTotal + laborTotal,
               discountType:

+ 5 - 1
src/app/api/protected/backup/import-lubelog/route.ts

@@ -383,6 +383,7 @@ export async function POST(request: NextRequest) {
         nextInvoiceNum++
 
         const cost = parseDecimal(sr.Cost)
+        const serviceDate = toSafeWorkshopDate(sr.Date, timeZone) ?? new Date()
 
         const created = await tx.serviceRecord.create({
           data: {
@@ -393,7 +394,10 @@ export async function POST(request: NextRequest) {
             status: 'completed',
             cost,
             mileage: sr.Mileage || null,
-            serviceDate: toSafeWorkshopDate(sr.Date, timeZone) ?? new Date(),
+            serviceDate,
+            // Reports and the calendar find a job by when it started; a job
+            // without a start is in none of them.
+            startDateTime: serviceDate,
             invoiceNumber,
             subtotal: cost,
             totalAmount: cost,

+ 5 - 1
src/features/import/Actions/importActions.ts

@@ -448,6 +448,7 @@ async function writeRow(tx: Tx, state: CommitState, row: RowPlan): Promise<void>
     if (!vehicleId || !s.date || !s.title)
       throw new Error('Service row is missing its vehicle, date or title')
     const total = s.total ?? 0
+    const serviceDate = new Date(s.date)
     await tx.serviceRecord.create({
       data: {
         title: s.title,
@@ -458,7 +459,10 @@ async function writeRow(tx: Tx, state: CommitState, row: RowPlan): Promise<void>
         subtotal: total,
         totalAmount: total,
         mileage: s.mileage,
-        serviceDate: new Date(s.date),
+        serviceDate,
+        // Reports and the calendar find a job by when it started; a job
+        // without a start is in none of them.
+        startDateTime: serviceDate,
         invoiceNumber: s.invoiceNumber,
         invoiceDate: s.invoiceNumber ? new Date(s.date) : null,
         diagnosticNotes: s.notes,

+ 1 - 0
src/features/onboarding/Lib/sampleData.ts

@@ -162,6 +162,7 @@ export async function seedSampleData(
         status: 'completed',
         mileage: 86200,
         serviceDate: daysAgo(12),
+        startDateTime: daysAgo(12),
         invoiceDate: daysAgo(12),
         invoiceNumber: `${invoicePrefix}1001`,
         subtotal: oilSubtotal,

+ 15 - 15
src/features/reports/Actions/reportActions.ts

@@ -4,7 +4,7 @@ import { db } from '@/lib/db'
 import { listOrgEntries } from '@/features/time-tracking/Lib/timeEntries'
 import { withAuth } from '@/lib/with-auth'
 import { PermissionAction, PermissionSubject } from '@/lib/permissions'
-import { netLineTotal } from '@/lib/tax'
+import { earnedLineTotal, netLineTotal } from '@/lib/tax'
 import { TaxByRateTable } from '../Lib/taxByRate'
 import { zonedDate, zonedDayKey, zonedParts } from '@/lib/timezone'
 import { workshopDayRange, workshopMonthKey } from '@/lib/workshop-datetime'
@@ -68,6 +68,8 @@ export async function getRevenueReport(params: ReportRange) {
           type: true,
           taxRate: true,
           taxInclusive: true,
+          subtotal: true,
+          discountAmount: true,
           manuallyPaid: true,
           payments: { select: { amount: true } },
           partItems: { select: { unitCost: true, quantity: true, total: true } },
@@ -78,8 +80,9 @@ export async function getRevenueReport(params: ReportRange) {
 
       // Monthly breakdown.
       // `revenue` is the gross customer-facing total (what was charged).
-      // `partsRevenue` and `laborRevenue` are NET (pre-tax) so they add up
-      // consistently with `partsCost` (which is always net) for profit math.
+      // `partsRevenue` and `laborRevenue` are NET (pre-tax) and carry their
+      // share of the job's discount, so they add up consistently with
+      // `partsCost` (which is always net) for profit math.
       const monthly: Record<
         string,
         {
@@ -105,14 +108,8 @@ export async function getRevenueReport(params: ReportRange) {
         const total = r.totalAmount > 0 ? r.totalAmount : r.cost
         const paid = r.manuallyPaid ? total : r.payments.reduce((s, p) => s + p.amount, 0)
         const partsCost = r.partItems.reduce((s, p) => s + p.unitCost * p.quantity, 0)
-        const partsRevenue = r.partItems.reduce(
-          (s, p) => s + netLineTotal(p.total, r.taxRate, r.taxInclusive),
-          0
-        )
-        const laborRevenue = r.laborItems.reduce(
-          (s, l) => s + netLineTotal(l.total, r.taxRate, r.taxInclusive),
-          0
-        )
+        const partsRevenue = r.partItems.reduce((s, p) => s + earnedLineTotal(p.total, r), 0)
+        const laborRevenue = r.laborItems.reduce((s, l) => s + earnedLineTotal(l.total, r), 0)
         const _date = r.startDateTime ?? r.serviceDate
         const month = workshopMonthKey(_date, tz)
 
@@ -431,15 +428,18 @@ export async function getPartsUsageReport(params: ReportRange) {
           quantity: true,
           total: true,
           unitCost: true,
-          serviceRecord: { select: { taxRate: true, taxInclusive: true } },
+          serviceRecord: {
+            select: { taxRate: true, taxInclusive: true, subtotal: true, discountAmount: true },
+          },
         },
       })
 
-      // Use net (pre-tax) line totals so revenue/profit math is consistent
-      // across inclusive- and exclusive-tax records.
+      // Use net (pre-tax) line totals, less each line's share of the job's
+      // discount, so revenue/profit math is consistent across inclusive- and
+      // exclusive-tax records and agrees with the revenue report.
       const partWithNet = parts.map((p) => ({
         ...p,
-        netTotal: netLineTotal(p.total, p.serviceRecord.taxRate, p.serviceRecord.taxInclusive),
+        netTotal: earnedLineTotal(p.total, p.serviceRecord),
       }))
 
       const byPart: Record<

+ 6 - 2
src/features/vehicles/Actions/serviceActions.ts

@@ -488,7 +488,8 @@ export async function createServiceRecord(input: unknown) {
         : data.vehicleId
           ? warrantyFieldsForNewDocument(readWarrantyDefaults(settingsMap), 'workOrder')
           : EMPTY_WARRANTY
-      const warrantyStartsOn = toSafeWorkshopDate(serviceDate, timeZone) ?? new Date()
+      const startsAt = toSafeWorkshopDate(serviceDate, timeZone) ?? new Date()
+      const warrantyStartsOn = startsAt
 
       const record = await db.$transaction(async (tx) => {
         const created = await tx.serviceRecord.create({
@@ -509,7 +510,10 @@ export async function createServiceRecord(input: unknown) {
               : {}),
             shopName,
             invoiceNumber,
-            serviceDate: toSafeWorkshopDate(serviceDate, timeZone) ?? new Date(),
+            serviceDate: startsAt,
+            // Reports and the calendar find a job by when it started; a job
+            // without a start is in none of them.
+            startDateTime: startsAt,
             invoiceDate:
               toSafeWorkshopDate(invoiceDate, timeZone) ??
               toSafeWorkshopDate(serviceDate, timeZone) ??

+ 10 - 12
src/lib/cron/report-schedules.ts

@@ -7,7 +7,7 @@ import React from 'react'
 import { sendOrgMail, getOrgFromAddress } from '@/lib/email'
 import { ReportPDF } from '@/features/reports/Components/ReportPDF'
 import { SETTING_KEYS } from '@/features/settings/Schema/settingsSchema'
-import { netLineTotal } from '@/lib/tax'
+import { earnedLineTotal } from '@/lib/tax'
 import { TaxByRateTable } from '@/features/reports/Lib/taxByRate'
 import {
   addZonedDays,
@@ -124,6 +124,8 @@ async function fetchRevenue(orgId: string, start: Date, end: Date, timeZone: str
       type: true,
       taxRate: true,
       taxInclusive: true,
+      subtotal: true,
+      discountAmount: true,
       manuallyPaid: true,
       payments: { select: { amount: true } },
       partItems: { select: { unitCost: true, quantity: true, total: true } },
@@ -155,15 +157,9 @@ async function fetchRevenue(orgId: string, start: Date, end: Date, timeZone: str
     const total = r.totalAmount > 0 ? r.totalAmount : r.cost
     const paid = r.manuallyPaid ? total : r.payments.reduce((s, p) => s + p.amount, 0)
     const partsCost = r.partItems.reduce((s, p) => s + p.unitCost * p.quantity, 0)
-    // Net (pre-tax) parts/labor so profit math works in both tax modes.
-    const partsRevenue = r.partItems.reduce(
-      (s, p) => s + netLineTotal(p.total, r.taxRate, r.taxInclusive),
-      0
-    )
-    const laborRevenue = r.laborItems.reduce(
-      (s, l) => s + netLineTotal(l.total, r.taxRate, r.taxInclusive),
-      0
-    )
+    // Net (pre-tax) parts/labor, after the job's discount, so profit math works in both tax modes.
+    const partsRevenue = r.partItems.reduce((s, p) => s + earnedLineTotal(p.total, r), 0)
+    const laborRevenue = r.laborItems.reduce((s, l) => s + earnedLineTotal(l.total, r), 0)
     const _d = r.startDateTime ?? r.serviceDate
     const month = workshopMonthKey(_d, timeZone)
 
@@ -329,12 +325,14 @@ async function fetchParts(orgId: string, start: Date, end: Date) {
       quantity: true,
       total: true,
       unitCost: true,
-      serviceRecord: { select: { taxRate: true, taxInclusive: true } },
+      serviceRecord: {
+        select: { taxRate: true, taxInclusive: true, subtotal: true, discountAmount: true },
+      },
     },
   })
   const partWithNet = parts.map((p) => ({
     ...p,
-    netTotal: netLineTotal(p.total, p.serviceRecord.taxRate, p.serviceRecord.taxInclusive),
+    netTotal: earnedLineTotal(p.total, p.serviceRecord),
   }))
   const byPart: Record<
     string,

+ 28 - 0
src/lib/tax.ts

@@ -215,3 +215,31 @@ export function netLineTotal(lineTotal: number, taxRate: number, taxInclusive: b
   if (!taxInclusive || taxRate <= 0) return lineTotal
   return lineTotal / (1 + taxRate / 100)
 }
+
+/**
+ * What a line earned before tax, once the job's discount is counted.
+ *
+ * The discount comes off the subtotal as a whole rather than off any one
+ * line, so each line carries it in proportion to its size. Reports that add
+ * lines up (parts revenue, labor revenue, profit) ask here, or a discounted
+ * job reads as having earned what it would have without the discount.
+ *
+ * `subtotal` and `discountAmount` are the stored figures the document was
+ * totalled from, in the same terms as the line (gross in inclusive mode), so
+ * the share is the same whichever way the prices were entered.
+ */
+export function earnedLineTotal(
+  lineTotal: number,
+  record: {
+    taxRate: number
+    taxInclusive: boolean
+    subtotal?: number | null
+    discountAmount?: number | null
+  }
+): number {
+  const net = netLineTotal(lineTotal, record.taxRate, record.taxInclusive)
+  const subtotal = Number(record.subtotal) || 0
+  const discount = Number(record.discountAmount) || 0
+  if (subtotal <= 0 || discount <= 0) return net
+  return net * (Math.max(0, subtotal - discount) / subtotal)
+}