login changes
This commit is contained in:
43
src/features/catalogue/tenantCategories.test.ts
Normal file
43
src/features/catalogue/tenantCategories.test.ts
Normal file
@@ -0,0 +1,43 @@
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { test } from 'node:test';
|
||||
import { APP_BROWSE_CATEGORY, categoryOptionsFor } from './tenantCategories';
|
||||
|
||||
test('a new tenant is offered a category, so it can import its first product', () => {
|
||||
// The regression. `gettenantcategories` is derived from products the tenant
|
||||
// ALREADY has, so a shop with none gets []. CatalogueBrowser then refuses
|
||||
// every import ("no category to file into") and the only way to acquire a
|
||||
// category is to import a product. Measured on 2026-09-02: 37 of the 45 most
|
||||
// recent tenants had zero categories and could not add their first catalogue
|
||||
// product at all.
|
||||
const options = categoryOptionsFor([], false);
|
||||
assert.equal(options.length, 1);
|
||||
assert.equal(options[0]?.value, String(APP_BROWSE_CATEGORY));
|
||||
});
|
||||
|
||||
test('that first product is filed where the customer app will actually find it', () => {
|
||||
// Not merely "some category". Anything other than 2 imports a product no
|
||||
// shopper can see — which is the exact failure the refusal existed to
|
||||
// prevent, so a floor that got this wrong would be worse than the deadlock.
|
||||
assert.equal(categoryOptionsFor([], false)[0]?.value, '2');
|
||||
});
|
||||
|
||||
test("an established tenant's own categories are left alone", () => {
|
||||
const own = [
|
||||
{ categoryid: 2, categoryname: 'Grocery' },
|
||||
{ categoryid: 9, categoryname: 'Chilled' },
|
||||
];
|
||||
assert.deepEqual(categoryOptionsFor(own, false), [
|
||||
{ value: '2', label: 'Grocery' },
|
||||
{ value: '9', label: 'Chilled' },
|
||||
]);
|
||||
});
|
||||
|
||||
test('nothing is offered while the real answer is still in flight', () => {
|
||||
// Showing "General" mid-load and swapping it for the tenant's real list a
|
||||
// moment later moves where a product lands without the merchant touching it.
|
||||
assert.deepEqual(categoryOptionsFor(undefined, true), []);
|
||||
});
|
||||
|
||||
test('a missing list settles the same way as an empty one', () => {
|
||||
assert.equal(categoryOptionsFor(undefined, false).length, 1);
|
||||
});
|
||||
55
src/features/catalogue/tenantCategories.ts
Normal file
55
src/features/catalogue/tenantCategories.ts
Normal file
@@ -0,0 +1,55 @@
|
||||
import type { ProductCategory } from '@/api/types';
|
||||
|
||||
/**
|
||||
* The category the customer app browses.
|
||||
*
|
||||
* A constant because it is one on the app side too — its browse screen asks for
|
||||
* `categoryid: 2` — and because a product filed anywhere else is invisible to
|
||||
* shoppers rather than merely misfiled.
|
||||
*
|
||||
* Measured, not assumed: every tenant on the platform that has products reports
|
||||
* category 2 and nothing else, and category 2 carries the ten real retail
|
||||
* subcategories.
|
||||
*/
|
||||
export const APP_BROWSE_CATEGORY = 2;
|
||||
|
||||
export interface CategoryOption {
|
||||
value: string;
|
||||
label: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* A tenant's categories, with a floor for one that has none yet.
|
||||
*
|
||||
* `gettenantcategories` is synthesised from the categories a tenant's products
|
||||
* ALREADY use. That makes it right for an established shop and empty for a new
|
||||
* one — and a new shop is exactly who is importing their first product.
|
||||
*
|
||||
* Without a floor the two rules deadlock: the catalogue refuses to import
|
||||
* without a category (correctly — `categoryid` 0 is invisible to shoppers), and
|
||||
* the only way to acquire a category is to import a product. Measured on
|
||||
* 2026-09-02: tenant 1148 had 0 products and 0 categories and could not add its
|
||||
* first one from the catalogue at all.
|
||||
*
|
||||
* The floor is the category the app actually browses, so a product filed under
|
||||
* it is sellable rather than merely stored. Note what is deliberately NOT
|
||||
* offered: `getproductcategories`, the master table, returns `1001: vegetables`
|
||||
* for every tenant — it looks like a perfectly good category and is a trap,
|
||||
* because the app never asks for it.
|
||||
*
|
||||
* `isLoading` returns an empty list rather than the floor: offering it while the
|
||||
* real answer is still in flight would show a shop a category it does not use
|
||||
* and then swap it underneath them.
|
||||
*/
|
||||
export function categoryOptionsFor(
|
||||
categories: readonly ProductCategory[] | undefined,
|
||||
isLoading: boolean,
|
||||
): CategoryOption[] {
|
||||
const own = (categories ?? []).map((entry) => ({
|
||||
value: String(entry.categoryid),
|
||||
label: entry.categoryname,
|
||||
}));
|
||||
if (own.length > 0) return own;
|
||||
if (isLoading) return [];
|
||||
return [{ value: String(APP_BROWSE_CATEGORY), label: 'General (the category the app shows)' }];
|
||||
}
|
||||
@@ -23,16 +23,7 @@ import { Selector } from '@astryxdesign/core/Selector';
|
||||
import { Text } from '@astryxdesign/core/Text';
|
||||
import { VStack } from '@astryxdesign/core/VStack';
|
||||
import { useTenantCategories, useTenantLocations, useTenants } from '@/queries/hooks';
|
||||
|
||||
/**
|
||||
* The category the customer app browses.
|
||||
*
|
||||
* A constant because it is one on the app side too — its browse screen asks for
|
||||
* `categoryid: 2` — and because a product filed anywhere else is invisible to
|
||||
* shoppers rather than merely misfiled. Used only as a floor for a merchant with
|
||||
* no categories of their own; an established tenant's real list always wins.
|
||||
*/
|
||||
const APP_BROWSE_CATEGORY = 2;
|
||||
import { categoryOptionsFor } from '@/features/catalogue/tenantCategories';
|
||||
|
||||
/** Everything an upload needs before it can be turned into stock. */
|
||||
export interface ImportTarget {
|
||||
@@ -100,15 +91,10 @@ export function ImportScope({ value, onChange, isTenantFixed = false }: ImportSc
|
||||
* perfectly good category and is a trap — the app does not browse it, so
|
||||
* anything filed there is invisible to shoppers.
|
||||
*/
|
||||
const categoryOptions = useMemo(() => {
|
||||
const own = (categories.data ?? []).map((entry) => ({
|
||||
value: String(entry.categoryid),
|
||||
label: entry.categoryname,
|
||||
}));
|
||||
if (own.length > 0) return own;
|
||||
if (categories.isLoading) return [];
|
||||
return [{ value: String(APP_BROWSE_CATEGORY), label: 'General (the category the app shows)' }];
|
||||
}, [categories.data, categories.isLoading]);
|
||||
const categoryOptions = useMemo(
|
||||
() => categoryOptionsFor(categories.data, categories.isLoading),
|
||||
[categories.data, categories.isLoading],
|
||||
);
|
||||
|
||||
/**
|
||||
* A single branch is not a choice, so it is made rather than offered.
|
||||
|
||||
@@ -2,8 +2,9 @@ import { useEffect } from 'react';
|
||||
import { useLocation, useNavigate } from 'react-router-dom';
|
||||
import { useAuth } from '@/auth/AuthContext';
|
||||
import { useBranchScope } from '@/features/store-admin/BranchScope';
|
||||
import { useOwnTenant } from '@/queries/hooks';
|
||||
import { readOnboarding } from './onboardingState';
|
||||
import { useLocationProducts } from '@/queries/hooks';
|
||||
import { readOnboarding, writeOnboarding } from './onboardingState';
|
||||
import { shouldOfferSetup } from './shouldOfferSetup';
|
||||
|
||||
/**
|
||||
* Sends a first-time merchant to setup instead of the dashboard.
|
||||
@@ -11,41 +12,63 @@ import { readOnboarding } from './onboardingState';
|
||||
* Renders nothing — it is a decision, not a screen, and it makes that decision
|
||||
* once. Three rules keep it from becoming a trap:
|
||||
*
|
||||
* - **Only once.** After the first redirect, `started` is set and nobody is
|
||||
* ever sent again. Somebody who leaves setup has left it.
|
||||
* - **Only once.** The redirect records `started`, so nobody is ever sent
|
||||
* twice. Somebody who leaves setup has left it.
|
||||
* - **Only from the landing page.** A merchant who deep-links to Sales, or is
|
||||
* already reading Inventory, is not hauled away from what they opened.
|
||||
* - **Only when there is something to do.** A shop whose profile is already
|
||||
* filled in is not a first-time user, however new the account.
|
||||
* - **Only when there is something to do.** A shop already trading is not a
|
||||
* first-time user, however new the account.
|
||||
*
|
||||
* The redirect waits for the shop record. Judging "incomplete" while the query
|
||||
* is still in flight would redirect every merchant on every first paint.
|
||||
* ── Why "something to do" is measured in products ───────────────────────────
|
||||
*
|
||||
* This used to ask whether the shop had a name, an address and a phone number,
|
||||
* and so it never fired once — measured 2026-09-02 on tenants 1150, 1148 and
|
||||
* 1146, every one of them newly created and every one of them already carrying
|
||||
* all three. It could not have been otherwise: `createtenantuser` COLLECTS
|
||||
* exactly those fields, so Nearle Admin fills them in at the moment the tenant
|
||||
* is created, before the merchant has ever signed in. The test was reading the
|
||||
* onboarding form's own output and concluding the merchant had been onboarded.
|
||||
*
|
||||
* Products are the honest question. A shop with none cannot sell anything, so
|
||||
* it is a shop that still needs setup no matter how complete its profile looks;
|
||||
* a shop with stock on the shelf is working and must not be interrupted.
|
||||
*
|
||||
* The product list is awaited before deciding. Judging "incomplete" while it is
|
||||
* in flight would redirect every merchant on their first paint — including the
|
||||
* established ones this is written to leave alone.
|
||||
*/
|
||||
export function OnboardingGate() {
|
||||
const navigate = useNavigate();
|
||||
const { pathname } = useLocation();
|
||||
const { user } = useAuth();
|
||||
const { tenantid } = useBranchScope();
|
||||
const shop = useOwnTenant(tenantid || undefined);
|
||||
// Tenant-wide: a merchant with stock in any branch is trading. `allBranches`
|
||||
// because no single outlet is selected this early.
|
||||
const products = useLocationProducts(tenantid || undefined, undefined, 0, { allBranches: true });
|
||||
|
||||
useEffect(() => {
|
||||
if (!user?.userid || !tenantid || shop.isLoading || !shop.data) return;
|
||||
if (pathname !== '/admin/console') return;
|
||||
if (!user?.userid || !tenantid) return;
|
||||
|
||||
const state = readOnboarding(user.userid, tenantid);
|
||||
if (state.started || state.finished) return;
|
||||
|
||||
// "Set up" means the shop can be found and described. Products and stock
|
||||
// come later and are not a reason to interrupt somebody.
|
||||
const record = shop.data as unknown as Record<string, unknown>;
|
||||
const hasBasics =
|
||||
String(record['tenantname'] ?? '').trim() !== '' &&
|
||||
String(record['address'] ?? '').trim() !== '' &&
|
||||
String(record['primarycontact'] ?? '').trim() !== '';
|
||||
if (hasBasics) return;
|
||||
const isReady = !products.isLoading && !!products.data;
|
||||
if (!shouldOfferSetup({ pathname, isReady, productCount: products.data?.length ?? 0, state })) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Recorded BEFORE navigating, so the decision cannot be taken twice. Without
|
||||
// this the merchant is trapped: the welcome screen offers no way out, so
|
||||
// clicking Console to leave setup lands back on `/admin/console` and is
|
||||
// redirected here again, forever.
|
||||
writeOnboarding(user.userid, tenantid, { ...state, started: true });
|
||||
navigate('/admin/onboarding', { replace: true });
|
||||
}, [user?.userid, tenantid, shop.isLoading, shop.data, pathname, navigate]);
|
||||
}, [
|
||||
user?.userid,
|
||||
tenantid,
|
||||
products.isLoading,
|
||||
products.data,
|
||||
pathname,
|
||||
navigate,
|
||||
]);
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
65
src/features/onboarding/shouldOfferSetup.test.ts
Normal file
65
src/features/onboarding/shouldOfferSetup.test.ts
Normal file
@@ -0,0 +1,65 @@
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { test } from 'node:test';
|
||||
import { EMPTY_DELIVERY, EMPTY_STORE, type OnboardingState } from './onboardingState';
|
||||
import { shouldOfferSetup } from './shouldOfferSetup';
|
||||
|
||||
const FRESH: OnboardingState = {
|
||||
step: 'welcome',
|
||||
completed: [],
|
||||
skipped: [],
|
||||
finished: false,
|
||||
started: false,
|
||||
store: EMPTY_STORE,
|
||||
delivery: EMPTY_DELIVERY,
|
||||
};
|
||||
|
||||
const at = (over: Partial<Parameters<typeof shouldOfferSetup>[0]> = {}) =>
|
||||
shouldOfferSetup({
|
||||
pathname: '/admin/console',
|
||||
isReady: true,
|
||||
productCount: 0,
|
||||
state: FRESH,
|
||||
...over,
|
||||
});
|
||||
|
||||
test('a brand-new store admin is sent to setup on first login', () => {
|
||||
// The reported bug, and it survived a day of testing because the old rule
|
||||
// asked whether the shop had a name, address and phone — which
|
||||
// `createtenantuser` fills in at creation, so it was true of every new
|
||||
// tenant and the redirect never fired once.
|
||||
assert.equal(at(), true);
|
||||
});
|
||||
|
||||
test('a shop whose profile Nearle Admin already filled in is STILL offered setup', () => {
|
||||
// Tenants 1150, 1148 and 1146 were all newly created and all carried a name,
|
||||
// an address and a phone number. A complete profile is evidence about who
|
||||
// created the tenant, not about whether the merchant has been onboarded.
|
||||
assert.equal(at({ productCount: 0 }), true);
|
||||
});
|
||||
|
||||
test('a trading shop is never interrupted', () => {
|
||||
// R mart, 24 products. Stock on the shelf means somebody is working.
|
||||
assert.equal(at({ productCount: 24 }), false);
|
||||
});
|
||||
|
||||
test('the offer is made once, so declining it is possible', () => {
|
||||
// Without this the merchant is trapped: the welcome screen offers no way out,
|
||||
// so clicking Console to leave lands on `/admin/console` and bounces back.
|
||||
assert.equal(at({ state: { ...FRESH, started: true } }), false);
|
||||
});
|
||||
|
||||
test('somebody who finished setup is not sent through it again', () => {
|
||||
assert.equal(at({ state: { ...FRESH, finished: true } }), false);
|
||||
});
|
||||
|
||||
test('nobody is hauled away from the page they opened', () => {
|
||||
for (const pathname of ['/admin/sales', '/admin/inventory', '/admin/reports']) {
|
||||
assert.equal(at({ pathname }), false, pathname);
|
||||
}
|
||||
});
|
||||
|
||||
test('no decision is taken while the shop and its products are still loading', () => {
|
||||
// An empty product list mid-flight looks exactly like a shop with no stock,
|
||||
// which would redirect every established merchant on their first paint.
|
||||
assert.equal(at({ isReady: false }), false);
|
||||
});
|
||||
41
src/features/onboarding/shouldOfferSetup.ts
Normal file
41
src/features/onboarding/shouldOfferSetup.ts
Normal file
@@ -0,0 +1,41 @@
|
||||
import type { OnboardingState } from './onboardingState';
|
||||
|
||||
/**
|
||||
* Whether to send this merchant to setup rather than the dashboard.
|
||||
*
|
||||
* Pulled out of `OnboardingGate` so it can be tested without a router, a query
|
||||
* client or a DOM — the previous version's bug was a single wrong boolean, and
|
||||
* a wrong boolean inside a `useEffect` is invisible to every test in this repo.
|
||||
*/
|
||||
export interface SetupDecision {
|
||||
/** Where the merchant is. The offer is only made from the landing page. */
|
||||
pathname: string;
|
||||
/** False while either query is still in flight — decide on facts, not blanks. */
|
||||
isReady: boolean;
|
||||
/** How many products the tenant has, across every branch. */
|
||||
productCount: number;
|
||||
state: OnboardingState;
|
||||
}
|
||||
|
||||
export function shouldOfferSetup({
|
||||
pathname,
|
||||
isReady,
|
||||
productCount,
|
||||
state,
|
||||
}: SetupDecision): boolean {
|
||||
// A merchant who opened Sales or Inventory directly is working. Only somebody
|
||||
// who arrived at the dashboard is between things.
|
||||
if (pathname !== '/admin/console') return false;
|
||||
|
||||
// Redirecting on incomplete data would catch established shops on first paint.
|
||||
if (!isReady) return false;
|
||||
|
||||
// Offered once. `started` is recorded by the redirect itself, not by the
|
||||
// merchant accepting it, so declining is possible at all.
|
||||
if (state.started || state.finished) return false;
|
||||
|
||||
// The honest test of a first-time user. NOT the shop's name, address and
|
||||
// phone: `createtenantuser` collects those, so Nearle Admin fills them in
|
||||
// before the merchant ever signs in and they are true of every new tenant.
|
||||
return productCount === 0;
|
||||
}
|
||||
@@ -2,6 +2,7 @@ import { useMemo } from 'react';
|
||||
import { useTenantCategories } from '@/queries/hooks';
|
||||
import { CatalogueBrowser } from '@/features/catalogue/CatalogueBrowser';
|
||||
import { useBranchScope } from './BranchScope';
|
||||
import { categoryOptionsFor } from '@/features/catalogue/tenantCategories';
|
||||
|
||||
/**
|
||||
* The Store Admin's half of the catalogue: browse it, add to your own products.
|
||||
@@ -35,16 +36,16 @@ export function CataloguePanel() {
|
||||
* return under any category it asks for. It was not an opt-out from filing a
|
||||
* product; it was an opt-out from selling it, offered as the default.
|
||||
*
|
||||
* A tenant with no categories now gets an explicit refusal from
|
||||
* `CatalogueBrowser` rather than a silent 0.
|
||||
* A tenant with no categories of its own is not refused either, which was
|
||||
* the overcorrection: the refusal was right about `categoryid` 0 and wrong
|
||||
* about who it caught. `gettenantcategories` is derived from products a
|
||||
* tenant already has, so the shops it blocked were new ones importing their
|
||||
* first product — 37 of the 45 most recent tenants. See
|
||||
* `categoryOptionsFor` for the floor that resolves it.
|
||||
*/
|
||||
const categoryOptions = useMemo(
|
||||
() =>
|
||||
(categories.data ?? []).map((entry) => ({
|
||||
value: String(entry.categoryid),
|
||||
label: entry.categoryname,
|
||||
})),
|
||||
[categories.data],
|
||||
() => categoryOptionsFor(categories.data, categories.isLoading),
|
||||
[categories.data, categories.isLoading],
|
||||
);
|
||||
|
||||
return (
|
||||
|
||||
Reference in New Issue
Block a user