Skip to content

Commit 29fa87a

Browse files
committed
fix(admin): surface a created user when only its reset email failed
1 parent 8dadbec commit 29fa87a

5 files changed

Lines changed: 84 additions & 45 deletions

File tree

apps/sim/app/workspace/[workspaceId]/settings/components/admin/add-user-modal.test.tsx

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ vi.mock('@/hooks/queries/admin-users', () => ({
115115
}))
116116

117117
import { AddUserModal } from '@/app/workspace/[workspaceId]/settings/components/admin/add-user-modal'
118-
import type { AddUserInput, AdminUser } from '@/hooks/queries/admin-users'
118+
import type { AddUserInput, AddUserResult, AdminUser } from '@/hooks/queries/admin-users'
119119

120120
const CREATED_USER: AdminUser = {
121121
id: 'user-1',
@@ -128,7 +128,7 @@ const CREATED_USER: AdminUser = {
128128

129129
let container: HTMLDivElement
130130
let root: Root
131-
let onCreated: ReturnType<typeof vi.fn<(user: AdminUser) => void>>
131+
let onCreated: ReturnType<typeof vi.fn<(user: AdminUser, resetEmailError?: string) => void>>
132132
let onOpenChange: ReturnType<typeof vi.fn<(open: boolean) => void>>
133133

134134
async function renderModal() {
@@ -203,8 +203,8 @@ describe('AddUserModal', () => {
203203

204204
it('creates a verified credential user and returns it to the admin view', async () => {
205205
mockMutate.mockImplementation(
206-
(_input: AddUserInput, options: { onSuccess: (user: AdminUser) => void }) => {
207-
options.onSuccess(CREATED_USER)
206+
(_input: AddUserInput, options: { onSuccess: (result: AddUserResult) => void }) => {
207+
options.onSuccess({ user: CREATED_USER })
208208
}
209209
)
210210
await renderModal()
@@ -226,7 +226,7 @@ describe('AddUserModal', () => {
226226
{ onSuccess: expect.any(Function), onSettled: expect.any(Function) }
227227
)
228228
expect(onOpenChange).toHaveBeenCalledWith(false)
229-
expect(onCreated).toHaveBeenCalledWith(CREATED_USER)
229+
expect(onCreated).toHaveBeenCalledWith(CREATED_USER, undefined)
230230
})
231231

232232
it('ignores repeated submissions before the pending state renders', async () => {
@@ -246,8 +246,8 @@ describe('AddUserModal', () => {
246246

247247
it('supports unverified accounts without exposing a platform-role control', async () => {
248248
mockMutate.mockImplementation(
249-
(_input: AddUserInput, options: { onSuccess: (user: AdminUser) => void }) => {
250-
options.onSuccess(CREATED_USER)
249+
(_input: AddUserInput, options: { onSuccess: (result: AddUserResult) => void }) => {
250+
options.onSuccess({ user: CREATED_USER })
251251
}
252252
)
253253
await renderModal()
@@ -269,8 +269,8 @@ describe('AddUserModal', () => {
269269

270270
it('drops the password field and submits without one when emailing a reset link', async () => {
271271
mockMutate.mockImplementation(
272-
(_input: AddUserInput, options: { onSuccess: (user: AdminUser) => void }) => {
273-
options.onSuccess(CREATED_USER)
272+
(_input: AddUserInput, options: { onSuccess: (result: AddUserResult) => void }) => {
273+
options.onSuccess({ user: CREATED_USER })
274274
}
275275
)
276276
await renderModal()
@@ -295,7 +295,7 @@ describe('AddUserModal', () => {
295295
},
296296
{ onSuccess: expect.any(Function), onSettled: expect.any(Function) }
297297
)
298-
expect(onCreated).toHaveBeenCalledWith(CREATED_USER)
298+
expect(onCreated).toHaveBeenCalledWith(CREATED_USER, undefined)
299299
})
300300

301301
it('keeps a typed password across a round trip through the reset-link flow', async () => {
@@ -308,6 +308,30 @@ describe('AddUserModal', () => {
308308
expect(buttonLabelled('Add user').disabled).toBe(false)
309309
})
310310

311+
it('still hands the user back when only its reset email failed', async () => {
312+
mockMutate.mockImplementation(
313+
(_input: AddUserInput, options: { onSuccess: (result: AddUserResult) => void }) => {
314+
options.onSuccess({ user: CREATED_USER, resetEmailError: 'SMTP unavailable' })
315+
}
316+
)
317+
await renderModal()
318+
await changeField('Name', 'Canary Writer')
319+
await changeField('Email', 'writer@synthetics.example.com')
320+
await changeField('Credentials', 'email')
321+
322+
await act(async () => {
323+
buttonLabelled('Add user').dispatchEvent(new MouseEvent('click', { bubbles: true }))
324+
await Promise.resolve()
325+
await Promise.resolve()
326+
})
327+
328+
// The account exists, so this closes like any other create — the host
329+
// surfaces the user (and the reason) rather than stranding the operator in
330+
// a modal whose form no longer maps to anything.
331+
expect(onOpenChange).toHaveBeenCalledWith(false)
332+
expect(onCreated).toHaveBeenCalledWith(CREATED_USER, 'SMTP unavailable')
333+
})
334+
311335
it('shows Better Auth failures without closing the modal', async () => {
312336
addUserMutation.current = {
313337
isPending: false,

apps/sim/app/workspace/[workspaceId]/settings/components/admin/add-user-modal.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,13 @@ type PasswordMode = (typeof PASSWORD_MODE_OPTIONS)[number]['value']
2828
interface AddUserModalProps {
2929
open: boolean
3030
onOpenChange: (open: boolean) => void
31-
onCreated: (user: AdminUser) => void
31+
/**
32+
* The account was created. `resetEmailError` is set when its provisioning
33+
* reset email could not be sent — the account still exists, so the host is
34+
* expected to surface the user (and report this) rather than treat it as a
35+
* failed create.
36+
*/
37+
onCreated: (user: AdminUser, resetEmailError?: string) => void
3238
}
3339

3440
export function AddUserModal({ open, onOpenChange, onCreated }: AddUserModalProps) {
@@ -86,10 +92,10 @@ export function AddUserModal({ open, onOpenChange, onCreated }: AddUserModalProp
8692
...(setsPassword ? { password } : {}),
8793
},
8894
{
89-
onSuccess: (user) => {
95+
onSuccess: ({ user, resetEmailError }) => {
9096
reset()
9197
onOpenChange(false)
92-
onCreated(user)
98+
onCreated(user, resetEmailError)
9399
},
94100
onSettled: () => {
95101
submissionInFlightRef.current = false

apps/sim/app/workspace/[workspaceId]/settings/components/admin/admin.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export function Admin() {
7777
const [impersonatingUserId, setImpersonatingUserId] = useState<string | null>(null)
7878
const [impersonationGuardError, setImpersonationGuardError] = useState<string | null>(null)
7979
const [isAddUserOpen, setIsAddUserOpen] = useState(false)
80+
const [provisionWarning, setProvisionWarning] = useState<string | null>(null)
8081

8182
const {
8283
data: usersData,
@@ -208,6 +209,7 @@ export function Admin() {
208209
variant='active'
209210
className='h-[28px] px-2 text-caption'
210211
onClick={() => {
212+
setProvisionWarning(null)
211213
sendPasswordReset.reset()
212214
sendPasswordReset.mutate({ userId: u.id, email: u.email })
213215
}}
@@ -441,6 +443,10 @@ export function Admin() {
441443
</p>
442444
)}
443445

446+
{provisionWarning && (
447+
<p className='text-[var(--text-error)] text-small'>{provisionWarning}</p>
448+
)}
449+
444450
{searchQuery.length > 0 && usersData ? (
445451
<>
446452
<div className='flex flex-col gap-0.5'>
@@ -500,9 +506,16 @@ export function Admin() {
500506
<AddUserModal
501507
open={isAddUserOpen}
502508
onOpenChange={setIsAddUserOpen}
503-
onCreated={(user) => {
509+
onCreated={(user, resetEmailError) => {
510+
// Search for the new user either way: when the reset email failed,
511+
// the recovery action named below lives on that user's row.
504512
setSearchInput(user.email)
505513
setAdminParams({ q: user.email, offset: null })
514+
setProvisionWarning(
515+
resetEmailError
516+
? `Created ${user.email}, but the password reset email failed to send (${resetEmailError}). Use Reset password on their row to try again.`
517+
: null
518+
)
506519
}}
507520
/>
508521
</SettingsPanel>

apps/sim/hooks/queries/admin-users.test.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -64,14 +64,7 @@ describe('addUser', () => {
6464
password: 'canary-password',
6565
emailVerified: true,
6666
})
67-
).resolves.toEqual({
68-
id: 'user-1',
69-
name: 'Canary Writer',
70-
email: 'writer@synthetics.example.com',
71-
role: 'user',
72-
banned: false,
73-
banReason: null,
74-
})
67+
).resolves.toEqual({ user: CREATED_USER })
7568
expect(mockCreateUser).toHaveBeenCalledWith({
7669
name: 'Canary Writer',
7770
email: 'writer@synthetics.example.com',
@@ -91,7 +84,7 @@ describe('addUser', () => {
9184
email: ' Writer@Synthetics.Example.com ',
9285
emailVerified: true,
9386
})
94-
).resolves.toEqual(CREATED_USER)
87+
).resolves.toEqual({ user: CREATED_USER })
9588

9689
expect(mockCreateUser).toHaveBeenCalledWith({
9790
name: 'Canary Writer',
@@ -124,17 +117,19 @@ describe('addUser', () => {
124117
expect(mockRequestJson).not.toHaveBeenCalled()
125118
})
126119

127-
it('names the row-level recovery path when the reset email fails to send', async () => {
120+
it('still returns the created user when only the reset email fails', async () => {
128121
mockCreateUser.mockResolvedValue({ data: { user: CREATED_USER }, error: null })
129122
mockRequestJson.mockRejectedValue(new Error('SMTP unavailable'))
130123

124+
// The account exists, so this must not surface as a failed create — the
125+
// caller needs the user to reach its row's own "Reset password" action.
131126
await expect(
132127
addUser({
133128
name: 'Canary Writer',
134129
email: 'writer@synthetics.example.com',
135130
emailVerified: true,
136131
})
137-
).rejects.toThrow(/Account created, but the password reset email failed to send/)
132+
).resolves.toEqual({ user: CREATED_USER, resetEmailError: 'SMTP unavailable' })
138133
})
139134

140135
it('surfaces resolved Better Auth errors', async () => {

apps/sim/hooks/queries/admin-users.ts

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,24 @@ function mapUser(u: {
6363
}
6464
}
6565

66+
export interface AddUserResult {
67+
user: AdminUser
68+
/**
69+
* Why the provisioning reset email could not be sent, when the account itself
70+
* was created. Deliberately not a thrown error: the account exists, so
71+
* re-submitting the form would only collide on the email. Callers finish the
72+
* create — surfacing the user so its row, and that row's "Reset password"
73+
* action, are reachable — and report this alongside.
74+
*/
75+
resetEmailError?: string
76+
}
77+
6678
export async function addUser({
6779
name,
6880
email,
6981
password,
7082
emailVerified,
71-
}: AddUserInput): Promise<AdminUser> {
83+
}: AddUserInput): Promise<AddUserResult> {
7284
const normalizedEmail = email.trim().toLowerCase()
7385
const { data, error } = await client.admin.createUser({
7486
name: name.trim(),
@@ -80,26 +92,15 @@ export async function addUser({
8092
if (error) throw new Error(error.message ?? 'Failed to add user')
8193
if (!data?.user) throw new Error('Better Auth did not return the created user')
8294

83-
if (!password) {
84-
try {
85-
await sendPasswordResetEmail(normalizedEmail)
86-
} catch (resetError) {
87-
/**
88-
* The account exists at this point, so re-submitting the form would only
89-
* collide on the email. Name the recovery path instead — the caller
90-
* surfaces this verbatim, and the new user is already in the list behind
91-
* the modal with its own "Reset password" action.
92-
*/
93-
throw new Error(
94-
`Account created, but the password reset email failed to send (${getErrorMessage(
95-
resetError,
96-
'unknown error'
97-
)}). Use "Reset password" on the user's row to try again.`
98-
)
99-
}
100-
}
95+
const user = mapUser(data.user)
96+
if (password) return { user }
10197

102-
return mapUser(data.user)
98+
try {
99+
await sendPasswordResetEmail(normalizedEmail)
100+
return { user }
101+
} catch (resetError) {
102+
return { user, resetEmailError: getErrorMessage(resetError, 'unknown error') }
103+
}
103104
}
104105

105106
/** Sends the standard password reset email, the same one the login page requests. */

0 commit comments

Comments
 (0)