feat(orders): log the client price figures the server discards (MON-08)
Step 2 of the owner's MON-08 ruling. An app order is priced by the server (steps 1, 3, 4 landed with the wage-formula work), but the client's own wagePercent, discountPercent and flatRialCharge were being thrown away in silence — so a stale app build, a promotion that expired between the product page and checkout, and a deliberately tampered request were indistinguishable and left no trace anywhere. They are now logged with both numbers, the same pattern already used for the body's customerId. The pricing itself is untouched: the server's figures are what the order is charged, and an operator ordering from the panel still keeps the numbers they typed. Also adds the comparison test the ruling asked for, which step 3 never actually had — the suite only proved a tampered request gets overridden. The new test places one honest order as a customer and the same order as an operator and asserts the totals are identical, which is the real claim: an ordinary customer's total does not move. Two more cover the logging, including that an honest order logs nothing — so the quiet case cannot pass merely because the method never ran. Closes STUB-05 in the docs as well: the reference's six hardcoded staff numbers have been behind PROFILE_ADMIN_MOBILES / ORDER_ADMIN_MOBILES for a while, but its status still read "pending". api: 286/286 tests, 26 suites, typecheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
83e5d2868b
commit
fd9191eca8
@@ -1,3 +1,4 @@
|
||||
import { Logger } from '@nestjs/common';
|
||||
import { Prisma } from '@prisma/client';
|
||||
import { OrdersService } from './orders.service';
|
||||
|
||||
@@ -122,6 +123,48 @@ describe('OrdersService.createOrder — server-side making charge', () => {
|
||||
expect(created[0].totalGoldWeightGrams.toString()).toBe('5');
|
||||
expect(created[0].flatRialCharge.toString()).toBe('999');
|
||||
});
|
||||
|
||||
/**
|
||||
* MON-08 step 3 (owner ruling): «تست مقایسهای که نشان دهد نتیجه برای کاربر
|
||||
* عادی یکسان است». An unmodified app sends what the server derives
|
||||
* (DECISIONS §"Report — MON-08"), so deriving server-side must not move the
|
||||
* total for an ordinary customer.
|
||||
*/
|
||||
it('gives an honest app exactly the total it sent', async () => {
|
||||
const honest = { items: [{ productVariantId: 'v1', quantity: 1, wagePercent: 10, discountPercent: 0, flatRialCharge: 0 }] } as never;
|
||||
const a = build();
|
||||
await a.service.createOrder(honest, 'c1', { placedBy: 'customer' });
|
||||
const b = build();
|
||||
await b.service.createOrder(honest, 'c1', { placedBy: 'operator' });
|
||||
expect(a.created[0].totalGoldWeightGrams.toString()).toBe(b.created[0].totalGoldWeightGrams.toString());
|
||||
expect(a.created[0].flatRialCharge.toString()).toBe(b.created[0].flatRialCharge.toString());
|
||||
});
|
||||
|
||||
/** MON-08 step 2: a discarded client figure must leave a trace. */
|
||||
it('logs what the client sent when it differs from the server', async () => {
|
||||
const warn = jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);
|
||||
try {
|
||||
await build().service.createOrder(tampered, 'c1', { placedBy: 'customer' });
|
||||
const lines = warn.mock.calls.map((c) => String(c[0])).join('\n');
|
||||
expect(lines).toContain('wagePercent sent=0 used=10');
|
||||
expect(lines).toContain('discountPercent sent=50 used=0');
|
||||
expect(lines).toContain('flatRialCharge sent=999 used=0');
|
||||
} finally {
|
||||
warn.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('stays quiet when the app sent the right figures', async () => {
|
||||
const warn = jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);
|
||||
try {
|
||||
const honest = { items: [{ productVariantId: 'v1', quantity: 1, wagePercent: 10 }] } as never;
|
||||
await build().service.createOrder(honest, 'c1', { placedBy: 'customer' });
|
||||
const pricing = warn.mock.calls.filter((c) => String(c[0]).includes('Order pricing'));
|
||||
expect(pricing).toHaveLength(0);
|
||||
} finally {
|
||||
warn.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
|
||||
@@ -4,7 +4,7 @@ import { PrismaService } from '../../prisma/prisma.service';
|
||||
import { CatalogService } from '../catalog/catalog.service';
|
||||
import { LedgerService } from '../accounting/ledger.service';
|
||||
import { AccountingHooksService } from '../accounting-sync/accounting-hooks.service';
|
||||
import { CreateOrderDto } from './dto/create-order.dto';
|
||||
import { CreateOrderDto, CreateOrderItemInputDto } from './dto/create-order.dto';
|
||||
import { WageService } from '../pricing/wage.service';
|
||||
import { SmsService } from '../sms/sms.service';
|
||||
|
||||
@@ -88,6 +88,7 @@ export class OrdersService {
|
||||
if (quotes && !quote) throw new NotFoundException(`Variant ${variant.id} not found`);
|
||||
const wagePercent = new Prisma.Decimal(quote ? quote.wagePercentSnapshot : item.wagePercent);
|
||||
const discountPercent = new Prisma.Decimal(quote ? quote.discountPercentSnapshot : (item.discountPercent ?? 0));
|
||||
if (quote) this.warnOnPriceMismatch(variant.id, item, wagePercent, discountPercent);
|
||||
const netPercent = wagePercent.sub(discountPercent);
|
||||
const computedWeightGrams = rashWeight.mul(netPercent.div(100).add(1));
|
||||
|
||||
@@ -173,6 +174,41 @@ export class OrdersService {
|
||||
return placed;
|
||||
}
|
||||
|
||||
/**
|
||||
* MON-08 step 2: the server's own figures are what the order is charged, but
|
||||
* a client that sent something else is worth seeing. An ordinary app sends
|
||||
* exactly what the server derives (DECISIONS §"Report — MON-08"), so a
|
||||
* mismatch means a stale app version, a promotion that expired between the
|
||||
* product page and checkout, or a tampered request.
|
||||
*
|
||||
* `flatRialCharge` is included because the app hardcodes 0 (cart.vue:179) and
|
||||
* only an operator ever sets one — anything non-zero from a customer is a
|
||||
* client that should not be sending it.
|
||||
*/
|
||||
private warnOnPriceMismatch(
|
||||
variantId: string,
|
||||
sent: CreateOrderItemInputDto,
|
||||
wagePercent: Prisma.Decimal,
|
||||
discountPercent: Prisma.Decimal,
|
||||
) {
|
||||
const differs: string[] = [];
|
||||
if (!wagePercent.eq(sent.wagePercent)) {
|
||||
differs.push(`wagePercent sent=${sent.wagePercent} used=${wagePercent.toString()}`);
|
||||
}
|
||||
if (!discountPercent.eq(sent.discountPercent ?? 0)) {
|
||||
differs.push(`discountPercent sent=${sent.discountPercent ?? 0} used=${discountPercent.toString()}`);
|
||||
}
|
||||
if ((sent.flatRialCharge ?? 0) !== 0) {
|
||||
differs.push(`flatRialCharge sent=${sent.flatRialCharge} used=0`);
|
||||
}
|
||||
if (differs.length) {
|
||||
this.logger.warn(
|
||||
`Order pricing for variant ${variantId}: the server's figures are used, ` +
|
||||
`the client's are ignored — ${differs.join('; ')}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The admins who are told a new order was placed.
|
||||
*
|
||||
|
||||
Reference in New Issue
Block a user