(fix) Fixing getAccountsByExternalId function (route was incorrect before) #14135
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThis pull request includes an update to the version number of the Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
🧹 Outside diff range and nitpick comments (1)
packages/sdk/src/server/__tests__/server.test.ts (1)
452-452: LGTM! Consider adding a comment explaining the URL structure.The URL change from an account-centric to a user-centric approach aligns with the PR objective of fixing the
getAccountsByExternalIdfunction. This update likely reflects the correct server implementation.Consider adding a brief comment explaining the URL structure, e.g.:
// URL structure: /connect/users/{externalUserId}/accounts // This endpoint retrieves accounts associated with a user's external ID expect(fetchMock).toHaveBeenCalledWith( "https://api.pipedream.com/v1/connect/users/external-id-1/accounts", expect.any(Object), );This comment would enhance code readability and provide context for future developers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/sdk/package.json (1 hunks)
- packages/sdk/src/server/tests/server.test.ts (1 hunks)
- packages/sdk/src/server/index.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/sdk/package.json
🔇 Additional comments (1)
packages/sdk/src/server/index.ts (1)
639-641: LGTM! The endpoint update aligns with the PR objective.The change from
/accounts/external_id/${externalId}to/users/${externalId}/accountscorrectly fixes the route for thegetAccountsByExternalIdfunction as mentioned in the PR title. This update suggests a shift in API design, now associating accounts with users, which is a more logical structure.To ensure this change doesn't introduce any regressions, please verify the following:
- The new endpoint is correctly implemented on the backend.
- Any existing tests for this method are updated to use the new endpoint.
- Documentation for this method is updated to reflect the new endpoint structure.
You can use the following script to check for any remaining occurrences of the old endpoint in the codebase:
If this script returns any results, those occurrences should be updated to use the new endpoint structure.
✅ Verification successful
Verification Successful: No remaining instances of the old endpoint found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining instances of the old endpoint rg --type typescript --type javascript '/accounts/external_id/\$\{externalId\}'Length of output: 116
Script:
#!/bin/bash # Search for any remaining instances of the old endpoint rg '/accounts/external_id/\$\{externalId\}' -g "*.ts" -g "*.js"Length of output: 65
Summary by CodeRabbit
New Features
Bug Fixes
Chores
@pipedream/sdkpackage from 0.1.6 to 0.1.7.