diff --git a/doc/how-to/how-to-add-allow-list-variables.md b/doc/how-to/how-to-add-allow-list-variables.md new file mode 100644 index 0000000000..486d97f5ac --- /dev/null +++ b/doc/how-to/how-to-add-allow-list-variables.md @@ -0,0 +1,114 @@ +# How to add `ALLOW_LIST` variables for analytics + +## Context + +For PlanX, we are using a platform called Metabase to handle analytics for how people use the services from different local authorities. In order to tell Metabase what to pick up, we allow a number of variables to be exposed, these are contained in an array called `ALLOW_LIST`. You can see these in the database as columns in the views `analytics_summary` and `submission_services_summary`. + +In these docs I will run through the steps for adding a new variable to `ALLOW_LIST` both in the codebase and in Hasura. + +> [!WARNING] +> Only passport variables that store non-personally-identifiable values should be added to the allow list. Sensitive or personally-identifiable data should **never** be tracked or exposed via analytics. + +## Process + +### Step 1 - Add variables to `ALLOW_LIST` array + + You will need to add the desired variables to two places in the codebase + `api.planx.uk/modules/webhooks/service/analyzeSessions/operations.ts` and `editor.planx.uk/src/pages/FlowEditor/lib/analytics/provider.tsx`. + + Each file should have an array looking like the below: + + ```ts + export const ALLOW_LIST = [ + "application.declaration.connection", + "application.information.harmful", + "application.information.sensitive", + "application.type", + ... + "service.type", + "usedFOIYNPP", + "user.role", + *your variable*, + ] as const; + ``` + + Add your variable to this array. + + With both arrays now populated with the new variable we can now add them to our database views in Hasura + + >[!NOTE] + > Variables should be added in alphabetical order and relate to a passport variable + +### Step 2 - Adding variables to Hasura Views + + For this part, you will need to pull the variable out of the passport jsonb in the SQL script and put them into two database views,`analytics_summary` and `submission_services_summary`. Examples can be found already in the current view script for how to do this. + + Example to look for: + +```sql + /* Example from submission_services_summary */ + (((ls.allow_list_answers -> 'your.variable'::text) -> 0))::text AS your_variable_column_name + + /*Example from analytics_summary */ + (((al.allow_list_answers -> 'your.variable'::text) -> 0))::text AS your_variable_column_name +``` + + Example of how a view may look: + +```sql + CREATE OR REPLACE VIEW "public"."submission_services_summary" AS + WITH resumes_per_session AS + /* deleted the code here for readability */ + SELECT (ls.id)::text AS session_id, + t.slug AS team_slug, + f.slug AS service_slug, + ls.created_at, + ls.submitted_at, + ((ls.submitted_at)::date - (ls.created_at)::date) AS session_length_days, + ls.has_user_saved AS user_clicked_save, + rps.number_times_resumed, + ls.allow_list_answers, + ((ls.allow_list_answers -> 'proposal.projectType'::text))::text AS proposal_project_type, + ((ls.allow_list_answers -> 'application.declaration.connection'::text))::text AS application_declaration_connection, + ((ls.allow_list_answers -> 'property.type'::text))::text AS property_type, + ((ls.allow_list_answers -> 'drawBoundary.action'::text))::text AS draw_boundary_action, + ((ls.allow_list_answers -> 'user.role'::text))::text AS user_role, + ((ls.allow_list_answers -> 'property.constraints.planning'::text))::text AS property_constraints_planning, + /* deleted the code here for readability */ + sa.s3_applications, + ((ls.allow_list_answers -> 'usedFOIYNPP'::text))::text AS used_foiynpp, + ((ls.allow_list_answers -> 'propertyInformation.action'::text))::text AS property_information_action, + ((ls.allow_list_answers -> 'planningConstraints.action'::text))::text AS planning_constraints_action, + ((ls.allow_list_answers -> '_overrides'::text))::text AS overrides, + ((ls.allow_list_answers -> 'rab.exitReason'::text))::text AS rab_exit_reason, + ((ls.allow_list_answers -> 'service.type'::text))::text AS pre_app_service_type, + ((ls.allow_list_answers -> 'application.information.harmful'::text))::text AS pre_app_harmful_info, + ((ls.allow_list_answers -> 'application.information.sensitive'::text))::text AS pre_app_sensitive_info, + (((ls.allow_list_answers -> 'application.type'::text) -> 0))::text AS application_type + FROM lowcal_sessions ls + /* deleted the code here for readability */ + WHERE ((f.slug IS NOT NULL) AND (t.slug IS NOT NULL)); +``` + + The values here are being pulled from the table `lowcal_sessions.allow_list_answers` or `analytics_logs.allow_list_answers` + + > [!IMPORTANT] At the end of your SQL script after the view creation/replacement, it is important to add another line which ensures the new variable is read by Metabase + + + We currently have two views, so you should add these two lines to the end of the migration file, one for each view: + +```sql +GRANT SELECT ON "public"."analytics_summary" TO metabase_read_only; +GRANT SELECT ON "public"."submission_services_summary" TO metabase_read_only; +``` + + + 🎊🎉🎈 Now your new variable is ready for testing 🎈🎉🎊 + +### Step 3 - Testing your new `ALLOW_LIST` variable + + Now you can begin testing your a service where your new passport variable is set, and as it is populated in the passport, it should come through to your new view.column. + + Key to ensure that it is coming through in the right format, and with the expected value. A typical issue may be that it comes through as something like `['your value']` which will be read as `jsonb` by metabase and only allow boolean based filtering when creating dashboards, *does it exist or not* + + You will need to alter your SQL script for creating the new view to fix this diff --git a/editor.planx.uk/src/@planx/components/shared/FlagsSelect.tsx b/editor.planx.uk/src/@planx/components/shared/FlagsSelect.tsx index 46832c0a39..896154dc90 100644 --- a/editor.planx.uk/src/@planx/components/shared/FlagsSelect.tsx +++ b/editor.planx.uk/src/@planx/components/shared/FlagsSelect.tsx @@ -72,7 +72,7 @@ export const FlagsSelect: React.FC = (props) => { flag.text} groupBy={(flag) => flag.category} diff --git a/editor.planx.uk/src/ui/editor/ListManager/ListManager.tsx b/editor.planx.uk/src/ui/editor/ListManager/ListManager.tsx index 9fb64dc58a..1f8fc49f1c 100644 --- a/editor.planx.uk/src/ui/editor/ListManager/ListManager.tsx +++ b/editor.planx.uk/src/ui/editor/ListManager/ListManager.tsx @@ -38,7 +38,7 @@ export interface Props { const Item = styled(Box)(({ theme }) => ({ display: "flex", - marginBottom: theme.spacing(1), + marginBottom: theme.spacing(2), })); export default function ListManager( diff --git a/editor.planx.uk/src/ui/shared/SelectMultiple.tsx b/editor.planx.uk/src/ui/shared/SelectMultiple.tsx index 30d32f277b..8de9e6c9c4 100644 --- a/editor.planx.uk/src/ui/shared/SelectMultiple.tsx +++ b/editor.planx.uk/src/ui/shared/SelectMultiple.tsx @@ -27,13 +27,21 @@ type OptionalAutocompleteProps = Partial< Omit, "multiple"> >; -type Props = { +type WithLabel = { label: string; + placeholder?: never; } & RequiredAutocompleteProps & OptionalAutocompleteProps; +type WithPlaceholder = { + label?: never; + placeholder: string; +} & RequiredAutocompleteProps & + OptionalAutocompleteProps; + +type Props = WithLabel | WithPlaceholder; + const StyledAutocomplete = styled(Autocomplete)(({ theme }) => ({ - marginTop: theme.spacing(2), "& > div > label": { paddingRight: theme.spacing(3), }, @@ -104,9 +112,14 @@ export const CustomCheckbox = styled("span")(({ theme }) => ({ })); export function SelectMultiple(props: Props) { + // MUI doesn't pass the Autocomplete value along to the TextField automatically + const isSelectEmpty = !props.value?.length; + const placeholder = isSelectEmpty ? props.placeholder : undefined + return ( + sx={{ mt: props.label ? 2 : 0 }} role="status" aria-atomic={true} aria-live="polite" @@ -122,6 +135,7 @@ export function SelectMultiple(props: Props) { notched: false, }} label={props.label} + placeholder={placeholder} /> )} ChipProps={{ diff --git a/scripts/seed-database/write/flows.sql b/scripts/seed-database/write/flows.sql index 6244bdfe91..2f42bb6dc0 100644 --- a/scripts/seed-database/write/flows.sql +++ b/scripts/seed-database/write/flows.sql @@ -12,7 +12,9 @@ CREATE TEMPORARY TABLE sync_flows ( copied_from uuid, analytics_link text, status text, - name text + name text, + templated_from uuid, + description text ); \copy sync_flows FROM '/tmp/flows.csv' WITH (FORMAT csv, DELIMITER ';'); @@ -28,7 +30,9 @@ INSERT INTO flows ( copied_from, analytics_link, status, - name + name, + templated_from, + description ) SELECT id, @@ -41,7 +45,9 @@ SELECT copied_from, NULL, status, - name + name, + templated_from, + description FROM sync_flows ON CONFLICT (id) DO UPDATE SET @@ -54,7 +60,9 @@ SET copied_from = EXCLUDED.copied_from, analytics_link = NULL, status = EXCLUDED.status, - name = EXCLUDED.name; + name = EXCLUDED.name, + templated_from = EXCLUDED.templated_from, + description = EXCLUDED.description; -- ensure that original flows.version is overwritten to match new operation inserted below, else sharedb will fail UPDATE flows SET version = 1;