fix(e2e): Update E2E tests for recent permissions changes (#2247)

* fix(e2e): Quick fix of failing e2e tests * fix(e2e): Uncomment skipped e2e tests - Remove redundant email from JWT, rely on ID - Update API tests - Update /me route * fix: Update frontend - Only init user store once - Update Header to use store for username - Avoid zustand namespace clashes in user store * fix: Linting * fix: Update API tests
theopensystemslab · Oct 2, 2023 · 758ea99 · 758ea99
1 parent 4cc16e3
commit 758ea99
Show file tree

Hide file tree

Showing 32 changed files with 208 additions and 219 deletions.
diff --git a/api.planx.uk/modules/misc/controller.ts b/api.planx.uk/modules/misc/controller.ts
@@ -11,17 +11,17 @@ export const getLoggedInUserDetails: RequestHandler = async (
   try {
     const $client = getClient();
 
-    const email = userContext.getStore()?.user.email;
-    if (!email)
+    const id = userContext.getStore()?.user.sub;
+    if (!id)
       throw new ServerError({
-        message: "User email missing from request",
+        message: "User ID missing from request",
         status: 400,
       });
 
-    const user = await $client.user.getByEmail(email);
+    const user = await $client.user.getById(parseInt(id));
     if (!user)
       throw new ServerError({
-        message: `Unable to locate user with email ${email}`,
+        message: `Unable to locate user with ID ${id}`,
         status: 400,
       });
 

diff --git a/api.planx.uk/modules/misc/routes.test.ts b/api.planx.uk/modules/misc/routes.test.ts
@@ -5,8 +5,8 @@ import { userContext } from "../auth/middleware";
 
 const getStoreMock = jest.spyOn(userContext, "getStore");
 
-const mockGetByEmail = jest.fn().mockResolvedValue({
-  id: 36,
+const mockGetById = jest.fn().mockResolvedValue({
+  id: 123,
   firstName: "Albert",
   lastName: "Einstein",
   email: "[email protected]",
@@ -27,7 +27,7 @@ jest.mock("@opensystemslab/planx-core", () => {
   return {
     CoreDomainClient: jest.fn().mockImplementation(() => ({
       user: {
-        getByEmail: () => mockGetByEmail(),
+        getById: () => mockGetById(),
       },
     })),
   };
@@ -38,7 +38,6 @@ describe("/me endpoint", () => {
     getStoreMock.mockReturnValue({
       user: {
         sub: "123",
-        email: "[email protected]",
         jwt: getJWT({ role: "teamEditor" }),
       },
     });
@@ -58,8 +57,7 @@ describe("/me endpoint", () => {
   it("returns an error for invalid user context", async () => {
     getStoreMock.mockReturnValue({
       user: {
-        sub: "123",
-        email: undefined,
+        sub: undefined,
         jwt: getJWT({ role: "teamEditor" }),
       },
     });
@@ -70,21 +68,21 @@ describe("/me endpoint", () => {
       .expect(400)
       .then((res) => {
         expect(res.body).toEqual({
-          error: "User email missing from request",
+          error: "User ID missing from request",
         });
       });
   });
 
   it("returns an error for an invalid email address", async () => {
-    mockGetByEmail.mockResolvedValueOnce(null);
+    mockGetById.mockResolvedValueOnce(null);
 
     await supertest(app)
       .get("/me")
       .set(authHeader({ role: "teamEditor" }))
       .expect(400)
       .then((res) => {
         expect(res.body).toEqual({
-          error: "Unable to locate user with email [email protected]",
+          error: "Unable to locate user with ID 123",
         });
       });
   });

diff --git a/api.planx.uk/modules/team/service.test.ts b/api.planx.uk/modules/team/service.test.ts
@@ -40,7 +40,6 @@ describe("TeamService", () => {
     getStoreMock.mockReturnValue({
       user: {
         sub: "123",
-        email: "[email protected]",
         jwt: getJWT({ role: "teamEditor" }),
       },
     });

diff --git a/api.planx.uk/package.json b/api.planx.uk/package.json
@@ -4,7 +4,7 @@
   "private": true,
   "dependencies": {
     "@airbrake/node": "^2.1.8",
-    "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#44e9ebe",
+    "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#44420b9",
     "@types/isomorphic-fetch": "^0.0.36",
     "adm-zip": "^0.5.10",
     "aws-sdk": "^2.1467.0",

diff --git a/api.planx.uk/pnpm-lock.yaml b/api.planx.uk/pnpm-lock.yaml
diff --git a/api.planx.uk/server.ts b/api.planx.uk/server.ts
@@ -529,7 +529,6 @@ declare global {
     interface User {
       jwt: string;
       sub?: string;
-      email?: string;
       "https://hasura.io/jwt/claims"?: {
         "x-hasura-allowed-roles": Role[];
       };

diff --git a/e2e/tests/ui-driven/.eslintrc b/e2e/tests/ui-driven/.eslintrc
@@ -1,6 +1,3 @@
 {
-  "extends": ["../../.eslintrc", "plugin:playwright/playwright-test"],
-  "rules": {
-    "playwright/no-skipped-test": "off"
-  }
+  "extends": ["../../.eslintrc", "plugin:playwright/playwright-test"]
 }
diff --git a/e2e/tests/ui-driven/src/context.ts b/e2e/tests/ui-driven/src/context.ts
@@ -102,7 +102,7 @@ export async function tearDownTestContext(context: Context) {
   }
 }
 
-export function generateAuthenticationToken(userId) {
+export function generateAuthenticationToken(userId: string) {
   assert(process.env.JWT_SECRET);
   return sign(
     {

diff --git a/e2e/tests/ui-driven/src/create-flow.spec.ts b/e2e/tests/ui-driven/src/create-flow.spec.ts
@@ -35,7 +35,7 @@ test.describe("Navigation", () => {
     await tearDownTestContext(context);
   });
 
-  test.skip("Create a flow", async ({ browser }) => {
+  test("Create a flow", async ({ browser }) => {
     const page = await getTeamPage({
       browser,
       userId: context.user!.id!,
@@ -91,11 +91,7 @@ test.describe("Navigation", () => {
     await expect(nodes.getByText(noBranchNoticeText)).toBeVisible();
   });
 
-  test.skip("Preview a created flow", async ({
-    browser,
-  }: {
-    browser: Browser;
-  }) => {
+  test("Preview a created flow", async ({ browser }: { browser: Browser }) => {
     const page = await createAuthenticatedSession({
       browser,
       userId: context.user!.id!,

diff --git a/e2e/tests/ui-driven/src/login.spec.ts b/e2e/tests/ui-driven/src/login.spec.ts
@@ -26,7 +26,7 @@ test.describe("Login", () => {
     await tearDownTestContext(context);
   });
 
-  test.skip("setting a cookie bypasses login", async ({ browser }) => {
+  test("setting a cookie bypasses login", async ({ browser }) => {
     const page = await createAuthenticatedSession({
       browser,
       userId: context.user!.id!,
@@ -41,7 +41,7 @@ test.describe("Login", () => {
     await expect(team).toBeVisible();
   });
 
-  test.skip("shows error toast when there is a network error and removes it when a retry is successful", async ({
+  test("shows error toast when there is a network error and removes it when a retry is successful", async ({
     browser,
   }) => {
     const page = await createAuthenticatedSession({

diff --git a/editor.planx.uk/.eslintrc b/editor.planx.uk/.eslintrc
@@ -11,7 +11,6 @@
   "extends": [
     "eslint:recommended",
     "plugin:jsx-a11y/recommended",
-    "plugin:testing-library/react",
     "plugin:@typescript-eslint/recommended",
     "prettier",
     "plugin:jest/recommended"
@@ -73,5 +72,11 @@
   },
   "settings": {
     "jest": { "version": 27 }
-  }
+  },
+  "overrides": [
+		{
+			"files": ["**/__tests__/**/*.[jt]s?(x)', '**/?(*.)+(spec|test).[jt]s?(x)"],
+			"extends": ["plugin:testing-library/react"]
+		}
+	]
 }
diff --git a/editor.planx.uk/package.json b/editor.planx.uk/package.json
@@ -14,7 +14,7 @@
     "@mui/styles": "^5.14.5",
     "@mui/utils": "^5.14.5",
     "@opensystemslab/map": "^0.7.5",
-    "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#44e9ebe",
+    "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#44420b9",
     "@tiptap/core": "^2.0.3",
     "@tiptap/extension-bold": "^2.0.3",
     "@tiptap/extension-bubble-menu": "^2.1.6",

diff --git a/editor.planx.uk/pnpm-lock.yaml b/editor.planx.uk/pnpm-lock.yaml
diff --git a/editor.planx.uk/src/@planx/components/DrawBoundary/Public/index.tsx b/editor.planx.uk/src/@planx/components/DrawBoundary/Public/index.tsx
@@ -12,7 +12,7 @@ import QuestionHeader from "@planx/components/shared/Preview/QuestionHeader";
 import { PrivateFileUpload } from "@planx/components/shared/PrivateFileUpload/PrivateFileUpload";
 import type { PublicProps } from "@planx/components/ui";
 import buffer from "@turf/buffer";
-import { type GeometryObject,point } from "@turf/helpers";
+import { type GeometryObject, point } from "@turf/helpers";
 import { Store, useStore } from "pages/FlowEditor/lib/store";
 import React, { useEffect, useRef, useState } from "react";
 import { FONT_WEIGHT_SEMI_BOLD } from "theme";
@@ -42,10 +42,13 @@ export default function Component(props: Props) {
   const [boundary, setBoundary] = useState<Boundary>(previousBoundary);
   const [slots, setSlots] = useState<FileUploadSlot[]>(previousFile ?? []);
   const [area, setArea] = useState<number | undefined>(previousArea);
-  const addressPoint = passport?.data?._address?.longitude && passport?.data?._address?.latitude && point([
-    Number(passport?.data?._address?.longitude),
-    Number(passport?.data?._address?.latitude),
-  ]);
+  const addressPoint =
+    passport?.data?._address?.longitude &&
+    passport?.data?._address?.latitude &&
+    point([
+      Number(passport?.data?._address?.longitude),
+      Number(passport?.data?._address?.latitude),
+    ]);
   const environment = useStore((state) => state.previewEnvironment);
 
   useEffect(() => {
@@ -120,9 +123,12 @@ export default function Component(props: Props) {
                 drawMode
                 drawPointer="crosshair"
                 drawGeojsonData={JSON.stringify(boundary)}
-                clipGeojsonData={addressPoint && JSON.stringify(
-                  buffer(addressPoint, BUFFER_IN_METERS, { units: "meters" }),
-                )}
+                clipGeojsonData={
+                  addressPoint &&
+                  JSON.stringify(
+                    buffer(addressPoint, BUFFER_IN_METERS, { units: "meters" }),
+                  )
+                }
                 zoom={20}
                 maxZoom={23}
                 latitude={Number(passport?.data?._address?.latitude)}

diff --git a/editor.planx.uk/src/@planx/components/FileUploadAndLabel/Editor.test.tsx b/editor.planx.uk/src/@planx/components/FileUploadAndLabel/Editor.test.tsx
@@ -1,11 +1,11 @@
 import { screen } from "@testing-library/react";
+import { vanillaStore } from "pages/FlowEditor/lib/store";
 import React from "react";
 import { DndProvider } from "react-dnd";
 import { HTML5Backend } from "react-dnd-html5-backend";
 import { setup } from "testUtils";
 
 import FileUploadAndLabelComponent from "./Editor";
-import { vanillaStore } from "pages/FlowEditor/lib/store";
 
 const { getState } = vanillaStore;
 

diff --git a/editor.planx.uk/src/@planx/components/FileUploadAndLabel/Modal.tsx b/editor.planx.uk/src/@planx/components/FileUploadAndLabel/Modal.tsx
@@ -285,7 +285,11 @@ const SelectMultiple = (props: SelectMultipleProps) => {
             <Typography variant="h4" component="h3" pr={3}>
               Select all labels that apply
             </Typography>
-            <Button variant="contained" onClick={handleClose} aria-label="Close list">
+            <Button
+              variant="contained"
+              onClick={handleClose}
+              aria-label="Close list"
+            >
               Done
             </Button>
           </ListHeader>

diff --git a/...uk/src/@planx/components/PlanningConstraints/mocks/classifiedRoadsNegativeResponseMock.ts b/...uk/src/@planx/components/PlanningConstraints/mocks/classifiedRoadsNegativeResponseMock.ts
@@ -7,7 +7,7 @@ export default {
     "road.classified": {
       name: "Classified road",
       plural: "Classified roads",
-      text: "This will effect your project if you are looking to add a dropped kerb. It may also impact some agricultural or forestry projects within 25 metres of a classified road."
+      text: "This will effect your project if you are looking to add a dropped kerb. It may also impact some agricultural or forestry projects within 25 metres of a classified road.",
     },
   },
   constraints: {

diff --git a/...r.planx.uk/src/@planx/components/PlanningConstraints/mocks/classifiedRoadsResponseMock.ts b/...r.planx.uk/src/@planx/components/PlanningConstraints/mocks/classifiedRoadsResponseMock.ts
@@ -7,7 +7,7 @@ export default {
     "road.classified": {
       name: "Classified road",
       plural: "Classified roads",
-      text: "This will effect your project if you are looking to add a dropped kerb. It may also impact some agricultural or forestry projects within 25 metres of a classified road."
+      text: "This will effect your project if you are looking to add a dropped kerb. It may also impact some agricultural or forestry projects within 25 metres of a classified road.",
     },
   },
   constraints: {

diff --git a/editor.planx.uk/src/@planx/components/Section/Editor.tsx b/editor.planx.uk/src/@planx/components/Section/Editor.tsx
@@ -28,10 +28,7 @@ function SectionComponent(props: Props) {
   return (
     <form onSubmit={formik.handleSubmit} id="modal">
       <ModalSection>
-        <ModalSectionContent
-          title="Section marker"
-          Icon={ICONS[TYPES.Section]}
-        >
+        <ModalSectionContent title="Section marker" Icon={ICONS[TYPES.Section]}>
           <InputRow>
             <Input
               required

diff --git a/editor.planx.uk/src/@planx/components/Section/Public.test.tsx b/editor.planx.uk/src/@planx/components/Section/Public.test.tsx
@@ -8,7 +8,13 @@ import Section, { SectionsOverviewList } from "./Public";
 describe("Section component", () => {
   it("renders correctly", () => {
     const handleSubmit = jest.fn();
-    setup(<Section title="Section one" description="Description of section one" handleSubmit={handleSubmit} />);
+    setup(
+      <Section
+        title="Section one"
+        description="Description of section one"
+        handleSubmit={handleSubmit}
+      />,
+    );
 
     expect(screen.getByText("Application incomplete.")).toBeInTheDocument();
     expect(screen.getByText("Continue")).toBeInTheDocument();
@@ -19,7 +25,11 @@ describe("Section component", () => {
     const handleSubmit = jest.fn();
 
     const { container } = setup(
-      <Section title="Section one" description="Description of section one" handleSubmit={handleSubmit} />,
+      <Section
+        title="Section one"
+        description="Description of section one"
+        handleSubmit={handleSubmit}
+      />,
     );
 
     const results = await axe(container);