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

Jump to bottom

fix: over-fetching data in workflows when getting all workflows for eventtype #16879

Merged
merged 1 commit into from
Sep 30, 2024
Merged

fix: over-fetching data in workflows when getting all workflows for eventtype #16879

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 59 additions & 3 deletions packages/trpc/server/routers/viewer/workflows/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { SENDER_ID, SENDER_NAME } from "@calcom/lib/constants";
import getOrgIdFromMemberOrTeamId from "@calcom/lib/getOrgIdFromMemberOrTeamId";
import { getTeamIdFromEventType } from "@calcom/lib/getTeamIdFromEventType";
import logger from "@calcom/lib/logger";
import { EventTypeRepository } from "@calcom/lib/server/repository/eventType";
import { WorkflowRepository } from "@calcom/lib/server/repository/workflow";
import { getTimeFormatStringFromUserTimeFormat } from "@calcom/lib/timeFormat";
import prisma from "@calcom/prisma";
Expand Down Expand Up @@ -781,6 +780,63 @@ export const getEventTypeWorkflows = async (
userId: number,
eventTypeId: number
): Promise<z.infer<typeof ZWorkflows>> => {
const rawEventType = await EventTypeRepository.findById({ id: eventTypeId, userId });
return rawEventType?.workflows;
const workflows = await prisma.workflow.findMany({
Copy link
Member Author

Choose a reason for hiding this comment

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

@CarinaWolli i tested this and im pretty sure this works as expected - this query has been changed a bit from the one in eventTypes repo.

Targeting the workflow table directly instead of the event types. This generates some nicer SQL for what we needed. Also doesnt over fetch the data outside of eventTypes.workflows that was un-needed in this call

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would be a good opportunity to add as a method in the workflow repository.

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 have a separate issue for this! Gonna be extracting a lot of the current workflow repository to a service and handling that migration there. Mind if we pick that up there?

Copy link
Contributor

Choose a reason for hiding this comment

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

Excited to hear! I'm ok with that.

where: {
OR: [
{
userId: userId,
},
{
team: {
members: {
some: {
userId: userId,
},
},
},
},
],
activeOn: {
some: {
eventTypeId: eventTypeId,
},
},
},
select: {
name: true,
id: true,
trigger: true,
time: true,
timeUnit: true,
userId: true,
teamId: true,
team: {
select: {
id: true,
slug: true,
name: true,
members: true,
},
},
activeOn: {
select: {
eventType: {
select: {
id: true,
title: true,
parentId: true,
_count: {
select: {
children: true,
},
},
},
},
},
},
steps: true,
},
});

return workflows.map((workflow) => ({ workflow }));
};
Loading