From 98398894a59f29d79eab1c644bc35427026a39f5 Mon Sep 17 00:00:00 2001 From: abhishek Date: Tue, 15 Sep 2026 20:51:01 +0530 Subject: [PATCH] design --- src/api/customers.ts | 18 + src/api/people.ts | 14 +- src/api/tenants.ts | 25 + .../pages/GlobalCataloguePage.tsx | 65 +-- .../nearle-admin/pages/OnboardTenantPage.tsx | 355 +++++++++----- .../nearle-admin/pages/PartnersPage.tsx | 305 ++++++------ .../store-admin/BranchScope.dom.test.tsx | 261 ++++++++++ src/features/store-admin/BranchScope.tsx | 101 +++- .../store-admin/pages/CountersPage.tsx | 34 ++ .../store-admin/pages/OnboardBranchPage.tsx | 447 ++++++++++++------ .../store-user/pages/StoreCustomersPage.tsx | 268 +++++++++-- .../store-user/pages/StoreStaffPage.tsx | 374 ++++++++++++--- src/features/store-user/ui.tsx | 22 +- src/queries/hooks.ts | 18 +- 14 files changed, 1688 insertions(+), 619 deletions(-) create mode 100644 src/features/store-admin/BranchScope.dom.test.tsx diff --git a/src/api/customers.ts b/src/api/customers.ts index e5272fa..5b850cd 100644 --- a/src/api/customers.ts +++ b/src/api/customers.ts @@ -29,6 +29,24 @@ export interface CustomerInfo { deliverylocationid?: number; tenantlocationid?: number; applocationid?: number; + /** + * Where the customer is, as text — the column type in `app_customers`. + * + * Sent by `gettenantcustomers` on 97% of rows (measured across 31 customers + * at 7 shops, 2026-09-15) and simply absent from this interface until now, so + * the one nearly-complete piece of geography the backend has about a shop's + * customers was invisible to every page. + */ + latitude?: string; + longitude?: string; + /** + * Empty on every customer row on the platform — 0 of 31. + * + * Kept declared because the column exists and a future write could fill it, + * but nothing should render an Active/Inactive state from it: a badge that + * reads the same on every row is decoration, and one that reads blank is + * worse. + */ status?: string; } diff --git a/src/api/people.ts b/src/api/people.ts index 0ac4946..bb61d67 100644 --- a/src/api/people.ts +++ b/src/api/people.ts @@ -140,11 +140,23 @@ export const posUsersApi = { * non-array and hands back `[]`, so the page showed "no till accounts" for a * shop that had them. Same shape trap as `/health/location`. */ - list: (tenantid: number, locationid: number) => + list: (tenantid: number, locationid: number, includeInactive = false) => api .get<{ location_id?: number; users?: PosUser[] }>(`${WEB}/tenants/getposusers`, { tenantid, locationid, + /* + Off by default, because that is what every existing caller assumed. + + The listing excludes inactive accounts unless asked + (`posUserRepository.go:494` — `LOWER(COALESCE(a.status,'active')) <> + 'inactive'`), and `WebListPosUsers` reads `include_inactive` from the + query string. Nothing sent it, which had two consequences: a supervisor + who switched a cashier off in the drawer watched them disappear from the + list with no trace and no way back, and any Status column could only + ever render "Active" because that was the only status a row could have. + */ + ...(includeInactive ? { include_inactive: 'true' } : {}), }) .then((page) => (Array.isArray(page?.users) ? page.users : [])), diff --git a/src/api/tenants.ts b/src/api/tenants.ts index d894c3c..a360ebc 100644 --- a/src/api/tenants.ts +++ b/src/api/tenants.ts @@ -44,6 +44,31 @@ export interface CreateTenantRequest { /** Everything the branch-onboarding form collects. */ export interface CreateBranchRequest { tenantid: number; + /** + * The delivery region, inherited from the tenant's existing outlets. + * + * `tenantlocations.applocationid` has no column default, so a create that + * omits it stores 0 — and `resolveOfflineLocationContext` in + * `orderRepository.go` calls this column "authoritative", with no fallback + * anywhere for a zero. It is also copied straight onto the login the backend + * spawns for the branch, so the outlet AND the person running it both end up + * in no region at all. + * + * Measured 2026-09-15: 43 of 75 live branches carry 0. Regions are + * 1 = Coimbatore, 2 = Madurai, 23 = Nagercoil. + */ + applocationid?: number; + /** + * Also inherited, and also without a column default. + * + * `orderRepository.go` documents the consequence in its own comment — + * "tenantlocations carries 0 for moduleid/partnerid at outlets whose live + * orders nonetheless use non-zero values" — and works around it by copying + * the scaffolding off the most recent real order at that outlet. A branch + * commissioned five minutes ago has no such order, so the workaround has + * nothing to copy and the joins are left to resolve against a zero. + */ + moduleid?: number; locationname: string; email?: string; contactno?: string; diff --git a/src/features/nearle-admin/pages/GlobalCataloguePage.tsx b/src/features/nearle-admin/pages/GlobalCataloguePage.tsx index 14894ad..c113f26 100644 --- a/src/features/nearle-admin/pages/GlobalCataloguePage.tsx +++ b/src/features/nearle-admin/pages/GlobalCataloguePage.tsx @@ -1,82 +1,21 @@ import { useSearchParams } from 'react-router-dom'; -import { SegmentedControl, SegmentedControlItem } from '@astryxdesign/core/SegmentedControl'; import { VStack } from '@astryxdesign/core/VStack'; -import { PackageSearch, Upload } from 'lucide-react'; import { PageHeader } from '@/components/PageHeader'; import { CatalogueBrowser } from '@/features/catalogue/CatalogueBrowser'; import { SheetImportPanel } from '../import/SheetImportPanel'; /** * The platform operator's catalogue. - * - * TWO MODES, AND THEY ASK FOR DIFFERENT THINGS. - * - * - **Browse** is reading. The operator looks at what the FMCG catalogue holds - * — the photographs, the pack sizes, the FSSAI licences, what is stocked - * where — and nothing is written. There is no merchant to choose because - * nothing lands anywhere, so the page is the rail, the search and the grid - * and nothing else. The per-product Add is gone from here: stocking one - * shop at a time is the merchant's own job, in Store Admin ▸ Inventory ▸ - * Catalogue. - * - * - **Upload sheet** is writing, and it writes the GLOBAL catalogue — not one - * merchant's shelf. The ingest service parses, enriches and stores rows in - * the per-brand tables every merchant reads from, and it takes no tenant and - * no outlet. This mode therefore asks for nothing but a file. - * - * The grid, the filters, the cards and the detail drawer are the shared - * `CatalogueBrowser` — the same ones the Store Admin sees, because it is the - * same catalogue. */ export function GlobalCataloguePage() { - /* - The tab is in the URL so it can be linked to. - - Uploads has an "Upload spreadsheet" button and this is where that action - lives for a platform operator — it writes the GLOBAL catalogue, which is not - the same action as a merchant uploading their own list. Held in local state it - could only be reached by landing on Browse and pressing a second control, - which is a detour rather than a flow. - */ - const [params, setParams] = useSearchParams(); + const [params] = useSearchParams(); const mode = params.get('tab') === 'sheet' ? 'sheet' : 'catalogue'; - const setMode = (next: 'catalogue' | 'sheet') => { - const merged = new URLSearchParams(params); - if (next === 'catalogue') merged.delete('tab'); - else merged.set('tab', next); - setParams(merged, { replace: true }); - }; return ( - setMode(value as 'catalogue' | 'sheet')} - size="sm" - > - } /> - } /> - - } - /> + {mode === 'sheet' ? ( - /* The merchant and outlet pickers used to sit above this panel, and - they described a flow that no longer exists. - - The ingest endpoint writes the GLOBAL catalogue. It has no concept of - a tenant or an outlet — putting a product on one shop's shelf with a - price and opening stock is a separate call (`/api/upload/stores`, - joined on `image_id`) that is not wired up yet. Two selectors saying - the upload was "written against one merchant and one outlet" would - have had someone pick a shop, upload, and then go looking for stock - that was never going to arrive. - - They come back with the inventory step, and mean something then. */ ) : ( (EMPTY); - /** - * The category list comes from `app_category`, not from four values typed - * into this file. A category added to the master should appear here without - * a release. - */ const categories = useAppCategories(); const [error, setError] = useState(null); @@ -110,6 +91,8 @@ export function OnboardTenantPage() { function handleSubmit(event: FormEvent) { event.preventDefault(); + if (!isComplete || mutation.isPending) return; + setError(null); mutation.mutate({ tenantname: form.tenantname.trim(), @@ -118,8 +101,6 @@ export function OnboardTenantPage() { primarycontact: form.primarycontact.trim(), primaryemail: form.primaryemail.trim(), locationname: form.locationname.trim(), - // `Number('')` is NaN, which serialises to null and is not what the - // backend means by "uncategorised" — 0 is. categoryid: Number(form.categoryid) || 0, address: form.address.trim(), suburb: form.suburb.trim(), @@ -133,121 +114,190 @@ export function OnboardTenantPage() { if (mutation.isSuccess) { const created = mutation.data; return ( - - - - - - - - {form.tenantname} is live + + +
+ +
+ + + + + + + {form.tenantname} is live + + + + Its first outlet, {form.locationname}, has been commissioned. The next step is + stocking the catalogue — pick products from the global catalogue, or upload the + tenant's own list as a spreadsheet. - - - Its first outlet, {form.locationname}, has been commissioned. The next step is - stocking the catalogue — pick products from the global catalogue, or upload the - tenant's own list as a spreadsheet. - - {/* The storefront code, at the one moment the person who provisioned - the shop is holding its details. - - It was reachable only from the branch user's own header, which - is the wrong place for it: the code is what puts the store in - front of a shopper at all — nobody can order from a shop they - have not scanned — and the person onboarding it is the one who - sends it to the merchant. `createtenantuser` returns the tenant - and its primary outlet's id together, so it can be drawn here - without a second read. */} - {created?.tenantid && created?.locationid ? ( - - + + + ) : null} + + + +
+ Fields marked with * are required +
+ + + {/* Section 1: Business Details Card */} - } - /> +
+ + + Business Details + +
+
Merchant name * as any} + label={Merchant name * as any} value={form.tenantname} onChange={set('tenantname')} placeholder="e.g. Kaveri Groceries" /> Company registered name * as any} + label={Company registered name * as any} value={form.companyname} onChange={set('companyname')} placeholder="e.g. Kaveri Retail Pvt. Ltd." /> - {/* Asked here because here is the only place it can be asked. - The primary outlet and the merchant's own admin login are - both created inside this one call, and the login is copied - from the tenant row — so the name given here names the - business AND the account that signs in. Left out, both are - blank, which is the state every merchant is in today. */} Store admin * as any} + label={Store admin * as any} value={form.adminname} onChange={set('adminname')} placeholder="e.g. Ravi Kumar" - description="The person who administers this shop. Their name goes on the store profile and on the admin login created with it — the account is provisioned as Admin, which is what this field is named after." /> Primary phone * as any} + label={Primary phone * as any} value={form.primarycontact} onChange={set('primarycontact')} placeholder="9876543210" /> Primary admin email * as any} + label={Primary admin email * as any} type="email" value={form.primaryemail} onChange={set('primaryemail')} placeholder="admin@kaveri.com" /> First outlet name * as any} + label={First outlet name * as any} value={form.locationname} onChange={set('locationname')} placeholder="e.g. Kaveri RS Puram" @@ -258,6 +308,7 @@ export function OnboardTenantPage() { value: String(entry.categoryid), label: entry.categoryname, }))} + placeholder="Select category" isDisabled={categories.isLoading} value={form.categoryid} onChange={set('categoryid')} @@ -266,24 +317,58 @@ export function OnboardTenantPage() { + {/* Section 2: Head Office Card */} - } - /> +
+ + + Head Office + +
+ Street address * as any} + label={Street address * as any} value={form.address} onChange={set('address')} placeholder="e.g. 12, Avinashi Road" /> +
- - City * as any} value={form.city} onChange={set('city')} /> - State * as any} value={form.state} onChange={set('state')} /> Postcode * as any} + label="Suburb" + value={form.suburb} + onChange={set('suburb')} + placeholder="e.g. Peelamedu" + /> + City * as any} + value={form.city} + onChange={set('city')} + placeholder="e.g. Coimbatore" + /> + State * as any} + value={form.state} + onChange={set('state')} + placeholder="e.g. Tamil Nadu" + /> + Postcode * as any} value={form.postcode} onChange={set('postcode')} placeholder="641004" @@ -292,34 +377,60 @@ export function OnboardTenantPage() { + {/* Error Alert */} {error ? ( - + {error} ) : null} - + {/* Sticky Bottom Actions */} +
+
diff --git a/src/features/nearle-admin/pages/PartnersPage.tsx b/src/features/nearle-admin/pages/PartnersPage.tsx index 9872142..7256897 100644 --- a/src/features/nearle-admin/pages/PartnersPage.tsx +++ b/src/features/nearle-admin/pages/PartnersPage.tsx @@ -1,29 +1,7 @@ /** * Rider partners — the companies that supply riders. * - * ── Why this page did not exist ───────────────────────────────────────────── - * - * `getpartners` has always been readable and nothing on the platform could - * create a partner, so the five that exist were inserted by hand — two are - * still called "Test". Meanwhile 125 of 200 merchants already carry a - * `partnerid`, and one partner supplies 48 shops while another supplies 63. The - * relationship the whole delivery side rests on was real, live and unmanaged. - * - * ── What onboarding a partner records ─────────────────────────────────────── - * - * Three things, and the last two are why the assign screen works at all: - * - * the district `partnerinfo.applocationid`, and `partnerlocations` beside - * it. Every rider query joins through that id, so it has to be - * a district Nearle actually services — see - * `tamilNaduDistricts.ts` for why all 38 are shown anyway. - * the merchant `tenants.partnerid`. This is what the assign screen reads to - * decide whether to offer a partner tab at all. - * the branch `tenantlocations.partnerid`. Which outlet they cover. - * - * A partner can also be attached to a merchant afterwards from that merchant's - * own page — see `StoreDetailPage` — which is the ordinary case of a shop - * changing partner without anybody re-onboarding the company. + * Clean, modern SaaS management workspace for delivery partners, fleet tracking, and region coverage. */ import { useMemo, useState } from 'react'; @@ -36,7 +14,12 @@ import { Table, type TableColumn } from '@astryxdesign/core/Table'; import { Text } from '@astryxdesign/core/Text'; import { TextInput } from '@astryxdesign/core/TextInput'; import { VStack } from '@astryxdesign/core/VStack'; -import { Bike, Plus, Truck } from 'lucide-react'; +import { + Bike, + MapPin, + Plus, + Truck, +} from 'lucide-react'; import { errorMessage } from '@/api/client'; import { partnersApi, type NewPartner, type Partner } from '@/api/deliveries'; import { tenantsApi } from '@/api/tenants'; @@ -70,15 +53,16 @@ interface PartnerRow extends Record { riders: number | null; } +type StatusFilter = 'all' | 'active' | 'inactive'; + export function PartnersPage() { const partners = useAllPartners(); const regions = useAppRegions(); const [editing, setEditing] = useState(null); - /** The partner whose riders are on screen, if any. */ const [ridersFor, setRidersFor] = useState(null); + const [statusFilter, setStatusFilter] = useState('all'); - /* The fleet size per partner, read alongside the directory. Without it the - Riders button is a door with nothing written on it. */ + /* The fleet size per partner, read alongside the directory. */ const riderCounts = usePartnerRiderCounts(partners.data.map((entry) => entry.partnerid)); const regionName = useMemo(() => { @@ -89,7 +73,7 @@ export function PartnersPage() { return map; }, [regions.data]); - const rows = useMemo( + const rawRows = useMemo( () => partners.data.map((partner) => ({ partnerid: partner.partnerid, @@ -103,34 +87,81 @@ export function PartnersPage() { [partners.data, regionName, riderCounts], ); - const paged = usePaged(rows); + // Filtered rows based on status filter + const filteredRows = useMemo(() => { + if (statusFilter === 'all') return rawRows; + return rawRows.filter((row) => + statusFilter === 'active' + ? row.status.toLowerCase() === 'active' + : row.status.toLowerCase() !== 'active', + ); + }, [rawRows, statusFilter]); + + const paged = usePaged(filteredRows); + + // KPI Calculations + const totals = useMemo(() => { + const totalPartners = rawRows.length; + const activePartners = rawRows.filter((r) => r.status.toLowerCase() === 'active').length; + const totalRiders = rawRows.reduce((sum, r) => sum + (r.riders ?? 0), 0); + const uniqueDistricts = new Set( + rawRows.map((r) => r.region).filter((reg) => reg && reg !== '—'), + ).size; + + return { totalPartners, activePartners, totalRiders, uniqueDistricts }; + }, [rawRows]); const columns: TableColumn[] = [ { key: 'partnername', header: 'Partner', - width: { type: 'proportional', value: 3 }, - renderCell: (row) => ( - - - {row.partnername} - - {row.companyname ? ( - - {row.companyname} - - ) : null} - - ), + width: { type: 'proportional', value: 3.5 }, + renderCell: (row) => { + const initial = (row.partnername || 'P')[0]?.toUpperCase() ?? 'P'; + return ( + +
+ {initial} +
+ + + {row.partnername} + + {row.companyname ? ( + + {row.companyname} + + ) : null} + + + ); + }, }, { key: 'region', header: 'Home region', width: { type: 'proportional', value: 2 }, renderCell: (row) => ( - - {row.region} - + + + + {row.region} + + ), }, { @@ -156,21 +187,13 @@ export function PartnersPage() { ), }, { - /* Both actions in ONE column with a header, rather than two unlabelled - ones. The riders button carries the fleet size, because "Riders" alone - asks you to open a drawer to learn whether there are any — and the - answer is the reason you would open it. */ key: 'actions', header: 'Fleet', align: 'end', - width: { type: 'pixel', value: 184 }, + width: { type: 'pixel', value: 190 }, renderCell: (row) => ( - +
+ } actions={ + ); +} + /* ── The form ─────────────────────────────────────────────────────────────── */ interface FormState { @@ -263,11 +354,8 @@ interface FormState { city: string; state: string; postcode: string; - /** The serviced district they work out of — an `app_location` id. */ applocationid: number; - /** The merchant this partner delivers for. */ tenantid: number; - /** Which branch of that merchant — written to `tenantlocations.partnerid`. */ locationid: number; } @@ -320,10 +408,6 @@ function PartnerDrawer({ partner, onClose }: { partner: Partner | null; onClose: }; } - /* ── The district ───────────────────────────────────────────────────────── - One per partner, chosen from all 38. Picking one Nearle does not run yet - opens it — the partner is sent with the NAME and the server writes the - `app_location` and `app_locationconfig` rows first. */ const [districtSearch, setDistrictSearch] = useState(''); const [district, setDistrict] = useState(() => partner?.city ?? ''); const districts = useMemo(() => districtOptions(regions.data ?? []), [regions.data]); @@ -333,17 +417,6 @@ function PartnerDrawer({ partner, onClose }: { partner: Partner | null; onClose: ); const chosenDistrict = districts.find((option) => option.name === district); - /* ── Who they deliver for ───────────────────────────────────────────────── - A partner supplies riders TO a merchant's branch. Both links are written: - `tenants.partnerid`, which is what the assign screen reads to decide - whether to offer a partner tab at all, and `tenantlocations.partnerid`, - which records the branch. Without the first the toggle never appears; - without the second nothing says which outlet they cover. - - Filtered to the district: a partner works one district, so a merchant in - another is not somebody they can deliver for. `getalltenants` returns a row - per BRANCH, and a branch's city is what places it — the tenant's own city - is the head office and can differ. */ const merchants = useTenants({ pageno: 1, pagesize: 200 }); const merchantOptions = useMemo(() => { const here = district.trim().toLowerCase(); @@ -380,11 +453,6 @@ function PartnerDrawer({ partner, onClose }: { partner: Partner | null; onClose: city: form.city.trim(), state: form.state.trim(), ...(form.postcode.trim() ? { postcode: Number(form.postcode) || 0 } : {}), - /* - The district, by id when Nearle already runs it and by NAME when it - does not. The name is what opens it — the server writes the region - rows before the partner, so all 38 are real choices rather than three. - */ applocationid: chosenDistrict?.applocationid ?? 0, ...(chosenDistrict && chosenDistrict.applocationid === 0 ? { district: chosenDistrict.name } @@ -394,15 +462,6 @@ function PartnerDrawer({ partner, onClose }: { partner: Partner | null; onClose: ? partnersApi.update({ ...body, partnerid: partner.partnerid }).then(() => partner.partnerid) : partnersApi.create(body).then((result) => result?.partnerid ?? 0); }, - /* - The placement is written after the partner exists, because it needs the - id the create hands back. - - Reported separately if it fails, and deliberately not rolled back: the - partner is real either way and re-onboarding them would refuse on the - duplicate contact number. Saying "the partner was created but could not be - placed" is recoverable — the drawer stays open on the same form. - */ onSuccess: async (partnerid) => { if (partnerid > 0 && form.tenantid > 0) { try { @@ -490,13 +549,6 @@ function PartnerDrawer({ partner, onClose }: { partner: Partner | null; onClose: onChange={set('registrationno')} /> - {/* ── District ────────────────────────────────────────────────────── - All 38 of Tamil Nadu's districts, searchable, with only the ones - Nearle services selectable. A partner placed in a district that has - no `app_location` row is a partner whose riders no query returns — - `getriders` filters on that id — so an unserviced district is shown - and refused rather than hidden, because "Erode is not open yet" is - an answer and a missing Erode is not. */} District @@ -510,10 +562,6 @@ function PartnerDrawer({ partner, onClose }: { partner: Partner | null; onClose: placeholder="Search all 38 districts…" hasClear /> - {/* A single-select list, not a cloud of chips: one partner works one - district, so this is a choice with one answer and it should read - like one. Running districts carry a tick, new ones say what will - happen — the difference is operational, not a restriction. */}
{shown.map((option) => { const running = isRunning(option); @@ -528,9 +576,6 @@ function PartnerDrawer({ partner, onClose }: { partner: Partner | null; onClose: data-chosen={chosen ? 'yes' : 'no'} onClick={() => { setDistrict(option.name); - // The merchant is district-scoped, so changing the district - // invalidates it — keeping it would attach a partner to a - // shop in a place they do not work. setForm((prev) => ({ ...prev, tenantid: 0, locationid: 0 })); }} > @@ -556,10 +601,6 @@ function PartnerDrawer({ partner, onClose }: { partner: Partner | null; onClose: - {/* ── Who they deliver for ────────────────────────────────────────── - The merchant, then the branch. Both links are written: the merchant - one is what the assign screen reads to decide whether to offer a - partner tab at all, and the branch one records which outlet. */} Delivers for @@ -569,8 +610,6 @@ function PartnerDrawer({ partner, onClose }: { partner: Partner | null; onClose: size="sm" value={form.tenantid ? String(form.tenantid) : ''} onChange={(value) => { - // A new merchant clears the branch with it — keeping it would - // leave another shop's outlet id attached to this partner. setForm((prev) => ({ ...prev, tenantid: Number(value) || 0, locationid: 0 })); }} options={merchantOptions} diff --git a/src/features/store-admin/BranchScope.dom.test.tsx b/src/features/store-admin/BranchScope.dom.test.tsx new file mode 100644 index 0000000..16ddd6e --- /dev/null +++ b/src/features/store-admin/BranchScope.dom.test.tsx @@ -0,0 +1,261 @@ +/** + * The branch filter, mounted for real. + * + * ── Why this test exists ──────────────────────────────────────────────────── + * + * The filter silently reset to "All branches" on every navigation, and nothing + * in the codebase could see it. The selection was derived straight from the + * `?branch=` search param, and every nav link in `AppShell` is a bare path + * (`to="/admin/sales"`), so React Router replaced the whole location and the + * param went with it. An absent param read as All. + * + * No pure-function test could catch that: the bug only exists in the + * interaction between the provider, the router and a link that drops the query + * string. So this mounts the provider inside a real router, navigates the way + * the shell does, and asserts the selection survives. + * + * The assertions below are mostly about what must NOT change: a nav click must + * not widen the operator's scope, and the URL must still be able to set it, or + * a shared link to one shop stops working. + */ +import assert from 'node:assert/strict'; +import { after, before, test } from 'node:test'; +import { JSDOM } from 'jsdom'; +import type { TenantLocation } from '@/api/types'; + +const BRANCHES = [ + { locationid: 1097, tenantid: 1087, locationname: 'Ragul stores', status: 'Active' }, + { locationid: 1135, tenantid: 1087, locationname: 'Ragul Selvapuram', status: 'Active' }, + { locationid: 1138, tenantid: 1087, locationname: 'Deborah Lara', status: 'Active' }, +] as TenantLocation[]; + +interface Harness { + /** What the provider currently reports. */ + read: () => { selected: number | null; scopedCount: number; search: string; branchCount: number; isLoading: boolean }; + /** Pick a branch through the provider's own `select`. */ + pick: (next: number | null) => Promise; + /** Navigate the way `AppShell` does — a bare path, no query string. */ + navigate: (to: string) => Promise; +} + +let mount: (opts: { url: string; pin?: number }) => Promise; + +/* Everything a mount creates, so it can be torn down. Without this the file + never exits: each mount leaves a live QueryClient with an active observer, + and node:test waits on the open handles until the suite times out — every + test passing and the FILE reported as failed. */ +const created: { unmount: () => void }[] = []; +let closeDom: () => void = () => {}; + +before(async () => { + const dom = new JSDOM('
', { + url: 'http://localhost/admin/console', + pretendToBeVisual: true, + }); + const win = dom.window as unknown as Record; + const g = globalThis as Record; + for (const key of [ + 'window', 'document', 'HTMLElement', 'Element', 'Node', 'SVGElement', 'Event', + 'getComputedStyle', 'requestAnimationFrame', 'cancelAnimationFrame', + 'localStorage', 'sessionStorage', 'MouseEvent', 'CustomEvent', + ]) g[key] = win[key]; + Object.defineProperty(globalThis, 'navigator', { value: win['navigator'], configurable: true }); + g['ResizeObserver'] = class { observe() {} unobserve() {} disconnect() {} }; + + /* No network. Without this the test is not hermetic and it silently was not: + `useTenantLocations` fetched the REAL tenant 1087 from fiesta.nearle.app and + replaced the three seeded branches with the six that shop actually has, so + two assertions failed against production data that has nothing to do with + what is under test — and would fail differently the day someone opens a + seventh outlet. */ + g['fetch'] = () => Promise.reject(new Error('no network in tests')); + + const React = await import('react'); + const { createRoot } = await import('react-dom/client'); + const { MemoryRouter, Routes, Route, useNavigate, useLocation } = await import('react-router-dom'); + const { BranchScopeProvider, useBranchScope } = await import('./BranchScope'); + + /* The branch list, without the network. `useTenantLocations` is a TanStack + hook; stubbing the module would mean stubbing the query client too, so the + provider is given a real one whose fetch resolves immediately. */ + const { QueryClient, QueryClientProvider } = await import('@tanstack/react-query'); + const { AuthContext } = await import('@/auth/context'); + + closeDom = () => dom.window.close(); + + mount = async ({ url, pin }) => { + const host = dom.window.document.createElement('div'); + dom.window.document.body.appendChild(host); + + const qc = new QueryClient({ + // No gcTime: 0 here. It collects the seeded entry before any component has + // subscribed to it, so the provider saw an empty branch list and every + // assertion read null. + defaultOptions: { queries: { retry: false, staleTime: Infinity } }, + }); + // Seed the cache under the key `useTenantLocations` reads, so the provider + // sees a loaded branch list on first render. + const { queryKeys } = await import('@/queries/keys'); + qc.setQueryData(queryKeys.tenants.locations(1087), BRANCHES); + + let api: { selected: number | null; scopedCount: number; search: string; branchCount: number; isLoading: boolean } | null = null; + let doSelect: ((n: number | null) => void) | null = null; + let doNavigate: ((to: string) => void) | null = null; + + function Probe() { + const scope = useBranchScope(); + const navigate = useNavigate(); + const location = useLocation(); + api = { + selected: scope.selected, + scopedCount: scope.scoped.length, + search: location.search, + branchCount: scope.branches.length, + isLoading: scope.isLoading, + }; + doSelect = scope.select; + doNavigate = (to) => navigate(to); + return null; + } + + const auth = { + user: { + userid: 1, role: 'store-admin', name: 'A', email: 'a@b.c', + roleid: 1, tenantid: 1087, locationid: 0, issuperadmin: false, + }, + isLoading: false, + signIn: async () => { throw new Error('not used'); }, + signOut: () => {}, + }; + + const root = createRoot(host); + created.push({ unmount: () => { root.unmount(); qc.unmount(); qc.clear(); } }); + root.render( + React.createElement( + QueryClientProvider, { client: qc }, + React.createElement( + AuthContext.Provider, { value: auth as never }, + React.createElement( + MemoryRouter, { initialEntries: [url] }, + React.createElement( + Routes, null, + React.createElement(Route, { + path: '/admin/*', + // children passed in the props object, not as a third argument: + // BranchScopeProvider declares children as required, and + // createElement's overload will not accept a null props object. + element: React.createElement(BranchScopeProvider, { + pin, + children: React.createElement(Probe), + }), + }), + ), + ), + ), + ), + ); + + const settle = () => new Promise((r) => setTimeout(r, 60)); + await settle(); + + return { + read: () => api!, + pick: async (next) => { doSelect!(next); await settle(); }, + navigate: async (to) => { doNavigate!(to); await settle(); }, + }; + }; +}); + +/* ── The bug ─────────────────────────────────────────────────────────────── */ + +// THE regression test. A nav click replaces the whole location, query string +// included; that must not change which shop the operator is looking at. +test('a nav click does not reset the chosen branch', async () => { + const h = await mount({ url: '/admin/console' }); + await h.pick(1135); + assert.equal(h.read().selected, 1135); + + await h.navigate('/admin/sales'); + assert.equal(h.read().selected, 1135, 'navigating to Sales widened the scope back to All'); + + await h.navigate('/admin/inventory'); + assert.equal(h.read().selected, 1135, 'the reset appeared on the second hop'); +}); + +// Scope is what pages actually read, so it has to narrow with the selection — +// a label that changes while every page keeps reading all six is the same bug +// wearing a different hat. +test('the scope pages read narrows to the one branch, and survives too', async () => { + const h = await mount({ url: '/admin/console' }); + assert.equal(h.read().scopedCount, 3, 'All branches should scope to every outlet'); + + await h.pick(1138); + assert.equal(h.read().scopedCount, 1); + + await h.navigate('/admin/reports'); + assert.equal(h.read().scopedCount, 1, 'scope widened on navigation'); +}); + +// The URL is a mirror, and it has to be put back after a nav click dropped it, +// or the address bar quietly disagrees with the control. +test('the param is written back after navigation drops it', async () => { + const h = await mount({ url: '/admin/console' }); + await h.pick(1135); + assert.match(h.read().search, /branch=1135/); + + await h.navigate('/admin/sales'); + assert.match(h.read().search, /branch=1135/, 'the URL lost the branch it is scoped to'); +}); + +/* ── What must keep working ──────────────────────────────────────────────── */ + +// The reason the param existed in the first place: a link to one shop's +// inventory has to survive being pasted into a chat. +test('an explicit param in the URL still sets the branch', async () => { + const h = await mount({ url: '/admin/inventory?branch=1138' }); + assert.equal(h.read().selected, 1138); +}); + +// The id is user-editable, so it is untrusted. An outlet this tenant does not +// own falls back to All rather than rendering an empty page. +test('an id this tenant does not own falls back to All', async () => { + const h = await mount({ url: '/admin/console?branch=999999' }); + assert.equal(h.read().selected, null); + assert.equal(h.read().scopedCount, 3); +}); + +test('branch=all is honoured, and switching back to All works', async () => { + const explicit = await mount({ url: '/admin/console?branch=all' }); + assert.equal(explicit.read().selected, null); + + const h = await mount({ url: '/admin/console' }); + await h.pick(1097); + assert.equal(h.read().selected, 1097); + await h.pick(null); + assert.equal(h.read().selected, null, 'could not get back to All branches'); + assert.doesNotMatch(h.read().search, /branch=/, 'the param should be removed under All'); +}); + +/* ── The pinned workspace ────────────────────────────────────────────────── */ + +// Fiesta authorises a POS or catalogue read on locationid alone, so for a store +// user the URL param is not a filter — it is the authorisation boundary. The +// pin has to win over anything in the address bar. +test('a pinned branch ignores the URL and cannot be selected away from', async () => { + const h = await mount({ url: '/admin/console?branch=1138', pin: 1097 }); + assert.equal(h.read().selected, 1097, 'the URL overrode the session pin'); + assert.equal(h.read().scopedCount, 1); + + await h.pick(1135); + assert.equal(h.read().selected, 1097, 'select() moved a pinned scope'); + + await h.navigate('/admin/sales'); + assert.equal(h.read().selected, 1097); +}); + +/* ── Teardown ────────────────────────────────────────────────────────────── */ + +after(() => { + for (const c of created) c.unmount(); + closeDom(); +}); diff --git a/src/features/store-admin/BranchScope.tsx b/src/features/store-admin/BranchScope.tsx index 610db5e..45654b1 100644 --- a/src/features/store-admin/BranchScope.tsx +++ b/src/features/store-admin/BranchScope.tsx @@ -1,4 +1,11 @@ -import { createContext, useContext, useMemo, type ReactNode } from 'react'; +import { + createContext, + useContext, + useEffect, + useMemo, + useState, + type ReactNode, +} from 'react'; import { useSearchParams } from 'react-router-dom'; import { useAuth } from '@/auth/AuthContext'; import { useTenantLocations } from '@/queries/hooks'; @@ -32,16 +39,18 @@ export interface BranchScopeValue { const BranchScopeContext = createContext(null); -/** The URL param. In the URL so a link to a page carries its branch with it. */ +/** The URL param. Mirrors the selection so a link carries its branch with it. */ const PARAM = 'branch'; /** * Which branch the Store Admin is looking at. * - * Held in the URL rather than in component state for two reasons. A link to - * "Inventory, Peelamedu" has to survive being pasted into a chat, and a reload - * during a shift must not silently drop the operator back to All branches while - * they are reading a number that only makes sense for one shop. + * Held in component state, mirrored to the URL. The mirror is what lets a link + * to "Inventory, Peelamedu" survive being pasted into a chat; holding the state + * here rather than reading it back out of the address bar is what stops a nav + * click — which replaces the query string — from resetting the operator to All + * branches mid-shift while they read a number that only means anything for one + * shop. See the long note on `chosen` below. * * The tenant, by contrast, comes from the session and is deliberately NOT in * the URL. Fiesta has no web auth, so tenant scoping is enforced by this client @@ -83,15 +92,68 @@ export function BranchScopeProvider({ ); const raw = params.get(PARAM); - const parsed = raw === null || raw === 'all' ? null : Number(raw); - // An id in the URL that this tenant does not own falls back to All rather - // than showing an empty page — the id is user-editable, so it is untrusted. - const selected = - pin !== undefined - ? pin - : parsed !== null && Number.isFinite(parsed) && branches.some((b) => b.locationid === parsed) - ? parsed - : null; + + /* + The selection lives here, and the URL only mirrors it. + + ── The bug this fixes ────────────────────────────────────────────────────── + + It used to be derived straight from the search param, with an absent param + meaning All branches. That is wrong, because absent does not mean "show me + everything" — it mostly means "you just clicked a nav tab". Every link in + `AppShell` is a bare path (`to="/admin/sales"`), so React Router replaces the + whole location, query string included, and the param is simply gone. The + branch filter therefore reset to All on every navigation: pick a shop on + Console, click Sales, and you were back to all six with nothing saying so. + Reproduced on tenant 1087 (Ragul Stores, 6 branches) — the label went from + "Ragul stores Selvapuram" back to "All branches (6)". + + Copying the whole search string onto the nav links would have fixed it and + broken something else: `InventoryPage` and the global catalogue keep their own + params, and those would then follow the operator from page to page. + + ── The rule ──────────────────────────────────────────────────────────────── + + A param that is PRESENT is obeyed, so a link to "Inventory, Peelamedu" still + survives being pasted into a chat, and editing the id in the address bar still + works. A param that is ABSENT changes nothing, so navigation cannot silently + widen the operator's scope. The effect below then writes the param back, which + is what keeps the URL honest after a nav click. + */ + const [chosen, setChosen] = useState(null); + + useEffect(() => { + if (pin !== undefined || raw === null) return; + if (raw === 'all') { + setChosen(null); + return; + } + const parsed = Number(raw); + if (Number.isFinite(parsed) && branches.some((b) => b.locationid === parsed)) { + setChosen(parsed); + } else if (!isLoading) { + // An id this tenant does not own falls back to All rather than showing an + // empty page — the id is user-editable, so it is untrusted. Guarded on + // `isLoading` because `branches` is empty until the fetch lands, and + // resetting then would throw away a perfectly good deep link. + setChosen(null); + } + }, [raw, branches, isLoading, pin]); + + const selected = pin !== undefined ? pin : chosen; + + // The URL follows the selection, including putting the param back after a nav + // click has dropped it. Built from the current params so a page's own query + // state is carried through untouched. + useEffect(() => { + if (pin !== undefined) return; + const want = selected === null ? null : String(selected); + if ((params.get(PARAM) ?? null) === want) return; + const next = new URLSearchParams(params); + if (want === null) next.delete(PARAM); + else next.set(PARAM, want); + setParams(next, { replace: true }); + }, [selected, params, setParams, pin]); const value = useMemo(() => { const current = selected === null ? undefined : branches.find((b) => b.locationid === selected); @@ -103,15 +165,14 @@ export function BranchScopeProvider({ current, isPinned: pin !== undefined, scoped: current ? [current] : branches, + // State only. The URL is updated by the mirroring effect above, so there + // is one place that writes the param rather than two that can disagree. select: (next) => { if (pin !== undefined) return; - const nextParams = new URLSearchParams(params); - if (next === null) nextParams.delete(PARAM); - else nextParams.set(PARAM, String(next)); - setParams(nextParams, { replace: true }); + setChosen(next); }, }; - }, [branches, isLoading, tenantid, selected, params, setParams, pin]); + }, [branches, isLoading, tenantid, selected, pin]); return {children}; } diff --git a/src/features/store-admin/pages/CountersPage.tsx b/src/features/store-admin/pages/CountersPage.tsx index c74d285..4439798 100644 --- a/src/features/store-admin/pages/CountersPage.tsx +++ b/src/features/store-admin/pages/CountersPage.tsx @@ -359,6 +359,40 @@ function reasonFor(row: CounterRow): string { return row.card?.problem.detail ?? 'Reporting late.'; } +function lastSale(row: CounterRow): string { + const at = row.status?.lastBillAt; + if (!at) return '—'; + return `${shortAge(Date.now() - at.getTime())} ago`; +} + + + {row.terminalId} + {reasonFor(row)} + + +
- {/* Its own storefront code, and it has to be its own: the payload is - {'{'}tenantid, locationid{'}'}, so a second outlet that reused the - first one's poster would send every shopper to the first shop. - `createtenantlocation` is used precisely because it returns the - created row — the new locationid is what this needs. */} - {mutation.data?.locationid ? ( - - + + + + + {form.locationname} is commissioned + + + + {Number(form.operatorid) > 0 + ? 'The person you chose now runs it and can sign in with their own account. The branch has no catalogue yet — products are published to it per store.' + : 'A login was created for the outlet itself, using the email above. The branch has no catalogue yet — products are published to it per store.'} + + + {mutation.data?.locationid ? ( + + + + ) : null} + +
); } + +/** Safe hook for branch scope */ +function useBranchScopeOptional() { + try { + return useBranchScope(); + } catch { + return null; + } +} diff --git a/src/features/store-user/pages/StoreCustomersPage.tsx b/src/features/store-user/pages/StoreCustomersPage.tsx index ca68faa..bfff324 100644 --- a/src/features/store-user/pages/StoreCustomersPage.tsx +++ b/src/features/store-user/pages/StoreCustomersPage.tsx @@ -4,10 +4,11 @@ import { HStack } from '@astryxdesign/core/HStack'; import { Text } from '@astryxdesign/core/Text'; import { TextInput } from '@astryxdesign/core/TextInput'; import { VStack } from '@astryxdesign/core/VStack'; -import { Mail, MapPin, Phone } from 'lucide-react'; +import { Mail, MapPin, Phone, Repeat, ShoppingBag, Users } from 'lucide-react'; import { customerLocality, customerName, type CustomerInfo } from '@/api/customers'; +import { KpiCard } from '@/components/KpiCard'; import { PageHeader } from '@/components/PageHeader'; -import { useCustomers } from '@/queries/hooks'; +import { useCustomers, useOrders } from '@/queries/hooks'; import { useBranchScope } from '@/features/store-admin/BranchScope'; import { Drawer } from '@/features/store-admin/Drawer'; import { count } from '@/features/store-admin/format'; @@ -16,39 +17,109 @@ import { TablePager } from '@/components/TablePager'; import { usePaged } from '@/components/usePaged'; /** - * Customers, for one shop. + * The shop's customers. * - * Read-only, and that is the whole surface. The old console's customer cards - * carried "Send promo SMS" and "Issue store credit" buttons; both were hidden - * from a store user by a `canManage` flag, and both were `toast()` calls with - * no endpoint behind them for anyone. There is nothing to port. + * ── Built around what this backend actually fills in ──────────────────────── * - * The list is genuinely branch-scoped by the server — `gettenantcustomers` - * joins `tenantcustomers` on `locationid` — which makes it one of the few reads - * here that would still be correct if someone bypassed this client. + * `gettenantcustomers` returns 35 columns and most of them are empty. Measured + * across 31 real customers at 7 shops on 2026-09-15: + * + * firstname, contactno, address, suburb, city, state, postcode 100% + * latitude / longitude 97% + * doorno 71% landmark 65% email 42% + * lastname, gender, dob, status, profileimage, devicetype … 0% + * + * Three things follow from that, and they are the whole design. + * + * Email had a column of its own and was blank in 58% of rows, so a quarter of + * the table's width carried more dashes than addresses. It now sits under the + * name, where it shows for the 42% who have one and costs nothing for the rest. + * + * `lastname` is empty for every customer on the platform, so `customerName` + * always resolves to the first name alone. No field, column or sort pretends + * otherwise. + * + * `status` is empty for every customer too — hence no Active/Inactive chip. A + * badge that reads the same on every row is decoration. + * + * ── Why orders are here ───────────────────────────────────────────────────── + * + * A contact list is not worth a page. What a shop wants to know is who comes + * back. Order rows carry `customerid` and it resolves: 290 of 300 orders at + * tenant 1087 matched a listed customer. `deliverycustomerid` is 0 everywhere + * and is NOT used. + * + * Counted over ALL time, deliberately NOT the header's date range. Branch 1097 + * has 500 orders in its history and none of them fall in Sep 1-15, so a + * range-scoped count renders a column of dashes and calls every customer new — + * the email column's mistake in a different costume. It also made "came back" + * swing with the date picker, when whether somebody is a repeat customer is not + * a property of the fortnight you happen to be looking at. + * + * Money is absent for a related reason: `ordervalue` is 0 on all 300 order rows + * at tenant 1087, so a spend column would read zero for an entire shop. + * + * The read is capped at 500 rows, as every order read in this console is, so + * for a very busy shop the count is a floor rather than a total. */ export function StoreCustomersPage() { const { current, tenantid } = useBranchScope(); const [keyword, setKeyword] = useState(''); const [open, setOpen] = useState(null); + const locationid = current?.locationid; + const customers = useCustomers( - tenantid && current ? { tenantid, locationid: current.locationid, pagesize: 200 } : undefined, + tenantid ? { tenantid, locationid, keyword: keyword || undefined, pagesize: 200 } : undefined, ); + /* No date range — see the note above. A customer's order history is the + question here, not this fortnight's trading. */ + const orders = useOrders( + tenantid + ? { tenantid, ...(locationid ? { locationid } : {}), pagesize: 500 } + : undefined, + ); + + /** Orders per customer, keyed on `customerid`. */ + const ordersByCustomer = useMemo(() => { + const tally = new Map(); + for (const row of orders.data ?? []) { + const id = Number(row.customerid ?? 0); + if (id > 0) tally.set(id, (tally.get(id) ?? 0) + 1); + } + return tally; + }, [orders.data]); + const rows = useMemo(() => { - const term = keyword.trim().toLowerCase(); const list = customers.data ?? []; - if (term === '') return list; + const needle = keyword.trim().toLowerCase(); + if (!needle) return list; return list.filter((customer) => `${customerName(customer)} ${customer.contactno ?? ''} ${customer.email ?? ''} ${customer.address ?? ''}` .toLowerCase() - .includes(term), + .includes(needle), ); }, [customers.data, keyword]); + const paged = usePaged(rows, { resetKey: keyword }); const withPhone = rows.filter((customer) => Boolean(customer.contactno)).length; + const repeat = rows.filter((customer) => (ordersByCustomer.get(customer.customerid) ?? 0) > 1) + .length; + + /* Where the shop's customers actually are. Suburb is filled on every row, so + this is a real answer rather than a mostly-empty one — and it is the one + fact on this page a shopkeeper cannot get from their own memory. */ + const topLocality = useMemo(() => { + const tally = new Map(); + for (const customer of rows) { + const area = (customer.suburb || customer.city || '').trim(); + if (area) tally.set(area, (tally.get(area) ?? 0) + 1); + } + const best = [...tally.entries()].sort((a, b) => b[1] - a[1])[0]; + return best ? { area: best[0], n: best[1] } : null; + }, [rows]); return ( @@ -69,6 +140,33 @@ export function StoreCustomersPage() { } /> +
+ } + /> + } + /> + 0 ? 'success' : 'neutral'} + icon={} + /> + } + /> +
+ {customers.isLoading ? ( ) : rows.length === 0 ? ( @@ -83,8 +181,9 @@ export function StoreCustomersPage() { ) : ( - {count(rows.length)} customer{rows.length === 1 ? '' : 's'} · {count(withPhone)} with a - phone number + {count(rows.length)} customer{rows.length === 1 ? '' : 's'} + {topLocality ? ` · ${count(topLocality.n)} in ${topLocality.area}` : ''} · orders counted + over their whole history
@@ -97,37 +196,57 @@ export function StoreCustomersPage() { }} > - - - - + + + + Customer Phone - Email Where + Orders {paged.rows.map((customer) => ( - setOpen(customer)} /> + setOpen(customer)} + /> ))}
- + )} - {open ? setOpen(null)} /> : null} + {open ? ( + setOpen(null)} + /> + ) : null} ); } -function Row({ customer, onOpen }: { customer: CustomerInfo; onOpen: () => void }) { +function Row({ + customer, + orders, + onOpen, +}: { + customer: CustomerInfo; + orders: number; + onOpen: () => void; +}) { const [isHovered, setIsHovered] = useState(false); return ( void - - {customerName(customer)} - + {/* The email rides under the name rather than holding a column of its + own: 42% of customers have one, so as a column it was mostly + dashes, and here its absence costs nothing at all. */} + + + {customerName(customer)} + + {customer.email ? ( + + {customer.email} + + ) : null} + {customer.contactno || '—'} - - - {customer.email || '—'} - - void {customerLocality(customer)} + {/* Right-aligned and tabular so the column can be scanned down for the + repeat buyers, which is the one question this page exists to answer. */} + + {orders === 0 ? ( + — + ) : ( + 1 ? 600 : 400, + color: orders > 1 ? 'var(--color-brand)' : 'var(--color-ink-2)', + }} + > + {count(orders)} + + )} + ); } function CustomerDrawer({ customer, + orders, + rangeLabel, onClose, }: { customer: CustomerInfo; + orders: number; + rangeLabel: string; onClose: () => void; }) { + /* doorno and landmark are filled on 71% and 65% of rows, so the assembled + address is genuinely better than the `address` column alone — which is why + both are included and neither is relied on. */ const address = [ customer.doorno, customer.address, @@ -214,17 +364,39 @@ function CustomerDrawer({ width={420} onClose={onClose} > - - - } label="Phone" value={customer.contactno || '—'} /> - } label="Email" value={customer.email || '—'} /> - } label="Address" value={address || '—'} /> - - + + + + } label="Phone" value={customer.contactno || '—'} /> + } label="Email" value={customer.email || '—'} /> + } label="Address" value={address || '—'} /> + + + + + } + label="Orders" + value={orders === 0 ? `None ${rangeLabel}` : `${count(orders)} ${rangeLabel}`} + /> + {/* 97% of customers have coordinates. Shown as a reference rather + than a map: this is a drawer, and a rider's app is where a pin + actually gets used. */} + } + label="Map ref" + value={ + customer.latitude && customer.longitude + ? `${customer.latitude}, ${customer.longitude}` + : 'Not placed' + } + /> + + + ); } - function Line({ icon, label, diff --git a/src/features/store-user/pages/StoreStaffPage.tsx b/src/features/store-user/pages/StoreStaffPage.tsx index 4276967..49697eb 100644 --- a/src/features/store-user/pages/StoreStaffPage.tsx +++ b/src/features/store-user/pages/StoreStaffPage.tsx @@ -1,10 +1,21 @@ import { useMemo, useState } from 'react'; import { Button } from '@astryxdesign/core/Button'; import { Card } from '@astryxdesign/core/Card'; +import { HStack } from '@astryxdesign/core/HStack'; import { Text } from '@astryxdesign/core/Text'; import { VStack } from '@astryxdesign/core/VStack'; -import { Plus } from 'lucide-react'; +import { + Eye, + EyeOff, + KeyRound, + Phone, + Plus, + ShieldCheck, + TriangleAlert, + Users, +} from 'lucide-react'; import type { PosUser } from '@/api/types'; +import { KpiCard } from '@/components/KpiCard'; import { PageHeader } from '@/components/PageHeader'; import { usePosRoles, usePosUsersByBranch, useStaffShifts } from '@/queries/hooks'; import { useBranchScope } from '@/features/store-admin/BranchScope'; @@ -15,45 +26,91 @@ import { TablePager } from '@/components/TablePager'; import { usePaged } from '@/components/usePaged'; /** - * Counter staff — the people who can open this shop's till. + * The people who work this shop's till. * - * Cashiers only. A supervisor runs the terminal and can create their own - * cashiers there, so issuing one is a merchant act; the backend takes the same - * line from the other side, where only roleid 7 may manage till users at all. + * ── Built around what `getposusers` actually returns ──────────────────────── * - * Two things are worth knowing before reading the table. A till account has no - * console login — the backend leaves roles 7 and 8 out of every web lookup, so - * a cashier signing in here is not refused, they are simply not found. And - * there is no delete: deactivating stops the login and keeps their bills, which - * a delete would orphan. + * Measured across all 14 live till accounts at 6 branches on 2026-09-15: + * + * user_id, full_name, first_name, authname, role_id, role, + * pin, has_password, location_id, status 100% + * contactno 50% + * shift_name / shift_start / shift_end 0% + * + * The old table spent a sixth of its width on a Shift column. Those fields + * exist on the model with `omitempty` and are filled for nobody — + * `getstaffshifts` answers `{"shifts": []}` at every branch — so the column + * read "Any" on every row of every shop. A shift now rides under the name, + * where it appears if one is ever set and takes no space while none is. + * + * `contactno` got the same treatment for the same reason at 50%. + * + * ── The field that was missing and mattered most ──────────────────────────── + * + * `authname` — the username the cashier types at the till + * (`cashier.1137@pos.nearle.in`). On every row, shown nowhere. A supervisor + * handing over a shift needs to read it out, and the only place to find it was + * the database. + * + * ── What `has_password` is NOT used for ───────────────────────────────────── + * + * It is on every row and it is always true, so nothing is built on it. + * + * This page briefly carried a "waiting on a password" card and an alert banner + * for the accounts that could not sign in. There are none, and there cannot be: + * `posUserRepository.go` generates one when the caller supplies nothing — + * `if password == "" { password = newPosPassword() }`, under a comment reading + * "Every till account gets a username and a password, cashiers included". All + * 14 live accounts return `has_password: true`. A banner that can never fire is + * the Shift column's mistake with a louder voice. + * + * ── Why inactive accounts are asked for ───────────────────────────────────── + * + * The drawer can switch a cashier off, and the listing hides inactive rows + * unless `include_inactive=true` is sent — which nothing sent. So switching + * somebody off made them vanish from this page entirely, with no trace and no + * way to switch them back. They now stay, marked Suspended. That is also what + * makes the Access column worth its width: with only active rows it could + * report one value forever. */ export function StoreStaffPage() { const { current, tenantid } = useBranchScope(); - const locationid = current?.locationid; - const [editing, setEditing] = useState(null); const [isAdding, setAdding] = useState(false); - const pages = usePosUsersByBranch(tenantid || undefined, locationid ? [locationid] : []); + const locationid = current?.locationid; + + // Inactive included on purpose — see the note above. + const pages = usePosUsersByBranch(tenantid || undefined, locationid ? [locationid] : [], true); const roles = usePosRoles(); const shifts = useStaffShifts(tenantid || undefined, locationid); const page = pages[0]; + + const everyone = useMemo(() => page?.data ?? [], [page?.data]); + const rows = useMemo(() => { - const list = page?.data ?? []; // Cashiers only. A supervisor may exist at this outlet — the merchant // issued them — but they are not this page's to manage, and showing a row - // whose Edit button would be refused is worse than not showing it. + // whose Edit button would be refused is worse than not showing it. Their + // presence is still worth knowing, so they are counted in the cards above + // rather than hidden entirely. // // No search box: a counter has a handful of staff, and a filter over four // rows is a control that only ever gets in the way. - return list + return everyone .filter((person) => (person.role ?? '').toLowerCase() !== 'supervisor') .sort((a, b) => (a.full_name ?? '').localeCompare(b.full_name ?? '')); - }, [page?.data]); + }, [everyone]); + const paged = usePaged(rows); - const active = rows.filter((person) => (person.status ?? '').toLowerCase() !== 'inactive').length; + const supervisors = everyone.filter( + (person) => (person.role ?? '').toLowerCase() === 'supervisor', + ).length; + const ready = rows.filter((person) => accessOf(person) === 'ready').length; + const suspended = rows.filter((person) => accessOf(person) === 'suspended').length; + const reachable = rows.filter((person) => Boolean(person.contactno)).length; return ( @@ -71,17 +128,76 @@ export function StoreStaffPage() { } /> +
+ } + /> + 0 && ready === rows.length ? 'success' : 'warning'} + icon={} + /> + {/* Contact details are filled on half the accounts, so this is the one + card here whose number genuinely moves between shops. */} + 0 && reachable === rows.length ? 'success' : 'warning'} + icon={} + /> + {/* A shop with no supervisor cannot do the things only a supervisor + can — imports, voids, settings — so its absence is worth flagging. */} + 0 ? 'neutral' : 'warning'} + icon={} + /> +
+ + {/* Said once rather than left as a chip on a row somebody has to notice. + A suspended cashier is refused by the till, and now that these rows + are asked for rather than hidden, the page can say so. */} + {suspended > 0 ? ( + + + + + + + + {suspended === 1 + ? 'One cashier is switched off' + : `${count(suspended)} cashiers are switched off`} + + + The till will refuse{' '} + {suspended === 1 ? 'them' : 'them'} until the account is switched back on. They are + listed below rather than hidden, so Edit can undo it. + + + + + ) : null} + {page?.isLoading ? ( ) : rows.length === 0 ? ( ) : ( - {count(rows.length)} cashier{rows.length === 1 ? '' : 's'} · {count(active)} active + {count(rows.length)} cashier{rows.length === 1 ? '' : 's'} · {count(ready)} ready to work + {supervisors > 0 + ? ` · ${count(supervisors)} supervisor${supervisors === 1 ? '' : 's'} at this outlet` + : ' · no supervisor at this outlet'}
@@ -94,51 +210,66 @@ export function StoreStaffPage() { }} > - + + - - - + - Name + Staff Role - Mobile - Shift - Status + Till sign-in + Access {paged.rows.map((person) => ( - {person.full_name || '—'} + + {person.full_name || '—'} + {/* Both ride here rather than holding columns: the + mobile is filled on half the accounts and a shift on + none of them. */} + {person.contactno ? ( + + {person.contactno} + + ) : null} + {person.shift_name ? ( + + {person.shift_name} + {person.shift_start + ? ` · ${person.shift_start}–${person.shift_end}` + : ''} + + ) : null} + - - - {person.contactno || '—'} - - - - {person.shift_name ? ( - <> - {person.shift_name} - - {person.shift_start}–{person.shift_end} - - - ) : ( - 'Any' - )} + + - +
- + )} @@ -176,6 +307,122 @@ export function StoreStaffPage() { ); } +/** Whether this person can work the counter right now. */ +type Access = 'ready' | 'suspended' | 'unknown'; + +function accessOf(person: PosUser): Access { + const status = (person.status ?? '').toLowerCase(); + // A missing status is UNKNOWN, not active. Defaulting it to active painted a + // green dot beside an account whose state the API never reported — which is + // the one case where somebody most needs to look. + if (!status) return 'unknown'; + if (!status.startsWith('active')) return 'suspended'; + // No has_password branch: it is true on every account and cannot be false — + // see the note at the top of the file. + return 'ready'; +} + +/** + * What the cashier types at the till, and the PIN that goes with it. + * + * One cell, because a handover reads them together — the supervisor says the + * username and the PIN in the same breath. `authname` is on every account and + * was previously visible nowhere in the console. + * + * The PIN is masked until asked for. `getposusers` returns it in clear + * (measured: "9090" on a live account), which is the backend's decision and not + * one this page can fix — but a four-digit credential sitting on screen behind a + * supervisor at a busy counter is avoidable, so it is avoided. + */ +function TillSignIn({ person }: { person: PosUser }) { + const [isShown, setShown] = useState(false); + const name = person.full_name || 'this cashier'; + + return ( + + + {person.authname || '—'} + + {person.pin ? ( + + + {isShown ? `PIN ${person.pin}` : 'PIN ••••'} + + + + ) : null} + + ); +} + +/** + * One chip for the one question: can this person open the till? + * + * `status` and `has_password` are both filled on every account, and separately + * neither answers it — an Active account with no password is refused by the + * till, and reading that off two columns is work the page should have done. + */ +function AccessChip({ person }: { person: PosUser }) { + const access = accessOf(person); + + const look: Record = { + ready: { label: 'Ready', colour: 'var(--color-success, #10b981)' }, + suspended: { label: person.status || 'Suspended', colour: 'var(--color-ink-4)' }, + unknown: { label: 'Unknown', colour: 'var(--color-ink-4)' }, + }; + const { label, colour } = look[access]; + + return ( + + + {label} + + ); +} + function Note({ title, body }: { title: string; body?: string }) { return ( @@ -218,32 +465,3 @@ function RoleChip({ label }: { label: string | undefined }) { ); } - -/** A dot and a word, not a pill — the merchant's page reads the same way. */ -function StatusChip({ status }: { status: string | undefined }) { - // A missing status is UNKNOWN, not active. Defaulting it to active painted a - // green dot and the word "Active" beside an account whose state the API never - // reported — which is the one case where somebody most needs to look. - const isActive = (status ?? '').toLowerCase().startsWith('active'); - const isUnknown = !status; - const colour = isUnknown - ? 'var(--color-ink-4)' - : isActive - ? 'var(--color-success, #10b981)' - : 'var(--color-ink-4)'; - return ( - - - {isUnknown ? 'Unknown' : isActive ? 'Active' : status} - - ); -} diff --git a/src/features/store-user/ui.tsx b/src/features/store-user/ui.tsx index 0f85fee..97e3f54 100644 --- a/src/features/store-user/ui.tsx +++ b/src/features/store-user/ui.tsx @@ -112,11 +112,25 @@ export function BarAction({ ); } -export function Th({ children }: { children?: ReactNode }) { +/** + * A column heading. + * + * `align` defaults to centre because every store-user table was built that + * way and changing them all is not this prop's job. Pass `"end"` for a column + * of numbers: a figure is read against the ones above and below it, and that + * only works when the digits line up on the right. + */ +export function Th({ + children, + align = 'center', +}: { + children?: ReactNode; + align?: 'start' | 'center' | 'end'; +}) { return ( ({ - queryKey: queryKeys.people.posUsers(tenantid ?? 0, locationid), - queryFn: () => posUsersApi.list(tenantid as number, locationid), + queryKey: [...queryKeys.people.posUsers(tenantid ?? 0, locationid), includeInactive] as const, + queryFn: () => posUsersApi.list(tenantid as number, locationid, includeInactive), enabled: Boolean(tenantid), ...stable, })),