chore: Unwrap JSONB columns in analytics_summary view
#2798
Conversation
| /** | ||
| * Describes the value held in analytics_logs.metadata | ||
| */ | ||
| type Metadata = |
There was a problem hiding this comment.
This might be a little redundant but it does mean we now have a documented type of the data held in this column.
There was a problem hiding this comment.
nit
I think this is worth adding and I think it better expresses what's going on I'm not sure it's 100% of the way there.
If I understand the intention of this correctly then there'd be a type called Metadata which would capture the structure of data that could be stored on the metadata column. The suggested union types shows that the full Metadata is never actually written in one mutation and is instead only even one of the nested types i.e. it might be HelpClickMetata or SelectedUrlsMetadata but these are done on different mutations.
To take the suggested change a bit further I'd add types for all of the discreet metadata and update so that whenever the metadata column is written to it is of type Metadata.
I've added a PR fro this here: #2805
|
Had a quick look and don't think it'll break anything (but it will help with some yet-unanswered questions!). |
|
Removed vultr server and associated DNS entries |
|
@Mike-Heneghan Please feel free to take over / amend / fix / merge this branch next week so that this can progress 👍 |
There was a problem hiding this comment.
Thanks for picking this up @DafyddLlyr it's an awesome change!
I had one comment on the change to types which is non-blocking but I have a PR to extend it a little which I'd be keen to get your perspective on but there's no rush to add.
|
|
||
| await publicClient.mutate({ | ||
| mutation: gql` | ||
| mutation UpdateHaInitiatedBackwardsNavigation( | ||
| mutation UpdateHasInitiatedBackwardsNavigation( |
| /** | ||
| * Describes the value held in analytics_logs.metadata | ||
| */ | ||
| type Metadata = |
There was a problem hiding this comment.
nit
I think this is worth adding and I think it better expresses what's going on I'm not sure it's 100% of the way there.
If I understand the intention of this correctly then there'd be a type called Metadata which would capture the structure of data that could be stored on the metadata column. The suggested union types shows that the full Metadata is never actually written in one mutation and is instead only even one of the nested types i.e. it might be HelpClickMetata or SelectedUrlsMetadata but these are done on different mutations.
To take the suggested change a bit further I'd add types for all of the discreet metadata and update so that whenever the metadata column is written to it is of type Metadata.
I've added a PR fro this here: #2805
There was a problem hiding this comment.
This is such a useful change 🥳
There was a problem hiding this comment.
nit
run_sql_migrations isn't a super meaningful name so it might be nice to call it something along the lines of alter_view_analytics_summary_unfold_metadata
Mike-Heneghan
force-pushed
the
dp/unfold-metadata-columns
branch
from
177eadc to
18b8332
Compare
What does this PR do?
user_agentcolumn intoplatform,os, andbrowserchange_metadata,back_metadata,help_metadataandselected_urlsI'll cross check with @zz-hh-aa before we move this to production that this won't be a breaking change