feat: use new rest api for api keys#144
Conversation
WalkthroughThe API key management commands were refactored from a GraphQL-based approach to use RESTful HTTP methods. This includes changes to creation, deletion, listing, and editing of API keys, with corresponding schema updates to support new response structures and properties. The code now uses explicit HTTP methods and simplified response parsing. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Command
participant API
Client->>Command: Create/List/Edit/Delete API Key (with input)
Command->>API: HTTP Request (POST/GET/PATCH/DELETE)
API-->>Command: HTTP Response (JSON)
Command-->>Client: Parsed Result (ApiKey/ApiKeyWithValue/boolean)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ 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). (1)
🔇 Additional comments (1)
✨ Finishing Touches
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/commands/api-key/api-key.create.ts(1 hunks)src/commands/api-key/api-key.delete.ts(1 hunks)src/commands/api-key/api-key.edit.ts(1 hunks)src/commands/api-key/api-key.list.ts(1 hunks)src/contracts/api-key.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: jbiskur
PR: flowcore-io/flowcore-sdk#59
File: src/contracts/tenant.ts:74-74
Timestamp: 2025-02-27T09:48:49.487Z
Learning: The tenant fetch functionality has been migrated to use a REST API with an updated schema (using 'websiteUrl'), while the tenant list functionality still uses GraphQL API with the old schema format (using 'website'). This is why there are different field names for the same concept in different parts of the codebase.
src/contracts/api-key.ts (1)
Learnt from: jbiskur
PR: flowcore-io/flowcore-sdk#59
File: src/contracts/tenant.ts:74-74
Timestamp: 2025-02-27T09:48:49.487Z
Learning: The tenant fetch functionality has been migrated to use a REST API with an updated schema (using 'websiteUrl'), while the tenant list functionality still uses GraphQL API with the old schema format (using 'website'). This is why there are different field names for the same concept in different parts of the codebase.
src/commands/api-key/api-key.delete.ts (1)
Learnt from: jbiskur
PR: flowcore-io/flowcore-sdk#59
File: src/contracts/tenant.ts:74-74
Timestamp: 2025-02-27T09:48:49.487Z
Learning: The tenant fetch functionality has been migrated to use a REST API with an updated schema (using 'websiteUrl'), while the tenant list functionality still uses GraphQL API with the old schema format (using 'website'). This is why there are different field names for the same concept in different parts of the codebase.
src/commands/api-key/api-key.list.ts (1)
Learnt from: jbiskur
PR: flowcore-io/flowcore-sdk#59
File: src/contracts/tenant.ts:74-74
Timestamp: 2025-02-27T09:48:49.487Z
Learning: The tenant fetch functionality has been migrated to use a REST API with an updated schema (using 'websiteUrl'), while the tenant list functionality still uses GraphQL API with the old schema format (using 'website'). This is why there are different field names for the same concept in different parts of the codebase.
🧬 Code Graph Analysis (3)
src/commands/api-key/api-key.edit.ts (2)
src/contracts/api-key.ts (2)
ApiKey(27-27)ApiKeySchema(6-22)src/utils/parse-response-helper.ts (1)
parseResponseHelper(8-18)
src/commands/api-key/api-key.create.ts (2)
src/contracts/api-key.ts (2)
ApiKeyWithValue(55-55)ApiKeyWithValueSchema(32-50)src/utils/parse-response-helper.ts (1)
parseResponseHelper(8-18)
src/commands/api-key/api-key.list.ts (2)
src/contracts/api-key.ts (2)
ApiKey(27-27)ApiKeySchema(6-22)src/utils/parse-response-helper.ts (1)
parseResponseHelper(8-18)
🔇 Additional comments (5)
src/contracts/api-key.ts (1)
1-55: LGTM! Well-structured schema updates for REST API migration.The schema changes appropriately support the REST API migration:
- New properties (
tenantId,description,maskedApiKey) align with typical REST API responses- Making
lastUsedAtnullable is sensible for newly created API keys- The separation between
ApiKeySchemaandApiKeyWithValueSchemafollows security best practices by only exposing the actual key value when necessarysrc/commands/api-key/api-key.edit.ts (1)
1-55: LGTM! Clean implementation of API key edit functionality.The PATCH endpoint implementation follows REST best practices:
- Appropriate use of PATCH for partial updates
- Restricting edits to only the
descriptionfield is a good security practice- Proper response validation using the schema
src/commands/api-key/api-key.create.ts (1)
1-57: LGTM! Clean refactoring from GraphQL to REST.The migration to REST is well-executed:
- POST method is appropriate for resource creation
- Optional
descriptionfield aligns with the updated schema- Returning
ApiKeyWithValueon creation is the expected behavior- Simplified response parsing improves maintainability
src/commands/api-key/api-key.list.ts (1)
1-53: Clean migration to REST API.The refactoring from GraphQL to REST is well-implemented with proper array schema validation.
src/commands/api-key/api-key.delete.ts (1)
1-44: Clean implementation of the DELETE endpoint.The migration to REST is well-structured with appropriate method and path configuration.
| protected override getPath(): string { | ||
| return `/api/v1/api-keys?tenantId=${this.input.tenantId}` | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider URL encoding the query parameter.
The tenantId is directly interpolated into the URL without encoding. This could cause issues if the tenantId contains special characters.
Apply this fix to properly encode the query parameter:
- return `/api/v1/api-keys?tenantId=${this.input.tenantId}`
+ return `/api/v1/api-keys?tenantId=${encodeURIComponent(this.input.tenantId)}`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected override getPath(): string { | |
| return `/api/v1/api-keys?tenantId=${this.input.tenantId}` | |
| } | |
| protected override getPath(): string { | |
| return `/api/v1/api-keys?tenantId=${encodeURIComponent(this.input.tenantId)}` | |
| } |
🤖 Prompt for AI Agents
In src/commands/api-key/api-key.list.ts around lines 39 to 41, the tenantId
query parameter is directly interpolated into the URL without encoding, which
can cause issues with special characters. Fix this by applying URL encoding to
the tenantId using encodeURIComponent before including it in the URL string.
| /** | ||
| * Parse the response | ||
| */ | ||
| protected override getBody(): Record<string, unknown> { | ||
| return { | ||
| query: graphQlQueryById, | ||
| variables: this.input, | ||
| } | ||
| protected override parseResponse(_rawResponse: unknown): boolean { | ||
| return true | ||
| } |
There was a problem hiding this comment.
Consider proper error handling for DELETE responses.
The parseResponse method always returns true regardless of the actual response or HTTP status. This could mask failures and lead to incorrect state management.
Consider checking the response status or at least documenting why always returning true is acceptable:
protected override parseResponse(_rawResponse: unknown): boolean {
+ // The base Command class handles HTTP errors, so reaching here means success
return true
}Alternatively, you could validate the response status if the base class provides access to it.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Parse the response | |
| */ | |
| protected override getBody(): Record<string, unknown> { | |
| return { | |
| query: graphQlQueryById, | |
| variables: this.input, | |
| } | |
| protected override parseResponse(_rawResponse: unknown): boolean { | |
| return true | |
| } | |
| /** | |
| * Parse the response | |
| */ | |
| protected override parseResponse(_rawResponse: unknown): boolean { | |
| // The base Command class handles HTTP errors, so reaching here means success | |
| return true | |
| } |
🤖 Prompt for AI Agents
In src/commands/api-key/api-key.delete.ts around lines 45 to 50, the
parseResponse method currently returns true unconditionally, which can hide
errors from the DELETE request. Modify this method to inspect the actual
response or HTTP status code to determine success or failure, returning true
only if the deletion was successful. If the base class provides access to the
response status, use it to validate the outcome; otherwise, document clearly why
always returning true is safe.
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes