Skip to content

Comments

docs: update insights chart dev guide for Prisma v6#23793

Merged
emrysal merged 2 commits intomainfrom
docs/insights-chart-guide
Sep 13, 2025
Merged

docs: update insights chart dev guide for Prisma v6#23793
emrysal merged 2 commits intomainfrom
docs/insights-chart-guide

Conversation

@eunjae-lee
Copy link
Contributor

What does this PR do?

  • Fixes #XXXX (GitHub issue number)
  • Fixes CAL-XXXX (Linear issue number - should be visible at the bottom of the GitHub issue description)

Visual Demo (For contributors especially)

A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).

Video Demo (if applicable):

  • Show screen recordings of the issue or feature.
  • Demonstrate how to reproduce the issue, the behavior before and after the change.

Image Demo (if applicable):

  • Add side-by-side screenshots of the original and updated change.
  • Highlight any significant change(s).

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.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

@vercel
Copy link

vercel bot commented Sep 12, 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 Sep 13, 2025 0:27am
cal-eu Ignored Ignored Sep 13, 2025 0:27am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 12, 2025

Walkthrough

Adds documentation guiding the addition of a new booking insights chart. Describes introducing a new UI component (MyNewChart), exposing it via the booking components barrel, adding a tRPC endpoint (myNewChartData) in the Insights router, and implementing a corresponding service method (InsightsBookingBaseService.getMyNewChartData). Documents refactoring raw SQL usage to Prisma.sql-composed queries passed to $queryRaw for Prisma v6 compatibility. Specifies data mapping to { date: YYYY-MM-DD, value: number } and includes best-practice notes focused on Prisma v6 constraints and consistent chart styling.

Possibly related PRs

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "docs: update insights chart dev guide for Prisma v6" succinctly and accurately captures the primary change in the changeset — updating the insights chart developer guide for Prisma v6 compatibility — and is concise, specific, and directly relevant to the modified documentation and example code.
Description Check ✅ Passed The PR description is a generic template but remains related to the changeset because it references documentation updates, testing guidance, and checklist items relevant to this docs-focused change; it is not completely off-topic. It does lack specific details about the Prisma v6 code adjustments and the exact files changed, so while it passes this lenient check it could be improved with a short summary of the concrete edits and testing steps.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch docs/insights-chart-guide

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@keithwillcode keithwillcode added consumer core area: core, team members only labels Sep 12, 2025
@eunjae-lee eunjae-lee marked this pull request as ready for review September 12, 2025 13:00
@graphite-app graphite-app bot requested a review from a team September 12, 2025 13:00
@dosubot dosubot bot added the docs area: docs, documentation, cal.com/docs label Sep 12, 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: 0

Caution

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

⚠️ Outside diff range comments (2)
packages/features/insights/HOW_TO_ADD_BOOKING_CHARTS.md (1)

148-169: Fix: DATE comes back as string; current mapping will throw.

Postgres DATE columns are returned as strings via $queryRaw. Typing as Date and calling toISOString() will fail at runtime. Return a pre-formatted YYYY-MM-DD from SQL and keep TS type as string.

-      SELECT
-        DATE("createdAt") as date,
-        COUNT(*)::int as "bookingsCount"
+      SELECT
+        to_char("createdAt", 'YYYY-MM-DD') as date,
+        COUNT(*)::int as "bookingsCount"
-    const data = await this.prisma.$queryRaw<
-      Array<{
-        date: Date;
-        bookingsCount: number;
-      }>
-    >(query);
+    const data = await this.prisma.$queryRaw<
+      Array<{
+        date: string; // 'YYYY-MM-DD'
+        bookingsCount: number;
+      }>
+    >(query);
-  return data.map((item) => ({
-    date: item.date.toISOString().split("T")[0], // Format as YYYY-MM-DD
-    value: item.bookingsCount,
-  }));
+  return data.map((item) => ({
+    date: item.date, // already 'YYYY-MM-DD'
+    value: item.bookingsCount,
+  }));
docs/developing/guides/insights/add-new-booking-charts.mdx (1)

161-173: Fix: DATE is returned as string; typing as Date and using toISOString() will break.

Align SQL and TS types to return a pre-formatted string and drop toISOString().

-          const query = Prisma.sql`
+          const query = Prisma.sql`
             SELECT
-              DATE("createdAt") as date,
+              to_char("createdAt", 'YYYY-MM-DD') as date,
               COUNT(*)::int as "bookingsCount"
             FROM "BookingTimeStatusDenormalized"
             WHERE ${baseConditions}
