Skip to content

Commit 2105ba3

Browse files
Test for onboarding after GitHub OAuth (#680)
* chore: gituhb oauth onboarding test * chore: fix typos * chore: rm duplicate test * chore: add 'title' to Icon component * chore: isolate github oauth test * chore: rm NB abbreviation in comment * chore: do not export 'getGitHubUsers' helper * chore: add 'deleteGitHubUser' helper, use for e2e test cleanup * chore: verify the user is not in system before signup * chore: update 'normalizeUsername' to work with string only * chore: use 'newConnection' and 'newUser' const names * chore: use 'testInfo.testId' as GitHub 'code' value * chore: split GitHub OAuth test into testcases * Use ternary rather than logical AND in JSX Co-authored-by: Kent C. Dodds <[email protected]> * Use 'loading' rather than 'spinner' for animated spin icon Co-authored-by: Kent C. Dodds <[email protected]> * Use 'success' rather than 'check mark' for successful submission icon Co-authored-by: Kent C. Dodds <[email protected]> * Use 'error' rather than 'cross' for submission failure icon Co-authored-by: Kent C. Dodds <[email protected]> * chore: update e2e after changes to icon title * chore: stick to test naming convention * chore: add 'prepareGitHubUser' fixture for onboarding tests after OAuth --------- Co-authored-by: Kent C. Dodds <[email protected]>
1 parent 6ec4ed5 commit 2105ba3

File tree

14 files changed

+302
-17
lines changed

14 files changed

+302
-17
lines changed

app/components/ui/icon.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,34 @@ const childrenSizeClassName = {
3333
* Alternatively, if you're not ok with the icon being to the left of the text,
3434
* you need to wrap the icon and text in a common parent and set the parent to
3535
* display "flex" (or "inline-flex") with "items-center" and a reasonable gap.
36+
*
37+
* Pass `title` prop to the `Icon` component to get `<title>` element rendered
38+
* in the SVG container, providing this way for accessibility.
3639
*/
3740
export function Icon({
3841
name,
3942
size = 'font',
4043
className,
44+
title,
4145
children,
4246
...props
4347
}: SVGProps<SVGSVGElement> & {
4448
name: IconName
4549
size?: Size
50+
title?: string
4651
}) {
4752
if (children) {
4853
return (
4954
<span
5055
className={`inline-flex items-center ${childrenSizeClassName[size]}`}
5156
>
52-
<Icon name={name} size={size} className={className} {...props} />
57+
<Icon
58+
name={name}
59+
size={size}
60+
className={className}
61+
title={title}
62+
{...props}
63+
/>
5364
{children}
5465
</span>
5566
)
@@ -59,6 +70,7 @@ export function Icon({
5970
{...props}
6071
className={cn(sizeClassName[size], 'inline self-center', className)}
6172
>
73+
{title ? <title>{title}</title> : null}
6274
<use href={`${href}#${name}`} />
6375
</svg>
6476
)

app/components/ui/status-button.tsx

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,31 @@ export const StatusButton = React.forwardRef<
2525
})
2626
const companion = {
2727
pending: delayedPending ? (
28-
<div className="inline-flex h-6 w-6 items-center justify-center">
29-
<Icon name="update" className="animate-spin" />
28+
<div
29+
role="status"
30+
className="inline-flex h-6 w-6 items-center justify-center"
31+
>
32+
<Icon name="update" className="animate-spin" title="loading" />
3033
</div>
3134
) : null,
3235
success: (
33-
<div className="inline-flex h-6 w-6 items-center justify-center">
34-
<Icon name="check" />
36+
<div
37+
role="status"
38+
className="inline-flex h-6 w-6 items-center justify-center"
39+
>
40+
<Icon name="check" title="success" />
3541
</div>
3642
),
3743
error: (
38-
<div className="inline-flex h-6 w-6 items-center justify-center rounded-full bg-destructive">
39-
<Icon name="cross-1" className="text-destructive-foreground" />
44+
<div
45+
role="status"
46+
className="inline-flex h-6 w-6 items-center justify-center rounded-full bg-destructive"
47+
>
48+
<Icon
49+
name="cross-1"
50+
className="text-destructive-foreground"
51+
title="error"
52+
/>
4053
</div>
4154
),
4255
idle: null,

app/routes/_auth+/auth.$provider.callback.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import { ProviderNameSchema, providerLabels } from '#app/utils/connections.tsx'
88
import { prisma } from '#app/utils/db.server.ts'
99
import { ensurePrimary } from '#app/utils/litefs.server.ts'
1010
import { combineHeaders } from '#app/utils/misc.tsx'
11+
import {
12+
normalizeEmail,
13+
normalizeUsername,
14+
} from '#app/utils/providers/provider.ts'
1115
import {
1216
destroyRedirectToHeader,
1317
getRedirectCookieValue,
@@ -140,8 +144,11 @@ export async function loader({ request, params }: LoaderFunctionArgs) {
140144
verifySession.set(onboardingEmailSessionKey, profile.email)
141145
verifySession.set(prefilledProfileKey, {
142146
...profile,
143-
email: profile.email.toLowerCase(),
144-
username: profile.username?.replace(/[^a-zA-Z0-9_]/g, '_').toLowerCase(),
147+
email: normalizeEmail(profile.email),
148+
username:
149+
typeof profile.username === 'string'
150+
? normalizeUsername(profile.username)
151+
: undefined,
145152
})
146153
verifySession.set(providerIdKey, profile.id)
147154
const onboardingRedirect = [

app/routes/_auth+/onboarding.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export const meta: MetaFunction = () => {
129129
return [{ title: 'Setup Epic Notes Account' }]
130130
}
131131

132-
export default function SignupRoute() {
132+
export default function OnboardingRoute() {
133133
const data = useLoaderData<typeof loader>()
134134
const actionData = useActionData<typeof action>()
135135
const isPending = useIsPending()

app/routes/_auth+/onboarding_.$provider.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ export const meta: MetaFunction = () => {
177177
return [{ title: 'Setup Epic Notes Account' }]
178178
}
179179

180-
export default function SignupRoute() {
180+
export default function OnboardingProviderRoute() {
181181
const data = useLoaderData<typeof loader>()
182182
const actionData = useActionData<typeof action>()
183183
const isPending = useIsPending()

app/utils/providers/constants.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export const MOCK_CODE_GITHUB = 'MOCK_CODE_GITHUB_KODY'
2+
3+
export const MOCK_CODE_GITHUB_HEADER = 'x-mock-code-github'

app/utils/providers/github.server.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { z } from 'zod'
55
import { cache, cachified } from '../cache.server.ts'
66
import { connectionSessionStorage } from '../connections.server.ts'
77
import { type Timings } from '../timing.server.ts'
8+
import { MOCK_CODE_GITHUB_HEADER, MOCK_CODE_GITHUB } from './constants.ts'
89
import { type AuthProvider } from './provider.ts'
910

1011
const GitHubUserSchema = z.object({ login: z.string() })
@@ -19,7 +20,9 @@ const GitHubUserParseResult = z
1920
}),
2021
)
2122

22-
const shouldMock = process.env.GITHUB_CLIENT_ID?.startsWith('MOCK_')
23+
const shouldMock =
24+
process.env.GITHUB_CLIENT_ID?.startsWith('MOCK_') ||
25+
process.env.NODE_ENV === 'test'
2326

2427
export class GitHubProvider implements AuthProvider {
2528
getAuthStrategy() {
@@ -87,7 +90,11 @@ export class GitHubProvider implements AuthProvider {
8790
)
8891
const state = cuid()
8992
connectionSession.set('oauth2:state', state)
90-
const code = 'MOCK_CODE_GITHUB_KODY'
93+
94+
// allows us to inject a code when running e2e tests,
95+
// but falls back to a pre-defined 🐨 constant
96+
const code =
97+
request.headers.get(MOCK_CODE_GITHUB_HEADER) || MOCK_CODE_GITHUB
9198
const searchParams = new URLSearchParams({ code, state })
9299
throw redirect(`/auth/github/callback?${searchParams}`, {
93100
headers: {

app/utils/providers/provider.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,8 @@ export interface AuthProvider {
2121
link?: string | null
2222
}>
2323
}
24+
25+
export const normalizeEmail = (s: string) => s.toLowerCase()
26+
27+
export const normalizeUsername = (s: string) =>
28+
s.replace(/[^a-zA-Z0-9_]/g, '_').toLowerCase()

app/utils/user-validation.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import { z } from 'zod'
22

3+
export const USERNAME_MIN_LENGTH = 3
4+
export const USERNAME_MAX_LENGTH = 20
5+
36
export const UsernameSchema = z
47
.string({ required_error: 'Username is required' })
5-
.min(3, { message: 'Username is too short' })
6-
.max(20, { message: 'Username is too long' })
8+
.min(USERNAME_MIN_LENGTH, { message: 'Username is too short' })
9+
.max(USERNAME_MAX_LENGTH, { message: 'Username is too long' })
710
.regex(/^[a-zA-Z0-9_]+$/, {
811
message: 'Username can only include letters, numbers, and underscores',
912
})

playwright.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export default defineConfig({
3636
stderr: 'pipe',
3737
env: {
3838
PORT,
39+
NODE_ENV: 'test',
3940
},
4041
},
4142
})

0 commit comments

Comments
 (0)