Skip to content

eng-257#357

Merged
Connorbelez merged 8 commits intomainfrom
ENG-257
Mar 31, 2026
Merged

eng-257#357
Connorbelez merged 8 commits intomainfrom
ENG-257

Conversation

@Connorbelez
Copy link
Copy Markdown
Owner

@Connorbelez Connorbelez commented Mar 31, 2026

eng-257

eng-257

Summary by Sourcery

Introduce polymorphic CRM record linking with admin-managed link types and bidirectional queries, and harden date filter handling to be timezone-safe.

New Features:

  • Add admin CRUD operations for link type definitions, including creation, deactivation, and listing of link types scoped to an org.
  • Implement record linking mutations that create and soft-delete polymorphic links between CRM records and native entities with cardinality and org-ownership enforcement.
  • Expose queries to fetch linked records bidirectionally, grouped by link type, and to list applicable link types for a given object definition.

Bug Fixes:

  • Change date filter encoding to use explicit UTC parsing, avoiding cross-browser timezone inconsistencies when converting date strings to timestamps.

Documentation:

  • Add ENG-257 specification chunks covering link type CRUD, record linking mutations, and bidirectional link queries, plus a master task list and chunk manifest.
  • Update ENG-254 calendar query spec to document result truncation behavior and extend the CalendarData contract with skippedFilters and truncated flags.

Summary by CodeRabbit

Release Notes

  • New Features

    • Link type management: create, deactivate, and list link types for your organization
    • Record linking: establish and remove relationships between records
    • Bidirectional record queries: view all linked records and available link types for objects
    • Enhanced date validation in filter inputs with stricter parsing and error handling
  • Tests

    • Added comprehensive test coverage for link type creation, deactivation, and record linking operations with cardinality enforcement and organization isolation validation

@linear
Copy link
Copy Markdown

linear bot commented Mar 31, 2026

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Adds comprehensive link type and record linking infrastructure to the CRM system, including mutations for creating/deactivating link types and linking records with cardinality enforcement, queries for retrieving linked records grouped by link type, extensive validation for organization isolation and active-link constraints, and full test coverage for all CRUD operations.

Changes

