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.

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

feat: org-wide workflows #15083

Open
wants to merge 118 commits into
base: main
Choose a base branch
from
Open

feat: org-wide workflows #15083

wants to merge 118 commits into from

Conversation

CarinaWolli
Copy link
Member

@CarinaWolli CarinaWolli commented May 16, 2024

What does this PR do?

  • Org-workflow can be set active on teams and will trigger for all the team event types + all the user event types of the team members.
    • if a user is member of several active teams, the workflow only triggers once
  • Add 'apply to all, including future teams/event-types'
  • workflows/update.handler is refactored into smaller functions that can be reused and tested.
  • Also fixes some small existing bugs that I found when refactoring the code

Fixes #14638
Fixes CAL-3489

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected)
  • I have added a Docs issue here if this PR makes changes that would require a documentation change --> Created an issue for that: Org-wide workflows docs#93
  • I have added or modified automated tests that prove my fix is effective or that my feature works (PRs might be rejected if logical changes are not properly tested)

How should this be tested?

Basic test:

  • Create an org workflow
  • Set active on a team (or all teams)
  • Book a team event type --> see if workflow triggers
  • Book a user event type of a team member --> see if workflow triggers

Test other cases, for example:

  • test with several teams when some team members are part of several teams --> a workflow should always just trigger once
  • Test updating a 'before event' workflow and see if WorkflowReminders are correctly adjusted
  • Test removing team workflow and see if reminders are deleted (if user is still part of another team that is active, the reminder should stay)
  • delete/add reminders for deleted/added steps
  • Add reminders for new active teams
  • Test if 'active on all teams/even-types, including future ones' works with new event types/teams

Comment on lines -191 to -205
const reminderMethods: { [x: string]: (id: number, referenceId: string | null) => void } = {
[WorkflowMethods.EMAIL]: deleteScheduledEmailReminder,
[WorkflowMethods.SMS]: deleteScheduledSMSReminder,
[WorkflowMethods.WHATSAPP]: deleteScheduledWhatsappReminder,
};

export const cancelWorkflowReminders = async (
workflowReminders: { method: WorkflowMethods; id: number; referenceId: string | null }[]
) => {
await Promise.all(
workflowReminders.map((reminder) => {
return reminderMethods[reminder.method](reminder.id, reminder.referenceId);
})
);
};
Copy link
Member Author

Choose a reason for hiding this comment

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

moved to viewer/workflows/util.ts and renamed to deleteWorkflowReminders


if (orgId) {
if (teamId) {
const orgTeamWorkflowsRel = await prisma.workflowsOnTeams.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.

all active org workflows that are active on the team of the event type

const orgTeamWorkflows = orgTeamWorkflowsRel?.map((workflowRel) => workflowRel.workflow) ?? [];
allWorkflows.push(...orgTeamWorkflows);
} else if (userId) {
const orgUserWorkflowsRel = await prisma.workflowsOnTeams.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.

all active org workflows where user is part of a team that is active on workflow

allWorkflows.push(...orgUserWorkflows);
}

const activeOnAllOrgWorkflows = 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.

all org workflows where isActiveOnAll is true, duplicated are removed later

}

if (teamId) {
const activeOnAllTeamWorkflows = 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.

All normal team workflows where isActiveOnAll is true. Other active workflows are already included in eventTypeWorkflows

Comment on lines -42 to -79
const verifyEmailSender = async (
email: string,
userId: number,
teamId: number | null,
prisma: PrismaClient
) => {
const verifiedEmail = await prisma.verifiedEmail.findFirst({
where: {
email,
OR: [{ userId }, { teamId }],
},
});

if (!verifiedEmail) {
throw new TRPCError({ code: "NOT_FOUND", message: "Email not verified" });
}

if (teamId) {
if (!verifiedEmail.teamId) {
await prisma.verifiedEmail.update({
where: {
id: verifiedEmail.id,
},
data: {
teamId,
},
});
} else if (verifiedEmail.teamId !== teamId) {
await prisma.verifiedEmail.create({
data: {
email,
userId,
teamId,
},
});
}
}
};
Copy link
Member Author

Choose a reason for hiding this comment

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

for (const newEventTypeId of newActiveEventTypes) {
const newEventType = await ctx.prisma.eventType.findFirst({
if (!isOrg) {
// activeOn are event types ids
Copy link
Member Author

Choose a reason for hiding this comment

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

This code didn't change much from before

eventTypeId,
})),
});
} else {
Copy link
Member Author

Choose a reason for hiding this comment

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

the else part handles updating org workflows, that's new code

}

