Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/features/insights/server/trpc-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ function createInsightsBookingService(
options: {
scope,
userId: ctx.user.id,
orgId: ctx.user.organizationId ?? 0,
orgId: ctx.user.organizationId,
...(selectedTeamId && { teamId: selectedTeamId }),
},
filters: {
Expand Down
50 changes: 32 additions & 18 deletions packages/lib/server/service/InsightsBookingBaseService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
replaceDateRangeColumnFilter,
} from "@calcom/features/insights/lib/bookingUtils";
import type { DateRange } from "@calcom/features/insights/server/insightsDateUtils";
import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
import type { PrismaClient } from "@calcom/prisma";
import { Prisma } from "@calcom/prisma/client";
import { MembershipRole } from "@calcom/prisma/enums";
Expand Down Expand Up @@ -106,7 +107,7 @@ export const insightsBookingServiceOptionsSchema = z.discriminatedUnion("scope",
z.object({
scope: z.literal("user"),
userId: z.number(),
orgId: z.number(),
orgId: z.number().nullish().optional(),
}),
z.object({
scope: z.literal("org"),
Expand All @@ -116,15 +117,15 @@ export const insightsBookingServiceOptionsSchema = z.discriminatedUnion("scope",
z.object({
scope: z.literal("team"),
userId: z.number(),
orgId: z.number(),
orgId: z.number().nullish().optional(),
teamId: z.number(),
}),
]);

export type InsightsBookingServicePublicOptions = {
scope: "user" | "org" | "team";
userId: number;
orgId: number;
orgId: number | null | undefined;
teamId?: number;
};

Expand Down Expand Up @@ -435,13 +436,27 @@ export class InsightsBookingBaseService {
options: Extract<InsightsBookingServiceOptions, { scope: "team" }>
): Promise<Prisma.Sql> {
const teamRepo = new TeamRepository(this.prisma);
const childTeamOfOrg = await teamRepo.findByIdAndParentId({
id: options.teamId,
parentId: options.orgId,
select: { id: true },
});
if (options.orgId && !childTeamOfOrg) {
return NOTHING_CONDITION;

if (options.orgId) {
// team under org
const childTeamOfOrg = await teamRepo.findByIdAndParentId({
id: options.teamId,
parentId: options.orgId,
select: { id: true },
});
if (!childTeamOfOrg) {
// teamId and its orgId does not match
return NOTHING_CONDITION;
}
} else {
// standalone team
const team = await teamRepo.findById({
id: options.teamId,
});
if (team?.parentId) {
// a team without orgId is not supposed to have parentId
return NOTHING_CONDITION;
}
}

const usersFromTeam = await MembershipRepository.findAllByTeamIds({
Expand Down Expand Up @@ -1256,13 +1271,12 @@ export class InsightsBookingBaseService {
}

private async isOwnerOrAdmin(userId: number, targetId: number): Promise<boolean> {
// Check if the user is an owner or admin of the organization or team
const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({ userId, teamId: targetId });
return Boolean(
membership &&
membership.accepted &&
membership.role &&
(membership.role === MembershipRole.OWNER || membership.role === MembershipRole.ADMIN)
);
const permissionCheckService = new PermissionCheckService();
return await permissionCheckService.checkPermission({
userId,
teamId: targetId,
permission: "insights.read",
fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN],
});
}
}
53 changes: 33 additions & 20 deletions packages/lib/server/service/InsightsRoutingBaseService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ import {
isSingleSelectFilterValue,
} from "@calcom/features/data-table/lib/utils";
import type { DateRange } from "@calcom/features/insights/server/insightsDateUtils";
import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
import type { PrismaClient } from "@calcom/prisma";
import { Prisma } from "@calcom/prisma/client";
import type { BookingStatus } from "@calcom/prisma/enums";
import { MembershipRole } from "@calcom/prisma/enums";

import { MembershipRepository } from "../repository/membership";
import { TeamRepository } from "../repository/team";

export const insightsRoutingServiceOptionsSchema = z.discriminatedUnion("scope", [
z.object({
scope: z.literal("user"),
userId: z.number(),
orgId: z.number(),
orgId: z.number().nullish().optional(),
}),
z.object({
scope: z.literal("org"),
Expand All @@ -33,16 +33,16 @@ export const insightsRoutingServiceOptionsSchema = z.discriminatedUnion("scope",
z.object({
scope: z.literal("team"),
userId: z.number(),
orgId: z.number(),
orgId: z.number().nullish().optional(),
teamId: z.number(),
}),
]);

export type InsightsRoutingServicePublicOptions = {
scope: "user" | "org" | "team";
userId: number;
orgId: number | null;
teamId: number | undefined;
orgId: number | null | undefined;
teamId?: number;
};

export type InsightsRoutingServiceOptions = z.infer<typeof insightsRoutingServiceOptionsSchema>;
Expand Down Expand Up @@ -852,27 +852,40 @@ export class InsightsRoutingBaseService {
options: Extract<InsightsRoutingServiceOptions, { scope: "team" }>
): Promise<Prisma.Sql> {
const teamRepo = new TeamRepository(this.prisma);
const childTeamOfOrg = await teamRepo.findByIdAndParentId({
id: options.teamId,
parentId: options.orgId,
select: { id: true },
});
if (options.orgId && !childTeamOfOrg) {
return NOTHING_CONDITION;

if (options.orgId) {
// team under org
const childTeamOfOrg = await teamRepo.findByIdAndParentId({
id: options.teamId,
parentId: options.orgId,
select: { id: true },
});
if (!childTeamOfOrg) {
// teamId and its orgId does not match
return NOTHING_CONDITION;
}
} else {
// standalone team
const team = await teamRepo.findById({
id: options.teamId,
});
if (team?.parentId) {
// a team without orgId is not supposed to have parentId
return NOTHING_CONDITION;
}
}

return Prisma.sql`rfrd."formTeamId" = ${options.teamId}`;
}

private async isOwnerOrAdmin(userId: number, targetId: number): Promise<boolean> {
// Check if the user is an owner or admin of the organization or team
const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({ userId, teamId: targetId });
return Boolean(
membership &&
membership.accepted &&
membership.role &&
(membership.role === MembershipRole.OWNER || membership.role === MembershipRole.ADMIN)
);
const permissionCheckService = new PermissionCheckService();
return await permissionCheckService.checkPermission({
userId,
teamId: targetId,
permission: "insights.read",
fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN],
});
}

private buildFormFieldSqlCondition(fieldId: string, filterValue: FilterValue): Prisma.Sql | null {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import type { Team, User, Membership } from "@calcom/prisma/client";
import { Prisma } from "@calcom/prisma/client";
import { BookingStatus, MembershipRole } from "@calcom/prisma/enums";

import { InsightsBookingBaseService as InsightsBookingService } from "../InsightsBookingBaseService";
import {
InsightsBookingBaseService as InsightsBookingService,
type InsightsBookingServicePublicOptions,
} from "../InsightsBookingBaseService";

const NOTHING_CONDITION = Prisma.sql`1=0`;

Expand Down Expand Up @@ -204,7 +207,7 @@ describe("InsightsBookingService Integration Tests", () => {
it("should return NOTHING for invalid options", async () => {
const service = new InsightsBookingService({
prisma,
options: null as any,
options: null as unknown as InsightsBookingServicePublicOptions,
});

const conditions = await service.getAuthorizationConditions();
Expand Down Expand Up @@ -345,6 +348,170 @@ describe("InsightsBookingService Integration Tests", () => {

await testData.cleanup();
});

it("should build user scope conditions with null orgId", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});

const service = new InsightsBookingService({
prisma,
options: {
scope: "user",
userId: testData.user.id,
orgId: null,
},
});

const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`);

await testData.cleanup();
});

it("should build user scope conditions with undefined orgId", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});

const service = new InsightsBookingService({
prisma,
options: {
scope: "user",
userId: testData.user.id,
orgId: null,
},
});

const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`);

await testData.cleanup();
});
Comment on lines +373 to +392
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix test: “undefined orgId” case currently passes null.

Set orgId to undefined to actually exercise that path.

-          orgId: null,
+          orgId: undefined,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("should build user scope conditions with undefined orgId", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const service = new InsightsBookingService({
prisma,
options: {
scope: "user",
userId: testData.user.id,
orgId: null,
},
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`);
await testData.cleanup();
});
it("should build user scope conditions with undefined orgId", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const service = new InsightsBookingService({
prisma,
options: {
scope: "user",
userId: testData.user.id,
orgId: undefined,
},
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`);
await testData.cleanup();
});
🤖 Prompt for AI Agents
In
packages/lib/server/service/__tests__/InsightsBookingService.integration-test.ts
around lines 373 to 392, the test intends to exercise the "undefined orgId" path
but sets orgId: null; change the test to pass orgId: undefined in the service
options so the code path for undefined is executed (update the object literal to
orgId: undefined and leave the rest unchanged).


it("should build team scope conditions with null orgId for standalone team", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});

const standaloneTeam = await prisma.team.create({
data: {
name: "Standalone Team",
slug: `standalone-team-${Date.now()}-${Math.random().toString(36).substring(7)}`,
isOrganization: false,
parentId: null,
},
});

await prisma.membership.create({
data: {
userId: testData.user.id,
teamId: standaloneTeam.id,
role: MembershipRole.OWNER,
accepted: true,
},
});

const service = new InsightsBookingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: null,
teamId: standaloneTeam.id,
},
});

const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(
Prisma.sql`(("teamId" = ${standaloneTeam.id}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
testData.user.id,
]})) AND ("isTeamBooking" = false))`
);

await prisma.membership.deleteMany({
where: { teamId: standaloneTeam.id },
});
await prisma.team.delete({
where: { id: standaloneTeam.id },
});
await testData.cleanup();
});

it("should return NOTHING_CONDITION for team scope when team belongs to org but no orgId provided", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});

const service = new InsightsBookingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: null,
teamId: testData.team.id,
},
});

const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(NOTHING_CONDITION);

await testData.cleanup();
});

it("should build team scope conditions with undefined orgId for standalone team", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});

const standaloneTeam = await prisma.team.create({
data: {
name: "Standalone Team 2",
slug: `standalone-team-2-${Date.now()}-${Math.random().toString(36).substring(7)}`,
isOrganization: false,
parentId: null,
},
});

await prisma.membership.create({
data: {
userId: testData.user.id,
teamId: standaloneTeam.id,
role: MembershipRole.OWNER,
accepted: true,
},
});

const service = new InsightsBookingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: null,
teamId: standaloneTeam.id,
},
});

const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(
Prisma.sql`(("teamId" = ${standaloneTeam.id}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
testData.user.id,
]})) AND ("isTeamBooking" = false))`
);

await prisma.membership.deleteMany({
where: { teamId: standaloneTeam.id },
});
await prisma.team.delete({
where: { id: standaloneTeam.id },
});
await testData.cleanup();
});
Comment on lines +466 to +514
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix test: “undefined orgId” case currently passes null (team scope).

Set orgId to undefined.

-          orgId: null,
+          orgId: undefined,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("should build team scope conditions with undefined orgId for standalone team", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const standaloneTeam = await prisma.team.create({
data: {
name: "Standalone Team 2",
slug: `standalone-team-2-${Date.now()}-${Math.random().toString(36).substring(7)}`,
isOrganization: false,
parentId: null,
},
});
await prisma.membership.create({
data: {
userId: testData.user.id,
teamId: standaloneTeam.id,
role: MembershipRole.OWNER,
accepted: true,
},
});
const service = new InsightsBookingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: null,
teamId: standaloneTeam.id,
},
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(
Prisma.sql`(("teamId" = ${standaloneTeam.id}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
testData.user.id,
]})) AND ("isTeamBooking" = false))`
);
await prisma.membership.deleteMany({
where: { teamId: standaloneTeam.id },
});
await prisma.team.delete({
where: { id: standaloneTeam.id },
});
await testData.cleanup();
});
it("should build team scope conditions with undefined orgId for standalone team", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const standaloneTeam = await prisma.team.create({
data: {
name: "Standalone Team 2",
slug: `standalone-team-2-${Date.now()}-${Math.random().toString(36).substring(7)}`,
isOrganization: false,
parentId: null,
},
});
await prisma.membership.create({
data: {
userId: testData.user.id,
teamId: standaloneTeam.id,
role: MembershipRole.OWNER,
accepted: true,
},
});
const service = new InsightsBookingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: undefined,
teamId: standaloneTeam.id,
},
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(
Prisma.sql`(("teamId" = ${standaloneTeam.id}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
testData.user.id,
]})) AND ("isTeamBooking" = false))`
);
await prisma.membership.deleteMany({
where: { teamId: standaloneTeam.id },
});
await prisma.team.delete({
where: { id: standaloneTeam.id },
});
await testData.cleanup();
});
🤖 Prompt for AI Agents
In
packages/lib/server/service/__tests__/InsightsBookingService.integration-test.ts
around lines 466 to 514, the test intended to exercise the "undefined orgId"
case incorrectly passes orgId: null when constructing the InsightsBookingService
options; update the test to pass orgId: undefined instead (i.e., replace orgId:
null with orgId: undefined) so the service receives an undefined orgId as
intended and run the test to confirm it still types and behaves correctly.

});

describe("Filter Conditions", () => {
Expand Down
Loading
Loading