Skip to content

Conversation

@abimaelmartell
Copy link

What does this PR do?

  • Enables deleting past bookings (past-only) via TRPC viewer.bookings.delete mutation.

  • Introduces a proper deleted boolean column on Booking (soft-delete), replacing the prior metadata flag usage.

  • Ensures all listings exclude deleted bookings by filtering where Booking.deleted = false in the shared getBookings flow.

  • Adds tests for delete behavior and for listings filtering.

  • Fixes [CAL-5114] Delete the booking history of past meetings #18787 (GitHub issue number)

  • Fixes CAL-5114

Visual Demo (For contributors especially)

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • [NA] I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

Log in using the pro user credentials, that include a past booking. The Delete button will show up. You can click it and a confirmation popup will show, once confirmed, the booking will be marked as deleted in the database (soft delete).

You can confirm by refreshing the page, that booking will not show in the db.

  • Are there environment variables that should be set?
    No
  • What are the minimal test data to have?
    One past booking.
  • What is expected (happy path) to have (input and output)?
    • New delete button on past bookings
    • Confirmation popup on delete button
    • Deletion should mark the booking as deleted
  • Any other important info that could help to test that PR
    Added a couple test for the endpoints.

@abimaelmartell abimaelmartell requested a review from a team as a code owner September 2, 2025 00:02
@abimaelmartell abimaelmartell requested a review from a team September 2, 2025 00:02
@vercel
Copy link

vercel bot commented Sep 2, 2025

@abimaelmartell is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 2, 2025

Walkthrough

Adds a soft-delete capability for bookings limited to past events. UI: BookingListItem integrates a Delete action and a DeleteBookingDialog to confirm deletion. State and action wiring added via getDeleteEventAction. Backend: New TRPC mutation viewer.bookings.delete with deleteHandler that forbids future deletions and sets Booking.deleted = true. Booking listings now filter out deleted rows. Database: Booking model gets a nullable deleted flag; BookingTimeStatusDenormalized view changes id from primary key to unique. Tests cover delete handler behavior and ensure get.handler filters deleted bookings.

Assessment against linked issues

