Skip to content

Commit d7f8b69

Browse files
BillLeoutsakosvl346Bill Leoutsakos
andauthored
refactor(emcn): share invalid OTP slot treatment (#8246)
* Add shared invalid state to EMCN OTP slots * fix(ui): distinguish OTP verification from resend errors * test(ui): type OTP input queries as inputs * test(ui): import OTP test button prop type * test(ui): use shared logger mock in OTP regression --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local>
1 parent 5d7f002 commit d7f8b69

7 files changed

Lines changed: 327 additions & 56 deletions

File tree

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
/** @vitest-environment jsdom */
2+
import { act, type ButtonHTMLAttributes, type InputHTMLAttributes, type ReactNode } from 'react'
3+
import { createRoot, type Root } from 'react-dom/client'
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const mocks = vi.hoisted(() => ({
7+
chatRequest: vi.fn(),
8+
chatVerify: vi.fn(),
9+
fileRequest: vi.fn(),
10+
fileVerify: vi.fn(),
11+
refresh: vi.fn(),
12+
}))
13+
14+
vi.mock('@sim/emcn', () => ({
15+
cn: (...values: Array<string | false | null | undefined>) => values.filter(Boolean).join(' '),
16+
ChipInput: ({
17+
error: _error,
18+
size: _size,
19+
...props
20+
}: Omit<InputHTMLAttributes<HTMLInputElement>, 'size'> & {
21+
error?: boolean
22+
size?: string
23+
}) => <input {...props} />,
24+
Label: ({ children, htmlFor }: { children: ReactNode; htmlFor?: string }) => (
25+
<label htmlFor={htmlFor}>{children}</label>
26+
),
27+
InputOTP: ({
28+
children,
29+
value,
30+
onChange,
31+
'aria-invalid': invalid,
32+
}: {
33+
children: ReactNode
34+
value: string
35+
onChange: (value: string) => void
36+
'aria-invalid'?: boolean
37+
}) => (
38+
<div>
39+
<input
40+
data-testid='otp-code'
41+
aria-invalid={invalid}
42+
value={value}
43+
onChange={(event) => onChange(event.target.value)}
44+
/>
45+
{children}
46+
</div>
47+
),
48+
InputOTPGroup: ({ children }: { children: ReactNode }) => <div>{children}</div>,
49+
InputOTPSlot: ({ invalid, index }: { invalid?: boolean; index: number }) => (
50+
<span data-otp-slot={index} data-invalid={invalid} />
51+
),
52+
}))
53+
vi.mock('@/lib/messaging/email/validation', () => ({
54+
quickValidateEmail: () => ({ isValid: true }),
55+
}))
56+
vi.mock('@/app/(auth)/components', () => ({
57+
AuthSubmitButton: ({
58+
children,
59+
loading: _loading,
60+
loadingLabel: _loadingLabel,
61+
...props
62+
}: ButtonHTMLAttributes<HTMLButtonElement> & {
63+
loading?: boolean
64+
loadingLabel?: string
65+
}) => (
66+
<button type='submit' {...props}>
67+
{children}
68+
</button>
69+
),
70+
}))
71+
vi.mock('@/app/(auth)/components/auth-button-classes', () => ({ AUTH_TEXT_LINK: '' }))
72+
vi.mock('@/components/auth/public-auth-header', () => ({
73+
PublicAuthHeader: ({ title }: { title: string }) => <h1>{title}</h1>,
74+
}))
75+
vi.mock('@/app/f/[token]/public-file-auth-shell', () => ({
76+
PublicFileAuthShell: ({ children }: { children: ReactNode }) => <div>{children}</div>,
77+
}))
78+
vi.mock('next/navigation', () => ({ useRouter: () => ({ refresh: mocks.refresh }) }))
79+
vi.mock('@/hooks/queries/chats', () => ({
80+
useChatEmailOtpRequest: () => ({ mutateAsync: mocks.chatRequest, isPending: false }),
81+
useChatEmailOtpVerify: () => ({ mutateAsync: mocks.chatVerify, isPending: false }),
82+
}))
83+
vi.mock('@/hooks/queries/public-shares', () => ({
84+
usePublicFileOtpRequest: () => ({ mutateAsync: mocks.fileRequest, isPending: false }),
85+
usePublicFileOtpVerify: () => ({ mutateAsync: mocks.fileVerify, isPending: false }),
86+
}))
87+
88+
import EmailAuth from '@/app/(interfaces)/chat/components/auth/email/email-auth'
89+
import { PublicFileEmailAuth } from '@/app/f/[token]/public-file-email-auth'
90+
91+
let root: Root
92+
let container: HTMLDivElement
93+
94+
function changeInput(input: HTMLInputElement, value: string) {
95+
Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set?.call(input, value)
96+
input.dispatchEvent(new Event('input', { bubbles: true }))
97+
}
98+
99+
function button(label: string) {
100+
const found = Array.from(container.querySelectorAll('button')).find(
101+
(candidate) => candidate.textContent?.trim() === label
102+
)
103+
if (!found) throw new Error(`Missing button: ${label}`)
104+
return found
105+
}
106+
107+
function expectOtpInvalid(invalid: boolean) {
108+
expect(container.querySelector('[data-testid="otp-code"]')?.getAttribute('aria-invalid')).toBe(
109+
String(invalid)
110+
)
111+
const slots = container.querySelectorAll('[data-otp-slot]')
112+
expect(slots).toHaveLength(6)
113+
for (const slot of slots) expect(slot.getAttribute('data-invalid')).toBe(String(invalid))
114+
}
115+
116+
beforeEach(() => {
117+
vi.clearAllMocks()
118+
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
119+
mocks.chatRequest.mockResolvedValue({})
120+
mocks.chatVerify.mockResolvedValue({})
121+
mocks.fileRequest.mockResolvedValue({})
122+
mocks.fileVerify.mockResolvedValue({})
123+
container = document.createElement('div')
124+
document.body.appendChild(container)
125+
root = createRoot(container)
126+
})
127+
128+
afterEach(() => {
129+
act(() => root.unmount())
130+
container.remove()
131+
vi.unstubAllGlobals()
132+
})
133+
134+
describe('OTP error provenance', () => {
135+
it('keeps the chat code valid on resend failure and marks only a failed verification invalid', async () => {
136+
act(() => root.render(<EmailAuth identifier='chat-1' />))
137+
act(() =>
138+
changeInput(container.querySelector<HTMLInputElement>('#email')!, 'member@example.com')
139+
)
140+
await act(async () => button('Continue').click())
141+
142+
mocks.chatRequest.mockRejectedValueOnce(new Error('Delivery failed'))
143+
await act(async () => button('Resend').click())
144+
expect(container.textContent).toContain('Delivery failed')
145+
expectOtpInvalid(false)
146+
147+
mocks.chatVerify.mockRejectedValueOnce(new Error('Incorrect code'))
148+
await act(async () =>
149+
changeInput(container.querySelector<HTMLInputElement>('[data-testid="otp-code"]')!, '123456')
150+
)
151+
expect(container.textContent).toContain('Incorrect code')
152+
expectOtpInvalid(true)
153+
})
154+
155+
it('keeps the public-file code valid on resend failure and marks only a failed verification invalid', async () => {
156+
act(() => root.render(<PublicFileEmailAuth token='share-1' />))
157+
act(() =>
158+
changeInput(container.querySelector<HTMLInputElement>('#email')!, 'member@example.com')
159+
)
160+
await act(async () => button('Continue').click())
161+
162+
mocks.fileRequest.mockRejectedValueOnce(new Error('Delivery failed'))
163+
await act(async () => button('Resend').click())
164+
expect(container.textContent).toContain('Delivery failed')
165+
expectOtpInvalid(false)
166+
167+
mocks.fileVerify.mockRejectedValueOnce(new Error('Incorrect code'))
168+
await act(async () =>
169+
changeInput(container.querySelector<HTMLInputElement>('[data-testid="otp-code"]')!, '123456')
170+
)
171+
expect(container.textContent).toContain('Incorrect code')
172+
expectOtpInvalid(true)
173+
})
174+
})

‎apps/sim/app/(auth)/verify/verify-content.tsx‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use client'
22

33
import { Suspense, useEffect, useState } from 'react'
4-
import { cn, InputOTP, InputOTPGroup, InputOTPSlot } from '@sim/emcn'
4+
import { InputOTP, InputOTPGroup, InputOTPSlot } from '@sim/emcn'
55
import { POST_AUTH_REDIRECT_STORAGE_KEY } from '@/app/(auth)/auth-redirect'
66
import {
77
AuthFormMessage,
@@ -84,14 +84,16 @@ function VerificationForm({
8484
</p>
8585

8686
<div className='flex justify-center'>
87-
<InputOTP maxLength={6} value={otp} onChange={handleOtpChange} disabled={isLoading}>
87+
<InputOTP
88+
maxLength={6}
89+
value={otp}
90+
onChange={handleOtpChange}
91+
disabled={isLoading}
92+
aria-invalid={isInvalidOtp}
93+
>
8894
<InputOTPGroup>
8995
{OTP_SLOTS.map((index) => (
90-
<InputOTPSlot
91-
key={index}
92-
index={index}
93-
className={cn(isInvalidOtp && 'border-[var(--text-error)]')}
94-
/>
96+
<InputOTPSlot key={index} index={index} invalid={isInvalidOtp} />
9597
))}
9698
</InputOTPGroup>
9799
</InputOTP>

‎apps/sim/app/(interfaces)/chat/components/auth/email/email-auth.tsx‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,17 @@ const validateEmailField = (emailValue: string): string[] => {
3434

3535
export default function EmailAuth({ identifier }: EmailAuthProps) {
3636
const [email, setEmail] = useState('')
37-
const [authError, setAuthError] = useState<string | null>(null)
37+
const [authError, setAuthError] = useState<{
38+
kind: 'verification' | 'request'
39+
message: string
40+
} | null>(null)
3841
const [emailErrors, setEmailErrors] = useState<string[]>([])
3942
const hasEmailError = emailErrors.length > 0
4043

4144
const [showOtpVerification, setShowOtpVerification] = useState(false)
4245
const [otpValue, setOtpValue] = useState('')
4346
const [countdown, setCountdown] = useState(0)
47+
const isInvalidOtp = authError?.kind === 'verification'
4448

4549
const requestOtp = useChatEmailOtpRequest(identifier)
4650
const verifyOtp = useChatEmailOtpVerify(identifier)
@@ -89,7 +93,10 @@ export default function EmailAuth({ identifier }: EmailAuthProps) {
8993
await verifyOtp.mutateAsync({ email, otp: codeToVerify })
9094
} catch (error) {
9195
logger.error('Error verifying OTP:', error)
92-
setAuthError(toError(error).message || 'Invalid verification code')
96+
setAuthError({
97+
kind: 'verification',
98+
message: toError(error).message || 'Invalid verification code',
99+
})
93100
}
94101
}
95102

@@ -102,7 +109,10 @@ export default function EmailAuth({ identifier }: EmailAuthProps) {
102109
setOtpValue('')
103110
} catch (error) {
104111
logger.error('Error resending OTP:', error)
105-
setAuthError(toError(error).message || 'Failed to resend verification code')
112+
setAuthError({
113+
kind: 'request',
114+
message: toError(error).message || 'Failed to resend verification code',
115+
})
106116
setCountdown(0)
107117
}
108118
}
@@ -181,23 +191,20 @@ export default function EmailAuth({ identifier }: EmailAuthProps) {
181191
}
182192
}}
183193
disabled={verifyOtp.isPending}
184-
className={cn('gap-2', authError && 'otp-error')}
194+
className={cn('gap-2', isInvalidOtp && 'otp-error')}
195+
aria-invalid={isInvalidOtp}
185196
>
186197
<InputOTPGroup>
187198
{[0, 1, 2, 3, 4, 5].map((index) => (
188-
<InputOTPSlot
189-
key={index}
190-
index={index}
191-
className={cn(authError && 'border-[var(--text-error)]')}
192-
/>
199+
<InputOTPSlot key={index} index={index} invalid={isInvalidOtp} />
193200
))}
194201
</InputOTPGroup>
195202
</InputOTP>
196203
</div>
197204

198205
{authError && (
199206
<div className='mt-1 space-y-1 text-center text-[var(--text-error)] text-xs'>
200-
<p>{authError}</p>
207+
<p>{authError.message}</p>
201208
</div>
202209
)}
203210

‎apps/sim/app/f/[token]/public-file-email-auth.tsx‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,12 @@ export function PublicFileEmailAuth({ token }: PublicFileEmailAuthProps) {
2828
const [email, setEmail] = useState('')
2929
const [otp, setOtp] = useState('')
3030
const [sent, setSent] = useState(false)
31-
const [error, setError] = useState<string | null>(null)
31+
const [error, setError] = useState<{
32+
kind: 'verification' | 'request'
33+
message: string
34+
} | null>(null)
3235
const [countdown, setCountdown] = useState(0)
36+
const isInvalidOtp = error?.kind === 'verification'
3337

3438
useEffect(() => {
3539
if (countdown <= 0) return
@@ -39,7 +43,7 @@ export function PublicFileEmailAuth({ token }: PublicFileEmailAuthProps) {
3943

4044
const sendCode = async () => {
4145
if (!quickValidateEmail(normalizeEmail(email)).isValid) {
42-
setError('Please enter a valid email address.')
46+
setError({ kind: 'request', message: 'Please enter a valid email address.' })
4347
return
4448
}
4549
setError(null)
@@ -48,7 +52,10 @@ export function PublicFileEmailAuth({ token }: PublicFileEmailAuthProps) {
4852
setSent(true)
4953
setOtp('')
5054
} catch (err) {
51-
setError(getErrorMessage(err, 'Failed to send verification code'))
55+
setError({
56+
kind: 'request',
57+
message: getErrorMessage(err, 'Failed to send verification code'),
58+
})
5259
}
5360
}
5461

@@ -59,7 +66,7 @@ export function PublicFileEmailAuth({ token }: PublicFileEmailAuthProps) {
5966
await verifyOtp.mutateAsync({ email: normalizeEmail(email), otp: code })
6067
router.refresh()
6168
} catch (err) {
62-
setError(getErrorMessage(err, 'Invalid verification code'))
69+
setError({ kind: 'verification', message: getErrorMessage(err, 'Invalid verification code') })
6370
}
6471
}
6572

@@ -71,7 +78,10 @@ export function PublicFileEmailAuth({ token }: PublicFileEmailAuthProps) {
7178
setError(null)
7279
} catch (err) {
7380
setCountdown(0)
74-
setError(getErrorMessage(err, 'Failed to resend verification code'))
81+
setError({
82+
kind: 'request',
83+
message: getErrorMessage(err, 'Failed to resend verification code'),
84+
})
7585
}
7686
}
7787

@@ -107,7 +117,7 @@ export function PublicFileEmailAuth({ token }: PublicFileEmailAuthProps) {
107117
size='lg'
108118
error={Boolean(error)}
109119
/>
110-
{error ? <p className='text-[var(--text-error)] text-xs'>{error}</p> : null}
120+
{error ? <p className='text-[var(--text-error)] text-xs'>{error.message}</p> : null}
111121
</div>
112122

113123
<AuthSubmitButton
@@ -143,21 +153,20 @@ export function PublicFileEmailAuth({ token }: PublicFileEmailAuthProps) {
143153
if (value.length === 6) verifyCode(value)
144154
}}
145155
disabled={verifyOtp.isPending}
146-
className={cn('gap-2', error && 'otp-error')}
156+
className={cn('gap-2', isInvalidOtp && 'otp-error')}
157+
aria-invalid={isInvalidOtp}
147158
>
148159
<InputOTPGroup>
149160
{[0, 1, 2, 3, 4, 5].map((i) => (
150-
<InputOTPSlot
151-
key={i}
152-
index={i}
153-
className={cn(error && 'border-[var(--text-error)]')}
154-
/>
161+
<InputOTPSlot key={i} index={i} invalid={isInvalidOtp} />
155162
))}
156163
</InputOTPGroup>
157164
</InputOTP>
158165
</div>
159166

160-
{error ? <p className='text-center text-[var(--text-error)] text-xs'>{error}</p> : null}
167+
{error ? (
168+
<p className='text-center text-[var(--text-error)] text-xs'>{error.message}</p>
169+
) : null}
161170

162171
<AuthSubmitButton
163172
type='button'

‎packages/emcn/src/components/index.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,13 @@ export {
161161
type InfoCardProps,
162162
} from './info-card/info-card'
163163
export { Input, type InputProps } from './input/input'
164-
export { InputOTP, InputOTPGroup, InputOTPSeparator, InputOTPSlot } from './input-otp/input-otp'
164+
export {
165+
InputOTP,
166+
InputOTPGroup,
167+
InputOTPSeparator,
168+
InputOTPSlot,
169+
type InputOTPSlotProps,
170+
} from './input-otp/input-otp'
165171
export { Label } from './label/label'
166172
export { Lightbox, type LightboxProps } from './lightbox/lightbox'
167173
export { focusFirstTextInput, focusFirstTextInputIn } from './modal/auto-focus'

0 commit comments

Comments
 (0)