Triggers: Fix patch/delete with explicit table names

[Nicolapps]

Fixes #876

Summary by CodeRabbit

• New Features

  • Added updateUser and deleteUser mutations
  • Introduced userCount table with automatic user count tracking

• Bug Fixes

  • Improved trigger handling when no triggers are registered for a table

• Tests

  • Added tests covering create, update, and delete flows and verifying user count consistency

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds a denormalized userCount table with a trigger-backed counter and new public mutations updateUser and deleteUser; extends tests to exercise create/update/delete flows. Also fixes trigger iteration in _queueTriggers to use a default empty array when no triggers are registered for a table.

Changes

Cohort / File(s)	Summary
Core triggers fix packages/convex-helpers/server/triggers.ts	_queueTriggers changed to iterate over a default empty array when no triggers are registered for a table, preventing runtime "not iterable" errors when using explicit IDs.
Tests, schema, and new public mutations packages/convex-helpers/server/triggers.test.ts	Adds userCount denormalized table and trigger logic, exposes new public mutations updateUser and deleteUser in the test API, and adds tests covering create → update → delete sequences verifying the running user count.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Server as MutationHandler
  participant Triggers
  participant DB

  Client->>Server: call createUser / updateUser / deleteUser
  Server->>DB: perform insert/patch/delete (explicit id allowed)
  Server->>Triggers: _queueTriggers checks registered[table] (now defaults to [])
  Triggers-->>DB: run trigger handlers (e.g., update userCount) if any
  DB-->>Server: ack
  Server-->>Client: result

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

• Areas to focus during review:
  • packages/convex-helpers/server/triggers.ts: ensure default empty-array change doesn't hide other registration issues and matches intended semantics.
  • packages/convex-helpers/server/triggers.test.ts: verify the userCount schema and trigger logic correctly handle concurrent insert/delete/update sequences and that the new public mutations are exported/mapped consistently.
  • Confirm tests actually exercise the explicit-ID paths (insert/patch/delete) related to linked issue #876.

Poem

🐇 I counted users, one by one,
Inserts and deletes kept the fun.
Triggers now skip empty rows with grace,
No more "not iterable" in this place.
Hoppity hops — tests pass, smiles run. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: addressing a bug in the triggers system when using explicit table names in patch and delete operations.
Linked Issues check	✅ Passed	The pull request addresses the core issue #876 by fixing the runtime error in _queueTriggers when handling patch/delete with explicit table IDs, demonstrated through comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes directly support the core fix: defensive iteration in triggers.ts prevents the error, while test file additions validate the fix works for create, update, and delete operations.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch nicolas/fix-triggers-explicit-table-ids

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5ba952 and 23bddf3.

📒 Files selected for processing (1)

• packages/convex-helpers/server/triggers.test.ts (4 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

When modifying complex TypeScript types, run npm run typecheck in the repository root to ensure types are correct, rather than running tsc manually

Files:

• packages/convex-helpers/server/triggers.test.ts

🧬 Code graph analysis (1)

packages/convex-helpers/server/triggers.test.ts (2)

packages/convex-helpers/validators.ts (1)

• v (272-272)

packages/convex-helpers/testing.ts (1)

• mutation (78-83)

🔇 Additional comments (6)

packages/convex-helpers/server/triggers.test.ts (6)

36-38: LGTM: Clean table definition for the counter.

The userCount table schema is straightforward and appropriate for maintaining a denormalized count.

---

77-89: Excellent test for the explicit ID fix, with a note on production usage.

This trigger effectively validates the fix for patch/delete operations with explicit table IDs (the core issue from #876). The counter logic correctly handles insert/delete operations while ignoring updates.

Note: The read-modify-write pattern (lines 79-85) could experience race conditions under concurrent load in production, as multiple operations might read the same count value before any patches complete. For this test suite, sequential execution makes this safe, but production counter implementations should use atomic operations or proper locking.

---

126-134: LGTM: Directly tests the patch operation fix.

This mutation exercises the exact scenario that was broken in #876—calling patch with an explicit table name and ID. The implementation is clean and correct.

---

136-141: LGTM: Directly tests the delete operation fix.

This mutation exercises the exact scenario that was broken in #876—calling delete with an explicit table name and ID. The implementation is clean and correct.

---

148-149: LGTM: Test API correctly extended.

The new mutations are properly added to the test API, following the established pattern.

---

189-224: Excellent comprehensive test of the fix.

This test thoroughly validates the fix for #876 by exercising create, update, and delete operations with explicit table IDs. The test correctly verifies that:

• The counter trigger fires on insert/delete operations
• Update operations don't affect the count (as intended)
• All operations complete without the "o.registered[a] is not iterable" error

The test flow is logical and the assertions are appropriate.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/get-convex/convex-helpers@881

commit: 23bddf3

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f1b4f0 and b5ba952.

📒 Files selected for processing (2)

• packages/convex-helpers/server/triggers.test.ts (5 hunks)
• packages/convex-helpers/server/triggers.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

When modifying complex TypeScript types, run npm run typecheck in the repository root to ensure types are correct, rather than running tsc manually

Files:

• packages/convex-helpers/server/triggers.ts
• packages/convex-helpers/server/triggers.test.ts

🪛 GitHub Check: Test and lint

packages/convex-helpers/server/triggers.test.ts

[warning] 16-16:
'describe' is defined but never used. Allowed unused vars must match /^_/u

🔇 Additional comments (5)

packages/convex-helpers/server/triggers.ts (1)

442-442: LGTM! This fix correctly addresses the issue.

The nullish coalescing operator (?? []) ensures that when no triggers are registered for a table, the code iterates over an empty array instead of undefined, preventing the "not iterable" error reported in issue #876.

packages/convex-helpers/server/triggers.test.ts (4)

36-38: LGTM!

The userCount table schema is correctly defined to support the denormalized counter trigger.

---

77-89: LGTM!

The denormalized counter trigger correctly maintains a running count of users. The use of explicit table IDs in the patch operations (ctx.db.patch("userCount", countId, ...)) provides excellent test coverage for the fix in triggers.ts.

---

126-141: LGTM!

These mutations provide comprehensive test coverage for the bug fix. Both updateUser (using patch with explicit table name) and deleteUser (using delete with explicit table name) exercise the exact code paths that were failing before the fix in triggers.ts.

---

189-224: LGTM! Excellent test coverage.

This test comprehensively validates the fix by exercising create, update, and delete operations with explicit table IDs. The assertions verify that the denormalized counter is correctly maintained throughout the lifecycle, confirming that the "not iterable" error no longer occurs.