refactor: use tableIds as input to get table conn

Author: kevinkim-ogp

packages/backend/src/graphql/__tests__/queries/tiles/get-table-connections.itest.ts

@@ -33,7 +33,20 @@ describe('get table connections query', () => {
 
     const tableConnections = await getTableConnections(
       null,
-      { limit: 10, offset: 0 },
+      { tableIds: [] },
+      context,
+    )
+    expect(tableConnections).toEqual({})
+    expect(Object.keys(tableConnections).length).toBe(0)
+  })
+
+  it('should return empty object if user has no access to any tables', async () => {
+    const numTables = 5
+    const testIds = Array.from({ length: numTables }, () => crypto.randomUUID())
+
+    const tableConnections = await getTableConnections(
+      null,
+      { tableIds: testIds },
       context,
     )
     expect(tableConnections).toEqual({})
@@ -79,13 +92,12 @@ describe('get table connections query', () => {
 
       const tableConnections = await getTableConnections(
         null,
-        { limit, offset },
+        { tableIds: pageTableIds },
         context,
       )
       expect(Object.keys(tableConnections).length).toBe(expectedLength)
       expect(Object.keys(tableConnections).sort()).toEqual(pageTableIds.sort())
 
-      //   Object.keys(tableConnections).forEach()
       Object.entries(tableConnections).forEach(([key, value]) => {
         expect(value).toBe(tablePipeCount[key])
       })
@@ -140,16 +152,77 @@ describe('get table connections query', () => {
 
       const tableConnections = await getTableConnections(
         null,
-        { limit, offset },
+        { tableIds: pageTableIds },
         context,
       )
       expect(Object.keys(tableConnections).length).toBe(expectedLength)
       expect(Object.keys(tableConnections).sort()).toEqual(pageTableIds.sort())
 
-      //   Object.keys(tableConnections).forEach()
       Object.entries(tableConnections).forEach(([key, value]) => {
         expect(value).toBe(tablePipeCount[key])
       })
     }
   })
+
+  it('should not return table connections for tables the user does not have access to', async () => {
+    const numTables = 5
+    const tablePipeCount: TablePipeCountObj = {}
+    for (let i = 0; i < numTables; i++) {
+      const res = await generateMockTable({ userId: context.currentUser.id })
+      const { id: tableId } = res.table
+      const { id: editorId } = res.editor
+
+      const [numFlows, numSteps] = [getRandNum(), getRandNum()]
+      const [editorFlows, editorSteps] = [getRandNum(), getRandNum()]
+
+      tablePipeCount[tableId] = numFlows + editorFlows
+      for (let i = 0; i < numFlows; i++) {
+        await generateMockFlow({
+          userId: context.currentUser.id,
+          tableId,
+          numSteps,
+        })
+      }
+      for (let i = 0; i < editorFlows; i++) {
+        await generateMockFlow({
+          userId: editorId,
+          tableId,
+          numSteps: editorSteps,
+        })
+      }
+    }
+
+    // test as a whole
+    const { edges, pageInfo } = await getTables(
+      null,
+      { limit: 10, offset: 0 },
+      context,
+    )
+
+    const pageTables = edges.map((edge) => edge.node)
+    expect(pageTables).toHaveLength(numTables)
+    expect(pageInfo.currentPage).toBe(1)
+    expect(pageInfo.totalCount).toBe(numTables)
+
+    const pageTableIds = edges.map((edge) => edge.node.id)
+    const testIds = [...pageTableIds, crypto.randomUUID()] // add a random table id
+    expect(testIds).toHaveLength(numTables + 1)
+    const tableConnections = await getTableConnections(
+      null,
+      { tableIds: testIds },
+      context,
+    )
+    expect(Object.keys(tableConnections).length).toBe(numTables)
+    expect(Object.keys(tableConnections).sort()).toEqual(pageTableIds.sort())
+
+    Object.entries(tableConnections).forEach(([key, value]) => {
+      expect(value).toBe(tablePipeCount[key])
+    })
+  })
+
+  it('should throw error if tableIds is null', async () => {
+    await expect(
+      getTableConnections(null, { tableIds: null }, context),
+    ).rejects.toThrow('tableIds is required')
+  })
 })

packages/backend/src/graphql/queries/tiles/get-table-connections.ts

@@ -1,9 +1,12 @@
+import { raw } from 'objection'
+
+import logger from '@/helpers/logger'
 import Flow from '@/models/flow'
+import Step from '@/models/step'
+import TableCollaborator from '@/models/table-collaborators'
 
 import type { QueryResolvers } from '../../__generated__/types.generated'
 
-import getTables from './get-tables'
-
 interface ExtendedFlow extends Flow {
   tableid: string
   count: number
@@ -15,39 +18,55 @@ interface TableConnection {
 
 const getTableConnections: QueryResolvers['getTableConnections'] = async (
   _parent,
-  { limit, offset, name },
+  params,
   context,
 ) => {
-  // get tables of currentUser in the current page
-  const tables = await getTables(_parent, { limit, offset, name }, context)
+  const { tableIds } = params
+  if (!tableIds) {
+    throw new Error('tableIds is required')
+  }
+
+  try {
+    // get distinct rows of tables used in flows
+    // returns flow id and table id
+    // MONITOR (ogp-kevin): add index on steps.parameters->>'tableId' if query is slow
+    const distinctTableFlows = await Flow.query()
+      .innerJoin(
+        Step.query()
+          .as('tileSteps')
+          .select(
+            raw("steps.parameters->>'tableId' AS tableid"),
+            'steps.flow_id',
+          )
+          .andWhere('steps.app_key', 'tiles')
+          .whereNotNull(raw("steps.parameters->>'tableId'"))
+          .whereIn(
+            raw("steps.parameters->>'tableId'"),
+            TableCollaborator.query()
+              .select(raw('"table_id"::TEXT AS table_id')) // Cast to TEXT, steps.parameters is JSONB
+              .whereIn('table_id', tableIds)
+              .where('user_id', context.currentUser.id),
+          ), // Only include tables that the user has access to
+        'flows.id',
+        'tileSteps.flow_id',
+      )
+      .select('tileSteps.tableid')
+      .countDistinct('flows.id')
+      .groupBy('tileSteps.tableid')
+
+    const result = distinctTableFlows.reduce(
+      (acc: TableConnection, row: ExtendedFlow) => {
+        acc[row.tableid] = row.count
+        return acc
+      },
+      {},
+    )
 
-  if (tables.edges.length === 0) {
-    return {}
+    return result
+  } catch (e) {
+    logger.error(e)
+    throw new Error('Error fetching table connections')
   }
-  const tableIds = tables.edges.map((t) => t.node.id)
-  const tableIdStr = tableIds.map((id) => `'${id}'`).join(', ')
-
-  // get distinct rows of tables used in flows
-  // returns flow id and table id
-  const distinctTableFlows = await Flow.query()
-    .select(Flow.raw("steps.parameters ->> 'tableId' AS tableid"))
-    .countDistinct('flows.id')
-    .innerJoinRelated('steps')
-    .innerJoinRelated('user')
-    .where('steps.app_key', 'tiles')
-    .whereRaw("steps.parameters ->> 'tableId' IS NOT NULL")
-    .whereRaw(`steps.parameters ->> 'tableId' IN (${tableIdStr})`)
-    .groupByRaw("steps.parameters ->> 'tableId'")
-
-  const result = distinctTableFlows.reduce(
-    (acc: TableConnection, row: ExtendedFlow) => {
-      acc[row.tableid] = row.count
-      return acc
-    },
-    {},
-  )
-
-  return result
 }
 
 export default getTableConnections

packages/frontend/src/graphql/queries/tiles/get-table-connections.ts

@@ -1,7 +1,7 @@
 import { gql } from '@apollo/client'
 
 export const GET_TABLE_CONNECTIONS = gql`
-  query GetTableConnections($limit: Int!, $offset: Int!, $name: String) {
-    getTableConnections(limit: $limit, offset: $offset, name: $name)
+  query GetTableConnections($tableIds: [String!]!) {
+    getTableConnections(tableIds: $tableIds)
   }
 `

packages/frontend/src/pages/Tiles/index.tsx

@@ -87,8 +87,19 @@ export default function Tiles(): JSX.Element {
     variables: queryVars,
   })
 
-  const { pageInfo, edges } = data?.getTables ?? {}
-  const tilesToDisplay = edges?.map(({ node }) => node) ?? []
+  const { pageInfo, edges = [] } = data?.getTables ?? {}
+
+  const { tilesToDisplay, tableIds } = edges.reduce<{
+    tilesToDisplay: TableMetadata[]
+    tableIds: string[]
+  }>(
+    (acc, { node }) => {
+      acc.tilesToDisplay.push(node)
+      acc.tableIds.push(node.id)
+      return acc
+    },
+    { tilesToDisplay: [], tableIds: [] },
+  )
 
   const hasNoUserTiles = tilesToDisplay.length === 0 && !isSearching
   const totalCount: number = pageInfo?.totalCount ?? 0
@@ -97,7 +108,7 @@ export default function Tiles(): JSX.Element {
   const { data: connectionsData, loading: isConnectionsLoading } = useQuery<{
     getTableConnections: JSON
   }>(GET_TABLE_CONNECTIONS, {
-    variables: queryVars,
+    variables: { tableIds },
     skip: hasNoUserTiles,
   })
   const tileConnections = connectionsData?.getTableConnections ?? {}