Objective Addressed Explanation
Provide UI action and backend to delete past bookings [#18787, CAL-5114]
Exclude deleted bookings from viewer lists [#18787, CAL-5114]
Support automatic/scheduled deletion of booking history [#18787, CAL-5114] No scheduler/cron or background job implemented.
Allow deletion of canceled (not just past) meetings [#18787, CAL-5114] Logic gates by endTime; no explicit canceled-status handling observed.

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
Change BookingTimeStatusDenormalized.id from primary key to unique (packages/prisma/schema.prisma) Not required for implementing delete functionality; unrelated to soft-delete or filtering logic.

Possibly related PRs

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Sep 2, 2025
@graphite-app graphite-app bot requested a review from a team September 2, 2025 00:02
@github-actions github-actions bot added the ✨ feature New feature or request label Sep 2, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2025

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "[CAL-5114] Implement Deleting Past Bookings". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@dosubot dosubot bot added the bookings area: bookings, availability, timezones, double booking label Sep 2, 2025
@dosubot dosubot bot added this to the v5.7 milestone Sep 2, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/prisma/schema.prisma (1)

818-841: Make soft-delete non-null with default and index for performance.

Nullable deleted complicates reads and harms index usage. Prefer NOT NULL with default false and an index.

Apply:

 model Booking {
@@
-  deleted                      Boolean?
+  deleted                      Boolean @default(false)
@@
   @@index([status])
   @@index([startTime, endTime, status])
+  @@index([deleted])
 }

If existing data has NULLs, backfill to false in a migration before adding NOT NULL.

🧹 Nitpick comments (15)
packages/trpc/server/routers/viewer/bookings/get.handler.ts (1)

916-963: Stabilize “now” to avoid boundary drift across OR branches.

Multiple calls to new Date() inside addStatusesQueryFilters can differ by milliseconds and lead to edge misclassification near boundaries. Capture once and pass in.

I can provide a small refactor to thread a now: Date through the helper if you’d like.

packages/trpc/server/routers/viewer/bookings/_router.tsx (1)

72-79: Optional: avoid reserved word for action id/name.

Using delete as a route key is valid, but a more explicit name (e.g., remove/deleteBooking) can reduce linting/eslint-plugin confusion and log search ambiguity.

packages/trpc/server/routers/viewer/bookings/delete.handler.ts (2)

12-13: Fix misleading comment.

We are not “marking as cancelled”; only setting deleted = true. Update the comment.

-  // Soft delete: mark as cancelled and set deleted flag
+  // Soft delete: set deleted flag

6-9: Validate input vs context to prevent mismatches.

input is unused. Either remove it from the signature or assert input.bookingId === ctx.booking.id to avoid accidental mismatches.

 type DeleteOptions = {
   ctx: BookingsProcedureContext;
   input: ZDeleteBookingInputSchema;
 };

Option A (assert):

 export async function deleteHandler({ ctx, input }: DeleteOptions) {
   const { booking } = ctx;
+  if (input.bookingId !== booking.id) {
+    throw new TRPCError({ code: "BAD_REQUEST", message: "Mismatched booking id" });
+  }

Option B (remove):

-type DeleteOptions = {
-  ctx: BookingsProcedureContext;
-  input: ZDeleteBookingInputSchema;
-};
+type DeleteOptions = { ctx: BookingsProcedureContext };
packages/trpc/server/routers/viewer/bookings/__tests__/get.handler.filter-deleted.test.ts (1)

7-17: Mock shape is fine; minor typing improvement optional.

You can type the mocked export to avoid as unknown as PrismaClient later, but current approach is acceptable for Vitest.

apps/web/components/booking/BookingListItem.tsx (3)

721-724: Let the dropdown close on click; remove preventDefault.

preventDefault() keeps the Radix dropdown open under the dialog. Remove it so the menu closes when opening the delete dialog.

-                          onClick={(e) => {
-                            e.preventDefault();
-                            setIsOpenDeleteDialog(true);
-                          }}
+                          onClick={() => setIsOpenDeleteDialog(true)}

53-53: Ensure imported dialog uses t() for all user-facing strings.

DeleteBookingDialog currently contains a hardcoded confirmation message. Please localize it with t().


712-731: Gate already via isBookingInPast; also rely on action.disabled.

The conditional render and disabled state look consistent. Consider surfacing a tooltip explaining why delete is disabled when applicable (optional UX).

packages/trpc/server/routers/viewer/bookings/__tests__/delete.handler.test.ts (3)

1-1: Stabilize time to avoid edge-case flakes.
Freeze the clock to make “past/future” deterministic across timezones and midnight boundaries.

-import { describe, it, expect, vi, beforeEach } from "vitest";
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";

 beforeEach(() => {
   vi.clearAllMocks();
+  vi.useFakeTimers();
+  vi.setSystemTime(new Date("2025-01-01T00:00:00Z"));
 });
+
+afterEach(() => {
+  vi.useRealTimers();
+});

Also applies to: 19-21


77-83: Avoid as any and assert return payload.
Type input with the schema for better safety and also assert the success message.

+import { z } from "zod";
+import { ZDeleteBookingInputSchema } from "../delete.schema";
@@
-    await deleteHandler({ ctx, input: { bookingId: 1 } as any });
+    const pastInput: z.infer<typeof ZDeleteBookingInputSchema> = { bookingId: 1 };
+    const res = await deleteHandler({ ctx, input: pastInput });
+    expect(res).toEqual({ message: "Booking deleted" });
@@
-    await expect(deleteHandler({ ctx, input: { bookingId: 2 } as any })).rejects.toThrow(
+    const futureInput: z.infer<typeof ZDeleteBookingInputSchema> = { bookingId: 2 };
+    await expect(deleteHandler({ ctx, input: futureInput })).rejects.toThrow(
       "Cannot delete future bookings"
     );

Also applies to: 139-143, 1-1


23-23: Soft-delete may not meet privacy deletion needs.
If the goal is privacy erasure, consider a follow-up hard-delete/retention policy (e.g., periodic purge of soft-deleted rows and related PII/attendees/integrations).

apps/web/components/dialog/DeleteBookingDialog.tsx (4)

33-33: Mark decorative icon as hidden from AT.
Small a11y win.

-            <Icon name="trash" className="m-auto h-6 w-6" />
+            <Icon name="trash" className="m-auto h-6 w-6" aria-hidden="true" />

20-25: Surface errors to users.
Add an onError handler (toast/snackbar) so failures aren’t silent; keep dialog open when it fails.

I can wire this to your existing toast utility if you share which one you use (e.g., ui toast hook/component).


21-24: Consider targeted cache invalidation.
If the bookings list is large, invalidate only affected queries (e.g., viewer.bookings.list or by filters) or optimistically remove the item with setData.


58-58: Prefer named export for DeleteBookingDialog and update its import
Drop the default export in apps/web/components/dialog/DeleteBookingDialog.tsx and switch to a named export. Update its import in apps/web/components/booking/BookingListItem.tsx accordingly.

--- a/apps/web/components/dialog/DeleteBookingDialog.tsx
-export default DeleteBookingDialog;
+export { DeleteBookingDialog };
--- a/apps/web/components/booking/BookingListItem.tsx:53
-import DeleteBookingDialog from "@components/dialog/DeleteBookingDialog";
+import { DeleteBookingDialog } from "@components/dialog/DeleteBookingDialog";
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f6f3675 and 9c10782.

📒 Files selected for processing (10)
  • apps/web/components/booking/BookingListItem.tsx (6 hunks)
  • apps/web/components/booking/bookingActions.ts (2 hunks)
  • apps/web/components/dialog/DeleteBookingDialog.tsx (1 hunks)
  • packages/prisma/schema.prisma (2 hunks)
  • packages/trpc/server/routers/viewer/bookings/__tests__/delete.handler.test.ts (1 hunks)
  • packages/trpc/server/routers/viewer/bookings/__tests__/get.handler.filter-deleted.test.ts (1 hunks)
  • packages/trpc/server/routers/viewer/bookings/_router.tsx (2 hunks)
  • packages/trpc/server/routers/viewer/bookings/delete.handler.ts (1 hunks)
  • packages/trpc/server/routers/viewer/bookings/delete.schema.ts (1 hunks)
  • packages/trpc/server/routers/viewer/bookings/get.handler.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/trpc/server/routers/viewer/bookings/delete.schema.ts
  • packages/trpc/server/routers/viewer/bookings/get.handler.ts
  • packages/trpc/server/routers/viewer/bookings/delete.handler.ts
  • packages/trpc/server/routers/viewer/bookings/__tests__/delete.handler.test.ts
  • packages/trpc/server/routers/viewer/bookings/__tests__/get.handler.filter-deleted.test.ts
  • apps/web/components/booking/bookingActions.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/trpc/server/routers/viewer/bookings/delete.schema.ts
  • packages/trpc/server/routers/viewer/bookings/get.handler.ts
  • packages/trpc/server/routers/viewer/bookings/delete.handler.ts
  • packages/trpc/server/routers/viewer/bookings/__tests__/delete.handler.test.ts
  • packages/trpc/server/routers/viewer/bookings/__tests__/get.handler.filter-deleted.test.ts
  • apps/web/components/dialog/DeleteBookingDialog.tsx
  • apps/web/components/booking/BookingListItem.tsx
  • packages/trpc/server/routers/viewer/bookings/_router.tsx
  • apps/web/components/booking/bookingActions.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/trpc/server/routers/viewer/bookings/delete.schema.ts
  • packages/trpc/server/routers/viewer/bookings/get.handler.ts
  • packages/trpc/server/routers/viewer/bookings/delete.handler.ts
  • packages/trpc/server/routers/viewer/bookings/__tests__/delete.handler.test.ts
  • packages/trpc/server/routers/viewer/bookings/__tests__/get.handler.filter-deleted.test.ts
  • apps/web/components/dialog/DeleteBookingDialog.tsx
  • apps/web/components/booking/BookingListItem.tsx
  • packages/trpc/server/routers/viewer/bookings/_router.tsx
  • apps/web/components/booking/bookingActions.ts
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

  • apps/web/components/dialog/DeleteBookingDialog.tsx
  • apps/web/components/booking/BookingListItem.tsx
  • packages/trpc/server/routers/viewer/bookings/_router.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-22T16:38:00.225Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/lib/server/service/InsightsBookingBaseService.ts:814-816
Timestamp: 2025-08-22T16:38:00.225Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), when filtering for "accepted" bookings in getMembersStatsWithCount(), using `endTime <= now()` in the SQL condition should be avoided as it conflicts with existing date filtering logic. The components already handle completion filtering by setting `endDate: currentTime` in their query parameters, making additional SQL-level endTime filtering unnecessary and potentially problematic.

Applied to files:

  • packages/trpc/server/routers/viewer/bookings/get.handler.ts
📚 Learning: 2025-08-19T09:47:49.478Z
Learnt from: eunjae-lee
PR: calcom/cal.com#23166
File: packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql:65-66
Timestamp: 2025-08-19T09:47:49.478Z
Learning: The Booking table has a unique constraint and index on the uid column (defined as `uid String unique` in schema.prisma), so JOINs on Booking.uid have optimal performance and don't require additional indexing.

Applied to files:

  • packages/trpc/server/routers/viewer/bookings/get.handler.ts
  • packages/prisma/schema.prisma
🧬 Code graph analysis (6)
packages/trpc/server/routers/viewer/bookings/delete.handler.ts (1)
packages/trpc/server/routers/viewer/bookings/delete.schema.ts (1)
  • ZDeleteBookingInputSchema (3-3)
packages/trpc/server/routers/viewer/bookings/__tests__/delete.handler.test.ts (1)
packages/trpc/server/routers/viewer/bookings/delete.handler.ts (1)
  • deleteHandler (11-28)
packages/trpc/server/routers/viewer/bookings/__tests__/get.handler.filter-deleted.test.ts (2)
packages/trpc/server/routers/viewer/bookings/get.handler.ts (1)
  • getBookings (71-736)
packages/prisma/index.ts (1)
  • PrismaClient (83-83)
apps/web/components/dialog/DeleteBookingDialog.tsx (1)
apps/web/app/_trpc/trpc.ts (1)
  • trpc (7-7)
apps/web/components/booking/BookingListItem.tsx (3)
apps/web/components/booking/bookingActions.ts (1)
  • getDeleteEventAction (79-89)
apps/web/components/dialog/DeleteBookingDialog.tsx (1)
  • DeleteBookingDialog (16-56)
packages/ui/components/dropdown/Dropdown.tsx (2)
  • DropdownMenuItem (63-71)
  • DropdownItem (161-181)
packages/trpc/server/routers/viewer/bookings/_router.tsx (2)
packages/trpc/server/routers/viewer/bookings/delete.schema.ts (1)
  • ZDeleteBookingInputSchema (3-3)
packages/trpc/server/routers/viewer/bookings/delete.handler.ts (1)
  • deleteHandler (11-28)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (8)
packages/trpc/server/routers/viewer/bookings/delete.schema.ts (1)

3-3: Schema reuse looks good.

Alias to commonBookingSchema is fine for consistency with other endpoints.

packages/prisma/schema.prisma (1)

1730-1755: Changing view PK to unique: verify client usage.

BookingTimeStatusDenormalized.id moved from @id to @unique. Ensure no Prisma Client code expects @id semantics (e.g., findUnique still works, but any logic relying on primary key metadata should be checked).

packages/trpc/server/routers/viewer/bookings/_router.tsx (1)

72-79: Good addition; uses bookingsProcedure for authz + booking preload.

Wiring matches existing pattern; dynamic import keeps bundle lean.

apps/web/components/booking/bookingActions.ts (2)

79-89: Delete action wiring looks correct and localized.

Action id, destructive styling, and localization via t("delete") are consistent with existing patterns.


232-234: Disable rule for delete aligns with “past-only” policy.

Return !isBookingInPast is simple and matches server restriction.

apps/web/components/booking/BookingListItem.tsx (1)

459-463: LGTM: dialog wiring and cache invalidation.

Dialog state, mutation wiring, and list invalidation look correct.

packages/trpc/server/routers/viewer/bookings/__tests__/delete.handler.test.ts (2)

8-16: Good isolation of DB via prisma mock.
This cleanly stubs only what the handler uses.


85-143: Error-path coverage looks good.
Verifies the thrown message and that no DB write occurs.

Comment on lines +36 to +38
<p className="text-subtle text-sm">
Are you sure you want to delete this event? This action cannot be undone.
</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Localize the confirmation text.
Per guidelines, avoid hardcoded strings in TSX.

-            <p className="text-subtle text-sm">
-              Are you sure you want to delete this event? This action cannot be undone.
-            </p>
+            <p className="text-subtle text-sm">
+              {t("delete_booking_confirmation")}
+            </p>

If the key doesn’t exist, add it to locales.

📝 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
<p className="text-subtle text-sm">
Are you sure you want to delete this event? This action cannot be undone.
</p>
<p className="text-subtle text-sm">
{t("delete_booking_confirmation")}
</p>
🤖 Prompt for AI Agents
In apps/web/components/dialog/DeleteBookingDialog.tsx around lines 36 to 38, the
confirmation string is hardcoded; replace it with the project's i18n translation
call (e.g., use the existing t(...) or i18n.t(...) hook used across the app) and
reference a new key like "deleteBooking.confirmation" (or the project's naming
convention), then add that key and its localized message to the locale JSON
files for each supported language; ensure fallback/default text is present if
key is missing.

Comment on lines +98 to +101
// Ensure we applied deleted filter on Booking builder
const calls = whereBookingMock.mock.calls.map((args) => args);
expect(calls).toContainEqual(["Booking.deleted", "=", false]);
});
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

Bug: filtering deleted = false will exclude rows with NULL (new column). Use “IS NOT TRUE”.

Since Booking.deleted is nullable, = false drops NULL rows. Either make the column NOT NULL DEFAULT false with backfill, or change the query to deleted IS NOT TRUE. Update the test to reflect the safer predicate.

-    const calls = whereBookingMock.mock.calls.map((args) => args);
-    expect(calls).toContainEqual(["Booking.deleted", "=", false]);
+    const calls = whereBookingMock.mock.calls.map((args) => args);
+    // Expect the safer predicate that retains NULL and FALSE
+    expect(calls).toContainEqual(["Booking.deleted", "is not", true]);

Follow-up: ensure get.handler.ts uses .where("Booking.deleted", "is not", true) or a COALESCE.

📝 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
// Ensure we applied deleted filter on Booking builder
const calls = whereBookingMock.mock.calls.map((args) => args);
expect(calls).toContainEqual(["Booking.deleted", "=", false]);
});
// Ensure we applied deleted filter on Booking builder
const calls = whereBookingMock.mock.calls.map((args) => args);
// Expect the safer predicate that retains NULL and FALSE
expect(calls).toContainEqual(["Booking.deleted", "is not", true]);
});
🤖 Prompt for AI Agents
In
packages/trpc/server/routers/viewer/bookings/__tests__/get.handler.filter-deleted.test.ts
around lines 98 to 101, the test asserts the deleted filter uses
["Booking.deleted","=" , false] which wrongly excludes NULLs; change the
expectation to assert the safer predicate (either ["Booking.deleted","is not",
true] or a COALESCE-based predicate like ["COALESCE(Booking.deleted,
false)","=", false] to match the runtime behavior), and then update the actual
query in get.handler.ts to use .where("Booking.deleted","is not", true) or use
COALESCE(Booking.deleted, false) so nullable deleted values are treated as not
deleted.

Comment on lines +11 to +18
export async function deleteHandler({ ctx }: DeleteOptions) {
// Soft delete: mark as cancelled and set deleted flag
const { booking } = ctx;

// Check if booking is in the past
if (booking.endTime > new Date()) {
throw new Error("Cannot delete future bookings");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use TRPCError instead of generic Error; add explicit code.

Throwing Error will surface as 500. Use TRPCError with a client-meaningful code (e.g., BAD_REQUEST) and avoid leaking stack traces.

+import { TRPCError } from "@trpc/server";
 ...
-export async function deleteHandler({ ctx }: DeleteOptions) {
+export async function deleteHandler({ ctx, input }: DeleteOptions) {
   // Soft delete: mark as cancelled and set deleted flag
   const { booking } = ctx;

   // Check if booking is in the past
-  if (booking.endTime > new Date()) {
-    throw new Error("Cannot delete future bookings");
-  }
+  const now = new Date();
+  if (booking.endTime > now) {
+    throw new TRPCError({ code: "BAD_REQUEST", message: "Cannot delete future bookings" });
+  }
📝 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
export async function deleteHandler({ ctx }: DeleteOptions) {
// Soft delete: mark as cancelled and set deleted flag
const { booking } = ctx;
// Check if booking is in the past
if (booking.endTime > new Date()) {
throw new Error("Cannot delete future bookings");
}
import { TRPCError } from "@trpc/server";
// …other imports
export async function deleteHandler({ ctx, input }: DeleteOptions) {
// Soft delete: mark as cancelled and set deleted flag
const { booking } = ctx;
// Check if booking is in the past
const now = new Date();
if (booking.endTime > now) {
throw new TRPCError({ code: "BAD_REQUEST", message: "Cannot delete future bookings" });
}
// …rest of the handler logic
}
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/bookings/delete.handler.ts around lines
11 to 18, replace the generic throw new Error with a TRPCError: import TRPCError
from '@trpc/server' (or the named export) at the top and throw new TRPCError({
code: 'BAD_REQUEST', message: 'Cannot delete future bookings' }) instead; ensure
booking.endTime is a Date (or parsed) before comparing to new Date() so the
check remains correct and returns a client-facing error code rather than a 500.

"in",
bookingsFromUnion.map((booking) => booking.id)
)
.where("Booking.deleted", "=", false)
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

Soft-delete filter excludes legacy rows (NULL) and causes count/list mismatch.

  • .where("Booking.deleted", "=", false) drops rows where deleted is NULL (legacy), and totalCount still includes deleted rows because the union count has no delete filter. Result: empty pages and wrong pagination totals.

Apply both fixes:

  1. Include not-deleted in all subqueries before the union to keep totalCount consistent.
  2. Make the final fetch tolerant of NULL by treating NULL as not-deleted.
@@
-        .where("Booking.deleted", "=", false)
+        // Treat NULL as not-deleted for legacy rows
+        .where(({ eb }) => eb.or([eb("Booking.deleted", "=", false), eb("Booking.deleted", "is", null)]))

And in the filtered subqueries block:

@@
   const queriesWithFilters = bookingQueries.map(({ query, tables }) => {
     // 1. Apply mandatory status filter
     let fullQuery = addStatusesQueryFilters(query, bookingListingByStatus);
@@
     // 7. Booking Start/End Time Range Filters
     if (filters?.afterStartDate) {
       fullQuery = fullQuery.where("Booking.startTime", ">=", dayjs.utc(filters.afterStartDate).toDate());
     }
     if (filters?.beforeEndDate) {
       fullQuery = fullQuery.where("Booking.endTime", "<=", dayjs.utc(filters.beforeEndDate).toDate());
     }
+
+    // 8. Exclude soft-deleted bookings (NULL = not-deleted for legacy data)
+    fullQuery = fullQuery.where(({ eb }) =>
+      eb.or([eb("Booking.deleted", "=", false), eb("Booking.deleted", "is", null)])
+    );
 
     return fullQuery;
   });
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/bookings/get.handler.ts around line 435,
the current WHERE clause `.where("Booking.deleted", "=", false)` excludes legacy
rows where `deleted` is NULL and causes totalCount/list mismatches; update the
code so that each subquery used in the UNION includes a non-deleted filter
(e.g., add the same deleted check to all subqueries before the union) so
totalCount and the unioned rows are consistent, and make the final fetch
tolerant of NULL by treating NULL as not-deleted (e.g., use a coalesce or
explicit `IS NOT TRUE` style check so NULL is considered equivalent to false for
the fetch).

Comment on lines +25 to +75
const ctx: BookingsProcedureContext = {
booking: {
id: 1,
uid: "uid",
eventTypeId: 1,
title: "title",
description: null,
customInputs: null,
responses: null,
startTime: past,
endTime: past,
location: null,
createdAt: new Date(),
updatedAt: new Date(),
status: "accepted",
paid: false,
userId: 1,
cancellationReason: null,
rejectionReason: null,
reassignReason: null,
reassignById: null,
dynamicEventSlugRef: null,
dynamicGroupSlugRef: null,
rescheduled: null,
fromReschedule: null,
recurringEventId: null,
smsReminderNumber: null,
metadata: {},
isRecorded: false,
iCalUID: "",
iCalSequence: 0,
rating: null,
ratingFeedback: null,
noShowHost: false,
oneTimePassword: "otp",
cancelledBy: null,
rescheduledBy: null,
tracking: null,
routingFormResponses: [],
expenseLogs: [],
attendees: [],
references: [],
destinationCalendar: null,
eventType: null,
user: null,
seatsReferences: [],
instantMeetingToken: null,
assignmentReason: [],
scheduledTriggers: [],
},
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know why this is needed 🤔

Comment on lines -1729 to +1730
id Int @id @unique
id Int @unique
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

@anikdhabal
Copy link
Contributor

fix the failing type error and unit tests

@Devanshusharma2005 Devanshusharma2005 marked this pull request as draft September 2, 2025 07:22
@dosubot dosubot bot modified the milestones: v5.7, v5.8 Sep 16, 2025
<div className="w-full pt-1">
<DialogHeader title={t("delete")} />
<p className="text-subtle text-sm">
Are you sure you want to delete this event? This action cannot be undone.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should use translations

@CarinaWolli CarinaWolli modified the milestones: v5.8, v5.9 Oct 16, 2025
@CarinaWolli CarinaWolli modified the milestones: v5.9, v6.0 Nov 17, 2025
@CarinaWolli CarinaWolli modified the milestones: v6.0, v6.1 Dec 16, 2025
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 community Created by Linear-GitHub Sync ✨ feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CAL-5114] Delete the booking history of past meetings

5 participants