From 1b8e5f83d780c9e6753b45e269207ddb6f0ed98e Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 12:57:40 +0200 Subject: [PATCH 1/3] fix(crm): tell the admin whether a customer's invitation actually went out Closes #1261. "Invite customer" is two calls: createDirect, then sendInvite. The mode wiring is right -- CustomerManagementPage passes mode='invite' and InlineCustomerCreate does call sendInvite -- so the reported symptom is not a missed branch. It is that nothing downstream distinguishes the outcomes. Three things could not be told apart afterwards: - The success toast claimed "portal invitation sent". sendInvite only queues an email_queue row; whether it was delivered is decided minutes later by the queue processor. The toast now says queued, and says what sends it. - When sendInvite failed, the warning read "Invitation email failed -- retry from the customer detail page", which sounds like the mail bounced. What actually remains is a PASSIVE customer with no invitation at all, so it says that instead. A 409 is now separated out: that means an invitation for the address is already open and the RE-invite was refused, so the customer is invited and telling them to retry sends them the wrong way. - The customers table rendered a customer whose invitation never went out identically to one the admin created as passive on purpose -- both showed only "Passive - admin only". Passive customers with an open invitation now show "Invitation pending", matched case-insensitively because customer_invitations lowercases the address while customer_accounts keeps what the admin typed. The invitations list was already being fetched for the tab; this only cross-references it. Active customers are left alone: they have portal access, so a stale invitation row for their address says nothing about them. 7 tests; the 5 that assert the new behaviour all fail before the change, and the 2 negative controls pass on both sides. --- .../components/admin/InlineCustomerCreate.tsx | 18 +- frontend/src/i18n/locales/de.json | 11 +- frontend/src/i18n/locales/en.json | 11 +- .../pages/admin/CustomerManagementPage.tsx | 36 +++- .../customerInvitationVisibility.test.tsx | 193 ++++++++++++++++++ 5 files changed, 254 insertions(+), 15 deletions(-) create mode 100644 frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx diff --git a/frontend/src/components/admin/InlineCustomerCreate.tsx b/frontend/src/components/admin/InlineCustomerCreate.tsx index 1bca4d5c..12ba71a1 100644 --- a/frontend/src/components/admin/InlineCustomerCreate.tsx +++ b/frontend/src/components/admin/InlineCustomerCreate.tsx @@ -16,6 +16,11 @@ * stays saved (passive) and a warning toast asks the admin to retry * from the customer detail page. * + * #1261 — the toasts here say what actually happened rather than what was + * intended. Two calls means three outcomes, and the middle one used to be + * indistinguishable from success: the invitation email is only QUEUED, so + * "invitation sent" was a claim this code cannot make. + * * Field set mirrors the customer detail page so admins see the same * shape regardless of where they're editing. */ @@ -182,10 +187,17 @@ export const InlineCustomerCreate: React.FC = ({ onCreated, onCancel, mod try { await customerAdminService.sendInvite(customer.id); toast.success(t('customers.create.savedActiveToast', - 'Customer created and portal invitation sent.')); + 'Customer created and portal invitation queued. It is sent by the email queue — check System health if it does not arrive.')); } catch (err: any) { - toast.warn(t('customers.create.inviteFailedToast', - 'Customer saved (passive). Invitation email failed — retry from the customer detail page.')); + // A 409 means an invitation for this address is already open, which + // is a different thing from the email failing: the customer is + // invited, and re-inviting is what was refused. + const alreadyInvited = err?.response?.status === 409; + toast.warn(alreadyInvited + ? 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.') + : t('customers.create.inviteFailedToast', + 'Customer saved as PASSIVE — no invitation went out. Retry "Send portal invitation" from the customer detail page.')); // eslint-disable-next-line no-console console.warn('sendInvite failed', err); } diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index a2c54391..9e88d8d5 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -4848,10 +4848,11 @@ "saveAsPassive": "Als passiven Kunden speichern", "saveAndInvite": "Speichern & Portal-Einladung senden", "savedPassiveToast": "Passiver Kunde erstellt.", - "savedActiveToast": "Kunde erstellt und Portal-Einladung gesendet.", - "inviteFailedToast": "Kunde gespeichert (passiv). Einladungs-E-Mail fehlgeschlagen — bitte aus dem Kundendetail erneut versuchen.", + "savedActiveToast": "Kunde angelegt und Portal-Einladung eingereiht. Der Versand läuft über die E-Mail-Warteschlange — prüfen Sie den Systemzustand, falls sie nicht ankommt.", + "inviteFailedToast": "Kunde als PASSIV gespeichert — es ging keine Einladung raus. Wiederholen Sie „Portal-Einladung senden“ auf der Kundendetailseite.", "emailRequired": "Eine gültige E-Mail-Adresse ist erforderlich.", - "nameRequired": "Geben Sie mindestens einen Firmennamen oder einen Ansprechpartner an." + "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." }, "passive": { "badge": "Passiv — nur Admin", @@ -5045,6 +5046,10 @@ "addedN_other": "{{count}} hinzugefügt", "removedN_one": "{{count}} entfernt", "removedN_other": "{{count}} entfernt" + }, + "invitePending": { + "badge": "Einladung offen", + "hint": "Einladung verschickt, noch nicht angenommen. Der Kunde bleibt passiv, bis er ein Passwort gesetzt hat." } }, "clients": { diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index f4762a49..dfec152b 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -4848,10 +4848,11 @@ "saveAsPassive": "Save as passive customer", "saveAndInvite": "Save & send portal invitation", "savedPassiveToast": "Passive customer created.", - "savedActiveToast": "Customer created and portal invitation sent.", - "inviteFailedToast": "Customer saved (passive). Invitation email failed — retry from the customer detail page.", + "savedActiveToast": "Customer created and portal invitation queued. It is sent by the email queue — check System health if it does not arrive.", + "inviteFailedToast": "Customer saved as PASSIVE — no invitation went out. Retry \"Send portal invitation\" from the customer detail page.", "emailRequired": "A valid email is required.", - "nameRequired": "Enter at least a company name or a contact name." + "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." }, "passive": { "badge": "Passive — admin only", @@ -5045,6 +5046,10 @@ "addedN_other": "{{count}} added", "removedN_one": "{{count}} removed", "removedN_other": "{{count}} removed" + }, + "invitePending": { + "badge": "Invitation pending", + "hint": "Invitation sent, not accepted yet. The customer stays passive until they set a password." } }, "clients": { diff --git a/frontend/src/pages/admin/CustomerManagementPage.tsx b/frontend/src/pages/admin/CustomerManagementPage.tsx index 6554e918..0b8d9fad 100644 --- a/frontend/src/pages/admin/CustomerManagementPage.tsx +++ b/frontend/src/pages/admin/CustomerManagementPage.tsx @@ -19,7 +19,7 @@ import { Link } from 'react-router-dom'; import { useQuery, useQueryClient } from '@tanstack/react-query'; import { useTranslation } from 'react-i18next'; import { - UserPlus, UserCog, Trash2, Search, X, AlertTriangle, CheckCircle2, Clock, + UserPlus, UserCog, Trash2, Search, X, AlertTriangle, CheckCircle2, Clock, MailCheck, } from 'lucide-react'; import { InlineCustomerCreate } from '../../components/admin/InlineCustomerCreate'; import { useMutationWithToast } from '../../hooks'; @@ -89,6 +89,18 @@ export const CustomerManagementPage: React.FC = () => { ); }, [customers, debouncedTerm]); + // #1261 — the "Invite customer" flow is createDirect-then-sendInvite, so a + // customer whose second call never landed is left looking identical to one + // the admin deliberately created as passive. Both showed only + // "Passive — admin only", and there was nothing on this row to tell them + // apart. Cross-reference the invitations we already fetch (the endpoint + // returns unaccepted, unexpired ones) so an invited customer says so. + const pendingInviteByEmail = useMemo(() => { + const map = new Map(); + for (const i of invitations || []) map.set(i.email.trim().toLowerCase(), i); + return map; + }, [invitations]); + const filteredInvitations = useMemo(() => { const list = invitations || []; if (!debouncedTerm.trim()) return list; @@ -239,11 +251,23 @@ export const CustomerManagementPage: React.FC = () => { status badge sits on its own line so a passive deactivated customer can still show both states clearly. */} - {c.isPassive && ( - - {t('customers.passive.badge', 'Passive — admin only')} - - )} + {c.isPassive && (() => { + const invite = pendingInviteByEmail.get(c.email.trim().toLowerCase()); + return invite ? ( + + + {t('customers.invitePending.badge', 'Invitation pending')} + + ) : ( + + {t('customers.passive.badge', 'Passive — admin only')} + + ); + })()} diff --git a/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx b/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx new file mode 100644 index 00000000..6e8a7036 --- /dev/null +++ b/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx @@ -0,0 +1,193 @@ +/** + * "Invite customer" is two calls: createDirect, then sendInvite. When the + * second one doesn't land, what's left behind is a passive customer — and the + * customers table rendered that identically to one the admin created as + * passive on purpose. Both showed "Passive — admin only", so the row could not + * answer the only question the admin had: did the invitation go out? (#1261) + */ +import React from 'react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter } from 'react-router-dom'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; + +vi.mock('react-i18next', async () => { + const actual = await vi.importActual('react-i18next'); + return { + ...actual, + useTranslation: () => ({ + t: (k: string, fb?: unknown) => (typeof fb === 'string' ? fb : k), + i18n: { language: 'en' }, + }), + }; +}); + +vi.mock('react-toastify', () => ({ + toast: { success: vi.fn(), warn: vi.fn(), error: vi.fn(), info: vi.fn() }, +})); +import { toast } from 'react-toastify'; +const toastSuccess = toast.success as ReturnType; +const toastWarn = toast.warn as ReturnType; + +vi.mock('../../../contexts/PermissionsContext', () => ({ + usePermissions: () => ({ + hasPermission: () => true, + hasAnyPermission: () => true, + hasAllPermissions: () => true, + isSuperAdmin: true, + isLoading: false, + }), +})); + +const list = vi.fn(); +const listInvitations = vi.fn(); +const createDirect = vi.fn(); +const sendInvite = vi.fn(); +vi.mock('../../../services/customerAdmin.service', () => ({ + customerAdminService: { + list: (...a: unknown[]) => list(...a), + listInvitations: (...a: unknown[]) => listInvitations(...a), + createDirect: (...a: unknown[]) => createDirect(...a), + sendInvite: (...a: unknown[]) => sendInvite(...a), + deactivate: vi.fn(), + cancelInvitation: vi.fn(), + }, +})); + +vi.mock('../../../services/businessProfile.service', () => ({ + businessProfileService: { get: vi.fn().mockResolvedValue({ profile: {} }) }, +})); + +import { CustomerManagementPage } from '../CustomerManagementPage'; +import { InlineCustomerCreate } from '../../../components/admin/InlineCustomerCreate'; + +const customer = (id: number, email: string, extra: Record = {}) => ({ + id, + email, + displayName: `Customer ${id}`, + firstName: null, + lastName: null, + salutation: null, + companyName: null, + isActive: true, + isPassive: true, + eventCount: 0, + lastLogin: null, + ...extra, +}); + +const invitation = (id: number, email: string) => ({ + id, + email, + expiresAt: '2026-12-01T00:00:00.000Z', + createdAt: '2026-09-01T00:00:00.000Z', + invitedBy: 'admin', +}); + +function renderWith(node: React.ReactElement) { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + {node} + , + ); +} + +beforeEach(() => { + vi.clearAllMocks(); + listInvitations.mockResolvedValue([]); + list.mockResolvedValue([]); +}); + +describe('CustomerManagementPage — invited vs merely passive (#1261)', () => { + it('marks a passive customer who has an open invitation', async () => { + list.mockResolvedValue([customer(1, 'invited@example.com')]); + listInvitations.mockResolvedValue([invitation(9, 'invited@example.com')]); + + renderWith(); + + expect(await screen.findByText('Invitation pending')).toBeTruthy(); + expect(screen.queryByText('Passive — admin only')).toBeNull(); + }); + + it('leaves a passive customer with no invitation reading as passive', async () => { + list.mockResolvedValue([customer(2, 'nobody@example.com')]); + listInvitations.mockResolvedValue([]); + + renderWith(); + + expect(await screen.findByText('Passive — admin only')).toBeTruthy(); + expect(screen.queryByText('Invitation pending')).toBeNull(); + }); + + it('matches the invitation regardless of address casing', async () => { + // customer_invitations stores the address lowercased; customer_accounts + // preserves what the admin typed. A case-sensitive match would show every + // mixed-case customer as never invited. + list.mockResolvedValue([customer(3, 'Mixed.Case@Example.com')]); + listInvitations.mockResolvedValue([invitation(11, 'mixed.case@example.com')]); + + renderWith(); + + expect(await screen.findByText('Invitation pending')).toBeTruthy(); + }); + + it('does not mark an active customer, who needs no invitation', async () => { + list.mockResolvedValue([customer(4, 'active@example.com', { isPassive: false })]); + listInvitations.mockResolvedValue([invitation(12, 'active@example.com')]); + + renderWith(); + + await screen.findByText('Customer 4'); + expect(screen.queryByText('Invitation pending')).toBeNull(); + }); +}); + +describe('InlineCustomerCreate — what the toast may claim (#1261)', () => { + const fill = async () => { + await userEvent.type(screen.getByPlaceholderText('name@example.com'), 'new@example.com'); + await userEvent.type(screen.getByLabelText(/Company name/i), 'Acme'); + }; + + it('says queued, not sent — this code cannot know it was delivered', async () => { + createDirect.mockResolvedValue({ id: 5, email: 'new@example.com' }); + sendInvite.mockResolvedValue({ id: 1, email: 'new@example.com', expiresAt: 'x' }); + + renderWith( {}} onCancel={() => {}} />); + await fill(); + await userEvent.click(screen.getByRole('button', { name: /Save & send portal invitation/i })); + + await waitFor(() => expect(toastSuccess).toHaveBeenCalled()); + const message = String(toastSuccess.mock.calls[0][0]); + expect(message).toMatch(/queued/i); + expect(message).not.toMatch(/invitation sent/i); + }); + + it('says the customer is PASSIVE when the invitation call failed', async () => { + createDirect.mockResolvedValue({ id: 6, email: 'new@example.com' }); + sendInvite.mockRejectedValue({ response: { status: 500 } }); + + renderWith( {}} 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/); + expect(toastSuccess).not.toHaveBeenCalled(); + }); + + it('distinguishes a 409 — the customer IS invited, the re-invite was refused', async () => { + createDirect.mockResolvedValue({ id: 7, email: 'new@example.com' }); + sendInvite.mockRejectedValue({ response: { status: 409 } }); + + renderWith( {}} 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(/already open/i); + expect(message).not.toMatch(/PASSIVE/); + }); +}); From 6bb12c6612ae104c2480515d16feccc77145b47c Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 14:16:24 +0200 Subject: [PATCH 2/3] fix(crm): stop the invitation UI claiming more than it can know Codex review round 1 on #1274. Three places where this branch replaced one overclaim with another. The badge tooltip said "Invitation sent". createInvitation inserts the customer_invitations row and only then queues the email, with no transaction around the pair, so an open invitation does not prove an email_queue row exists -- and even when it does, delivery is the queue processor's business minutes later. The tooltip now describes the invitation link itself and points at System health, which is the same distinction #1273 draws. Both conflicts are 409 and were being treated as one. Migration-era send-invite returns 409 with code CUSTOMER_ALREADY_ACTIVE when the customer already has a password, which happens if an open invitation for that address is accepted between createDirect and sendInvite. There is then no invitation row to cancel, so directing the admin to the Invitations tab points them at something that does not exist. The code is now read before the message is chosen. A dropped connection or a timeout rejects with no `response` at all, and the request may well have succeeded server-side. Saying "no invitation went out" there sends the admin into a retry that then 409s, which is the same trap the CUSTOMER_ALREADY_ACTIVE case sets. That branch is now explicitly unconfirmed and says where the answer is. 3 more tests, all 3 failing before this commit. --- .../components/admin/InlineCustomerCreate.tsx | 35 +++++++++--- frontend/src/i18n/locales/de.json | 6 ++- frontend/src/i18n/locales/en.json | 6 ++- .../pages/admin/CustomerManagementPage.tsx | 7 ++- .../customerInvitationVisibility.test.tsx | 54 +++++++++++++++++++ 5 files changed, 95 insertions(+), 13 deletions(-) diff --git a/frontend/src/components/admin/InlineCustomerCreate.tsx b/frontend/src/components/admin/InlineCustomerCreate.tsx index 12ba71a1..fc012449 100644 --- a/frontend/src/components/admin/InlineCustomerCreate.tsx +++ b/frontend/src/components/admin/InlineCustomerCreate.tsx @@ -189,15 +189,34 @@ export const InlineCustomerCreate: React.FC = ({ 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) { - // A 409 means an invitation for this address is already open, which - // is a different thing from the email failing: the customer is - // invited, and re-inviting is what was refused. - const alreadyInvited = err?.response?.status === 409; - toast.warn(alreadyInvited - ? 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.') - : t('customers.create.inviteFailedToast', + // Three different things reach this branch and they need three + // different answers. Only one of them means "no invitation exists". + 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. + 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. + 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. + 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.')); + } else { + toast.warn(t('customers.create.inviteFailedToast', 'Customer saved as PASSIVE — no invitation went out. Retry "Send portal invitation" from the customer detail page.')); + } // eslint-disable-next-line no-console console.warn('sendInvite failed', err); } diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 9e88d8d5..7926a35b 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -4852,7 +4852,9 @@ "inviteFailedToast": "Kunde als PASSIV gespeichert — es ging keine Einladung raus. Wiederholen Sie „Portal-Einladung senden“ auf der Kundendetailseite.", "emailRequired": "Eine gültige E-Mail-Adresse ist erforderlich.", "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." + "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." }, "passive": { "badge": "Passiv — nur Admin", @@ -5049,7 +5051,7 @@ }, "invitePending": { "badge": "Einladung offen", - "hint": "Einladung verschickt, noch nicht angenommen. Der Kunde bleibt passiv, bis er ein Passwort gesetzt hat." + "hint": "Für diese Adresse ist ein Einladungslink offen und noch nicht angenommen. Das ist kein Beleg dafür, dass die E-Mail angekommen ist — prüfen Sie den Systemzustand, falls der Kunde nichts erhalten hat." } }, "clients": { diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index dfec152b..c7fcab28 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -4852,7 +4852,9 @@ "inviteFailedToast": "Customer saved as PASSIVE — no invitation went out. Retry \"Send portal invitation\" from the customer detail page.", "emailRequired": "A valid email is required.", "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." + "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." }, "passive": { "badge": "Passive — admin only", @@ -5049,7 +5051,7 @@ }, "invitePending": { "badge": "Invitation pending", - "hint": "Invitation sent, not accepted yet. The customer stays passive until they set a password." + "hint": "An invitation link for this address is open and has not been accepted. That is not proof the email reached them — check System health if they say it never arrived." } }, "clients": { diff --git a/frontend/src/pages/admin/CustomerManagementPage.tsx b/frontend/src/pages/admin/CustomerManagementPage.tsx index 0b8d9fad..1f861b22 100644 --- a/frontend/src/pages/admin/CustomerManagementPage.tsx +++ b/frontend/src/pages/admin/CustomerManagementPage.tsx @@ -256,8 +256,13 @@ export const CustomerManagementPage: React.FC = () => { return invite ? ( {t('customers.invitePending.badge', 'Invitation pending')} diff --git a/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx b/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx index 6e8a7036..a52dcfae 100644 --- a/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx +++ b/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx @@ -29,6 +29,7 @@ vi.mock('react-toastify', () => ({ import { toast } from 'react-toastify'; const toastSuccess = toast.success as ReturnType; const toastWarn = toast.warn as ReturnType; +const toastInfo = toast.info as ReturnType; vi.mock('../../../contexts/PermissionsContext', () => ({ usePermissions: () => ({ @@ -190,4 +191,57 @@ describe('InlineCustomerCreate — what the toast may claim (#1261)', () => { expect(message).toMatch(/already open/i); expect(message).not.toMatch(/PASSIVE/); }); + + it('separates CUSTOMER_ALREADY_ACTIVE from a pending-invitation 409', async () => { + // Codex review round 1. Both conflicts are 409. This one means an open + // invitation was accepted between createDirect and sendInvite, so there is + // no invitation row to cancel — sending the admin to the Invitations tab + // points them at something that does not exist. + createDirect.mockResolvedValue({ id: 8, email: 'new@example.com' }); + sendInvite.mockRejectedValue({ + response: { status: 409, data: { code: 'CUSTOMER_ALREADY_ACTIVE' } }, + }); + + renderWith( {}} onCancel={() => {}} />); + await fill(); + await userEvent.click(screen.getByRole('button', { name: /Save & send portal invitation/i })); + + await waitFor(() => expect(toastInfo).toHaveBeenCalled()); + const message = String(toastInfo.mock.calls[0][0]); + expect(message).toMatch(/already has portal access/i); + expect(message).not.toMatch(/Invitations tab/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 + // "no invitation went out" sends the admin into a retry that then 409s. + createDirect.mockResolvedValue({ id: 9, email: 'new@example.com' }); + sendInvite.mockRejectedValue(new Error('Network Error')); + + renderWith( {}} 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('does not claim the invitation email was delivered', async () => { + // createInvitation inserts the customer_invitations row and only then + // queues the email, without a transaction — so an open invitation is not + // evidence that an email_queue row exists, let alone that it was sent. + list.mockResolvedValue([customer(10, 'invited@example.com')]); + listInvitations.mockResolvedValue([invitation(20, 'invited@example.com')]); + + renderWith(); + + const badge = await screen.findByText('Invitation pending'); + const tooltip = badge.getAttribute('title') || badge.closest('[title]')?.getAttribute('title') || ''; + expect(tooltip).not.toMatch(/invitation sent/i); + expect(tooltip).toMatch(/not proof/i); + }); }); From bc90b4db6209954ba354faf4dd1c76c1bcd6478d Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 14:52:03 +0200 Subject: [PATCH 3/3] 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. --- .../src/services/customerAccountsService.js | 15 ++++- .../components/admin/InlineCustomerCreate.tsx | 33 ++++++----- frontend/src/i18n/locales/de.json | 2 +- frontend/src/i18n/locales/en.json | 2 +- .../customerInvitationVisibility.test.tsx | 58 ++++++++++++++++++- 5 files changed, 90 insertions(+), 20 deletions(-) diff --git a/backend/src/services/customerAccountsService.js b/backend/src/services/customerAccountsService.js index e6a9567a..bcc0e6fe 100644 --- a/backend/src/services/customerAccountsService.js +++ b/backend/src/services/customerAccountsService.js @@ -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. diff --git a/frontend/src/components/admin/InlineCustomerCreate.tsx b/frontend/src/components/admin/InlineCustomerCreate.tsx index fc012449..7bc4f4f8 100644 --- a/frontend/src/components/admin/InlineCustomerCreate.tsx +++ b/frontend/src/components/admin/InlineCustomerCreate.tsx @@ -189,31 +189,36 @@ export const InlineCustomerCreate: React.FC = ({ 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.')); } diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 7926a35b..52202aa0 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -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", diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index c7fcab28..9a99a02f 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -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", diff --git a/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx b/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx index a52dcfae..ab36e30f 100644 --- a/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx +++ b/frontend/src/pages/admin/__tests__/customerInvitationVisibility.test.tsx @@ -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: 'new@example.com' }); - sendInvite.mockRejectedValue({ response: { status: 500 } }); + sendInvite.mockRejectedValue({ response: { status: 400, data: {} } }); renderWith( {}} 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: 'new@example.com' }); + sendInvite.mockRejectedValue({ response: { status: 500, data: {} } }); + + renderWith( {}} 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: 'new@example.com' }); + sendInvite.mockRejectedValue({ response: { status: 403, data: {} } }); + + renderWith( {}} 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: 'new@example.com' }); + sendInvite.mockRejectedValue({ + response: { status: 409, data: { code: 'CUSTOMER_ALREADY_ACTIVE', error: 'A customer account with this email already exists' } }, + }); + + renderWith( {}} 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