[OTE-543] Add leaderboard pnl table #1938
Conversation
WalkthroughThe recent changes introduce enhancements for managing leaderboard Profit and Loss (PNL) data within the database. New enum values and interfaces are added for handling query configurations. A new migration script creates an index to optimize leaderboard PNL queries. Additionally, a Changes
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 as PR comments)
Additionally, you can add 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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- indexer/packages/postgres/tests/helpers/constants.ts (2 hunks)
- indexer/packages/postgres/tests/stores/leaderboard-pnl-table.test.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20240717160024_create_leaderboard_pnl_table.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20240717171246_create_leaderboard_pnl_rank_timespan_index.ts (1 hunks)
- indexer/packages/postgres/src/helpers/db-helpers.ts (1 hunks)
- indexer/packages/postgres/src/index.ts (2 hunks)
- indexer/packages/postgres/src/models/leaderboard-pnl-model.ts (1 hunks)
- indexer/packages/postgres/src/stores/leaderboard-pnl-table.ts (1 hunks)
- indexer/packages/postgres/src/types/db-model-types.ts (1 hunks)
- indexer/packages/postgres/src/types/index.ts (1 hunks)
- indexer/packages/postgres/src/types/leaderboard-pnl-types.ts (1 hunks)
- indexer/packages/postgres/src/types/query-types.ts (2 hunks)
Files skipped from review due to trivial changes (3)
- indexer/packages/postgres/src/helpers/db-helpers.ts
- indexer/packages/postgres/src/types/db-model-types.ts
- indexer/packages/postgres/src/types/index.ts
Additional comments not posted (20)
indexer/packages/postgres/src/types/leaderboard-pnl-types.ts (1)
1-17: LGTM!The interface and enum are well-defined and provide clear structure and type safety for leaderboard PNL data.
indexer/packages/postgres/src/db/migrations/migration_files/20240717171246_create_leaderboard_pnl_rank_timespan_index.ts (1)
1-17: LGTM!The migration script is well-defined and handles index creation and deletion appropriately.
indexer/packages/postgres/src/db/migrations/migration_files/20240717160024_create_leaderboard_pnl_table.ts (1)
1-26: LGTM!The migration script is well-defined and the table schema is appropriate for storing leaderboard PNL data.
indexer/packages/postgres/src/models/leaderboard-pnl-model.ts (1)
1-60: LGTM!The model is well-defined and includes necessary configurations and validations.
indexer/packages/postgres/__tests__/stores/leaderboard-pnl-table.test.ts (4)
33-47: LGTM!The test case is well-structured and correctly verifies the creation of multiple
LeaderboardPNLrecords.
49-67: LGTM!The test case is well-structured and correctly verifies the retrieval of a
LeaderboardPNLrecord usingsubaccountIdandtimeSpan.
69-83: LGTM!The test case is well-structured and correctly verifies the upsert operation for a
LeaderboardPNLrecord.
85-98: LGTM!The test case is well-structured and correctly verifies the bulk upsert operation for multiple
LeaderboardPNLrecords.indexer/packages/postgres/src/index.ts (2)
20-20: LGTM!The export for
LeaderboardPNLModelis correctly added.
44-44: LGTM!The export for
LeaderboardPNLTableis correctly added.indexer/packages/postgres/src/stores/leaderboard-pnl-table.ts (4)
27-83: LGTM!The
findAllfunction is well-structured and includes necessary checks and query setups.
85-94: LGTM!The
createfunction is well-structured and correctly uses theinsertmethod.
96-106: LGTM!The
upsertfunction is well-structured and correctly uses theupsertmethod.
108-146: LGTM!The
bulkUpsertfunction is well-structured and includes necessary checks and query setups.indexer/packages/postgres/src/types/query-types.ts (2)
89-90: LGTM!The additions to the
QueryableFieldenum are correctly added.
319-323: LGTM!The
LeaderboardPNLQueryConfiginterface is well-defined and correctly extendsQueryConfig.indexer/packages/postgres/__tests__/helpers/constants.ts (4)
862-868: LGTM!The
defaultLeaderboardPnlOneDayobject is correctly defined with appropriate values.
870-876: LGTM!The
defaultLeaderboardPnl2OneDayobject is correctly defined with appropriate values.
878-884: LGTM!The
defaultLeaderboardPnl1AllTimeobject is correctly defined with appropriate values.
886-892: LGTM!The
defaultLeaderboardPnlOneDayToUpsertobject is correctly defined with appropriate values.
| it('Successfully creates a LeaderboardPNL', async () => { | ||
| await LeaderboardPNLTable.create(defaultLeaderboardPnlOneDay); | ||
| }); |
There was a problem hiding this comment.
Add assertions to verify the creation.
The test case should include assertions to verify that the LeaderboardPNL record was created successfully.
- await LeaderboardPNLTable.create(defaultLeaderboardPnlOneDay);
+ const result = await LeaderboardPNLTable.create(defaultLeaderboardPnlOneDay);
+ expect(result).toEqual(expect.objectContaining(defaultLeaderboardPnlOneDay));Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('Successfully creates a LeaderboardPNL', async () => { | |
| await LeaderboardPNLTable.create(defaultLeaderboardPnlOneDay); | |
| }); | |
| it('Successfully creates a LeaderboardPNL', async () => { | |
| const result = await LeaderboardPNLTable.create(defaultLeaderboardPnlOneDay); | |
| expect(result).toEqual(expect.objectContaining(defaultLeaderboardPnlOneDay)); | |
| }); |
| table.integer('rank').notNullable(); | ||
| table.primary(['subaccountId', 'timeSpan']); | ||
| }); | ||
|
|
| CREATE INDEX CONCURRENTLY IF NOT EXISTS "leaderboard_pnl_rank_timespan_index" ON leaderboard_pnl("rank", "timeSpan"); | ||
| `); | ||
| } | ||
|
|
||
| export async function down(knex: Knex): Promise<void> { | ||
| await knex.raw(` | ||
| DROP INDEX CONCURRENTLY IF EXISTS "leaderboard_pnl_rank_timespan_index"; | ||
| `); |
| @@ -17,6 +17,7 @@ export { default as TransferModel } from './models/transfer-model'; | |||
| export { default as TradingRewardModel } from './models/trading-reward-model'; | |||
| export { default as TradingRewardAggregationModel } from './models/trading-reward-aggregation-model'; | |||
| export { default as SubaccountUsernamesModel } from './models/subaccount-usernames-model'; | |||
| export { default as LeaderboardPNLModel } from './models/leaderboard-pnl-model'; | |||
There was a problem hiding this comment.
we refer to pnl in the codebase as Pnl, lets keep that heuristic and rename to LeaderboardPnlModel, and update accordingly for other objects
| 'timeSpan', | ||
| 'pnl', | ||
| 'currentEquity', | ||
| 'rank'], |
There was a problem hiding this comment.
spacing looks right to me?
| leaderboardPNLObjects: LeaderboardPNLCreateObject[], | ||
| options: Options = { txId: undefined }, | ||
| ): Promise<void> { | ||
|
|
There was a problem hiding this comment.
nit: remove new line
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- indexer/packages/postgres/tests/helpers/constants.ts (2 hunks)
- indexer/packages/postgres/tests/stores/leaderboard-pnl-table.test.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20240717160024_create_leaderboard_pnl_table.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20240717171246_create_leaderboard_pnl_rank_timespan_index.ts (1 hunks)
- indexer/packages/postgres/src/index.ts (2 hunks)
- indexer/packages/postgres/src/models/leaderboard-pnl-model.ts (1 hunks)
- indexer/packages/postgres/src/stores/leaderboard-pnl-table.ts (1 hunks)
- indexer/packages/postgres/src/types/db-model-types.ts (1 hunks)
- indexer/packages/postgres/src/types/leaderboard-pnl-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 (10)
- indexer/packages/postgres/tests/helpers/constants.ts
- indexer/packages/postgres/tests/stores/leaderboard-pnl-table.test.ts
- indexer/packages/postgres/src/db/migrations/migration_files/20240717160024_create_leaderboard_pnl_table.ts
- indexer/packages/postgres/src/db/migrations/migration_files/20240717171246_create_leaderboard_pnl_rank_timespan_index.ts
- indexer/packages/postgres/src/index.ts
- indexer/packages/postgres/src/models/leaderboard-pnl-model.ts
- indexer/packages/postgres/src/stores/leaderboard-pnl-table.ts
- indexer/packages/postgres/src/types/db-model-types.ts
- indexer/packages/postgres/src/types/leaderboard-pnl-types.ts
- indexer/packages/postgres/src/types/query-types.ts
Co-authored-by: Christopher-Li <Christopher-Li@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- indexer/packages/postgres/src/db/migrations/migration_files/20240717171246_create_leaderboard_pnl_rank_timespan_index.ts (1 hunks)
- indexer/packages/postgres/src/index.ts (2 hunks)
- indexer/packages/postgres/src/models/leaderboard-pnl-model.ts (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- indexer/packages/postgres/src/db/migrations/migration_files/20240717171246_create_leaderboard_pnl_rank_timespan_index.ts
- indexer/packages/postgres/src/index.ts
- indexer/packages/postgres/src/models/leaderboard-pnl-model.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- indexer/packages/postgres/src/models/leaderboard-pnl-model.ts (1 hunks)
Additional comments not posted (7)
indexer/packages/postgres/src/models/leaderboard-pnl-model.ts (7)
9-9: Class declaration looks good.The class
LeaderboardPnlModelcorrectly extendsBaseModel.
11-13: MethodtableNamelooks good.The method correctly returns the table name as
leaderboard_pnl.
15-17: MethodidColumnlooks good.The method correctly returns the primary key columns as
subaccountIdandtimeSpan.
19-28: MethodrelationMappingslooks good.The method correctly defines a
BelongsToOneRelationwith thesubaccountmodel.
30-47: MethodjsonSchemalooks good.The method correctly defines the schema validation for the model.
50-60: Properties are correctly defined.The properties
subaccountId,timeSpan,QueryBuilderType,pnl,currentEquity, andrankare correctly defined with appropriate type annotations.
1-7: Ensure all imported modules are used.Verify that all imported modules are necessary and used in the file.
Verification successful
All imported modules are used correctly.
The
path,Model,NumericPattern,UpsertQueryBuilder, andBaseModelmodules are all utilized in theleaderboard-pnl-model.tsfile.
path: Used inmodelClass: path.join(__dirname, 'subaccount-model').Model: Used inrelation: Model.BelongsToOneRelation.NumericPattern: Used inpnl: { type: 'string', pattern: NumericPattern }andcurrentEquity: { type: 'string', pattern: NumericPattern }.UpsertQueryBuilder: Used inQueryBuilderType!: UpsertQueryBuilder<this>;.BaseModel: Used inexport default class LeaderboardPnlModel extends BaseModel.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all imported modules are used in the file. # Test: Search for the usage of each imported module. Expect: Each module should be referenced in the file. rg --type ts 'path|Model|NumericPattern|UpsertQueryBuilder|BaseModel' indexer/packages/postgres/src/models/leaderboard-pnl-model.tsLength of output: 680
Changelist
Test Plan
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
Backend Enhancements
Models
LeaderboardPnlModelto manage leaderboard PNL data, including subaccount relations and validation schema.