From 637f116b58263e0be122d0f7fb81bdf5d28c4e66 Mon Sep 17 00:00:00 2001 From: Mike Heneghan Date: Fri, 3 Nov 2023 11:26:34 +0000 Subject: [PATCH 1/9] feat: store analytics link in new column metabase_link on flow table --- .../down.sql | 1 + .../up.sql | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/down.sql create mode 100644 hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/up.sql diff --git a/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/down.sql b/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/down.sql new file mode 100644 index 0000000000..e4781b54e1 --- /dev/null +++ b/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/down.sql @@ -0,0 +1 @@ +ALTER TABLE "public"."flows" DROP COLUMN "metabase_link"; diff --git a/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/up.sql b/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/up.sql new file mode 100644 index 0000000000..0b43519762 --- /dev/null +++ b/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/up.sql @@ -0,0 +1,2 @@ +alter table "public"."flows" add column "metabase_link" text + null; From 3f55f0d24b54d7440ef407207b01a517043f4165 Mon Sep 17 00:00:00 2001 From: Mike Heneghan Date: Mon, 6 Nov 2023 17:16:39 +0000 Subject: [PATCH 2/9] feat: conditionally render link to analytics in PreviewBrowser - Add analytcis link via bar chart icon - Only renders if the flow has a metabase link associated in the db --- .../FlowEditor/components/PreviewBrowser.tsx | 23 ++++++++++++++++++- .../src/pages/FlowEditor/lib/store/shared.ts | 3 +++ editor.planx.uk/src/routes/flow.tsx | 17 ++++++++++---- hasura.planx.uk/metadata/tables.yaml | 1 + 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx b/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx index aa762dce73..fbd69d9d88 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx @@ -10,7 +10,13 @@ import Tooltip from "@mui/material/Tooltip"; import Typography from "@mui/material/Typography"; import formatDistanceToNow from "date-fns/formatDistanceToNow"; import React, { useState } from "react"; -import { ExternalLink, Globe, RefreshCw, Terminal } from "react-feather"; +import { + BarChart, + ExternalLink, + Globe, + RefreshCw, + Terminal, +} from "react-feather"; import { useAsync } from "react-use"; import Input from "ui/Input"; @@ -93,6 +99,7 @@ const PreviewBrowser: React.FC<{ const [showDebugConsole, setDebugConsoleVisibility] = useState(false); const [ flowId, + flowMetabaseLink, resetPreview, publishFlow, lastPublished, @@ -100,6 +107,7 @@ const PreviewBrowser: React.FC<{ validateAndDiffFlow, ] = useStore((state) => [ state.id, + state.flowMetabaseLink, state.resetPreview, state.publishFlow, state.lastPublished, @@ -163,6 +171,19 @@ const PreviewBrowser: React.FC<{ + {!!flowMetabaseLink && ( + + + + + + )} + Store.node | undefined; resetPreview: () => void; @@ -51,6 +52,8 @@ export const sharedStore: StateCreator< flowName: "", + flowMetabaseLink: "", + id: "", previewEnvironment: "standalone", diff --git a/editor.planx.uk/src/routes/flow.tsx b/editor.planx.uk/src/routes/flow.tsx index c8f4ede53d..f3e2a84c48 100644 --- a/editor.planx.uk/src/routes/flow.tsx +++ b/editor.planx.uk/src/routes/flow.tsx @@ -173,10 +173,10 @@ const nodeRoutes = mount({ "/:parent/nodes/:id/edit": editNode, }); -const getFlowSettings = async ( +const getFlowMetadata = async ( flow: string, team: string, -): Promise => { +): Promise<{ settings: FlowSettings; flowMetabaseLink: string }> => { const { data } = await client.query({ query: gql` query GetFlow($slug: String!, $team_slug: String!) { @@ -186,6 +186,7 @@ const getFlowSettings = async ( ) { id settings + flowMetabaseLink: metabase_link } } `, @@ -194,7 +195,11 @@ const getFlowSettings = async ( team_slug: team, }, }); - return data.flows[0]?.settings; + const metadata = { + settings: data.flows[0]?.settings, + flowMetabaseLink: data.flows[0]?.flowMetabaseLink, + }; + return metadata; }; const routes = compose( @@ -204,8 +209,12 @@ const routes = compose( withView(async (req) => { const [flow, ...breadcrumbs] = req.params.flow.split(","); - const settings: FlowSettings = await getFlowSettings(flow, req.params.team); + const { settings, flowMetabaseLink } = await getFlowMetadata( + flow, + req.params.team, + ); useStore.getState().setFlowSettings(settings); + useStore.setState({ flowMetabaseLink }); return ( <> diff --git a/hasura.planx.uk/metadata/tables.yaml b/hasura.planx.uk/metadata/tables.yaml index 08721faad7..31e088b0f0 100644 --- a/hasura.planx.uk/metadata/tables.yaml +++ b/hasura.planx.uk/metadata/tables.yaml @@ -347,6 +347,7 @@ - creator_id - data - id + - metabase_link - settings - slug - team_id From 559b233326477e0aed30c3b447d16b27c2407c28 Mon Sep 17 00:00:00 2001 From: Mike Heneghan Date: Tue, 7 Nov 2023 10:35:49 +0000 Subject: [PATCH 3/9] refactor: update flowMetabaseLink type - As per: https://github.com/theopensystemslab/planx-new/pull/2387#discussion_r1383876991 - Update the flowMetabaseLink to handle the query returning null --- editor.planx.uk/src/pages/FlowEditor/lib/store/shared.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/store/shared.ts b/editor.planx.uk/src/pages/FlowEditor/lib/store/shared.ts index 2f31a63dd9..1b96cb8394 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/store/shared.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/store/shared.ts @@ -13,7 +13,7 @@ export interface SharedStore extends Store.Store { flow: Store.flow; flowSlug: string; flowName: string; - flowMetabaseLink: string; + flowMetabaseLink: string | null; id: string; getNode: (id: Store.nodeId) => Store.node | undefined; resetPreview: () => void; @@ -52,7 +52,7 @@ export const sharedStore: StateCreator< flowName: "", - flowMetabaseLink: "", + flowMetabaseLink: null, id: "", previewEnvironment: "standalone", From 60f9fc156867d75d641d186c1fe04ad47ebf1112 Mon Sep 17 00:00:00 2001 From: Mike Heneghan Date: Tue, 7 Nov 2023 11:07:46 +0000 Subject: [PATCH 4/9] refactor: rationalise the syntax for updating state with flowSettings and flowMetabaseLink - Consistently use the pattern of directly settings state attributes --- editor.planx.uk/src/routes/flow.tsx | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/editor.planx.uk/src/routes/flow.tsx b/editor.planx.uk/src/routes/flow.tsx index f3e2a84c48..5962de34dc 100644 --- a/editor.planx.uk/src/routes/flow.tsx +++ b/editor.planx.uk/src/routes/flow.tsx @@ -176,7 +176,7 @@ const nodeRoutes = mount({ const getFlowMetadata = async ( flow: string, team: string, -): Promise<{ settings: FlowSettings; flowMetabaseLink: string }> => { +): Promise<{ flowSettings: FlowSettings; flowMetabaseLink: string }> => { const { data } = await client.query({ query: gql` query GetFlow($slug: String!, $team_slug: String!) { @@ -185,7 +185,7 @@ const getFlowMetadata = async ( where: { slug: { _eq: $slug }, team: { slug: { _eq: $team_slug } } } ) { id - settings + flowSettings: settings flowMetabaseLink: metabase_link } } @@ -196,7 +196,7 @@ const getFlowMetadata = async ( }, }); const metadata = { - settings: data.flows[0]?.settings, + flowSettings: data.flows[0]?.flowSettings, flowMetabaseLink: data.flows[0]?.flowMetabaseLink, }; return metadata; @@ -209,12 +209,11 @@ const routes = compose( withView(async (req) => { const [flow, ...breadcrumbs] = req.params.flow.split(","); - const { settings, flowMetabaseLink } = await getFlowMetadata( + const { flowSettings, flowMetabaseLink } = await getFlowMetadata( flow, req.params.team, ); - useStore.getState().setFlowSettings(settings); - useStore.setState({ flowMetabaseLink }); + useStore.setState({ flowSettings, flowMetabaseLink }); return ( <> From 8d208e01eaa322626b56aadb49065a0048499e68 Mon Sep 17 00:00:00 2001 From: Mike Heneghan Date: Tue, 7 Nov 2023 11:18:10 +0000 Subject: [PATCH 5/9] fix: add permissions for teamEditors to view metabase_link --- hasura.planx.uk/metadata/tables.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/hasura.planx.uk/metadata/tables.yaml b/hasura.planx.uk/metadata/tables.yaml index 31e088b0f0..eb35927fdd 100644 --- a/hasura.planx.uk/metadata/tables.yaml +++ b/hasura.planx.uk/metadata/tables.yaml @@ -378,6 +378,7 @@ - creator_id - data - id + - metabase_link - settings - slug - team_id From ad518cab746e8530179175bca33dec08a9ae19d1 Mon Sep 17 00:00:00 2001 From: Mike Heneghan Date: Tue, 7 Nov 2023 12:17:42 +0000 Subject: [PATCH 6/9] refactor: conditionally render disabled link to analytics page when unavailable - As per: https://github.com/theopensystemslab/planx-new/pull/2387#discussion_r1384610938 - Move the analytics link to be the middle option to maintain groupings - As per: https://github.com/theopensystemslab/planx-new/pull/2387#discussion_r1384699943 - Render a disable version of the analytics page link to covney active/inactive to improve usability --- .../FlowEditor/components/PreviewBrowser.tsx | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx b/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx index fbd69d9d88..375e52fee7 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx @@ -160,18 +160,7 @@ const PreviewBrowser: React.FC<{ /> - - - - - - - {!!flowMetabaseLink && ( + {flowMetabaseLink ? ( + ) : ( + + + + + )} + + + + + + Date: Tue, 7 Nov 2023 15:41:06 +0000 Subject: [PATCH 7/9] refactor: add aria-disabled analytics link for accessibility --- .../src/pages/FlowEditor/components/PreviewBrowser.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx b/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx index 375e52fee7..69a3a9a246 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx @@ -173,7 +173,7 @@ const PreviewBrowser: React.FC<{ ) : ( - + From ef329a703f26c17d4557ff923c80eb4e5c6d6751 Mon Sep 17 00:00:00 2001 From: Mike Heneghan Date: Tue, 7 Nov 2023 15:57:06 +0000 Subject: [PATCH 8/9] refactor: rename metabase_link to analytics_link - As per: https://github.com/theopensystemslab/planx-new/pull/2387#discussion_r1384614481 - Best practice not to tie the variable name to a single provider --- .../src/pages/FlowEditor/components/PreviewBrowser.tsx | 8 ++++---- .../src/pages/FlowEditor/lib/store/shared.ts | 4 ++-- editor.planx.uk/src/routes/flow.tsx | 10 +++++----- hasura.planx.uk/metadata/tables.yaml | 4 ++-- .../down.sql | 1 + .../up.sql | 1 + 6 files changed, 15 insertions(+), 13 deletions(-) create mode 100644 hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/down.sql create mode 100644 hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/up.sql diff --git a/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx b/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx index 69a3a9a246..65d772e9e9 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/PreviewBrowser.tsx @@ -99,7 +99,7 @@ const PreviewBrowser: React.FC<{ const [showDebugConsole, setDebugConsoleVisibility] = useState(false); const [ flowId, - flowMetabaseLink, + flowAnalyticsLink, resetPreview, publishFlow, lastPublished, @@ -107,7 +107,7 @@ const PreviewBrowser: React.FC<{ validateAndDiffFlow, ] = useStore((state) => [ state.id, - state.flowMetabaseLink, + state.flowAnalyticsLink, state.resetPreview, state.publishFlow, state.lastPublished, @@ -160,10 +160,10 @@ const PreviewBrowser: React.FC<{ /> - {flowMetabaseLink ? ( + {flowAnalyticsLink ? ( Store.node | undefined; resetPreview: () => void; @@ -52,7 +52,7 @@ export const sharedStore: StateCreator< flowName: "", - flowMetabaseLink: null, + flowAnalyticsLink: null, id: "", previewEnvironment: "standalone", diff --git a/editor.planx.uk/src/routes/flow.tsx b/editor.planx.uk/src/routes/flow.tsx index 5962de34dc..caa1bd796c 100644 --- a/editor.planx.uk/src/routes/flow.tsx +++ b/editor.planx.uk/src/routes/flow.tsx @@ -176,7 +176,7 @@ const nodeRoutes = mount({ const getFlowMetadata = async ( flow: string, team: string, -): Promise<{ flowSettings: FlowSettings; flowMetabaseLink: string }> => { +): Promise<{ flowSettings: FlowSettings; flowAnalyticsLink: string }> => { const { data } = await client.query({ query: gql` query GetFlow($slug: String!, $team_slug: String!) { @@ -186,7 +186,7 @@ const getFlowMetadata = async ( ) { id flowSettings: settings - flowMetabaseLink: metabase_link + flowAnalyticsLink: analytics_link } } `, @@ -197,7 +197,7 @@ const getFlowMetadata = async ( }); const metadata = { flowSettings: data.flows[0]?.flowSettings, - flowMetabaseLink: data.flows[0]?.flowMetabaseLink, + flowAnalyticsLink: data.flows[0]?.flowAnalyticsLink, }; return metadata; }; @@ -209,11 +209,11 @@ const routes = compose( withView(async (req) => { const [flow, ...breadcrumbs] = req.params.flow.split(","); - const { flowSettings, flowMetabaseLink } = await getFlowMetadata( + const { flowSettings, flowAnalyticsLink } = await getFlowMetadata( flow, req.params.team, ); - useStore.setState({ flowSettings, flowMetabaseLink }); + useStore.setState({ flowSettings, flowAnalyticsLink }); return ( <> diff --git a/hasura.planx.uk/metadata/tables.yaml b/hasura.planx.uk/metadata/tables.yaml index eb35927fdd..84f69e67eb 100644 --- a/hasura.planx.uk/metadata/tables.yaml +++ b/hasura.planx.uk/metadata/tables.yaml @@ -347,7 +347,7 @@ - creator_id - data - id - - metabase_link + - analytics_link - settings - slug - team_id @@ -378,7 +378,7 @@ - creator_id - data - id - - metabase_link + - analytics_link - settings - slug - team_id diff --git a/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/down.sql b/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/down.sql new file mode 100644 index 0000000000..6ac9d973a5 --- /dev/null +++ b/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/down.sql @@ -0,0 +1 @@ +alter table "public"."flows" rename column "analytics_link" to "metabase_link"; diff --git a/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/up.sql b/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/up.sql new file mode 100644 index 0000000000..9d044830f5 --- /dev/null +++ b/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/up.sql @@ -0,0 +1 @@ +alter table "public"."flows" rename column "metabase_link" to "analytics_link"; From 4ed65920ad1abee9d12f9bf9d467fccf1d160e06 Mon Sep 17 00:00:00 2001 From: Mike Heneghan Date: Wed, 8 Nov 2023 14:35:55 +0000 Subject: [PATCH 9/9] chore: consolidate new column create and rename into one migration - As the name of the new column changed there was a create and a rename migration - Removed both of these and added a new migration which names the column correctly from the get go --- .../down.sql | 1 - .../up.sql | 2 -- .../down.sql | 1 - .../up.sql | 1 - .../down.sql | 1 + .../up.sql | 2 ++ 6 files changed, 3 insertions(+), 5 deletions(-) delete mode 100644 hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/down.sql delete mode 100644 hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/up.sql delete mode 100644 hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/down.sql delete mode 100644 hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/up.sql create mode 100644 hasura.planx.uk/migrations/1699453886259_alter_table_public_flows_add_column_analytics_link/down.sql create mode 100644 hasura.planx.uk/migrations/1699453886259_alter_table_public_flows_add_column_analytics_link/up.sql diff --git a/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/down.sql b/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/down.sql deleted file mode 100644 index e4781b54e1..0000000000 --- a/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/down.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE "public"."flows" DROP COLUMN "metabase_link"; diff --git a/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/up.sql b/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/up.sql deleted file mode 100644 index 0b43519762..0000000000 --- a/hasura.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/up.sql +++ /dev/null @@ -1,2 +0,0 @@ -alter table "public"."flows" add column "metabase_link" text - null; diff --git a/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/down.sql b/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/down.sql deleted file mode 100644 index 6ac9d973a5..0000000000 --- a/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/down.sql +++ /dev/null @@ -1 +0,0 @@ -alter table "public"."flows" rename column "analytics_link" to "metabase_link"; diff --git a/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/up.sql b/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/up.sql deleted file mode 100644 index 9d044830f5..0000000000 --- a/hasura.planx.uk/migrations/1699371548814_alter_table_public_flows_alter_column_metabase_link/up.sql +++ /dev/null @@ -1 +0,0 @@ -alter table "public"."flows" rename column "metabase_link" to "analytics_link"; diff --git a/hasura.planx.uk/migrations/1699453886259_alter_table_public_flows_add_column_analytics_link/down.sql b/hasura.planx.uk/migrations/1699453886259_alter_table_public_flows_add_column_analytics_link/down.sql new file mode 100644 index 0000000000..5aa66133a8 --- /dev/null +++ b/hasura.planx.uk/migrations/1699453886259_alter_table_public_flows_add_column_analytics_link/down.sql @@ -0,0 +1 @@ +ALTER TABLE "public"."flows" DROP COLUMN "analytics_link"; \ No newline at end of file diff --git a/hasura.planx.uk/migrations/1699453886259_alter_table_public_flows_add_column_analytics_link/up.sql b/hasura.planx.uk/migrations/1699453886259_alter_table_public_flows_add_column_analytics_link/up.sql new file mode 100644 index 0000000000..7d091e7dcf --- /dev/null +++ b/hasura.planx.uk/migrations/1699453886259_alter_table_public_flows_add_column_analytics_link/up.sql @@ -0,0 +1,2 @@ +alter table "public"."flows" add column "analytics_link" text +null; \ No newline at end of file