[KW-search] Enforce always passing parentId in connectors #9580
Conversation
…aSourceTableParents
| return [ | ||
| getHelpCenterCollectionInternalId(connectorId, collectionId), | ||
| ...parentIds, | ||
| getHelpCenterInternalId(connectorId, helpCenterId), |
There was a problem hiding this comment.
changed this purely to allow typing it as a tuple
| @@ -228,7 +228,7 @@ export async function upsertCollectionWithChildren({ | |||
| folderId: internalCollectionId, | |||
| title: collection.name, | |||
| parents: collectionParents, | |||
| parentId: collectionParents.length > 2 ? collectionParents[1] : null, | |||
| parentId: collectionParents[1], | |||
There was a problem hiding this comment.
as the type now shows, we at least have [collectionInternalId, ...maybe some parent collections, helpCenterInternalId], which is why this is correct
| @@ -252,7 +253,7 @@ export async function markPageHasVisited({ | |||
| interface ConfluenceUpsertPageInput { | |||
| page: NonNullable<Awaited<ReturnType<ConfluenceClient["getPageById"]>>>; | |||
| spaceName: string; | |||
| parents: string[]; | |||
| parents: [string, ...string[], string]; | |||
There was a problem hiding this comment.
you can't, i'm trying to enforce a Confluence logic rule that there's always gonna be the internalId and the spaceID (or the hidden node thing if parents are not already computed)
There was a problem hiding this comment.
Ah right it's confluence 👍 Thought it was datasource.ts
aubin-tchoi
requested review from
philipperolet
and removed request for
philipperolet
| @@ -252,7 +253,7 @@ export async function markPageHasVisited({ | |||
| interface ConfluenceUpsertPageInput { | |||
| page: NonNullable<Awaited<ReturnType<ConfluenceClient["getPageById"]>>>; | |||
| spaceName: string; | |||
| parents: string[]; | |||
| parents: [string, ...string[], string]; | |||
There was a problem hiding this comment.
I assume confluence activities were already passing parentIds appropriately
There was a problem hiding this comment.
(as per pr description actually :))
| @@ -149,7 +149,7 @@ export async function getParentIdsForArticle({ | |||
| connectorId: number; | |||
| parentCollectionId: string; | |||
| helpCenterId: string; | |||
| }) { | |||
| }): Promise<[string, string, ...string[], string]> { | |||
There was a problem hiding this comment.
I assume articles can't be at the root of a help center (not inside a collection), correct?
There was a problem hiding this comment.
That's an interesting one, AFAICT it is possible to create an orphaned article in Intercom by deleting the collection but these are not displayed correctly in Intercom nor in our connector
There was a problem hiding this comment.
And we don't fetch orphaned articles (we make the assumption that we have a collection everywhere) so the type is valid here 👍
| @@ -477,6 +478,7 @@ export async function syncFiles({ | |||
| dataSourceConfig, | |||
| folderId: createdOrUpdatedResource.internalId, | |||
| parents: [createdOrUpdatedResource.internalId, ...parents], | |||
| parentId: parents[0], | |||
There was a problem hiding this comment.
super nit: parents[1] everywhere in connectors, except here parents[0], is a bit confounding
There was a problem hiding this comment.
especially with parents[1] in the same file a bit below
There was a problem hiding this comment.
Indeed, I initially wrote it that way to keep the diff small, I'll change it now that it's been reviewed
Description
parentIdat every location where they were missing.parentIdmandatory on the interface from connectors to api/v1.Risk
Deploy Plan