Skip to content
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.

Sign up for GitHub

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

Fixes to List Workflows Basic route handler #758

Merged
merged 2 commits into from
Dec 13, 2024
Merged

Fixes to List Workflows Basic route handler #758

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d1777b
Fixes to route handler
adhityamamallan Dec 10, 2024
27e38b4
Address comment
adhityamamallan Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 31 additions & 20 deletions src/route-handlers/list-workflows-basic/list-workflows-basic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { type NextRequest, NextResponse } from 'next/server';

import { type ListOpenWorkflowExecutionsRequest__Input } from '@/__generated__/proto-ts/uber/cadence/api/v1/ListOpenWorkflowExecutionsRequest';
import decodeUrlParams from '@/utils/decode-url-params';
import { getHTTPStatusCode, GRPCError } from '@/utils/grpc/grpc-error';
import logger, { type RouteHandlerErrorPayload } from '@/utils/logger';
Expand Down Expand Up @@ -38,33 +39,43 @@ export async function listWorkflowsBasic(
);
}

try {
const listEndpoint =
queryParams.kind === 'open'
? ctx.grpcClusterMethods.openWorkflows
: ctx.grpcClusterMethods.closedWorkflows;

const res = await listEndpoint({
domain: decodedParams.domain,
pageSize: queryParams.pageSize,
nextPageToken: queryParams.nextPage,
startTimeFilter: {
earliestTime: queryParams.timeRangeStart,
latestTime: queryParams.timeRangeEnd,
},
const baseParams = {
domain: decodedParams.domain,
pageSize: queryParams.pageSize,
nextPageToken: queryParams.nextPage,
startTimeFilter: {
earliestTime: queryParams.timeRangeStart,
latestTime: queryParams.timeRangeEnd,
},
...((queryParams.workflowId || queryParams.runId) && {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This looks cool, but might be easier to make it more readable explicitly:

var params = ...
if queryParams.workflowId || queryParams.runId {
   params.filters = 'executionFilter'
   executionFilter: {
        workflowId: queryParams.workflowId,
        runId: queryParams.runId,
      },
}
if queryParams.workflowType {
   params.filters = 'typeFilter'
   typeFilter: {
        name: queryParams.workflowType,
      },
}

Though, this is what we usually do in Go. I'm unsure about JS style :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this would be more JS-y; I tend to avoid lets and vars if possible.

filters: 'executionFilter',
executionFilter: {
workflowId: queryParams.workflowId,
runId: queryParams.runId,
},
}),
...(queryParams.workflowType && {
filters: 'typeFilter',
typeFilter: {
name: queryParams.workflowType,
},
...(queryParams.kind === 'closed' && {
statusFilter: {
status: queryParams.closeStatus,
},
}),
});
}),
} satisfies ListOpenWorkflowExecutionsRequest__Input;

try {
const res =
queryParams.kind === 'closed'
? await ctx.grpcClusterMethods.closedWorkflows({
...baseParams,
...(queryParams.kind === 'closed' &&
adhityamamallan marked this conversation as resolved.
Show resolved Hide resolved
queryParams.closeStatus && {
filters: 'statusFilter',
statusFilter: {
status: queryParams.closeStatus,
},
}),
})
: await ctx.grpcClusterMethods.openWorkflows({ ...baseParams });

const response: ListWorkflowsBasicResponse = {
workflows: mapExecutionsToWorkflows(res.executions),
Expand Down
59 changes: 39 additions & 20 deletions src/route-handlers/list-workflows-basic/schemas/list-workflows-basic-query-params-schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,44 @@ import getGrpcTimestampFromIso from '@/utils/datetime/get-grpc-timestamp-from-is
import isWorkflowStatus from '@/views/shared/workflow-status-tag/helpers/is-workflow-status';
import { type WorkflowStatus } from '@/views/shared/workflow-status-tag/workflow-status-tag.types';

const listWorkflowsBasicQueryParamsSchema = z.object({
kind: z.enum(['open', 'closed']),
pageSize: z
.string()
.transform((val) => parseInt(val, 10))
.pipe(
z.number().positive({ message: 'Page size must be a positive integer' })
),
workflowId: z.string().optional(),
workflowType: z.string().optional(),
runId: z.string().optional(),
closeStatus: z
.custom<WorkflowStatus>(isWorkflowStatus, {
message: 'Invalid workflow status',
})
.optional(),
timeRangeStart: z.string().datetime().transform(getGrpcTimestampFromIso),
timeRangeEnd: z.string().datetime().transform(getGrpcTimestampFromIso),
nextPage: z.string().optional(),
});
const listWorkflowsBasicQueryParamsSchema = z
.object({
kind: z.enum(['open', 'closed']),
pageSize: z
.string()
.transform((val) => parseInt(val, 10))
.pipe(
z.number().positive({ message: 'Page size must be a positive integer' })
),
workflowId: z.string().optional(),
workflowType: z.string().optional(),
runId: z.string().optional(),
closeStatus: z
.custom<WorkflowStatus>(isWorkflowStatus, {
message: 'Invalid workflow status',
})
.optional(),
timeRangeStart: z.string().datetime().transform(getGrpcTimestampFromIso),
timeRangeEnd: z.string().datetime().transform(getGrpcTimestampFromIso),
nextPage: z.string().optional(),
})
.refine((params) => {
let definedFiltersCount = 0;
// Execution filter
if (params.workflowId || params.runId) {
definedFiltersCount += 1;
}
// Type filter
if (params.workflowType) {
definedFiltersCount += 1;
}
// Status filter
if (params.closeStatus) {
definedFiltersCount += 1;
}

return definedFiltersCount <= 1;
// TODO @adhitya.mamallan - If we add RunID back, add it in the message below
}, 'Only one of the following filters is allowed - Workflow ID, Workflow Type, Close Status');

export default listWorkflowsBasicQueryParamsSchema;
Loading