-
Notifications
You must be signed in to change notification settings - Fork 403
feat(clerk-js,clerk-react,types): Navigate to next task #5377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1921ede
576509e
f14c14b
f2844b9
3633f39
34c50df
0e9a10c
f512dfc
d9b5c2d
a25c190
349a17f
f935c83
65e6be4
f8b7a53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@clerk/clerk-js': patch | ||
| '@clerk/clerk-react': patch | ||
| '@clerk/types': patch | ||
| --- | ||
|
|
||
| Introduce `__experimental_nextTask` method for navigating to next tasks on a after-auth flow |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| import type { ClerkClient, Organization } from '@clerk/backend'; | ||
| import { faker } from '@faker-js/faker'; | ||
|
|
||
| export type FakeOrganization = Pick<Organization, 'slug' | 'name'>; | ||
|
|
||
| export type OrganizationService = { | ||
| deleteAll: () => Promise<void>; | ||
| createFakeOrganization: () => FakeOrganization; | ||
| }; | ||
|
|
||
| export const createOrganizationsService = (clerkClient: ClerkClient) => { | ||
| const self: OrganizationService = { | ||
| createFakeOrganization: () => ({ | ||
| slug: faker.helpers.slugify(faker.commerce.department()).toLowerCase(), | ||
| name: faker.commerce.department(), | ||
| }), | ||
| deleteAll: async () => { | ||
| const organizations = await clerkClient.organizations.getOrganizationList(); | ||
| const bulkDeletionPromises = organizations.data.map(({ id }) => clerkClient.organizations.deleteOrganization(id)); | ||
| await Promise.all(bulkDeletionPromises); | ||
| }, | ||
| }; | ||
|
|
||
| return self; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { expect } from '@playwright/test'; | ||
|
|
||
| import { common } from './commonPageObject'; | ||
| import type { FakeOrganization } from './organizationsService'; | ||
| import type { TestArgs } from './signInPageObject'; | ||
|
|
||
| export const createSessionTaskComponentPageObject = (testArgs: TestArgs) => { | ||
| const { page } = testArgs; | ||
|
|
||
| const self = { | ||
| ...common(testArgs), | ||
| resolveForceOrganizationSelectionTask: async (fakeOrganization: FakeOrganization) => { | ||
| const createOrganizationButton = page.getByRole('button', { name: /create organization/i }); | ||
|
|
||
| await expect(createOrganizationButton).toBeVisible(); | ||
| expect(page.url()).toContain('add-organization'); | ||
|
|
||
| await page.locator('input[name=name]').fill(fakeOrganization.name); | ||
| await page.locator('input[name=slug]').fill(fakeOrganization.slug); | ||
|
|
||
| await createOrganizationButton.click(); | ||
| }, | ||
| }; | ||
|
|
||
| return self; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,38 +1,49 @@ | ||
| import { expect, test } from '@playwright/test'; | ||
| import { test } from '@playwright/test'; | ||
|
|
||
| import { appConfigs } from '../presets'; | ||
| import type { FakeUser } from '../testUtils'; | ||
| import { createTestUtils, testAgainstRunningApps } from '../testUtils'; | ||
| import type { FakeOrganization } from '../testUtils/organizationsService'; | ||
|
|
||
| testAgainstRunningApps({ withEnv: [appConfigs.envs.withSessionTasks] })( | ||
| 'session tasks after sign-in flow @nextjs', | ||
| ({ app }) => { | ||
| test.describe.configure({ mode: 'serial' }); | ||
|
|
||
| let fakeUser: FakeUser; | ||
| let fakeOrganization: FakeOrganization; | ||
|
|
||
| test.beforeAll(async () => { | ||
| const u = createTestUtils({ app }); | ||
| fakeUser = u.services.users.createFakeUser(); | ||
| fakeOrganization = u.services.organizations.createFakeOrganization(); | ||
| await u.services.users.createBapiUser(fakeUser); | ||
| }); | ||
|
|
||
| test.afterAll(async () => { | ||
| const u = createTestUtils({ app }); | ||
| await fakeUser.deleteIfExists(); | ||
| await u.services.organizations.deleteAll(); | ||
| await app.teardown(); | ||
| }); | ||
|
|
||
| test('navigate to task on after sign-in', async ({ page, context }) => { | ||
| const u = createTestUtils({ app, page, context }); | ||
|
|
||
| // Performs sign-in | ||
| await u.po.signIn.goTo(); | ||
| await u.po.signIn.setIdentifier(fakeUser.email); | ||
| await u.po.signIn.continue(); | ||
| await u.po.signIn.setPassword(fakeUser.password); | ||
| await u.po.signIn.continue(); | ||
| await u.po.expect.toBeSignedIn(); | ||
|
|
||
| await expect(u.page.getByRole('button', { name: /create organization/i })).toBeVisible(); | ||
| expect(page.url()).toContain('add-organization'); | ||
| // Resolves task | ||
| await u.po.sessionTask.resolveForceOrganizationSelectionTask(fakeOrganization); | ||
| await u.po.expect.toHaveResolvedTask(); | ||
|
|
||
| // Navigates to after sign-in | ||
| await u.page.waitForAppUrl('/'); | ||
| }); | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,34 +1,50 @@ | ||
| import { expect, test } from '@playwright/test'; | ||
| import { test } from '@playwright/test'; | ||
|
|
||
| import { appConfigs } from '../presets'; | ||
| import type { FakeUser } from '../testUtils'; | ||
| import { createTestUtils, testAgainstRunningApps } from '../testUtils'; | ||
| import type { FakeOrganization } from '../testUtils/organizationsService'; | ||
|
|
||
| testAgainstRunningApps({ withEnv: [appConfigs.envs.withSessionTasks] })( | ||
| 'session tasks after sign-up flow @nextjs', | ||
| ({ app }) => { | ||
| test.describe.configure({ mode: 'serial' }); | ||
|
|
||
| let fakeUser: FakeUser; | ||
| let fakeOrganization: FakeOrganization; | ||
|
|
||
| test.beforeAll(() => { | ||
| const u = createTestUtils({ app }); | ||
| fakeUser = u.services.users.createFakeUser({ | ||
| fictionalEmail: true, | ||
| withPhoneNumber: true, | ||
| withUsername: true, | ||
| }); | ||
| fakeOrganization = u.services.organizations.createFakeOrganization(); | ||
| }); | ||
|
|
||
| test.afterAll(async () => { | ||
| const u = createTestUtils({ app }); | ||
| await u.services.organizations.deleteAll(); | ||
| await fakeUser.deleteIfExists(); | ||
| await app.teardown(); | ||
| }); | ||
|
|
||
| test('navigate to task on after sign-up', async ({ page, context }) => { | ||
| // Performs sign-up | ||
| const u = createTestUtils({ app, page, context }); | ||
| const fakeUser = u.services.users.createFakeUser({ | ||
| fictionalEmail: true, | ||
| withPhoneNumber: true, | ||
| withUsername: true, | ||
| }); | ||
| await u.po.signUp.goTo(); | ||
| await u.po.signUp.signUpWithEmailAndPassword({ | ||
| email: fakeUser.email, | ||
| password: fakeUser.password, | ||
| }); | ||
|
|
||
| await expect(u.page.getByRole('button', { name: /create organization/i })).toBeVisible(); | ||
| expect(page.url()).toContain('add-organization'); | ||
| // Resolves task | ||
| await u.po.sessionTask.resolveForceOrganizationSelectionTask(fakeOrganization); | ||
| await u.po.expect.toHaveResolvedTask(); | ||
|
|
||
| await fakeUser.deleteIfExists(); | ||
| // Navigates to after sign-up | ||
| await u.page.waitForAppUrl('/'); | ||
| }); | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import type { | ||
| ActiveSessionResource, | ||
| PendingSessionResource, | ||
| SignedInSessionResource, | ||
| SignInJSON, | ||
| SignUpJSON, | ||
|
|
@@ -486,6 +487,15 @@ describe('Clerk singleton', () => { | |
| lastActiveToken: { getRawString: () => 'mocked-token' }, | ||
| tasks: [{ key: 'org' }], | ||
| currentTask: { key: 'org', __internal_getUrl: () => 'https://foocorp.com/add-organization' }, | ||
| reload: jest.fn(() => | ||
| Promise.resolve({ | ||
| id: '1', | ||
| status: 'pending', | ||
| user: {}, | ||
| tasks: [{ key: 'org' }], | ||
| currentTask: { key: 'org', __internal_getUrl: () => 'https://foocorp.com/add-organization' }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For posterity: I'm always just a little nervous of including random routable URLs in tests. Truly unlikely security threat potential: If we actually GET this URL, and someone knows we're getting it, they could register the domain and have it serve some exploit that gets us to leak our CI secrets. I'm not even proposing you change this - it's the pattern laid down elsewhere in the test - but when in doubt, I like
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense! Also just to note that our tests only run if the triggering actor (owner of the PR) has the GitHub org permissions to access secrets, otherwise it'll fail For URLs like those that are used to trigger navigation rather than HTTP requests, then I think it's fine! |
||
| }), | ||
| ), | ||
| }; | ||
| let eventBusSpy; | ||
|
|
||
|
|
@@ -2258,4 +2268,91 @@ describe('Clerk singleton', () => { | |
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('nextTask', () => { | ||
| describe('with `pending` session status', () => { | ||
| const mockSession = { | ||
| id: '1', | ||
| status: 'pending', | ||
| user: {}, | ||
| tasks: [{ key: 'org' }], | ||
| currentTask: { key: 'org', __internal_getUrl: () => 'https://foocorp.com/add-organization' }, | ||
| lastActiveToken: { getRawString: () => 'mocked-token' }, | ||
| }; | ||
|
|
||
| const mockResource = { | ||
| ...mockSession, | ||
| remove: jest.fn(), | ||
| touch: jest.fn(() => Promise.resolve()), | ||
| getToken: jest.fn(), | ||
| reload: jest.fn(() => Promise.resolve(mockSession)), | ||
| }; | ||
|
|
||
| beforeAll(() => { | ||
| mockResource.touch.mockReturnValueOnce(Promise.resolve()); | ||
| mockClientFetch.mockReturnValue(Promise.resolve({ signedInSessions: [mockResource] })); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| mockResource.remove.mockReset(); | ||
| mockResource.touch.mockReset(); | ||
| }); | ||
|
|
||
| it('navigates to next task', async () => { | ||
| const sut = new Clerk(productionPublishableKey); | ||
| await sut.load(mockedLoadOptions); | ||
|
|
||
| await sut.setActive({ session: mockResource as any as PendingSessionResource }); | ||
| await sut.__experimental_nextTask(); | ||
|
|
||
| expect(mockNavigate.mock.calls[0][0]).toBe('/sign-in#/add-organization'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('with `active` session status', () => { | ||
| const mockSession = { | ||
| id: '1', | ||
| remove: jest.fn(), | ||
| status: 'active', | ||
| user: {}, | ||
| touch: jest.fn(() => Promise.resolve()), | ||
| getToken: jest.fn(), | ||
| lastActiveToken: { getRawString: () => 'mocked-token' }, | ||
| reload: jest.fn(() => | ||
| Promise.resolve({ | ||
| id: '1', | ||
| remove: jest.fn(), | ||
| status: 'active', | ||
| user: {}, | ||
| touch: jest.fn(() => Promise.resolve()), | ||
| getToken: jest.fn(), | ||
| lastActiveToken: { getRawString: () => 'mocked-token' }, | ||
| }), | ||
| ), | ||
| }; | ||
|
|
||
| afterEach(() => { | ||
| mockSession.remove.mockReset(); | ||
| mockSession.touch.mockReset(); | ||
| (window as any).__unstable__onBeforeSetActive = null; | ||
| (window as any).__unstable__onAfterSetActive = null; | ||
| }); | ||
|
|
||
| it('navigates to redirect url on completion', async () => { | ||
| mockSession.touch.mockReturnValue(Promise.resolve()); | ||
| mockClientFetch.mockReturnValue(Promise.resolve({ signedInSessions: [mockSession] })); | ||
|
|
||
| const sut = new Clerk(productionPublishableKey); | ||
| await sut.load(mockedLoadOptions); | ||
| await sut.setActive({ session: mockSession as any as ActiveSessionResource }); | ||
|
|
||
| const redirectUrlComplete = '/welcome-to-app'; | ||
| await sut.__experimental_nextTask({ redirectUrlComplete }); | ||
|
|
||
| console.log(mockNavigate.mock.calls); | ||
|
|
||
| expect(mockNavigate.mock.calls[0][0]).toBe('/welcome-to-app'); | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm bumping this to +4 to not break too soon on the next PRs - let me know if you have any concerns!