eng-274 feat(crm): add personal saved views#383
Merged
Connorbelez merged 3 commits intomainfrom Apr 9, 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
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Owner
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Apr 3, 2026
Merged
![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.
Connorbelez
changed the title
Connorbelez
changed the title
![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/recordQueries.ts (1)
265-285:⚠️ Potential issue | 🟡 Minor
logicalOperatorfield declared inRecordFilterbut not used.The
applyFiltersfunction uses.every()which applies AND logic to all filters, ignoring thelogicalOperatorfield. Per the type definition intypes.ts,RecordFilterincludeslogicalOperator?: "and" | "or", but it's never consulted here.Note that
calendarQuery.tsdoes uselogicalOperatorin itsapplyViewFiltersfunction, so there's an inconsistency between the two filter application paths.🔧 Suggested fix to support OR logic
export function applyFilters( records: UnifiedRecord[], filters: RecordFilter[], fieldDefsById: Map<string, FieldDef> ): UnifiedRecord[] { // No filters = no constraints; return all records (permissive empty case) if (filters.length === 0) { return records; } - return records.filter((record) => - filters.every((filter) => { + return records.filter((record) => { + let combined: boolean | undefined; + for (const filter of filters) { const fieldDef = fieldDefsById.get(filter.fieldDefId.toString()); if (!fieldDef) { - return false; // fail-closed: unknown fieldDefId never matches + return false; } const fieldValue = record.fields[fieldDef.name]; - return matchesFilter(fieldValue, filter.operator, filter.value); - }) - ); + const nextMatch = matchesFilter(fieldValue, filter.operator, filter.value); + if (combined === undefined) { + combined = nextMatch; + continue; + } + combined = filter.logicalOperator === "or" + ? combined || nextMatch + : combined && nextMatch; + } + return combined ?? true; + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/recordQueries.ts` around lines 265 - 285, applyFilters currently uses filters.every(...) which enforces AND for all filters and ignores RecordFilter.logicalOperator; update applyFilters to evaluate each filter using matchesFilter but combine their boolean results according to each filter.logicalOperator (treat undefined as "and" or use a default behavior matching calendarQuery.applyViewFilters), e.g., iterate filters for a record and accumulate a running result switching between AND/OR when filter.logicalOperator is "and"/"or"; ensure you still fetch fieldDef via fieldDefsById.get(filter.fieldDefId.toString()) and treat a missing fieldDef as a false match for that individual filter (but do not short-circuit incorrectly when combining with OR).
🧹 Nitpick comments (6)
convex/crm/types.ts (1)
69-74:SavedViewFilterDefinition.valuetype mismatch withRecordFilter.value.
SavedViewFilterDefinition.valueis typed asv.optional(v.string()), butRecordFilter.valueisunknown. This creates a type mismatch when converting saved view filters to record filters. Operators likebetweenexpectvalueto benumber[], andis_any_ofexpectsstring[].Consider either:
- Using a union type for
valueinSavedViewFilterDefinitionto match supported filter value shapes, or- Documenting that string serialization/deserialization is expected during conversion
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/types.ts` around lines 69 - 74, SavedViewFilterDefinition.value currently typed as optional string conflicts with RecordFilter.value (unknown) and operators that expect arrays or numbers; update the SavedViewFilterDefinition interface so value is a union covering the supported shapes (e.g., value?: string | string[] | number | number[] | boolean | null) to match operator expectations (and adjust any v.* validators accordingly), and ensure conversion code that maps SavedViewFilterDefinition -> RecordFilter handles deserialization/typing for operators like "between" and "is_any_of" based on SavedViewFilterDefinition.operator.convex/crm/calendarQuery.ts (1)
99-158: Consider consolidating filter matching logic withrecordQueries.ts.
matchesViewFilterduplicates much of the logic frommatchesFilterinrecordQueries.ts, with some differences (array handling forequals/is/is_not). This creates maintenance burden and potential for drift.Consider either:
- Extracting shared logic to a common helper module
- Enhancing
matchesFilterto handle the array cases and reusing it here🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/calendarQuery.ts` around lines 99 - 158, matchesViewFilter duplicates most of the logic in matchesFilter (recordQueries.ts) and only differs in special-casing arrays for "equals"/"is"/"is_not", which risks drift; refactor by either extracting the shared predicate logic into a common helper (e.g., create a new normalizeAndMatch/compareValue helper used by both matchesFilter and matchesViewFilter) or by enhancing matchesFilter to accept an options flag to handle array semantics and then replace the duplicated branches in matchesViewFilter to call matchesFilter with that flag; update the implementations of matchesViewFilter and any call sites to use the shared helper or updated matchesFilter so array handling is centralized and tests updated accordingly.convex/crm/validators.ts (1)
164-169: Validatorvaluetype may be too restrictive for some operators.The
valuefield isv.optional(v.string()), but operators likebetween(expects 2-element numeric array) andis_any_of(expects string array) require non-string values. This means these operators cannot be used with saved view filters as currently defined.If this is intentional (e.g., stored as JSON strings and parsed at runtime), consider adding a comment documenting this behavior. Otherwise, consider using a union type:
♻️ Suggested fix
export const savedViewFilterValidator = v.object({ fieldDefId: v.id("fieldDefs"), operator: filterOperatorValidator, - value: v.optional(v.string()), + value: v.optional( + v.union( + v.string(), + v.number(), + v.array(v.union(v.string(), v.number())) + ) + ), logicalOperator: v.optional(logicalOperatorValidator), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/validators.ts` around lines 164 - 169, The savedViewFilterValidator's value is currently v.optional(v.string()) which is too restrictive for operators like "between" (expects a two-element numeric array) and "is_any_of" (expects a string array); update savedViewFilterValidator (the value field) to accept a union of the expected types (e.g., optional string, optional array of strings, optional array of numbers, and optional single number where applicable) or a more permissive typed union that covers "between" and "is_any_of" cases, and add a short comment in the savedViewFilterValidator explaining the accepted shapes (including the two-element numeric array requirement for "between") so callers know the expected formats.convex/crm/userSavedViews.ts (1)
238-339: Avoid type assertion; use returned value from validation.
requireSourceViewreturns the validatedsourceView, but line 267 uses a type assertion on the input. Using the returned value is cleaner.♻️ Proposed fix
- await requireSourceView(ctx, { + const sourceView = await requireSourceView(ctx, { objectDefId: args.objectDefId, orgId, sourceViewDefId: args.sourceViewDefId, viewType: args.viewType, }); const snapshot = await loadBaseViewState( ctx, - args.sourceViewDefId as Id<"viewDefs">, + sourceView._id, orgId );And update the
insertcall to usesourceView._id:const userSavedViewId = await ctx.db.insert("userSavedViews", { orgId, objectDefId: args.objectDefId, ownerAuthId: ctx.viewer.authId, - sourceViewDefId: args.sourceViewDefId, + sourceViewDefId: sourceView._id,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/userSavedViews.ts` around lines 238 - 339, The handler in createUserSavedView currently asserts args.sourceViewDefId instead of using the validated return from requireSourceView; change it to capture the return value from requireSourceView (e.g., const sourceView = await requireSourceView(ctx, {...})) and pass sourceView._id into loadBaseViewState and into the ctx.db.insert payload (replace uses of args.sourceViewDefId with sourceView._id), ensuring all subsequent logic uses the validated sourceView instead of the raw arg.convex/crm/viewState.ts (1)
32-42: Silent JSON parse fallback may hide malformed data.When
JSON.parsefails, the function returns the raw string. This is acceptable for forward-compatibility, but malformed JSON in stored filters could silently produce incorrect filter behavior at runtime.Consider adding observability (logging or metrics) when parsing fails to detect data corruption:
function parseStoredFilterValue(value: string | undefined): unknown { if (value === undefined) { return undefined; } try { return JSON.parse(value); } catch { + // Consider logging this case for observability return value; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/viewState.ts` around lines 32 - 42, The parseStoredFilterValue function currently swallows JSON.parse errors and returns the raw string, which can hide malformed stored filter data; update the catch block in parseStoredFilterValue to record observability (e.g., call an existing logger or telemetry/metrics function) including the error message and the original value (or a truncated/hashed version for privacy) so malformed JSON is detectable, and then continue returning the fallback value as before; ensure you reference parseStoredFilterValue in your changes and use the project's standard logging/telemetry utilities rather than adding ad-hoc console calls.convex/crm/viewQueries.ts (1)
260-296: Redundant sort operation.The
displayOrderis set to the index during iteration overfieldOrder, so the final sort (line 295) will always produce the same order as the iteration. Consider removing the sort.♻️ Proposed fix to remove redundant sort
return args.effectiveView.fieldOrder .flatMap((fieldId, index) => { const fieldDef = args.fieldDefsById.get(fieldId.toString()); if (!fieldDef) { return []; } const sourceViewField = viewFieldByFieldId.get(fieldId.toString()); return [ { fieldDefId: fieldId, name: fieldDef.name, label: fieldDef.label, fieldType: fieldDef.fieldType, width: sourceViewField?.width, isVisible: visibleFieldIds.has(fieldId.toString()), displayOrder: index, }, ]; - }) - .sort((left, right) => left.displayOrder - right.displayOrder); + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/crm/viewQueries.ts` around lines 260 - 296, The final .sort in buildEffectiveColumns is redundant because displayOrder is assigned from the iteration index of effectiveView.fieldOrder, so the array already reflects the correct order; remove the .sort((left, right) => left.displayOrder - right.displayOrder) and return the flatMap result directly (keeping the existing logic that builds ColumnDef items using fieldOrder, displayOrder, viewFieldByFieldId, visibleFieldIds and fieldDefsById).
🤖 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/recordQueries.ts`:
- Around line 265-285: applyFilters currently uses filters.every(...) which
enforces AND for all filters and ignores RecordFilter.logicalOperator; update
applyFilters to evaluate each filter using matchesFilter but combine their
boolean results according to each filter.logicalOperator (treat undefined as
"and" or use a default behavior matching calendarQuery.applyViewFilters), e.g.,
iterate filters for a record and accumulate a running result switching between
AND/OR when filter.logicalOperator is "and"/"or"; ensure you still fetch
fieldDef via fieldDefsById.get(filter.fieldDefId.toString()) and treat a missing
fieldDef as a false match for that individual filter (but do not short-circuit
incorrectly when combining with OR).
---
Nitpick comments:
In `@convex/crm/calendarQuery.ts`:
- Around line 99-158: matchesViewFilter duplicates most of the logic in
matchesFilter (recordQueries.ts) and only differs in special-casing arrays for
"equals"/"is"/"is_not", which risks drift; refactor by either extracting the
shared predicate logic into a common helper (e.g., create a new
normalizeAndMatch/compareValue helper used by both matchesFilter and
matchesViewFilter) or by enhancing matchesFilter to accept an options flag to
handle array semantics and then replace the duplicated branches in
matchesViewFilter to call matchesFilter with that flag; update the
implementations of matchesViewFilter and any call sites to use the shared helper
or updated matchesFilter so array handling is centralized and tests updated
accordingly.
In `@convex/crm/types.ts`:
- Around line 69-74: SavedViewFilterDefinition.value currently typed as optional
string conflicts with RecordFilter.value (unknown) and operators that expect
arrays or numbers; update the SavedViewFilterDefinition interface so value is a
union covering the supported shapes (e.g., value?: string | string[] | number |
number[] | boolean | null) to match operator expectations (and adjust any v.*
validators accordingly), and ensure conversion code that maps
SavedViewFilterDefinition -> RecordFilter handles deserialization/typing for
operators like "between" and "is_any_of" based on
SavedViewFilterDefinition.operator.
In `@convex/crm/userSavedViews.ts`:
- Around line 238-339: The handler in createUserSavedView currently asserts
args.sourceViewDefId instead of using the validated return from
requireSourceView; change it to capture the return value from requireSourceView
(e.g., const sourceView = await requireSourceView(ctx, {...})) and pass
sourceView._id into loadBaseViewState and into the ctx.db.insert payload
(replace uses of args.sourceViewDefId with sourceView._id), ensuring all
subsequent logic uses the validated sourceView instead of the raw arg.
In `@convex/crm/validators.ts`:
- Around line 164-169: The savedViewFilterValidator's value is currently
v.optional(v.string()) which is too restrictive for operators like "between"
(expects a two-element numeric array) and "is_any_of" (expects a string array);
update savedViewFilterValidator (the value field) to accept a union of the
expected types (e.g., optional string, optional array of strings, optional array
of numbers, and optional single number where applicable) or a more permissive
typed union that covers "between" and "is_any_of" cases, and add a short comment
in the savedViewFilterValidator explaining the accepted shapes (including the
two-element numeric array requirement for "between") so callers know the
expected formats.
In `@convex/crm/viewQueries.ts`:
- Around line 260-296: The final .sort in buildEffectiveColumns is redundant
because displayOrder is assigned from the iteration index of
effectiveView.fieldOrder, so the array already reflects the correct order;
remove the .sort((left, right) => left.displayOrder - right.displayOrder) and
return the flatMap result directly (keeping the existing logic that builds
ColumnDef items using fieldOrder, displayOrder, viewFieldByFieldId,
visibleFieldIds and fieldDefsById).
In `@convex/crm/viewState.ts`:
- Around line 32-42: The parseStoredFilterValue function currently swallows
JSON.parse errors and returns the raw string, which can hide malformed stored
filter data; update the catch block in parseStoredFilterValue to record
observability (e.g., call an existing logger or telemetry/metrics function)
including the error message and the original value (or a truncated/hashed
version for privacy) so malformed JSON is detectable, and then continue
returning the fallback value as before; ensure you reference
parseStoredFilterValue in your changes and use the project's standard
logging/telemetry utilities rather than adding ad-hoc console calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f4fca17-3a61-4df7-843e-7a478a361ef1
📒 Files selected for processing (11)
convex/crm/__tests__/userSavedViews.test.tsconvex/crm/calendarQuery.tsconvex/crm/recordQueries.tsconvex/crm/types.tsconvex/crm/userSavedViews.tsconvex/crm/validators.tsconvex/crm/viewQueries.tsconvex/crm/viewState.tsconvex/schema.tsconvex/test/moduleMaps.tsconvex/test/registerAuditLogComponent.ts
There was a problem hiding this comment.
Pull request overview
This PR adds support for personal (per-user) saved views in the CRM by introducing a userSavedViews data model and resolving an “effective view” that overlays a system view with a user’s saved configuration (columns/filters/defaults).
Changes:
- Add
crm/userSavedViewsqueries/mutations plus shared view-resolution utilities (resolveEffectiveViewState). - Update view record/schema queries (table/kanban/calendar) to accept
userSavedViewIdand/or apply the user’s default saved view. - Update schema/validators/types for structured saved-view filters and add coverage tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| convex/test/registerAuditLogComponent.ts | Reworks component module loading for audit log tests; registers aggregate components used by audit log. |
| convex/test/moduleMaps.ts | Adds module-map entries for new CRM modules used in tests. |
| convex/schema.ts | Updates userSavedViews schema (structured filters) and adds an index for default lookup. |
| convex/crm/viewState.ts | New shared logic for resolving system vs saved vs effective view state. |
| convex/crm/viewQueries.ts | Applies effective view state to table/kanban queries and expands getViewSchema output. |
| convex/crm/validators.ts | Adds savedViewFilterValidator. |
| convex/crm/userSavedViews.ts | New CRUD + default-selection endpoints for personal saved views with validation + audit logging. |
| convex/crm/types.ts | Introduces SavedViewFilterDefinition + EffectiveViewDefinition; expands filter operator unions. |
| convex/crm/recordQueries.ts | Extends filter operator matching behavior (e.g., between/is_not/after/before). |
| convex/crm/calendarQuery.ts | Switches calendar filtering to operate on resolved effective view filters. |
| convex/crm/tests/userSavedViews.test.ts | Adds test coverage for default scoping, overlay application, and explicit saved-view selection across layouts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Connorbelez
force-pushed
the
eng-273
branch
2 times, most recently
from
a870ae5 to
59a11dc
Compare
Connorbelez
force-pushed
the
graphite-base/383
branch
from
59a11dc to
686653c
Compare
Owner
Author
Merge activity
|
Merged
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
feat(crm): add personal saved views responding to feedback <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Users can now create, manage, and set default personal saved views for CRM objects. * Saved views support custom filters, field visibility, ordering, grouping, and aggregates with proper permission isolation. * Expanded filter operators including date comparisons (before, after), range filtering (between), and equality operators. * Calendar and kanban views now integrate with personal saved views. * **Tests** * Added comprehensive test coverage for user saved views creation, defaults, and permission scoping. <!-- 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.
feat(crm): add personal saved views
responding to feedback
Summary by CodeRabbit
New Features
Tests