Skip to content

Comments

feat: add paid, userEmail, userName, and rating filters to insights page#23299

Merged
Udit-takkar merged 9 commits intomainfrom
devin/add-insights-filters-1755940191
Aug 27, 2025
Merged

feat: add paid, userEmail, userName, and rating filters to insights page#23299
Udit-takkar merged 9 commits intomainfrom
devin/add-insights-filters-1755940191

Conversation

@eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Aug 23, 2025

What does this PR do?

This PR adds three new filter types to the /insights page to enhance booking data analysis capabilities:

  • paid - SINGLE_SELECT filter with "Paid"/"Free" options to distinguish between paid and free bookings
  • userEmail - TEXT filter for searching bookings by user email addresses
  • userName - TEXT filter for searching bookings by user names

The implementation follows established patterns from the /bookings page and leverages existing columns from the BookingTimeStatusDenormalized view.

Link to Devin run: https://app.devin.ai/sessions/e4288a28d13247e9af7ae9a8767d17b7
Requested by: @eunjae-lee

Technical Changes

Frontend Changes

  • Updated useInsightsBookings.ts to add three new column definitions with appropriate filter types
  • Modified DummyTableRow type to include the new filter fields
  • Enhanced useInsightsBookingFacetedUniqueValues.ts to provide "Paid"/"Free" options for the paid filter

Backend Changes

  • Added filter handling logic in InsightsBookingBaseService.ts buildColumnFilterCondition method
  • Used proper Prisma.sql parameterization to prevent SQL injection
  • Added missing isTextFilterValue import for text filter support

⚠️ Critical Review Requirements

IMPORTANT: Due to authentication issues in the local dev environment, end-to-end UI testing could not be completed. Manual testing of the filter functionality is essential before merging.

High-priority review areas:

  1. 🔴 Manual UI Testing Required:

    • Navigate to /insights page and verify all three filters appear correctly
    • Test each filter's functionality independently and in combination
    • Verify filter state persistence and clearing works properly
  2. 🔴 Translation Keys Verification:

    • Confirm paid, free, user_email, user_name keys exist in the i18n system
    • Check if any additional translation keys need to be added
  3. 🔴 Data Availability:

    • Verify userEmail and userName columns are properly populated in the BookingTimeStatusDenormalized database view
    • Confirm the paid column contains boolean values as expected
  4. 🔴 Type Safety:

    • Review the paid filter's boolean-to-string conversion logic in useInsightsBookingFacetedUniqueValues.ts
    • Ensure the buildColumnFilterCondition method handles the paid filter type conversion correctly
  5. 🔴 SQL Query Construction:

    • Verify the SQL query construction in buildColumnFilterCondition is secure and correct
    • Test with various input values to ensure ILIKE queries work as expected for text filters

How should this be tested?

Prerequisites

  • Local Cal.com development environment with working authentication
  • Database with booking data containing populated paid, userEmail, and userName fields
  • Access to /insights page (requires appropriate permissions)

Test Cases

1. Paid Filter (SINGLE_SELECT)

  • Navigate to /insights page
  • Verify "Paid" filter appears with dropdown containing "Paid" and "Free" options
  • Test filtering by "Paid" only - should show only paid bookings
  • Test filtering by "Free" only - should show only free bookings
  • Verify only one option can be selected at a time

2. User Email Filter (TEXT)

  • Test searching for complete email addresses
  • Test partial email searches (should use ILIKE behavior)
  • Test with special characters and various email formats
  • Verify case-insensitive matching works

3. User Name Filter (TEXT)

  • Test searching for complete user names
  • Test partial name searches
  • Test with various name formats and special characters
  • Verify case-insensitive matching works

4. Integration Testing

  • Test combining multiple filters (e.g., paid + userEmail)
  • Verify existing filters (eventTypeId, status, userId) still work
  • Test clearing individual filters and all filters
  • Verify filter state is maintained during page navigation

Expected Behavior

  • All three new filters should integrate seamlessly with existing filter UI
  • Filters should return accurate, filtered results from the backend
  • No console errors or TypeScript warnings
  • Filter combinations should work logically (AND behavior)

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. If N/A, write N/A here and check the checkbox. N/A - This is an internal feature enhancement that doesn't require documentation changes.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works. Note: Due to authentication limitations, comprehensive manual testing is required to verify functionality.

