feat(payment-detection): option to use TheGraph Network with API key #1610
Conversation
|
""" WalkthroughThe changes introduce support for TheGraph Explorer as a new subgraph endpoint option in the payment detection module. This involves adding new constants, updating type definitions to include an optional API key for TheGraph Explorer, and refactoring the logic for selecting the appropriate subgraph URL into a new function. The main client creation function is simplified to use this new URL resolution function. Additionally, comprehensive tests are added to verify correct URL generation for various networks and configurations, including cases with and without TheGraph Explorer API keys. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ClientModule as TheGraph Client Module
Caller->>ClientModule: defaultGetTheGraphClient(network, options)
ClientModule->>ClientModule: defaultGetTheGraphClientUrl(network, options)
alt Network supported by TheGraph Explorer and API key provided
ClientModule->>ClientModule: Construct Explorer URL with API key
else Network supported by Alchemy
ClientModule->>ClientModule: Construct Alchemy URL
else Network is special (e.g., mantle, core)
ClientModule->>ClientModule: Construct special network URL
else
ClientModule->>ClientModule: Construct Studio URL
end
ClientModule->>Caller: Return instantiated client or undefined
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (7)
🪧 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 (
|
github-actions
bot
requested review from
aimensahnoun,
kevindavee,
rodrigopavezi and
yomarion
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/payment-detection/src/thegraph/client.ts (1)
40-58: Consider documenting subgraph IDs maintenance processThe mapping provides necessary subgraph IDs for TheGraph Explorer, but lacks documentation about how these IDs are determined or maintained. These IDs appear to be static identifiers, but if they change in the future, it would be helpful to know the process for updating them.
Consider adding a comment that explains:
- Where these IDs come from
- How stable these IDs are expected to be
- How/when they should be updated if needed
packages/payment-detection/test/thegraph/client.test.ts (1)
3-46: Comprehensive test coverage with room for improvementThe tests cover the main use cases for different networks and configurations, providing good verification of the URL construction logic. However, there are a few scenarios that should be tested for completeness:
- The 'private' network case which should return undefined
- The case where an API key is provided but the network has no subgraph ID
- Testing for 'core' network which has a special URL
Consider adding these additional test cases for complete coverage:
it('should return undefined for private network', () => { const url = defaultGetTheGraphClientUrl('private'); expect(url).toBeUndefined(); }); it('should fallback to non-Explorer URL when network has no subgraph ID', () => { // Assuming a network that's not in THE_GRAPH_EXPLORER_SUBGRAPH_ID but in THE_GRAPH_ALCHEMY_CHAINS const url = defaultGetTheGraphClientUrl('unsupported-network', { theGraphExplorerApiKey: 'test' }); expect(url).toBe( 'https://api.studio.thegraph.com/query/67444/request-payments-unsupported-network/version/latest' ); }); it('should build the correct URL for Core', () => { const url = defaultGetTheGraphClientUrl('core'); expect(url).toBe( 'https://thegraph.coredao.org/subgraphs/name/requestnetwork/request-payments-core' ); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/payment-detection/src/thegraph/client.ts(5 hunks)packages/payment-detection/test/thegraph/client.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/payment-detection/test/thegraph/client.test.ts (1)
packages/payment-detection/src/thegraph/client.ts (1)
defaultGetTheGraphClientUrl(137-169)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-test
🔇 Additional comments (5)
packages/payment-detection/src/thegraph/client.ts (5)
12-14: Well-structured URL template for TheGraph ExplorerThe addition of this constant follows the existing pattern for URL templates and clearly defines the structure for TheGraph Explorer endpoints.
27-38: Explicit Alchemy chains enumeration improves maintainabilityDefining supported Alchemy chains as a constant array makes the code more maintainable. If Alchemy adds support for more chains, we'll only need to update this list.
83-84: Clear documentation for new API key optionThe added option is well-documented with a helpful comment that explains its purpose.
96-102: Properly excludes API key from client optionsThe updated destructuring correctly extracts and excludes the API key from client options, with appropriate comments explaining why the variable is unused.
171-179: Simplified client creation logicThe refactored function is more concise and easier to understand by leveraging the new URL resolution function. The early return for unsupported networks is also a good practice.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/payment-detection/src/thegraph/client.ts (1)
153-153: Good implementation of handling missing subgraph IDsThis line properly addresses the previous review comment by ensuring TheGraph Explorer is only used when both an API key and a valid subgraph ID are available.
🧹 Nitpick comments (1)
packages/payment-detection/src/thegraph/client.ts (1)
137-172: Consider adding comments to explain the network normalizationThe URL resolution function is well-structured with clear prioritization logic. However, the reasoning behind replacing 'aurora' with 'near' in the network name could use some explanation.
+ // Aurora uses the same subgraph as Near, so we normalize the network name const filteredNetwork = network.replace('aurora', 'near');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (1)
packages/payment-detection/src/thegraph/client.ts(5 hunks)
🔇 Additional comments (5)
packages/payment-detection/src/thegraph/client.ts (5)
12-14: Great addition of TheGraph Explorer supportAdding support for TheGraph Explorer with API key integration is a valuable feature that gives developers more flexibility in their deployment options.
27-38: Well-structured constants for network configurationThe clear enumeration of Alchemy-supported chains and TheGraph Explorer subgraph IDs improves code readability and maintainability. This approach makes it easy to update supported networks in the future.
Also applies to: 40-58
83-84: Good type extension for the new API key optionThe type definition is properly extended with clear documentation. This ensures type safety when users provide the new optional parameter.
96-102: Proper handling of the API key in options extractionThe code correctly extracts and excludes the API key from client options, which is a good practice since it's only needed for URL construction.
178-181: Good refactoring of client creationThe client creation function is now more concise and follows a clear separation of concerns by delegating URL resolution to a dedicated function.
3 participants
Description of the changes
The goal of this PR is to make it easier for builders to use TheGraph Network for payment detection with their own API key.
Summary by CodeRabbit
New Features
Tests