fix: no availability email frequency fix (#17430)

* pass teamID to only send to relevant team admins not org admins

* a lot more tests aroundthis

* improve test cases

* change reddis cache key to take into account teamID

* add start/endtime to template

* fix text in email

* add endTime to tests
This commit is contained in:
sean-brydon authored and GitHub committed 2024-11-01 14:54:34 -04:00
1 parent 1add8e73ee
commit 0c60ad1c82
5 files changed
+404 -20

No files matched your search

@@ -11,7 +11,9 @@ export type OrganizationAdminNoSlotsEmailInput = {
user: string;
slug: string;
startTime: string;
endTime: string;
editLink: string;
teamSlug: string;
};
export const OrganizationAdminNoSlotsEmail = (
@@ -33,14 +35,19 @@ export const OrganizationAdminNoSlotsEmail = (
<br />
<br />
Please note: It has been brought to our attention that {props.user} has not had any availability
when a user has visited {props.user}/{props.slug}
when a user has visited {props.teamSlug}/{props.slug}.
<br />
Start time: {props.startTime}
<br />
End time: {props.endTime}
<br />
<br />
There’s a few reasons why this could be happening
There’s a few reasons why this could be happening:
<br />
The user does not have any calendars connected
<br />
Their schedules attached to this event are not enabled
<ul>
<li>The user does not have any calendars connected</li>
<li>Their schedules attached to this event are not enabled</li>
</ul>
</Trans>
</p>
<div style={{ marginTop: "3rem", marginBottom: "0.75rem" }}>
@@ -13,6 +13,8 @@ export type OrganizationAdminNoSlotsEmailInput = {
user: string;
slug: string;
startTime: string;
endTime: string;
teamSlug: string;
editLink: string;
};
@@ -42,8 +44,8 @@ Hi Admins,
It has been brought to our attention that ${this.adminNoSlots.user} has not had availability users have visited ${this.adminNoSlots.user}/${this.adminNoSlots.slug}.
There’s a few reasons why this could be happening
The user does not have any calendars connected
Their schedules attached to this event are not enabled
* The user does not have any calendars connected
* Their schedules attached to this event are not enabled
We recommend checking their availability to resolve this
`;
@@ -77,21 +77,22 @@ describe("(Orgs) Send admin notifications when a user has no availability", () =
username: "user1",
eventSlug: "event1",
startTime: dayjs(), // Mocking Dayjs format function
endTime: dayjs().add(1, "hour"),
};
const orgDetails = {
currentOrgDomain: "org1",
isValidOrgDomain: true,
};
// Call the function
await handleNotificationWhenNoSlots({ eventDetails, orgDetails });
// Call the function with teamId
await handleNotificationWhenNoSlots({ eventDetails, orgDetails, teamId: 123 });
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).not.toHaveBeenCalled();
// Mock length to be one then recall to trigger email
mocked.lrange.mockResolvedValueOnce([""]);
await handleNotificationWhenNoSlots({ eventDetails, orgDetails });
await handleNotificationWhenNoSlots({ eventDetails, orgDetails, teamId: 123 });
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).toHaveBeenCalled();
});
it("Should not send a notification if the org has them disabled", async () => {
@@ -106,15 +107,371 @@ describe("(Orgs) Send admin notifications when a user has no availability", () =
username: "user1",
eventSlug: "event1",
startTime: dayjs(), // Mocking Dayjs format function
endTime: dayjs().add(1, "hour"),
};
const orgDetails = {
currentOrgDomain: "org1",
isValidOrgDomain: true,
};
// Call the function
await handleNotificationWhenNoSlots({ eventDetails, orgDetails });
await handleNotificationWhenNoSlots({ eventDetails, orgDetails, teamId: 123 });
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).not.toHaveBeenCalled();
});
it("Should only send notifications to admins of the specified teamId", async () => {
const redisService = new RedisService();
const mocked = vi.mocked(redisService);
prismaMock.team.findFirst.mockResolvedValue({
organizationSettings: {
adminGetsNoSlotsNotification: true,
},
});
// Mock finding team members
prismaMock.membership.findMany.mockResolvedValue([
{
user: {
email: "correctteam@test.com",
locale: "en",
},
},
]);
const eventDetails = {
username: "user1",
eventSlug: "event1",
startTime: dayjs(),
endTime: dayjs().add(1, "hour"),
};
const orgDetails = {
currentOrgDomain: "org1",
isValidOrgDomain: true,
};
// Mock Redis to simulate second no-slots occurrence
mocked.lrange.mockResolvedValueOnce([""]); // This will trigger email sending
// Call with specific teamId
await handleNotificationWhenNoSlots({
eventDetails,
orgDetails,
teamId: 123, // specific teamId
});
// Verify that membership query included correct teamId
expect(prismaMock.membership.findMany).toHaveBeenCalledWith(
expect.objectContaining({
where: expect.objectContaining({
team: expect.objectContaining({
id: 123,
}),
}),
})
);
// Verify email was sent only once (to the one correct team member)
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).toHaveBeenCalledTimes(1);
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).toHaveBeenCalledWith(
expect.objectContaining({
to: {
email: "correctteam@test.com",
},
})
);
});
it("Should not send notifications when no teamId is provided", async () => {
const redisService = new RedisService();
const mocked = vi.mocked(redisService);
prismaMock.team.findFirst.mockResolvedValue({
organizationSettings: {
adminGetsNoSlotsNotification: true,
},
});
const eventDetails = {
username: "user1",
eventSlug: "event1",
startTime: dayjs(),
endTime: dayjs().add(1, "hour"),
};
const orgDetails = {
currentOrgDomain: "org1",
isValidOrgDomain: true,
};
mocked.lrange.mockResolvedValueOnce([""]);
await handleNotificationWhenNoSlots({
eventDetails,
orgDetails,
// teamId intentionally omitted
});
expect(prismaMock.membership.findMany).not.toHaveBeenCalled();
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).not.toHaveBeenCalled();
});
it("Should not send notifications when no orgDomain is provided", async () => {
const redisService = new RedisService();
const mocked = vi.mocked(redisService);
const eventDetails = {
username: "user1",
eventSlug: "event1",
startTime: dayjs(),
endTime: dayjs().add(1, "hour"),
};
const orgDetails = {
currentOrgDomain: null, // No org domain
isValidOrgDomain: true,
};
mocked.lrange.mockResolvedValueOnce([""]);
await handleNotificationWhenNoSlots({
eventDetails,
orgDetails,
teamId: 123,
});
expect(prismaMock.team.findFirst).not.toHaveBeenCalled();
expect(prismaMock.membership.findMany).not.toHaveBeenCalled();
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).not.toHaveBeenCalled();
});
it("Should not send notifications when Redis environment variables are not set", async () => {
// Temporarily unset Redis env vars
const originalToken = process.env.UPSTASH_REDIS_REST_TOKEN;
const originalUrl = process.env.UPSTASH_REDIS_REST_URL;
delete process.env.UPSTASH_REDIS_REST_TOKEN;
delete process.env.UPSTASH_REDIS_REST_URL;
const eventDetails = {
username: "user1",
eventSlug: "event1",
startTime: dayjs(),
endTime: dayjs().add(1, "hour"),
};
const orgDetails = {
currentOrgDomain: "org1",
isValidOrgDomain: true,
};
await handleNotificationWhenNoSlots({
eventDetails,
orgDetails,
teamId: 123,
});
expect(prismaMock.team.findFirst).not.toHaveBeenCalled();
expect(prismaMock.membership.findMany).not.toHaveBeenCalled();
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).not.toHaveBeenCalled();
// Restore env vars
process.env.UPSTASH_REDIS_REST_TOKEN = originalToken;
process.env.UPSTASH_REDIS_REST_URL = originalUrl;
});
it("Should handle multiple admins correctly", async () => {
const redisService = new RedisService();
const mocked = vi.mocked(redisService);
prismaMock.team.findFirst.mockResolvedValue({
organizationSettings: {
adminGetsNoSlotsNotification: true,
},
});
// Mock multiple team admins
prismaMock.membership.findMany.mockResolvedValue([
{
user: {
email: "admin1@test.com",
locale: "en",
},
},
{
user: {
email: "admin2@test.com",
locale: "fr",
},
},
]);
const eventDetails = {
username: "user1",
eventSlug: "event1",
startTime: dayjs(),
endTime: dayjs().add(1, "hour"),
};
const orgDetails = {
currentOrgDomain: "org1",
isValidOrgDomain: true,
};
mocked.lrange.mockResolvedValueOnce([""]);
await handleNotificationWhenNoSlots({
eventDetails,
orgDetails,
teamId: 123,
});
// Verify emails were sent to both admins
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).toHaveBeenCalledTimes(2);
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).toHaveBeenCalledWith(
expect.objectContaining({
to: {
email: "admin1@test.com",
},
})
);
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).toHaveBeenCalledWith(
expect.objectContaining({
to: {
email: "admin2@test.com",
},
})
);
});
it("Should not send duplicate notifications within NO_SLOTS_NOTIFICATION_FREQUENCY", async () => {
const redisService = new RedisService();
const mocked = vi.mocked(redisService);
prismaMock.team.findFirst.mockResolvedValue({
organizationSettings: {
adminGetsNoSlotsNotification: true,
},
});
prismaMock.membership.findMany.mockResolvedValue([
{
user: {
email: "admin@test.com",
locale: "en",
},
},
]);
const eventDetails = {
username: "user1",
eventSlug: "event1",
startTime: dayjs(),
endTime: dayjs().add(1, "hour"),
};
const orgDetails = {
currentOrgDomain: "org1",
isValidOrgDomain: true,
};
// First notification cycle - simulate having one previous occurrence
mocked.lrange.mockResolvedValueOnce([""]); // One previous occurrence
await handleNotificationWhenNoSlots({
eventDetails,
orgDetails,
teamId: 123,
});
// Verify first notification was sent
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).toHaveBeenCalledTimes(1);
// Reset call counts
vi.clearAllMocks();
// For the second attempt, simulate having TWO occurrences already in Redis
// This better simulates the real Redis state after the first notification
mocked.lrange.mockResolvedValueOnce(["", ""]); // Two occurrences now
await handleNotificationWhenNoSlots({
eventDetails,
orgDetails,
teamId: 123,
});
// Verify no additional notifications were sent
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).not.toHaveBeenCalled();
// Verify Redis operations
expect(mocked.lpush).toHaveBeenCalledTimes(1); // Still records the occurrence
});
it("Should maintain separate notification frequencies for different event types", async () => {
const redisService = new RedisService();
const mocked = vi.mocked(redisService);
prismaMock.team.findFirst.mockResolvedValue({
organizationSettings: {
adminGetsNoSlotsNotification: true,
},
});
prismaMock.membership.findMany.mockResolvedValue([
{
user: {
email: "admin@test.com",
locale: "en",
},
},
]);
const baseEventDetails = {
username: "user1",
startTime: dayjs(),
endTime: dayjs().add(1, "hour"),
};
const orgDetails = {
currentOrgDomain: "org1",
isValidOrgDomain: true,
};
// First event type notification
mocked.lrange.mockResolvedValueOnce([""]); // Simulate one previous occurrence for first event
await handleNotificationWhenNoSlots({
eventDetails: { ...baseEventDetails, eventSlug: "event1" },
orgDetails,
teamId: 123,
});
// Verify first notification was sent
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).toHaveBeenCalledTimes(1);
// Reset only the email mock, keep Redis mocks
vi.mocked(CalcomEmails.sendOrganizationAdminNoSlotsNotification).mockClear();
// For the second event type, also simulate one previous occurrence
// This needs to be a separate mock since it's a different key in Redis
mocked.lrange.mockResolvedValueOnce([""]); // Simulate one previous occurrence for second event
await handleNotificationWhenNoSlots({
eventDetails: { ...baseEventDetails, eventSlug: "event2" },
orgDetails,
teamId: 123,
});
// Verify second notification was sent (different event type)
expect(CalcomEmails.sendOrganizationAdminNoSlotsNotification).toHaveBeenCalledTimes(1);
// Get all lpush calls
const lpushCalls = mocked.lpush.mock.calls;
expect(lpushCalls.length).toBe(2);
// Extract the Redis keys used for each event
const firstEventKey = lpushCalls[0][0];
const secondEventKey = lpushCalls[1][0];
// Verify different keys were used
expect(firstEventKey).not.toBe(secondEventKey);
expect(firstEventKey).toContain("event1");
expect(secondEventKey).toContain("event2");
});
});
@@ -9,6 +9,7 @@ type EventDetails = {
username: string;
eventSlug: string;
startTime: Dayjs;
endTime: Dayjs;
visitorTimezone?: string;
visitorUid?: string;
};
@@ -21,9 +22,9 @@ const NO_SLOTS_NOTIFICATION_FREQUENCY = IS_PRODUCTION ? 7 * 24 * 3600 : 60;
const NO_SLOTS_COUNT_FOR_NOTIFICATION = 2;
const constructRedisKey = (eventDetails: EventDetails, orgSlug?: string) => {
const constructRedisKey = (eventDetails: EventDetails, orgSlug?: string, teamId?: number) => {
return `${REDIS_KEY_VERSION}.${eventDetails.username}:${eventDetails.eventSlug}${
orgSlug ? `@${orgSlug}` : ""
orgSlug ? `@${orgSlug}:team_${teamId}` : ""
}`;
};
@@ -40,12 +41,14 @@ const constructDataHash = (eventDetails: EventDetails) => {
export const handleNotificationWhenNoSlots = async ({
eventDetails,
orgDetails,
teamId,
}: {
eventDetails: EventDetails;
orgDetails: { currentOrgDomain: string | null };
teamId?: number;
}) => {
// Check for org
if (!orgDetails.currentOrgDomain) return;
if (!orgDetails.currentOrgDomain || !teamId) return;
const UPSTASH_ENV_FOUND = process.env.UPSTASH_REDIS_REST_TOKEN && process.env.UPSTASH_REDIS_REST_URL;
if (!UPSTASH_ENV_FOUND) return;
@@ -68,7 +71,7 @@ export const handleNotificationWhenNoSlots = async ({
const redis = new RedisService();
const usersUniqueKey = constructRedisKey(eventDetails, orgDetails.currentOrgDomain);
const usersUniqueKey = constructRedisKey(eventDetails, orgDetails.currentOrgDomain, teamId);
// Get only the required amount of data so the request is as small as possible
// We may need to get more data and check the startDate occurrence of this
// Not trigger email if the start months are the same
@@ -82,12 +85,14 @@ export const handleNotificationWhenNoSlots = async ({
// We add one as we know we just added one to the list - saves us re-fetching the data
if (usersExistingNoSlots.length + 1 === NO_SLOTS_COUNT_FOR_NOTIFICATION) {
// Get all org admins to send the email too
// Get all team admins to send the email too
const foundAdmins = await prisma.membership.findMany({
where: {
team: {
slug: orgDetails.currentOrgDomain,
isOrganization: true,
id: teamId,
parentId: {
not: null,
},
},
role: {
in: ["ADMIN", "OWNER"],
@@ -102,6 +107,15 @@ export const handleNotificationWhenNoSlots = async ({
},
},
});
const teamSlug = await prisma.team.findUnique({
where: {
id: teamId,
},
select: {
slug: true,
},
});
// TODO: use new tasker as we dont want this blocking loading slots (Just out of scope for this PR)
// Tasker isn't 100% working with emails - will refactor after i have made changes to Tasker in another PR.
const emailsToSend: Array<Promise<void>> = [];
@@ -116,9 +130,11 @@ export const handleNotificationWhenNoSlots = async ({
},
user: eventDetails.username,
slug: eventDetails.eventSlug,
startTime: eventDetails.startTime.format("YYYY-MM"),
startTime: eventDetails.startTime.format("YYYY-MM-DD"),
endTime: eventDetails.endTime.format("YYYY-MM-DD"),
// For now navigate here - when impersonation via parameter has been pushed we will impersonate and then navigate to availability
editLink: `${WEBAPP_URL}/availability?type=team`,
teamSlug: teamSlug?.slug ?? "",
};
emailsToSend.push(sendOrganizationAdminNoSlotsNotification(payload));
@@ -846,9 +846,11 @@ async function _getAvailableSlots({ input, ctx }: GetScheduleOptions): Promise<I
eventDetails: {
username: input.usernameList?.[0],
startTime: startTime,
endTime: endTime,
eventSlug: eventType.slug,
},
orgDetails,
teamId: eventType.team?.id,
});
} catch (e) {
loggerWithEventDetails.error(