-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add values to analytics allow lists for Pre Apps #3769
Conversation
const ALLOW_LIST = [ | ||
"application.declaration.connection", | ||
"application.information.harmful", | ||
"application.information.sensitive", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are both yes/no questions - see here https://editor.planx.uk/doncaster/pre-application-advice
They additionally have extra "description" & "reason" free text inputs each, but those seem less chart-able & potentially sensitve info so I'm not including here - is that okay @zz-hh-aa (it's a bit different than original Trello request!)?
Intention of analytics "allow lists" is to track generic, non-personally-identifiable stuff only in aggregate over all time. Doncaster of course will still get all answers with their submission data !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think that'll do just fine! Thanks Jess :)
Removed vultr server and associated DNS entries |
* ALLOW_LIST should stay in sync with | ||
* api.planx.uk/modules/webhooks/service/analyzeSessions/operations.ts | ||
* | ||
* If appending values to ALLOW_LIST please also update the | ||
* `analytics_summary` & `submission_services_summary` views to extract the value into its own column | ||
* | ||
* Please also ensure your migration ends with `GRANT SELECT ON public.{VIEW_NAME} TO metabase_read_only` | ||
* so that Metabase picks up the new columns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit(non-blocking): Maybe these comments point towards the fact we might need a simple doc file explaining how to add values to ALLOW_LIST
?
Changes:
ALLOW_LISTS
for Pre App servicesanalytics_summary
&submission_services_summary
viewsGRANT SELECT ON "public"."{view_name}" TO metabase_read_only;
so that Metabase should smoothly sync/pick up new columns 🤞