eng-254: Canelndar View & Filter Builder#354
Merged
Connorbelez merged 2 commits intomainfrom Mar 30, 2026
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR implements calendar-view query functionality and a filter management component. It adds a Convex backend query for retrieving calendar-view records with date filtering and period grouping, extracts shared filter constants and validation logic, exports additional helpers from recordQueries, refactors viewDefs validation routines, and introduces a React FilterBuilder component for creating and managing view filters through UI. Changes
Sequence DiagramsequenceDiagram
participant Client
participant CalendarQuery as queryCalendarRecords
participant IndexScan as recordValuesDate Index
participant RecordAssembly as Record Assembly
participant FilterEngine as Filter Evaluation
participant Grouping as Date Grouping
Client->>CalendarQuery: viewDefId, rangeStart, rangeEnd, granularity
CalendarQuery->>CalendarQuery: Validate viewDef, objectDef, calendar config
CalendarQuery->>IndexScan: Scan by_object_field_value(objectDefId, fieldDefId, [rangeStart, rangeEnd])
IndexScan-->>CalendarQuery: Deduplicated record IDs with date values
CalendarQuery->>RecordAssembly: Load record docs and assemble UnifiedRecord objects
RecordAssembly-->>CalendarQuery: UnifiedRecord[] (with field definitions applied)
CalendarQuery->>FilterEngine: Apply view-level filters to assembled records
FilterEngine->>FilterEngine: Parse filter operators, validate field types, evaluate predicates
FilterEngine-->>CalendarQuery: Filtered UnifiedRecord[]
CalendarQuery->>Grouping: Group records by truncated date (granularity: day/week/month)
Grouping-->>CalendarQuery: Bucketed events with date keys
CalendarQuery-->>Client: { events: [{ date, records }], range, skippedFilters, truncated }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Connorbelez
force-pushed
the
Connorbelez/eng-254-view-definitions
branch
from
8c67749 to
6d4c4f5
Compare
There was a problem hiding this comment.
Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Pull request overview
Implements ENG-254 by adding a reusable CRM view FilterBuilder UI and a calendar-view backend query that range-scans date values, assembles records, and applies view-level filters. Also introduces shared filter constants and minor formatting/refactor changes to support reuse.
Changes:
- Added
FilterBuilderReact component for creating/removing view filters (field/operator/value + AND/OR connector). - Added
queryCalendarRecordsConvex query to fetch calendar events by date range and granularity, with second-pass view filter application. - Centralized filter operator/type constants into a shared module and exported record-assembly/filter helpers for reuse.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/admin/shell/FilterBuilder.tsx | New UI for managing viewFilters via Convex queries/mutations. |
| convex/crm/calendarQuery.ts | New calendar view query: date range scan + record assembly + view filter application. |
| convex/crm/filterConstants.ts | New shared client-safe filter/operator constants (used by client + server). |
| convex/crm/filterOperatorValidation.ts | Updated to consume shared constants instead of _generated types. |
| convex/crm/recordQueries.ts | Exported shared helpers and tightened filter input validation. |
| convex/crm/types.ts | Converted several exported types to interfaces (shape preserved). |
| convex/crm/viewDefs.ts | Refactored helper naming and adjusted kanban group creation behavior. |
| convex/crm/viewFilters.ts | Import formatting only. |
| convex/crm/viewFields.ts | Error throw formatting only. |
| convex/crm/viewKanbanGroups.ts | Error throw formatting only. |
| convex/_generated/api.d.ts | Regenerated API type declarations for new/updated modules. |
| specs/ENG-254/tasks.md | Added ENG-254 master task list. |
| specs/ENG-254/chunks/manifest.md | Added chunk manifest. |
| specs/ENG-254/chunks/chunk-01-calendar-query/* | Added calendar query chunk context/tasks. |
| specs/ENG-254/chunks/chunk-02-filter-builder/* | Added filter builder chunk context/tasks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Connorbelez
force-pushed
the
Connorbelez/eng-254-view-definitions
branch
from
6d4c4f5 to
f12eb20
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
convex/crm/filterOperatorValidation.ts (1)
12-14:⚠️ Potential issue | 🟡 MinorStale error message references wrong file.
The error message directs developers to update
filterOperatorValidation.ts, butOPERATOR_MAPhas been moved tofilterConstants.ts.🔧 Suggested fix
if (operators === undefined) { throw new Error( - `No operators defined for field type "${fieldType}". Update OPERATOR_MAP in filterOperatorValidation.ts.` + `No operators defined for field type "${fieldType}". Update OPERATOR_MAP in filterConstants.ts.` ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/filterOperatorValidation.ts` around lines 12 - 14, The thrown Error in filterOperatorValidation.ts currently tells devs to update OPERATOR_MAP in filterOperatorValidation.ts but OPERATOR_MAP was moved to filterConstants.ts; update the error message in the throw new Error(...) to reference OPERATOR_MAP in filterConstants.ts (preserve existing message structure and interpolation of fieldType and only change the filename text) so the guidance points to the correct symbol location.
🧹 Nitpick comments (3)
convex/crm/filterConstants.ts (1)
1-98: Well-structured shared constants module.The types and mappings are correctly synchronized with the validators in
convex/crm/validators.ts. The module is properly designed for client-side reuse without server dependencies.One consideration: since
FieldTypeandFilterOperatorare duplicated string unions (here and in validators), changes to one must be manually reflected in the other to avoid runtime mismatches.💡 Optional: Consider deriving types from a single source of truth
You could potentially derive the validator from this constants file (or vice versa) to ensure they stay in sync:
// In filterConstants.ts - define as const arrays export const FIELD_TYPES = [ "text", "number", "boolean", /* ... */ ] as const; export type FieldType = (typeof FIELD_TYPES)[number]; // In validators.ts - derive validator from constants import { FIELD_TYPES } from "./filterConstants"; export const fieldTypeValidator = v.union( ...FIELD_TYPES.map(t => v.literal(t)) );This is optional since the current approach works and is well-documented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/filterConstants.ts` around lines 1 - 98, The file duplicates the string-union types (FieldType, FilterOperator) which can drift from validators; replace them with exported const arrays (e.g., FIELD_TYPES and FILTER_OPERATORS declared as `as const`) and derive the types via `export type FieldType = (typeof FIELD_TYPES)[number]` and `export type FilterOperator = (typeof FILTER_OPERATORS)[number]`; update usages (OPERATOR_MAP, OPERATOR_LABELS, VALUELESS_OPERATORS) to remain compatible with the derived types and then update validators.ts to import FIELD_TYPES / FILTER_OPERATORS and build v.literal/v.union validators from those arrays so both modules share a single source of truth (refer to symbols FIELD_TYPES, FILTER_OPERATORS, FieldType, FilterOperator, OPERATOR_MAP, OPERATOR_LABELS, VALUELESS_OPERATORS).src/components/admin/shell/FilterBuilder.tsx (1)
366-375: Remove redundant cast onfilter._id.The
filter._idvalue is already typed asId<"viewFilters">from the Convex query result, so the explicit cast is unnecessary.Suggested change
<button aria-label="Remove filter" className="ml-1 rounded-sm p-0.5 hover:bg-muted" - onClick={() => - handleRemoveFilter(filter._id as Id<"viewFilters">) - } + onClick={() => handleRemoveFilter(filter._id)} type="button" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/shell/FilterBuilder.tsx` around lines 366 - 375, Remove the unnecessary type cast on filter._id in the FilterBuilder component: update the onClick handler that calls handleRemoveFilter(filter._id as Id<"viewFilters">) to pass filter._id directly (handleRemoveFilter(filter._id)), since filter._id is already typed as Id<"viewFilters"> from the Convex query result; this touches the onClick for the remove button that renders the <X /> icon and the handleRemoveFilter call.convex/crm/calendarQuery.ts (1)
446-455: Add clarifying comment about earliest date selection in calendar bucketing.The deduplication logic intentionally keeps one date value per record for calendar bucketing. The earliest date is selected because the index is sorted ascending by value. Consider adding a comment explaining this design choice, e.g.:
// Keep earliest date within range for deterministic bucketing; index is sorted ascending by value🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/calendarQuery.ts` around lines 446 - 455, Add a short clarifying comment above the deduplication loop explaining that recordIdToDate and uniqueRecordIds are deduplicating rows by recordId and intentionally keeping the earliest date (since the index/query results `capped` are sorted ascending by `value`) so that calendar bucketing is deterministic; place this comment near the loop that iterates `for (const row of capped)` referencing `recordIdToDate`, `uniqueRecordIds`, and `capped`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@convex/crm/filterOperatorValidation.ts`:
- Around line 12-14: The thrown Error in filterOperatorValidation.ts currently
tells devs to update OPERATOR_MAP in filterOperatorValidation.ts but
OPERATOR_MAP was moved to filterConstants.ts; update the error message in the
throw new Error(...) to reference OPERATOR_MAP in filterConstants.ts (preserve
existing message structure and interpolation of fieldType and only change the
filename text) so the guidance points to the correct symbol location.
---
Nitpick comments:
In `@convex/crm/calendarQuery.ts`:
- Around line 446-455: Add a short clarifying comment above the deduplication
loop explaining that recordIdToDate and uniqueRecordIds are deduplicating rows
by recordId and intentionally keeping the earliest date (since the index/query
results `capped` are sorted ascending by `value`) so that calendar bucketing is
deterministic; place this comment near the loop that iterates `for (const row of
capped)` referencing `recordIdToDate`, `uniqueRecordIds`, and `capped`.
In `@convex/crm/filterConstants.ts`:
- Around line 1-98: The file duplicates the string-union types (FieldType,
FilterOperator) which can drift from validators; replace them with exported
const arrays (e.g., FIELD_TYPES and FILTER_OPERATORS declared as `as const`) and
derive the types via `export type FieldType = (typeof FIELD_TYPES)[number]` and
`export type FilterOperator = (typeof FILTER_OPERATORS)[number]`; update usages
(OPERATOR_MAP, OPERATOR_LABELS, VALUELESS_OPERATORS) to remain compatible with
the derived types and then update validators.ts to import FIELD_TYPES /
FILTER_OPERATORS and build v.literal/v.union validators from those arrays so
both modules share a single source of truth (refer to symbols FIELD_TYPES,
FILTER_OPERATORS, FieldType, FilterOperator, OPERATOR_MAP, OPERATOR_LABELS,
VALUELESS_OPERATORS).
In `@src/components/admin/shell/FilterBuilder.tsx`:
- Around line 366-375: Remove the unnecessary type cast on filter._id in the
FilterBuilder component: update the onClick handler that calls
handleRemoveFilter(filter._id as Id<"viewFilters">) to pass filter._id directly
(handleRemoveFilter(filter._id)), since filter._id is already typed as
Id<"viewFilters"> from the Convex query result; this touches the onClick for the
remove button that renders the <X /> icon and the handleRemoveFilter call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 958deb9f-a1c1-4874-96ea-9dc171f78b5e
⛔ Files ignored due to path filters (1)
convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (16)
convex/crm/calendarQuery.tsconvex/crm/filterConstants.tsconvex/crm/filterOperatorValidation.tsconvex/crm/recordQueries.tsconvex/crm/types.tsconvex/crm/viewDefs.tsconvex/crm/viewFields.tsconvex/crm/viewFilters.tsconvex/crm/viewKanbanGroups.tsspecs/ENG-254/chunks/chunk-01-calendar-query/context.mdspecs/ENG-254/chunks/chunk-01-calendar-query/tasks.mdspecs/ENG-254/chunks/chunk-02-filter-builder/context.mdspecs/ENG-254/chunks/chunk-02-filter-builder/tasks.mdspecs/ENG-254/chunks/manifest.mdspecs/ENG-254/tasks.mdsrc/components/admin/shell/FilterBuilder.tsx
Connorbelez
force-pushed
the
Connorbelez/eng-254-view-definitions
branch
from
f12eb20 to
f9bb59a
Compare
Connorbelez
force-pushed
the
Connorbelez/eng-254-view-definitions
branch
from
f9bb59a to
74c9e9e
Compare
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
eng-252 eng-254 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Calendar query supporting date-based record grouping by day, week, or month with filter application * Filter builder component enabling creation and management of typed filters for calendar views * **Refactor** * Extracted shared filter validation constants for consistent operator handling across the application <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
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.
eng-252
eng-254
Summary by CodeRabbit
New Features
Improvements