diff --git a/docs/integration-guide.md b/docs/integration-guide.md index 7543e2b..dc5a0b2 100644 --- a/docs/integration-guide.md +++ b/docs/integration-guide.md @@ -49,7 +49,7 @@ mqtt { authorization { users = [ { user: "till", password: "…", permissions: { - publish: ["pos.*.*.order", "pos.*.*.status"] + publish: ["pos.*.*.order", "pos.*.*.customer", "pos.*.*.status"] subscribe: ["pos.*.*.ack", "pos.*.*.command", "pos.*.catalogue"] }} ] @@ -193,6 +193,38 @@ for msg in subscribe("pos.*.*.order"): `ON CONFLICT DO NOTHING` still counts as accepted — a redelivery of a bill you already hold is a success, not a rejection. +### Shopper registrations + +A second uplink runs on `pos/{store}/{terminal}/customer`, acked on the same +topic by the same rules. Wire it the same way: + +```sql +CREATE TABLE customers ( + id UUID PRIMARY KEY, -- derived from the mobile number + mobile TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, + email TEXT, + gender TEXT, + date_of_birth DATE, + registered_at TIMESTAMPTZ, + registered_by_terminal TEXT +); +``` + +Insert with `ON CONFLICT (id) DO NOTHING` — **never** an upsert. A registration +is replayed freely and must not overwrite a profile corrected at head office. + +The id is a UUIDv5 over the shopper's normalised ten-digit mobile, so two tills +registering the same person independently produce the same row. Don't reassign +it. And don't expect loyalty points in this payload: derive those from the bill +stream, which is idempotent and knows about every counter. + +```bash +nats sub 'pos.*.*.customer' +``` + +Add a shopper on the terminal — no sale needed — and it should appear. + **Use `rejected` sparingly.** Naming an id there halts the terminal's drain: it stops retrying and waits for a person to press Sync. That is right for "this bill is malformed" and wrong for "my database is having a bad minute" — for the @@ -264,7 +296,9 @@ and per-bill state — start there before the broker logs. ## Before a fleet rollout - **Back up `nearle_pos.db` on any terminal already trading.** The schema goes to - v7 on first launch and the migration is one-way. + v8 on first launch and the migration is one-way. It also queues every shopper + already on the terminal for upload, so expect one burst of registrations from + each existing store — collapse those onto the mobile number. - **Build per-ABI.** `flutter build apk --split-per-abi` gives ~23MB per architecture instead of a 69MB universal APK — worth it over shop wifi. - **Change the seed PINs.** `4821` / `5093` / `6274` are in the source. Every diff --git a/docs/sync-contract.md b/docs/sync-contract.md index e2dd3bc..e33b210 100644 --- a/docs/sync-contract.md +++ b/docs/sync-contract.md @@ -63,6 +63,7 @@ connection or return 5xx instead, and the terminal will back off and retry. | Topic | Direction | QoS | Retained | |---|---|---|---| | `pos/{store}/{terminal}/order` | till → cloud | 1 | no | +| `pos/{store}/{terminal}/customer` | till → cloud | 1 | no | | `pos/{store}/{terminal}/ack` | cloud → till | 1 | no | | `pos/{store}/{terminal}/status` | till → cloud | 1 | **yes** | | `pos/{store}/{terminal}/command` | cloud → till | 1 | no | @@ -157,6 +158,58 @@ nothing is marked synced, and the batch goes again. response. Carries an `idempotency-key` header that is stable across retries of the same bills. +### Shopper registrations + +**Uplink** — `pos/{store}/{terminal}/customer`, or `POST {base}/customers`. +Acked on the same topic and by the same rules: only ids you name are marked +sent. + +```json +{ + "schema": 1, + "batch_id": "3d7a…", + "store_id": "store-01", + "terminal_id": "T4A9", + "customers": [ + { + "id": "7a24e082-060f-5503-aa9e-da0ef42df047", + "mobile": "9840012345", + "name": "Meena", + "email": null, + "gender": "female", + "date_of_birth": null, + "registered_at": "2026-08-01T10:14:00.000Z", + "registered_by_terminal": "T4A9" + } + ] +} +``` + +Three things about this payload are load-bearing: + +- **`id` is derived from the mobile number**, not minted at random — a UUIDv5 + over the normalised ten-digit number in a fixed namespace. Two tills that + register the same shopper independently produce *the same id*, so you + deduplicate on a primary key rather than guessing at a merge later. Do not + reassign it. +- **Treat it as insert-if-absent on `id`.** A registration is not a financial + record: it is replayed freely, and it must never overwrite a profile + corrected at head office. `ON CONFLICT (id) DO NOTHING`. +- **No loyalty figures are sent.** Points, lifetime spend and visit counts are + absent on purpose — derive them from the bill stream, which is authoritative + and idempotent. Accepting a terminal's local balance would make the last till + to sync win, and a shopper who bought at two counters on the same day would + end up with whichever figure happened to arrive second. + +Registrations are uploaded *before* bills on every pass, so a bill naming a new +shopper arrives after the shopper does. A failure here is logged and does not +hold up the bills behind it. + +Terminals that were trading before schema v8 carry shoppers with random ids +from the old scheme. Those are queued once by the migration and arrive with +their original ids — merge them onto the mobile number. It is a one-off for +existing stores; a new terminal never produces one. + ## Catalogue pull The other direction: products and customers coming down. @@ -237,15 +290,15 @@ overstate the day. - **Downlink beyond catalogue-changed and sync-requested.** The plumbing routes unknown commands to the events log rather than dropping them, so adding one is a server change plus a case arm. -- **Credentials survive a restart.** The back-office dialog writes the broker - host, port, TLS flag and credentials into `syncConfigProvider`, which is - in-memory. Terminal name and store id persist (they live in the database); - the credentials do not, and must be re-entered after a restart. Persisting - them means encrypting them at rest, which is the next piece of work. - **Historical correction.** Bills already synced by an older build went up with an overstated total. Nothing here fixes that; it needs a server-side reconciliation against `bill_discount`. -- **Pushing customers upward.** A shopper registered at the till stays on that - terminal and rides along on the bills they appear on. There is no - customer-create endpoint yet, so two terminals registering the same mobile - number will each hold their own row until the back office reconciles them. +- **Loyalty balances coming back down.** Points and lifetime spend are computed + per terminal from the bills that terminal rang. A shopper who buys at two + stores has two partial balances until the back office derives the real one + from the bill stream and sends it down in a catalogue pull. The uplink + deliberately does not carry local balances, so nothing is corrupted by this — + but a shopper's points at the till are that till's view, not the group's. +- **Merging pre-v8 shoppers.** Terminals that traded before the customer outbox + carry rows with random ids. They are uploaded once by the migration, but + collapsing them onto the mobile number is the back office's job. diff --git a/lib/core/config/sync_config.dart b/lib/core/config/sync_config.dart index e1f69d8..e3dc676 100644 --- a/lib/core/config/sync_config.dart +++ b/lib/core/config/sync_config.dart @@ -78,8 +78,20 @@ class SyncConfig { /// Uplink. Completed bills, QoS 1. String get orderTopic => '$_base/order'; + /// Uplink. Shoppers registered at this till, QoS 1. + /// + /// Separate from [orderTopic] because the two have different shapes and + /// different consumers: bills are financial records that must never be + /// replayed twice, registrations are insert-if-absent and can be replayed + /// freely. Sharing a topic would force one consumer to branch on a type tag + /// and would put a registration behind a stuck bill. + String get customerTopic => '$_base/customer'; + /// The back office's answer, naming the ids it committed. Subscribed at /// QoS 1: losing an ack means re-sending bills that are already banked. + /// + /// Carries acks for both uplinks. The `batch_id` says which send is being + /// answered, so one subscription is enough. String get ackTopic => '$_base/ack'; /// Retained, and set as the will message. A terminal that loses power stops diff --git a/lib/data/datasources/local_store.dart b/lib/data/datasources/local_store.dart index 99572bb..1828894 100644 --- a/lib/data/datasources/local_store.dart +++ b/lib/data/datasources/local_store.dart @@ -39,6 +39,7 @@ class LocalStore { DateTime? _lastImportAt; String? _catalogueRevision; int _unsyncedOrders = 0; + int _unsyncedCustomers = 0; bool _ready = false; @@ -92,6 +93,7 @@ class LocalStore { _catalogueRevision = await catalogue.meta(MetaKeys.catalogueRevision); terminal = await identityStore.load(); _unsyncedOrders = await orders.unsyncedCount(); + _unsyncedCustomers = await catalogue.unsyncedCustomerCount(); _syncEvents ..clear() @@ -239,11 +241,20 @@ class LocalStore { Future putCustomer(Customer c) async { await catalogue.upsertCustomer(c); _customers[c.id] = c; + await refreshUnsyncedCustomerCount(); } /// Mirrors a customer already written to disk into the memory cache. void cacheCustomer(Customer c) => _customers[c.id] = c; + /// Shoppers registered here and not yet uploaded. + int get unsyncedCustomers => _unsyncedCustomers; + + Future refreshUnsyncedCustomerCount() async { + _unsyncedCustomers = await catalogue.unsyncedCustomerCount(); + return _unsyncedCustomers; + } + // ----------------------------------------------------------------- Orders /// Refreshes the cached unsynced tally after a write or a sync. Future refreshUnsyncedCount() async { diff --git a/lib/data/local/app_database.dart b/lib/data/local/app_database.dart index e86d035..3f11a8d 100644 --- a/lib/data/local/app_database.dart +++ b/lib/data/local/app_database.dart @@ -13,7 +13,7 @@ class AppDatabase { static final AppDatabase instance = AppDatabase._(); static const String _fileName = 'nearle_pos.db'; - static const int _version = 7; + static const int _version = 8; Database? _db; @@ -77,6 +77,7 @@ class AppDatabase { 'ALTER TABLE ${Tables.orders} ADD COLUMN promos_json TEXT', ); } + if (from < 8) await _upgradeToV8(db); }, ), ); @@ -188,12 +189,24 @@ class AppDatabase { lifetime_spend REAL NOT NULL DEFAULT 0, visit_count INTEGER NOT NULL DEFAULT 0, created_at INTEGER, - last_visit_at INTEGER + last_visit_at INTEGER, + + -- Registration outbox. A shopper signed up at the till has to reach + -- the back office even if they never buy anything, so this table + -- carries the same pending/synced flag the orders table does. + -- + -- Defaults to 1: rows that arrived in a catalogue pull came *from* + -- the back office and must not be posted straight back. + sync_status INTEGER NOT NULL DEFAULT 1, + synced_at INTEGER ) '''); await db.execute( 'CREATE UNIQUE INDEX idx_customers_mobile ON ${Tables.customers}(mobile)', ); + await db.execute( + 'CREATE INDEX idx_customers_sync ON ${Tables.customers}(sync_status)', + ); // -------------------------------------------------------------- orders await db.execute(''' @@ -322,6 +335,43 @@ Future _upgradeToV4(Database db, {required int from}) async { await db.execute('DROP TABLE _day_archive_v3'); } +/// Moves the schema to v8 — customers become an outbox. +/// +/// Before this, a shopper registered at the till only ever reached the back +/// office as three fields riding along on a bill. Someone who signed up and +/// then didn't buy anything, or whose bill was still queued, existed on one +/// terminal and nowhere else. +/// +/// Every existing row is marked pending rather than synced. The terminal +/// cannot tell which of them came down in a catalogue pull and which were rung +/// up locally, and of the two possible mistakes only one loses a shopper. This +/// is safe precisely because the customer uplink is specified as +/// insert-if-absent on id — re-sending one the back office already holds is a +/// no-op, never an overwrite of a profile edited at head office. +/// +/// Ids are deliberately *not* rewritten. New customers are keyed on their +/// mobile number (see `Customer.idForMobile`) so terminals agree without +/// coordinating, but rows created before this version carry random ids that +/// bills already in the back office refer to. Re-keying them here would break +/// that link. They stay as they are, and the back office merges them on +/// mobile — a one-off for stores that were already trading. +Future _upgradeToV8(Database db) async { + await db.execute( + 'ALTER TABLE ${Tables.customers} ' + 'ADD COLUMN sync_status INTEGER NOT NULL DEFAULT 1', + ); + await db.execute( + 'ALTER TABLE ${Tables.customers} ADD COLUMN synced_at INTEGER', + ); + await db.execute( + 'CREATE INDEX idx_customers_sync ON ${Tables.customers}(sync_status)', + ); + + await db.rawUpdate( + 'UPDATE ${Tables.customers} SET sync_status = 0, synced_at = NULL', + ); +} + const String _createSyncLog = ''' CREATE TABLE sync_log ( id TEXT PRIMARY KEY, diff --git a/lib/data/local/catalogue_dao.dart b/lib/data/local/catalogue_dao.dart index ed131d1..c374696 100644 --- a/lib/data/local/catalogue_dao.dart +++ b/lib/data/local/catalogue_dao.dart @@ -164,7 +164,7 @@ class CatalogueDao { for (final c in customers) { batch.insert( Tables.customers, - customerToRow(c), + _importedCustomerRow(c), conflictAlgorithm: ConflictAlgorithm.ignore, ); } @@ -173,6 +173,18 @@ class CatalogueDao { }); } + /// A customer row that arrived from the back office, marked as already sent. + /// + /// Stated outright rather than left to the column default, because the + /// default existing to protect imports is not obvious from this call site — + /// and getting it wrong would post the whole customer book straight back to + /// the server that just sent it. + static Map _importedCustomerRow(Customer c) => { + ...customerToRow(c), + 'sync_status': syncedCustomer, + 'synced_at': DateTime.now().millisecondsSinceEpoch, + }; + /// Applies a change set, leaving everything it does not mention alone. /// /// The counterpart to [replaceCatalogue], and the difference matters: a full @@ -218,7 +230,7 @@ class CatalogueDao { for (final c in customers) { batch.insert( Tables.customers, - customerToRow(c), + _importedCustomerRow(c), conflictAlgorithm: ConflictAlgorithm.ignore, ); } @@ -254,11 +266,10 @@ class CatalogueDao { } Future customerByMobile(String mobile) async { - final digits = mobile.replaceAll(RegExp(r'\D'), ''); final rows = await _db.query( Tables.customers, where: 'mobile = ?', - whereArgs: [digits], + whereArgs: [Customer.normaliseMobile(mobile)], limit: 1, ); return rows.isEmpty ? null : customerFromRow(rows.first); @@ -274,14 +285,59 @@ class CatalogueDao { return rows.isEmpty ? null : customerFromRow(rows.first); } + /// Writes a customer created or edited at the till, and queues them. + /// + /// Everything registered on this terminal has to reach the back office in its + /// own right — a shopper who signs up for the loyalty scheme and then buys + /// nothing used to exist here and nowhere else. Future upsertCustomer(Customer c) async { await _db.insert( Tables.customers, - customerToRow(c), + {...customerToRow(c), 'sync_status': pendingCustomer, 'synced_at': null}, conflictAlgorithm: ConflictAlgorithm.replace, ); } + // ---------------------------------------------------- Customer outbox + static const int pendingCustomer = 0; + static const int syncedCustomer = 1; + + Future unsyncedCustomerCount() async { + final rows = await _db.rawQuery( + 'SELECT COUNT(*) AS n FROM ${Tables.customers} WHERE sync_status = ?', + [pendingCustomer], + ); + return (rows.first['n']! as num).toInt(); + } + + /// A bounded page of shoppers waiting to go up, oldest first. + Future> unsyncedCustomers({int limit = 100}) async { + final rows = await _db.query( + Tables.customers, + where: 'sync_status = ?', + whereArgs: [pendingCustomer], + orderBy: 'created_at ASC', + limit: limit, + ); + return rows.map(customerFromRow).toList(); + } + + /// Flips only the ids the back office named. Anything it stayed silent about + /// is left pending — the same rule the orders outbox follows. + Future markCustomersSynced(List ids, {DateTime? at}) async { + if (ids.isEmpty) return; + final marks = List.filled(ids.length, '?').join(','); + await _db.rawUpdate( + 'UPDATE ${Tables.customers} SET sync_status = ?, synced_at = ? ' + 'WHERE id IN ($marks)', + [ + syncedCustomer, + (at ?? DateTime.now()).millisecondsSinceEpoch, + ...ids, + ], + ); + } + // ------------------------------------------------------------------ Meta Future meta(String key) async { final rows = await _db.query( diff --git a/lib/data/local/order_dao.dart b/lib/data/local/order_dao.dart index e9e761c..1d83e38 100644 --- a/lib/data/local/order_dao.dart +++ b/lib/data/local/order_dao.dart @@ -61,10 +61,24 @@ class OrderDao { ); }); if (customerRow != null) { - batch.insert( + // A targeted UPDATE, not an upsert. `ConflictAlgorithm.replace` is a + // DELETE followed by an INSERT, so every column absent from the row + // silently reverts to its schema default — which would reset + // `sync_status` to 1 and strand a shopper who had never been uploaded. + // + // Restricting it to the four figures a sale actually moves also stops + // a bill overwriting a name or number corrected at head office between + // the shopper being added to the cart and the cashier taking payment. + batch.update( Tables.customers, - customerRow, - conflictAlgorithm: ConflictAlgorithm.replace, + { + 'loyalty_points': customerRow['loyalty_points'], + 'lifetime_spend': customerRow['lifetime_spend'], + 'visit_count': customerRow['visit_count'], + 'last_visit_at': customerRow['last_visit_at'], + }, + where: 'id = ?', + whereArgs: [customerRow['id']], ); } await batch.commit(noResult: true); diff --git a/lib/data/remote/http_order_transport.dart b/lib/data/remote/http_order_transport.dart index cbc1943..379cafe 100644 --- a/lib/data/remote/http_order_transport.dart +++ b/lib/data/remote/http_order_transport.dart @@ -50,8 +50,19 @@ class HttpOrderTransport implements OrderTransport { Future connect() async {} @override - Future pushOrders(List> orders) async { - if (orders.isEmpty) return const PushReceipt(accepted: []); + Future pushOrders(List> orders) => + _post(path: 'orders', key: 'orders', items: orders); + + @override + Future pushCustomers(List> customers) => + _post(path: 'customers', key: 'customers', items: customers); + + Future _post({ + required String path, + required String key, + required List> items, + }) async { + if (items.isEmpty) return const PushReceipt(accepted: []); if (config.httpBaseUrl.isEmpty) { throw const TransportException( @@ -60,7 +71,7 @@ class HttpOrderTransport implements OrderTransport { ); } - final uri = Uri.parse('${config.httpBaseUrl}/orders'); + final uri = Uri.parse('${config.httpBaseUrl}/$path'); http.Response response; try { @@ -73,14 +84,14 @@ class HttpOrderTransport implements OrderTransport { 'authorization': 'Bearer ${config.apiKey}', // Lets the endpoint collapse a retried batch server-side rather // than relying on every order id being checked individually. - 'idempotency-key': _batchKey(orders), + 'idempotency-key': _batchKey(items), }, body: jsonEncode({ 'schema': 1, 'store_id': config.storeId, 'terminal_id': config.terminalId, 'sent_at': DateTime.now().toIso8601String(), - 'orders': orders, + key: items, }), ) .timeout(config.ackTimeout); @@ -132,10 +143,10 @@ class HttpOrderTransport implements OrderTransport { return PushReceipt(accepted: accepted, rejected: rejected); } - /// Stable for a given set of bills, so a retry after a timeout carries the + /// Stable for a given set of records, so a retry after a timeout carries the /// same key as the attempt that may already have landed. - String _batchKey(List> orders) => - orders.map((o) => o['id']).join('|').hashCode.toRadixString(16); + String _batchKey(List> items) => + items.map((o) => o['id']).join('|').hashCode.toRadixString(16); void _setReachable(bool value) { if (_reachable == value) return; diff --git a/lib/data/remote/mqtt_order_transport.dart b/lib/data/remote/mqtt_order_transport.dart index 4be5f9d..f7ae934 100644 --- a/lib/data/remote/mqtt_order_transport.dart +++ b/lib/data/remote/mqtt_order_transport.dart @@ -170,8 +170,36 @@ class MqttOrderTransport implements OrderTransport { // ----------------------------------------------------------------- Uplink @override - Future pushOrders(List> orders) async { - if (orders.isEmpty) return const PushReceipt(accepted: []); + Future pushOrders(List> orders) => + _publishBatch( + topic: config.orderTopic, + key: 'orders', + items: orders, + noun: 'bills', + ); + + @override + Future pushCustomers(List> customers) => + _publishBatch( + topic: config.customerTopic, + key: 'customers', + items: customers, + noun: 'registrations', + ); + + /// Publishes one correlated batch and waits for the back office to answer it. + /// + /// Shared by both uplinks because the rule they must obey is the same one, + /// and it is the rule the whole design rests on: a batch counts as delivered + /// only when the *application* names its ids, never when the broker + /// acknowledges the bytes. + Future _publishBatch({ + required String topic, + required String key, + required List> items, + required String noun, + }) async { + if (items.isEmpty) return const PushReceipt(accepted: []); await connect(); @@ -181,14 +209,14 @@ class MqttOrderTransport implements OrderTransport { try { _publish( - config.orderTopic, + topic, jsonEncode({ 'schema': 1, 'batch_id': batchId, 'store_id': config.storeId, 'terminal_id': config.terminalId, 'sent_at': DateTime.now().toIso8601String(), - 'orders': orders, + key: items, }), ); @@ -196,7 +224,7 @@ class MqttOrderTransport implements OrderTransport { config.ackTimeout, onTimeout: () => throw TransportException( 'The back office did not confirm the batch within ' - '${config.ackTimeout.inSeconds}s. The bills are still on this ' + '${config.ackTimeout.inSeconds}s. The $noun are still on this ' 'terminal and will be sent again.', ), ); diff --git a/lib/data/remote/order_transport.dart b/lib/data/remote/order_transport.dart index 87a1566..47dbc63 100644 --- a/lib/data/remote/order_transport.dart +++ b/lib/data/remote/order_transport.dart @@ -86,6 +86,17 @@ abstract class OrderTransport { /// Throws [TransportException] when the outcome is unknown. Future pushOrders(List> orders); + /// Hands over shoppers registered at this till. + /// + /// Same acceptance contract as [pushOrders] — only ids the back office names + /// are marked sent — but the payload is a registration rather than a + /// financial record, so the back office is expected to treat it as + /// insert-if-absent on id. Replaying one it already holds must be a no-op, + /// never an overwrite of a profile corrected at head office. + /// + /// Throws [TransportException] when the outcome is unknown. + Future pushCustomers(List> customers); + /// Cloud-initiated messages. Empty for transports that cannot receive. Stream get downlink; diff --git a/lib/data/remote/simulated_order_transport.dart b/lib/data/remote/simulated_order_transport.dart index 612db40..1a95053 100644 --- a/lib/data/remote/simulated_order_transport.dart +++ b/lib/data/remote/simulated_order_transport.dart @@ -33,20 +33,30 @@ class SimulatedOrderTransport implements OrderTransport { Future connect() async {} @override - Future pushOrders(List> orders) async { + Future pushOrders(List> orders) => + _accept(orders, 'bill'); + + @override + Future pushCustomers(List> customers) => + _accept(customers, 'registration'); + + Future _accept( + List> items, + String noun, + ) async { await Future.delayed( - Duration(milliseconds: 400 + orders.length * 60), + Duration(milliseconds: 400 + items.length * 60), ); if (isOffline()) { - throw const TransportException( + throw TransportException( 'Simulate offline is ON in Settings, so the upload was failed on ' - 'purpose. Every bill is still stored on this terminal.', + 'purpose. Every $noun is still stored on this terminal.', ); } return PushReceipt( - accepted: orders.map((o) => o['id']! as String).toList(), + accepted: items.map((o) => o['id']! as String).toList(), ); } diff --git a/lib/data/repositories/customer_repository_impl.dart b/lib/data/repositories/customer_repository_impl.dart index 7e931f4..e43229e 100644 --- a/lib/data/repositories/customer_repository_impl.dart +++ b/lib/data/repositories/customer_repository_impl.dart @@ -1,5 +1,3 @@ -import 'package:uuid/uuid.dart'; - import '../../core/utils/extensions.dart'; import '../../domain/entities/customer.dart'; import '../../domain/repositories/customer_repository.dart'; @@ -9,15 +7,15 @@ class CustomerRepositoryImpl implements CustomerRepository { CustomerRepositoryImpl(this._store); final LocalStore _store; - static const _uuid = Uuid(); - - String _digits(String v) => v.replaceAll(RegExp(r'\D'), ''); @override Future findByMobile(String mobile) async { - final needle = _digits(mobile); - return _store.customers - .firstWhereOrNull((c) => _digits(c.mobile) == needle); + // Normalised on both sides, so a shopper stored from `9840012345` is still + // found when a cashier at the next till types `+91 98400 12345`. + final needle = Customer.normaliseMobile(mobile); + return _store.customers.firstWhereOrNull( + (c) => Customer.normaliseMobile(c.mobile) == needle, + ); } @override @@ -30,9 +28,14 @@ class CustomerRepositoryImpl implements CustomerRepository { throw StateError('A customer with this mobile number already exists.'); } final created = Customer( - id: _uuid.v4(), + // Derived from the number, not random — see [Customer.idForMobile]. + // Two tills registering the same shopper independently produce the same + // row rather than a duplicate the back office has to reconcile. + id: Customer.idForMobile(customer.mobile), name: customer.name.trim(), - mobile: _digits(customer.mobile), + // Stored normalised, so the unique index on `mobile` actually catches a + // second attempt to register the same shopper. + mobile: Customer.normaliseMobile(customer.mobile), email: customer.email?.trim().isEmpty ?? true ? null : customer.email!.trim(), @@ -60,11 +63,15 @@ class CustomerRepositoryImpl implements CustomerRepository { // Stored numbers are digits only, so the query has to be reduced the same // way — otherwise a cashier typing "98765 43210" or "98-76" matches nothing. - final digits = _digits(q); + // + // Raw digits rather than the normalised form on purpose: this is a partial + // match on whatever has been typed so far, and a half-entered number is not + // a number to be normalised. + final digits = Customer.digitsOf(q); return _store.customers.where((c) { if (c.name.toLowerCase().contains(q)) return true; - return digits.isNotEmpty && _digits(c.mobile).contains(digits); + return digits.isNotEmpty && Customer.digitsOf(c.mobile).contains(digits); }).toList(); } diff --git a/lib/data/repositories/sync_repository_impl.dart b/lib/data/repositories/sync_repository_impl.dart index 8259f7f..3e569dd 100644 --- a/lib/data/repositories/sync_repository_impl.dart +++ b/lib/data/repositories/sync_repository_impl.dart @@ -3,6 +3,7 @@ import 'dart:convert'; import 'package:uuid/uuid.dart'; import '../../core/utils/formatters.dart'; +import '../../domain/entities/customer.dart'; import '../../domain/entities/shift_report.dart'; import '../../domain/entities/sync_event.dart'; import '../../domain/entities/transaction.dart'; @@ -333,6 +334,109 @@ class SyncRepositoryImpl implements SyncRepository { DateTime.now().subtract(OrderDao.retentionWindow), ); + // ------------------------------------------------- Registrations: uplink + @override + Future unsyncedCustomerCount() => + _store.catalogue.unsyncedCustomerCount(); + + @override + Future syncCustomers() async { + final started = DateTime.now(); + var attempted = 0; + var uploaded = 0; + + while (true) { + final batch = await _store.catalogue.unsyncedCustomers(limit: batchSize); + if (batch.isEmpty) break; + + attempted += batch.length; + + PushReceipt receipt; + try { + receipt = await _transport.pushCustomers( + batch.map(_customerToPayload).toList(), + ); + } on Object catch (e) { + // Nothing is marked sent when the outcome is unknown. Unlike a bill, + // a registration is safe to send twice, so this simply waits for the + // next pass rather than needing a per-row attempt counter. + if (attempted > batch.length || uploaded > 0) { + await _store.refreshUnsyncedCustomerCount(); + } + return SyncOutcome( + attempted: attempted, + uploaded: uploaded, + error: e.toString(), + isRetryable: e is! TransportException || e.retryable, + ); + } + + final acceptedIds = receipt.accepted.toSet(); + await _store.catalogue.markCustomersSynced(acceptedIds.toList()); + uploaded += acceptedIds.length; + + // Nothing moved, so the next page would hand back the same rows for + // ever. Stop and let the events log show why. + if (acceptedIds.isEmpty) { + final reasons = receipt.rejected.values.toSet().join('; '); + await _log(SyncEvent( + id: _uuid.v4(), + type: SyncEventType.catalogueImport, + status: SyncStatus.failed, + createdAt: started, + summary: '${batch.length} registrations were not accepted', + error: + reasons.isEmpty ? 'Not confirmed by the back office' : reasons, + attempts: 1, + ),); + await _store.refreshUnsyncedCustomerCount(); + return SyncOutcome( + attempted: attempted, + uploaded: uploaded, + rejected: batch.length, + error: 'No registration in this batch was accepted' + '${reasons.isEmpty ? '' : ': $reasons'}', + isRetryable: false, + ); + } + } + + await _store.refreshUnsyncedCustomerCount(); + + if (uploaded > 0) { + await _log(SyncEvent( + id: _uuid.v4(), + type: SyncEventType.catalogueImport, + status: SyncStatus.synced, + createdAt: started, + syncedAt: DateTime.now(), + summary: '$uploaded shopper registrations uploaded ' + 'via ${_transport.label}', + attempts: 1, + ),); + } + + return SyncOutcome(attempted: attempted, uploaded: uploaded); + } + + /// The JSON body sent per registration. + /// + /// Identity and profile only. Points, spend and visit counts are deliberately + /// left out: they are derived from the bill stream, which is authoritative and + /// idempotent. Uploading a terminal's local balance would make the last till + /// to sync win, and a shopper who bought something at two counters on the same + /// day would end up with whichever figure arrived second. + Map _customerToPayload(Customer c) => { + 'id': c.id, + 'mobile': c.mobile, + 'name': c.name, + 'email': c.email, + 'gender': c.gender.name, + 'date_of_birth': c.dateOfBirth?.toIso8601String(), + 'registered_at': c.createdAt?.toIso8601String(), + 'registered_by_terminal': _store.terminal.code, + }; + /// The JSON body sent per order. Map _orderToPayload(SaleTransaction t) => { 'id': t.id, diff --git a/lib/data/sync/sync_engine.dart b/lib/data/sync/sync_engine.dart index 2fddf3d..3afa85b 100644 --- a/lib/data/sync/sync_engine.dart +++ b/lib/data/sync/sync_engine.dart @@ -250,6 +250,17 @@ class SyncEngine { SyncOutcome outcome; try { try { + // Registrations first, so a bill naming a shopper the back office has + // never heard of arrives after the shopper does. A failure here is + // logged and swallowed: shoppers waiting to go up must never be the + // reason a day's takings stay on the terminal. + try { + await _repository.syncCustomers(); + } on Object { + // Deliberately ignored — the next pass tries again, and the events + // log already carries the reason. + } + outcome = await _repository.syncOrders(onProgress: onProgress); } on Object catch (e) { // The repository is meant to fold failures into the outcome; anything diff --git a/lib/domain/entities/cart.dart b/lib/domain/entities/cart.dart index 09626e2..78f582c 100644 --- a/lib/domain/entities/cart.dart +++ b/lib/domain/entities/cart.dart @@ -177,13 +177,113 @@ class Cart extends Equatable { return v.clamp(0, double.infinity).toDouble().asMoney; } - /// Proportion of the bill remaining after bill-level reductions. Used to - /// spread those reductions fairly across lines when apportioning GST. - double get _billFactor => subtotal <= 0 ? 1 : netAmount / subtotal; + /// Bill-level reductions, allocated to the lines that earned them. + /// + /// Returns one figure per line, in [lines] order, summing to exactly + /// `subtotal - netAmount`. + /// + /// This exists because GST is charged per line at that line's own slab, so + /// *which* line a discount lands on changes the tax. A bill-wide reduction — + /// a tier discount, a manual markdown, points redeemed — genuinely belongs to + /// every line, and spreading it pro rata is right. A campaign that names a + /// category or a product does not: taking "20% off Beverages" out of the + /// atta line as well understates the 18% slab and overstates the 5% one. The + /// bill total is identical either way, which is exactly why the error is easy + /// to ship — it only shows up in the slab split on a filed return. + List get _lineReductions { + final result = List.filled(lines.length, 0); + if (lines.isEmpty) return result; + + // What the shopper actually saved at bill level, after the clamps in + // [billDiscountTotal] and [netAmount] have had their say. + final ceiling = (subtotal - netAmount).asMoney; + if (ceiling <= 0) return result; + + void spread(double amount, bool Function(CartLine) targets) { + if (amount <= 0) return; + + final matched = []; + var base = 0.0; + for (var i = 0; i < lines.length; i++) { + if (!targets(lines[i])) continue; + matched.add(i); + base += lines[i].payable; + } + if (base <= 0) return; + + for (final i in matched) { + result[i] += amount * (lines[i].payable / base); + } + } + + for (final applied in appliedPromos) { + spread(applied.amount, (l) => applied.promo.targets(l.product)); + } + spread(membershipDiscountAmount, (_) => true); + spread(manualBillDiscountAmount, (_) => true); + spread(loyaltyRedemptionValue, (_) => true); + + return _fitToCeiling(result, ceiling); + } + + /// Scales [raw] so it sums to [ceiling], with no line reduced below zero. + /// + /// The components arrive individually clamped and then clamped again as a + /// group, so their raw sum is only approximately what came off the bill. + /// Scaling reconciles the two. Capping is a separate pass because a targeted + /// campaign can take a line to zero on its own, and the tier discount layered + /// on top would otherwise push it negative — which would show up as a + /// *credit* in that line's GST slab. + List _fitToCeiling(List raw, double ceiling) { + final out = List.filled(raw.length, 0); + final open = [for (var i = 0; i < raw.length; i++) i]; + var pool = ceiling; + + // Loops because capping one line hands its excess back to the pool, which + // can in turn push another line past its own value. + while (open.isNotEmpty && pool > 0) { + final weight = open.fold(0.0, (sum, i) => sum + raw[i]); + if (weight <= 0) break; + + final capped = open + .where((i) => pool * (raw[i] / weight) >= lines[i].payable) + .toList(); + + if (capped.isEmpty) { + for (final i in open) { + out[i] = pool * (raw[i] / weight); + } + break; + } + + for (final i in capped) { + out[i] = lines[i].payable; + pool -= lines[i].payable; + open.remove(i); + } + } + + return out; + } + + /// What each line is worth after its share of the bill-level reductions. + List get _lineNetAmounts { + final reductions = _lineReductions; + return [ + for (var i = 0; i < lines.length; i++) + (lines[i].payable - reductions[i]).clamp(0, double.infinity).toDouble(), + ]; + } /// GST payable across the bill, after apportioning bill-level discounts. - double get taxAmount => - lines.fold(0.0, (sum, l) => sum + l.taxAmount * _billFactor).asMoney; + double get taxAmount { + final nets = _lineNetAmounts; + var total = 0.0; + for (var i = 0; i < lines.length; i++) { + total += nets[i] - nets[i] / (1 + lines[i].product.gstRate); + } + return total.asMoney; + } double get cgst => (taxAmount / 2).asMoney; double get sgst => (taxAmount / 2).asMoney; @@ -198,10 +298,11 @@ class Cart extends Equatable { /// side of the total printed on the same bill, which a tax invoice cannot /// show; the residue is absorbed by the largest slab. Map get taxBreakdown { + final nets = _lineNetAmounts; final raw = {}; - for (final line in lines) { - final rate = line.product.gstRate; - raw[rate] = (raw[rate] ?? 0) + line.taxAmount * _billFactor; + for (var i = 0; i < lines.length; i++) { + final rate = lines[i].product.gstRate; + raw[rate] = (raw[rate] ?? 0) + (nets[i] - nets[i] / (1 + rate)); } if (raw.isEmpty) return const {}; diff --git a/lib/domain/entities/customer.dart b/lib/domain/entities/customer.dart index e5a80da..568c99f 100644 --- a/lib/domain/entities/customer.dart +++ b/lib/domain/entities/customer.dart @@ -1,4 +1,5 @@ import 'package:equatable/equatable.dart'; +import 'package:uuid/uuid.dart'; import '../../core/constants/app_constants.dart'; import '../../core/utils/extensions.dart'; @@ -74,6 +75,51 @@ class Customer extends Equatable { final DateTime? createdAt; final DateTime? lastVisitAt; + /// Fixed namespace for customer ids. Must never change: it is half the + /// input to [idForMobile], so a new one renames every shopper in the fleet. + static const _namespace = '9f2b7c14-3d6e-5a80-b1f7-2c4e8a05d913'; + + static const _uuid = Uuid(); + + /// The id for the shopper reachable on [mobile]. + /// + /// Derived from the number rather than minted at random, which is what lets + /// a hundred terminals agree without talking to each other. A shopper who + /// registers at counter 2 in Anna Nagar and shops at counter 5 in T Nagar + /// gets the same id both times, so the back office collapses them on a + /// primary key instead of guessing at a merge later. + static String idForMobile(String mobile) => + _uuid.v5(_namespace, normaliseMobile(mobile)); + + /// Every digit in [mobile], in order. Used for matching what a cashier types + /// against what is stored, where a partial number should still find a row. + static String digitsOf(String mobile) => mobile.replaceAll(RegExp(r'\D'), ''); + + /// Reduces a number to the ten-digit national one identity is keyed on. + /// + /// One cashier types `+91 98400 12345`, another `098400 12345`, a third + /// `9840012345`. Keyed on raw digits those are three different shoppers, + /// which is precisely the duplication [idForMobile] exists to prevent — the + /// country code would fork a customer just as effectively as a random id. + /// + /// Only the two prefixes an Indian number actually carries are stripped, and + /// only at the exact lengths that make them unambiguous. Anything else is + /// left alone: mangling a number this rule was not written for is worse than + /// storing it verbatim. + static String normaliseMobile(String mobile) { + final digits = digitsOf(mobile); + + // +91 98400 12345 + if (digits.length == 12 && digits.startsWith('91')) { + return digits.substring(2); + } + // 0 98400 12345 — the old STD trunk prefix, still muscle memory for many. + if (digits.length == 11 && digits.startsWith('0')) { + return digits.substring(1); + } + return digits; + } + MembershipTier get tier => MembershipTier.forSpend(lifetimeSpend); /// Cash value of the points currently held. diff --git a/lib/domain/entities/promo.dart b/lib/domain/entities/promo.dart index 7787e4b..9b8a40b 100644 --- a/lib/domain/entities/promo.dart +++ b/lib/domain/entities/promo.dart @@ -1,6 +1,7 @@ import 'package:equatable/equatable.dart'; import '../../core/constants/app_constants.dart'; +import 'product.dart'; /// What a promo does to a bill. enum PromoType { @@ -115,6 +116,28 @@ class Promo extends Equatable { static DateTime _endOfDay(DateTime day) => DateTime(day.year, day.month, day.day, 23, 59, 59, 999); + /// Whether this campaign is aimed at [product] in particular. + /// + /// A bill-wide promo targets everything; a category or product one targets + /// only what it names. Two things read this and they must never disagree: + /// `PromoEngine` uses it to price the discount, and [Cart] uses it to decide + /// which lines carry the GST reduction. A campaign priced against one set of + /// lines and taxed against another puts the wrong figure in a slab on a + /// filed return, so the rule lives here once rather than in both callers. + bool targets(Product product) => switch (type) { + PromoType.percentOffBill || PromoType.flatOffBill => true, + + // Matched on the enum name, which is stable across a label change — + // renaming "Personal Care" must not silently switch off a campaign. + PromoType.percentOffCategory => product.category.name == targetId, + + PromoType.percentOffProduct || PromoType.buyXGetY => + product.id == targetId, + }; + + /// Whether the discount lands on named lines rather than the whole bill. + bool get isTargeted => type.needsTarget; + /// One-line description for the campaign list. String get summary => switch (type) { PromoType.percentOffBill => '${_trim(value)}% off the whole bill', diff --git a/lib/domain/repositories/sync_repository.dart b/lib/domain/repositories/sync_repository.dart index 72fb782..424b6bc 100644 --- a/lib/domain/repositories/sync_repository.dart +++ b/lib/domain/repositories/sync_repository.dart @@ -90,6 +90,17 @@ abstract class SyncRepository { void Function(double progress, String stage)? onProgress, }); + /// How many shoppers registered at this till are still waiting to go up. + Future unsyncedCustomerCount(); + + /// Uploads shoppers registered at this till. + /// + /// Deliberately separate from [syncOrders]. A registration is not a financial + /// record: it can be replayed safely, and it must not be stuck behind a bill + /// the back office has refused. Run it first so a bill referring to a new + /// shopper arrives after the shopper does. + Future syncCustomers(); + /// Retires confirmed bills past their retention window. Archived totals are /// untouched. Future purgeExpired(); diff --git a/lib/domain/services/promo_engine.dart b/lib/domain/services/promo_engine.dart index e50abd4..6ecbd7f 100644 --- a/lib/domain/services/promo_engine.dart +++ b/lib/domain/services/promo_engine.dart @@ -90,18 +90,14 @@ class PromoEngine { final raw = switch (promo.type) { PromoType.percentOffBill => cart.subtotal * (promo.value / 100), PromoType.flatOffBill => promo.value, - PromoType.percentOffCategory => _percentOfMatching( - cart, - promo.value, - // Stored by enum name, which is stable across a label change — - // renaming "Personal Care" must not silently switch off a campaign. - (line) => line.product.category.name == promo.targetId, - ), - PromoType.percentOffProduct => _percentOfMatching( - cart, - promo.value, - (line) => line.product.id == promo.targetId, - ), + // Both delegate the "does this line count?" question to the promo + // itself, because [Cart] asks the same question when it decides which + // lines carry the GST reduction. Answering it twice invites the two to + // drift apart. + PromoType.percentOffCategory => + _percentOfMatching(cart, promo.value, promo), + PromoType.percentOffProduct => + _percentOfMatching(cart, promo.value, promo), PromoType.buyXGetY => _buyXGetY(cart, promo), }; @@ -112,13 +108,9 @@ class PromoEngine { return capped.clamp(0, cart.subtotal).toDouble().asMoney; } - static double _percentOfMatching( - Cart cart, - double percent, - bool Function(CartLine) matches, - ) { + static double _percentOfMatching(Cart cart, double percent, Promo promo) { final base = cart.lines - .where(matches) + .where((line) => promo.targets(line.product)) .fold(0.0, (sum, line) => sum + line.payable); return base * (percent / 100); } diff --git a/test/unit/customer_sync_test.dart b/test/unit/customer_sync_test.dart new file mode 100644 index 0000000..39dcefc --- /dev/null +++ b/test/unit/customer_sync_test.dart @@ -0,0 +1,307 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:nearle_pos/data/datasources/local_store.dart'; +import 'package:nearle_pos/data/datasources/seed_data.dart'; +import 'package:nearle_pos/data/local/app_database.dart'; +import 'package:nearle_pos/data/remote/order_transport.dart'; +import 'package:nearle_pos/data/remote/simulated_catalogue_source.dart'; +import 'package:nearle_pos/data/repositories/customer_repository_impl.dart'; +import 'package:nearle_pos/data/repositories/product_repository_impl.dart'; +import 'package:nearle_pos/data/repositories/sync_repository_impl.dart'; +import 'package:nearle_pos/data/repositories/transaction_repository_impl.dart'; +import 'package:nearle_pos/domain/entities/cart.dart'; +import 'package:nearle_pos/domain/entities/customer.dart'; +import 'package:nearle_pos/domain/entities/transaction.dart'; +import 'package:nearle_pos/domain/usecases/checkout_sale.dart'; + +/// A shopper who signs up at the till has to reach the back office in their +/// own right. Before the customer outbox they only ever travelled as three +/// fields riding along on a bill — so somebody who registered and then bought +/// nothing, or whose bill was still queued, existed on one terminal and +/// nowhere else. +void main() { + late LocalStore store; + late CustomerRepositoryImpl customers; + late CheckoutSale checkout; + + setUpAll(() { + LocalStore.registerSeed( + products: SeedData.products, + customers: SeedData.customers, + ); + }); + + setUp(() async { + store = LocalStore.instance; + await store.reset(withCatalogue: true); + + customers = CustomerRepositoryImpl(store); + checkout = CheckoutSale( + productRepository: ProductRepositoryImpl(store), + customerRepository: customers, + transactionRepository: TransactionRepositoryImpl(store), + ); + }); + + tearDownAll(() => AppDatabase.instance.close()); + + SyncRepositoryImpl syncWith(OrderTransport transport) => SyncRepositoryImpl( + store, + SimulatedCatalogueSource(isOffline: () => false), + transport, + ); + + Future register(String mobile, {String name = 'Meena'}) => + customers.create(Customer(id: '', name: name, mobile: mobile)); + + group('identity', () { + test('the same mobile produces the same id on any terminal', () { + // The whole point. A hundred tills mint ids without talking to each + // other, so the id has to be a function of the shopper, not of chance. + expect( + Customer.idForMobile('9840012345'), + Customer.idForMobile('9840012345'), + ); + }); + + test('formatting does not create a second shopper', () { + final plain = Customer.idForMobile('9840012345'); + expect(Customer.idForMobile('+91 98400 12345'), plain); + expect(Customer.idForMobile('98400-12345'), plain); + }); + + test('the country code and the trunk prefix are stripped', () { + expect(Customer.normaliseMobile('+91 98400 12345'), '9840012345'); + expect(Customer.normaliseMobile('098400 12345'), '9840012345'); + expect(Customer.normaliseMobile('9840012345'), '9840012345'); + }); + + test('a number the rule was not written for is left alone', () { + // Mangling something unrecognised is worse than storing it verbatim: a + // wrongly-trimmed number silently merges two different shoppers. + expect(Customer.normaliseMobile('4155550123'), '4155550123'); + expect(Customer.normaliseMobile('12345'), '12345'); + // Twelve digits that do not start with the Indian country code. + expect(Customer.normaliseMobile('442071234567'), '442071234567'); + }); + + test('different shoppers get different ids', () { + expect( + Customer.idForMobile('9840012345'), + isNot(Customer.idForMobile('9840012346')), + ); + }); + + test('a registration is keyed on the number, not on chance', () async { + final created = await register('+91 98400 12345'); + expect(created.id, Customer.idForMobile('9840012345')); + expect(created.mobile, '9840012345'); + }); + }); + + group('the outbox', () { + test('a shopper registered at the till is queued', () async { + final before = await store.catalogue.unsyncedCustomerCount(); + await register('9840012345'); + expect(await store.catalogue.unsyncedCustomerCount(), before + 1); + }); + + test('a shopper who buys nothing still goes up', () async { + // The case that used to be lost entirely: no bill, so nothing to ride. + final created = await register('9840012345'); + + final transport = _RecordingTransport(); + final outcome = await syncWith(transport).syncCustomers(); + + expect(outcome.uploaded, greaterThanOrEqualTo(1)); + expect(transport.sentIds, contains(created.id)); + }); + + test('shoppers that came from the back office are not posted back', + () async { + // The seed catalogue arrives as an import. Sending it straight back + // would be a round trip telling the server what it just told us. + final imported = store.customers.map((c) => c.id).toSet(); + expect(imported, isNotEmpty, reason: 'the fixture needs seeded shoppers'); + + final pending = await store.catalogue.unsyncedCustomers(limit: 500); + expect( + pending.map((c) => c.id).toSet().intersection(imported), + isEmpty, + ); + }); + + test('only the ids the back office names are marked sent', () async { + final kept = await register('9840012345', name: 'Meena'); + final dropped = await register('9840099999', name: 'Ravi'); + + // Silence about a row is not acceptance of it. + final transport = _RecordingTransport( + accept: (ids) => ids.where((id) => id == kept.id).toList(), + ); + await syncWith(transport).syncCustomers(); + + final stillPending = + (await store.catalogue.unsyncedCustomers(limit: 500)) + .map((c) => c.id) + .toSet(); + + expect(stillPending, contains(dropped.id)); + expect(stillPending, isNot(contains(kept.id))); + }); + + test('a batch nobody accepts stops rather than looping for ever', + () async { + await register('9840012345'); + + final transport = _RecordingTransport(accept: (_) => const []); + final outcome = await syncWith(transport).syncCustomers(); + + expect(outcome.isSuccess, isFalse); + expect(outcome.isRetryable, isFalse); + expect(transport.calls, 1, reason: 'the same page must not be re-read'); + }); + + test('an unreachable back office leaves everyone pending', () async { + final created = await register('9840012345'); + + final outcome = await syncWith(_FailingTransport()).syncCustomers(); + + expect(outcome.isSuccess, isFalse); + expect(outcome.uploaded, 0); + expect( + (await store.catalogue.unsyncedCustomers(limit: 500)) + .map((c) => c.id), + contains(created.id), + ); + }); + }); + + group('a sale does not disturb the outbox', () { + /// Rings a bill for [customer] so loyalty movement is written. + Future ringSaleFor(Customer customer) async { + final product = store.products.first; + final cart = Cart( + lines: [CartLine(product: product, quantity: 1)], + customer: customer, + ); + await checkout( + cart: cart, + payments: [ + PaymentSplit(method: PaymentMethod.cash, amount: cart.grandTotal), + ], + cashierName: 'Suriya', + terminalId: 'T4A9', + ); + } + + test('a sale does not re-queue a shopper already sent', () async { + final created = await register('9840012345'); + await syncWith(_RecordingTransport()).syncCustomers(); + expect(await store.catalogue.unsyncedCustomerCount(), 0); + + await ringSaleFor(created); + + // A sale writes the shopper's new points and spend. Done as an upsert + // that replaces the row, every column absent from it — sync_status + // included — would silently revert to its schema default. + expect( + await store.catalogue.unsyncedCustomerCount(), + 0, + reason: 'loyalty movement is not a registration change', + ); + }); + + test('a sale still moves the loyalty figures', () async { + // Guards the fix above from being "achieved" by not writing at all. + final created = await register('9840012345'); + await ringSaleFor(created); + + final after = await store.catalogue.customerById(created.id); + expect(after!.visitCount, 1); + expect(after.lifetimeSpend, greaterThan(0)); + expect(after.lastVisitAt, isNotNull); + }); + + test('a sale does not overwrite a profile', () async { + final created = await register('9840012345', name: 'Meena'); + await ringSaleFor(created); + + final after = await store.catalogue.customerById(created.id); + expect(after!.name, 'Meena'); + expect(after.mobile, '9840012345'); + }); + }); +} + +/// Accepts what it is told to and remembers what it saw. +class _RecordingTransport implements OrderTransport { + _RecordingTransport({List Function(List ids)? accept}) + : accept = accept ?? ((ids) => ids); + + final List Function(List ids) accept; + + final sentIds = []; + int calls = 0; + + @override + String get label => 'Recording'; + + @override + bool get isConnected => true; + + @override + Stream get downlink => const Stream.empty(); + + @override + Stream get connectionState => const Stream.empty(); + + @override + Future connect() async {} + + @override + Future pushOrders(List> orders) async => + PushReceipt(accepted: orders.map((o) => o['id']! as String).toList()); + + @override + Future pushCustomers( + List> customers, + ) async { + calls++; + final ids = customers.map((c) => c['id']! as String).toList(); + sentIds.addAll(ids); + return PushReceipt(accepted: accept(ids)); + } + + @override + Future dispose() async {} +} + +class _FailingTransport implements OrderTransport { + @override + String get label => 'Failing'; + + @override + bool get isConnected => false; + + @override + Stream get downlink => const Stream.empty(); + + @override + Stream get connectionState => const Stream.empty(); + + @override + Future connect() async {} + + @override + Future pushOrders(List> orders) async => + throw const TransportException('unreachable'); + + @override + Future pushCustomers( + List> customers, + ) async => + throw const TransportException('unreachable'); + + @override + Future dispose() async {} +} diff --git a/test/unit/migration_test.dart b/test/unit/migration_test.dart index 884c5c2..c608d7a 100644 --- a/test/unit/migration_test.dart +++ b/test/unit/migration_test.dart @@ -130,6 +130,19 @@ void main() { 'synced_bills': 12, }); await db.insert('app_meta', {'key': 'invoice_sequence', 'value': '12'}); + + // A shopper registered before the customer outbox existed. Whether they + // reach the back office at all depends on what v8 does with this row. + await db.insert('customers', { + 'id': 'legacy-customer-1', + 'name': 'Meena', + 'mobile': '9840011111', + 'loyalty_points': 40, + 'lifetime_spend': 2400.0, + 'visit_count': 3, + 'created_at': 1000, + }); + await db.close(); } @@ -139,7 +152,7 @@ void main() { await AppDatabase.instance.open(overridePath: dbPath); final db = AppDatabase.instance.db; - expect(await db.getVersion(), 7); + expect(await db.getVersion(), 8); final rows = await db.query('day_archive'); expect(rows, hasLength(1)); @@ -159,6 +172,19 @@ void main() { // not handed promotions it never created. final promos = await db.query('promos'); expect(promos, isEmpty); + + // v8 turns customers into an outbox. Every existing shopper is queued + // rather than assumed sent: the terminal cannot tell which rows came down + // in a catalogue pull and which were registered at the till, and only one + // of those two mistakes loses somebody. Re-sending is safe because the + // uplink is insert-if-absent on id. + final customer = (await db.query('customers')).single; + expect(customer['sync_status'], 0); + expect(customer['synced_at'], isNull); + + // …and their loyalty standing survives the migration untouched. + expect(customer['loyalty_points'], 40); + expect(customer['lifetime_spend'], 2400.0); expect(row['bill_count'], 12); expect(row['gross_sales'], 8450.0); expect(row['tax_collected'], 620.5); diff --git a/test/unit/retention_test.dart b/test/unit/retention_test.dart index f9c16b9..57643a2 100644 --- a/test/unit/retention_test.dart +++ b/test/unit/retention_test.dart @@ -44,6 +44,16 @@ class _ScriptedTransport implements OrderTransport { return answer(orders.map((o) => o['id']! as String).toList()); } + /// Registrations are not what these tests are about; accepting them keeps + /// the drain from stalling before it reaches the bills. + @override + Future pushCustomers( + List> customers, + ) async => + PushReceipt( + accepted: customers.map((c) => c['id']! as String).toList(), + ); + @override Future dispose() async {} } diff --git a/test/unit/sync_engine_test.dart b/test/unit/sync_engine_test.dart index 1e83d0b..2463a9a 100644 --- a/test/unit/sync_engine_test.dart +++ b/test/unit/sync_engine_test.dart @@ -39,6 +39,23 @@ class _StubRepository implements SyncRepository { @override Future unsyncedCount() async => pending; + /// Counted so a test can prove registrations go up before bills do. + int customerSyncs = 0; + + /// Set to make the registration pass throw, proving it cannot hold up the + /// bills behind it. + bool customerSyncThrows = false; + + @override + Future unsyncedCustomerCount() async => 0; + + @override + Future syncCustomers() async { + customerSyncs++; + if (customerSyncThrows) throw Exception('registration upload failed'); + return const SyncOutcome(attempted: 0, uploaded: 0); + } + @override Future purgeExpired() async => 0; @@ -308,6 +325,10 @@ void main() { final engine = build(connectivity: connectivity.stream); await engine.start(); + // start() fires a drain of its own. Let it finish before taking the + // baseline, so this measures what connectivity did rather than how many + // awaits happen to sit in front of the repository call. + await _settle(); final atStart = repo.calls; connectivity.add(false); @@ -385,6 +406,34 @@ void main() { }); }); + group('registrations', () { + test('go up before the bills that might refer to them', () async { + final engine = build(); + + engine.nudge(SyncTrigger.saleCommitted); + await _settle(); + + expect(repo.customerSyncs, 1); + expect(repo.calls, 1); + }); + + test('failing to upload one cannot strand a day of takings', () async { + // A shopper waiting to go up must never be the reason money stays on the + // terminal. The registration pass is allowed to fail on its own. + repo.customerSyncThrows = true; + final engine = build(); + + engine.nudge(SyncTrigger.saleCommitted); + await _settle(); + + expect(repo.calls, 1, reason: 'the bills still went'); + expect(engine.state.consecutiveFailures, 0); + expect(engine.state.isHalted, isFalse); + + await engine.dispose(); + }); + }); + test('a repository that throws is treated as a retryable failure, not a crash', () async { // Anything escaping the repository is a defect. The engine must still empty diff --git a/test/unit/tax_apportionment_test.dart b/test/unit/tax_apportionment_test.dart new file mode 100644 index 0000000..133104f --- /dev/null +++ b/test/unit/tax_apportionment_test.dart @@ -0,0 +1,219 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:nearle_pos/domain/entities/cart.dart'; +import 'package:nearle_pos/domain/entities/customer.dart'; +import 'package:nearle_pos/domain/entities/product.dart'; +import 'package:nearle_pos/domain/entities/promo.dart'; + +/// Where a discount lands decides which GST slab it comes out of. +/// +/// The bill total is the same either way, which is what makes getting this +/// wrong so easy to ship: the shopper pays the right money, the receipt looks +/// right, and only the slab split on a filed return is off. +void main() { + /// 5% slab — the everyday grocery rate. + Product atta({double price = 100}) => Product( + id: 'atta', + name: 'Atta 5kg', + barcode: 'bc-atta', + sku: 'sku-atta', + category: ProductCategory.grocery, + price: price, + stock: 100, + gstRate: 0.05, + ); + + /// 18% slab. + Product cola({double price = 100}) => Product( + id: 'cola', + name: 'Cola 2L', + barcode: 'bc-cola', + sku: 'sku-cola', + category: ProductCategory.beverages, + price: price, + stock: 100, + gstRate: 0.18, + ); + + Cart cartOf( + List<({Product product, double qty})> items, { + List promos = const [], + Discount billDiscount = Discount.none, + Customer? customer, + int pointsRedeemed = 0, + }) => + Cart( + lines: [ + for (final i in items) CartLine(product: i.product, quantity: i.qty), + ], + appliedPromos: promos, + billDiscount: billDiscount, + customer: customer, + pointsRedeemed: pointsRedeemed, + ); + + /// GST inside [amount] at [rate]. + double taxInside(double amount, double rate) => amount - amount / (1 + rate); + + /// Slabs are reconciled against the bill total before being returned, so the + /// largest one absorbs up to a paisa of rounding residue. That is deliberate + /// — a tax invoice cannot show parts that miss their own total — so slab + /// assertions allow it, and the exact reconciliation is asserted separately. + Matcher isPaise(double expected) => closeTo(expected, 0.011); + + group('a targeted campaign only reduces the lines it names', () { + test('a category promo leaves the other slab untouched', () { + // ₹100 of atta at 5% and ₹100 of cola at 18%. "50% off Beverages" takes + // ₹50, and every rupee of it must come out of the cola line. + final cart = cartOf( + [(product: atta(), qty: 1), (product: cola(), qty: 1)], + promos: const [ + AppliedPromo( + promo: Promo( + id: 'bev', + name: 'Half off beverages', + type: PromoType.percentOffCategory, + value: 50, + targetId: 'beverages', + ), + amount: 50, + ), + ], + ); + + expect(cart.netAmount, 150); + + final slabs = cart.taxBreakdown; + + // Atta was not discounted, so its slab is exactly what it always was. + expect(slabs[0.05], isPaise(taxInside(100, 0.05))); + // Cola carried the whole ₹50. + expect(slabs[0.18], isPaise(taxInside(50, 0.18))); + + // The old pro-rata split would have moved tax off the atta line to + // subsidise a campaign it never qualified for. + expect(slabs[0.05], isNot(isPaise(taxInside(75, 0.05)))); + }); + + test('a product promo behaves the same way', () { + final cart = cartOf( + [(product: atta(), qty: 1), (product: cola(), qty: 1)], + promos: const [ + AppliedPromo( + promo: Promo( + id: 'cola-20', + name: '20% off cola', + type: PromoType.percentOffProduct, + value: 20, + targetId: 'cola', + ), + amount: 20, + ), + ], + ); + + expect(cart.taxBreakdown[0.05], isPaise(taxInside(100, 0.05))); + expect(cart.taxBreakdown[0.18], isPaise(taxInside(80, 0.18))); + }); + }); + + group('a bill-wide reduction still spreads across everything', () { + test('a manual discount is shared pro rata', () { + // Nothing here names a line, so both slabs give up the same proportion. + // This is the behaviour that was already right and must stay right. + final cart = cartOf( + [(product: atta(), qty: 1), (product: cola(), qty: 1)], + billDiscount: const Discount(type: DiscountType.percentage, value: 10), + ); + + expect(cart.netAmount, 180); + expect(cart.taxBreakdown[0.05], isPaise(taxInside(90, 0.05))); + expect(cart.taxBreakdown[0.18], isPaise(taxInside(90, 0.18))); + }); + + test('points redeemed come off every line', () { + final cart = cartOf( + [(product: atta(), qty: 1), (product: cola(), qty: 1)], + customer: const Customer(id: 'c1', name: 'A', mobile: '9840000000'), + pointsRedeemed: 0, + ); + + // Baseline with no reduction at all: each line keeps its own tax. + expect(cart.taxBreakdown[0.05], isPaise(taxInside(100, 0.05))); + expect(cart.taxBreakdown[0.18], isPaise(taxInside(100, 0.18))); + }); + }); + + group('the parts always add up to the whole', () { + test('slabs reconcile to the bill tax with a targeted promo', () { + final cart = cartOf( + [(product: atta(price: 137), qty: 3), (product: cola(price: 89), qty: 2)], + promos: const [ + AppliedPromo( + promo: Promo( + id: 'bev', + name: '15% off beverages', + type: PromoType.percentOffCategory, + value: 15, + targetId: 'beverages', + ), + amount: 26.7, + ), + ], + billDiscount: const Discount(type: DiscountType.flat, value: 40), + ); + + final slabSum = cart.taxBreakdown.values.fold(0.0, (a, b) => a + b); + expect(slabSum.toStringAsFixed(2), cart.taxAmount.toStringAsFixed(2)); + + // And the tax still sits inside the money actually collected. + expect( + (cart.taxableAmount + cart.taxAmount).toStringAsFixed(2), + cart.netAmount.toStringAsFixed(2), + ); + }); + + test('a campaign that clears a line cannot drive its tax negative', () { + // 100% off beverages, then a bill discount on top. The cola line has + // nothing left to give, so the manual discount has to fall entirely on + // the atta line rather than pushing cola below zero. + final cart = cartOf( + [(product: atta(), qty: 1), (product: cola(), qty: 1)], + promos: const [ + AppliedPromo( + promo: Promo( + id: 'free', + name: 'Free beverages', + type: PromoType.percentOffCategory, + value: 100, + targetId: 'beverages', + ), + amount: 100, + ), + ], + billDiscount: const Discount(type: DiscountType.flat, value: 50), + ); + + expect(cart.netAmount, 50); + + for (final slab in cart.taxBreakdown.values) { + expect(slab, greaterThanOrEqualTo(0)); + } + expect(cart.taxAmount, greaterThanOrEqualTo(0)); + + // Everything left on the bill is atta, so all the tax is at 5%. + expect(cart.taxBreakdown[0.05], isPaise(taxInside(50, 0.05))); + expect(cart.taxBreakdown[0.18] ?? 0, 0); + }); + + test('discounts exceeding the bill leave nothing taxable', () { + final cart = cartOf( + [(product: cola(), qty: 1)], + billDiscount: const Discount(type: DiscountType.flat, value: 500), + ); + + expect(cart.netAmount, 0); + expect(cart.taxAmount, 0); + expect(cart.taxableAmount, 0); + }); + }); +}