Checklist

  • I have read the contributing guide
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have checked if my changes generate no new warnings (TypeScript checks pass)

- Add paid filter as MULTI_SELECT with 'Paid'/'Free' options
- Add userEmail filter as TEXT for searching user emails
- Add userName filter as TEXT for searching user names
- Update DummyTableRow type to include new filter columns
- Add backend filtering logic in buildColumnFilterCondition method
- Use existing columns from BookingTimeStatusDenormalized view
- Follow established patterns from /bookings page implementation

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 23, 2025

Walkthrough

Adds i18n support for a new paid facet in useInsightsBookingFacetedUniqueValues. Extends DummyTableRow with paid: boolean, userEmail: string | null, userName: string | null, and rating: number | null, and appends columns paid, userEmail, userName, and rating in useInsightsBookings with corresponding filter types. Imports makeSqlCondition, isTextFilterValue, and isNumberFilterValue, and adds paid, userEmail, userName, and rating branches to buildColumnFilterCondition in InsightsBookingBaseService. Adds "free": "Free", "user_email": "User Email", and "user_name": "User Name" to the English locale. No exported/public signatures changed.

Possibly related PRs


📜 Recent 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 d28ef5f and d47894c.

📒 Files selected for processing (1)
  • apps/web/public/static/locales/en/common.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/public/static/locales/en/common.json
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/add-insights-filters-1755940191

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 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.

- Updated useInsightsBookings.ts to use ColumnFilterType.SINGLE_SELECT for paid filter
- Modified buildColumnFilterCondition to handle single select with isSingleSelectFilterValue
- Simplified backend logic to handle single boolean value instead of array

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
@vercel
Copy link

vercel bot commented Aug 24, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Aug 27, 2025 3:26pm
cal-eu Ignored Ignored Aug 27, 2025 3:26pm

@devin-ai-integration devin-ai-integration bot marked this pull request as ready for review August 24, 2025 17:51
@graphite-app graphite-app bot requested a review from a team August 24, 2025 17:51
@dosubot dosubot bot added insights area: insights, analytics ✨ feature New feature or request labels Aug 24, 2025
@graphite-app
Copy link

graphite-app bot commented Aug 24, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (08/24/25)

1 reviewer was added to this PR based on Keith Williams's automation.

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: 1

🧹 Nitpick comments (2)
packages/lib/server/service/InsightsBookingBaseService.ts (1)

331-337: Trim operands and optionally escape wildcard characters in ILIKE filters.

Functionally OK and parameterized (safe). For better UX and predictability:

  • Trim operands; ignore empty strings to avoid adding no-op conditions.
  • Optionally escape % and _ if you intend a literal substring match rather than allowing user-supplied wildcards.

Apply minimal trimming (keeps current wildcard behavior):

-    if (id === "userEmail" && isTextFilterValue(value)) {
-      return Prisma.sql`"userEmail" ILIKE ${`%${value.data.operand}%`}`;
-    }
+    if (id === "userEmail" && isTextFilterValue(value)) {
+      const operand = value.data.operand.trim();
+      if (!operand) return null;
+      return Prisma.sql`"userEmail" ILIKE '%' || ${operand} || '%'`;
+    }
@@
-    if (id === "userName" && isTextFilterValue(value)) {
-      return Prisma.sql`"userName" ILIKE ${`%${value.data.operand}%`}`;
-    }
+    if (id === "userName" && isTextFilterValue(value)) {
+      const operand = value.data.operand.trim();
+      if (!operand) return null;
+      return Prisma.sql`"userName" ILIKE '%' || ${operand} || '%'`;
+    }

If you prefer strict literal substring matching, I can provide a small escapeLikeForIlike() helper and add ESCAPE '\\'.

On large datasets, consider trigram GIN indexes to keep these ILIKE scans fast:

  • CREATE EXTENSION IF NOT EXISTS pg_trgm;
  • CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_bts_useremail_trgm ON "BookingTimeStatusDenormalized" USING gin ("userEmail" gin_trgm_ops);
  • Same for "userName".
packages/features/insights/hooks/useInsightsBookings.ts (1)

