chore(backend): Add missing orderBy field to machine list params #6767
Conversation
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
🦋 Changeset detectedLatest commit: 7564c37 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughAdds pagination type integration and sorting/filtering options to machines.list via GetMachineListParams, replacing limit/offset with ClerkPaginationRequest and supporting orderBy (name or created_at with sign) and query. Documents the public API addition in a changeset entry. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant BackendSDK as Backend SDK (machines.list)
participant ClerkAPI as Clerk API
participant DB as Store
Client->>BackendSDK: list({ page/size, orderBy?, query? })
note right of BackendSDK: orderBy supports +/− sign<br/>for name or created_at
BackendSDK->>ClerkAPI: GET /machines with pagination, orderBy, query
ClerkAPI->>DB: Fetch machines with filters/sort
DB-->>ClerkAPI: Rows
ClerkAPI-->>BackendSDK: Paged result set
BackendSDK-->>Client: Machines[], page info
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment Pre-merge checks✅ Passed checks (3 passed)
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/backend/src/api/endpoints/MachineApi.ts (4)
45-48: Validate WithSign semantics to avoid a silent breaking change.Does WithSign<'name' | 'created_at'> accept bare 'name' (no sign)? The changeset example uses 'name' without +/−. If WithSign requires +/−, existing calls may break. If unsure, widen the type to accept both.
- orderBy?: WithSign<'name' | 'created_at'>; + // Accept bare field or signed variant to preserve BC + orderBy?: WithSign<'name' | 'created_at'> | 'name' | 'created_at';
50-52: Grammar nit in JSDoc.Use “an ID”.
- * Returns machines that have a ID or name that matches the given query. + * Returns machines that have an ID or name that matches the given query.
43-53: Clarify behavior details in docs.Consider documenting:
- Allowed fields explicitly: 'name' | 'created_at'
- Sort direction examples: '+name', '-created_at'
- Whether query does partial/contains match and case sensitivity
75-81: Add explicit return type to public list method.Matches our guidelines and improves API clarity.
- async list(queryParams: GetMachineListParams = {}) { + async list(queryParams: GetMachineListParams = {}): Promise<PaginatedResourceResponse<Machine[]>> { return this.request<PaginatedResourceResponse<Machine[]>>({ method: 'GET', path: basePath, queryParams, }); }.changeset/gentle-lamps-collect.md (1)
5-14: Align the changeset with the actual API shape and usage.Document allowed fields and +/− sign; update example accordingly to prevent confusion.
-Added missing `orderBy` field to machines list method +Add missing `orderBy` field to machines.list input. Supports sorting by `'name'` or `'created_at'`. Prefix with `+` for ASC or `-` for DESC. Example: ```ts clerkClient.machines.list({ ...params, - orderBy: 'name' + orderBy: '+name' // or '-created_at' })</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Disabled knowledge base sources:** - Linear integration is disabled by default for public repositories > You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 526d50506d620f700e3530ef566aad16fb4b4e7c and 7564c37fa8e03677603001d0054bfb5fedc5e6b1. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `.changeset/gentle-lamps-collect.md` (1 hunks) * `packages/backend/src/api/endpoints/MachineApi.ts` (2 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (7)</summary> <details> <summary>.changeset/**</summary> **📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)** > Automated releases must use Changesets. Files: - `.changeset/gentle-lamps-collect.md` </details> <details> <summary>**/*.{js,jsx,ts,tsx}</summary> **📄 CodeRabbit inference engine (.cursor/rules/development.mdc)** > `**/*.{js,jsx,ts,tsx}`: All code must pass ESLint checks with the project's configuration > Follow established naming conventions (PascalCase for components, camelCase for variables) > Maintain comprehensive JSDoc comments for public APIs > Use dynamic imports for optional features > All public APIs must be documented with JSDoc > Provide meaningful error messages to developers > Include error recovery suggestions where applicable > Log errors appropriately for debugging > Lazy load components and features when possible > Implement proper caching strategies > Use efficient data structures and algorithms > Profile and optimize critical paths > Validate all inputs and sanitize outputs > Implement proper logging with different levels Files: - `packages/backend/src/api/endpoints/MachineApi.ts` </details> <details> <summary>**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}</summary> **📄 CodeRabbit inference engine (.cursor/rules/development.mdc)** > Use Prettier for consistent code formatting Files: - `packages/backend/src/api/endpoints/MachineApi.ts` </details> <details> <summary>packages/**/*.{ts,tsx}</summary> **📄 CodeRabbit inference engine (.cursor/rules/development.mdc)** > TypeScript is required for all packages Files: - `packages/backend/src/api/endpoints/MachineApi.ts` </details> <details> <summary>packages/**/*.{ts,tsx,d.ts}</summary> **📄 CodeRabbit inference engine (.cursor/rules/development.mdc)** > Packages should export TypeScript types alongside runtime code Files: - `packages/backend/src/api/endpoints/MachineApi.ts` </details> <details> <summary>**/*.{ts,tsx}</summary> **📄 CodeRabbit inference engine (.cursor/rules/development.mdc)** > Use proper TypeScript error types > > `**/*.{ts,tsx}`: Always define explicit return types for functions, especially public APIs > Use proper type annotations for variables and parameters where inference isn't clear > Avoid `any` type - prefer `unknown` when type is uncertain, then narrow with type guards > Use `interface` for object shapes that might be extended > Use `type` for unions, primitives, and computed types > Prefer `readonly` properties for immutable data structures > Use `private` for internal implementation details > Use `protected` for inheritance hierarchies > Use `public` explicitly for clarity in public APIs > Prefer `readonly` for properties that shouldn't change after construction > Prefer composition and interfaces over deep inheritance chains > Use mixins for shared behavior across unrelated classes > Implement dependency injection for loose coupling > Let TypeScript infer when types are obvious > Use `const assertions` for literal types: `as const` > Use `satisfies` operator for type checking without widening > Use mapped types for transforming object types > Use conditional types for type-level logic > Leverage template literal types for string manipulation > Use ES6 imports/exports consistently > Use default exports sparingly, prefer named exports > Use type-only imports: `import type { ... } from ...` > No `any` types without justification > Proper error handling with typed errors > Consistent use of `readonly` for immutable data > Proper generic constraints > No unused type parameters > Proper use of utility types instead of manual type construction > Type-only imports where possible > Proper tree-shaking friendly exports > No circular dependencies > Efficient type computations (avoid deep recursion) Files: - `packages/backend/src/api/endpoints/MachineApi.ts` </details> <details> <summary>**/*.{js,ts,tsx,jsx}</summary> **📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)** > Support multiple Clerk environment variables (CLERK_*, NEXT_PUBLIC_CLERK_*, etc.) for configuration. Files: - `packages/backend/src/api/endpoints/MachineApi.ts` </details> </details><details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>packages/backend/src/api/endpoints/MachineApi.ts (1)</summary><blockquote> <details> <summary>packages/types/src/pagination.ts (1)</summary> * `ClerkPaginationRequest` (4-13) </details> </blockquote></details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)</summary> * GitHub Check: Integration Tests (machine, chrome) * GitHub Check: Integration Tests (vue, chrome) * GitHub Check: Integration Tests (billing, chrome) * GitHub Check: Integration Tests (custom, chrome) * GitHub Check: Integration Tests (nextjs, chrome, 14) * GitHub Check: Integration Tests (localhost, chrome) * GitHub Check: Integration Tests (elements, chrome) * GitHub Check: Integration Tests (tanstack-react-router, chrome) * GitHub Check: Integration Tests (tanstack-react-start, chrome) * GitHub Check: Integration Tests (react-router, chrome) * GitHub Check: Integration Tests (sessions, chrome) * GitHub Check: Integration Tests (nextjs, chrome, 15) * GitHub Check: Integration Tests (ap-flows, chrome) * GitHub Check: Integration Tests (expo-web, chrome) * GitHub Check: Integration Tests (astro, chrome) * GitHub Check: Integration Tests (express, chrome) * GitHub Check: Integration Tests (nuxt, chrome) * GitHub Check: Integration Tests (quickstart, chrome) * GitHub Check: Integration Tests (generic, chrome) * GitHub Check: Publish with pkg-pr-new * GitHub Check: Unit Tests (22, **) * GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c... * GitHub Check: Static analysis * GitHub Check: semgrep-cloud-platform/scan * GitHub Check: semgrep-cloud-platform/scan </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>packages/backend/src/api/endpoints/MachineApi.ts (3)</summary><blockquote> `1-1`: **LGTM: type-only import for pagination.** Consistent with our TS import style and tree-shaking. --- `9-9`: **No change needed — WithSign is defined locally.** WithSign is exported at packages/backend/src/api/endpoints/util-types.ts:1, so importing from './util-types' is correct. --- `75-81`: **No change required — client converts orderBy → order_by.** Tests assert the query param 'order_by' is emitted when passing orderBy (packages/backend/src/api/__tests__/SamlConnectionApi.test.ts); endpoints use orderBy in types (e.g. packages/backend/src/api/endpoints/MachineApi.ts). > Likely an incorrect or invalid review comment. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
tmilewski
approved these changes
Sep 13, 2025
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.
Description
https://clerk.com/docs/reference/backend-api/tag/machines/get/machines.query.order_by
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Chores