-
-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Recurring donations setup on Base #4889
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces several enhancements to the GraphQL queries and mutations across multiple files, primarily focusing on the addition of the Changes
Possibly related PRs
Suggested reviewers
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 (
|
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (4)
src/components/views/create/AddressInterface.tsx (1)
65-68
: Consider renaming variables to improve clarity between current and target chainsThe variables
isOnOptimism
andisOptimism
are similar but represent different contexts—the current chain and the target chain, respectively. Renaming them can enhance code readability and prevent confusion. For example:
- Rename
isOnOptimism
toisCurrentChainOptimism
- Rename
isOptimism
toisTargetChainOptimism
Also applies to: 76-77
src/components/views/create/AlloProtocol/AlloProtocolModal.tsx (1)
34-88
: Refactor to eliminate code duplication betweensaveAnchorContract
andhandleButtonClick
The
saveAnchorContract
function and the logic insidehandleButtonClick
perform similar operations, leading to code duplication. RefactoringhandleButtonClick
to utilizesaveAnchorContract
will improve maintainability and reduce redundancy.Apply this diff to refactor
handleButtonClick
:const handleButtonClick = async () => { if (!isOnOptimism) { switchChain?.({ chainId: config.OPTIMISM_NETWORK_NUMBER }); } else { setIsLoading(true); try { + await saveAnchorContract({ + addedProjectState, + chainId: config.OPTIMISM_NETWORK_NUMBER, + recipientAddress: addedProjectState?.adminUser?.walletAddress, + }); if (!isEditMode || (isEditMode && isDraft)) { await router.push( slugToSuccessView(addedProjectState.slug), ); } else { await router.push( slugToProjectView(addedProjectState.slug), ); } setShowModal(false); // Close the modal } catch (error) { console.error('Error saving anchor contract', error); } finally { setIsLoading(false); } } };This ensures that all logic related to saving the anchor contract is centralized within
saveAnchorContract
, enhancing code reuse and clarity.src/types/config.ts (1)
175-177
: Consider adding JSDoc documentation for the new interface.The
BaseNetworkConfig
interface is well-structured and properly extendsNetworkConfig
. Consider adding JSDoc documentation to explain the purpose ofanchorRegistryAddress
and its significance for Base network configuration.+/** + * Configuration specific to Base network, extending the base NetworkConfig + * @property anchorRegistryAddress - The address of the anchor registry contract on Base + */ export interface BaseNetworkConfig extends NetworkConfig { anchorRegistryAddress: Address; }src/components/views/create/CreateProject.tsx (1)
63-63
: Document the purpose of STOP_RECURRING_SETUP_ON_CREATIONConsider adding a JSDoc comment explaining the purpose and implications of this flag, as it controls important functionality.
+/** + * When true, prevents the setup of recurring donations during project creation. + * This flag controls whether the AlloProtocolModal is shown after project creation. + */ export const STOP_RECURRING_SETUP_ON_CREATION = true;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
src/apollo/gql/gqlProjects.ts
(4 hunks)src/apollo/gql/gqlSuperfluid.ts
(1 hunks)src/apollo/gql/gqlUser.ts
(1 hunks)src/apollo/types/types.ts
(1 hunks)src/components/views/create/AddressInterface.tsx
(8 hunks)src/components/views/create/AlloProtocol/AlloProtocolModal.tsx
(1 hunks)src/components/views/create/CreateProject.tsx
(5 hunks)src/config/development.tsx
(1 hunks)src/config/production.tsx
(1 hunks)src/types/config.ts
(2 hunks)
🔇 Additional comments (11)
src/apollo/gql/gqlSuperfluid.ts (1)
58-65
: LGTM!
The addition of the optional $recipientAddress
parameter to the CREATE_ANCHOR_CONTRACT_ADDRESS_QUERY
mutation is correctly implemented, and the variable is properly passed in the mutation.
src/apollo/gql/gqlUser.ts (1)
157-157
: LGTM!
Adding the networkId
field to the anchorContracts
within the FETCH_USER_RECURRING_DONATIONS
query enriches the data with necessary network context.
src/types/config.ts (1)
229-229
: LGTM!
The type change for BASE_CONFIG
to BaseNetworkConfig
ensures proper type safety and consistency.
src/apollo/types/types.ts (1)
35-35
: Verify handling of existing anchor contract data.
The addition of networkId
to IAnchorContractData
is correct. However, we should verify how existing anchor contract data will be handled.
✅ Verification successful
Based on the search results, I can now generate the final response:
Existing anchor contract data will be handled correctly
The addition of networkId
to IAnchorContractData
is safe because:
- All existing usages properly handle optional anchor contracts with null checks
- The interface is used consistently across the codebase with proper type safety
- The
findAnchorContractAddress
helper function has null-safe implementation - GraphQL queries already include the full anchor contract data structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing anchor contract implementations and usages
# to ensure backward compatibility
# Search for anchor contract implementations
echo "Searching for anchor contract implementations..."
ast-grep --pattern 'interface IAnchorContractData {
$$$
}'
# Search for anchor contract usages
echo "Searching for anchor contract usages..."
rg -A 5 'anchorContracts.*{'
Length of output: 634
Script:
#!/bin/bash
# Let's search for usages of IAnchorContractData and anchor contracts with better patterns
# Search for IAnchorContractData usages
echo "Searching for IAnchorContractData usages..."
rg "IAnchorContractData" -B 2 -A 2
# Search for anchor contract related code
echo "Searching for anchor contract related code..."
rg "anchorContracts?" -B 2 -A 2
# Search for any mutations or queries that might handle anchor contract data
echo "Searching for GraphQL operations..."
rg "(mutation|query).*[Aa]nchor" -B 2 -A 2
Length of output: 24177
src/apollo/gql/gqlProjects.ts (3)
58-58
: LGTM! Consistent addition of networkId field.
The addition of networkId
to the anchorContracts
fragment in PROJECT_CARD_FIELDS
is correct and will be reused across multiple queries.
200-200
: LGTM! Consistent networkId field across queries.
The addition of networkId
to anchorContracts
is consistently implemented across all relevant queries:
- FETCH_PROJECT_BY_SLUG_DONATION
- FETCH_PROJECT_BY_SLUG_SINGLE_PROJECT
- FETCH_PROJECT_BY_ID
Also applies to: 299-299, 338-338
Line range hint 1-1
: Verify complete coverage of networkId field.
Let's ensure all necessary queries include the networkId
field.
✅ Verification successful
Let me try a different approach to verify the GraphQL queries.
Let me try one more approach to verify the GraphQL queries.
Based on the content of gqlProjects.ts file, I can now generate the final response:
All GraphQL queries consistently include networkId field ✓
After reviewing the file content, I can confirm that the networkId
field is consistently included in all relevant GraphQL queries and fragments where anchorContracts
is used:
- In
PROJECT_CARD_FIELDS
fragment - In
FETCH_PROJECT_BY_SLUG_DONATION
query - In
FETCH_PROJECT_BY_SLUG_SINGLE_PROJECT
query - In
FETCH_PROJECT_BY_ID
query - In
FETCH_GIVETH_PROJECT_BY_ID
query
The field is also properly included in mutations where address-related data is handled, such as ADD_RECIPIENT_ADDRESS_TO_PROJECT
.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any missing networkId fields in GraphQL queries
# Search for GraphQL queries with anchorContracts
echo "Searching for GraphQL queries with anchorContracts..."
rg -l 'anchorContracts.*{[^}]*}'
# Search for potential missing networkId fields
echo "Searching for potential queries missing networkId..."
rg -l 'anchorContracts.*{[^}]*}' | xargs rg -L 'networkId'
Length of output: 15945
Script:
#!/bin/bash
# Search for GraphQL queries in gqlProjects.ts and check networkId field
echo "Checking GraphQL queries in gqlProjects.ts..."
ast-grep --pattern 'gql`
$$$
{
$$$
}
`' src/apollo/gql/gqlProjects.ts
# Also check for mutations
echo -e "\nChecking GraphQL mutations in gqlProjects.ts..."
ast-grep --pattern 'mutation $$$' src/apollo/gql/gqlProjects.ts
Length of output: 359
Script:
#!/bin/bash
# Search for content in gqlProjects.ts file
echo "Content of gqlProjects.ts:"
cat src/apollo/gql/gqlProjects.ts
# Search for anchorContracts fields
echo -e "\nSearching for anchorContracts fields:"
rg "anchorContracts\s*{[^}]*}" src/apollo/gql/gqlProjects.ts
Length of output: 13278
src/config/development.tsx (1)
426-426
: Verify the use of the same anchorRegistryAddress
across networks
The same address (0x4AAcca72145e1dF2aeC137E1f3C5E3D75DB8b5f3
) is used for both BASE_CONFIG and OPTIMISM_CONFIG. While this might be intentional for the development environment, it's worth confirming.
✅ Verification successful
Using the same anchorRegistryAddress
across networks is correct
The same anchorRegistryAddress
is consistently used across both development and production environments for different networks. This is the expected behavior as the Anchor Registry is a cross-chain contract that needs to be accessible from all networks. The address 0x4AAcca72145e1dF2aeC137E1f3C5E3D75DB8b5f3
is correctly used in:
- Production config for both networks
- Development config for both networks
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the anchorRegistryAddress is consistently used across different environments and networks
# Search for all occurrences of the address
echo "Checking usage of anchorRegistryAddress across files:"
rg -i "0x4AAcca72145e1dF2aeC137E1f3C5E3D75DB8b5f3"
Length of output: 557
Script:
#!/bin/bash
# Let's check the context around these addresses to understand the network configurations better
echo "Checking the context of anchorRegistryAddress in production.tsx:"
rg -B 2 -A 2 "anchorRegistryAddress.*0x4AAcca72145e1dF2aeC137E1f3C5E3D75DB8b5f3" src/config/production.tsx
echo -e "\nChecking the context of anchorRegistryAddress in development.tsx:"
rg -B 2 -A 2 "anchorRegistryAddress.*0x4AAcca72145e1dF2aeC137E1f3C5E3D75DB8b5f3" src/config/development.tsx
Length of output: 1263
src/components/views/create/CreateProject.tsx (2)
555-561
: LGTM! Improved anchor contract lookup
The change from direct access to using find()
with network ID matching is a more robust approach for handling anchor contracts.
410-415
: Verify the impact of disabling recurring donations
The condition effectively prevents the AlloProtocolModal from showing up during project creation. Ensure this is the intended behavior and won't affect existing projects.
✅ Verification successful
Let me gather more information about the STOP_RECURRING_SETUP_ON_CREATION
constant and its usage.
Based on the code analysis, I can now generate the final response since I have enough information to verify the impact of disabling recurring donations.
Disabling recurring donations setup during project creation is safe
The STOP_RECURRING_SETUP_ON_CREATION
flag only affects the display of the AlloProtocolModal during project creation. Existing projects and their recurring donation functionality remain unaffected because:
- The flag is only used in the project creation flow (
CreateProject.tsx
andAddressInterface.tsx
) - The recurring donation setup for existing projects is handled separately through
RecurringDonationCard.tsx
which has its own modal control logic - The core recurring donation functionality (creation, updates, termination) is independent of this UI flag
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for dependencies on AlloProtocolModal across the codebase
# Search for AlloProtocolModal usage
echo "Checking AlloProtocolModal usage:"
rg "AlloProtocolModal"
# Search for recurring donation setup references
echo -e "\nChecking recurring donation setup references:"
rg -i "recurring.*donation.*setup"
Length of output: 3426
Script:
#!/bin/bash
# Search for STOP_RECURRING_SETUP_ON_CREATION constant
echo "Checking STOP_RECURRING_SETUP_ON_CREATION usage:"
rg "STOP_RECURRING_SETUP_ON_CREATION"
# Search for any environment variable related to this
echo -e "\nChecking environment files for recurring setup configuration:"
fd -e env -e env.example -x cat {}
# Check for any configuration files that might contain this setting
echo -e "\nChecking configuration files:"
fd -e config -e json -e js -e ts --exclude '*.d.ts' -x cat {}
Length of output: 66154
src/config/production.tsx (1)
599-599
: Verify the production anchor registry address
The same anchorRegistryAddress
(0x4AAcca72145e1dF2aeC137E1f3C5E3D75DB8b5f3
) is used in both development and production environments. Please confirm this is the correct production address for Base network.
onClick={async () => { | ||
if (!project) return; | ||
if ( | ||
isRecurringOnOptimismReady && | ||
!isOnOptimism | ||
) { | ||
switchChain?.({ | ||
chainId: | ||
config.OPTIMISM_NETWORK_NUMBER, | ||
}); | ||
} else if ( | ||
isRecurringOnBaseReady && | ||
!isOnBase | ||
) { | ||
switchChain?.({ | ||
chainId: | ||
config.BASE_NETWORK_NUMBER, | ||
}); | ||
} | ||
|
||
await saveAnchorContract({ | ||
addedProjectState: project, | ||
chainId: networkId, | ||
recipientAddress: | ||
walletAddress || value, | ||
}); | ||
setHasAnchorContract(true); | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure chain switching completes before proceeding to save the anchor contract
The switchChain
function is asynchronous and returns a Promise. You should await
this function to ensure the chain switch has completed before calling saveAnchorContract
. This prevents potential issues where the contract might be saved on the wrong network.
Apply this diff to fix the issue:
if (
isRecurringOnOptimismReady &&
!isOnOptimism
) {
- switchChain?.({
+ await switchChain?.({
chainId:
config.OPTIMISM_NETWORK_NUMBER,
});
} else if (
isRecurringOnBaseReady &&
!isOnBase
) {
- switchChain?.({
+ await switchChain?.({
chainId:
config.BASE_NETWORK_NUMBER,
});
}
await saveAnchorContract({
addedProjectState: project,
chainId: networkId,
recipientAddress:
walletAddress || value,
});
Additionally, consider adding error handling around the switchChain
and saveAnchorContract
calls to manage any potential errors gracefully.
📝 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.
onClick={async () => { | |
if (!project) return; | |
if ( | |
isRecurringOnOptimismReady && | |
!isOnOptimism | |
) { | |
switchChain?.({ | |
chainId: | |
config.OPTIMISM_NETWORK_NUMBER, | |
}); | |
} else if ( | |
isRecurringOnBaseReady && | |
!isOnBase | |
) { | |
switchChain?.({ | |
chainId: | |
config.BASE_NETWORK_NUMBER, | |
}); | |
} | |
await saveAnchorContract({ | |
addedProjectState: project, | |
chainId: networkId, | |
recipientAddress: | |
walletAddress || value, | |
}); | |
setHasAnchorContract(true); | |
}} | |
onClick={async () => { | |
if (!project) return; | |
if ( | |
isRecurringOnOptimismReady && | |
!isOnOptimism | |
) { | |
await switchChain?.({ | |
chainId: | |
config.OPTIMISM_NETWORK_NUMBER, | |
}); | |
} else if ( | |
isRecurringOnBaseReady && | |
!isOnBase | |
) { | |
await switchChain?.({ | |
chainId: | |
config.BASE_NETWORK_NUMBER, | |
}); | |
} | |
await saveAnchorContract({ | |
addedProjectState: project, | |
chainId: networkId, | |
recipientAddress: | |
walletAddress || value, | |
}); | |
setHasAnchorContract(true); | |
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ;) , thx @mateodaza
Summary by CodeRabbit
New Features
networkId
,categories
,organization
, andsocialMedia
.Bug Fixes
Chores
anchorRegistryAddress
property in both development and production settings.