-
Notifications
You must be signed in to change notification settings - Fork 18
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
Implement allocatedGivbacks function #1808
Implement allocatedGivbacks function #1808
Conversation
WalkthroughThis pull request introduces enhancements to the donation management system, including a new function for calculating Givback-eligible donations and corresponding tests to validate functionality. It also adds a GraphQL query for retrieving allocated Givbacks, updates the ProjectResolver for type safety, and modifies utility functions to support additional parameters. Overall, these changes improve the structure and robustness of the donation-related functionalities in the application. Changes
Possibly related PRs
Poem
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: 0
Outside diff range and nitpick comments (3)
src/repositories/donationRepository.ts (1)
485-523
: LGTM! The function is well-structured and efficiently calculates the sum of Givback-eligible donations.The function follows a clear and logical flow, correctly handles the case when no
powerRound
is provided, and includes appropriate error handling.A few additional suggestions for improvement:
- Consider adding JSDoc comments to document the function's purpose, parameters, and return value. This will enhance the code's readability and maintainability.
- If the
getPowerRound
function is only used within this function, you can consider inlining it to avoid an additional function call.- Consider extracting the raw SQL query into a separate constant or configuration file to improve code organization and reusability.
Overall, the function is well-implemented and ready to be merged.
src/resolvers/donationResolver.ts (1)
451-493
: LGTM, but consider the following improvements:
- Consider breaking down the
allocatedGivbacks
function into smaller, more focused functions to improve readability and maintainability.- Consider extracting the caching logic into a separate utility function to make it more reusable across the codebase.
- Replace the magic number
1_000_000
with a constant variable with a descriptive name to improve code clarity.- Consider moving the cache timeout of 1 hour to a configuration file to make it more easily adjustable.
test/testUtils.ts (1)
1955-1957
: The new optional properties inCreateDonationData
interface look good, but clarify their purpose.The additions of
powerRound
,givbackFactor
andisProjectVerified
as optional properties to theCreateDonationData
interface are fine and unlikely to cause any issues since they are optional.However, to better understand the feature these properties are intended to support, could you please provide some more context on:
- What is the purpose and expected usage of these new properties?
- How do
powerRound
andgivbackFactor
factor into the donation flow or calculations?- What's the significance of the
isProjectVerified
flag and how is it used?Some code comments explaining these would also be helpful for future reference and maintainability.
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 (2 hunks)
- src/resolvers/donationResolver.test.ts (3 hunks)
- src/resolvers/donationResolver.ts (3 hunks)
- src/resolvers/projectResolver.ts (1 hunks)
- src/utils/tokenUtils.ts (2 hunks)
- test/graphqlQueries.ts (1 hunks)
- test/testUtils.ts (1 hunks)
Additional comments not posted (22)
src/utils/tokenUtils.ts (1)
11-11
: LGTM!The changes to the
findTokenByNetworkAndSymbol
function are approved:
- The addition of the optional
coingeckoId
parameter provides flexibility in token identification.- The return object has been correctly updated to include the
coingeckoId
property.The changes modify the function's interface, impacting how it can be called and what data it can return, thereby altering its overall functionality. However, the changes are correctly implemented and there are no issues.
Also applies to: 31-31
src/resolvers/donationResolver.ts (1)
80-93
: LGTM!The
AllocatedGivbacks
class is correctly defined with appropriate fields and types.test/graphqlQueries.ts (1)
2252-2265
: LGTM!The new
allocatedGivbacksQuery
looks good:
- The query name is descriptive and follows the naming convention.
- The
$refreshCache
argument is clearly named and typed.- The response structure is well-defined.
Great job!
src/repositories/donationRepository.test.ts (1)
1466-1824
: LGTM!The test suite for
getSumOfGivbackEligibleDonationsForSpecificRound
is comprehensive and well-structured. It covers important scenarios and edge cases, ensuring the correctness of the function's behavior.The code changes are approved.
src/resolvers/projectResolver.ts (1)
1791-1792
: LGTM!The changes to explicitly set the type of
chainType
using a lambda function are approved. This enhances clarity, type safety and readability.src/resolvers/donationResolver.test.ts (17)
Line range hint
113-145
: LGTM!The test cases for
totalDonationsPerCategory
query look good. They cover the important scenarios and make the necessary assertions.
Line range hint
147-188
: LGTM!The test cases for
totalDonationsNumberPerDate
query look good. They cover the important scenarios and make the necessary assertions.
Line range hint
190-265
: LGTM!The test cases for
totalDonorsCountPerDate
query look good. They cover the important scenarios like invalid date formats, unique donor count calculation, and handling of anonymous donations. The assertions verify the expected behavior.
Line range hint
267-305
: LGTM!The test case for
newDonorsCountPerDate
andnewDonorsDonationTotalUsdPerDate
queries looks good. It sets up the necessary test data and verifies the count of new donors and their total donation amount as expected.
Line range hint
307-449
: LGTM!The test cases for
doesDonatedToProjectInQfRound
query look good. They cover scenarios like verified donation, non-verified donation, and invalid project ID, QF round ID, or user ID. The assertions verify the expected behavior in each case.
Line range hint
451-494
: LGTM!The test case for
donationsTotalUsdPerDate
query looks good. It verifies the total USD amount of donations with different date ranges and theonlyVerified
flag. The assertions check the results against the expected values.
Line range hint
496-620
: LGTM!The test cases for
donations
query look comprehensive. They cover scenarios like invalid date formats, filtering by date ranges, inclusion of project categories, and filtering by project status. The assertions verify the expected behavior in each case.
Line range hint
622-2480
: LGTM!The test cases for
createDonation
mutation are very comprehensive. They cover a wide range of scenarios, including donating to own project, successful donation, referral handling, QF round association, different organizations and networks, invalid inputs, and more. The assertions thoroughly verify the expected behavior in each case.
Line range hint
2482-2558
: LGTM!The test cases for
donationsFromWallets
query look good. They verify the retrieval of donations based on thefromWalletAddresses
filter, including case-insensitive matching and handling of non-existent addresses. The assertions check the expected behavior.
Line range hint
2560-2930
: LGTM!The test cases for
donationsByProjectId
query are comprehensive. They cover scenarios like filtering by QF round, sorting by various fields, searching by user name and donation details, and filtering by donation status. The assertions verify the expected behavior in each case.
Line range hint
2932-3269
: LGTM!The test cases for
donationsByUserId
query are comprehensive. They cover scenarios like sorting by various fields, handling of anonymous donations, filtering by donation status, and pagination. The assertions verify the expected behavior in each case.
Line range hint
3271-3322
: LGTM!The test cases for
donationsByDonor
query look good. They verify the retrieval of donations made by the authenticated user and handle the case when the user is not logged in. The assertions check the expected behavior.
Line range hint
3324-3400
: LGTM!The test cases for
donationsToWallets
query look good. They verify the retrieval of donations based on thetoWalletAddresses
filter, including case-insensitive matching and handling of non-existent addresses. The assertions check the expected behavior.
Line range hint
4402-4451
: LGTM!The test case for
recentDonations
query looks good. It verifies the retrieval of the most recent donations limited by thetake
parameter. The assertions check the expected behavior.
Line range hint
4453-4513
: LGTM!The test case for
donationMetrics
query looks good. It sets up the necessary test data with donations to different projects and verifies the calculated metrics like total donations, USD value, and average percentage against the expected values.
4972-5164
: LGTM!The test cases for
allocatedGivbacks
query look good. They set up the necessary test data with donations having different USD values and Givback factors. The assertions verify the calculated allocated Givbacks against the expected values, both with and without therefreshCache
variable.
Line range hint
5166-5198
: LGTM!The test cases for
getDonationById
query look good. They verify the retrieval of a donation by its ID and handle the case when the donation is not found. The assertions check the expected behavior.
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 @mohammadranjbarz
* fix: remove memo for project verification managing funds * fix: remove memo for project verification managing funds * add projectId and qfRoundId to qf data export * 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 * onlyEndaement option added to donationResolvers to get only endaoment projects * chore: implementing coderabbitai suggestion to remove string literal * 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]> * improve adminjs to import qfround matching and better filters * fix eslint * fix: remove adding secondary donation logic * fix minor form issues * order middleware in bootstrap file * test: add test cases to fetch only Endaoment projects * chore: change the second Project to first Project * chore: change the second Project to first Project * chore: change the second Project to first Project * chore: change the second user to new user since it is interfering with the pre-existing test cases * delete previous_round_rank when deleting a project (#1809) * Implement allocatedGivbacks function (#1808) * 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 * migration: project banners for endaoment projects need to have the correct banners * chore: underscore before unused variable in add_endaoment_project_banners * Use Gnosis giv token for getting price of GIV * Use superfluid mock adapter for test cases * Use superfluid adapter on test env again * Feat/separate givback verfied (#1770) * add isGivbackEligible field * add AddIsGivbackEligibleColumnToProject1637168932304 * add UpdateIsGivbackEligibleForVerifiedProjects1637168932305 migration * add migration to rename isProjectVerified to isProjectGivbackEligible * change isProjectVerified tp isProjectGivbackEligible * update octant donation * add approve project * treat project.verified and project.isGivbackEligible equally on sorting * remove reset verification status on verify * check isGivbackEligible on create ProjectVerificationForm * add ProjectInstantPowerViewV3 migration * use verifiedOrIsGivbackEligibleCondition * Use different materialized view for givback factor related to #1770 * Fix build error * Fix build error * Fix project query for isGivbackEligible and verified * Fix add base token migration * Fix eslint errors * Fix add base token migration * Fix add base token migration * Fix add base token migration * Fix donation test cases related to isGivbackEligible * Fix build error --------- Co-authored-by: Mohammad Ranjbar Z <[email protected]> * Fix test cases related to isProjectVerified * add isImported And categories to project tab * fix isProjectGivbackEligible Migration in wrong folder * add chaintype and solana networks to tokenTab * update branch * add environment and energy image mapping * add categories to show and edit forms in adminjs for projects * fix eslint * add best match sort option * update addSearchQuery to prioritize the title * Add Stellar to QFRound * run linter * remove eager from project categories in entity * Add isGivbackEligible filter * Hotfix automatic model score sync (#1849) * add user mbdscore sync workers and cronjob * add active env var for syncing score * add tests to the user sync worker and cronjob * prevent duplicate tokens being added in adminJS * Ensure correct emails are sent for project status changes related to decentralized verification * fix test * fix test cases * fix test cases --------- Co-authored-by: Meriem-BM <[email protected]> Co-authored-by: Carlos <[email protected]> Co-authored-by: HrithikSampson <[email protected]> Co-authored-by: HrithikSampson <[email protected]> Co-authored-by: Hrithik Sampson <[email protected]> Co-authored-by: CarlosQ96 <[email protected]> Co-authored-by: Cherik <[email protected]> Co-authored-by: Ramin <[email protected]>
related to Giveth/giveth-dapps-v2#4678 Giveth/giveth-dapps-v2#4679
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores