Skip to content

Comments

feat/mobula-search-api#279

Merged
RanaBug merged 3 commits intostagingfrom
feat/mobula-search-api
Apr 2, 2025
Merged

feat/mobula-search-api#279
RanaBug merged 3 commits intostagingfrom
feat/mobula-search-api

Conversation

@RanaBug
Copy link
Collaborator

@RanaBug RanaBug commented Apr 1, 2025

Description

  • Mobula's search API implementation to replace search of the local token list

How Has This Been Tested?

  • Existing Unit Tests and Manual Testing

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Summary by CodeRabbit

  • New Features

    • Introduced a loading indicator that displays a spinner during token searches.
    • Enhanced token search inputs across the platform using a debouncing mechanism for smoother, more responsive lookups.
    • Improved display of search results, including clear feedback when no tokens are found.
    • Added error handling for token searches to notify users of issues.
  • Refactor

    • Streamlined token search logic to integrate live API responses for more accurate and efficient results.
    • Updated the structure of token search functions to consolidate functionality and improve maintainability.

@RanaBug RanaBug requested a review from IAmKio April 1, 2025 15:38
@RanaBug RanaBug self-assigned this Apr 1, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 1, 2025

Walkthrough

This PR introduces multiple modifications across token-related components, tests, reducers, services, and type definitions. It updates test mocks for token data, refactors token search inputs to use a debounced API-driven approach, and adds loading states to the token list rendering. New API endpoints and helper functions are implemented, while legacy search methods are removed. The changes also extend Redux state management for tracking search loading and expand TypeScript types for API responses.

Changes

Files Change Summary
src/apps/…/CardsSwap.test.tsx, src/apps/…/DropdownTokensList/test/DropdownTokensList.test.tsx Updated mocks: added mockTokenAssets, introduced and updated chain mapping mocks, removed the old searchTokens implementation.
src/apps/…/TokenSearchInput.tsx, src/apps/…/TokensSearchInput.tsx Refactored token search inputs: implemented debounced search using searchText and debouncedSearchText, removed synchronous search, and integrated API calls via useGetSearchTokensQuery.
src/apps/…/DropdownTokensList.tsx, src/apps/…/TokensSearchResult.tsx Introduced loading state (isTokenSearchLoading) with conditional rendering: displays a CircularProgress spinner during token search operations.
src/apps/…/theExchangeSlice.ts, src/apps/…/tokenAtlasSlice.ts Added isTokenSearchLoading property and a new reducer setIsTokenSearchLoading to manage the loading state in Redux slices.
src/services/…/pillarXApiSearchTokens.ts, src/services/…/tokensData.ts Introduced new service with the getSearchTokens endpoint and implemented convertAPIResponseToTokens to process API responses, replacing the old searchTokens.
src/types/api.ts Added new TypeScript types: MobulaToken, Exchange, Pair, TokenAssetResponse, PairResponse, and MobulaApiResponse.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant Input as TokenSearchInput
    participant API as PillarXApiSearchTokens
    participant Store as Redux Store
    participant Display as Token List Renderer

    User->>Input: Enter search text
    Input->>Input: Debounce input (1s delay)
    Input->>API: Trigger API call with debounced text
    API-->>Input: Return token data
    Input->>Store: Dispatch search results & update loading state
    Store->>Display: Provide updated state
    Display->>User: Render loading spinner or token list based on state
Loading

Suggested reviewers

  • IAmKio

Poem

I'm a happy rabbit, coding with delight,
Hopping through tests and debounces so light.
Mocks and APIs now dance in my glen,
Loading spinners join the carrot-filled den.
CodeRabbit cheers with each neat little tweak,
Celebrating smooth flows in the changes we seek!
🥕🐇 Happy coding!

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 1, 2025

Deploying x with  Cloudflare Pages  Cloudflare Pages

Latest commit: 84b714b
Status: ✅  Deploy successful!
Preview URL: https://26b08c70.x-e62.pages.dev
Branch Preview URL: https://feat-mobula-search-api.x-e62.pages.dev

View logs

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (1)
src/apps/the-exchange/components/DropdownTokensList/test/DropdownTokensList.test.tsx (1)

179-195: ⚠️ Potential issue

