Mh/metadata types update

[Mike-Heneghan]

What

• Expand on the update to types as outlined here: chore: Unwrap JSONB columns in analytics_summary view #2798

Why

• I'm not sure but I think this expands on the logic of implementing the Metadata type

[github-actions]

Removed vultr server and associated DNS entries

[jessicamcinchak]

I was surprised to see a migration here - but it looks to be the same migration from #2798, right? If you change your base branch to Daf's branch it may display a more meaningful comparison of changes, then if #2798 merges first it should automatically switch to main again & rebase ?

[Mike-Heneghan]

I created my branch off of Daf's but when I created the PR I didn't notice I was pointing at main I'll get that sorted 👍

[jessicamcinchak]

It seems that this change means the type is less desscriptive/meaningful rather than more - I'm not sure I understand?

If trackHelpClick has type HelpClickMetadata then it already is valid Metadata right? But with this update, I could incorrectly pass trackHelpClick({ "back": { ...metadata } }) and I don't think it wouldn't get caught, right?

[Mike-Heneghan]

If I understand Daf's intention with adding Metadata I think it's nice to make clear the expected content that could be written to the metadata jsonb column which could have anything written to it but we're setting our expectations which I really like and could see the value of. I was associating all functions which write to the column with Metadata so that it's clear it'll be one of the types it's composed of.

Although, as you say it does mean we're not enforcing the types on the functions which as you say is less descriptive.

I've updated my PR to expand on the Metadata but the new type never gets used anywhere preserving the more explicit types on the functions 🤔

Rather than go with this approach it would maybe be best to remove the Metadata type?

[jessicamcinchak]

In my mind this distinction is:

• specific types like HelpClickMetadata should be used to restrict the arguments passed to methods like trackHelpClick
• the broad union type Metadata should be used to define what the GraphQL requests (mutations and queries alike) for this JSONB column expect

[Mike-Heneghan]

I definitely agree with that @jessicamcinchak although the bit I'm not sure on is whether we can actually assign the Metadata type to the GraphQL request, is that what you're suggesting?

With trackHelpClick I think the function is associated with HelpClickMetadata but there's no where to add Metadata as I understand it although I think I might be missing something.

  async function trackHelpClick(metadata?: HelpClickMetadata) {
    if (shouldSkipTracking()) return;

    await publicClient.mutate({
      mutation: gql`
        mutation UpdateHasClickedHelp($id: bigint!, $metadata: jsonb = {}) {
          update_analytics_logs_by_pk(
            pk_columns: { id: $id }
            _set: { has_clicked_help: true }
            _append: { metadata: $metadata }
          ) {
            id
          }
        }
      `,
      variables: {
        id: lastVisibleNodeAnalyticsLogId,
        metadata,
      },
    });
  }

Or would you suggest we maintain the Metadata but just have it in the file not being called but serving as a marker?

I'm a little bamboozled I think but it would be good to fix this PR / close it as I fear I'm wasting your time with this.

[Mike-Heneghan]

As discussed I'll leave the Metadata type in the file with a comment explaining it describes that the mutation which write to metadata will have payload which can be described by one of the options in the union.

[Mike-Heneghan]

These have to be optional as some change events we can't access the target nodeId so we only track change: {}