Merge master into staging #1807
Conversation
Next Release Aug 2024
* fix: remove memo for project verification managing funds * fix: remove memo for project verification managing funds * fix: getDraftDonationById bug (toWalletMemo can be null) * fix: add memo for stellar project address uniqueness * fix: add memo for manage address validation * fix: add duplicate address error message for stellar * fix: linter error * add index for project stellar address * eslint error * fix: case when owner donate to his own peoject (Stellar chain) * fix: add calculateGivbackFactor to Stellar cron job * feat: register secondary donation * running migration to set project banners appropriately for endaoment … (#1778) * running migration to set project banners appropriately for endaoment projects * chore: correcting tab spaces for syntax * fix: linter errors * Modify add banner to endaoment projects migration (#1791) related to #1600 * Fix lint errors * Fix running tests * Fix projectResolver test cases * Fix donationResolver test cases * skip should renew the expiration date of the draft donation test case --------- Co-authored-by: Hrithik Sampson <[email protected]> Co-authored-by: mohammadranjbarz <[email protected]> * fix: remove adding secondary donation logic --------- Co-authored-by: Meriem-BM <[email protected]> Co-authored-by: HrithikSampson <[email protected]> Co-authored-by: Hrithik Sampson <[email protected]>
WalkthroughThis pull request introduces multiple database migrations and code modifications aimed at enhancing project and donation management functionalities. Key changes include a migration for updating project records with banner images, enforcing unique constraints on project addresses for Stellar, and refining donation-related logic to accommodate new parameters. Additionally, modifications to GraphQL schemas and validation logic improve flexibility and clarity in handling wallet addresses and donation processes. Changes
Possibly related PRs
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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: 0
Outside diff range, codebase verification and nitpick comments (1)
src/resolvers/projectResolver.test.ts (1)
Line range hint
217-264: Fix test isolation issue so that all tests can run reliably together.The test cases
should return all tokens for giveth projectsandshould return all tokens for trace projectshave been skipped because they fail when run together with other tests due to database state changes.This indicates an issue with test isolation where tests are impacting the state/data used by other tests. Each test should create its own isolated data and not rely on or impact the data from other tests.
To fix this:
- Ensure each test creates its own project, tokens and any other required data.
- Clean up the data created by the test in the
afterEachhook.- Don't rely on data created outside the test or by other tests.
- Use database transactions so that changes from one test don't impact other tests.
After fixing the test isolation, remove the
it.skipand ensure the tests pass reliably.
* WIP Implement allocatedGivbacks function related to Giveth/giveth-dapps-v2#4678 Giveth/giveth-dapps-v2#4679 * allocatedGivbacks() endpoint implemented and works related to Giveth/giveth-dapps-v2#4678 Giveth/giveth-dapps-v2#4679 * Fix allocatedGivbacksQuery test cases
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
src/repositories/donationRepository.ts (1)
Line range hint
89-147: LGTM!The addition of the new optional parameters (
givbackFactor,projectRank,bottomRankInRound, andpowerRound) to thecreateDonationfunction enhances its capability to handle more complex donation scenarios. ThegivbackFactorparameter allows for specifying a custom Givback factor for each donation, enabling more flexible Givback calculations. TheprojectRank,bottomRankInRound, andpowerRoundparameters provide additional context about the project's ranking and the associated PowerRound, which can be useful for further analysis and processing. The function update is well-implemented, with the new parameters being destructured from thedataobject and included in theDonation.createmethod call.To improve the code documentation and maintainability, consider adding JSDoc comments to describe the purpose and usage of the new parameters. This will provide clarity for future developers working with the
createDonationfunction.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- src/repositories/donationRepository.test.ts (3 hunks)
- src/repositories/donationRepository.ts (5 hunks)
- src/resolvers/donationResolver.test.ts (4 hunks)
- src/resolvers/donationResolver.ts (3 hunks)
- src/resolvers/projectResolver.ts (2 hunks)
- src/utils/tokenUtils.ts (2 hunks)
- test/graphqlQueries.ts (1 hunks)
- test/testUtils.ts (2 hunks)
Additional comments not posted (12)
src/utils/tokenUtils.ts (1)
Line range hint
4-35: LGTM!The addition of the optional
coingeckoIdparameter and property in thefindTokenByNetworkAndSymbolfunction provides more flexibility in handling token information. The change is implemented correctly and does not introduce any breaking changes or potential issues.src/repositories/donationRepository.ts (1)
485-523: LGTM!The
getSumOfGivbackEligibleDonationsForSpecificRoundfunction is well-implemented and introduces a useful capability to calculate the total USD value of Givback-eligible donations for a specific PowerRound. The function handles the case when nopowerRoundparameter is provided by retrieving the latest PowerRound from the database. It executes a raw SQL query using theAppDataSourceto efficiently calculate the sum of donation values that meet the specified criteria, including donation status, project verification, and the specifiedpowerRound. The query also excludes donations made from project recipient addresses to avoid counting self-donations. Error handling is in place to catch any exceptions, log them, and return a default value of 0 to prevent disruption. Overall, the function is a valuable addition to the codebase.src/resolvers/donationResolver.ts (4)
80-93: LGTM!The
AllocatedGivbacksclass definition follows the GraphQL type-graphql library conventions. The field names are descriptive and the types are appropriate.
95-96: LGTM!The
allocatedGivbacksCachevariable declaration is appropriate for caching theAllocatedGivbacksobject. Initializing it tonullindicates the cache is empty.
451-493: LGTM!The
allocatedGivbacksquery in theDonationResolverclass is well-structured and follows the GraphQL resolver conventions. The caching mechanism is implemented correctly to avoid unnecessary computations. The calculation logic for allocated GIV tokens is accurate and the cache expiration time of one hour seems reasonable.
Line range hint
54-76: LGTM!The new import statements for
getSumOfGivbackEligibleDonationsForSpecificRound,getTokenPrice, andfindTokenByNetworkAndSymbolare correctly added. They follow the proper syntax and naming conventions.test/graphqlQueries.ts (1)
2252-2265: LGTM!The addition of the
allocatedGivbacksQuerylooks good. It provides a useful way for clients to retrieve data related to allocated givbacks. The$refreshCacheparameter adds flexibility to control data freshness.src/repositories/donationRepository.test.ts (1)
1466-1824: LGTM!The test cases for
getSumOfGivbackEligibleDonationsForSpecificRoundare comprehensive and well-structured. They cover various important scenarios and use appropriate assertions to verify the expected behavior. The test cases also handle test data setup and cleanup properly.test/testUtils.ts (1)
276-284: ImprovedchainTypedetermination based onnetworkId.The changes to the
saveProjectDirectlyToDbfunction enhance its ability to correctly identify the blockchain type (chainType) associated with the givennetworkId. By checking if thenetworkIdis part of theSolanaNetworkIdsarray, the function can now accurately set thechainTypeto eitherChainType.SOLANAorChainType.EVM. This improves the function's flexibility and eliminates the previous hardcoded assignment ofChainType.EVM.src/resolvers/projectResolver.ts (1)
1789-1795: LGTM!The addition of
chainTypeandmemoparameters to thewalletAddressIsValidquery method enhances its flexibility by allowing validation based on the blockchain type and supporting an optional memoization feature. The explicit typing ofchainTypeasChainTypeenum improves type safety. Making thememoparameter nullable provides flexibility in specifying the memo.These changes align with the AI-generated summary and improve the functionality of the wallet address validation.
src/resolvers/donationResolver.test.ts (2)
4972-5164: LGTM!The test suite
allocatedGivbacksQueryTestCasesis well-structured and enhances the test coverage for theallocatedGivbacksQueryfunctionality. The test cases effectively validate the calculation of allocated givbacks based on different donation scenarios and the behavior when therefreshCachevariable is passed.
Line range hint
5166-5202: LGTM!The test suite
getDonationByIdTestCaseseffectively validates thegetDonationByIdquery. It covers the basic functionality of retrieving a donation by its ID and handles the scenario when the donation is not found. The test cases ensure the correctness of the query's behavior.
| @@ -1255,7 +1255,7 @@ export class ProjectResolver { | |||
| ); | |||
| } | |||
|
|
|||
| await validateProjectWalletAddress(address, projectId, chainType); | |||
| await validateProjectWalletAddress(address, projectId, chainType, memo); | |||
There was a problem hiding this comment.
Tip
Codebase Verification
Function signature change verified, but test cases may need updating
The validateProjectWalletAddress function signature has been successfully updated to include the new memo parameter. The function definition and usage in src/resolvers/projectResolver.ts correctly incorporate this change.
However, the test cases in src/utils/validators/projectValidator.test.ts do not include the memo parameter in their function calls. These test cases should be reviewed and potentially updated to ensure they cover the new parameter:
- Lines 101, 112, 121, 127, 130, 136, 142, 151, and 158 in
src/utils/validators/projectValidator.test.ts
Please review these test cases and update them as necessary to maintain comprehensive test coverage for the validateProjectWalletAddress function.
Analysis chain
Verify the function signature change in the codebase.
The validateProjectWalletAddress function is now being invoked with an additional memo parameter. Ensure that the function definition has been updated to accept this new parameter and that all other invocations of this function have been updated to pass the memo argument.
Run the following script to verify the function usage:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all invocations of `validateProjectWalletAddress` match the new signature.
# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type typescript -A 5 $'validateProjectWalletAddress'
Length of output: 91
Script:
#!/bin/bash
# Description: Search for validateProjectWalletAddress function usage in TypeScript files
# Find TypeScript files and search for validateProjectWalletAddress
fd -e ts -e tsx | xargs grep -n -C 3 "validateProjectWalletAddress"
Length of output: 8994
…dmin Release 17.1
* Change compare amount logic, in draft donation service related to #1819 https://github.com/GeneralMagicio/QAcc-BE/blob/809fa8e57f6cc881acfdf0c387a0a0a8ceebaeae/src/services/chains/index.ts#L105 * Fix test cases
Add stellar into qf round
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
src/server/adminJs/tabs/qfRoundTab.ts (1)
Line range hint
202-228: Consider refactoring network configuration for better maintainability.The
availableNetworkValuesarray is used for multiple properties and might grow further as more networks are added. To improve maintainability and reduce duplication, consider the following refactoring suggestion:
- Extract the
availableNetworkValuesarray to a separate configuration file, e.g.,networkConfig.ts.- Import and use this configuration in the current file and any other relevant parts of the application.
Example implementation:
// networkConfig.ts import { NETWORK_IDS } from '../../../provider'; export const availableNetworkValues = [ { value: NETWORK_IDS.MAIN_NET, label: 'MAINNET' }, // ... other network entries ... { value: NETWORK_IDS.STELLAR_MAINNET, label: 'STELLAR MAINNET'}, // ... remaining network entries ... ]; // qfRoundTab.ts import { availableNetworkValues } from './networkConfig'; // Use availableNetworkValues in your configurationThis refactoring will centralize network configuration, making it easier to maintain and update across the application.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/server/adminJs/tabs/qfRoundTab.ts (1 hunks)
Additional comments not posted (1)
src/server/adminJs/tabs/qfRoundTab.ts (1)
214-214: LGTM! Verify related components for Stellar Mainnet support.The addition of Stellar Mainnet to the
availableNetworkValuesarray is implemented correctly and consistently with other network entries. This expands the options forallocatedTokenChainIdandeligibleNetworksin QF round configurations.To ensure full support for Stellar Mainnet throughout the application, please verify:
- Wallet connection support for Stellar
- Transaction processing for Stellar network
- Any network-specific logic in other parts of the application
Run the following script to check for Stellar-related implementations:
This script will help identify areas that may need updates to fully support Stellar Mainnet operations.
fix eslint error
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests