-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[UI v2] feat: adds query to join flow runs with parent flow #17217
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
1 change: 1 addition & 0 deletions
1
ui-v2/src/api/flow-runs/use-paginate-flow-runs-with-flows/index.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { usePaginateFlowRunswithFlows } from "./use-paginate-flow-runs-with-flows"; |
101 changes: 101 additions & 0 deletions
101
...api/flow-runs/use-paginate-flow-runs-with-flows/use-paginate-flow-runs-with-flows.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
import { createFakeFlow, createFakeFlowRun } from "@/mocks"; | ||
|
||
import type { FlowRun } from "@/api/flow-runs"; | ||
import type { Flow } from "@/api/flows"; | ||
|
||
import { QueryClient } from "@tanstack/react-query"; | ||
import { renderHook, waitFor } from "@testing-library/react"; | ||
import { buildApiUrl, createWrapper, server } from "@tests/utils"; | ||
import { http, HttpResponse } from "msw"; | ||
import { describe, expect, it } from "vitest"; | ||
import { usePaginateFlowRunswithFlows } from "./use-paginate-flow-runs-with-flows"; | ||
|
||
describe("usePaginateFlowRunswithFlows", () => { | ||
const mockPaginateFlowRunsAPI = (flowRuns: Array<FlowRun>) => { | ||
server.use( | ||
http.post(buildApiUrl("/flow_runs/paginate"), () => { | ||
return HttpResponse.json({ | ||
limit: 10, | ||
page: 1, | ||
pages: 1, | ||
results: flowRuns, | ||
count: flowRuns.length, | ||
}); | ||
}), | ||
); | ||
}; | ||
|
||
const mockFilterFlowsAPI = (flows: Array<Flow>) => { | ||
server.use( | ||
http.post(buildApiUrl("/flows/filter"), () => { | ||
return HttpResponse.json(flows); | ||
}), | ||
); | ||
}; | ||
|
||
it("returns a pagination object with no results", async () => { | ||
// SETUP | ||
const queryClient = new QueryClient(); | ||
|
||
mockPaginateFlowRunsAPI([]); | ||
|
||
// TEST | ||
const { result } = renderHook( | ||
() => usePaginateFlowRunswithFlows({ page: 1, sort: "NAME_ASC" }), | ||
{ wrapper: createWrapper({ queryClient }) }, | ||
); | ||
|
||
await waitFor(() => expect(result.current.status).toEqual("success")); | ||
expect(result.current.data?.results).toHaveLength(0); | ||
}); | ||
|
||
it("returns a pagination object with joined flows and flow runs", async () => { | ||
// SETUP | ||
const queryClient = new QueryClient(); | ||
const MOCK_FLOW_RUN_0 = createFakeFlowRun({ | ||
id: "0", | ||
flow_id: "flow-id-0", | ||
}); | ||
const MOCK_FLOW_RUN_1 = createFakeFlowRun({ | ||
id: "0", | ||
flow_id: "flow-id-0", | ||
}); | ||
const MOCK_FLOW_RUN_2 = createFakeFlowRun({ | ||
id: "0", | ||
flow_id: "flow-id-1", | ||
}); | ||
const MOCK_FLOW_0 = createFakeFlow({ id: "flow-id-0" }); | ||
const MOCK_FLOW_1 = createFakeFlow({ id: "flow-id-1" }); | ||
|
||
const mockFlowRuns = [MOCK_FLOW_RUN_0, MOCK_FLOW_RUN_1, MOCK_FLOW_RUN_2]; | ||
const mockFlows = [MOCK_FLOW_0, MOCK_FLOW_1]; | ||
mockPaginateFlowRunsAPI(mockFlowRuns); | ||
mockFilterFlowsAPI(mockFlows); | ||
|
||
// TEST | ||
const { result } = renderHook( | ||
() => usePaginateFlowRunswithFlows({ page: 1, sort: "NAME_ASC" }), | ||
{ wrapper: createWrapper({ queryClient }) }, | ||
); | ||
|
||
await waitFor(() => expect(result.current.status).toEqual("success")); | ||
|
||
// ASSERT | ||
const EXPECTED = [ | ||
{ | ||
...MOCK_FLOW_RUN_0, | ||
flow: MOCK_FLOW_0, | ||
}, | ||
{ | ||
...MOCK_FLOW_RUN_1, | ||
flow: MOCK_FLOW_0, | ||
}, | ||
{ | ||
...MOCK_FLOW_RUN_2, | ||
flow: MOCK_FLOW_1, | ||
}, | ||
]; | ||
|
||
expect(result.current.data?.results).toEqual(EXPECTED); | ||
}); | ||
}); |
92 changes: 92 additions & 0 deletions
92
.../src/api/flow-runs/use-paginate-flow-runs-with-flows/use-paginate-flow-runs-with-flows.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
import { | ||
FlowRunWithFlow, | ||
type FlowRunsPaginateFilter, | ||
buildPaginateFlowRunsQuery, | ||
} from "@/api/flow-runs"; | ||
import { Flow, buildListFlowsQuery } from "@/api/flows"; | ||
import { useQuery } from "@tanstack/react-query"; | ||
import { useMemo } from "react"; | ||
|
||
/** | ||
* | ||
* @param filter | ||
* @returns a simplified query object that joins a flow run's pagination data with it's parent flow | ||
*/ | ||
export const usePaginateFlowRunswithFlows = ( | ||
filter: FlowRunsPaginateFilter, | ||
) => { | ||
const { data: paginateFlowRunsData, error: paginateFlowRunsError } = useQuery( | ||
buildPaginateFlowRunsQuery(filter), | ||
); | ||
|
||
const flowIds = useMemo(() => { | ||
if (!paginateFlowRunsData) { | ||
return []; | ||
} | ||
return paginateFlowRunsData.results.map((flowRun) => flowRun.flow_id); | ||
}, [paginateFlowRunsData]); | ||
|
||
const { data: flows, error: flowsError } = useQuery( | ||
buildListFlowsQuery( | ||
{ | ||
flows: { id: { any_: flowIds }, operator: "and_" }, | ||
offset: 0, | ||
sort: "CREATED_DESC", | ||
}, | ||
{ enabled: flowIds.length > 0 }, | ||
), | ||
); | ||
|
||
const flowMap = useMemo(() => { | ||
if (!flows) { | ||
return new Map<string, Flow>(); | ||
} | ||
return new Map(flows.map((flow) => [flow.id, flow])); | ||
}, [flows]); | ||
|
||
// If there's no results from the query, return empty | ||
if (paginateFlowRunsData && paginateFlowRunsData.results.length === 0) { | ||
return { | ||
status: "success" as const, | ||
error: null, | ||
data: { | ||
...paginateFlowRunsData, | ||
results: [] satisfies Array<FlowRunWithFlow>, | ||
}, | ||
}; | ||
} | ||
|
||
if (paginateFlowRunsData && flowMap.size > 0) { | ||
return { | ||
status: "success" as const, | ||
error: null, | ||
data: { | ||
...paginateFlowRunsData, | ||
results: paginateFlowRunsData.results.map((flowRun) => { | ||
const flow = flowMap.get(flowRun.flow_id); | ||
if (!flow) { | ||
throw new Error("Expecting parent flow to be found"); | ||
} | ||
return { | ||
...flowRun, | ||
flow, | ||
}; | ||
}), | ||
}, | ||
}; | ||
} | ||
|
||
if (paginateFlowRunsError || flowsError) { | ||
return { | ||
status: "error" as const, | ||
error: paginateFlowRunsError || flowsError, | ||
data: undefined, | ||
}; | ||
} | ||
|
||
return { | ||
status: "pending" as const, | ||
error: null, | ||
data: undefined, | ||
}; | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
would we be better off creating a single query action that does this? That way we can await the flow runs, await the flows, and then combine them. And then we can just use a normal query rather than this custom hook with its custom pending an error states.
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.
I remember seeing a thread with @desertaxle where we would want to do it this way, so that we won't find ourselves writing so many specific queries for joining data.
Should we continue doing this way?
I do see the benefit of having nice RQ object, but it can become an anti pattern to write specific joining queries vs using re-usable query configs.
@devangrose @desertaxle Any thoughts on how we should continue joining data?