86-111: Text filter columns are consistent; consider a tiny helper to avoid duplication.

Both columns share identical config except id/header/accessor/size. Optional: extract a small factory to reduce repetition.

Example helper (outside this block) and replacement use:

// helper near top of file
const textFilterCol = <K extends keyof DummyTableRow>(key: K, id: string, header: string, size = 200) =>
  columnHelper.accessor(key, {
    id,
    header,
    size,
    meta: { filter: { type: ColumnFilterType.TEXT } },
    enableColumnFilter: true,
    enableSorting: false,
    cell: () => null,
  });

// inside columns:
textFilterCol("userEmail", "userEmail", t("user_email"));
textFilterCol("userName", "userName", t("user_name"));
📜 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 d2f8076 and 58b7c61.

📒 Files selected for processing (3)
  • packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts (3 hunks)
  • packages/features/insights/hooks/useInsightsBookings.ts (2 hunks)
  • packages/lib/server/service/InsightsBookingBaseService.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*Service.ts

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

Service files must include Service suffix, use PascalCase matching exported class, and avoid generic names (e.g., MembershipService.ts)

Files:

  • packages/lib/server/service/InsightsBookingBaseService.ts
**/*.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/lib/server/service/InsightsBookingBaseService.ts
  • packages/features/insights/hooks/useInsightsBookings.ts
  • packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.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/lib/server/service/InsightsBookingBaseService.ts
  • packages/features/insights/hooks/useInsightsBookings.ts
  • packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.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/lib/server/service/InsightsBookingBaseService.ts
  • packages/features/insights/hooks/useInsightsBookings.ts
  • packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.539Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
📚 Learning: 2025-07-24T08:39:06.185Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.

Applied to files:

  • packages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-07-15T13:02:17.403Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/lib/server/service/insightsRouting.ts:367-368
Timestamp: 2025-07-15T13:02:17.403Z
Learning: In the InsightsRoutingService (packages/lib/server/service/insightsRouting.ts), multi-select filter data is already validated by zod before reaching the buildFormFieldSqlCondition method, so null/undefined values are not present in filterValue.data arrays and don't need to be filtered out.

Applied to files:

  • packages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-08-22T16:38:00.182Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/lib/server/service/InsightsBookingBaseService.ts:814-816
Timestamp: 2025-08-22T16:38:00.182Z
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/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-08-21T12:28:42.000Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.000Z
Learning: In BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.

Applied to files:

  • packages/features/insights/hooks/useInsightsBookings.ts
🧬 Code graph analysis (2)
packages/lib/server/service/InsightsBookingBaseService.ts (1)
packages/features/data-table/lib/utils.ts (2)
  • isSingleSelectFilterValue (86-88)
  • isTextFilterValue (49-51)
packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts (2)
packages/features/data-table/lib/utils.ts (1)
  • convertFacetedValuesToMap (150-154)
packages/features/data-table/lib/types.ts (1)
  • FacetedValue (222-226)
⏰ 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). (10)
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (6)
packages/lib/server/service/InsightsBookingBaseService.ts (2)

8-12: Importing isTextFilterValue improves type-safety for text filters.

This aligns with the backend filter additions and prevents unsafe access to value.data for TEXT filters.


326-329: Confirmed UI sends “true”/“false” strings for the paid filter
The frontend hook at packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts defines the paid column’s options as { value: "true" } and { value: "false" }, which aligns with the backend coercion in InsightsBookingBaseService.ts (lines 326–329). No changes are needed.

packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts (2)

73-77: Paid facet values and labels match backend expectations.

Values "true"/"false" align with the service’s coercion. Order (Paid, Free) is sensible.


81-81: Dependency array correctly includes t.

Prevents stale labels when locale changes.

packages/features/insights/hooks/useInsightsBookings.ts (2)

15-17: DummyTableRow extension is consistent with backend schema.

Types for paid, userEmail, and userName align with BookingTimeStatusDenormalized.


73-85: "Paid" column wiring is correct.

  • SINGLE_SELECT matches the faceted map in useInsightsBookingFacetedUniqueValues.
  • Sorting disabled is appropriate for a boolean-as-select column.

import { useCallback } from "react";

import { convertFacetedValuesToMap, type FacetedValue } from "@calcom/features/data-table";
import { useLocale } from "@calcom/lib/hooks/useLocale";
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 24, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

i18n hookup looks good; ensure keys exist.

Importing useLocale and using t is correct. Please verify that "paid" and "free" keys exist in locale JSONs.

Also applies to: 28-28


🏁 Script executed:

#!/bin/bash
# Verify i18n keys exist somewhere in locale JSON files
rg -nP '"(paid|free|user_email|user_name)"\s*:' -g '!**/node_modules/**' -S

Length of output: 3059


Add missing i18n keys for “free”, “user_email”, and “user_name”

The lookup for “paid” succeeded across all locale JSONs, but the keys “free”, “user_email”, and “user_name” were not found. Please add these entries to each common.json (and any other relevant namespace) under apps/web/public/static/locales/*/. For example, in apps/web/public/static/locales/en/common.json:

 {
   "paid": "Paid",
+  "free": "Free",
+  "user_email": "User email",
+  "user_name": "User name",
   …
 }

