Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughRefactored multiple REST API endpoints to use fluent chainable API pattern with AJV schema validation, replacing legacy Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## chore/apis #39779 +/- ##
==============================================
+ Coverage 70.55% 70.57% +0.01%
==============================================
Files 3256 3256
Lines 115791 115791
Branches 21083 21015 -68
==============================================
+ Hits 81694 81715 +21
+ Misses 32023 32011 -12
+ Partials 2074 2065 -9
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…messages to OpenAPI Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…umers The declare module augmentation from ExtractRoutesFromAPI only works within apps/meteor compilation. External packages like ddp-client need the types to remain in rest-typings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tion The endpoint was missing a query validator, causing ExtractRoutesFromAPI to extract undefined params. This broke consumers like useAppSlashCommands that pass offset/count params. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sages
The migrated endpoints were using forbidden('error-not-allowed') as a
type workaround, but this changed the error response from 'unauthorized'
to 'error-not-allowed', breaking existing tests.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
API.v1.forbidden() without arguments returns 'unauthorized' for backward compatibility. The test was expecting the old 'Not allowed' message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/app/api/server/v1/im.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/im.ts:313">
P2: `dm.setTopic` can return `topic: null` even though its response schema only permits string values.</violation>
<violation number="2" location="apps/meteor/app/api/server/v1/im.ts:752">
P3: `dm.create` and `im.create` now duplicate the same endpoint config and handler; extract one shared action/config to avoid divergence.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| projection: fields, | ||
| }); | ||
| return API.v1.success({ | ||
| topic, |
There was a problem hiding this comment.
P2: dm.setTopic can return topic: null even though its response schema only permits string values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/api/server/v1/im.ts, line 313:
<comment>`dm.setTopic` can return `topic: null` even though its response schema only permits string values.</comment>
<file context>
@@ -240,296 +219,597 @@ const dmCloseAction = <Path extends string>(_path: Path): TypedAction<typeof dmC
- projection: fields,
- });
+ return API.v1.success({
+ topic,
+ });
+ };
</file context>
| topic, | |
| topic: topic ?? '', |
| .post('dm.close', dmCloseEndpointsProps, dmCloseAction('dm.close')) | ||
| .post('im.close', dmCloseEndpointsProps, dmCloseAction('im.close')) | ||
| .post( | ||
| 'dm.create', |
There was a problem hiding this comment.
P3: dm.create and im.create now duplicate the same endpoint config and handler; extract one shared action/config to avoid divergence.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/api/server/v1/im.ts, line 752:
<comment>`dm.create` and `im.create` now duplicate the same endpoint config and handler; extract one shared action/config to avoid divergence.</comment>
<file context>
@@ -240,296 +219,597 @@ const dmCloseAction = <Path extends string>(_path: Path): TypedAction<typeof dmC
+ .post('dm.close', dmCloseEndpointsProps, dmCloseAction('dm.close'))
+ .post('im.close', dmCloseEndpointsProps, dmCloseAction('im.close'))
+ .post(
+ 'dm.create',
+ {
+ authRequired: true,
</file context>
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Release Notes
Bug Fixes
Documentation