Cohort / File(s) Summary
Link Type Management
convex/crm/linkTypes.ts, convex/crm/__tests__/linkTypes.test.ts
Introduces admin mutations createLinkType and deactivateLinkType, plus query listLinkTypes for link type CRUD. Creates fixture helpers and 7 test cases validating creation/listing, inactive object rejection, cross-org isolation, and deactivation constraints with active link checks.
Record Linking Mutations
convex/crm/recordLinks.ts, convex/crm/__tests__/recordLinks.test.ts
Implements createLink and deleteLink mutations with cardinality enforcement (one\_to\_one, one\_to\_many, many\_to\_many), soft-delete semantics, duplicate detection, and org isolation. Includes 592 lines of tests covering all cardinality rules, reverse-direction detection, and cross-org rejection.
Link Query APIs
convex/crm/linkQueries.ts
Adds getLinkedRecords (filtered by direction, grouped by link type) and getLinkTypesForObject queries supporting bidirectional link traversal and native entity resolution.
Schema & Type Updates
convex/schema.ts, convex/crm/validators.ts, convex/crm/types.ts, convex/crm/recordQueries.ts
Introduces entityKindValidator (record|native), adds org\_scoped indexes to linkTypeDefs, updates LinkedRecord.linkId type from string to Id<"recordLinks">, and fixes linkId resolution in getRecord.
Specifications & Documentation
specs/ENG-257/.../*, specs/ENG-254/.../context.md
Documents Link Type CRUD (chunk-01), Record Linking (chunk-02), and Bidirectional Queries (chunk-03) with context, task lists, and manifest. Minor calendar query truncation spec update.
Admin UI Enhancement
src/components/admin/shell/FilterBuilder.tsx
Adds strict parseDateToUtcMs validator for date-valued filter inputs with regex format enforcement and round-trip UTC validation.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Backend as Convex Backend
    participant DB as Database
    participant Audit as Audit Log

    Client->>Backend: createLinkType(name, sourceObjDefId, targetObjDefId, cardinality)
    Backend->>Backend: Validate ctx.viewer.orgId present
    Backend->>DB: Load sourceObjectDef
    Backend->>DB: Load targetObjectDef
    Backend->>Backend: Verify both exist, active, belong to org
    Backend->>DB: Insert linkTypeDefs record (isActive: true)
    Backend->>Audit: Log crm.linkType.created
    Backend->>Client: Return linkTypeDefId

    Client->>Backend: deactivateLinkType(linkTypeDefId)
    Backend->>DB: Load linkTypeDef, verify org ownership
    Backend->>DB: Query recordLinks by linkTypeDefId (isDeleted: false)
    Backend->>Backend: Throw if active links exist
    DB->>DB: (No active links found)
    Backend->>DB: Patch linkTypeDef (isActive: false)
    Backend->>Audit: Log crm.linkType.deactivated
    Backend->>Client: Return success
Loading
sequenceDiagram
    participant Client
    participant Backend as Convex Backend
    participant DB as Database
    participant Audit as Audit Log

    Client->>Backend: createLink(linkTypeDefId, sourceKind, sourceId, targetKind, targetId)
    Backend->>Backend: Validate ctx.viewer.orgId present
    Backend->>DB: Load linkTypeDef, verify active & org ownership
    Backend->>DB: Validate source entity exists & org-owned
    Backend->>DB: Validate target entity exists & org-owned
    Backend->>DB: Verify object types match linkTypeDef expectations
    Backend->>Backend: Check cross-org pairing (reject if mismatch)
    Backend->>DB: Query existing active links (by_org_source index)
    Backend->>Backend: Reject if duplicate in forward or reverse direction
    Backend->>Backend: Enforce cardinality constraints (one_to_one/one_to_many/many_to_many)
    DB->>DB: (Cardinality checks pass)
    Backend->>DB: Insert recordLinks record (isDeleted: false)
    Backend->>Audit: Log crm.link.created
    Backend->>Client: Return linkId

    Client->>Backend: deleteLink(linkId)
    Backend->>DB: Load recordLinks, verify org ownership & not already deleted
    Backend->>DB: Patch isDeleted to true
    Backend->>Audit: Log crm.link.deleted
    Backend->>Client: Return success
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Possibly related PRs

  • eng-249 and eng-250 #350: Both modify the recordLinks table schema and indexes in convex/schema.ts to support org-scoped link queries and cardinality validation.
  • eng-251 #351: Both change LinkedRecord.linkId type from string to Id<"recordLinks"> and adjust the link resolution pattern in recordQueries.ts.
  • eng-255 #353: Both modify convex/crm/recordQueries.ts to update link resolution behavior and the LinkedRecord interface type alignment.

Poem

🐰 Hops excitedly

Link types dance in organized rows,
Records now intertwine with care,
Cardinality guards what flows,
Orgs stay isolated in their lair!
From source to target, the chain now glows! 🔗

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'eng-257' is a ticket identifier that references the feature, but lacks descriptive detail about the actual implementation changes. Use a more descriptive title like 'Add polymorphic CRM record linking and link type CRUD' or 'Implement ENG-257: Link types, record linking, and bidirectional queries'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ENG-257

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Owner Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented Mar 31, 2026

Reviewer's Guide

Implements polymorphic CRM record linking, including admin CRUD for link types, data-plane link create/delete mutations, bidirectional link queries grouped by link type, and a small date-handling bugfix plus spec updates for ENG-254 and ENG-257.

Sequence diagram for createLink CRM mutation

sequenceDiagram
  actor User
  participant Client
  participant createLink_handler
  participant MutationCtx
  participant ConvexDb as ConvexDb_recordLinks
  participant auditLog_module

  User->>Client: Request createLink(linkTypeDefId, sourceKind, sourceId, targetKind, targetId)
  Client->>createLink_handler: crmMutation.createLink(args)

  createLink_handler->>MutationCtx: Read viewer.orgId
  alt orgId is missing
    createLink_handler-->>Client: ConvexError Org context required
  else orgId present
    createLink_handler->>ConvexDb: get(linkTypeDefId)
    ConvexDb-->>createLink_handler: linkTypeDef
    createLink_handler->>ConvexDb: get(sourceObjectDefId)
    ConvexDb-->>createLink_handler: sourceObjectDef
    createLink_handler->>ConvexDb: get(targetObjectDefId)
    ConvexDb-->>createLink_handler: targetObjectDef

    createLink_handler->>createLink_handler: validateEntityExists(source)
    createLink_handler->>createLink_handler: validateEntityExists(target)

    createLink_handler->>ConvexDb: query(recordLinks by_org_source)
    ConvexDb-->>createLink_handler: existingSourceLinks
    createLink_handler->>createLink_handler: check duplicate and cardinality

    createLink_handler->>ConvexDb: insert(recordLinks, newLink)
    ConvexDb-->>createLink_handler: linkId

    createLink_handler->>auditLog_module: log crm.link.created
    auditLog_module-->>createLink_handler: ack

    createLink_handler-->>Client: linkId
  end

  Client-->>User: Link created
Loading

Sequence diagram for getLinkedRecords CRM query

sequenceDiagram
  actor User
  participant Client
  participant getLinkedRecords_handler
  participant QueryCtx
  participant ConvexDb as ConvexDb_recordLinks

  User->>Client: View linked records for entity
  Client->>getLinkedRecords_handler: crmQuery.getLinkedRecords(recordId, recordKind, direction)

  getLinkedRecords_handler->>QueryCtx: Read viewer.orgId
  alt orgId is missing
    getLinkedRecords_handler-->>Client: ConvexError Org context required
  else orgId present
    getLinkedRecords_handler->>getLinkedRecords_handler: Resolve direction flags

    opt outbound requested
      getLinkedRecords_handler->>ConvexDb: query(recordLinks by_org_source)
      ConvexDb-->>getLinkedRecords_handler: outboundLinks
    end

    opt inbound requested
      getLinkedRecords_handler->>ConvexDb: query(recordLinks by_org_target)
      ConvexDb-->>getLinkedRecords_handler: inboundLinks
    end

    getLinkedRecords_handler->>getLinkedRecords_handler: Group links by linkTypeDefId

    getLinkedRecords_handler->>ConvexDb: get(linkTypeDefIds)
    ConvexDb-->>getLinkedRecords_handler: linkTypeDefs

    loop each outbound group
      getLinkedRecords_handler->>ConvexDb: get(peer records for outbound)
      ConvexDb-->>getLinkedRecords_handler: records with labelValue
      getLinkedRecords_handler->>getLinkedRecords_handler: build LinkedRecord for outbound
    end

    loop each inbound group
      getLinkedRecords_handler->>ConvexDb: get(peer records for inbound)
      ConvexDb-->>getLinkedRecords_handler: records with labelValue
      getLinkedRecords_handler->>getLinkedRecords_handler: build LinkedRecord for inbound
    end

    getLinkedRecords_handler-->>Client: LinkGroup[]
  end

  Client-->>User: Render grouped linked records
Loading

ER diagram for CRM record link data model

erDiagram
  objectDefs {
    string _id
    string orgId
    string name
    string nativeTable
    boolean isActive
  }

  linkTypeDefs {
    string _id
    string orgId
    string name
    string sourceObjectDefId
    string targetObjectDefId
    string cardinality
    boolean isActive
    number createdAt
  }

  recordLinks {
    string _id
    string orgId
    string linkTypeDefId
    string sourceObjectDefId
    string sourceKind
    string sourceId
    string targetObjectDefId
    string targetKind
    string targetId
    boolean isDeleted
    number createdAt
    string createdBy
  }

  records {
    string _id
    string orgId
    string objectDefId
    boolean isDeleted
    string labelValue
  }

  mortgages {
    string _id
    string orgId
  }

  borrowers {
    string _id
    string orgId
  }

  lenders {
    string _id
    string orgId
  }

  brokers {
    string _id
    string orgId
  }

  deals {
    string _id
    string orgId
  }

  obligations {
    string _id
    string orgId
  }

  objectDefs ||--o{ linkTypeDefs : sourceObject
  objectDefs ||--o{ linkTypeDefs : targetObject

  linkTypeDefs ||--o{ recordLinks : typedBy

  objectDefs ||--o{ records : defines

  records ||--o{ recordLinks : canBeSource
  records ||--o{ recordLinks : canBeTarget

  objectDefs ||--o{ mortgages : native
  objectDefs ||--o{ borrowers : native
  objectDefs ||--o{ lenders : native
  objectDefs ||--o{ brokers : native
  objectDefs ||--o{ deals : native
  objectDefs ||--o{ obligations : native

  mortgages ||--o{ recordLinks : canBeLinked
  borrowers ||--o{ recordLinks : canBeLinked
  lenders ||--o{ recordLinks : canBeLinked
  brokers ||--o{ recordLinks : canBeLinked
  deals ||--o{ recordLinks : canBeLinked
  obligations ||--o{ recordLinks : canBeLinked
Loading

Class diagram for CRM record linking modules

classDiagram
  class recordLinks_module {
    +getNativeEntity(ctx, tableName, entityId) Promise~object~
    +validateEntityExists(ctx, kind, entityId, orgId, objectDef) Promise~void~
    +createLink(ctx, args) Promise~Id_recordLinks~
    +deleteLink(ctx, args) Promise~void~
  }

  class linkQueries_module {
    +resolveLinkedRecords(ctx, links, direction) Promise~LinkedRecord[]~
    +getLinkedRecords(ctx, args) Promise~LinkGroup[]~
    +getLinkTypesForObject(ctx, args) Promise~Doc_linkTypeDefs[]~
  }

  class linkTypes_module {
    +createLinkType(ctx, args) Promise~Id_linkTypeDefs~
    +deactivateLinkType(ctx, args) Promise~void~
    +listLinkTypes(ctx) Promise~Doc_linkTypeDefs[]~
  }

  class LinkedRecord {
    +string linkId
    +Id_linkTypeDefs linkTypeDefId
    +string recordId
    +string recordKind
    +Id_objectDefs objectDefId
    +string labelValue
  }

  class LinkGroup {
    +string direction
    +LinkedRecord[] links
    +Id_linkTypeDefs linkTypeDefId
    +string linkTypeName
  }

  class auditLog_module {
    +log(ctx, entry) Promise~void~
  }

  class crmMutation {
    +input(schema)
    +handler(fn)
    +public()
  }

  class crmAdminMutation {
    +input(schema)
    +handler(fn)
    +public()
  }

  class crmQuery {
    +input(schema)
    +handler(fn)
    +public()
  }

  class crmAdminQuery {
    +input(schema)
    +handler(fn)
    +public()
  }

  class cardinalityValidator {
  }

  class ConvexDb {
    +get(id)
    +insert(table, doc)
    +patch(id, fields)
    +query(table)
  }

  class MutationCtx {
    +ConvexDb db
    +Viewer viewer
  }

  class QueryCtx {
    +ConvexDb db
    +Viewer viewer
  }

  class Viewer {
    +string orgId
    +string authId
  }

  recordLinks_module --> MutationCtx
  recordLinks_module --> auditLog_module
  recordLinks_module --> crmMutation

  linkQueries_module --> QueryCtx
  linkQueries_module --> LinkedRecord
  linkQueries_module --> LinkGroup
  linkQueries_module --> crmQuery

  linkTypes_module --> MutationCtx
  linkTypes_module --> auditLog_module
  linkTypes_module --> crmAdminMutation
  linkTypes_module --> crmAdminQuery
  linkTypes_module --> cardinalityValidator

  MutationCtx --> ConvexDb
  QueryCtx --> ConvexDb
  MutationCtx --> Viewer
  QueryCtx --> Viewer
Loading

File-Level Changes

Change Details Files
Fix date filter encoding to use UTC-safe parsing for cross-browser consistency.
  • Replace new Date(filterValue).getTime() with manual parsing of yyyy-mm-dd into components.
  • Use Date.UTC(year, month - 1, day) to compute Unix ms timestamp.
  • Keep existing validation and error toast when parsing fails.
src/components/admin/shell/FilterBuilder.tsx
Document calendar query truncation behavior and enriched response metadata for ENG-254.
  • Add step describing truncation at FILTERED_QUERY_CAP + 1 results and truncated flag behavior.
  • Extend CalendarData type with skippedFilters and truncated fields to capture filter-application outcomes.
specs/ENG-254/chunks/chunk-01-calendar-query/context.md
Add admin CRUD endpoints for link type definitions with audit logging and org scoping.
  • Introduce createLinkType admin mutation enforcing org ownership, active objectDefs, cardinality validation, and audit logging.
  • Add deactivateLinkType admin mutation that soft-deactivates link types only when no active links exist, with audit logging.
  • Provide listLinkTypes admin query returning active link types per org via by_org index.
convex/crm/linkTypes.ts
specs/ENG-257/chunks/chunk-01-link-type-crud/context.md
specs/ENG-257/chunks/chunk-01-link-type-crud/tasks.md
Implement polymorphic record linking mutations with validation, cardinality checks, and auditing.
  • Create getNativeEntity helper using a Convex table-name switch for native entities.
  • Create validateEntityExists helper to enforce existence, org ownership, and objectDef matching for both records and native entities.
  • Add createLink mutation that validates org context, linkTypeDef, endpoints, duplicate links, and cardinality (one_to_one, one_to_many), then inserts recordLinks row and writes audit log.
  • Add deleteLink mutation that soft-deletes a link with org checks and audit logging.
  • Document recordLinks/linkTypeDefs schema, validation rules, cardinality semantics, and quality gates in ENG-257 specs.
convex/crm/recordLinks.ts
specs/ENG-257/chunks/chunk-02-record-linking/context.md
specs/ENG-257/chunks/chunk-02-record-linking/tasks.md
Expose bidirectional link queries grouped by link type for both record and native entities.
  • Introduce getLinkedRecords crmQuery that fetches outbound/inbound links via org-scoped indexes, filters soft-deleted links, groups by linkTypeDefId, resolves peer records into LinkedRecord shapes (including labelValue), and annotates groups with linkTypeName and direction.
  • Introduce getLinkTypesForObject crmQuery that returns all active linkTypeDefs where an objectDef participates as source or target, deduplicated by id and org-scoped.
  • Add ENG-257 spec context describing schemas, LinkedRecord contract, grouping semantics vs existing getRecord behavior, and required quality gates.
convex/crm/linkQueries.ts
specs/ENG-257/chunks/chunk-03-bidirectional-queries/context.md
specs/ENG-257/chunks/chunk-03-bidirectional-queries/tasks.md
Add ENG-257 master task list and chunk manifest documentation.
  • Create ENG-257 master tasks file listing chunks, tasks, and completion status.
  • Add chunk manifest mapping chunk IDs to labels, tasks, status, and dependencies.
specs/ENG-257/tasks.md
specs/ENG-257/chunks/manifest.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@Connorbelez Connorbelez marked this pull request as ready for review March 31, 2026 03:42
Copilot AI review requested due to automatic review settings March 31, 2026 03:42
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements ENG-257 by introducing polymorphic CRM record linking (admin-managed link types, data-plane link create/delete, and bidirectional link queries) and updates date filter encoding to be UTC-safe, alongside supporting schema/type changes and specs/tests.

Changes:

  • Add Convex CRUD for linkTypeDefs (create, deactivate, list) and record linking mutations (create + soft-delete) with audit logging.
  • Add bidirectional link queries (getLinkedRecords, getLinkTypesForObject) and supporting schema/index + type updates.
  • Harden admin FilterBuilder date encoding to parse yyyy-mm-dd as UTC, and add ENG-257/ENG-254 spec documentation + new tests.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/components/admin/shell/FilterBuilder.tsx Adds explicit UTC parsing for date filter encoding to avoid timezone inconsistencies.
specs/ENG-257/tasks.md Adds master task list for ENG-257 implementation chunks.
specs/ENG-257/chunks/manifest.md Adds chunk manifest for ENG-257 spec breakdown.
specs/ENG-257/chunks/chunk-03-bidirectional-queries/tasks.md Documents requirements for bidirectional link queries and verification steps.
specs/ENG-257/chunks/chunk-03-bidirectional-queries/context.md Provides schema/type/context references for implementing bidirectional queries.
specs/ENG-257/chunks/chunk-02-record-linking/tasks.md Documents requirements for record linking mutations and validation rules.
specs/ENG-257/chunks/chunk-02-record-linking/context.md Provides schema + validation/audit patterns for linking mutations.
specs/ENG-257/chunks/chunk-01-link-type-crud/tasks.md Documents requirements for admin link type CRUD.
specs/ENG-257/chunks/chunk-01-link-type-crud/context.md Provides schema + middleware/audit patterns for link type CRUD.
specs/ENG-254/chunks/chunk-01-calendar-query/context.md Updates calendar query spec with truncation behavior and contract fields.
convex/schema.ts Adds org-scoped indexes for link types; standardizes recordLink kind validation via shared validator.
convex/crm/validators.ts Introduces entityKindValidator for consistent `"record"
convex/crm/types.ts Tightens LinkedRecord.linkId type to Id<"recordLinks">.
convex/crm/recordQueries.ts Aligns getRecord link output with updated LinkedRecord.linkId type.
convex/crm/recordLinks.ts Adds createLink/deleteLink mutations plus entity validation helpers and audit logging.
convex/crm/linkTypes.ts Adds admin mutations/queries for link type creation, deactivation, and listing.
convex/crm/linkQueries.ts Adds bidirectional link queries grouped by link type and direction.
convex/crm/tests/recordLinks.test.ts Adds mutation tests for cardinality, duplicates, org isolation, and soft-delete behaviors.
convex/crm/tests/linkTypes.test.ts Adds tests for link type creation, listing, deactivation, and org isolation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/admin/shell/FilterBuilder.tsx Outdated
Comment thread convex/crm/recordLinks.ts
Comment thread convex/crm/recordLinks.ts
Comment thread convex/crm/recordLinks.ts Outdated
Comment thread convex/crm/linkTypes.ts
Comment thread convex/crm/__tests__/recordLinks.test.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
specs/ENG-257/chunks/chunk-03-bidirectional-queries/tasks.md (1)

17-24: Spec references non-org-scoped indexes, but implementation correctly uses org-scoped variants.

The spec states getLinkTypesForObject should query via by_source_object and by_target_object indexes, but the implementation in convex/crm/linkQueries.ts (lines 221-232) correctly uses the org-scoped by_org_source_object and by_org_target_object indexes instead. The implementation is preferable for org isolation. Consider updating the spec to match.

📝 Suggested spec update
 ## T-007: Create `getLinkTypesForObject` query
 - **File:** `convex/crm/linkQueries.ts` (add to same file)
 - **Details:**
   - Use `crmQuery`
   - Args: `{ objectDefId: v.id("objectDefs") }`
-  - Query `linkTypeDefs` via `by_source_object` and `by_target_object` indexes
+  - Query `linkTypeDefs` via `by_org_source_object` and `by_org_target_object` indexes
   - Filter `isActive === true`
   - Return combined list (deduplicated by _id) — used by ENG-258's "Add Link" dialog
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/ENG-257/chunks/chunk-03-bidirectional-queries/tasks.md` around lines 17
- 24, Update the spec for getLinkTypesForObject to match the implementation:
state that the query (crmQuery getLinkTypesForObject) should use the org-scoped
indexes by_org_source_object and by_org_target_object (not the non-org-scoped
by_source_object/by_target_object), accept args { objectDefId:
v.id("objectDefs") }, filter linkTypeDefs where isActive === true, and return a
combined deduplicated list by _id for use by the "Add Link" dialog.
specs/ENG-257/chunks/chunk-03-bidirectional-queries/context.md (1)

32-42: Minor documentation drift: linkId type shown as string but implementation uses Id<"recordLinks">.

The LinkedRecord interface in this context doc shows linkId: string, but the actual implementation in convex/crm/linkQueries.ts (line 45) assigns linkId: link._id which is typed as Id<"recordLinks">. The type inference is correct in the implementation; consider updating the spec to reflect the actual type for accuracy.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/ENG-257/chunks/chunk-03-bidirectional-queries/context.md` around lines
32 - 42, The spec's LinkedRecord type is out of sync: change the linkId field
from string to the proper Id<"recordLinks"> type to match the implementation
(see linkQueries.ts where linkId is set from link._id) and ensure the context
doc mirrors the type defined in convex/crm/types.ts for LinkedRecord; update the
type annotation in
specs/ENG-257/chunks/chunk-03-bidirectional-queries/context.md accordingly.
convex/crm/linkTypes.ts (1)

125-137: Consider adding isActive to the by_org index for query efficiency.

The current implementation fetches all linkTypeDefs for the org, then filters isActive in memory. While acceptable for admin operations with typically few link types, adding a composite index like by_org_active: ["orgId", "isActive"] would enable more efficient querying as usage scales.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@convex/crm/linkTypes.ts` around lines 125 - 137, The current listLinkTypes
handler queries linkTypeDefs using the by_org index then filters isActive in
memory; add a composite index (e.g., by_org_active with keys ["orgId",
"isActive"]) and update the query in listLinkTypes to use
withIndex("by_org_active", q => q.eq("orgId", orgId).eq("isActive", true)) so
the database returns only active link types instead of fetching all and
filtering; ensure the index name matches your schema and tests are updated where
linkTypeDefs/by_org is referenced.
convex/crm/recordLinks.ts (1)

168-186: Tighten these lookups to the active link type.

Both queries load all links for the source/target and then filter in memory for linkTypeDefId and isDeleted. That makes createLink cost grow with total fanout on each side, including soft-deleted history, even though the handler only needs a few existence checks. Convex calls out read/write limits at the mutation level and recommends indexing queries to reduce scanned documents, so this is worth narrowing before link volume grows. (docs.convex.dev)

Also applies to: 189-249

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@convex/crm/recordLinks.ts` around lines 168 - 186, The queries that populate
existingSourceLinks and existingTargetLinks (the .withIndex("by_org_source",
...) and .withIndex("by_org_target", ...) calls) currently fetch all links for
the source/target and then filter in memory; update those index queries to
include eq("linkTypeDefId", args.linkTypeDefId) and eq("isDeleted", false) so
the database only scans active links of the requested type, and apply the same
tightening to the other similar queries in this file (the subsequent queries
around the createLink logic referenced in lines 189-249) to avoid scanning
soft-deleted history and reduce mutation read cost.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@convex/crm/recordLinks.ts`:
- Around line 24-45: The getNativeEntity function is casting plain strings to
Convex Ids before fetching, which is unsafe; replace the direct cast +
ctx.db.get calls with validation via ctx.db.normalizeId(tableName, entityId) and
if normalizeId returns null, return null (do not call ctx.db.get); otherwise
pass the normalized Id value to ctx.db.get. Apply the same pattern to other
places handling unvalidated polymorphic IDs (e.g., any code working with
sourceId/targetId in this file between the earlier and later switch blocks) so
every ctx.db.get uses a normalized Id from ctx.db.normalizeId(tableName, id)
rather than a raw cast.

In `@specs/ENG-254/chunks/chunk-01-calendar-query/context.md`:
- Around line 50-51: The spec's type for skippedFilters is wrong: the
implementation's CalendarData contract uses a numeric skipped count, not
string[], so update the spec to match the code by changing skippedFilters:
string[] to skippedFilters: number (or rename to skippedCount: number if
aligning naming with CalendarData); ensure references to
skippedFilters/CalendarData in the document and any examples reflect the numeric
type so clients correctly expect a count rather than an array.

In `@src/components/admin/shell/FilterBuilder.tsx`:
- Around line 38-45: The parseDateToUtcMs function currently uses Date.UTC which
normalizes invalid calendar dates; update parseDateToUtcMs to reject impossible
dates by validating the produced UTC date matches the input components: after
parsing year/month/day and doing basic range checks, compute ms = Date.UTC(year,
month - 1, day) then create a Date from ms and compare getUTCFullYear(),
getUTCMonth()+1 and getUTCDate() against the original year, month and day; if
any differ return null, otherwise return ms. This ensures functions like
parseDateToUtcMs only accept true calendar dates.

---

Nitpick comments:
In `@convex/crm/linkTypes.ts`:
- Around line 125-137: The current listLinkTypes handler queries linkTypeDefs
using the by_org index then filters isActive in memory; add a composite index
(e.g., by_org_active with keys ["orgId", "isActive"]) and update the query in
listLinkTypes to use withIndex("by_org_active", q => q.eq("orgId",
orgId).eq("isActive", true)) so the database returns only active link types
instead of fetching all and filtering; ensure the index name matches your schema
and tests are updated where linkTypeDefs/by_org is referenced.

In `@convex/crm/recordLinks.ts`:
- Around line 168-186: The queries that populate existingSourceLinks and
existingTargetLinks (the .withIndex("by_org_source", ...) and
.withIndex("by_org_target", ...) calls) currently fetch all links for the
source/target and then filter in memory; update those index queries to include
eq("linkTypeDefId", args.linkTypeDefId) and eq("isDeleted", false) so the
database only scans active links of the requested type, and apply the same
tightening to the other similar queries in this file (the subsequent queries
around the createLink logic referenced in lines 189-249) to avoid scanning
soft-deleted history and reduce mutation read cost.

In `@specs/ENG-257/chunks/chunk-03-bidirectional-queries/context.md`:
- Around line 32-42: The spec's LinkedRecord type is out of sync: change the
linkId field from string to the proper Id<"recordLinks"> type to match the
implementation (see linkQueries.ts where linkId is set from link._id) and ensure
the context doc mirrors the type defined in convex/crm/types.ts for
LinkedRecord; update the type annotation in
specs/ENG-257/chunks/chunk-03-bidirectional-queries/context.md accordingly.

In `@specs/ENG-257/chunks/chunk-03-bidirectional-queries/tasks.md`:
- Around line 17-24: Update the spec for getLinkTypesForObject to match the
implementation: state that the query (crmQuery getLinkTypesForObject) should use
the org-scoped indexes by_org_source_object and by_org_target_object (not the
non-org-scoped by_source_object/by_target_object), accept args { objectDefId:
v.id("objectDefs") }, filter linkTypeDefs where isActive === true, and return a
combined deduplicated list by _id for use by the "Add Link" dialog.
🪄 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

Run ID: df02a803-d176-4dc5-858a-6a05b0d52562

📥 Commits

Reviewing files that changed from the base of the PR and between 71568a9 and b4c130c.

📒 Files selected for processing (19)
  • convex/crm/__tests__/linkTypes.test.ts
  • convex/crm/__tests__/recordLinks.test.ts
  • convex/crm/linkQueries.ts
  • convex/crm/linkTypes.ts
  • convex/crm/recordLinks.ts
  • convex/crm/recordQueries.ts
  • convex/crm/types.ts
  • convex/crm/validators.ts
  • convex/schema.ts
  • specs/ENG-254/chunks/chunk-01-calendar-query/context.md
  • specs/ENG-257/chunks/chunk-01-link-type-crud/context.md
  • specs/ENG-257/chunks/chunk-01-link-type-crud/tasks.md
  • specs/ENG-257/chunks/chunk-02-record-linking/context.md
  • specs/ENG-257/chunks/chunk-02-record-linking/tasks.md
  • specs/ENG-257/chunks/chunk-03-bidirectional-queries/context.md
  • specs/ENG-257/chunks/chunk-03-bidirectional-queries/tasks.md
  • specs/ENG-257/chunks/manifest.md
  • specs/ENG-257/tasks.md
  • src/components/admin/shell/FilterBuilder.tsx

Comment thread convex/crm/recordLinks.ts
Comment thread specs/ENG-254/chunks/chunk-01-calendar-query/context.md Outdated
Comment thread src/components/admin/shell/FilterBuilder.tsx
…ilters, and ID validation in recordLinks

- Use ctx.db.normalizeId() instead of unsafe as Id<> casts in getNativeEntity and validateEntityExists
- Add isDeleted=false and linkTypeDefId filters to source/target link queries to reduce reads
- Fix reverse duplicate check to actually query the reverse direction (B→A when A→B exists)
- Enforce full one_to_one semantics by checking cross-direction participation
@Connorbelez Connorbelez merged commit 7891d36 into main Mar 31, 2026
0 of 3 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Mar 31, 2026
Connorbelez added a commit that referenced this pull request Apr 20, 2026
eng-257

eng-257

## Summary by Sourcery

Introduce polymorphic CRM record linking with admin-managed link types and bidirectional queries, and harden date filter handling to be timezone-safe.

New Features:
- Add admin CRUD operations for link type definitions, including creation, deactivation, and listing of link types scoped to an org.
- Implement record linking mutations that create and soft-delete polymorphic links between CRM records and native entities with cardinality and org-ownership enforcement.
- Expose queries to fetch linked records bidirectionally, grouped by link type, and to list applicable link types for a given object definition.

Bug Fixes:
- Change date filter encoding to use explicit UTC parsing, avoiding cross-browser timezone inconsistencies when converting date strings to timestamps.

Documentation:
- Add ENG-257 specification chunks covering link type CRUD, record linking mutations, and bidirectional link queries, plus a master task list and chunk manifest.
- Update ENG-254 calendar query spec to document result truncation behavior and extend the CalendarData contract with skippedFilters and truncated flags.

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

## Release Notes

* **New Features**
  * Link type management: create, deactivate, and list link types for your organization
  * Record linking: establish and remove relationships between records
  * Bidirectional record queries: view all linked records and available link types for objects
  * Enhanced date validation in filter inputs with stricter parsing and error handling

* **Tests**
  * Added comprehensive test coverage for link type creation, deactivation, and record linking operations with cardinality enforcement and organization isolation validation
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants