Skip to content

Conversation

@pumfleet
Copy link
Contributor

@pumfleet pumfleet commented Jan 21, 2026

What does this PR do?

Adds a download bookings CSV button for organization users on the bookings page. The button fetches all bookings matching the current filters in batches and exports them as a CSV file.

Visual Demo

Image Demo:

Download button visible for org users:
Screenshot 2026-01-21 at 6 53 05 PM

Button hidden for non-org users:
Screenshot 2026-01-21 at 6 53 28 PM

Downloaded CSV:
Screenshot 2026-01-21 at 6 54 00 PM

Updates since last revision

  • Localized CSV headers and filename using i18n t() function instead of hardcoded English strings (addresses Cubic AI review feedback)
  • CSV headers now use existing translation keys: booking_uid, title, status, start_time, end_time, attendee_name, email, event_type, location
  • Filename uses localized "bookings" text
  • Added safeguard to prevent infinite loop when batch returns empty bookings during CSV download (addresses DevinAI review feedback)

Mandatory Tasks (DO NOT REMOVE)

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

How should this be tested?

  1. Log in as a user who is part of an organization
  2. Navigate to the Bookings page
  3. Verify the "Download" button appears next to the view toggle
  4. Click the button and verify a CSV file is downloaded with localized headers
  5. Log in as a non-org user and verify the button is not visible

Human Review Checklist

  • Verify all translation keys used exist in apps/web/public/static/locales/en/common.json
  • Consider if localized CSV headers could cause issues for users who process CSVs programmatically (expecting English headers)
  • Verify the infinite loop safeguard at line 93 correctly handles edge cases where bookings are deleted between batch fetches

Link to Devin run: https://app.devin.ai/sessions/29e729784b1f40518a404ed9c352ad78
Link to Devin run (infinite loop fix): https://app.devin.ai/sessions/68fa2f67f0db4b38bd0e01622caf635f
Requested by: alex@cal.com (@emrysal)

@graphite-app graphite-app bot added the core area: core, team members only label Jan 21, 2026
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/web/modules/bookings/components/BookingsCsvDownload.tsx">

<violation number="1" location="apps/web/modules/bookings/components/BookingsCsvDownload.tsx:25">
P2: Localize the CSV headers (and filename text) using t() with existing locale keys instead of hardcoded strings so the exported CSV matches the UI language.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@github-actions
Copy link
Contributor

Devin AI is addressing Cubic AI's review feedback

A Devin session has been created to address the issues identified by Cubic AI.

View Devin Session

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View issue and 4 additional flags in Devin Review.

Open in Devin Review

Comment on lines 89 to 96
while (allBookings.length < totalCount) {
const offset = allBookings.length;
const batch = await fetchBatch(offset);
allBookings = [...allBookings, ...batch.bookings];

const currentProgress = Math.min(Math.round((allBookings.length / totalCount) * 100), 99);
showProgressToast(currentProgress);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 Infinite loop when batch returns empty bookings during CSV download

In the handleDownload function, the while loop at line 89 (while (allBookings.length < totalCount)) can run infinitely if a subsequent batch returns an empty bookings array while totalCount remains greater than allBookings.length.

This can occur in race conditions where bookings are deleted between the first batch fetch and subsequent fetches, or if there's a server-side issue returning empty results.

When batch.bookings is empty, allBookings = [...allBookings, ...batch.bookings] doesn't increase the length of allBookings, causing the condition allBookings.length < totalCount to remain true forever.

The existing similar implementations in Download.tsx:70-78 have a safeguard:

if (!result) break;

But BookingsCsvDownload.tsx lacks this protection. The impact is that users clicking the download button could have their browser freeze with an infinite loop making continuous API calls.

Recommendation: Add a break condition when the batch returns empty bookings:

while (allBookings.length < totalCount) {
  const offset = allBookings.length;
  const batch = await fetchBatch(offset);
  if (batch.bookings.length === 0) break; // Prevent infinite loop
  allBookings = [...allBookings, ...batch.bookings];
  ...
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Contributor

Choose a reason for hiding this comment

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

DevinAI could you have a go at fixing this edge case?

devin-ai-integration bot and others added 2 commits January 22, 2026 01:01
Co-Authored-By: alex@cal.com <me@alexvanandel.com>
…SV download

Co-Authored-By: alex@cal.com <me@alexvanandel.com>
Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

Approved, it's got progress and i18n, all good to go in.

@emrysal emrysal enabled auto-merge (squash) January 22, 2026 16:23
@emrysal emrysal merged commit 89ae748 into main Jan 22, 2026
83 of 84 checks passed
@emrysal emrysal deleted the download-bookings branch January 22, 2026 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants