Provide realtime skipColumns option via untamperable public access tokens #2201
Conversation
🦋 Changeset detectedLatest commit: bd9b9e8 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis change introduces support for an optional 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (25)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/test/realtimeClient.test.ts (1)
71-77: Consider adding tests for realtime options functionality.While the current tests correctly use the new parameter signature, they only pass empty objects and don't test the actual
skipColumnsfunctionality. Consider adding test cases that verify:
- Realtime options are properly passed through to the Electric API
- Column filtering works as expected when
skipColumnsis specified- Invalid realtime options are handled appropriately
Example test structure:
+ const responseWithSkipColumns = await client.streamRun( + "http://localhost:3000?offset=-1", + environment, + run.id, + { skipColumns: ['metadata', 'payload'] }, + "0.8.1" + ); + + // Verify that the Electric API receives the correct parameters + // This would require mocking or integration testing approachAlso applies to: 109-115, 218-226
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/test/realtimeClient.test.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`apps/webapp/**/*`: The webapp at apps/webapp is a Remix 2.1 app that uses Node.js v20. Use zod extensively in the webapp.
apps/webapp/**/*: The webapp at apps/webapp is a Remix 2.1 app that uses Node.js v20.
Use zod extensively in the webapp.
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
apps/webapp/test/realtimeClient.test.ts
🧠 Learnings (1)
apps/webapp/test/realtimeClient.test.ts (3)
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-06-24T08:06:34.136Z
Learning: When writing tests in the webapp (apps/webapp/app/**/*.test.{ts,tsx}), avoid importing env.server.ts directly or indirectly; instead, ensure that all dependencies are testable and receive configuration via options, not environment variables.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-06-24T08:06:34.136Z
Learning: Maintain a clear separation between service logic and configuration: implement testable services in dedicated files (e.g., services/realtimeClient.server.ts) and handle configuration in separate files (e.g., services/realtimeClientGlobal.server.ts).
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2155
File: docs/docs.json:179-183
Timestamp: 2025-06-06T16:54:23.316Z
Learning: In the docs.json configuration for the Trigger.dev documentation (Mintlify system), both "tags": ["v4"] and "tag": "v4" properties can be used together and work correctly, even though this behavior is undocumented and may not work in local development environments.
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/webapp/test/realtimeClient.test.ts (2)
75-75: LGTM: Parameter updates align with new method signatures.The addition of empty object
{}parameters tostreamRunandstreamRunscalls correctly reflects the newRealtimeRequestOptionsparameter. The positioning before the version string is consistent and appropriate.Also applies to: 85-85, 113-113, 123-123, 224-224, 315-315
306-306: ```shell
#!/bin/bashSearch for streamRun method definitions and calls across the repository with context
rg -C5 "streamRun"
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
No description provided.