-
Notifications
You must be signed in to change notification settings - Fork 125
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
[OTE-752] Add totalVolume and isWhitelistAffiliate to wallets table #2166
Conversation
9263bb6
to
c9e4411
Compare
WalkthroughThe changes introduce two new properties, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WalletTable
participant Database
User->>WalletTable: Create Wallet
WalletTable->>Database: Insert wallet with totalVolume and isWhitelistAffiliate
Database-->>WalletTable: Confirm insert
WalletTable-->>User: Return wallet details
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 as PR comments)
Additionally, you can add 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- 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 (4 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20240827162119_add_wallets_total_volume_and_affiliates_whitelist.ts (1 hunks)
- indexer/packages/postgres/src/models/wallet-model.ts (3 hunks)
- indexer/packages/postgres/src/stores/wallet-table.ts (3 hunks)
- indexer/packages/postgres/src/types/db-model-types.ts (1 hunks)
- indexer/packages/postgres/src/types/query-types.ts (2 hunks)
- indexer/packages/postgres/src/types/wallet-types.ts (1 hunks)
- indexer/services/comlink/tests/controllers/api/v4/transfers-controller.test.ts (6 hunks)
- indexer/services/ender/tests/handlers/trading-rewards-handler.test.ts (3 hunks)
- indexer/services/ender/tests/handlers/transfer-handler.test.ts (3 hunks)
- indexer/services/ender/src/scripts/handlers/dydx_transfer_handler.sql (2 hunks)
Additional comments not posted (46)
indexer/packages/postgres/src/types/wallet-types.ts (3)
6-7
: LGTM!The new properties
totalVolume
andisWhitelistAffiliate
are correctly added and typed.The code changes are approved.
13-14
: LGTM!The new properties
totalVolume
andisWhitelistAffiliate
are correctly added and typed.The code changes are approved.
20-21
: LGTM!The new members
totalVolume
andisWhitelistAffiliate
are correctly added and typed.The code changes are approved.
indexer/packages/postgres/src/db/migrations/migration_files/20240827162119_add_wallets_total_volume_and_affiliates_whitelist.ts (2)
3-9
: LGTM!The new columns
totalVolume
andisWhitelistAffiliate
are correctly added with appropriate types and default values.The code changes are approved.
12-18
: LGTM!The columns
totalVolume
andisWhitelistAffiliate
are correctly dropped.The code changes are approved.
indexer/packages/postgres/src/models/wallet-model.ts (4)
43-44
: LGTM!The new properties
totalVolume
andisWhitelistAffiliate
are correctly added and typed.The code changes are approved.
Also applies to: 49-50
65-66
: LGTM!The new mappings
totalVolume
andisWhitelistAffiliate
are correctly added and typed.The code changes are approved.
76-76
: LGTM!The new class field
totalVolume
is correctly added and typed.The code changes are approved.
78-78
: LGTM!The new class field
isWhitelistAffiliate
is correctly added and typed.The code changes are approved.
indexer/packages/postgres/__tests__/stores/wallet-table.test.ts (3)
30-41
: LGTM!The function correctly tests the
upsert
method with the new propertiesisWhitelistAffiliate
andtotalVolume
.The code changes are approved.
50-51
: LGTM!The function correctly tests the
findAll
method with the new propertiesisWhitelistAffiliate
andtotalVolume
.The code changes are approved.
78-94
: LGTM!The function correctly tests the
findAll
method with theisWhitelistAffiliate
property.The code changes are approved.
indexer/packages/postgres/src/stores/wallet-table.ts (1)
22-23
: LGTM!The function correctly includes
isWhitelistAffiliate
in the query, allowing for more granular filtering.The code changes are approved.
Also applies to: 31-32, 46-48
indexer/services/ender/src/scripts/handlers/dydx_transfer_handler.sql (1)
60-61
: LGTM!The function correctly includes
totalVolume
andisWhitelistAffiliate
in the wallet records, enhancing the data model.The code changes are approved.
Also applies to: 70-71
indexer/packages/postgres/src/types/db-model-types.ts (2)
31-31
: LGTM! But verify the usage of the new property.The addition of
totalVolume
to theWalletFromDatabase
interface looks good. Ensure that this new property is consistently integrated and utilized in the codebase.The code changes are approved.
Run the following script to verify the usage of the new property:
Verification successful
The
totalVolume
property is consistently integrated and utilized across the codebase.The property appears in various files, including models, migration scripts, and test files, indicating thorough integration.
- Files:
indexer/packages/postgres/src/models/wallet-model.ts
indexer/packages/postgres/src/db/migrations/migration_files/20240827162119_add_wallets_total_volume_and_affiliates_whitelist.ts
indexer/packages/postgres/__tests__/helpers/constants.ts
indexer/packages/postgres/__tests__/stores/wallet-table.test.ts
- And others.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of the `totalVolume` property. # Test: Search for the property usage. Expect: Consistent integration and utilization. rg --type python -A 5 $'totalVolume'Length of output: 66
Script:
#!/bin/bash # Description: Verify all usages of the `totalVolume` property in TypeScript files. # Test: Search for the property usage. Expect: Consistent integration and utilization. rg --type ts -A 5 $'totalVolume'Length of output: 21559
32-32
: LGTM! But verify the usage of the new property.The addition of
isWhitelistAffiliate
to theWalletFromDatabase
interface looks good. Ensure that this new property is consistently integrated and utilized in the codebase.The code changes are approved.
Run the following script to verify the usage of the new property:
Verification successful
The
isWhitelistAffiliate
property is consistently integrated and utilized across the codebase.The property is used in various test files, store files, and migration files, indicating thorough integration and testing.
- Test Files: The property is tested in multiple test cases, ensuring its behavior is verified.
- Store Files: It is part of the data handling logic, showing its role in application functionality.
- Migration Files: The property is included in the database schema, confirming its support at the database level.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of the `isWhitelistAffiliate` property. # Test: Search for the property usage. Expect: Consistent integration and utilization. rg --type python -A 5 $'isWhitelistAffiliate'Length of output: 75
Script:
#!/bin/bash # Description: Verify all usages of the `isWhitelistAffiliate` property in TypeScript files. # Test: Search for the property usage. Expect: Consistent integration and utilization. rg --type ts -A 5 $'isWhitelistAffiliate'Length of output: 19751
indexer/services/ender/__tests__/handlers/trading-rewards-handler.test.ts (6)
177-177
: LGTM!The addition of
totalVolume
with a default value of '0' to the wallet object in the test case looks good.The code changes are approved.
178-178
: LGTM!The addition of
isWhitelistAffiliate
with a default value offalse
to the wallet object in the test case looks good.The code changes are approved.
202-202
: LGTM!The addition of
totalVolume
with a default value of '0' to the wallet object in the test case looks good.The code changes are approved.
203-203
: LGTM!The addition of
isWhitelistAffiliate
with a default value offalse
to the wallet object in the test case looks good.The code changes are approved.
213-213
: LGTM!The addition of
totalVolume
with a default value of '0' to the wallet object in the test case looks good.The code changes are approved.
214-214
: LGTM!The addition of
isWhitelistAffiliate
with a default value offalse
to the wallet object in the test case looks good.The code changes are approved.
indexer/packages/postgres/src/types/query-types.ts (2)
114-114
: LGTM! But verify the usage of the new property.The addition of
[QueryableField.IS_WHITELIST_AFFILIATE]?: boolean;
to theWalletQueryConfig
interface looks good. Ensure that this new property is consistently integrated and utilized in the codebase.The code changes are approved.
Run the following script to verify the usage of the new property:
91-91
: LGTM! But verify the usage of the new value.The addition of
IS_WHITELIST_AFFILIATE
to theQueryableField
enum looks good. Ensure that this new value is consistently integrated and utilized in the codebase.The code changes are approved.
Run the following script to verify the usage of the new value:
indexer/packages/postgres/__tests__/stores/transfer-table.test.ts (2)
325-325
: Verify handling oftotalVolume
inWalletTable
.Ensure that the
totalVolume
field is properly handled in theWalletTable
implementation.
326-326
: Verify handling ofisWhitelistAffiliate
inWalletTable
.Ensure that the
isWhitelistAffiliate
field is properly handled in theWalletTable
implementation.indexer/services/ender/__tests__/handlers/transfer-handler.test.ts (4)
309-309
: Verify handling oftotalVolume
inWalletTable
.Ensure that the
totalVolume
field is properly handled in theWalletTable
implementation.
310-310
: Verify handling ofisWhitelistAffiliate
inWalletTable
.Ensure that the
isWhitelistAffiliate
field is properly handled in theWalletTable
implementation.
363-363
: Verify handling oftotalVolume
inWalletTable
.Ensure that the
totalVolume
field is properly handled in theWalletTable
implementation.
364-364
: Verify handling ofisWhitelistAffiliate
inWalletTable
.Ensure that the
isWhitelistAffiliate
field is properly handled in theWalletTable
implementation.indexer/packages/postgres/__tests__/helpers/constants.ts (4)
161-161
: Verify handling oftotalVolume
inWalletTable
.Ensure that the
totalVolume
field is properly handled in theWalletTable
implementation.
162-162
: Verify handling ofisWhitelistAffiliate
inWalletTable
.Ensure that the
isWhitelistAffiliate
field is properly handled in theWalletTable
implementation.
168-168
: Verify handling oftotalVolume
inWalletTable
.Ensure that the
totalVolume
field is properly handled in theWalletTable
implementation.
169-169
: Verify handling ofisWhitelistAffiliate
inWalletTable
.Ensure that the
isWhitelistAffiliate
field is properly handled in theWalletTable
implementation.indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts (12)
59-60
: LGTM!The addition of
totalVolume
to theWalletTable.create
function call is correct and consistent with the PR objectives.
60-60
: LGTM!The addition of
isWhitelistAffiliate
to theWalletTable.create
function call is correct and consistent with the PR objectives.
185-186
: LGTM!The addition of
totalVolume
to theWalletTable.create
function call is correct and consistent with the PR objectives.
186-186
: LGTM!The addition of
isWhitelistAffiliate
to theWalletTable.create
function call is correct and consistent with the PR objectives.
468-469
: LGTM!The addition of
totalVolume
to theWalletTable.create
function call is correct and consistent with the PR objectives.
469-469
: LGTM!The addition of
isWhitelistAffiliate
to theWalletTable.create
function call is correct and consistent with the PR objectives.
594-595
: LGTM!The addition of
totalVolume
to theWalletTable.create
function call is correct and consistent with the PR objectives.
595-595
: LGTM!The addition of
isWhitelistAffiliate
to theWalletTable.create
function call is correct and consistent with the PR objectives.
749-750
: LGTM!The addition of
totalVolume
to theWalletTable.create
function call is correct and consistent with the PR objectives.
750-750
: LGTM!The addition of
isWhitelistAffiliate
to theWalletTable.create
function call is correct and consistent with the PR objectives.
837-838
: LGTM!The addition of
totalVolume
to theWalletTable.create
function call is correct and consistent with the PR objectives.
838-838
: LGTM!The addition of
isWhitelistAffiliate
to theWalletTable.create
function call is correct and consistent with the PR objectives.
indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts
Outdated
Show resolved
Hide resolved
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- indexer/services/comlink/tests/controllers/api/v4/transfers-controller.test.ts (8 hunks)
- indexer/services/ender/tests/handlers/trading-rewards-handler.test.ts (5 hunks)
- indexer/services/ender/tests/handlers/transfer-handler.test.ts (4 hunks)
Files skipped from review as they are similar to previous changes (3)
- indexer/services/comlink/tests/controllers/api/v4/transfers-controller.test.ts
- indexer/services/ender/tests/handlers/trading-rewards-handler.test.ts
- indexer/services/ender/tests/handlers/transfer-handler.test.ts
7bf6b74
to
8f8400c
Compare
@@ -27,6 +27,13 @@ import { | |||
} from '@dydxprotocol-indexer/postgres/build/__tests__/helpers/constants'; | |||
import Big from 'big.js'; | |||
|
|||
const defaultWallet = { |
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.
nit: why not use testConstants.defaultWallet imported from postgres?
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.
good point
8f8400c
to
8a8a2c1
Compare
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- indexer/services/comlink/tests/controllers/api/v4/transfers-controller.test.ts (7 hunks)
- indexer/services/ender/tests/handlers/trading-rewards-handler.test.ts (3 hunks)
- indexer/services/ender/tests/handlers/transfer-handler.test.ts (5 hunks)
Files skipped from review as they are similar to previous changes (3)
- indexer/services/comlink/tests/controllers/api/v4/transfers-controller.test.ts
- indexer/services/ender/tests/handlers/trading-rewards-handler.test.ts
- indexer/services/ender/tests/handlers/transfer-handler.test.ts
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- indexer/packages/postgres/src/types/db-model-types.ts (1 hunks)
- indexer/packages/postgres/src/types/query-types.ts (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- indexer/packages/postgres/src/types/db-model-types.ts
- indexer/packages/postgres/src/types/query-types.ts
✅ Backports have been created
|
Commented on wrong PR, will close packport PR once created |
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
New Features
totalVolume
andisWhitelistAffiliate
across various wallet-related functionalities to enhance wallet management and reporting capabilities.Bug Fixes
Documentation
Chores