– Repeat for every locale to ensure complete coverage.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts
around line 5, the code references i18n keys "free", "user_email", and
"user_name" that are missing; add these keys to every locale file (e.g.,
apps/web/public/static/locales/*/common.json and any other used namespace) with
appropriate translations (matching structure of existing keys like "paid"),
ensure JSON syntax is valid, and run the i18n/locales validation or app build to
confirm no missing-key errors remain.

Copy link
Contributor

Choose a reason for hiding this comment

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

@eunjae-lee this is correct. user_email, user_name doesn't exist in common.json

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Udit-takkar thanks for pointing it out ! just added a commit for that

@github-actions
Copy link
Contributor

github-actions bot commented Aug 24, 2025

E2E results are ready!

- Add rating field to DummyTableRow type
- Add rating column with SINGLE_SELECT filter type
- Add rating options 1-5 in faceted unique values
- Add rating filtering logic in backend service
- Rating filter allows filtering bookings by 1-5 star ratings

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
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: 1

♻️ Duplicate comments (2)
packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts (1)

73-77: Ensure “free” i18n key exists across locales.

The UI labels t("paid") and t("free") are correct, but prior checks showed “free” missing in several locale JSONs. Add/verify "free" in all locales to avoid runtime fallbacks.

Run to confirm coverage:

#!/bin/bash
# Verify presence of keys in locale files
rg -nP '"(paid|free)"\s*:' apps/web/public/static/locales -S -g '!**/node_modules/**'
packages/features/insights/hooks/useInsightsBookings.ts (1)

100-112: i18n keys for “user_email” and “user_name”.

Headers use t("user_email") and t("user_name"). Prior scans indicated these keys were missing in some locales. Add them across locales to avoid untranslated labels.

Check coverage:

#!/bin/bash
rg -nP '"(user_email|user_name)"\s*:' apps/web/public/static/locales -S -g '!**/node_modules/**'
🧹 Nitpick comments (3)
packages/features/insights/hooks/useInsightsBookings.ts (3)

15-18: Double-check nullability for “paid”; align with data source.

If BookingTimeStatusDenormalized.paid can be null (unknown/NA), consider typing as boolean | null to avoid accidental narrowing. If it’s guaranteed non-null in the denormalized view, ignore.

Optional type tweak:

-  paid: boolean;
+  paid: boolean | null;

113-125: Confirm “rating” column belongs in this PR; ensure end-to-end support.

This adds a rating filter/column not mentioned in the PR objectives. Confirm product intent, ensure i18n for t("rating") exists, and verify the backend implements columnId === "rating" handling.

If keeping it, consider constraining the domain in types:

-  rating: number | null;
+  rating: 1 | 2 | 3 | 4 | 5 | null;

33-36: Reduce duplication of cell: () => null and filter meta boilerplate.

Consider a small helper to DRY column creation for “filter-only” columns (no cell renderer, sorting disabled), improving readability and consistency.

Example helper (outside this diff scope):

const filterOnly = <T,>(c: ColumnDef<T>) => ({ enableSorting: false, cell: () => null, ...c });

Then wrap each column def with filterOnly({...}).

Also applies to: 62-73, 74-126

📜 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 5dd6427 and cc1e849.

📒 Files selected for processing (3)
  • packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts (3 hunks)
  • packages/features/insights/hooks/useInsightsBookings.ts (2 hunks)
  • packages/lib/server/service/InsightsBookingBaseService.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/lib/server/service/InsightsBookingBaseService.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/features/insights/hooks/useInsightsBookings.ts
  • packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.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/features/insights/hooks/useInsightsBookings.ts
  • packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.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/features/insights/hooks/useInsightsBookings.ts
  • packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts
🧬 Code graph analysis (1)
packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts (2)
packages/features/data-table/lib/utils.ts (1)
  • convertFacetedValuesToMap (150-154)
packages/features/data-table/lib/types.ts (1)
  • FacetedValue (222-226)
⏰ 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). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (3)
packages/features/insights/hooks/useInsightsBookingFacetedUniqueValues.ts (1)

5-5: Locale hookup and dependency coverage look correct.

Good use of useLocale and inclusion of t in the useCallback deps to ensure labels react to locale changes.

Also applies to: 28-28, 89-89

packages/features/insights/hooks/useInsightsBookings.ts (2)

74-86: Frontend ‘paid’ column wiring confirmed; please verify backend alignment

  • In useInsightsBookings.ts, the paid accessor is correctly set up with SINGLE_SELECT and cell: () => null.
  • In useInsightsBookingFacetedUniqueValues.ts (line 73), there is a branch for columnId === "paid" that maps faceted values { value: "true", label: t("paid") }.

Ensure that the backend schema or API for bookings exposes a field named "paid" so that this columnId aligns end-to-end and avoids silent mismatches.


87-99: PII Filter Safety: Verify Server-Side Access Controls for userEmail

Exposing a free-text userEmail filter in the UI means users can query bookings by arbitrary email strings. Before shipping, please confirm on the server that every viewer.insights.* endpoint:

• Authenticates the caller (e.g. via TRPC’s session)
• Restricts returned records to the caller’s own team/org context (e.g. WHERE teamId = session.teamId or orgId = session.orgId)
• Applies these scoping rules before any text filter on email is applied

Without explicit tenant scoping in the resolver, a malicious user could craft a filter that leaks PII across org boundaries.

Action items:

  • Review each resolver in your viewer.insights TRPC router (e.g. the handler for bookings queries such as bookingsByHourStats, membersWithMostBookings, etc.).
  • Ensure the incoming session’s teamId/orgId is enforced in the database query before injecting any WHERE email LIKE … clause.
  • Add automated tests that attempt to fetch another tenant’s email and assert it returns no results.

devin-ai-integration bot and others added 2 commits August 26, 2025 09:26
- Update rating filter from SINGLE_SELECT to NUMBER in useInsightsBookings.ts
- Remove rating options from useInsightsBookingFacetedUniqueValues.ts since NUMBER filters don't need predefined options
- Add isNumberFilterValue import and implement number filter logic in InsightsBookingBaseService.ts
- Support eq, gt, gte, lt, lte operators for rating number filter

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
- Import makeSqlCondition from @calcom/features/data-table/lib/server
- Replace manual SQL condition building with makeSqlCondition utility for userEmail, userName, and rating filters
- Maintain existing paid filter logic for boolean conversion
- Follow established pattern from InsightsRoutingBaseService
- Cleaner, more maintainable code with consistent filter handling

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
@eunjae-lee eunjae-lee marked this pull request as ready for review August 26, 2025 09:40
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.

Left one comment #23299 (comment)

@eunjae-lee eunjae-lee changed the title feat: add paid, userEmail, and userName filters to insights page feat: add paid, userEmail, userName, and rating filters to insights page Aug 27, 2025
@Udit-takkar Udit-takkar enabled auto-merge (squash) August 27, 2025 15:26
@Udit-takkar Udit-takkar merged commit b7a7797 into main Aug 27, 2025
40 checks passed
@Udit-takkar Udit-takkar deleted the devin/add-insights-filters-1755940191 branch August 27, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consumer core area: core, team members only ✨ feature New feature or request insights area: insights, analytics ready-for-e2e

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants