Skip to content

Commit 63406ca

Browse files
authored
PLU-310: show num pipes connected to a tile (#797)
## Problem Pipes are failing when users accidentally delete Tiles that are connected to existing Pipes. Closes [PLU-310: show connected pipes to tiles](https://linear.app/ogp/issue/PLU-310/show-connected-pipes-to-tiles) ## Solution Display the number of Pipes using the Tile, including Pipes of current user and collaborators of the Tile. **Improvements**: - Removed unnecessary menu button (current menu only shows "View" / "Delete") - User can view the Tile by clicking on the row - Delete button will be made visible to Tile owners when they hover a Tile ## Before & After Screenshots **BEFORE**: <img width="1512" alt="Screenshot 2024-11-15 at 3 05 26 PM" src="https://github.com/user-attachments/assets/235e190d-fdd5-47a4-b0d7-23c7a2b13ca5"> **AFTER**: - Show number of Pipes and remove menu <img width="1512" alt="Screenshot 2024-11-15 at 3 06 03 PM" src="https://github.com/user-attachments/assets/817af8d3-725d-47fc-a044-e2a096603a64"> - Show delete icon on hover <img width="1512" alt="Screenshot 2024-11-15 at 3 06 46 PM" src="https://github.com/user-attachments/assets/b087596b-1fac-4c5e-ac06-83a4c7e37cc6"> ## Tests - [x] Number of Pipes displayed is correct, including pipes of Tile collaborators - [x] Able to view Tiles for all roles (Owner / Editor / Viewer) - [x] Able to delete Tiles **New scripts**: - `packages/backend/src/graphql/__tests__/queries/tiles/get-table-connections.itest.ts` : tests for retrieving table connections - `packages/backend/src/graphql/queries/tiles/get-table-connections.ts` : query for retrieving table connections - `packages/frontend/src/graphql/queries/tiles/get-table-connections.ts` : query for retrieving table connections
1 parent f99941b commit 63406ca

File tree

9 files changed

+597
-97
lines changed

9 files changed

+597
-97
lines changed

packages/backend/src/graphql/__tests__/mutations/tiles/table.mock.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { randomUUID } from 'crypto'
22

3+
import Step from '@/models/step'
34
import TableCollaborator from '@/models/table-collaborators'
45
import TableColumnMetadata from '@/models/table-column-metadata'
56
import TableMetadata from '@/models/table-metadata'
@@ -52,6 +53,56 @@ export async function generateMockTable({
5253
}
5354
}
5455

56+
// generate mock flow including steps
57+
export async function generateMockFlow({
58+
userId,
59+
tableId,
60+
numSteps,
61+
}: {
62+
userId: string
63+
tableId: string
64+
numSteps: number
65+
}) {
66+
const currentUser = await User.query().findById(userId)
67+
68+
const tableName = `test-flow-${tableId}`
69+
const flowRes = await currentUser.$relatedQuery('flows').insert({
70+
name: tableName,
71+
})
72+
const flowId = flowRes.id
73+
74+
for (let i = 0; i < numSteps; i++) {
75+
await generateMockStep({
76+
tableId,
77+
flowId,
78+
position: i + 2,
79+
})
80+
}
81+
}
82+
83+
export async function generateMockStep({
84+
tableId,
85+
flowId,
86+
position,
87+
}: {
88+
tableId: string
89+
flowId: string
90+
position?: number
91+
}) {
92+
await Step.query().insert({
93+
key: 'createTileRow',
94+
appKey: 'tiles',
95+
type: 'action',
96+
parameters: {
97+
rowData: [],
98+
tableId: tableId,
99+
},
100+
flowId: flowId,
101+
status: 'incomplete',
102+
position: position,
103+
})
104+
}
105+
55106
export async function generateMockTableColumns({
56107
tableId,
57108
numColumns = 5,
Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
import crypto from 'crypto'
2+
import { beforeEach, describe, expect, it, vi } from 'vitest'
3+
4+
import getTableConnections from '@/graphql/queries/tiles/get-table-connections'
5+
import getTables from '@/graphql/queries/tiles/get-tables'
6+
import Flow from '@/models/flow'
7+
import Context from '@/types/express/context'
8+
9+
import {
10+
generateMockContext,
11+
generateMockFlow,
12+
generateMockTable,
13+
} from '../../mutations/tiles/table.mock'
14+
15+
interface TablePipeCountObj {
16+
[key: string]: number
17+
}
18+
19+
function getRandNum() {
20+
return crypto.randomInt(1, 6)
21+
}
22+
23+
describe('get table connections query', () => {
24+
let context: Context
25+
26+
beforeEach(async () => {
27+
context = await generateMockContext()
28+
})
29+
30+
it('should return empty object if no tables found', async () => {
31+
const { edges } = await getTables(null, { limit: 10, offset: 0 }, context)
32+
const tables = edges.map((edge) => edge.node)
33+
expect(tables).toHaveLength(0)
34+
35+
const tableConnections = await getTableConnections(
36+
null,
37+
{ tableIds: [] },
38+
context,
39+
)
40+
const flowQuerySpy = vi.spyOn(Flow, 'query')
41+
expect(flowQuerySpy).not.toHaveBeenCalled()
42+
expect(tableConnections).toEqual({})
43+
expect(Object.keys(tableConnections).length).toBe(0)
44+
})
45+
46+
it('should return empty object if user has no access to any tables', async () => {
47+
const numTables = 5
48+
const testIds = Array.from({ length: numTables }, () => crypto.randomUUID())
49+
50+
const tableConnections = await getTableConnections(
51+
null,
52+
{ tableIds: testIds },
53+
context,
54+
)
55+
expect(tableConnections).toEqual({})
56+
expect(Object.keys(tableConnections).length).toBe(0)
57+
})
58+
59+
it('should return the correct number of table connections for user flows only', async () => {
60+
const numTables = 5
61+
const tablePipeCount: TablePipeCountObj = {}
62+
for (let i = 0; i < numTables; i++) {
63+
const res = await generateMockTable({ userId: context.currentUser.id })
64+
const { id: tableId } = res.table
65+
66+
const [numFlows, numSteps] = [getRandNum(), getRandNum()]
67+
tablePipeCount[tableId] = numFlows
68+
69+
for (let i = 0; i < numFlows; i++) {
70+
await generateMockFlow({
71+
userId: context.currentUser.id,
72+
tableId,
73+
numSteps,
74+
})
75+
}
76+
}
77+
78+
// tests each page
79+
const limit = 2
80+
for (let i = 0; i < Math.ceil(numTables / 2); i++) {
81+
const offset = limit * i
82+
const { edges, pageInfo } = await getTables(
83+
null,
84+
{ limit, offset },
85+
context,
86+
)
87+
88+
const pageTables = edges.map((edge) => edge.node)
89+
const expectedLength = Math.min(limit, numTables - limit * i)
90+
expect(pageTables).toHaveLength(expectedLength)
91+
expect(pageInfo.currentPage).toBe(i + 1)
92+
expect(pageInfo.totalCount).toBe(numTables)
93+
94+
const pageTableIds = edges.map((edge) => edge.node.id)
95+
96+
const tableConnections = await getTableConnections(
97+
null,
98+
{ tableIds: pageTableIds },
99+
context,
100+
)
101+
expect(Object.keys(tableConnections).length).toBe(expectedLength)
102+
expect(Object.keys(tableConnections).sort()).toEqual(pageTableIds.sort())
103+
104+
Object.entries(tableConnections).forEach(([key, value]) => {
105+
expect(value).toBe(tablePipeCount[key])
106+
})
107+
}
108+
})
109+
110+
it('should return the correct number of table connections for flows by user and editor', async () => {
111+
const numTables = 5
112+
const tablePipeCount: TablePipeCountObj = {}
113+
for (let i = 0; i < numTables; i++) {
114+
const res = await generateMockTable({ userId: context.currentUser.id })
115+
const { id: tableId } = res.table
116+
const { id: editorId } = res.editor
117+
118+
const [numFlows, numSteps] = [getRandNum(), getRandNum()]
119+
const [editorFlows, editorSteps] = [getRandNum(), getRandNum()]
120+
121+
tablePipeCount[tableId] = numFlows + editorFlows
122+
for (let i = 0; i < numFlows; i++) {
123+
await generateMockFlow({
124+
userId: context.currentUser.id,
125+
tableId,
126+
numSteps,
127+
})
128+
}
129+
for (let i = 0; i < editorFlows; i++) {
130+
await generateMockFlow({
131+
userId: editorId,
132+
tableId,
133+
numSteps: editorSteps,
134+
})
135+
}
136+
}
137+
138+
// tests each page
139+
const limit = 2
140+
for (let i = 0; i < Math.ceil(numTables / 2); i++) {
141+
const offset = limit * i
142+
const { edges, pageInfo } = await getTables(
143+
null,
144+
{ limit, offset },
145+
context,
146+
)
147+
148+
const pageTables = edges.map((edge) => edge.node)
149+
const expectedLength = Math.min(limit, numTables - limit * i)
150+
expect(pageTables).toHaveLength(expectedLength)
151+
expect(pageInfo.currentPage).toBe(i + 1)
152+
expect(pageInfo.totalCount).toBe(numTables)
153+
154+
const pageTableIds = edges.map((edge) => edge.node.id)
155+
156+
const tableConnections = await getTableConnections(
157+
null,
158+
{ tableIds: pageTableIds },
159+
context,
160+
)
161+
expect(Object.keys(tableConnections).length).toBe(expectedLength)
162+
expect(Object.keys(tableConnections).sort()).toEqual(pageTableIds.sort())
163+
164+
Object.entries(tableConnections).forEach(([key, value]) => {
165+
expect(value).toBe(tablePipeCount[key])
166+
})
167+
}
168+
})
169+
170+
it('should not return table connections for tables the user does not have access to', async () => {
171+
const numTables = 5
172+
const tablePipeCount: TablePipeCountObj = {}
173+
for (let i = 0; i < numTables; i++) {
174+
const res = await generateMockTable({ userId: context.currentUser.id })
175+
const { id: tableId } = res.table
176+
const { id: editorId } = res.editor
177+
178+
const [numFlows, numSteps] = [getRandNum(), getRandNum()]
179+
const [editorFlows, editorSteps] = [getRandNum(), getRandNum()]
180+
181+
tablePipeCount[tableId] = numFlows + editorFlows
182+
for (let i = 0; i < numFlows; i++) {
183+
await generateMockFlow({
184+
userId: context.currentUser.id,
185+
tableId,
186+
numSteps,
187+
})
188+
}
189+
for (let i = 0; i < editorFlows; i++) {
190+
await generateMockFlow({
191+
userId: editorId,
192+
tableId,
193+
numSteps: editorSteps,
194+
})
195+
}
196+
}
197+
198+
// test as a whole
199+
const { edges, pageInfo } = await getTables(
200+
null,
201+
{ limit: 10, offset: 0 },
202+
context,
203+
)
204+
205+
const pageTables = edges.map((edge) => edge.node)
206+
expect(pageTables).toHaveLength(numTables)
207+
expect(pageInfo.currentPage).toBe(1)
208+
expect(pageInfo.totalCount).toBe(numTables)
209+
210+
const pageTableIds = edges.map((edge) => edge.node.id)
211+
const testIds = [...pageTableIds, crypto.randomUUID()] // add a random table id
212+
expect(testIds).toHaveLength(numTables + 1)
213+
const tableConnections = await getTableConnections(
214+
null,
215+
{ tableIds: testIds },
216+
context,
217+
)
218+
expect(Object.keys(tableConnections).length).toBe(numTables)
219+
expect(Object.keys(tableConnections).sort()).toEqual(pageTableIds.sort())
220+
221+
Object.entries(tableConnections).forEach(([key, value]) => {
222+
expect(value).toBe(tablePipeCount[key])
223+
})
224+
})
225+
226+
it('should throw error if tableIds is null', async () => {
227+
await expect(
228+
getTableConnections(null, { tableIds: null }, context),
229+
).rejects.toThrow('tableIds is required')
230+
})
231+
})
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { raw } from 'objection'
2+
3+
import logger from '@/helpers/logger'
4+
import Flow from '@/models/flow'
5+
import Step from '@/models/step'
6+
import TableCollaborator from '@/models/table-collaborators'
7+
8+
import type { QueryResolvers } from '../../__generated__/types.generated'
9+
10+
interface ExtendedFlow extends Flow {
11+
tableid: string
12+
count: number
13+
}
14+
15+
interface TableConnection {
16+
[key: string]: number
17+
}
18+
19+
const getTableConnections: QueryResolvers['getTableConnections'] = async (
20+
_parent,
21+
params,
22+
context,
23+
) => {
24+
const { tableIds } = params
25+
if (!tableIds) {
26+
throw new Error('tableIds is required')
27+
}
28+
if (tableIds.length === 0) {
29+
return {}
30+
}
31+
32+
try {
33+
// get distinct rows of tables used in flows
34+
// returns flow id and table id
35+
// MONITOR (ogp-kevin): add index on steps.parameters->>'tableId' if query is slow
36+
const distinctTableFlows = await Flow.query()
37+
.innerJoin(
38+
Step.query()
39+
.as('tileSteps')
40+
.select(
41+
raw("steps.parameters->>'tableId' AS tableid"),
42+
'steps.flow_id',
43+
)
44+
.andWhere('steps.app_key', 'tiles')
45+
.whereNotNull(raw("steps.parameters->>'tableId'"))
46+
.whereIn(
47+
raw("steps.parameters->>'tableId'"),
48+
TableCollaborator.query()
49+
.select(raw('"table_id"::TEXT AS table_id')) // Cast to TEXT, steps.parameters is JSONB
50+
.whereIn('table_id', tableIds)
51+
.where('user_id', context.currentUser.id),
52+
), // Only include tables that the user has access to
53+
'flows.id',
54+
'tileSteps.flow_id',
55+
)
56+
.select('tileSteps.tableid')
57+
.countDistinct('flows.id')
58+
.groupBy('tileSteps.tableid')
59+
60+
const result = distinctTableFlows.reduce(
61+
(acc: TableConnection, row: ExtendedFlow) => {
62+
acc[row.tableid] = row.count
63+
return acc
64+
},
65+
{},
66+
)
67+
68+
return result
69+
} catch (e) {
70+
logger.error(e)
71+
throw new Error('Error fetching table connections')
72+
}
73+
}
74+
75+
export default getTableConnections

packages/backend/src/graphql/queries/tiles/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@ import type { QueryResolvers } from '../../__generated__/types.generated'
22

33
import getAllRows from './get-all-rows'
44
import getTable from './get-table'
5+
import getTableConnections from './get-table-connections'
56
import getTables from './get-tables'
67

7-
export default { getTable, getTables, getAllRows } satisfies QueryResolvers
8+
export default {
9+
getTable,
10+
getTableConnections,
11+
getTables,
12+
getAllRows,
13+
} satisfies QueryResolvers

packages/backend/src/graphql/schema.graphql

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,8 @@ type Query {
3636
): [JSONObject]
3737
# Tiles
3838
getTable(tableId: String!): TableMetadata!
39-
getTables(
40-
limit: Int!
41-
offset: Int!
42-
name: String
43-
): PaginatedTables!
39+
getTableConnections(tableIds: [String!]!): JSONObject
40+
getTables(limit: Int!, offset: Int!, name: String): PaginatedTables!
4441
# Tiles rows
4542
getAllRows(tableId: String!): Any!
4643
getCurrentUser: User

0 commit comments

Comments
 (0)