fix(crm): label the two invitation conflicts and stop guessing after a 5xx

Codex review round 2 on #1274. Both findings are the same shape as round 1:
a message that asserts more than the response supports, and sends the admin
somewhere that makes it worse.

A 5xx is no longer treated as a clean failure. createInvitation inserts the
customer_invitations row and only then queues the email, with no transaction
around the pair, so a 500 out of the queueing step leaves an OPEN invitation
behind. Telling the admin "no invitation went out, retry" there walks them into
a 409 that still queues nothing. 5xx now joins the no-response case as
unconfirmed; a plain 4xx keeps the clean-failure message, because that is the
one shape where nothing was written.

The two already-active conflicts now say so. The send-invite route returns
CUSTOMER_ALREADY_ACTIVE from its own check, but createInvitation rechecks
customer_accounts afterwards and threw a bare ConflictError -- code CONFLICT,
indistinguishable from the pending-invitation conflict. An invitation accepted
between the two checks therefore landed in the pending branch, telling the
admin to cancel an invitation that acceptance had just closed. Both conflicts
in the service now carry a code of their own, so the client reads the code
rather than inferring from the status.

4 more tests. One round-1 test changed with the behaviour it pinned: its 500
now asserts the unconfirmed message, and a new 400 case covers the clean
failure it used to stand for.
This commit is contained in:
Paul Nothaft
2026-09-02 14:52:03 +02:00
parent 6bb12c6612
commit bc90b4db62
5 changed files with 90 additions and 20 deletions
@@ -126,7 +126,16 @@ async function createInvitation({ email, invitedById, prefill }) {
.first();
if (existingCustomer && existingCustomer.password_hash) {
// Already-active customer with this email — duplicate, reject.
throw new ConflictError('A customer account with this email already exists', 'email');
//
// Carries the same code the send-invite route uses for its own
// already-active check (#1261). Both conflicts are 409 and both can mean
// "this address already has portal access" — the route reads the customer
// before this recheck runs, so an invitation accepted in between lands
// here instead — and a caller that cannot tell the two apart ends up
// telling the admin to cancel an invitation acceptance already closed.
const err = new ConflictError('A customer account with this email already exists', 'email');
err.code = 'CUSTOMER_ALREADY_ACTIVE';
throw err;
}
// If the existing customer is PASSIVE (password_hash IS NULL), this
// is the "promote to active" path: the admin clicked "Send portal
@@ -139,7 +148,9 @@ async function createInvitation({ email, invitedById, prefill }) {
.where('expires_at', '>', new Date())
.first();
if (pendingInvite) {
throw new ConflictError('A pending invitation already exists for this email', 'email');
const err = new ConflictError('A pending invitation already exists for this email', 'email');
err.code = 'INVITATION_ALREADY_PENDING';
throw err;
}
// 64-char hex = 32 bytes = 256 bits of entropy. Same as admin invites.
@@ -189,31 +189,36 @@ export const InlineCustomerCreate: React.FC<Props> = ({ onCreated, onCancel, mod
toast.success(t('customers.create.savedActiveToast',
'Customer created and portal invitation queued. It is sent by the email queue — check System health if it does not arrive.'));
} catch (err: any) {
// Three different things reach this branch and they need three
// different answers. Only one of them means "no invitation exists".
// Four different things reach this branch and they need four
// different answers. Only ONE of them means "nothing happened, go
// ahead and retry" — every other message that says so sends the
// admin into a retry that then 409s.
const status = err?.response?.status;
const code = err?.response?.data?.code;
if (status === 409 && code === 'CUSTOMER_ALREADY_ACTIVE') {
// An open invitation for this address was accepted between
// createDirect and sendInvite. The customer has portal access
// already, so there is nothing to cancel and nothing to resend.
if (code === 'CUSTOMER_ALREADY_ACTIVE') {
// This address already has portal access — either the route's own
// check, or an invitation accepted between it and the service's
// recheck. Nothing to cancel, nothing to resend.
toast.info(t('customers.create.inviteAlreadyActiveToast',
'Customer saved. This address already has portal access, so no invitation was needed.'));
} else if (status === 409) {
// An invitation for this address is already open and the RE-invite
// was refused. The customer IS invited; telling them to retry
// would send them the wrong way.
// was refused. The customer IS invited.
toast.warn(t('customers.create.inviteAlreadyPendingToast',
'Customer saved. An invitation for this address is already open — cancel it on the Invitations tab before sending a new one.'));
} else if (!err?.response) {
// No response at all: a dropped connection or a timeout. The
// request may well have succeeded server-side, so claiming it
// failed sends the admin into a retry that then 409s. Say what is
// actually known and point at where the answer is.
} else if (!err?.response || status >= 500) {
// No response (dropped connection, timeout) or a server error.
// Neither says the invitation was not created: createInvitation
// inserts the customer_invitations row and only then queues the
// email, without a transaction, so a 500 out of the queueing step
// leaves an open invitation behind. Asserting a clean failure here
// sends the admin into a retry that 409s and still queues nothing.
toast.warn(t('customers.create.inviteUnconfirmedToast',
'Customer saved, but the invitation could not be confirmed — the server did not answer. Check the Invitations tab before sending another.'));
'Customer saved, but the invitation could not be confirmed — check the Invitations tab before sending another.'));
} else {
// A 4xx that is not a conflict: validation, permissions, no such
// customer. The request was rejected before anything was written.
toast.warn(t('customers.create.inviteFailedToast',
'Customer saved as PASSIVE — no invitation went out. Retry "Send portal invitation" from the customer detail page.'));
}
+1 -1
View File
@@ -4854,7 +4854,7 @@
"nameRequired": "Geben Sie mindestens einen Firmennamen oder einen Ansprechpartner an.",
"inviteAlreadyPendingToast": "Kunde gespeichert. Für diese Adresse ist bereits eine Einladung offen — stornieren Sie sie im Tab „Einladungen“, bevor Sie eine neue senden.",
"inviteAlreadyActiveToast": "Kunde gespeichert. Diese Adresse hat bereits Portalzugang, eine Einladung war nicht nötig.",
"inviteUnconfirmedToast": "Kunde gespeichert, aber die Einladung ist unbestätigt — der Server hat nicht geantwortet. Prüfen Sie den Tab „Einladungen“, bevor Sie erneut senden."
"inviteUnconfirmedToast": "Kunde gespeichert, aber die Einladung ist unbestätigt — prüfen Sie den Tab „Einladungen“, bevor Sie erneut senden."
},
"passive": {
"badge": "Passiv — nur Admin",
+1 -1
View File
@@ -4854,7 +4854,7 @@
"nameRequired": "Enter at least a company name or a contact name.",
"inviteAlreadyPendingToast": "Customer saved. An invitation for this address is already open — cancel it on the Invitations tab before sending a new one.",
"inviteAlreadyActiveToast": "Customer saved. This address already has portal access, so no invitation was needed.",
"inviteUnconfirmedToast": "Customer saved, but the invitation could not be confirmed — the server did not answer. Check the Invitations tab before sending another."
"inviteUnconfirmedToast": "Customer saved, but the invitation could not be confirmed — check the Invitations tab before sending another."
},
"passive": {
"badge": "Passive — admin only",
@@ -165,9 +165,12 @@ describe('InlineCustomerCreate — what the toast may claim (#1261)', () => {
expect(message).not.toMatch(/invitation sent/i);
});
it('says the customer is PASSIVE when the invitation call failed', async () => {
it('says the customer is PASSIVE when the invitation was cleanly rejected', async () => {
// A 400 is the shape where "no invitation went out, retry" is true: the
// request never got as far as writing anything. Round 2 moved 5xx out of
// this branch, because a 500 can leave an invitation row behind.
createDirect.mockResolvedValue({ id: 6, email: '[email protected]' });
sendInvite.mockRejectedValue({ response: { status: 500 } });
sendInvite.mockRejectedValue({ response: { status: 400, data: {} } });
renderWith(<InlineCustomerCreate mode="invite" onCreated={() => {}} onCancel={() => {}} />);
await fill();
@@ -213,6 +216,57 @@ describe('InlineCustomerCreate — what the toast may claim (#1261)', () => {
expect(toastWarn).not.toHaveBeenCalled();
});
it('stays indeterminate on a 5xx — the invitation row may already exist', async () => {
// Codex review round 2. createInvitation inserts customer_invitations and
// only then queues the email, with no transaction around the pair, so a
// 500 out of the queueing step leaves an open invitation behind. Telling
// the admin to retry sends them into a 409 that still queues nothing.
createDirect.mockResolvedValue({ id: 11, email: '[email protected]' });
sendInvite.mockRejectedValue({ response: { status: 500, data: {} } });
renderWith(<InlineCustomerCreate mode="invite" onCreated={() => {}} onCancel={() => {}} />);
await fill();
await userEvent.click(screen.getByRole('button', { name: /Save & send portal invitation/i }));
await waitFor(() => expect(toastWarn).toHaveBeenCalled());
const message = String(toastWarn.mock.calls[0][0]);
expect(message).toMatch(/could not be confirmed/i);
expect(message).not.toMatch(/PASSIVE/);
});
it('still calls a plain 4xx a clean failure, where retrying is right', async () => {
// The one case where "nothing happened, go ahead and retry" is true: the
// request was rejected before anything was written.
createDirect.mockResolvedValue({ id: 12, email: '[email protected]' });
sendInvite.mockRejectedValue({ response: { status: 403, data: {} } });
renderWith(<InlineCustomerCreate mode="invite" onCreated={() => {}} onCancel={() => {}} />);
await fill();
await userEvent.click(screen.getByRole('button', { name: /Save & send portal invitation/i }));
await waitFor(() => expect(toastWarn).toHaveBeenCalled());
expect(String(toastWarn.mock.calls[0][0])).toMatch(/PASSIVE/);
});
it('reads the service-level already-active 409, not just the route-level one', async () => {
// Codex review round 2. createInvitation's own recheck throws
// ConflictError, which used to serialise as code CONFLICT — indistinguishable
// from the pending-invitation conflict, so the admin was told to cancel an
// invitation that acceptance had already closed. The service now labels it.
createDirect.mockResolvedValue({ id: 13, email: '[email protected]' });
sendInvite.mockRejectedValue({
response: { status: 409, data: { code: 'CUSTOMER_ALREADY_ACTIVE', error: 'A customer account with this email already exists' } },
});
renderWith(<InlineCustomerCreate mode="invite" onCreated={() => {}} onCancel={() => {}} />);
await fill();
await userEvent.click(screen.getByRole('button', { name: /Save & send portal invitation/i }));
await waitFor(() => expect(toastInfo).toHaveBeenCalled());
expect(String(toastInfo.mock.calls[0][0])).toMatch(/already has portal access/i);
expect(toastWarn).not.toHaveBeenCalled();
});
it('stays indeterminate when the server never answered', async () => {
// Codex review round 1. A dropped connection or timeout rejects with no
// `response`, but the request may have succeeded server-side. Declaring