//todo: test this
if (userWorkflow.trigger !== trigger) {
Copy link
Member Author

Choose a reason for hiding this comment

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

updating already scheduled reminders when trigger is changed was not handled before, this was a bug

Comment on lines -50 to -88
const eventTypeOptions = useMemo(
() =>
data?.eventTypeGroups.reduce((options, group) => {
/** don't show team event types for user workflow */
if (!teamId && group.teamId) return options;
/** only show correct team event types for team workflows */
if (teamId && teamId !== group.teamId) return options;
return [
...options,
...group.eventTypes
.filter(
(evType) =>
!evType.metadata?.managedEventConfig ||
!!evType.metadata?.managedEventConfig.unlockedFields?.workflows ||
!!teamId
)
.map((eventType) => ({
value: String(eventType.id),
label: `${eventType.title} ${
eventType.children && eventType.children.length ? `(+${eventType.children.length})` : ``
}`,
})),
];
}, [] as Option[]) || [],
[data]
);

let allEventTypeOptions = eventTypeOptions;
const distinctEventTypes = new Set();

if (!teamId && isMixedEventType) {
allEventTypeOptions = [...eventTypeOptions, ...selectedEventTypes];
allEventTypeOptions = allEventTypeOptions.filter((option) => {
const duplicate = distinctEventTypes.has(option.value);
distinctEventTypes.add(option.value);
return !duplicate;
});
}

Copy link
Member Author

Choose a reason for hiding this comment

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

moved to parent component workflow.tsx

@CarinaWolli CarinaWolli marked this pull request as ready for review June 11, 2024 13:54
@CarinaWolli CarinaWolli requested a review from a team June 11, 2024 13:54
@graphite-app graphite-app bot requested a review from a team June 11, 2024 13:54
@dosubot dosubot bot added bookings area: bookings, availability, timezones, double booking teams area: teams, round robin, collective, managed event-types workflows area: workflows, automations ✨ feature New feature or request labels Jun 11, 2024
Copy link

graphite-app bot commented Jun 11, 2024

Graphite Automations

"Add foundation team as reviewer" took an action on this PR • (06/11/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add consumer team as reviewer" took an action on this PR • (06/11/24)

1 reviewer was added to this PR based on Keith Williams's automation.

@hariombalhara
Copy link
Member

hariombalhara commented Jun 12, 2024

Workflow count is wrong on the left. This is the case for Acme's owner's event

Also, the Org workflow toggle isn't in its disabled state visually
image

@hariombalhara
Copy link
Member

hariombalhara commented Jun 12, 2024

I think to clearly communicate the meaning, we should change the message to Active on all teams and their members

Initially I thought it could be just the team events but it is those team's members' events as well.

image

Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

Getting my head around workflows right now. So, submitting partial review

return workflow;
}

describe("deleteRemindersFromRemovedActiveOn", () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
describe("deleteRemindersFromRemovedActiveOn", () => {
describe("deleteRemindersOfActiveOnIds", () => {

//team event types + children managed event types
booking: {
eventType: {
OR: [{ teamId: teamId }, { parentId: teamId, teamId: null }],
Copy link
Member

Choose a reason for hiding this comment

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

Isn't parentId an eventTypeId ?

Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

Getting my head around workflows right now. So, submitting partial review

@hariombalhara
Copy link
Member

Have we handled the cleanup of workflows when a user is removed from a team. In my testing I found that the reminder remains even when there is no team.

No Team is there. The user was a member of a team earlier
image

But workflow that came from Org is still there
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bookings area: bookings, availability, timezones, double booking consumer core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ✨ feature New feature or request High priority Created by Linear-GitHub Sync ❗️ migrations contains migration files organizations area: organizations, orgs teams area: teams, round robin, collective, managed event-types workflows area: workflows, automations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-3489] org-wide workflows
3 participants