fix(accounting): let "bill to a customer" work with the portal off
CustomerAccountPicker returns null when customerPortal is off. That is right
for its original use -- the event form assigns portal logins that bypass the
gallery password -- but the Accounting flows reuse it as-is, so their required
"Client" field rendered a bare label with no input and the submit button could
never enable, with no explanation. Accounting-on + CRM-off is a valid,
UI-supported flag combination.
Took option (a): the bill-to-customer path does not depend on the portal.
POST /admin/expenses/:id/invoice is gated by requireExpenses + accounting.manage
only, and /admin/customers{,/search} are permission-gated rather than
flag-gated -- POST /admin/customers exists precisely to create passive,
portal-less customers "to attach a quote / invoice / gallery to". The
un-gated CustomerPicker used by the quote/bill/contract editors is the
precedent. (The comment claiming search 410s with the flag off was stale.)
Add portalAssignment (default true) so the gate and the portal-specific
label/help text apply only in event-assignment mode; the accounting call
sites render their own label. Event-form behaviour is unchanged.
Also fixes AccountingInboxPage's TriageModal, which has the identical
label-only failure on the rebill disposition from the same root cause --
outside the reported surface, but leaving it would half-fix the bug.
Refs testplan REPORT.md #7 (Part 8, S10).
This commit is contained in:
@@ -24,6 +24,21 @@ interface Props {
|
|||||||
value: SelectedCustomer[];
|
value: SelectedCustomer[];
|
||||||
onChange: (next: SelectedCustomer[]) => void;
|
onChange: (next: SelectedCustomer[]) => void;
|
||||||
disabled?: boolean;
|
disabled?: boolean;
|
||||||
|
/**
|
||||||
|
* Event-form mode (default): this picker IS part of the customer-portal
|
||||||
|
* feature — it assigns portal logins to a gallery, so it hides itself
|
||||||
|
* when `customerPortal` is off and explains the password bypass.
|
||||||
|
*
|
||||||
|
* Pass false where the picker only needs to identify an existing
|
||||||
|
* customer record (Accounting → "bill this to a client"). Those
|
||||||
|
* surfaces have their own gates (`accounting` / `expenses` /
|
||||||
|
* `incomingInvoices`) and their data path never touches the portal:
|
||||||
|
* /admin/customers{,/search} are permission-gated, not flag-gated, and
|
||||||
|
* POST /admin/customers explicitly creates passive, portal-less
|
||||||
|
* customers "to attach a quote / invoice / gallery to". Callers in this
|
||||||
|
* mode render their own field label.
|
||||||
|
*/
|
||||||
|
portalAssignment?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
const labelFor = (c: { email: string; displayName?: string | null; companyName?: string | null }) => {
|
const labelFor = (c: { email: string; displayName?: string | null; companyName?: string | null }) => {
|
||||||
@@ -31,7 +46,7 @@ const labelFor = (c: { email: string; displayName?: string | null; companyName?:
|
|||||||
return display ? `${display} · ${c.email}` : c.email;
|
return display ? `${display} · ${c.email}` : c.email;
|
||||||
};
|
};
|
||||||
|
|
||||||
export const CustomerAccountPicker: React.FC<Props> = ({ value, onChange, disabled }) => {
|
export const CustomerAccountPicker: React.FC<Props> = ({ value, onChange, disabled, portalAssignment = true }) => {
|
||||||
const { t } = useTranslation();
|
const { t } = useTranslation();
|
||||||
// Rules of Hooks: the feature-flag gate (early-return) is moved to
|
// Rules of Hooks: the feature-flag gate (early-return) is moved to
|
||||||
// the very end of this hook list (see end of function). The previous
|
// the very end of this hook list (see end of function). The previous
|
||||||
@@ -111,19 +126,24 @@ export const CustomerAccountPicker: React.FC<Props> = ({ value, onChange, disabl
|
|||||||
);
|
);
|
||||||
|
|
||||||
// Feature-flag gate (deliberately placed AFTER all hooks — see the
|
// Feature-flag gate (deliberately placed AFTER all hooks — see the
|
||||||
// long comment at the top of this component for why). When the
|
// long comment at the top of this component for why). Only applies to
|
||||||
// customerPortal flag is off the backend returns 410 on
|
// the event-assignment mode: hiding the UI there keeps the event form
|
||||||
// /admin/customers/search anyway, but hiding the UI here keeps the
|
// clean and removes the dangling "Customer accounts" label that would
|
||||||
// event form clean and removes the dangling "Customer accounts"
|
// otherwise appear above an empty placeholder. Non-portal call sites
|
||||||
// label that would otherwise appear above an empty placeholder.
|
// must NOT be gated — their required customer field would render as a
|
||||||
if (!customerPortalEnabled) return null;
|
// lone label with no input at all (QA S10).
|
||||||
|
if (portalAssignment && !customerPortalEnabled) return null;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div ref={containerRef} className="relative">
|
<div ref={containerRef} className="relative">
|
||||||
|
{portalAssignment && (
|
||||||
|
<>
|
||||||
<label className="block text-sm font-medium text-neutral-900 dark:text-neutral-100 mb-1">
|
<label className="block text-sm font-medium text-neutral-900 dark:text-neutral-100 mb-1">
|
||||||
{t('events.customerPicker.label', 'Customer accounts')}
|
{t('events.customerPicker.label', 'Customer accounts')}
|
||||||
</label>
|
</label>
|
||||||
<p className="text-xs text-neutral-500 dark:text-neutral-400 mb-2">{helpText}</p>
|
<p className="text-xs text-neutral-500 dark:text-neutral-400 mb-2">{helpText}</p>
|
||||||
|
</>
|
||||||
|
)}
|
||||||
|
|
||||||
{/* Selected chips */}
|
{/* Selected chips */}
|
||||||
{value.length > 0 && (
|
{value.length > 0 && (
|
||||||
|
|||||||
@@ -0,0 +1,62 @@
|
|||||||
|
/**
|
||||||
|
* The Accounting "bill this to a client" modals reuse CustomerAccountPicker,
|
||||||
|
* which used to hide itself whenever `customerPortal` was off — the default.
|
||||||
|
* The required field then rendered as a lone label with no input and the
|
||||||
|
* submit button could never enable (QA S10).
|
||||||
|
*
|
||||||
|
* Accounting/customerPortal is a supported flag combination: /admin/customers
|
||||||
|
* and /admin/customers/search are permission-gated, not flag-gated, and
|
||||||
|
* POST /admin/customers creates passive (portal-less) customers on purpose.
|
||||||
|
*/
|
||||||
|
import React from 'react';
|
||||||
|
import { describe, it, expect, vi } from 'vitest';
|
||||||
|
import { render, screen } from '@testing-library/react';
|
||||||
|
|
||||||
|
vi.mock('react-i18next', async () => {
|
||||||
|
const actual = await vi.importActual<typeof import('react-i18next')>('react-i18next');
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
useTranslation: () => ({ t: (k: string, fb?: unknown) => (typeof fb === 'string' ? fb : k) }),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
let portalEnabled = false;
|
||||||
|
vi.mock('../../../contexts/FeatureFlagsContext', () => ({
|
||||||
|
useFeatureEnabled: () => portalEnabled,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../../../services/customerAdmin.service', () => ({
|
||||||
|
customerAdminService: { search: vi.fn().mockResolvedValue([]) },
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { CustomerAccountPicker } from '../CustomerAccountPicker';
|
||||||
|
|
||||||
|
const SEARCH_PLACEHOLDER = 'Search by email, name, or company';
|
||||||
|
const PORTAL_LABEL = 'Customer accounts';
|
||||||
|
|
||||||
|
describe('CustomerAccountPicker portal gate (QA S10)', () => {
|
||||||
|
it('renders a usable search input with customerPortal off when portalAssignment=false', () => {
|
||||||
|
portalEnabled = false;
|
||||||
|
render(<CustomerAccountPicker portalAssignment={false} value={[]} onChange={() => {}} />);
|
||||||
|
|
||||||
|
expect(screen.getByPlaceholderText(SEARCH_PLACEHOLDER)).toBeInTheDocument();
|
||||||
|
// The caller renders its own field label ("Client *"), so the portal
|
||||||
|
// label + gallery-password help text stay out of the way.
|
||||||
|
expect(screen.queryByText(PORTAL_LABEL)).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still hides itself entirely on the event form when customerPortal is off', () => {
|
||||||
|
portalEnabled = false;
|
||||||
|
const { container } = render(<CustomerAccountPicker value={[]} onChange={() => {}} />);
|
||||||
|
|
||||||
|
expect(container).toBeEmptyDOMElement();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps the portal label + help text on the event form when customerPortal is on', () => {
|
||||||
|
portalEnabled = true;
|
||||||
|
render(<CustomerAccountPicker value={[]} onChange={() => {}} />);
|
||||||
|
|
||||||
|
expect(screen.getByText(PORTAL_LABEL)).toBeInTheDocument();
|
||||||
|
expect(screen.getByPlaceholderText(SEARCH_PLACEHOLDER)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -295,7 +295,11 @@ const TriageModal: React.FC<{ doc: InboundDocument; categories: ExpenseCategory[
|
|||||||
{BOOKING_DISPOSITIONS.includes(disposition) && (
|
{BOOKING_DISPOSITIONS.includes(disposition) && (
|
||||||
<div className="space-y-3 rounded-lg border border-neutral-200 dark:border-neutral-700 p-3">
|
<div className="space-y-3 rounded-lg border border-neutral-200 dark:border-neutral-700 p-3">
|
||||||
<div><label className={labelCls}>{t('accounting.inbox.field.customer', 'Client')} {disposition === 'rebill' ? '*' : ''}</label>
|
<div><label className={labelCls}>{t('accounting.inbox.field.customer', 'Client')} {disposition === 'rebill' ? '*' : ''}</label>
|
||||||
<CustomerAccountPicker value={customer.slice(0, 1)} onChange={(next) => setCustomer(next.slice(-1))} />
|
{/* portalAssignment={false} — same reason as the expenses
|
||||||
|
ledger: this is an `incomingInvoices` flow, not a
|
||||||
|
customer-portal one, and the rebill disposition's
|
||||||
|
required field would otherwise render label-only. */}
|
||||||
|
<CustomerAccountPicker portalAssignment={false} value={customer.slice(0, 1)} onChange={(next) => setCustomer(next.slice(-1))} />
|
||||||
{disposition === 'durchlaufend' && <p className="mt-1 text-xs text-neutral-500 dark:text-neutral-400">{t('accounting.inbox.field.passthroughCustomerHint', 'Optional — attach a client to re-bill this passthrough; leave empty to only book it to the event.')}</p>}
|
{disposition === 'durchlaufend' && <p className="mt-1 text-xs text-neutral-500 dark:text-neutral-400">{t('accounting.inbox.field.passthroughCustomerHint', 'Optional — attach a client to re-bill this passthrough; leave empty to only book it to the event.')}</p>}
|
||||||
</div>
|
</div>
|
||||||
{/* Markup is a re-bill concept only. A pass-through is invoiced
|
{/* Markup is a re-bill concept only. A pass-through is invoiced
|
||||||
|
|||||||
@@ -209,7 +209,11 @@ const InvoiceExpenseModal: React.FC<{ expense: Expense; onClose: () => void; onD
|
|||||||
</p>
|
</p>
|
||||||
<p className="text-xs text-neutral-500 dark:text-neutral-400">{t('accounting.ledger.invoiceHint', 'This creates a billable line on the client’s next scheduled invoice and locks the expense from further edits.')}</p>
|
<p className="text-xs text-neutral-500 dark:text-neutral-400">{t('accounting.ledger.invoiceHint', 'This creates a billable line on the client’s next scheduled invoice and locks the expense from further edits.')}</p>
|
||||||
<div><label className={labelCls}>{t('accounting.inbox.field.customer', 'Client')} *</label>
|
<div><label className={labelCls}>{t('accounting.inbox.field.customer', 'Client')} *</label>
|
||||||
<CustomerAccountPicker value={customer.slice(0, 1)} onChange={(next) => setCustomer(next.slice(-1))} />
|
{/* portalAssignment={false}: re-billing an expense is an Accounting
|
||||||
|
flow gated by `expenses`, not by the customer portal — without
|
||||||
|
this the required field renders a bare label and the submit
|
||||||
|
button can never enable (QA S10). */}
|
||||||
|
<CustomerAccountPicker portalAssignment={false} value={customer.slice(0, 1)} onChange={(next) => setCustomer(next.slice(-1))} />
|
||||||
</div>
|
</div>
|
||||||
<div><label className={labelCls}>{t('accounting.inbox.field.markup', 'Markup')}</label>
|
<div><label className={labelCls}>{t('accounting.inbox.field.markup', 'Markup')}</label>
|
||||||
<select value={markupType} onChange={(e) => setMarkupType(e.target.value as MarkupType)} className={selectCls}>
|
<select value={markupType} onChange={(e) => setMarkupType(e.target.value as MarkupType)} className={selectCls}>
|
||||||
|
|||||||
Reference in New Issue
Block a user