-
Notifications
You must be signed in to change notification settings - Fork 10.4k
chore: Routing Forms - remove unused report.handler.ts and jsonLogicToPrisma functionality #23032
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Routing Forms - remove unused report.handler.ts and jsonLogicToPrisma functionality #23032
Conversation
…lity - Remove unused packages/app-store/routing-forms/trpc/report.handler.ts - Remove packages/app-store/routing-forms/jsonLogicToPrisma.ts implementation - Remove packages/app-store/routing-forms/__tests__/jsonLogicToPrisma.test.ts test file These files were no longer being used in the routing-forms package. Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
- Remove unused report.schema.ts file - Remove buildResponsesForReporting test file - Remove report endpoint from router - Update comment referencing jsonLogicToPrisma All references to the removed jsonLogicToPrisma and report handler functionality have been cleaned up. Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
🤖 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 PR removes the Routing Forms reporting backend. It deletes the jsonLogicToPrisma translator and its tests, the TRPC report handler, its schema, and related tests. The TRPC router’s report endpoint is removed. A comment in getQueryBuilderConfigForFormFields is simplified without behavior changes. Public API removals include: jsonLogicToPrisma and JsonLogicQuery, reportHandler (default and named) and buildResponsesForReporting, ZReportInputSchema and TReportInputSchema, and the appRoutingForms.report route. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/12/25)1 reviewer was added to this PR based on Keith Williams's automation. |
There was a problem hiding this 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
♻️ Duplicate comments (1)
packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts (1)
87-90
: LGTM: Comment simplification matches prior suggestionThe simplified comment aligns with the earlier suggestion and the runtime behavior remains unchanged.
🧹 Nitpick comments (1)
packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts (1)
87-90
: Retain brief rationale to avoid future reintroduction of unsupported operatorsThe new comment loses the original rationale. A short reason helps future maintainers understand why these are disabled and prevents accidental reintroduction.
Apply this diff to clarify intent:
- // These operators can be implemented later if needed. + // "between" and "not_between" are disabled for reporting because our Prisma JSON query path + // can't translate them reliably today. Revisit if/when a safe translation layer is added.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/app-store/routing-forms/__tests__/jsonLogicToPrisma.test.ts
(0 hunks)packages/app-store/routing-forms/jsonLogicToPrisma.ts
(0 hunks)packages/app-store/routing-forms/lib/getQueryBuilderConfig.ts
(1 hunks)packages/app-store/routing-forms/trpc/_router.ts
(0 hunks)packages/app-store/routing-forms/trpc/buildResponsesForReporting.test.ts
(0 hunks)packages/app-store/routing-forms/trpc/report.handler.ts
(0 hunks)packages/app-store/routing-forms/trpc/report.schema.ts
(0 hunks)
💤 Files with no reviewable changes (6)
- packages/app-store/routing-forms/trpc/report.schema.ts
- packages/app-store/routing-forms/trpc/buildResponsesForReporting.test.ts
- packages/app-store/routing-forms/trpc/_router.ts
- packages/app-store/routing-forms/jsonLogicToPrisma.ts
- packages/app-store/routing-forms/tests/jsonLogicToPrisma.test.ts
- packages/app-store/routing-forms/trpc/report.handler.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts
: For Prisma queries, only select data you need; never useinclude
, always useselect
Ensure thecredential.key
field is never returned from tRPC endpoints or APIs
Files:
packages/app-store/routing-forms/lib/getQueryBuilderConfig.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/app-store/routing-forms/lib/getQueryBuilderConfig.ts
E2E results are ready! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
feat: remove unused report.handler.ts and jsonLogicToPrisma functionality
Fixes PRI-303
Summary
This PR removes unused reporting functionality from the routing-forms package as requested in the continuation of PR #22990. The changes include:
report.handler.ts
(182 lines) - unused tRPC handler for form reportingjsonLogicToPrisma.ts
(277 lines) - unused utility for converting JSON Logic queries to Prisma where clausesjsonLogicToPrisma.test.ts
- comprehensive test suite for the removed utilitybuildResponsesForReporting.test.ts
- test file that depended on report.handlerreport.schema.ts
- Zod schema definitions for report input_router.ts
- removed the unusedreport
tRPC endpoint and related importsgetQueryBuilderConfig.ts
- cleaned up comment that referenced the removed functionReview & Testing Checklist for Human
Diagram
Notes
yarn type-check:ci --force
Link to Devin run: https://app.devin.ai/sessions/abad8df0a97f484f9fee98ab9d87b1fa
Requested by: @hariombalhara