Update tests to reflect new API-based search behavior.

The test is still using the old local filtering approach by directly changing the search input and expecting immediate results. With the new API-based approach, this test should be updated to account for debouncing and asynchronous loading states.

Consider updating this test to:

  1. Mock the API call response
  2. Wait for the loading state to complete
  3. Then assert the expected results

This would ensure the test accurately reflects the new API-driven search behavior.

🧹 Nitpick comments (4)
src/services/pillarXApiSearchTokens.ts (1)

31-45: Consider adding error handling for API failures.

While the implementation is solid, consider adding error handling to manage API failures gracefully. This would improve user experience when the API is unavailable or returns errors.

src/apps/token-atlas/components/TokensSearchInput/TokensSearchInput.tsx (1)

62-69: Consider adding error handling for API failures.

The API query is well-structured with appropriate skip logic for empty searches, but lacks error handling for failed API requests.

  const { data: searchData, isLoading } = useGetSearchTokensQuery(
    {
      searchInput: debouncedSearchText,
      filterBlockchains: chainIdToChainNameTokensData(selectedChain.chainId),
    },
    { skip: !debouncedSearchText }
  );
+ // Consider adding error state handling:
+ // const { data: searchData, isLoading, error } = useGetSearchTokensQuery...
src/types/api.ts (1)

495-581: Consider breaking down the extensive PairResponse type.

The PairResponse type includes a large number of time-interval-specific metrics that make the type quite lengthy and potentially difficult to maintain.

Consider creating separate types for different time intervals and composing them:

type TimeIntervalMetrics = {
  trades?: number;
  buys?: number;
  sells?: number;
  volume?: number;
  buy_volume?: number;
  sell_volume?: number;
  sellers?: number;
  buyers?: number;
  traders?: number;
};

type PairTimeMetrics = {
  '1min'?: TimeIntervalMetrics;
  '5min'?: TimeIntervalMetrics;
  '15min'?: TimeIntervalMetrics;
  '1h'?: TimeIntervalMetrics;
  '4h'?: TimeIntervalMetrics;
  '12h'?: TimeIntervalMetrics;
  '24h'?: TimeIntervalMetrics;
};

export type PairResponse = {
  // Base pair properties
  token0: MobulaToken;
  token1: MobulaToken;
  volume24h: number;
  // ...other base properties
  
  // Time-based metrics
  metrics: PairTimeMetrics;
  
  // Price changes
  price_change_1min?: number;
  price_change_5min?: number;
  // ...other price changes
};
src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx (1)

261-265: Uncomment or remove commented-out test code.

There's commented-out code that would override the global mock for useGetSearchTokensQuery. Either uncomment it if needed for this specific test case or remove it if it's no longer needed.

-    // (useGetSearchTokensQuery as jest.Mock).mockReturnValue({
-    //   data: undefined,
-    //   isLoading: false,
-    //   isFetching: false,
-    // });
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 603e5d5 and 3099a9e.

⛔ Files ignored due to path filters (2)
  • src/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snap is excluded by !**/*.snap
  • src/apps/pillarx-app/components/PointsTile/test/__snapshots__/PointsTile.test.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (11)
  • src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx (3 hunks)
  • src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx (3 hunks)
  • src/apps/the-exchange/components/DropdownTokensList/test/DropdownTokensList.test.tsx (1 hunks)
  • src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (3 hunks)
  • src/apps/the-exchange/reducer/theExchangeSlice.ts (4 hunks)
  • src/apps/token-atlas/components/TokensSearchInput/TokensSearchInput.tsx (2 hunks)
  • src/apps/token-atlas/components/TokensSearchResult/TokensSearchResult.tsx (3 hunks)
  • src/apps/token-atlas/reducer/tokenAtlasSlice.ts (4 hunks)
  • src/services/pillarXApiSearchTokens.ts (1 hunks)
  • src/services/tokensData.ts (2 hunks)
  • src/types/api.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx (1)
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (1)
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
🧬 Code Definitions (7)
src/apps/token-atlas/components/TokensSearchResult/TokensSearchResult.tsx (1)
src/apps/token-atlas/hooks/useReducerHooks.tsx (1)
  • useAppSelector (6-6)
src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx (1)
src/apps/the-exchange/hooks/useReducerHooks.tsx (1)
  • useAppSelector (6-6)
src/services/tokensData.ts (2)
src/types/api.ts (1)
  • TokenAssetResponse (475-493)
src/utils/blockchain.ts (2)
  • CompatibleChains (260-289)
  • getNativeAssetForChainId (65-137)
src/services/pillarXApiSearchTokens.ts (1)
src/types/api.ts (1)
  • MobulaApiResponse (583-587)
src/apps/token-atlas/components/TokensSearchInput/TokensSearchInput.tsx (5)
src/apps/token-atlas/hooks/useReducerHooks.tsx (1)
  • useAppSelector (6-6)
src/apps/token-atlas/types/types.tsx (1)
  • ChainType (1-4)
src/services/tokensData.ts (3)
  • chainIdToChainNameTokensData (211-232)
  • convertAPIResponseToTokens (267-387)
  • chainNameToChainIdTokensData (235-256)
src/apps/token-atlas/reducer/tokenAtlasSlice.ts (3)
  • setIsTokenSearchLoading (115-117)
  • setSearchToken (112-114)
  • setSearchTokenResult (73-75)
src/types/api.ts (1)
  • TokenAssetResponse (475-493)
src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (3)
src/services/tokensData.ts (2)
  • chainIdToChainNameTokensData (211-232)
  • convertAPIResponseToTokens (267-387)
src/apps/the-exchange/reducer/theExchangeSlice.ts (3)
  • setIsTokenSearchLoading (106-108)
  • setSearchToken (103-105)
  • setSearchTokenResult (91-93)
src/types/api.ts (1)
  • TokenAssetResponse (475-493)
src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx (1)
src/services/tokensData.ts (1)
  • Token (11-19)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: unit-tests
  • GitHub Check: build
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (43)
src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx (3)

2-2: LGTM: Material-UI import added for loading indicator.

The CircularProgress component from Material-UI is correctly imported for implementing the loading state indicator.


84-86: LGTM: Redux state selector added for loading state.

Good implementation of a selector that retrieves the token search loading state from Redux. The type assertion ensures type safety.


248-276: LGTM: Proper implementation of loading state with conditional rendering.

The loading state is handled well with conditional rendering. When loading, a properly sized spinner is shown; otherwise, the token list is displayed. This improves user experience by providing feedback during API calls.

src/services/pillarXApiSearchTokens.ts (4)

1-9: LGTM: Proper imports and structure for the RTK Query API service.

The file correctly imports necessary dependencies and types for implementing the search API service.


10-21: LGTM: Well-structured API service definition with environment-aware base URL.

The API service is properly defined with a dynamic base URL that adapts to testnet/mainnet environments. The correct content type header is set for JSON requests.


51-53: LGTM: Proper middleware registration and hook export.

The API service is correctly registered with middleware, and the query hook is properly exported for use in components.


22-47:

Details

✅ Verification successful

Verify Mobula API response format matches the expected type.

The endpoint is well-implemented with proper typing for the request parameters and response. The query construction logic correctly handles the optional filter parameter.


🏁 Script executed:

#!/bin/bash
# Verify that the MobulaApiResponse type matches the actual API response structure

# Check how MobulaApiResponse is used in other files
rg -A 10 "MobulaApiResponse" --type typescript

Length of output: 81


Action Required: Verify MobulaApiResponse Definition

The endpoint implementation appears sound and handles request parameters correctly. However, the initial verification command failed because of an unrecognized file type. To ensure that the Mobula API response format truly matches the expected MobulaApiResponse type, please run an updated search that targets TypeScript files, for example:

After executing the above command, manually confirm that the actual API response structure aligns with the declared type. If discrepancies exist, further adjustments may be required.


🏁 Script executed:

#!/bin/bash
# Search for the definition of MobulaApiResponse in all TypeScript files
rg -A 10 "export type MobulaApiResponse" -g "*.ts"

Length of output: 362


API Response Type Verification Complete

The implementation of the getSearchTokens query properly constructs the request and handles optional filter parameters as expected. The MobulaApiResponse type—defined in src/types/api.ts—confirms that the expected API response structure (i.e., a result object with a data field containing TokenAssetResponse[] | PairResponse[]) is being enforced.

No further changes are required.

src/apps/the-exchange/components/DropdownTokensList/test/DropdownTokensList.test.tsx (1)

67-76: LGTM: Proper mock implementation for chain ID to name conversion.

The mock implementation for chainIdToChainNameTokensData is well-structured and provides the necessary test coverage for the chain conversion functionality.

src/apps/token-atlas/reducer/tokenAtlasSlice.ts (4)

33-33: LGTM: State type properly extended for loading state.

The TokenAtlasState type is correctly extended with the new isTokenSearchLoading property.


54-54: LGTM: Initial state properly set for loading indicator.

The isTokenSearchLoading state is correctly initialized to false in the initial state.


115-117: LGTM: Reducer function properly implemented for updating loading state.

The setIsTokenSearchLoading reducer function is well-implemented to update the loading state based on the action payload.


136-136: LGTM: Action creator properly exported.

The setIsTokenSearchLoading action creator is correctly exported for use in other components.

src/apps/token-atlas/components/TokensSearchResult/TokensSearchResult.tsx (3)

47-49: Great addition of loading state management!

The new isTokenSearchLoading state variable from Redux provides better user experience by indicating when a search is in progress.


97-99: Good use of CircularProgress component for loading indication

The loading spinner provides clear visual feedback to users during search operations, enhancing the user experience.


100-102: Well-structured conditional rendering logic

The rendering logic is now properly organized to handle different states:

  1. Loading state shows a spinner
  2. Empty results with no loading shows "No tokens found"
  3. Results available with no loading shows the token list

This approach ensures a clear user experience through all possible states.

Also applies to: 103-113

src/apps/the-exchange/reducer/theExchangeSlice.ts (4)

25-25: Good addition to state type definition

Adding the isTokenSearchLoading boolean property to the SwapState type ensures type safety for the new loading state.


51-51: Proper initialization of new state property

The new isTokenSearchLoading state is correctly initialized to false in the initial state object.


106-108: Clean implementation of loading state reducer

The implementation of setIsTokenSearchLoading reducer is clean and follows Redux patterns.


129-129: Complete action export

The new action creator is properly exported, making it available for use in components.

src/services/tokensData.ts (6)

258-266: Well-documented new function

The JSDoc comment clearly explains the purpose and behavior of the new convertAPIResponseToTokens function, which is crucial for maintainability.


267-270: Good function signature with optional parameter

The function signature properly accepts the API response array and an optional search input parameter, making it flexible for different use cases.


271-271: Proper error handling for empty results

The function correctly handles the case when no results are returned or the result array is empty, preventing runtime errors.


274-337: Comprehensive token data processing

The token data processing logic handles multiple edge cases:

  1. Filters for compatible blockchains
  2. Renames wrapped tokens appropriately
  3. Handles special cases for native tokens
  4. Properly maps API response structure to the application's Token type

This ensures consistent data representation across the application.


341-369: Smart searching implementation

The search implementation is well thought out:

  1. Includes native tokens that aren't in Mobula's list
  2. Uses Fuse.js for fuzzy matching with appropriate thresholds
  3. Handles contract addresses (longer than 40 chars) with exact matching
  4. Returns properly mapped results

This provides a good user experience for various search scenarios.


372-384: Comprehensive handling of empty search case

When no search input is provided, the function correctly adds native tokens to the result list, ensuring users see all relevant tokens.

src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (7)

63-64: Good implementation of search state with debouncing

Using separate state variables for immediate input (searchText) and debounced API calls (debouncedSearchText) is a good practice to prevent excessive API calls.


67-73: Well-implemented debounce mechanism

The debounce implementation using setTimeout with proper cleanup in the effect return function prevents unnecessary API calls while typing.


76-84: Efficient API query implementation

The useGetSearchTokensQuery hook is used effectively with:

  1. Proper parameters for search input and blockchain filtering
  2. Skip option to prevent unnecessary API calls when search text is empty
  3. Automatic handling of loading state

86-121: Comprehensive search results handling

The effect correctly:

  1. Updates the loading state in Redux
  2. Converts API responses to the application's Token format
  3. Updates search token and results in Redux
  4. Applies proper chain filtering when necessary
  5. Has appropriate dependencies in the dependency array

The implementation ensures proper state management throughout the search process.


124-133: Updated presence recording

The presence recording now correctly uses the debounced search text for analytics, ensuring more meaningful usage data is collected.


142-142: Simplified input handling

The input handler now directly updates the local state without complex logic, making the code cleaner and easier to maintain.


158-158: Controlled component implementation

The input value is now correctly bound to the searchText state, ensuring it behaves as a controlled component.

src/apps/token-atlas/components/TokensSearchInput/TokensSearchInput.tsx (5)

46-48: Good implementation of a debounced search input.

The separation of immediate input (searchText) from debounced API queries (debouncedSearchText) is a good practice to prevent excessive API calls while typing.


53-60: LGTM: Well-implemented debounce mechanism.

The 1-second debounce timing is a good balance between responsiveness and reducing unnecessary API calls. The timeout cleanup in the effect return ensures no memory leaks.


71-97: Well-structured state management for the search results.

The effect correctly handles loading state and processes API responses, with appropriate filtering based on the selected chain. The dependency array is properly defined to react to relevant changes.

However, consider extracting the chain filtering logic to the API call if the server supports it, which would improve performance by reducing the amount of data transferred.


99-109: Good update to presence tracking.

The effect now correctly tracks the debounced search text rather than immediate input value, which ensures more meaningful analytics data about what users are actually searching for.


111-113: Simple and effective event handler.

The handler now updates only the immediate search text state, while the debounce effect takes care of updating the debounced value that triggers the API call.

src/types/api.ts (3)

432-448: Well-structured MobulaToken type.

The type definition is comprehensive and includes all necessary fields for token data from the Mobula API.


473-493: Good documentation and type structure for TokenAssetResponse.

The comment explaining that this type merges both Token and Asset types is helpful. The type definition itself is well-structured with appropriate optional fields marked with question marks.


583-587: LGTM: Generic API response type.

The MobulaApiResponse type appropriately wraps the various data types that can be returned from the API.

src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx (3)

34-53: Well-defined mock token assets for testing.

The mockTokenAssets array provides consistent test data that matches the Token type structure, which is essential for reliable test outcomes.


55-62: Good API mock implementation.

The mock correctly simulates the RTK Query hook behavior with appropriate default values for the initial test state.


64-87:

Details

❓ Verification inconclusive

Ensure consistent mock return values across the codebase.

The chainNameToChainIdTokensData mock now returns an empty string instead of null for unrecognized chain names, which differs from some implementations. Ensure this change is consistent with how the function is used in the actual code.

Run this script to verify the consistency:


🏁 Script executed:

#!/bin/bash
# Check how chainNameToChainIdTokensData handles unrecognized chains
ast-grep --pattern 'chainNameToChainIdTokensData($$$)' | grep -A 5 -B 5 "default"

Length of output: 81


Action: Verify Consistent Default Return Value for chainNameToChainIdTokensData

It appears that the current mock for chainNameToChainIdTokensData in the test file (lines 64–87 of src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx) returns an empty string for unrecognized chain names. Since some implementations elsewhere in the codebase might be using null as the default, please manually verify that this behavior is intentional and consistent with how the function is consumed in production.

  • File Concern: src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx (lines 64–87)
  • Issue: The mock returns '' for an unrecognized chain name, which might conflict with production expectations if other parts of the code rely on a null default.

Please confirm if this default return value is correct or adjust accordingly.

Copy link
Collaborator

@IAmKio IAmKio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only change i would request is to have a search request error state in place for requests that fail for any reason. Because we're now calling an API for each debounced request as opposed to reading from disk. Other than that there's just one question and the rest looks good!

@github-actions github-actions bot temporarily deployed to Preview (feat/mobula-search-api) April 2, 2025 10:35 Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/apps/token-atlas/components/TokensSearchInput/TokensSearchInput.tsx (1)

64-103: Consider adding a retry mechanism for failed searches

The current implementation sets the error state but doesn't provide any retry mechanism for failed searches. This could lead to a poor user experience if the API call fails.

Consider adding a retry button or automatic retry logic for failed searches:

+ const [retryCount, setRetryCount] = useState(0);

  const {
    data: searchData,
    isLoading,
    error,
+   refetch
  } = useGetSearchTokensQuery(
    {
      searchInput: debouncedSearchText,
      filterBlockchains: chainIdToChainNameTokensData(selectedChain.chainId),
    },
-   { skip: !debouncedSearchText }
+   { skip: !debouncedSearchText, retryOnFocus: true }
  );

  useEffect(() => {
    dispatch(setIsTokenSearchLoading(isLoading));
    dispatch(setIsTokenSearchErroring(Boolean(error)));
+   
+   // Auto-retry on error (up to 3 times)
+   if (error && retryCount < 3) {
+     const retryTimer = setTimeout(() => {
+       setRetryCount(prev => prev + 1);
+       refetch();
+     }, 2000);
+     
+     return () => clearTimeout(retryTimer);
+   }

    if (!searchData) return;

    // Rest of the code remains the same
  }, [searchData, debouncedSearchText, selectedChain, error, retryCount]);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3099a9e and 84b714b.

⛔ Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json
  • src/apps/pillarx-app/components/PointsTile/test/__snapshots__/PointsTile.test.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (7)
  • src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx (4 hunks)
  • src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (3 hunks)
  • src/apps/the-exchange/reducer/theExchangeSlice.ts (4 hunks)
  • src/apps/token-atlas/components/TokensSearchInput/TokensSearchInput.tsx (2 hunks)
  • src/apps/token-atlas/components/TokensSearchResult/TokensSearchResult.tsx (3 hunks)
  • src/apps/token-atlas/reducer/tokenAtlasSlice.ts (4 hunks)
  • src/services/pillarXApiSearchTokens.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx
  • src/apps/token-atlas/components/TokensSearchResult/TokensSearchResult.tsx
  • src/services/pillarXApiSearchTokens.ts
  • src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx
🧰 Additional context used
🧬 Code Definitions (1)
src/apps/token-atlas/components/TokensSearchInput/TokensSearchInput.tsx (3)
src/services/tokensData.ts (2)
  • chainIdToChainNameTokensData (211-232)
  • convertAPIResponseToTokens (267-387)
src/apps/token-atlas/reducer/tokenAtlasSlice.ts (4)
  • setIsTokenSearchLoading (117-119)
  • setIsTokenSearchErroring (120-122)
  • setSearchToken (114-116)
  • setSearchTokenResult (75-77)
src/types/api.ts (1)
  • TokenAssetResponse (475-493)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: build
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (14)
src/apps/the-exchange/reducer/theExchangeSlice.ts (4)

25-26: Well-structured additions to the SwapState type

Adding boolean flags for tracking token search loading and error states is a good practice when implementing API-driven search functionality. These properties will allow components to properly display loading indicators and error messages during search operations.


52-53: Appropriate default values for the new state properties

Initializing both loading and error states to false is the correct approach. This ensures the UI starts in a clean state before any search operations are performed.


108-113: Properly implemented reducer functions

The new reducer functions follow the established pattern in this slice. They correctly use the payload action pattern to update their respective state properties.


134-135: Action creators properly exported

The new action creators are correctly exported, making them available for dispatch throughout the application.

src/apps/token-atlas/reducer/tokenAtlasSlice.ts (4)

33-34: LGTM: State properties added for tracking token search status.

The addition of isTokenSearchLoading and isTokenSearchErroring properties to the TokenAltasState type is a good practice for managing API call states.


55-56: LGTM: Initial state values properly set.

Both new properties are correctly initialized to false in the initial state.


117-122: LGTM: New reducers follow Redux patterns.

The implementation of setIsTokenSearchLoading and setIsTokenSearchErroring reducers follows Redux best practices by using immutable state updates through the Redux Toolkit's internal Immer functionality.


141-142: LGTM: New actions properly exported.

The new reducer actions are correctly exported for use in other components.

src/apps/token-atlas/components/TokensSearchInput/TokensSearchInput.tsx (6)

12-27: LGTM: Imports updated for new search functionality.

The imports have been appropriately updated to support the new API-driven search functionality, including the necessary Redux actions, API hooks, and type definitions.


47-48: LGTM: State management implemented for debounced search.

Good implementation of separate state variables for immediate user input (searchText) and debounced API calls (debouncedSearchText), which will improve performance by reducing unnecessary API requests.


54-61: LGTM: Debouncing implementation is correct.

The debounce implementation using setTimeout and clearTimeout is correct and follows best practices. The 1-second delay is a reasonable timeframe for search operations.


63-74: LGTM: RTK Query integration optimizes API handling.

The implementation of the token search using RTK Query's useGetSearchTokensQuery hook is well-structured. The skip parameter correctly prevents unnecessary API calls when there's no search text.


105-115: LGTM: User presence tracking updated for debounced search.

The presence tracking has been correctly updated to use debouncedSearchText, ensuring that analytics are only recorded when the user has finished typing.


117-119: LGTM: Input handler simplified with state updates.

The handleSearch function has been simplified to update only the searchText state, with the debouncing and API call logic handled by the useEffect hooks.

Comment on lines +76 to +103
useEffect(() => {
dispatch(setIsTokenSearchLoading(isLoading));
dispatch(setIsTokenSearchErroring(Boolean(error)));

if (!searchData) return;

const result = convertAPIResponseToTokens(
searchData?.result?.data as TokenAssetResponse[],
debouncedSearchText
);

// This is to check what has been the searched token, for other components to action
dispatch(setSearchToken(debouncedSearchText));

// This sets the token results list that will be displayed in the UI
dispatch(
setSearchTokenResult(
selectedChain.chainId === 0
? result
: result.filter(
(token) =>
chainNameToChainIdTokensData(token.blockchain) ===
selectedChain.chainId
)
.map((tokens) => tokens)
)
);
}
};
)
);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [searchData, debouncedSearchText, selectedChain]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add null check for searchData.result

While the code includes a check for searchData, it assumes that searchData.result exists when passing it to convertAPIResponseToTokens.

- const result = convertAPIResponseToTokens(
-   searchData?.result?.data as TokenAssetResponse[],
-   debouncedSearchText
- );

+ const result = convertAPIResponseToTokens(
+   searchData?.result?.data || [] as TokenAssetResponse[],
+   debouncedSearchText
+ );
📝 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.

Suggested change
useEffect(() => {
dispatch(setIsTokenSearchLoading(isLoading));
dispatch(setIsTokenSearchErroring(Boolean(error)));
if (!searchData) return;
const result = convertAPIResponseToTokens(
searchData?.result?.data as TokenAssetResponse[],
debouncedSearchText
);
// This is to check what has been the searched token, for other components to action
dispatch(setSearchToken(debouncedSearchText));
// This sets the token results list that will be displayed in the UI
dispatch(
setSearchTokenResult(
selectedChain.chainId === 0
? result
: result.filter(
(token) =>
chainNameToChainIdTokensData(token.blockchain) ===
selectedChain.chainId
)
.map((tokens) => tokens)
)
);
}
};
)
);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [searchData, debouncedSearchText, selectedChain]);
useEffect(() => {
dispatch(setIsTokenSearchLoading(isLoading));
dispatch(setIsTokenSearchErroring(Boolean(error)));
if (!searchData) return;
const result = convertAPIResponseToTokens(
searchData?.result?.data || [] as TokenAssetResponse[],
debouncedSearchText
);
// This is to check what has been the searched token, for other components to action
dispatch(setSearchToken(debouncedSearchText));
// This sets the token results list that will be displayed in the UI
dispatch(
setSearchTokenResult(
selectedChain.chainId === 0
? result
: result.filter(
(token) =>
chainNameToChainIdTokensData(token.blockchain) ===
selectedChain.chainId
)
)
);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [searchData, debouncedSearchText, selectedChain]);

Copy link
Collaborator

@IAmKio IAmKio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@RanaBug RanaBug merged commit ddaedef into staging Apr 2, 2025
7 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Apr 7, 2025
3 tasks
This was referenced Jun 24, 2025
@coderabbitai coderabbitai bot mentioned this pull request Sep 8, 2025
3 tasks
@coderabbitai coderabbitai bot mentioned this pull request Dec 3, 2025
@coderabbitai coderabbitai bot mentioned this pull request Jan 10, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants