refactor: rename insights routing service files #22917
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Rename insightsRoutingDI.ts -> InsightsRoutingDIService.ts - Rename insightsRoutingBase.ts -> InsightsRoutingBaseService.ts - Update all import statements to use new file names - No functional changes, only file renames and import updates Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
WalkthroughThis change updates import paths for several service classes and interfaces related to insights routing across multiple files. The modifications standardize the capitalization and naming conventions of module paths, such as changing imports from lowercased file names to their PascalCase equivalents (e.g., Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/06/25)1 reviewer was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/lib/server/service/InsightsRoutingDIService.ts (1)
13-29: Align filename and exported class name in the DI wrapperThe file
packages/lib/server/service/InsightsRoutingDIService.tsdeclaresexport class InsightsRoutingServicebut its filename includes the extra “DI” segment. To satisfy our rule—service files must use PascalCase matching the exported class—you must choose one of:
- Rename the class to include “DI”:
--- a/packages/lib/server/service/InsightsRoutingDIService.ts +++ b/packages/lib/server/service/InsightsRoutingDIService.ts @@ -export class InsightsRoutingService { +export class InsightsRoutingDIService {- Or rename the file (and update imports) to remove “DI” so it matches the class:
• RenameInsightsRoutingDIService.ts→InsightsRoutingService.tsLeaving them out of sync may break our tooling and complicate future refactors.
🧹 Nitpick comments (1)
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts (1)
10-10: Alias may cause confusion in future tests.
InsightsRoutingBaseServiceis imported then aliased asInsightsRoutingService.
Consider keeping the original name to avoid clashing with the DI wrapper (InsightsRoutingServicefromInsightsRoutingDIService.ts).-import { InsightsRoutingBaseService as InsightsRoutingService } from "../../service/InsightsRoutingBaseService"; +import { InsightsRoutingBaseService } from "../../service/InsightsRoutingBaseService";and update usages accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/lib/di/containers/insights-routing.ts(1 hunks)packages/lib/di/modules/insights-routing.ts(1 hunks)packages/lib/server/service/InsightsRoutingDIService.ts(1 hunks)packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts(1 hunks)
🧰 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 useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.tspackages/lib/server/service/InsightsRoutingDIService.tspackages/lib/di/containers/insights-routing.tspackages/lib/di/modules/insights-routing.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/__tests__/InsightsRoutingService.integration-test.tspackages/lib/server/service/InsightsRoutingDIService.tspackages/lib/di/containers/insights-routing.tspackages/lib/di/modules/insights-routing.ts
**/*Service.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/lib/server/service/InsightsRoutingDIService.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Service.ts : Service files must include `Service` suffix, use PascalCase matching exported class, and avoid generic names (e.g., `MembershipService.ts`)
📚 Learning: applies to **/*service.ts : service files must include `service` suffix, use pascalcase matching exp...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*Service.ts : Service files must include `Service` suffix, use PascalCase matching exported class, and avoid generic names (e.g., `MembershipService.ts`)
Applied to files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.tspackages/lib/server/service/InsightsRoutingDIService.ts
📚 Learning: applies to **/*.{service,repository}.ts : avoid dot-suffixes like `.service.ts` or `.repository.ts` ...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{service,repository}.ts : Avoid dot-suffixes like `.service.ts` or `.repository.ts` for new files; reserve `.test.ts`, `.spec.ts`, `.types.ts` for their specific purposes
Applied to files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.tspackages/lib/server/service/InsightsRoutingDIService.ts
📚 Learning: in packages/app-store/office365calendar/lib/calendarservice.ts, the fetcher method in office365calen...
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
Applied to files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts
📚 Learning: in the insightsbookingservice (packages/lib/server/service/insightsbooking.ts), the constructor stor...
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/__tests__/InsightsRoutingService.integration-test.tspackages/lib/server/service/InsightsRoutingDIService.tspackages/lib/di/containers/insights-routing.ts
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.tspackages/lib/server/service/InsightsRoutingDIService.tspackages/lib/di/containers/insights-routing.tspackages/lib/di/modules/insights-routing.ts
📚 Learning: in the insightsroutingservice (packages/lib/server/service/insightsrouting.ts), multi-select filter ...
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/__tests__/InsightsRoutingService.integration-test.tspackages/lib/server/service/InsightsRoutingDIService.tspackages/lib/di/containers/insights-routing.ts
📚 Learning: applies to **/*.ts : for prisma queries, only select data you need; never use `include`, always use ...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : For Prisma queries, only select data you need; never use `include`, always use `select`
Applied to files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.ts
📚 Learning: in the insights routing funnel component (packages/features/insights/components/routingfunnel.tsx), ...
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.
Applied to files:
packages/lib/server/service/__tests__/InsightsRoutingService.integration-test.tspackages/lib/server/service/InsightsRoutingDIService.tspackages/lib/di/containers/insights-routing.tspackages/lib/di/modules/insights-routing.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Detect changes
- GitHub Check: Detect changes
- GitHub Check: Detect changes
- GitHub Check: Detect changes
- GitHub Check: Detect changes
- GitHub Check: Detect changes
- GitHub Check: Detect changes
- GitHub Check: Detect changes
- GitHub Check: Detect changes
- GitHub Check: Detect changes
🔇 Additional comments (3)
packages/lib/server/service/InsightsRoutingDIService.ts (1)
4-8: Import path update looks good.The path now correctly references
InsightsRoutingBaseService.tsafter the rename.packages/lib/di/modules/insights-routing.ts (1)
3-5: Updated imports are consistent with the new filenames.No functional changes introduced; DI bindings remain intact.
packages/lib/di/containers/insights-routing.ts (1)
8-10: Import paths correctly reflect the renamed services.Container wiring remains valid and type-safe.
volnei
previously approved these changes
Aug 8, 2025
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
volnei
approved these changes
Aug 18, 2025
E2E results are ready! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refactor: rename insights routing service files
Summary
This PR renames two insights routing service files to follow a more consistent naming convention with proper capitalization and "Service" suffix:
insightsRoutingDI.ts→InsightsRoutingDIService.tsinsightsRoutingBase.ts→InsightsRoutingBaseService.tsAll corresponding import statements throughout the codebase have been updated to use the new file names. This is a pure refactoring change with no functional modifications - only file renames and import path updates.
Files affected:
Review & Testing Checklist for Human
insightsRoutingDI,insightsRoutingBase) that might have been missedDiagram
%%{ init : { "theme" : "default" }}%% graph TD DIM["packages/lib/di/modules/<br/>insights-routing.ts"]:::minor-edit DIC["packages/lib/di/containers/<br/>insights-routing.ts"]:::minor-edit TEST["packages/lib/server/service/__tests__/<br/>InsightsRoutingService.integration-test.ts"]:::minor-edit DISERVICE["packages/lib/server/service/<br/>InsightsRoutingDIService.ts<br/>(renamed from insightsRoutingDI.ts)"]:::major-edit BASESERVICE["packages/lib/server/service/<br/>InsightsRoutingBaseService.ts<br/>(renamed from insightsRoutingBase.ts)"]:::major-edit DIM -->|"imports IInsightsRoutingService,<br/>InsightsRoutingService"| DISERVICE DIC -->|"imports types from both services"| DISERVICE DIC -->|"imports types"| BASESERVICE TEST -->|"imports InsightsRoutingBaseService as InsightsRoutingService"| BASESERVICE DISERVICE -->|"imports InsightsRoutingBaseService"| BASESERVICE subgraph Legend L1[Major Edit]:::major-edit L2[Minor Edit]:::minor-edit L3[Context/No Edit]:::context end classDef major-edit fill:#90EE90 classDef minor-edit fill:#87CEEB classDef context fill:#FFFFFFNotes
yarn type-check:ci), indicating all major import references were successfully updatedSession Info: Requested by @eunjae-lee
Devin Session: https://app.devin.ai/sessions/4b5f44c1b68a405aa7f64e87308a0c87