From 8f0478ba5898cbbfdef312b32cf247bc3e7a4cf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Fri, 24 May 2024 09:20:38 +0100 Subject: [PATCH] feat(backend): Toggle flow online or offline (#3170) --- .../flowStatusHistory.feature | 14 +++ .../src/flowStatusHistory/helpers.ts | 72 ++++++++++++ .../api-driven/src/flowStatusHistory/steps.ts | 110 ++++++++++++++++++ hasura.planx.uk/metadata/tables.yaml | 48 +++++--- .../down.sql | 11 ++ .../up.sql | 74 ++++++++++++ .../tests/flow_status_history.test.js | 78 +++++++++++++ 7 files changed, 390 insertions(+), 17 deletions(-) create mode 100644 e2e/tests/api-driven/src/flowStatusHistory/flowStatusHistory.feature create mode 100644 e2e/tests/api-driven/src/flowStatusHistory/helpers.ts create mode 100644 e2e/tests/api-driven/src/flowStatusHistory/steps.ts create mode 100644 hasura.planx.uk/migrations/1715954131936_create_table_public_flow_status_enum/down.sql create mode 100644 hasura.planx.uk/migrations/1715954131936_create_table_public_flow_status_enum/up.sql create mode 100644 hasura.planx.uk/tests/flow_status_history.test.js diff --git a/e2e/tests/api-driven/src/flowStatusHistory/flowStatusHistory.feature b/e2e/tests/api-driven/src/flowStatusHistory/flowStatusHistory.feature new file mode 100644 index 0000000000..95cf70321b --- /dev/null +++ b/e2e/tests/api-driven/src/flowStatusHistory/flowStatusHistory.feature @@ -0,0 +1,14 @@ +Feature: Flow status history + + @regression @flow-status-history + Scenario: Creating a flow + Given a flow exists + Then the status of the flow is offline by default + And a flow_status_history record is created + + @regression @flow-status-history + Scenario: Updating a flow + Given a flow exists + When the flow status is changed to online + Then the open flow_status_history record is updated + And a new flow_status_history record is created \ No newline at end of file diff --git a/e2e/tests/api-driven/src/flowStatusHistory/helpers.ts b/e2e/tests/api-driven/src/flowStatusHistory/helpers.ts new file mode 100644 index 0000000000..808cb1c37d --- /dev/null +++ b/e2e/tests/api-driven/src/flowStatusHistory/helpers.ts @@ -0,0 +1,72 @@ +import { FlowStatus } from "@opensystemslab/planx-core/types"; +import { $admin } from "../client"; +import { createTeam } from "../globalHelpers"; +import gql from "graphql-tag"; + +export const setup = async () => { + const teamId = await createTeam(); + + const world = { teamId }; + + return world; +}; + +export const cleanup = async () => { + await $admin.user._destroyAll(); + await $admin.flow._destroyAll(); + await $admin.team._destroyAll(); +}; + +interface GetFlowStatus { + flows: { + status: FlowStatus; + }; +} + +export const getFlowStatus = async (flowId: string) => { + const { + flows: { status }, + } = await $admin.client.request( + gql` + query GetFlowStatus($flowId: uuid!) { + flows: flows_by_pk(id: $flowId) { + status + } + } + `, + { flowId }, + ); + + return status; +}; + +interface GetFlowStatusHistory { + flowStatusHistory: { + id: number; + status: FlowStatus; + eventStart: string; + eventEnd: string; + }[]; +} + +export const getFlowStatusHistory = async (flowId: string) => { + const { flowStatusHistory } = + await $admin.client.request( + gql` + query GetFlowStatusHistory($flowId: uuid!) { + flowStatusHistory: flow_status_history( + where: { flow_id: { _eq: $flowId } } + order_by: { event_start: asc } + ) { + id + status + eventStart: event_start + eventEnd: event_end + } + } + `, + { flowId }, + ); + + return flowStatusHistory; +}; diff --git a/e2e/tests/api-driven/src/flowStatusHistory/steps.ts b/e2e/tests/api-driven/src/flowStatusHistory/steps.ts new file mode 100644 index 0000000000..bd36d3e407 --- /dev/null +++ b/e2e/tests/api-driven/src/flowStatusHistory/steps.ts @@ -0,0 +1,110 @@ +import { When, Then, World, After, Before, Given } from "@cucumber/cucumber"; +import assert from "assert"; +import { cleanup, getFlowStatus, getFlowStatusHistory, setup } from "./helpers"; +import { createFlow } from "../globalHelpers"; +import { $admin } from "../client"; + +export class CustomWorld extends World { + teamId!: number; + flowId!: string; +} + +After("@flow-status-history", async function () { + await cleanup(); +}); + +Before("@flow-status-history", async function () { + const { teamId } = await setup(); + this.teamId = teamId; +}); + +Given("a flow exists", async function () { + const flowId = await createFlow({ teamId: this.teamId, slug: "test-flow" }); + + assert.ok(flowId, "flowId is not defined"); + + this.flowId = flowId; +}); + +Then("the status of the flow is offline by default", async function () { + const status = await getFlowStatus(this.flowId); + + assert.equal( + status, + "offline", + `Flow status is ${status} - it should be "offline"`, + ); +}); + +Then("a flow_status_history record is created", async function () { + const flowStatusHistory = await getFlowStatusHistory(this.flowId); + + assert.notEqual( + flowStatusHistory.length, + 0, + "No records found for flow_status_history", + ); + assert.equal( + flowStatusHistory.length, + 1, + "Multiple records found for flow_status_history", + ); + assert.ok(flowStatusHistory[0], "flow_status_history record not created"); + assert.equal( + flowStatusHistory[0].status, + "offline", + `Flow status is ${flowStatusHistory[0].status} - it should be "offline"`, + ); + assert.notEqual( + flowStatusHistory[0].eventStart, + null, + "Event start should be set on INSERT", + ); + assert.equal( + flowStatusHistory[0].eventEnd, + null, + "Event end should not be set on INSERT", + ); +}); + +When("the flow status is changed to online", async function () { + const flow = await $admin.flow.setStatus({ + flow: { id: this.flowId }, + status: "online", + }); + assert.ok(flow, "Flow not defined after setStatus()"); + assert.equal( + flow.status, + "online", + `Flow status is ${flow.status} - it should be "online`, + ); +}); + +Then("the open flow_status_history record is updated", async function () { + const flowStatusHistory = await getFlowStatusHistory(this.flowId); + assert.ok( + flowStatusHistory[0].eventEnd, + "Event end should be set on update to status column", + ); +}); + +Then("a new flow_status_history record is created", async function () { + const flowStatusHistory = await getFlowStatusHistory(this.flowId); + assert.equal(flowStatusHistory.length, 2, "New record not created on UPDATE"); + assert.ok(flowStatusHistory[1], "flow_status_history record not created"); + assert.equal( + flowStatusHistory[1].status, + "offline", + `Flow status is ${flowStatusHistory[1].status} - it should be "offline"`, + ); + assert.notEqual( + flowStatusHistory[1].eventStart, + null, + "Event start should be set on INSERT", + ); + assert.equal( + flowStatusHistory[1].eventEnd, + null, + "Event end should not be set on INSERT", + ); +}); diff --git a/hasura.planx.uk/metadata/tables.yaml b/hasura.planx.uk/metadata/tables.yaml index aa49202e93..eaf09ef72b 100644 --- a/hasura.planx.uk/metadata/tables.yaml +++ b/hasura.planx.uk/metadata/tables.yaml @@ -362,6 +362,13 @@ - document_template - flow_id filter: {} +- table: + name: flow_status_enum + schema: public + is_enum: true +- table: + name: flow_status_history + schema: public - table: name: flows schema: public @@ -406,16 +413,16 @@ permission: check: {} columns: + - copied_from + - created_at - creator_id - - team_id + - data + - id - settings - slug - - created_at + - team_id - updated_at - - copied_from - - id - version - - data validate_input: definition: forward_client_headers: false @@ -482,16 +489,17 @@ - role: api permission: columns: + - copied_from + - created_at - creator_id - - team_id + - data + - id - settings - slug - - created_at + - status + - team_id - updated_at - - copied_from - - id - version - - data computed_fields: - data_merged filter: {} @@ -499,13 +507,14 @@ - role: platformAdmin permission: columns: + - analytics_link - created_at - creator_id - data - id - - analytics_link - settings - slug + - status - team_id - updated_at - version @@ -522,6 +531,7 @@ - id - settings - slug + - status - team_id - updated_at - version @@ -532,13 +542,14 @@ - role: teamEditor permission: columns: + - analytics_link - created_at - creator_id - data - id - - analytics_link - settings - slug + - status - team_id - updated_at - version @@ -550,16 +561,17 @@ - role: api permission: columns: + - copied_from + - created_at - creator_id - - team_id + - data + - id - settings - slug - - created_at + - status + - team_id - updated_at - - copied_from - - id - version - - data filter: {} check: {} validate_input: @@ -577,6 +589,7 @@ - data - settings - slug + - status - team_id filter: {} check: null @@ -595,6 +608,7 @@ - data - settings - slug + - status - team_id filter: team: diff --git a/hasura.planx.uk/migrations/1715954131936_create_table_public_flow_status_enum/down.sql b/hasura.planx.uk/migrations/1715954131936_create_table_public_flow_status_enum/down.sql new file mode 100644 index 0000000000..8931600fa4 --- /dev/null +++ b/hasura.planx.uk/migrations/1715954131936_create_table_public_flow_status_enum/down.sql @@ -0,0 +1,11 @@ +DROP TRIGGER IF EXISTS flow_status_history_trigger on flows; +DROP FUNCTION IF EXISTS track_flow_status_history(); + +DROP TABLE "public"."flow_status_history"; + +alter table + "public"."flows" drop constraint "flows_status_fkey"; + +ALTER TABLE flows DROP COLUMN "status"; + +DROP TABLE "public"."flow_status_enum"; \ No newline at end of file diff --git a/hasura.planx.uk/migrations/1715954131936_create_table_public_flow_status_enum/up.sql b/hasura.planx.uk/migrations/1715954131936_create_table_public_flow_status_enum/up.sql new file mode 100644 index 0000000000..14176cc170 --- /dev/null +++ b/hasura.planx.uk/migrations/1715954131936_create_table_public_flow_status_enum/up.sql @@ -0,0 +1,74 @@ +-- Setup enum table of possible values for flow.status +CREATE TABLE "public"."flow_status_enum" ( + "value" text NOT NULL, + "comment" text, + PRIMARY KEY ("value") +); + +COMMENT ON TABLE "public"."flow_status_enum" IS E'An enum for tracking the status of a flow'; + +INSERT INTO "public"."flow_status_enum"("value", "comment") VALUES (E'online', null); +INSERT INTO "public"."flow_status_enum"("value", "comment") VALUES (E'offline', null); + +-- Add flow.status column +alter table "public"."flows" add column "status" text + not null default 'offline'; + +alter table "public"."flows" + add constraint "flows_status_fkey" + foreign key ("status") + references "public"."flow_status_enum" + ("value") on update restrict on delete restrict; + +-- Create audit table to track changes to status +-- Could be used for analytics or other audit features in future +CREATE TABLE "public"."flow_status_history" ( + "id" serial NOT NULL, + "flow_id" uuid NOT NULL, + "status" text NOT NULL, + "event_start" timestamptz NOT NULL, + "event_end" timestamptz, + PRIMARY KEY ("id"), + FOREIGN KEY ("flow_id") REFERENCES "public"."flows"("id") ON UPDATE restrict ON DELETE cascade, + FOREIGN KEY ("status") REFERENCES "public"."flow_status_enum"("value") ON UPDATE restrict ON DELETE restrict, + UNIQUE ("id") +); + +COMMENT ON TABLE "public"."flow_status_history" IS E'Temporal table to track the status of a flow over time'; + +-- Populate initial table values +-- All existing flows have had status "online" since they were created +INSERT INTO flow_status_history (flow_id, status, event_start) +SELECT id, 'online', created_at +FROM flows; + +-- Setup function which updates and adds audit records to flow_status_history +CREATE OR REPLACE FUNCTION track_flow_status_history() +RETURNS TRIGGER AS $$ +BEGIN + IF (TG_OP = 'UPDATE') THEN + -- End previous event + UPDATE flow_status_history + SET event_end = NOW() + WHERE flow_id = OLD.id AND event_end IS NULL; + + -- Start new event + INSERT INTO flow_status_history (flow_id, status, event_start) + VALUES (NEW.id, OLD.status, NOW()); + + ELSIF (TG_OP = 'INSERT') THEN + -- Start new event + INSERT INTO flow_status_history (flow_id, status, event_start) + VALUES (NEW.id, NEW.status, NOW()); + END IF; + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +-- Trigger to call above function +-- Called on insert or update to flows.status +CREATE TRIGGER flow_status_history_trigger +AFTER INSERT OR UPDATE OF status ON flows +FOR EACH ROW +EXECUTE FUNCTION track_flow_status_history(); + diff --git a/hasura.planx.uk/tests/flow_status_history.test.js b/hasura.planx.uk/tests/flow_status_history.test.js new file mode 100644 index 0000000000..cee2e236d4 --- /dev/null +++ b/hasura.planx.uk/tests/flow_status_history.test.js @@ -0,0 +1,78 @@ +const { introspectAs } = require("./utils"); + +describe("flows status history", () => { + describe("public", () => { + let i; + beforeAll(async () => { + i = await introspectAs("public"); + }); + + test("cannot query flow_status_history", () => { + expect(i.queries).not.toContain("flow_status_history"); + }); + + test("cannot create, update, or delete flows or their associated operations", () => { + expect(i).toHaveNoMutationsFor("flow_status_history"); + }); + }); + + describe("admin", () => { + let i; + beforeAll(async () => { + i = await introspectAs("admin"); + }); + + test("has full access to flow_status_history", () => { + expect(i.queries).toContain("flow_status_history"); + expect(i.mutations).toContain("insert_flow_status_history"); + expect(i.mutations).toContain("insert_flow_status_history_one"); + expect(i.mutations).toContain("update_flow_status_history_by_pk"); + expect(i.mutations).toContain("delete_flow_status_history"); + }); + }); + + describe("platformAdmin", () => { + let i; + beforeAll(async () => { + i = await introspectAs("platformAdmin"); + }); + + test("cannot query flow_status_history", () => { + expect(i.queries).not.toContain("flow_status_history"); + }); + + test("cannot create, update, or delete flows or their associated operations", () => { + expect(i).toHaveNoMutationsFor("flow_status_history"); + }); + }); + + describe("teamEditor", () => { + let i; + beforeAll(async () => { + i = await introspectAs("teamEditor"); + }); + + test("cannot query flow_status_history", () => { + expect(i.queries).not.toContain("flow_status_history"); + }); + + test("cannot create, update, or delete flows or their associated operations", () => { + expect(i).toHaveNoMutationsFor("flow_status_history"); + }); + }); + + describe("api", () => { + let i; + beforeAll(async () => { + i = await introspectAs("api"); + }); + + test("cannot query flow_status_history", () => { + expect(i.queries).not.toContain("flow_status_history"); + }); + + test("cannot create, update, or delete flows or their associated operations", () => { + expect(i).toHaveNoMutationsFor("flow_status_history"); + }); + }); +});