-            GROUP BY DATE("createdAt")
+            GROUP BY to_char("createdAt", 'YYYY-MM-DD')
             ORDER BY date ASC
           `;
-          const data = await this.prisma.$queryRaw<
-            Array<{
-              date: Date;
-              bookingsCount: number;
-            }>
-          >(query);
+          const data = await this.prisma.$queryRaw<
+            Array<{
+              date: string; // 'YYYY-MM-DD'
+              bookingsCount: number;
+            }>
+          >(query);
-          return data.map((item) => ({
-            date: item.date.toISOString().split("T")[0], // Format as YYYY-MM-DD
-            value: item.bookingsCount,
-          }));
+          return data.map((item) => ({
+            date: item.date,
+            value: item.bookingsCount,
+          }));
🧹 Nitpick comments (9)
packages/features/insights/HOW_TO_ADD_BOOKING_CHARTS.md (5)

146-148: Clarify Prisma v6 guidance and imports.

Add the explicit import for Prisma’s sql tag in the snippet header and remind devs to avoid $queryRawUnsafe unless absolutely necessary.

Proposed additions above the class snippet:

+import { Prisma } from "@prisma/client";

And add a short note: “Prefer prisma.$queryRaw(Prisma.sql...) and avoid $queryRawUnsafe.”


41-55: Add error state UI.

You handle pending and success but not errors. Include a simple error fallback to guide consumers.

-  if (isPending) return <LoadingInsight />;
+  if (isPending) return <LoadingInsight />;
+  if (!isPending && !isSuccess) {
+    return <ChartCard title={t("my_new_chart_title")}><p className="text-red-500">{t("failed_to_load")}</p></ChartCard>;
+  }

41-55: Document i18n keys.

The example uses t("my_new_chart_title") and t("no_data_yet"). Add a note to create these keys in the locale files to prevent missing-string fallbacks in production.


181-184: Best Practices: add security and timezone notes.

  • Mention “Avoid $queryRawUnsafe; always compose queries with Prisma.sql/Prisma.join.”
  • Consider timezone when grouping by day (user vs UTC). If needed, use date_trunc with AT TIME ZONE and still return YYYY-MM-DD.

Example bullet additions:

 7. **Date Handling**: Use `getDateRanges()` and `getTimeView()` for time-based charts
 8. **Prisma v6 Compatibility**: Use `Prisma.sql` for the entire query instead of mixing template literals with SQL fragments
+9. **Security**: Prefer `$queryRaw(Prisma.sql\`...\`)`; avoid `$queryRawUnsafe`.
+10. **Timezone**: If grouping by day in a user timezone, use `date_trunc('day', "createdAt" AT TIME ZONE <tz>)` and format with `to_char(..., 'YYYY-MM-DD')`.

106-131: Fix docs: use the same helper in text and snippet (createInsightsBookingService vs getInsightsBookingService).

createInsightsBookingService(ctx, input) is defined in packages/features/insights/server/trpc-router.ts (~line 314) and proxies to getInsightsBookingService exported from packages/lib/di/containers/InsightsBooking.ts (~line 13). Update packages/features/insights/HOW_TO_ADD_BOOKING_CHARTS.md and docs/developing/guides/insights/add-new-booking-charts.mdx so the guidance and code example consistently reference the same helper (either call getInsightsBookingService directly or instruct callers to use the local createInsightsBookingService helper).

docs/developing/guides/insights/add-new-booking-charts.mdx (4)

21-39: Add error state and i18n note in the UI example.

  • Provide an error fallback alongside pending/success.
  • Call out that my_new_chart_title and no_data_yet must be added to locales.

Example minimal addition:

-      if (isPending) return <LoadingInsight />;
+      if (isPending) return <LoadingInsight />;
+      if (!isPending && !isSuccess) {
+        return <ChartCard title={t("my_new_chart_title")}><p className="text-red-500">{t("failed_to_load")}</p></ChartCard>;
+      }

And a short doc note: “Add i18n keys: my_new_chart_title, no_data_yet, failed_to_load.”


140-160: Include Prisma import and caution against $queryRawUnsafe.

For completeness, show import { Prisma } from "@prisma/client" above the class snippet and add a caution to avoid $queryRawUnsafe unless strictly necessary.


182-190: Best Practices: add security and timezone guidance.

Mirror the HOW_TO_ADD_BOOKING_CHARTS.md additions: recommend composing SQL with Prisma.sql and address timezone when grouping by day.


110-135: Docs mismatch — reference createInsightsBookingService in router examples

getInsightsBookingService(...) is the DI container (packages/lib/di/containers/InsightsBooking.ts); packages/features/insights/server/trpc-router.ts defines and uses a local helper createInsightsBookingService(ctx, input). Update the docs to match: change getInsightsBookingService() → createInsightsBookingService(ctx, input) in docs/developing/guides/insights/add-new-booking-charts.mdx and packages/features/insights/HOW_TO_ADD_BOOKING_CHARTS.md, or alternatively update the example to import/use getInsightsBookingService directly (see helper in packages/features/insights/server/trpc-router.ts).

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • 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 74288b0 and ab540ee.

📒 Files selected for processing (2)
  • docs/developing/guides/insights/add-new-booking-charts.mdx (7 hunks)
  • packages/features/insights/HOW_TO_ADD_BOOKING_CHARTS.md (3 hunks)
⏰ 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). (19)
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis

@emrysal emrysal enabled auto-merge (squash) September 13, 2025 12:27
@emrysal emrysal merged commit 06fcea4 into main Sep 13, 2025
37 checks passed
@emrysal emrysal deleted the docs/insights-chart-guide branch September 13, 2025 12:48
@github-actions
Copy link
Contributor

E2E results are ready!

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 docs area: docs, documentation, cal.com/docs ready-for-e2e size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants