From 459e21c4164bcad323cf43d55bb5e8963da15204 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Mon, 9 Oct 2023 22:35:09 +0100 Subject: [PATCH 1/5] feat: Setup trigger to insert team_member record --- .../down.sql | 2 ++ .../up.sql | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 hasura.planx.uk/migrations/1696884217237_grant_new_user_template_team_access/down.sql create mode 100644 hasura.planx.uk/migrations/1696884217237_grant_new_user_template_team_access/up.sql diff --git a/hasura.planx.uk/migrations/1696884217237_grant_new_user_template_team_access/down.sql b/hasura.planx.uk/migrations/1696884217237_grant_new_user_template_team_access/down.sql new file mode 100644 index 0000000000..2daff6714a --- /dev/null +++ b/hasura.planx.uk/migrations/1696884217237_grant_new_user_template_team_access/down.sql @@ -0,0 +1,2 @@ +DROP FUNCTION IF EXISTS grant_new_user_template_team_access; +DROP TRIGGER grant_new_user_template_team_access on users; \ No newline at end of file diff --git a/hasura.planx.uk/migrations/1696884217237_grant_new_user_template_team_access/up.sql b/hasura.planx.uk/migrations/1696884217237_grant_new_user_template_team_access/up.sql new file mode 100644 index 0000000000..a0ae84a00e --- /dev/null +++ b/hasura.planx.uk/migrations/1696884217237_grant_new_user_template_team_access/up.sql @@ -0,0 +1,28 @@ +CREATE OR REPLACE FUNCTION grant_new_user_template_team_access() RETURNS trigger AS $$ +DECLARE + templates_team_id INT; +BEGIN + SELECT id INTO templates_team_id FROM teams WHERE slug = 'templates'; + IF templates_team_id IS NOT NULL THEN + INSERT INTO team_members (user_id, team_id, role) VALUES (NEW.id, templates_team_id, 'teamEditor'); + END IF; + + RETURN NULL; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER grant_new_user_template_team_access AFTER INSERT ON users + FOR EACH ROW EXECUTE PROCEDURE grant_new_user_template_team_access(); + +COMMENT ON TRIGGER grant_new_user_template_team_access ON users +IS 'Automatically grant all new users teamEditor access to the shared Templates team'; + +-- Insert a record to team_members for all existing users +INSERT INTO + team_members (user_id, team_id, role) +SELECT + id, + 29, + 'teamEditor' +FROM + users; \ No newline at end of file From 10c44f8c1b1fd30f0d13b237250a60698af97f9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Mon, 9 Oct 2023 22:35:47 +0100 Subject: [PATCH 2/5] feat: Remove responsibility from frontend --- editor.planx.uk/src/components/Header.tsx | 1 - editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts | 9 ++++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/editor.planx.uk/src/components/Header.tsx b/editor.planx.uk/src/components/Header.tsx index ee2c12563f..c2a668a2e5 100644 --- a/editor.planx.uk/src/components/Header.tsx +++ b/editor.planx.uk/src/components/Header.tsx @@ -484,7 +484,6 @@ const EditorToolbar: React.FC<{ ? `All teams` : user.teams .map((team) => team.team.name) - .concat(["Templates"]) .join(", ")} diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts b/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts index 0f8cec17fd..8ca692315c 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts @@ -1,4 +1,4 @@ -import { User } from "@opensystemslab/planx-core/types"; +import { User, UserTeams } from "@opensystemslab/planx-core/types"; import { _client } from "client"; import jwtDecode from "jwt-decode"; import { Team } from "types"; @@ -24,13 +24,12 @@ export const userStore: StateCreator = ( canUserEditTeam(teamSlug) { const user = this.getUser(); if (!user) return false; + + const hasTeamEditorRole = (team: UserTeams) => team.role === "teamEditor" && team.team.slug === teamSlug; return ( user.isPlatformAdmin || - teamSlug === "templates" || - user.teams.filter( - (team) => team.role === "teamEditor" && team.team.slug === teamSlug, - ).length > 0 + user.teams.some(hasTeamEditorRole) ); }, From 05879d3ded10dc7a101b07760abd987fa73a550e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Mon, 9 Oct 2023 22:49:12 +0100 Subject: [PATCH 3/5] test(e2e): Add test coverage for new behaviour --- .../src/hasuraTriggers/hasuraTriggers.feature | 13 +++++ .../api-driven/src/hasuraTriggers/helpers.ts | 6 +++ .../api-driven/src/hasuraTriggers/steps.ts | 52 +++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 e2e/tests/api-driven/src/hasuraTriggers/hasuraTriggers.feature create mode 100644 e2e/tests/api-driven/src/hasuraTriggers/helpers.ts create mode 100644 e2e/tests/api-driven/src/hasuraTriggers/steps.ts diff --git a/e2e/tests/api-driven/src/hasuraTriggers/hasuraTriggers.feature b/e2e/tests/api-driven/src/hasuraTriggers/hasuraTriggers.feature new file mode 100644 index 0000000000..aa9703b8c3 --- /dev/null +++ b/e2e/tests/api-driven/src/hasuraTriggers/hasuraTriggers.feature @@ -0,0 +1,13 @@ +Feature: Database triggers + + @regression @add-user-trigger + Scenario: Adding a user to Planx - with Templates team + Given the Templates team exists + When a new user is added + Then they are granted access to the Templates team + And have the teamEditor role + + @regression @add-user-trigger + Scenario: Adding a user to Planx - without Templates team + When a new user is added + Then they are not granted access to the Templates team \ No newline at end of file diff --git a/e2e/tests/api-driven/src/hasuraTriggers/helpers.ts b/e2e/tests/api-driven/src/hasuraTriggers/helpers.ts new file mode 100644 index 0000000000..1d63422601 --- /dev/null +++ b/e2e/tests/api-driven/src/hasuraTriggers/helpers.ts @@ -0,0 +1,6 @@ +import { $admin } from "../client"; + +export const cleanup = async () => { + await $admin.user._destroyAll(); + await $admin.team._destroyAll(); +}; diff --git a/e2e/tests/api-driven/src/hasuraTriggers/steps.ts b/e2e/tests/api-driven/src/hasuraTriggers/steps.ts new file mode 100644 index 0000000000..389fa259a3 --- /dev/null +++ b/e2e/tests/api-driven/src/hasuraTriggers/steps.ts @@ -0,0 +1,52 @@ +import { After, Given, Then, When, World } from "@cucumber/cucumber"; +import { cleanup } from "./helpers"; +import { User } from "@opensystemslab/planx-core/types"; +import { $admin } from "../client"; +import assert from "assert"; +import { createTeam, createUser } from "../globalHelpers"; + +export class CustomWorld extends World { + user!: User; + templatesTeamId!: number; +} + +After("@add-user-trigger", async function () { + await cleanup(); +}); + +Given("the Templates team exists", async function (this) { + const templatesTeamId = await createTeam({ slug: "templates" }); + + assert.ok(templatesTeamId, "Templates team is not defined"); + + this.templatesTeamId = templatesTeamId; +}); + +When("a new user is added", async function (this) { + const userId = await createUser(); + const user = await $admin.user.getById(userId); + + assert.ok(user, "User is not defined"); + + this.user = user; +}); + +Then( + "they are granted access to the Templates team", + async function (this) { + assert.strictEqual(this.user.teams.length, 1); + assert.strictEqual(this.user.teams[0].team.slug, "templates"); + assert.strictEqual(this.user.teams[0].team.id, this.templatesTeamId); + }, +); + +Then("have the teamEditor role", async function (this) { + assert.strictEqual(this.user.teams[0].role, "teamEditor"); +}); + +Then( + "they are not granted access to the Templates team", + async function (this) { + assert.strictEqual(this.user.teams.length, 0); + }, +); From 712f91672084115fda3a94b31111b51b8e0cb288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Tue, 10 Oct 2023 09:15:21 +0100 Subject: [PATCH 4/5] test: Be more explicit in negative case --- .../src/hasuraTriggers/hasuraTriggers.feature | 1 + e2e/tests/api-driven/src/hasuraTriggers/steps.ts | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/e2e/tests/api-driven/src/hasuraTriggers/hasuraTriggers.feature b/e2e/tests/api-driven/src/hasuraTriggers/hasuraTriggers.feature index aa9703b8c3..ab3c9b8e0f 100644 --- a/e2e/tests/api-driven/src/hasuraTriggers/hasuraTriggers.feature +++ b/e2e/tests/api-driven/src/hasuraTriggers/hasuraTriggers.feature @@ -9,5 +9,6 @@ Feature: Database triggers @regression @add-user-trigger Scenario: Adding a user to Planx - without Templates team + Given the Templates team does not exist When a new user is added Then they are not granted access to the Templates team \ No newline at end of file diff --git a/e2e/tests/api-driven/src/hasuraTriggers/steps.ts b/e2e/tests/api-driven/src/hasuraTriggers/steps.ts index 389fa259a3..e76d02c591 100644 --- a/e2e/tests/api-driven/src/hasuraTriggers/steps.ts +++ b/e2e/tests/api-driven/src/hasuraTriggers/steps.ts @@ -22,6 +22,16 @@ Given("the Templates team exists", async function (this) { this.templatesTeamId = templatesTeamId; }); +Given("the Templates team does not exist", async function (this) { + const templatesTeam = await $admin.team.getBySlug("templates"); + + assert.equal( + templatesTeam, + undefined, + "Templates team exists but should not be defined", + ); +}); + When("a new user is added", async function (this) { const userId = await createUser(); const user = await $admin.user.getById(userId); From 0cd8a28eb53674598be30f66737b2dc256f7b539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Tue, 10 Oct 2023 13:45:19 +0100 Subject: [PATCH 5/5] test: Add basic test cases for UserStore --- editor.planx.uk/src/components/Header.tsx | 4 +- .../FlowEditor/lib/__tests__/user.test.ts | 101 ++++++++++++++++++ .../src/pages/FlowEditor/lib/store/user.ts | 12 +-- 3 files changed, 107 insertions(+), 10 deletions(-) create mode 100644 editor.planx.uk/src/pages/FlowEditor/lib/__tests__/user.test.ts diff --git a/editor.planx.uk/src/components/Header.tsx b/editor.planx.uk/src/components/Header.tsx index c2a668a2e5..d8ea18c2c9 100644 --- a/editor.planx.uk/src/components/Header.tsx +++ b/editor.planx.uk/src/components/Header.tsx @@ -482,9 +482,7 @@ const EditorToolbar: React.FC<{ {user.isPlatformAdmin ? `All teams` - : user.teams - .map((team) => team.team.name) - .join(", ")} + : user.teams.map((team) => team.team.name).join(", ")} )} diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/user.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/user.test.ts new file mode 100644 index 0000000000..5c0f9e24c7 --- /dev/null +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/user.test.ts @@ -0,0 +1,101 @@ +import { User } from "@opensystemslab/planx-core/types"; + +import { FullStore, vanillaStore } from "../store"; + +const { getState, setState } = vanillaStore; +const { canUserEditTeam } = getState(); + +const redUser: User = { + id: 1, + isPlatformAdmin: false, + firstName: "Red", + lastName: "Reddison", + email: "red@red-team.com", + teams: [ + { + role: "teamEditor", + team: { + name: "Red Team", + slug: "red-team", + id: 1, + }, + }, + { + role: "teamViewer", + team: { + name: "Blue Team", + slug: "blue-team", + id: 1, + }, + }, + ], +}; + +const blueUser: User = { + id: 2, + isPlatformAdmin: false, + firstName: "Blue", + lastName: "Bluey", + email: "blue@blue-team.com", + teams: [ + { + role: "teamEditor", + team: { + name: "Blue Team", + slug: "blue-team", + id: 1, + }, + }, + ], +}; + +const readOnlyUser: User = { + id: 3, + isPlatformAdmin: false, + firstName: "Read", + lastName: "Only", + email: "readonly@no-team.com", + teams: [], +}; + +const adminUser: User = { + id: 4, + isPlatformAdmin: true, + firstName: "Platform", + lastName: "Admin", + email: "admin@opensystemslab.io", + teams: [], +}; + +let initialState: FullStore; + +beforeEach(() => { + initialState = getState(); +}); + +afterEach(() => setState(initialState)); + +describe("canUserEditTeam helper function", () => { + it("returns true when a user has teamEditor permission for a team", () => { + setState({ user: redUser }); + expect(canUserEditTeam("red-team")).toBe(true); + expect(canUserEditTeam("blue-team")).toBe(false); + }); + + it("returns false when a user does not have permission for a team", () => { + setState({ user: blueUser }); + expect(canUserEditTeam("red-team")).toBe(false); + }); + + it("returns false when a user does not have any permissions", () => { + setState({ user: readOnlyUser }); + expect(canUserEditTeam("red-team")).toBe(false); + expect(canUserEditTeam("blue-team")).toBe(false); + }); + + it("returns true when a user is has the platformAdmin role", () => { + setState({ user: adminUser }); + expect(canUserEditTeam("red-team")).toBe(true); + expect(canUserEditTeam("blue-team")).toBe(true); + }); +}); diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts b/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts index 8ca692315c..861ad8c764 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/store/user.ts @@ -22,15 +22,13 @@ export const userStore: StateCreator = ( getUser: () => get().user, canUserEditTeam(teamSlug) { - const user = this.getUser(); + const user = get().getUser(); if (!user) return false; - - const hasTeamEditorRole = (team: UserTeams) => team.role === "teamEditor" && team.team.slug === teamSlug; - return ( - user.isPlatformAdmin || - user.teams.some(hasTeamEditorRole) - ); + const hasTeamEditorRole = (team: UserTeams) => + team.role === "teamEditor" && team.team.slug === teamSlug; + + return user.isPlatformAdmin || user.teams.some(hasTeamEditorRole); }, async initUserStore(jwt: string) {