fix(rest-api): /destinationTokens returns empty list when bridgeable token doesnt exist in bridge map
#3314
Conversation
…ridgemap with destinations
WalkthroughThe changes involve modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
![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 (2)
packages/rest-api/src/controllers/destinationTokensController.ts (1)
20-26: LGTM: Improved handling of non-existent tokens.This change effectively addresses the PR objective by returning an empty list when a bridgeable token doesn't exist in the bridge map. The logic handles both cases appropriately.
For improved clarity, consider adding a comment explaining the significance of an empty
fromTokenInfo:if (!fromTokenInfo) { + // Return an empty list if the token doesn't exist or is not bridgeable payload = [] } else { const constructedKey = `${fromTokenInfo.symbol}-${fromChain}` payload = BRIDGE_ROUTE_MAPPING[constructedKey] }packages/rest-api/src/tests/destinationTokensRoute.test.ts (1)
84-93: LGTM: New test case for empty destination tokens listThe new test case is well-structured and covers an important scenario where no destination tokens are available. It aligns with the PR objectives and enhances the test coverage.
Suggestion for improvement:
Consider adding a comment explaining the significance of chain ID 1666600000 (Harmony Mainnet) and why it's expected to return an empty list. This would enhance the test's readability and maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- packages/rest-api/src/controllers/destinationTokensController.ts (1 hunks)
- packages/rest-api/src/tests/destinationTokensRoute.test.ts (2 hunks)
🧰 Additional context used
🔇 Additional comments (5)
packages/rest-api/src/controllers/destinationTokensController.ts (3)
13-14: LGTM: Introduction ofpayloadvariable improves code structure.The addition of the
payloadvariable enhances code readability and maintainability by providing a consistent structure for the response data throughout the function.
33-33: LGTM: Improved response handling.The explicit
returnstatement ensures that the function exits immediately after sending the response. This change improves control flow and prevents potential issues with code execution after the response has been sent.
Line range hint
40-44: LGTM: Consistent error response handling.The explicit
returnstatement in the error handling block is consistent with the improvement made in the successful response handling. This ensures that the function exits immediately after sending the error response, improving the overall control flow of the function.packages/rest-api/src/tests/destinationTokensRoute.test.ts (2)
6-6: LGTM: New import added for ONEETHThe new import for ONEETH is consistent with the existing import style and is necessary for the new test case.
Line range hint
1-161: Overall improvement in test coverageThe changes to this file enhance the test suite by adding coverage for the scenario where no destination tokens are available. This aligns well with the PR objectives and improves the robustness of the tests. The existing tests remain intact, ensuring that previously covered scenarios are still validated.
Deploying sanguine-fe with
|
| Latest commit: |
5084e97
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1b86e0bb.sanguine-fe.pages.dev |
| Branch Preview URL: | https://rest-api-handle-tokens-not-i.sanguine-fe.pages.dev |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3314 +/- ##
===================================================
- Coverage 31.92469% 26.98386% -4.94083%
===================================================
Files 238 178 -60
Lines 14553 11833 -2720
Branches 356 82 -274
===================================================
- Hits 4646 3193 -1453
+ Misses 9614 8357 -1257
+ Partials 293 283 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Description
Not all
bridgeabletokens exist in bridgemap, so this handles the case by returning an empty list.Summary by CodeRabbit
New Features
Bug Fixes
Tests