feat(data): Data querying - #36
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a new gRPC data query service: updates the proto submodule and build script to generate bindings, introduces Zod request schemas, implements a parameterized query handler with nested filters/sorting/pagination, and wires the service into the gRPC server. ChangesData Query gRPC Service
Sequence Diagram(s)sequenceDiagram
participant gRPCClient
participant queryData
participant ZodValidator
participant TableRegistry
participant Database
participant QueryResponse
gRPCClient->>queryData: QueryRequest
queryData->>ZodValidator: validate with dataQuerySchema
alt validation fails
ZodValidator-->>queryData: Zod error
queryData->>QueryResponse: set validationFailed error
else validation succeeds
ZodValidator-->>queryData: DataQueryRequest
queryData->>TableRegistry: lookup table and field defs
queryData->>queryData: build WHERE predicates from filters
queryData->>queryData: build order-by clauses
par count query
queryData->>Database: SELECT COUNT(*)
and paginated select
queryData->>Database: SELECT columns WITH LIMIT/OFFSET
end
Database-->>queryData: count and rows
queryData->>QueryResponse: populate columns, rows, total
end
QueryResponse-->>gRPCClient: QueryResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/gRPC/data/query.ts`:
- Around line 143-147: The code currently silently skips unknown filter/order
field names (when reading group.conditions and later in the order processing),
which can broaden results; update the loops that reference fieldDef =
tableDef.fields[condition.field] (inside the group.conditions iteration that
calls applyOp) and the analogous order-handling block (lines referencing
order.field and tableDef.fields) to validate that fieldDef exists and
immediately return/throw a validation error (e.g., BadRequest/ValidationError)
with a clear message when a field is not found instead of continuing; ensure the
error is emitted before any DB operation so input validation fails fast.
- Around line 99-107: The castValue function currently coerces non-"true"
strings to false and passes invalid integers as strings, which lets bad inputs
reach SQL predicate building; update castValue (and validate inputs before
calling it) so that for fieldDef.cast === "boolean" you accept only "true" or
"false" and otherwise throw/return a validation error, and for fieldDef.cast ===
"integer" parse with Number and ensure the value is a finite integer (rejecting
non-numeric or non-integer strings) so callers building SQL predicates receive
validated typed values; reference the function name castValue and the FieldDef
type so the validation logic is applied at the same call site that constructs DB
predicates.
- Around line 39-42: Update the TableDef interface and the local result variable
to use explicit types: replace table: any in interface TableDef with table:
typeof usersTable | typeof sessionsTable | typeof tagsTable | typeof
expressionsTable | typeof metadataTable (matching the keys in TABLE_REGISTRY),
and change const result: Record<string, any> to const result: Record<string,
AnyPgColumn>; ensure AnyPgColumn is imported/available where this file defines
the query function so the declared return type aligns with the actual variable
type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c638e5b6-6774-478c-be6d-18a6d97a96f8
⛔ Files ignored due to path filters (3)
src/gen/data/v1/data_grpc_pb.jsis excluded by!**/gen/**src/gen/data/v1/data_pb.d.tsis excluded by!**/gen/**src/gen/data/v1/data_pb.jsis excluded by!**/gen/**
📒 Files selected for processing (5)
package.jsonprotosrc/routes/gRPC/data/query.tssrc/servers/rawGrpcServer.tssrc/zod/data.ts
Summary by CodeRabbit
New Features
Chores