fix(data-ingestion): log end user #3086
Conversation
|
Tested in staging |
| const getEndUser = await getEndUserByConnectionId(db.knex, { connectionId: task.connection.id }); | ||
| endUser = getEndUser.isOk() | ||
| ? { id: getEndUser.value.id, endUserId: getEndUser.value.endUserId, orgId: getEndUser.value.organization?.organizationId || null } | ||
| : null; |
There was a problem hiding this comment.
endUser is already null so why not if(getEndUser.isOk()) { endUser = ... }
| @@ -158,3 +158,16 @@ export async function linkConnection(db: Knex, { endUserId, connection }: { endU | |||
| export async function unlinkConnection(db: Knex, { connection }: { connection: Pick<StoredConnection, 'id'> }) { | |||
| await db<StoredConnection>('_nango_connections').where({ id: connection.id! }).update({ end_user_id: null }); | |||
| } | |||
|
|
|||
| export async function getEndUserByConnectionId(db: Knex, props: { connectionId: number }): Promise<Result<EndUser, EndUserError>> { | |||
| const endUser = await db<DBEndUser>(END_USERS_TABLE) | |||
There was a problem hiding this comment.
do we need the generic here since it is specified by the select below?
| @@ -392,6 +392,7 @@ export interface NangoProps { | |||
| runnerFlags: RunnerFlags; | |||
| debug: boolean; | |||
| startedAt: Date; | |||
| endUser: { id: number; endUserId: string | null; orgId: string | null } | null; | |||
There was a problem hiding this comment.
why not endUser: EndUser | null
There was a problem hiding this comment.
Because the schema will change at some point and I don't need anything else, plus it's passing through zod which would be annoying to verify.
We do the same for team (and environment but differently)
| @@ -392,6 +392,7 @@ export interface NangoProps { | |||
| runnerFlags: RunnerFlags; | |||
| debug: boolean; | |||
| startedAt: Date; | |||
| endUser: { id: number; endUserId: string | null; orgId: string | null } | null; | |||
There was a problem hiding this comment.
we passing the endUser only so we can have it when handling the job result. Have you thought about fetching it from the db when starting the jobs AND again when handling the job result?
There was a problem hiding this comment.
yes, I'm not a fan. The way I see it, a job should be self-contained you send a payload and you receive the same thing, plus querying a new thing is one more possible way to fail. Tbf almost everything we put in NangoProps is not for execution per se.
Wdyt?
There was a problem hiding this comment.
I like the self-contained aspect of NangoProps but that's a lot of data doing the round trip. I am not sure what makes the most sense tbh
|
Im going to go ahead and merge since it's for billing, but happy to iterate if you find it useful |
Changes