-
Notifications
You must be signed in to change notification settings - Fork 11
fix: Show parent nodes for job relationships #247
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -247,7 +247,7 @@ const getNodeDataForResource = async ( | |
|
|
||
| type Node = Awaited<ReturnType<typeof getNodeDataForResource>>; | ||
|
|
||
| const getNodesRecursivelyHelper = async ( | ||
| const getChildrenNodesRecursivelyHelper = async ( | ||
| db: Tx, | ||
| node: Node, | ||
| nodes: NonNullable<Node>[], | ||
|
|
@@ -286,15 +286,76 @@ const getNodesRecursivelyHelper = async ( | |
| const children = await Promise.all(childrenPromises); | ||
|
|
||
| const childrenNodesPromises = children.map((c) => | ||
| getNodesRecursivelyHelper(db, c, []), | ||
| getChildrenNodesRecursivelyHelper(db, c, []), | ||
| ); | ||
| const childrenNodes = (await Promise.all(childrenNodesPromises)).flat(); | ||
| return [...nodes, node, ...childrenNodes].filter(isPresent); | ||
| }; | ||
|
|
||
| const getNodesRecursively = async (db: Tx, resourceId: string) => { | ||
| const getChildrenNodesRecursively = async (db: Tx, resourceId: string) => { | ||
| const baseNode = await getNodeDataForResource(db, resourceId); | ||
| return getNodesRecursivelyHelper(db, baseNode, []); | ||
| return getChildrenNodesRecursivelyHelper(db, baseNode, []); | ||
| }; | ||
|
|
||
| type ParentNodesResult = { | ||
| parentNodes: NonNullable<Node>[]; | ||
| node: Node; | ||
| }; | ||
|
|
||
| const getParentNodesRecursivelyHelper = async ( | ||
| db: Tx, | ||
| node: Node, | ||
| nodes: NonNullable<Node>[], | ||
| ): Promise<ParentNodesResult> => { | ||
| if (node == null) return { parentNodes: nodes, node }; | ||
|
|
||
| const parentJob = await db | ||
| .select() | ||
| .from(schema.jobResourceRelationship) | ||
| .innerJoin( | ||
| schema.job, | ||
| eq(schema.jobResourceRelationship.jobId, schema.job.id), | ||
| ) | ||
| .innerJoin( | ||
| schema.releaseJobTrigger, | ||
| eq(schema.releaseJobTrigger.jobId, schema.job.id), | ||
| ) | ||
| .innerJoin( | ||
| schema.resource, | ||
| eq(schema.releaseJobTrigger.resourceId, schema.resource.id), | ||
| ) | ||
| .where( | ||
| and( | ||
| eq(schema.jobResourceRelationship.resourceIdentifier, node.identifier), | ||
| isNull(schema.resource.deletedAt), | ||
| eq(schema.resource.workspaceId, node.workspaceId), | ||
| ), | ||
| ) | ||
| .orderBy(desc(schema.releaseJobTrigger.createdAt)) | ||
| .limit(1) | ||
| .then(takeFirstOrNull); | ||
|
|
||
| if (parentJob == null) return { parentNodes: nodes, node }; | ||
|
|
||
| const parentNode = await getNodeDataForResource(db, parentJob.resource.id); | ||
| if (parentNode == null) return { parentNodes: nodes, node }; | ||
|
|
||
| const { job_resource_relationship: parentRelationship } = parentJob; | ||
|
|
||
| const { parentNodes, node: parentNodeWithData } = | ||
| await getParentNodesRecursivelyHelper(db, parentNode, []); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential issue with node accumulation in recursive parent function In Apply this diff to pass the const { parentNodes, node: parentNodeWithData } =
- await getParentNodesRecursivelyHelper(db, parentNode, []);
+ await getParentNodesRecursivelyHelper(db, parentNode, nodes);
|
||
|
|
||
| const nodeWithParent = { ...node, parent: parentRelationship }; | ||
|
|
||
| return { | ||
| parentNodes: [...parentNodes, parentNodeWithData].filter(isPresent), | ||
| node: nodeWithParent, | ||
| }; | ||
| }; | ||
|
|
||
| const getParentNodesRecursively = async (db: Tx, resourceId: string) => { | ||
| const baseNode = await getNodeDataForResource(db, resourceId); | ||
| return getParentNodesRecursivelyHelper(db, baseNode, []); | ||
| }; | ||
|
|
||
| export const resourceRouter = createTRPCRouter({ | ||
|
|
@@ -361,9 +422,17 @@ export const resourceRouter = createTRPCRouter({ | |
| where: eq(schema.resource.id, input), | ||
| }); | ||
| if (resource == null) return null; | ||
| const childrenNodes = await getNodesRecursively(ctx.db, input); | ||
| const childrenNodes = await getChildrenNodesRecursively(ctx.db, input); | ||
| const { parentNodes, node } = await getParentNodesRecursively( | ||
| ctx.db, | ||
| input, | ||
| ); | ||
|
|
||
| const childrenNodesUpdated = childrenNodes.map((n) => | ||
| n.id === node?.id ? node : n, | ||
| ); | ||
|
|
||
| const fromNodesPromises = ctx.db | ||
| const nodesQuery = ctx.db | ||
| .select() | ||
| .from(schema.resourceRelationship) | ||
| .innerJoin( | ||
|
|
@@ -372,7 +441,9 @@ export const resourceRouter = createTRPCRouter({ | |
| schema.resourceRelationship.fromIdentifier, | ||
| schema.resource.identifier, | ||
| ), | ||
| ) | ||
| ); | ||
|
|
||
| const fromNodesPromises = nodesQuery | ||
| .where( | ||
| and( | ||
| eq(schema.resourceRelationship.workspaceId, resource.workspaceId), | ||
|
|
@@ -387,16 +458,7 @@ export const resourceRouter = createTRPCRouter({ | |
| ) | ||
| .then((promises) => Promise.all(promises)); | ||
|
|
||
| const toNodesPromises = ctx.db | ||
| .select() | ||
| .from(schema.resourceRelationship) | ||
| .innerJoin( | ||
| schema.resource, | ||
| eq( | ||
| schema.resourceRelationship.toIdentifier, | ||
| schema.resource.identifier, | ||
| ), | ||
| ) | ||
| const toNodesPromises = nodesQuery | ||
| .where( | ||
| and( | ||
| eq(schema.resourceRelationship.workspaceId, resource.workspaceId), | ||
|
|
@@ -419,7 +481,8 @@ export const resourceRouter = createTRPCRouter({ | |
| return { | ||
| resource, | ||
| nodes: [ | ||
| ...childrenNodes, | ||
| ...parentNodes, | ||
| ...childrenNodesUpdated, | ||
| ...fromNodes.map((n) => n.node), | ||
| ...toNodes.map((n) => n.node), | ||
| ].filter(isPresent), | ||
|
|
||
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.
Potential issue with node accumulation in recursive function
In
getChildrenNodesRecursivelyHelper, you are passing an empty array[]as thenodesparameter in the recursive calls at line 289. This may result in only immediate child nodes being collected, without accumulating nodes from deeper levels of the tree. Consider passing the existingnodesarray to properly accumulate all descendant nodes.Apply this diff to pass the
nodesarray in recursive calls: