From 56e6ca63b33fb58e7b7e996c9f6b688fce71ed5b Mon Sep 17 00:00:00 2001 From: Rodrigo Maia Date: Wed, 29 Mar 2023 18:15:05 +0000 Subject: [PATCH 1/5] fix(site): fix redirection to login after logout/change password --- site/e2e/tests/logout.spec.ts | 13 +++++++++++++ .../SecurityPage/SecurityPage.test.tsx | 2 ++ site/src/xServices/auth/authXService.ts | 8 +++----- .../userSecuritySettingsXService.ts | 5 ++++- 4 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 site/e2e/tests/logout.spec.ts diff --git a/site/e2e/tests/logout.spec.ts b/site/e2e/tests/logout.spec.ts new file mode 100644 index 0000000000000..1a25b786db689 --- /dev/null +++ b/site/e2e/tests/logout.spec.ts @@ -0,0 +1,13 @@ +import { test, expect } from "@playwright/test" +import { getStatePath } from "../helpers" + +test.use({ storageState: getStatePath("authState") }) + +test("signing out redirects to login page", async ({ page, baseURL }) => { + await page.goto(`${baseURL}/`, { waitUntil: "networkidle" }) + + await page.getByTestId("user-dropdown-trigger").click() + await page.getByRole("menuitem", { name: "Sign Out" }).click() + + await expect(page.getByRole("heading", { name: "Sign in to Coder" })).toBeVisible() +}) diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx index 89b3dac89e428..f7aac577c7a9d 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx @@ -47,6 +47,8 @@ describe("SecurityPage", () => { expect(successMessage).toBeDefined() expect(API.updateUserPassword).toBeCalledTimes(1) expect(API.updateUserPassword).toBeCalledWith(user.id, newData) + + await waitFor(() => expect(window.location).toBeAt("/")) }) }) diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index 44030790830e6..7392aaedaf010 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -389,12 +389,10 @@ export const authMachine = }), redirect: (_, { data }) => { if (!("redirectUrl" in data)) { - throw new Error( - "Redirect only should be called with data.redirectUrl", - ) + window.location.href = location.origin + } else { + window.location.replace(data.redirectUrl) } - - window.location.replace(data.redirectUrl) }, }, guards: { diff --git a/site/src/xServices/userSecuritySettings/userSecuritySettingsXService.ts b/site/src/xServices/userSecuritySettings/userSecuritySettingsXService.ts index 6cc8712542b7f..a2082f7dd9b3f 100644 --- a/site/src/xServices/userSecuritySettings/userSecuritySettingsXService.ts +++ b/site/src/xServices/userSecuritySettings/userSecuritySettingsXService.ts @@ -35,7 +35,7 @@ export const userSecuritySettingsMachine = createMachine( src: "updateSecurity", onDone: [ { - actions: "notifyUpdate", + actions: ["notifyUpdate", "redirectToHome"], target: "idle", }, ], @@ -66,6 +66,9 @@ export const userSecuritySettingsMachine = createMachine( assignError: assign({ error: (_, event) => event.data, }), + redirectToHome: () => { + window.location.href = location.origin + } }, }, ) From 25d3b327388e016d912f80071dfe45075ceaa3f6 Mon Sep 17 00:00:00 2001 From: Rodrigo Maia Date: Wed, 29 Mar 2023 20:24:06 +0000 Subject: [PATCH 2/5] chore: add login verification assert --- site/e2e/tests/logout.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/site/e2e/tests/logout.spec.ts b/site/e2e/tests/logout.spec.ts index 1a25b786db689..00ee982b7673e 100644 --- a/site/e2e/tests/logout.spec.ts +++ b/site/e2e/tests/logout.spec.ts @@ -10,4 +10,6 @@ test("signing out redirects to login page", async ({ page, baseURL }) => { await page.getByRole("menuitem", { name: "Sign Out" }).click() await expect(page.getByRole("heading", { name: "Sign in to Coder" })).toBeVisible() + + expect(page.url()).toMatch(/\/login$/) // ensure we're on the login page with no query params }) From 68882bd4ca3a0effbd69d58dd429bcc22e3be5d7 Mon Sep 17 00:00:00 2001 From: Rodrigo Maia Date: Wed, 29 Mar 2023 20:26:27 +0000 Subject: [PATCH 3/5] prettier --- site/e2e/tests/logout.spec.ts | 4 +++- .../userSecuritySettings/userSecuritySettingsXService.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/site/e2e/tests/logout.spec.ts b/site/e2e/tests/logout.spec.ts index 00ee982b7673e..5f8f4c892afeb 100644 --- a/site/e2e/tests/logout.spec.ts +++ b/site/e2e/tests/logout.spec.ts @@ -9,7 +9,9 @@ test("signing out redirects to login page", async ({ page, baseURL }) => { await page.getByTestId("user-dropdown-trigger").click() await page.getByRole("menuitem", { name: "Sign Out" }).click() - await expect(page.getByRole("heading", { name: "Sign in to Coder" })).toBeVisible() + await expect( + page.getByRole("heading", { name: "Sign in to Coder" }), + ).toBeVisible() expect(page.url()).toMatch(/\/login$/) // ensure we're on the login page with no query params }) diff --git a/site/src/xServices/userSecuritySettings/userSecuritySettingsXService.ts b/site/src/xServices/userSecuritySettings/userSecuritySettingsXService.ts index a2082f7dd9b3f..adccf76dbaa43 100644 --- a/site/src/xServices/userSecuritySettings/userSecuritySettingsXService.ts +++ b/site/src/xServices/userSecuritySettings/userSecuritySettingsXService.ts @@ -68,7 +68,7 @@ export const userSecuritySettingsMachine = createMachine( }), redirectToHome: () => { window.location.href = location.origin - } + }, }, }, ) From dbca115e07bb086c29c1e5ab4853a57b42ad2bb9 Mon Sep 17 00:00:00 2001 From: Rodrigo Maia Date: Wed, 29 Mar 2023 21:18:01 +0000 Subject: [PATCH 4/5] eslint false positive --- site/.eslintrc.yaml | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/site/.eslintrc.yaml b/site/.eslintrc.yaml index 1942b8a8120bb..635364c718962 100644 --- a/site/.eslintrc.yaml +++ b/site/.eslintrc.yaml @@ -43,6 +43,10 @@ overrides: # https://coder.com/docs/v2/latest/contributing/frontend#tests-getting-too-slow. testing-library/no-node-access: off testing-library/no-container: off + - files: ["e2e/**/*.[tj]s"] + extends: ["plugin:testing-library/react", "plugin:testing-library/dom"] + rules: + testing-library/prefer-screen-queries: "off" root: true rules: "@typescript-eslint/brace-style": @@ -107,20 +111,16 @@ rules: - error - paths: - name: "@material-ui/core" - message: - "Use path imports to avoid pulling in unused modules. See: + message: "Use path imports to avoid pulling in unused modules. See: https://material-ui.com/guides/minimizing-bundle-size/" - name: "@material-ui/icons" - message: - "Use path imports to avoid pulling in unused modules. See: + message: "Use path imports to avoid pulling in unused modules. See: https://material-ui.com/guides/minimizing-bundle-size/" - name: "@material-ui/styles" - message: - "Use path imports to avoid pulling in unused modules. See: + message: "Use path imports to avoid pulling in unused modules. See: https://material-ui.com/guides/minimizing-bundle-size/" - name: "@material-ui/core/Avatar" - message: - "You should use the Avatar component provided on + message: "You should use the Avatar component provided on components/Avatar/Avatar" no-unused-vars: "off" "object-curly-spacing": "off" @@ -156,7 +156,6 @@ rules: message: "Default React import not allowed", }, ] - settings: react: version: detect From ea81738ec4be32ecea84acadde25d8283116133f Mon Sep 17 00:00:00 2001 From: Rodrigo Maia Date: Wed, 29 Mar 2023 21:31:55 +0000 Subject: [PATCH 5/5] prettier --- site/.eslintrc.yaml | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/site/.eslintrc.yaml b/site/.eslintrc.yaml index 635364c718962..e2d09dc4b192f 100644 --- a/site/.eslintrc.yaml +++ b/site/.eslintrc.yaml @@ -46,6 +46,9 @@ overrides: - files: ["e2e/**/*.[tj]s"] extends: ["plugin:testing-library/react", "plugin:testing-library/dom"] rules: + # Sometimes the eslint-plugin-testing-library believes playwright queries are + # also react-testing-library queries, which is not the case. So we disable this + # rule for all e2e tests. testing-library/prefer-screen-queries: "off" root: true rules: @@ -111,16 +114,20 @@ rules: - error - paths: - name: "@material-ui/core" - message: "Use path imports to avoid pulling in unused modules. See: + message: + "Use path imports to avoid pulling in unused modules. See: https://material-ui.com/guides/minimizing-bundle-size/" - name: "@material-ui/icons" - message: "Use path imports to avoid pulling in unused modules. See: + message: + "Use path imports to avoid pulling in unused modules. See: https://material-ui.com/guides/minimizing-bundle-size/" - name: "@material-ui/styles" - message: "Use path imports to avoid pulling in unused modules. See: + message: + "Use path imports to avoid pulling in unused modules. See: https://material-ui.com/guides/minimizing-bundle-size/" - name: "@material-ui/core/Avatar" - message: "You should use the Avatar component provided on + message: + "You should use the Avatar component provided on components/Avatar/Avatar" no-unused-vars: "off" "object-curly-spacing": "off"