Skip to content
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

feat(feedV2): consolidate earn types into deposit and withdraw #6189

Draft
wants to merge 2 commits into
base: jeanregisser/remove-typename
Choose a base branch
from

Conversation

jeanregisser
Copy link
Member

Description

Test plan

Related issues

  • Fixes #[issue number here]

Backwards compatibility

Network scalability

If a new NetworkId and/or Network are added in the future, the changes in this PR will:

  • Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)

timestamp: number
block: string
fees: Fee[]
providerName: string | undefined
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we could also keep providerId and would use it first.
providerName would be used as a fallback. For all the other dapps Zerion already knows about.

EarnWithdraw = 'EARN_WITHDRAW',
/** @deprecated Use Received instead */
EarnClaimReward = 'EARN_CLAIM_REWARD',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't implemented the changes to Received yet in this PR.

Copy link
Member Author

@jeanregisser jeanregisser Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the implementation here and in other files is a copy of the Earn components, just adapted for the new types.

Copy link

codecov bot commented Oct 25, 2024

Codecov Report

Attention: Patch coverage is 91.50943% with 9 lines in your changes missing coverage. Please review.

Project coverage is 88.89%. Comparing base (67ad8b6) to head (f786c75).

Files with missing lines Patch % Lines
...rc/transactions/feed/DepositOrWithdrawFeedItem.tsx 94.54% 3 Missing ⚠️
src/transactions/feed/TransactionFeed.tsx 0.00% 3 Missing ⚠️
src/transactions/feed/TransactionFeedV2.tsx 25.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                       Coverage Diff                        @@
##           jeanregisser/remove-typename    #6189      +/-   ##
================================================================
+ Coverage                         88.88%   88.89%   +0.01%     
================================================================
  Files                               733      735       +2     
  Lines                             31166    31270     +104     
  Branches                           5736     5465     -271     
================================================================
+ Hits                              27702    27798      +96     
- Misses                             3265     3427     +162     
+ Partials                            199       45     -154     
Files with missing lines Coverage Δ
src/transactions/feed/TransactionDetailsScreen.tsx 92.72% <100.00%> (+0.34%) ⬆️
src/transactions/feed/TransactionFeedItemImage.tsx 95.74% <100.00%> (+0.18%) ⬆️
...ns/feed/detailContent/DepositOrWithdrawContent.tsx 100.00% <100.00%> (ø)
src/transactions/types.ts 97.91% <100.00%> (+0.09%) ⬆️
...rc/transactions/feed/DepositOrWithdrawFeedItem.tsx 94.54% <94.54%> (ø)
src/transactions/feed/TransactionFeed.tsx 77.27% <0.00%> (-2.73%) ⬇️
src/transactions/feed/TransactionFeedV2.tsx 90.82% <25.00%> (-1.25%) ⬇️

... and 66 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67ad8b6...f786c75. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant