Feat/Showing base super tokens on user recurring tab #4902
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces network-specific enhancements to recurring donations functionality across multiple components. The changes primarily focus on adding a Changes
Sequence DiagramsequenceDiagram
participant User
participant RecurringDonationComponent
participant DonationService
participant GraphQLEndpoint
User->>RecurringDonationComponent: Select Network
RecurringDonationComponent->>DonationService: Fetch Streams (with networkId)
DonationService->>GraphQLEndpoint: Query with networkId
GraphQLEndpoint-->>DonationService: Return Network-Specific Streams
DonationService-->>RecurringDonationComponent: Display Filtered Streams
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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: 4
🔭 Outside diff range comments (1)
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (1)
Line range hint
71-85: Consider implementing balance fetching retry mechanismThe balance fetching logic should handle network errors and implement retries for better reliability.
const fetchAllBalances = async () => { + const MAX_RETRIES = 3; + const RETRY_DELAY = 1000; + + const fetchWithRetry = async (attempt = 0) => { + try { const _allTokens = superTokens.reduce((acc, token) => { acc.push(token); acc.push(token.underlyingToken); return acc; }, [] as IToken[]); return await fetchEVMTokenBalances(_allTokens, address); + } catch (error) { + if (attempt < MAX_RETRIES) { + await new Promise(resolve => setTimeout(resolve, RETRY_DELAY)); + return fetchWithRetry(attempt + 1); + } + throw error; + } + }; + + try { - const results = await fetchEVMTokenBalances(_allTokens, address); + const results = await fetchWithRetry(); // ... rest of the function + } catch (error) { + console.error('Failed to fetch balances:', error); + setTokens([]); + setUnderlyingTokens([]); + setBalances({}); + } };
🧹 Nitpick comments (12)
src/components/views/userProfile/donationsTab/recurringTab/StreamRow.tsx (3)
59-62: Handle undefined tokens gracefully.
If findTokenByAddress returns undefined, ensure the parent code or UI handles the null case (e.g., fallback to a default symbol or message).
132-134: Consider error handling for chain switching.
If switchChain fails or is unavailable, the UI might remain unresponsive. Exposing an error message would improve user experience.
145-153: Remove unnecessary React fragment.
Static analysis warns you have a fragment containing only one child. You can remove the wrapper for cleaner JSX.Proposed fix:
- <> <ModifySuperTokenModal tokenStreams={tokenStream} setShowModal={setShowModifyModal} selectedToken={superToken} refreshBalance={refetch} recurringNetworkID={recurringNetworkId} /> - </>🧰 Tools
🪛 Biome (1.9.4)
[error] 145-153: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
src/components/views/userProfile/donationsTab/recurringTab/ActiveStreamsSection.tsx (1)
105-105: Consider using CSS gridfrunits for better responsiveness.The grid template could be more responsive by using fractional units instead of fixed ratios.
- : 'auto auto auto auto auto auto'}; + : 'minmax(100px, 1fr) minmax(100px, 1fr) minmax(80px, 1fr) minmax(120px, 1.5fr) minmax(100px, 1fr) minmax(80px, 1fr)'};src/components/views/donate/Recurring/ModifySuperToken/ModifySection.tsx (1)
67-67: Consider memoizing token lookup.The findTokenByAddress call could benefit from memoization to prevent unnecessary recalculations.
+ const _token = useMemo( + () => findTokenByAddress(token?.id, recurringNetworkID), + [token?.id, recurringNetworkID] + ); - const _token = findTokenByAddress(token?.id, recurringNetworkID);src/components/views/userProfile/donationsTab/recurringTab/ActiveProjectsSection.tsx (1)
86-86: Document the significance of networkId: 0.The use of
0as a default network ID when multiple networks are selected needs documentation.-networkId: networkIds.length === 1 ? networkIds[0] : 0, +// When networkId is 0, the backend includes all networks in the response +networkId: networkIds.length === 1 ? networkIds[0] : 0,src/components/views/donate/Recurring/ModifySuperToken/WithDrawSuperToken.tsx (1)
Line range hint
89-91: Enhance error messages for better debuggingThe error messages could be more descriptive to aid in troubleshooting.
-throw new Error('address not found1'); +throw new Error('Wallet address not found for withdrawal operation'); -throw new Error('SuperToken not found'); +throw new Error(`SuperToken not found for address: ${address}`);src/components/views/donate/Recurring/ModifySuperToken/DepositSuperToken.tsx (1)
Line range hint
134-146: Consider refactoring token decimals conversion logicThe current implementation of decimals conversion is complex and could be extracted into a utility function for better maintainability.
+const convertTokenDecimals = (amount: bigint, fromDecimals: number, toDecimals: number = 18): bigint => { + const divisor = BigInt(10 ** fromDecimals); + const currentAmount = Number(amount) / Number(divisor); + return ethers.utils.parseUnits(currentAmount.toString(), toDecimals).toBigInt(); +}; -if (token && token.decimals === 6) { - const divisor = BigInt(10 ** token.decimals); - const currentAmount = Number(amount) / Number(divisor); - newAmount = ethers.utils.parseUnits(currentAmount.toString(), 18).toBigInt(); +if (token && token.decimals !== 18) { + newAmount = convertTokenDecimals(amount, token.decimals); }src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (1)
Line range hint
34-39: Consider caching network-specific token configurationsThe network-specific token configuration could be memoized to prevent unnecessary recalculations.
+const useSuperTokens = (chainId?: number) => { + return useMemo(() => { + return chainId === config.OPTIMISM_NETWORK_NUMBER + ? config.OPTIMISM_CONFIG.SUPER_FLUID_TOKENS + : config.BASE_CONFIG.SUPER_FLUID_TOKENS; + }, [chainId]); +}; -const superTokens = - chain?.id === config.OPTIMISM_NETWORK_NUMBER - ? config.OPTIMISM_CONFIG.SUPER_FLUID_TOKENS - : config.BASE_CONFIG.SUPER_FLUID_TOKENS; +const superTokens = useSuperTokens(chain?.id);src/components/views/userProfile/donationsTab/recurringTab/FilterMenu.tsx (2)
87-163: Consider refactoring network filter implementation to reduce code duplicationThe network filter implementation contains duplicated logic for Optimism and Base networks. Consider abstracting this into a reusable component or function.
+const NetworkFilterItem = ({ + networkId, + networkName, + checked, + onChange +}: { + networkId: number; + networkName: string; + checked: boolean; + onChange: () => void; +}) => ( + <FeatureItem> + <CheckBox + onChange={onChange} + checked={checked} + size={14} + > + <Flex $alignItems='center' gap='4px'> + <NetworkLogo + chainId={networkId} + logoSize={16} + /> + <GLink size='Medium'>{networkName}</GLink> + </Flex> + </CheckBox> + </FeatureItem> +); // Usage: <NetworkFilterItem networkId={config.OPTIMISM_NETWORK_NUMBER} networkName="Optimism" checked={networkIds.includes(config.OPTIMISM_NETWORK_NUMBER)} onChange={() => { setNetworkIds(prev => prev.includes(config.OPTIMISM_NETWORK_NUMBER) ? prev.filter(id => id !== config.OPTIMISM_NETWORK_NUMBER) : [...prev, config.OPTIMISM_NETWORK_NUMBER] ); handleClose(); }} />
188-193: Consider moving token filtering logic to configurationThe token filtering logic is currently hard-coded in the component. Consider moving this to the configuration for better maintainability.
// In config.ts +export const EXCLUDED_BASE_TOKENS = ['USDC', 'DAI', 'ETH']; // In FilterMenu.tsx -token.underlyingToken.symbol !== 'USDC' && -token.underlyingToken.symbol !== 'DAI' && -token.underlyingToken.symbol !== 'ETH' +!config.EXCLUDED_BASE_TOKENS.includes(token.underlyingToken.symbol)src/services/donation.ts (1)
133-149: Consider consolidating network-specific stream fetching logicThe stream fetching logic for Base and Optimism networks is duplicated. Consider extracting this into a reusable function.
+const fetchNetworkStreams = async ( + config: { superFluidSubgraph: string }, + networkId: number, + address: string +): Promise<ITokenStreams> => { + const { data } = await gqlRequest( + config.superFluidSubgraph, + undefined, + FETCH_USER_STREAMS, + { address: address.toLowerCase() }, + ); + const streams: ISuperfluidStream[] = data?.streams || []; + const tokenStreams: ITokenStreams = {}; + + streams.forEach(stream => { + if (!tokenStreams[stream.token.id]) { + tokenStreams[stream.token.id] = []; + } + stream.networkId = networkId; + tokenStreams[stream.token.id].push(stream); + }); + + return tokenStreams; +}; // Usage: const baseTokenStreams = await fetchNetworkStreams( config.BASE_CONFIG, config.BASE_NETWORK_NUMBER, address ); const optimismTokenStreams = await fetchNetworkStreams( config.OPTIMISM_CONFIG, config.OPTIMISM_NETWORK_NUMBER, address );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/images/tokens/USDCx.svgis excluded by!**/*.svg
📒 Files selected for processing (16)
src/apollo/gql/gqlUser.ts(2 hunks)src/components/views/donate/Recurring/ModifySuperToken/DepositSuperToken.tsx(1 hunks)src/components/views/donate/Recurring/ModifySuperToken/ModifySection.tsx(3 hunks)src/components/views/donate/Recurring/ModifySuperToken/ModifySuperTokenModal.tsx(1 hunks)src/components/views/donate/Recurring/ModifySuperToken/WithDrawSuperToken.tsx(1 hunks)src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx(1 hunks)src/components/views/donate/Recurring/SelectTokenModal/StreamInfo.tsx(3 hunks)src/components/views/userProfile/donationsTab/recurringTab/ActiveProjectsSection.tsx(4 hunks)src/components/views/userProfile/donationsTab/recurringTab/ActiveStreamsSection.tsx(2 hunks)src/components/views/userProfile/donationsTab/recurringTab/FilterMenu.tsx(4 hunks)src/components/views/userProfile/donationsTab/recurringTab/RecurringDonationFiltersButton.tsx(2 hunks)src/components/views/userProfile/donationsTab/recurringTab/StreamRow.tsx(5 hunks)src/helpers/superfluid.ts(1 hunks)src/hooks/useUserStreams.ts(0 hunks)src/services/donation.ts(1 hunks)src/types/superFluid.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/hooks/useUserStreams.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/views/userProfile/donationsTab/recurringTab/StreamRow.tsx
[error] 145-153: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (22)
src/components/views/userProfile/donationsTab/recurringTab/StreamRow.tsx (6)
16-16: New import looks good.
Importing 'NetworkLogo' is straightforward and integrates well with the rest of the changes.
56-56: Duplicate concern regarding tokenStream usage.
Same issue as lines 49-50: referencing tokenStream[0]. Confirm non-empty array.
99-103: Network logo usage is correct.
Displaying the network logo with the recurringNetworkId aligns well with the multi-network approach.
170-173: Styling for NetworkLogoWrapper looks good.
The margin-left is a neat touch to space out the logo.
49-50: Ensure tokenStream is never empty.
Using tokenStream[0] could cause an out-of-bounds error if the array is empty. Verify if there's a guard in upstream code that ensures at least one element in tokenStream.
23-42: Consider a fallback approach for unrecognized networks.
When the networkId doesn't match Base or Optimism, the code defaults to an empty array. Confirm whether an empty array is acceptable behavior or if you should display a "network not supported" message.
Use this script to see if the codebase handles other networks gracefully:
✅ Verification successful
Let me analyze the codebase further to understand how unrecognized networks are handled.
Let me check how the application handles network validation in the broader context.
Let me check the configuration files to understand how networks are configured and handled.
Based on the analysis of the codebase, I can now provide a final response:
Empty array is the intended behavior for unsupported networks
The code explicitly handles only Base and Optimism networks for Superfluid tokens, returning an empty array for other networks. This is correct because:
- The configuration files show that only Base and Optimism networks have specific Superfluid token configurations
- The StreamRow component uses this empty array safely, as it's used to find matching tokens, and when none are found, it returns null
- The UI gracefully handles this case by not displaying invalid token options for unsupported networks
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching references to networkId in superfluid or other relevant places
rg 'networkId'
Length of output: 18492
Script:
#!/bin/bash
# Check how superfluid tokens are configured and if there's any error handling for unsupported networks
ast-grep --pattern 'SUPER_FLUID_TOKENS'
# Check if there are any error messages or handling related to unsupported networks
rg -A 2 "network.*not.*support"
Length of output: 73
Script:
#!/bin/bash
# Let's check the config files for network configurations and superfluid tokens
rg -A 5 "SUPER_FLUID_TOKENS.*="
# Also check for any network validation or error handling in the StreamRow component
ast-grep --pattern 'function $_(networkId$_) {
$$$
}'
Length of output: 725
Script:
#!/bin/bash
# Check for network validation or supported networks checks
rg -B 2 -A 4 "NETWORKS_CONFIG|supportedNetworks|acceptedNetworks"
# Check StreamRow component's complete implementation
rg -l "StreamRow" | xargs cat
Length of output: 49825
Script:
#!/bin/bash
# Check the configuration files for network handling
rg -A 5 "BASE_CONFIG|OPTIMISM_CONFIG" src/config/
# Check if there are any network validation or error handling components
rg -A 3 "network.*not.*supported|unsupported.*network" src/components/
Length of output: 1332
src/types/superFluid.ts (1)
27-27: Adding networkId is beneficial for multi-network support.
This field aligns with the rest of the changes enabling cross-network streams.
src/helpers/superfluid.ts (3)
5-8: Parameterizing the findTokenByAddress function is a solid improvement.
Accepting networkId allows more flexible handling of tokens across multiple networks.
20-25: Token search logic is clear and concise.
Finding either the super token or its underlying token by address is well-structured.
12-19: Confirm fallback for unknown networks.
Returning an empty array might be desired, but if there's any new chain introduced, you may need to update this logic.
✅ Verification successful
Let me analyze the references to networks in the codebase to understand if there are other networks that need to be considered.
Let me check how the Superfluid tokens are configured for other networks.
Based on the analysis of the codebase, I can now generate the final response:
Empty array fallback for unknown networks is intentional and safe.
The code correctly handles Superfluid tokens for Base and Optimism networks, which are the only networks where Superfluid functionality is implemented. The empty array fallback is appropriate as these are the only networks with configured Superfluid tokens in both production and development environments. No action needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching references to new networks introduced beyond Base and Optimism
rg 'networkId'
Length of output: 18492
Script:
#!/bin/bash
# Let's check the network configurations to see all supported networks
rg -A 5 'NETWORK_NUMBER' src/config/
Length of output: 16070
Script:
#!/bin/bash
# Let's check the Superfluid token configurations
rg -A 10 'SUPER_FLUID_TOKENS' src/
Length of output: 15387
src/components/views/userProfile/donationsTab/recurringTab/RecurringDonationFiltersButton.tsx (2)
25-26: Adding networkIds and setNetworkIds is logical.
This provides the necessary hooks for filtering donations by network.
69-70: Passing networkIds and setNetworkIds to FilterMenu is coherent.
Ensuring the filter component can properly manage the new state is a straightforward approach.
src/components/views/userProfile/donationsTab/recurringTab/ActiveStreamsSection.tsx (1)
55-61: LGTM! Network column header added correctly.
The network column header is properly internationalized and integrated into the table structure.
src/components/views/donate/Recurring/SelectTokenModal/StreamInfo.tsx (2)
14-14: LGTM! Optional network ID prop added correctly.
The recurringNetworkId prop is properly typed as optional, maintaining backward compatibility.
41-41: Verify network ID handling in findTokenByAddress.
The network ID is correctly passed to findTokenByAddress, but let's verify its implementation.
✅ Verification successful
Network ID handling in findTokenByAddress is properly implemented
The function correctly handles network IDs by:
- Accepting an optional network ID parameter with BASE_NETWORK_NUMBER as default
- Using the network ID to select the appropriate token configuration (BASE or OPTIMISM)
- Performing case-insensitive address matching against the selected network's tokens
- Checking both super token and underlying token addresses
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check findTokenByAddress implementation for network ID handling
ast-grep --pattern 'function findTokenByAddress($token, $network) {
$$$
}'
Length of output: 78
Script:
#!/bin/bash
# Let's try searching for findTokenByAddress with ripgrep to locate its definition
rg "findTokenByAddress" -A 5
Length of output: 4278
Script:
#!/bin/bash
# Let's examine the full implementation of findTokenByAddress in superfluid.ts
cat src/helpers/superfluid.ts
Length of output: 1169
src/components/views/donate/Recurring/ModifySuperToken/ModifySection.tsx (1)
34-34: LGTM! Required network ID prop added correctly.
The recurringNetworkID prop is properly typed as required, ensuring network context is always available.
src/apollo/gql/gqlUser.ts (1)
Line range hint 128-140: LGTM! Verify query usage across the codebase.
The addition of the optional networkId parameter to the GraphQL query is well-implemented and maintains backward compatibility.
Let's verify the query usage across the codebase:
✅ Verification successful
The networkId parameter needs to be added to the query usage in ActiveProjectsSection
The query is used in src/components/views/userProfile/donationsTab/recurringTab/ActiveProjectsSection.tsx but doesn't include the new networkId parameter in its variables. Let's verify the full component implementation:
The networkId parameter is properly handled in ActiveProjectsSection
The code review concern has been resolved. The ActiveProjectsSection component already handles the networkId parameter correctly in the query variables:
networkId: networkIds.length === 1 ? networkIds[0] : 0,The implementation:
- Maintains a
networkIdsstate array - Properly passes a single network ID when one is selected
- Falls back to 0 when multiple networks are selected
- Re-fetches data when
networkIdschanges (included in the useEffect dependencies)
The usage aligns with the GraphQL query modification and no further changes are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for all usages of FETCH_USER_RECURRING_DONATIONS to ensure they handle the new networkId parameter
rg -l "FETCH_USER_RECURRING_DONATIONS" | xargs rg -A 10 "FETCH_USER_RECURRING_DONATIONS.*variables"
Length of output: 99
Script:
#!/bin/bash
# Let's try a different approach to find usages of the query
rg -A 5 "FETCH_USER_RECURRING_DONATIONS" --type ts
Length of output: 2027
Script:
#!/bin/bash
# Also search for any React components or hooks that might be using this query
rg -A 5 "useQuery.*FETCH_USER_RECURRING_DONATIONS" --type ts
Length of output: 62
Script:
#!/bin/bash
# Look for the query definition itself
ast-grep --pattern 'const FETCH_USER_RECURRING_DONATIONS = gql`$$$`'
Length of output: 2492
Script:
#!/bin/bash
# Get the full implementation of ActiveProjectsSection to see the query usage in context
cat src/components/views/userProfile/donationsTab/recurringTab/ActiveProjectsSection.tsx
Length of output: 5534
src/components/views/userProfile/donationsTab/recurringTab/ActiveProjectsSection.tsx (1)
129-130: LGTM! Props are correctly passed to RecurringDonationFiltersButton.
The network filtering props are properly passed to the filter component.
src/components/views/donate/Recurring/ModifySuperToken/WithDrawSuperToken.tsx (2)
157-157: LGTM: Network ID integration for ModifySection
The addition of recurringNetworkID prop enhances network-specific token handling.
Line range hint 19-20: Consider adding network validation in the balance calculation
The balance modification logic should validate the network context to prevent potential issues with cross-network operations.
src/components/views/donate/Recurring/ModifySuperToken/DepositSuperToken.tsx (1)
187-187: LGTM: Network ID integration for deposit operations
The addition of recurringNetworkID prop maintains consistency with withdrawal functionality.
src/components/views/donate/Recurring/SelectTokenModal/SelectTokenModal.tsx (1)
144-144: LGTM: Network ID integration for StreamInfo
The addition of recurringNetworkId prop enables network-specific stream information display.
src/components/views/donate/Recurring/ModifySuperToken/ModifySuperTokenModal.tsx
Show resolved
Hide resolved
src/components/views/userProfile/donationsTab/recurringTab/ActiveProjectsSection.tsx
Show resolved
Hide resolved
|
@kkatusic don't forget to merge develop into the testing branch for recurring donations after this PR merges https://github.com/Giveth/giveth-dapps-v2/tree/test-recurring-donations |
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements
Interface Changes