-
Notifications
You must be signed in to change notification settings - Fork 117
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
[IND-495]: Update wallets postgres table with totalTradingRewards field #819
Conversation
IND-495 Update wallets postgres table with totalTradingRewards field
Updated the
Note: The wallet’s table is currently only being written to by the |
Warning Rate Limit Exceeded@Christopher-Li has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 0 minutes and 51 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the @coderabbitai review command. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughA new Changes
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (10)
- indexer/packages/postgres/tests/helpers/constants.ts (1 hunks)
- indexer/packages/postgres/tests/stores/transfer-table.test.ts (2 hunks)
- indexer/packages/postgres/tests/stores/wallet-table.test.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20231128175805_add_wallets_total_trading_rewards.ts (1 hunks)
- indexer/packages/postgres/src/models/wallet-model.ts (3 hunks)
- indexer/packages/postgres/src/types/db-model-types.ts (1 hunks)
- indexer/packages/postgres/src/types/wallet-types.ts (1 hunks)
- indexer/services/comlink/tests/controllers/api/v4/transfers-controller.test.ts (1 hunks)
- indexer/services/ender/tests/handlers/transfer-handler.test.ts (3 hunks)
- indexer/services/ender/src/scripts/dydx_transfer_handler.sql (2 hunks)
Files skipped from review due to trivial changes (3)
- indexer/packages/postgres/tests/stores/wallet-table.test.ts
- indexer/packages/postgres/src/types/db-model-types.ts
- indexer/services/ender/src/scripts/dydx_transfer_handler.sql
Additional comments: 10
indexer/packages/postgres/__tests__/helpers/constants.ts (1)
- 97-100: The
totalTradingRewards
property is added as a string in thedefaultWallet
object. Please ensure that when this value is used in database operations, it is correctly converted to a decimal type as per the database schema requirements.indexer/packages/postgres/__tests__/stores/transfer-table.test.ts (2)
267-271: The addition of the
totalTradingRewards
field in theWalletTable.create
method aligns with the summary and the pull request description. This ensures that the new field is being considered in the test cases.270-270: Verify that initializing
totalTradingRewards
with a string value of '0' is consistent with the expected data type in the database schema and the application's data handling logic.indexer/packages/postgres/src/models/wallet-model.ts (1)
- 1-1: The import statement for
NumericPattern
has been correctly added.indexer/packages/postgres/src/types/wallet-types.ts (2)
3-6: The addition of the
totalTradingRewards
property to theWalletCreateObject
interface is correct and aligns with the summary.8-11: The addition of the
totalTradingRewards
value to theWalletColumns
enum is correct and aligns with the summary.indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts (1)
- 40-42: The
totalTradingRewards
field is being initialized as a string with a value of '0'. Verify that this is consistent with the database schema, which expects a decimal type. If the database expects a numeric type, initializing it as a string could lead to type mismatch issues.indexer/services/ender/__tests__/handlers/transfer-handler.test.ts (3)
301-307: The test case correctly checks the wallet object for the expected structure and static value for 'totalTradingRewards' after a deposit event.
356-358: The test case correctly checks the wallet object for the expected structure and static value for 'totalTradingRewards' after creating a new deposit for a previously non-existent subaccount.
410-411: The test case correctly checks the wallet object for the expected structure and static value for 'totalTradingRewards' after a withdrawal event.
...stgres/src/db/migrations/migration_files/20231128175805_add_wallets_total_trading_rewards.ts
Outdated
Show resolved
Hide resolved
indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts
Show resolved
Hide resolved
5cc6cbf
to
9d8fe04
Compare
Is this necessary? Can we just calculate this based on the trading_rewards table? |
It can be calculated from the |
Changelist
Update wallets postgres table with totalTradingRewards field
Test Plan
Updated tests
Author/Reviewer Checklist
state-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.