diff --git a/apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts b/apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts index 4e54f316e1..6e6b73b9dd 100644 --- a/apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts +++ b/apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts @@ -889,6 +889,8 @@ export class BookingsService_2024_08_13 { const res = await handleCancelBooking({ bookingData: bookingRequest.body, userId: bookingRequest.userId, + userUuid: authUser?.uuid, + actionSource: "API_V2", arePlatformEmailsEnabled: bookingRequest.arePlatformEmailsEnabled, platformClientId: bookingRequest.platformClientId, platformCancelUrl: bookingRequest.platformCancelUrl, diff --git a/apps/web/app/api/cancel/route.ts b/apps/web/app/api/cancel/route.ts index d217305b83..f16ad65ae1 100644 --- a/apps/web/app/api/cancel/route.ts +++ b/apps/web/app/api/cancel/route.ts @@ -41,6 +41,8 @@ async function handler(req: NextRequest) { const result = await handleCancelBooking({ bookingData, userId: session?.user?.id || -1, + userUuid: session?.user?.uuid, + actionSource: "WEBAPP", }); // const bookingCancelService = getBookingCancelService(); diff --git a/packages/features/booking-audit/lib/actions/CancelledAuditActionService.ts b/packages/features/booking-audit/lib/actions/CancelledAuditActionService.ts index 81607f2d85..4dcb6f5dfa 100644 --- a/packages/features/booking-audit/lib/actions/CancelledAuditActionService.ts +++ b/packages/features/booking-audit/lib/actions/CancelledAuditActionService.ts @@ -1,6 +1,6 @@ import { z } from "zod"; -import { StringChangeSchema } from "../common/changeSchemas"; +import { StringChangeSchema, BookingStatusChangeSchema } from "../common/changeSchemas"; import { AuditActionServiceHelper } from "./AuditActionServiceHelper"; import type { IAuditActionService, TranslationWithParams, GetDisplayTitleParams, GetDisplayJsonParams } from "./IAuditActionService"; @@ -11,9 +11,9 @@ import type { IAuditActionService, TranslationWithParams, GetDisplayTitleParams, // Module-level because it is passed to IAuditActionService type outside the class scope const fieldsSchemaV1 = z.object({ - cancellationReason: StringChangeSchema, - cancelledBy: StringChangeSchema, - status: StringChangeSchema, + cancellationReason: z.string().nullable(), + cancelledBy: z.string().nullable(), + status: BookingStatusChangeSchema, }); export class CancelledAuditActionService implements IAuditActionService { @@ -69,10 +69,8 @@ export class CancelledAuditActionService implements IAuditActionService { }: GetDisplayJsonParams): CancelledAuditDisplayData { const { fields } = this.parseStored({ version: storedData.version, fields: storedData.fields }); return { - cancellationReason: fields.cancellationReason.new ?? null, - previousReason: fields.cancellationReason.old ?? null, - cancelledBy: fields.cancelledBy.new ?? null, - previousCancelledBy: fields.cancelledBy.old ?? null, + cancellationReason: fields.cancellationReason ?? null, + cancelledBy: fields.cancelledBy ?? null, previousStatus: fields.status.old ?? null, newStatus: fields.status.new ?? null, }; @@ -83,9 +81,7 @@ export type CancelledAuditData = z.infer; export type CancelledAuditDisplayData = { cancellationReason: string | null; - previousReason: string | null; cancelledBy: string | null; - previousCancelledBy: string | null; previousStatus: string | null; newStatus: string | null; }; diff --git a/packages/features/booking-audit/lib/makeActor.ts b/packages/features/booking-audit/lib/makeActor.ts index c77c8c7a13..2c7e53305f 100644 --- a/packages/features/booking-audit/lib/makeActor.ts +++ b/packages/features/booking-audit/lib/makeActor.ts @@ -1,4 +1,5 @@ import type { UserActor, GuestActor, AttendeeActor, ActorById, AppActorByCredentialId, AppActorBySlug } from "./dto/types"; +import { v4 as uuidv4 } from "uuid"; const SYSTEM_ACTOR_ID = "00000000-0000-0000-0000-000000000000"; @@ -77,7 +78,13 @@ export function makeAppActorUsingSlug(params: { appSlug: string; name: string }) }; } +/** + * identifier should be unique for that actor + */ export function buildActorEmail({ identifier, actorType }: { identifier: string, actorType: "system" | "guest" | "app" }): string { return `${identifier}@${actorType}.internal`; } +export function getUniqueIdentifier({ prefix }: { prefix: string }): string { + return `${prefix}-${uuidv4()}`; +} \ No newline at end of file diff --git a/packages/features/booking-audit/lib/service/__tests__/BookingAuditViewerService.test.ts b/packages/features/booking-audit/lib/service/__tests__/BookingAuditViewerService.test.ts index 5b29a95e23..c26feae390 100644 --- a/packages/features/booking-audit/lib/service/__tests__/BookingAuditViewerService.test.ts +++ b/packages/features/booking-audit/lib/service/__tests__/BookingAuditViewerService.test.ts @@ -10,7 +10,12 @@ import type { ISimpleLogger } from "@calcom/features/di/shared/services/logger.s import { BookingAuditViewerService } from "../BookingAuditViewerService"; import { BookingAuditPermissionError, BookingAuditErrorCode } from "../BookingAuditAccessService"; -import type { IBookingAuditRepository, BookingAuditWithActor, BookingAuditAction, BookingAuditType } from "../../repository/IBookingAuditRepository"; +import type { + IBookingAuditRepository, + BookingAuditWithActor, + BookingAuditAction, + BookingAuditType, +} from "../../repository/IBookingAuditRepository"; import type { AuditActorType } from "../../repository/IAuditActorRepository"; import type { BookingAuditContext } from "../../dto/types"; @@ -31,7 +36,7 @@ const createMockTeamBooking = (overrides?: { email: "test@example.com", }, eventType: { - teamId: (overrides && "teamId" in overrides ? overrides.teamId : overrides?.teamId ?? 100) ?? null, + teamId: (overrides && "teamId" in overrides ? overrides.teamId : (overrides?.teamId ?? 100)) ?? null, parent: (overrides?.parentTeamId ? { teamId: overrides.parentTeamId } : undefined) ?? null, hosts: [], users: [], @@ -39,7 +44,6 @@ const createMockTeamBooking = (overrides?: { attendees: [], }); - const createMockAuditLog = ( overrides?: Partial<{ id: string; @@ -65,14 +69,19 @@ const createMockAuditLog = ( createdAt: overrides?.createdAt ?? new Date("2024-01-15T10:00:00Z"), updatedAt: overrides?.updatedAt ?? new Date("2024-01-15T10:00:00Z"), actorId: overrides?.actorId ?? "actor-1", - data: overrides?.data ?? { version: 1, fields: { startTime: 1705315200000, endTime: 1705318800000, status: "ACCEPTED" } }, + data: overrides?.data ?? { + version: 1, + fields: { startTime: 1705315200000, endTime: 1705318800000, status: "ACCEPTED" }, + }, source: "WEBAPP" as const, operationId: "operation-id-123", context: (overrides && "context" in overrides ? overrides.context : null) as BookingAuditContext | null, actor: { id: overrides?.actorId ?? "actor-1", - type: overrides?.actorType ?? "USER" as const, - userUuid: (overrides && "actorUserUuid" in overrides ? overrides.actorUserUuid : "user-uuid-123") as string | null, + type: overrides?.actorType ?? ("USER" as const), + userUuid: (overrides && "actorUserUuid" in overrides ? overrides.actorUserUuid : "user-uuid-123") as + | string + | null, attendeeId: null, credentialId: null, name: (overrides && "actorName" in overrides ? overrides.actorName : "John Doe") as string | null, @@ -80,7 +89,9 @@ const createMockAuditLog = ( }, }); -const createMockUser = (overrides?: Partial<{ id: number; name: string | null; email: string; avatarUrl: string | null }>) => ({ +const createMockUser = ( + overrides?: Partial<{ id: number; name: string | null; email: string; avatarUrl: string | null }> +) => ({ id: overrides?.id ?? 123, name: (overrides && "name" in overrides ? overrides.name : "John Doe") as string | null, email: overrides?.email ?? "john@example.com", @@ -157,11 +168,21 @@ describe("BookingAuditViewerService - Integration Tests", () => { error: vi.fn(), }; - vi.mocked(BookingRepository).mockImplementation(function () { return mockBookingRepository as unknown as BookingRepository; }); - vi.mocked(UserRepository).mockImplementation(function () { return mockUserRepository as unknown as UserRepository; }); - vi.mocked(MembershipRepository).mockImplementation(function () { return mockMembershipRepository as unknown as MembershipRepository; }); - vi.mocked(PermissionCheckService).mockImplementation(function () { return mockPermissionCheckService as unknown as PermissionCheckService; }); - vi.mocked(CredentialRepository).mockImplementation(function () { return mockCredentialRepository as unknown as CredentialRepository; }); + vi.mocked(BookingRepository).mockImplementation(function () { + return mockBookingRepository as unknown as BookingRepository; + }); + vi.mocked(UserRepository).mockImplementation(function () { + return mockUserRepository as unknown as UserRepository; + }); + vi.mocked(MembershipRepository).mockImplementation(function () { + return mockMembershipRepository as unknown as MembershipRepository; + }); + vi.mocked(PermissionCheckService).mockImplementation(function () { + return mockPermissionCheckService as unknown as PermissionCheckService; + }); + vi.mocked(CredentialRepository).mockImplementation(function () { + return mockCredentialRepository as unknown as CredentialRepository; + }); service = new BookingAuditViewerService({ bookingAuditRepository: mockBookingAuditRepository as unknown as IBookingAuditRepository, @@ -260,13 +281,16 @@ describe("BookingAuditViewerService - Integration Tests", () => { id: "log-1", action: "CREATED", timestamp: new Date("2024-01-15T10:00:00Z"), - data: { version: 1, fields: { startTime: 1705315200000, endTime: 1705318800000, status: "ACCEPTED" } } + data: { + version: 1, + fields: { startTime: 1705315200000, endTime: 1705318800000, status: "ACCEPTED" }, + }, }), createMockAuditLog({ id: "log-2", action: "ACCEPTED", timestamp: new Date("2024-01-15T11:00:00Z"), - data: { version: 1, fields: { status: { old: "PENDING", new: "ACCEPTED" } } } + data: { version: 1, fields: { status: { old: "PENDING", new: "ACCEPTED" } } }, }), createMockAuditLog({ id: "log-3", @@ -276,10 +300,10 @@ describe("BookingAuditViewerService - Integration Tests", () => { version: 1, fields: { status: { old: "ACCEPTED", new: "CANCELLED" }, - cancellationReason: { old: null, new: "User requested" }, - cancelledBy: { old: null, new: "user-123" }, - } - } + cancellationReason: "User requested", + cancelledBy: "user-123", + }, + }, }), ]; @@ -357,7 +381,7 @@ describe("BookingAuditViewerService - Integration Tests", () => { it("should use email as display name when user name is null", async () => { const mockLog = createMockAuditLog({ actorType: "USER", - actorName: null + actorName: null, }); const mockUser = createMockUser({ name: null, email: "user@example.com" }); diff --git a/packages/features/booking-audit/lib/service/__tests__/cancelled-action.integration-test.ts b/packages/features/booking-audit/lib/service/__tests__/cancelled-action.integration-test.ts new file mode 100644 index 0000000000..61b45b615c --- /dev/null +++ b/packages/features/booking-audit/lib/service/__tests__/cancelled-action.integration-test.ts @@ -0,0 +1,126 @@ +import { describe, expect, it, beforeEach, afterEach } from "vitest"; + +import { BookingStatus } from "@calcom/prisma/enums"; + +import type { BookingAuditTaskConsumer } from "../BookingAuditTaskConsumer"; +import type { BookingAuditViewerService } from "../BookingAuditViewerService"; +import { makeUserActor } from "../../makeActor"; +import { getBookingAuditTaskConsumer } from "../../../di/BookingAuditTaskConsumer.container"; +import { getBookingAuditViewerService } from "../../../di/BookingAuditViewerService.container"; +import { + createTestUser, + createTestOrganization, + createTestMembership, + createTestEventType, + createTestBooking, + enableFeatureForOrganization, + cleanupTestData, +} from "./integration-utils"; + +describe("Cancelled Action Integration", () => { + let bookingAuditTaskConsumer: BookingAuditTaskConsumer; + let bookingAuditViewerService: BookingAuditViewerService; + + let testData: { + owner: { id: number; uuid: string; email: string }; + attendee: { id: number; email: string }; + organization: { id: number }; + eventType: { id: number }; + booking: { uid: string; startTime: Date; endTime: Date; status: BookingStatus }; + }; + + beforeEach(async () => { + bookingAuditTaskConsumer = getBookingAuditTaskConsumer(); + bookingAuditViewerService = getBookingAuditViewerService(); + + const owner = await createTestUser({ name: "Test Audit User" }); + const organization = await createTestOrganization(); + await createTestMembership(owner.id, organization.id); + await enableFeatureForOrganization(organization.id, "booking-audit"); + const eventType = await createTestEventType(owner.id); + const attendee = await createTestUser({ name: "Test Attendee" }); + + const booking = await createTestBooking(owner.id, eventType.id, { + attendees: [ + { + email: attendee.email, + name: attendee.name || "Test Attendee", + timeZone: "UTC", + }, + ], + }); + + testData = { + owner: { id: owner.id, uuid: owner.uuid, email: owner.email }, + attendee: { id: attendee.id, email: attendee.email }, + organization: { id: organization.id }, + eventType: { id: eventType.id }, + booking: { + uid: booking.uid, + startTime: booking.startTime, + endTime: booking.endTime, + status: booking.status, + }, + }; + }); + + afterEach(async () => { + if (!testData) return; + + await cleanupTestData({ + bookingUid: testData.booking?.uid, + userUuids: testData.owner?.uuid ? [testData.owner.uuid] : [], + attendeeEmails: testData.attendee?.email ? [testData.attendee.email] : [], + eventTypeId: testData.eventType?.id, + organizationId: testData.organization?.id, + userIds: [testData.owner?.id, testData.attendee?.id].filter((id): id is number => id !== undefined), + featureSlug: "booking-audit", + }); + }); + + describe("when booking is cancelled", () => { + it("should create audit record with cancellation details and retrieve it correctly", async () => { + const actor = makeUserActor(testData.owner.uuid); + const cancellationReason = "Schedule conflict"; + const cancelledBy = "owner"; + + await bookingAuditTaskConsumer.onBookingAction({ + bookingUid: testData.booking.uid, + actor, + action: "CANCELLED", + source: "WEBAPP", + operationId: `op-${Date.now()}`, + data: { + cancellationReason, + cancelledBy, + status: { + old: BookingStatus.ACCEPTED, + new: BookingStatus.CANCELLED, + }, + }, + timestamp: Date.now(), + }); + + const result = await bookingAuditViewerService.getAuditLogsForBooking({ + bookingUid: testData.booking.uid, + userId: testData.owner.id, + userEmail: testData.owner.email, + userTimeZone: "UTC", + organizationId: testData.organization.id, + }); + + expect(result.auditLogs).toHaveLength(1); + const auditLog = result.auditLogs[0]; + + expect(auditLog.action).toBe("CANCELLED"); + expect(auditLog.type).toBe("RECORD_UPDATED"); + + const displayData = auditLog.displayJson as Record; + expect(displayData.cancellationReason).toBe(cancellationReason); + expect(displayData.cancelledBy).toBe(cancelledBy); + expect(displayData.previousStatus).toBe(BookingStatus.ACCEPTED); + expect(displayData.newStatus).toBe(BookingStatus.CANCELLED); + }); + }); +}); + diff --git a/packages/features/bookings/lib/handleCancelBooking.ts b/packages/features/bookings/lib/handleCancelBooking.ts index 36648c0643..19ffa6e88e 100644 --- a/packages/features/bookings/lib/handleCancelBooking.ts +++ b/packages/features/bookings/lib/handleCancelBooking.ts @@ -1,10 +1,13 @@ import type { z } from "zod"; +import { v4 as uuidv4 } from "uuid"; import { DailyLocationType } from "@calcom/app-store/constants"; import { FAKE_DAILY_CREDENTIAL } from "@calcom/app-store/dailyvideo/lib/VideoApiAdapter"; import { eventTypeMetaDataSchemaWithTypedApps } from "@calcom/app-store/zod-utils"; import dayjs from "@calcom/dayjs"; import { sendCancelledEmailsAndSMS } from "@calcom/emails/email-manager"; +import type { ActionSource } from "@calcom/features/booking-audit/lib/types/actionSource"; +import { getBookingEventHandlerService } from "@calcom/features/bookings/di/BookingEventHandlerService.container"; import EventManager from "@calcom/features/bookings/lib/EventManager"; import { getCalEventResponses } from "@calcom/features/bookings/lib/getCalEventResponses"; import { processNoShowFeeOnCancellation } from "@calcom/features/bookings/lib/payment/processNoShowFeeOnCancellation"; @@ -51,6 +54,8 @@ import { getBookingToDelete } from "./getBookingToDelete"; import { handleInternalNote } from "./handleInternalNote"; import cancelAttendeeSeat from "./handleSeats/cancel/cancelAttendeeSeat"; import type { IBookingCancelService } from "./interfaces/IBookingCancelService"; +import { buildActorEmail, getUniqueIdentifier, makeGuestActor, makeUserActor } from "@calcom/features/booking-audit/lib/makeActor"; +import type { Actor } from "@calcom/features/booking-audit/lib/dto/types"; const log = logger.getSubLogger({ prefix: ["handleCancelBooking"] }); @@ -68,8 +73,46 @@ export type CancelBookingInput = { userId?: number; userUuid?: string; bookingData: z.infer; + actionSource?: ActionSource; } & PlatformParams; +/** + * Its job is to ensure that an actor is always returned, otherwise we won't be able to audit the action. + */ +function getAuditActor({ + userUuid, + cancelledByEmailInQueryParam, + bookingUid, +}: { + userUuid: string | null; + cancelledByEmailInQueryParam: string | null; + bookingUid: string; +}): Actor { + if (userUuid) { + return makeUserActor(userUuid); + } + + let actorEmail: string; + // Fallback to guest actor for unauthenticated cancellation + if (!cancelledByEmailInQueryParam) { + log.warn( + "No cancelledBy email in query param available, creating fallback guest actor for audit", + safeStringify({ + bookingUid, + }) + ); + // Having fallback prefix makes it clear that we created guest actor from fallback logic + actorEmail = buildActorEmail({ identifier: getUniqueIdentifier({ prefix: "fallback" }), actorType: "guest" }); + } + else { + // We can't trust cancelledByEmail and thus can't reuse it as is because it can be set anything by anyone. If we use that as guest actor, we could accidentally attribute the action to the wrong guest actor. + // Having param prefix makes it clear that we created guest actor from query param and we still don't use the email as is. + actorEmail = buildActorEmail({ identifier: getUniqueIdentifier({ prefix: "param" }), actorType: "guest" }); + } + + return makeGuestActor({ email: actorEmail, name: null }); +} + async function handler(input: CancelBookingInput) { const body = input.bookingData; const { @@ -93,6 +136,23 @@ async function handler(input: CancelBookingInput) { arePlatformEmailsEnabled, } = input; + const userUuid = input.userUuid ?? null; + + // Extract action source once for reuse + const actionSource = input.actionSource ?? "UNKNOWN"; + if (actionSource === "UNKNOWN") { + log.warn("Booking cancellation with unknown actionSource", safeStringify({ + bookingUid: bookingToDelete.uid, + userUuid, + })); + } + + const actorToUse = getAuditActor({ + userUuid, + cancelledByEmailInQueryParam: cancelledBy ?? null, + bookingUid: bookingToDelete.uid, + }); + /** * Important: We prevent cancelling an already cancelled booking. * A booking could have been CANCELLED due to a reschedule, @@ -293,17 +353,17 @@ async function handler(input: CancelBookingInput) { destinationCalendar: bookingToDelete?.destinationCalendar ? [bookingToDelete?.destinationCalendar] : bookingToDelete?.user.destinationCalendar - ? [bookingToDelete?.user.destinationCalendar] - : [], + ? [bookingToDelete?.user.destinationCalendar] + : [], cancellationReason: cancellationReason, ...(teamMembers && teamId && { - team: { - name: bookingToDelete?.eventType?.team?.name || "Nameless", - members: teamMembers, - id: teamId, - }, - }), + team: { + name: bookingToDelete?.eventType?.team?.name || "Nameless", + members: teamMembers, + id: teamId, + }, + }), seatsPerTimeSlot: bookingToDelete.eventType?.seatsPerTimeSlot, seatsShowAttendees: bookingToDelete.eventType?.seatsShowAttendees, iCalUID: bookingToDelete.iCalUID, @@ -393,6 +453,8 @@ async function handler(input: CancelBookingInput) { endTime: Date; }[] = []; + const bookingEventHandlerService = getBookingEventHandlerService(); + // by cancelling first, and blocking whilst doing so; we can ensure a cancel // action always succeeds even if subsequent integrations fail cancellation. if ( @@ -440,6 +502,25 @@ async function handler(input: CancelBookingInput) { }, }); updatedBookings = updatedBookings.concat(allUpdatedBookings); + + const operationId = uuidv4(); + await bookingEventHandlerService.onBulkBookingsCancelled({ + bookings: allUpdatedBookings.map((updatedRecurringBooking) => ({ + bookingUid: updatedRecurringBooking.uid, + auditData: { + cancellationReason: cancellationReason ?? null, + cancelledBy: cancelledBy ?? null, + status: { + old: bookingToDelete.status, + new: BookingStatus.CANCELLED, + }, + }, + })), + actor: actorToUse, + organizationId: orgId ?? null, + operationId, + source: actionSource, + }); } else { if (bookingToDelete?.eventType?.seatsPerTimeSlot) { await prisma.attendee.deleteMany({ @@ -478,6 +559,21 @@ async function handler(input: CancelBookingInput) { }); updatedBookings.push(updatedBooking); + await bookingEventHandlerService.onBookingCancelled({ + bookingUid: updatedBooking.uid, + actor: actorToUse, + organizationId: orgId ?? null, + source: actionSource, + auditData: { + cancellationReason: cancellationReason ?? null, + cancelledBy: cancelledBy ?? null, + status: { + old: bookingToDelete.status, + new: BookingStatus.CANCELLED, + }, + }, + }); + if (bookingToDelete.payment.some((payment) => payment.paymentOption === "ON_BOOKING")) { try { await processPaymentRefund({ @@ -618,7 +714,7 @@ type BookingCancelServiceDependencies = { * Handles both individual booking cancellations and bulk cancellations for recurring events. */ export class BookingCancelService implements IBookingCancelService { - constructor(private readonly deps: BookingCancelServiceDependencies) {} + constructor(private readonly deps: BookingCancelServiceDependencies) { } async cancelBooking(input: { bookingData: CancelRegularBookingData; bookingMeta?: CancelBookingMeta }) { const cancelBookingInput: CancelBookingInput = {