[IND-496]: Create trading_reward_aggregations postgres table #825
Conversation
|
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 52 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. WalkthroughThe changes involve the introduction of a new Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 8
Configuration used: CodeRabbit UI
Files selected for processing (14)
- indexer/packages/postgres/tests/helpers/constants.ts (3 hunks)
- indexer/packages/postgres/tests/stores/trading-reward-aggregation-table.test.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20231130153800_create_trading_reward_aggregations_table.ts (1 hunks)
- indexer/packages/postgres/src/helpers/db-helpers.ts (1 hunks)
- indexer/packages/postgres/src/index.ts (1 hunks)
- indexer/packages/postgres/src/models/block-model.ts (1 hunks)
- indexer/packages/postgres/src/models/trading-reward-aggregation-model.ts (1 hunks)
- indexer/packages/postgres/src/models/trading-reward-model.ts (2 hunks)
- indexer/packages/postgres/src/models/wallet-model.ts (2 hunks)
- indexer/packages/postgres/src/stores/trading-reward-aggregation-table.ts (1 hunks)
- indexer/packages/postgres/src/types/db-model-types.ts (2 hunks)
- indexer/packages/postgres/src/types/index.ts (1 hunks)
- indexer/packages/postgres/src/types/query-types.ts (3 hunks)
- indexer/packages/postgres/src/types/trading-reward-aggregation-types.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- indexer/packages/postgres/src/helpers/db-helpers.ts
- indexer/packages/postgres/src/index.ts
- indexer/packages/postgres/src/types/index.ts
Additional comments: 18
indexer/packages/postgres/__tests__/helpers/constants.ts (4)
13-18: The import of
TradingRewardAggregationTableis correctly added to support the new functionality introduced in the pull request.44-51: The imports of
TradingRewardAggregationCreateObjectandTradingRewardAggregationPeriodare correctly added to support the new types introduced in the pull request.600-611: The
defaultTradingRewardAggregationobject is correctly defined with the necessary properties to represent a default object for testing purposes.607-611: The
defaultTradingRewardAggregationIdis correctly initialized usingTradingRewardAggregationTable.uuidto generate a unique identifier for the default aggregation object.indexer/packages/postgres/src/db/migrations/migration_files/20231130153800_create_trading_reward_aggregations_table.ts (2)
24-25: Verify that the referenced columns 'wallets.address' and 'blocks.blockHeight' exist in their respective tables and that the data types are compatible with the foreign key columns.
7-21: The migration script correctly defines the structure of the new
trading_reward_aggregationstable with appropriate data types and constraints for each column.indexer/packages/postgres/src/models/block-model.ts (1)
- 47-57: The addition of the
tradingRewardAggregationsproperty to theBlockModelclass is correctly implemented with theHasManyRelationand the join paths are properly set up.indexer/packages/postgres/src/models/trading-reward-aggregation-model.ts (3)
19-35: The relation mappings are correctly set up for
walletsandblocks. Ensure that the correspondingwallet-modelandblock-modelfiles have the inverse relation mappings set up to maintain bidirectional relations.38-59: The jsonSchema is well-defined with appropriate types and formats. However, ensure that the
startedAtHeightandendedAtHeightfields are intended to be strings and not integers, as heights are typically numeric.83-97: The model fields are correctly typed according to the jsonSchema. Ensure that the
IsoStringtype is consistently used across the application for date-time representations.indexer/packages/postgres/src/models/trading-reward-model.ts (1)
- 4-10: The import of
IsoStringand its use in theTradingRewardModelclass align with the changes described in the summary.indexer/packages/postgres/src/models/wallet-model.ts (1)
- 18-35: The relation mappings for
tradingRewardAggregationsandtradingRewardshave been added correctly usingModel.HasManyRelation. The use ofpath.joinformodelClassensures cross-platform compatibility for file paths. Ensure that the corresponding model files (trading-reward-aggregation-modelandtrading-reward-model) are correctly implemented and located in the same directory asWalletModel.indexer/packages/postgres/src/types/db-model-types.ts (2)
9-15: The changes to the imports and the addition of the
TradingRewardAggregationFromDatabaseinterface align with the summary provided and the overall context of the pull request.235-244: The
TradingRewardAggregationFromDatabaseinterface has been added correctly with all the specified fields. This aligns with the summary and the new database schema changes.indexer/packages/postgres/src/types/query-types.ts (3)
5-8: The import statement for
TradingRewardAggregationPeriodhas been correctly added to support the new types related to trading reward aggregations.73-77: The
QueryableFieldenum has been correctly updated withSTARTED_AT_HEIGHTandPERIODto support querying on the newtrading_reward_aggregationstable.277-280: The
TradingRewardAggregationQueryConfiginterface has been correctly defined with fieldsADDRESS,STARTED_AT_HEIGHT, andPERIODto match the new query capabilities for thetrading_reward_aggregationstable.indexer/packages/postgres/src/types/trading-reward-aggregation-types.ts (1)
- 1-35: The types and enums defined in
trading-reward-aggregation-types.tsare consistent with the summary and appear to be correctly implemented.
indexer/packages/postgres/src/models/trading-reward-aggregation-model.ts
Show resolved
Hide resolved
indexer/packages/postgres/__tests__/stores/trading-reward-aggregation-table.test.ts
Outdated
Show resolved
Hide resolved
indexer/packages/postgres/src/stores/trading-reward-aggregation-table.ts
Show resolved
Hide resolved
indexer/packages/postgres/src/stores/trading-reward-aggregation-table.ts
Show resolved
Hide resolved
indexer/packages/postgres/__tests__/stores/trading-reward-aggregation-table.test.ts
Outdated
Show resolved
Hide resolved
indexer/packages/postgres/__tests__/stores/trading-reward-aggregation-table.test.ts
Outdated
Show resolved
Hide resolved
| it('Successfully creates a TradingRewardAggregation', async () => { | ||
| await TradingRewardAggregationTable.create(defaultTradingRewardAggregation); | ||
| }); |
There was a problem hiding this comment.
The test case for creating a TradingRewardAggregation does not assert the result of the creation. It is recommended to add an assertion to verify that the creation was successful.
it('Successfully creates a TradingRewardAggregation', async () => {
+ const result = await TradingRewardAggregationTable.create(defaultTradingRewardAggregation);
+ expect(result).toBeDefined();
});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.
| it('Successfully creates a TradingRewardAggregation', async () => { | |
| await TradingRewardAggregationTable.create(defaultTradingRewardAggregation); | |
| }); | |
| it('Successfully creates a TradingRewardAggregation', async () => { | |
| const result = await TradingRewardAggregationTable.create(defaultTradingRewardAggregation); | |
| expect(result).toBeDefined(); | |
| }); |
| it('Successfully updates an existing TradingRewardAggregation', async () => { | ||
| await TradingRewardAggregationTable.create(defaultTradingRewardAggregation); | ||
|
|
||
| const amount: string = '100000.00'; | ||
| const endedAt: string = '2021-01-01T00:00:00.000Z'; | ||
| const endedAtHeight: string = '1000'; | ||
| const update: | ||
| TradingRewardAggregationFromDatabase | undefined = await TradingRewardAggregationTable.update({ | ||
| id: defaultTradingRewardAggregationId, | ||
| endedAt, | ||
| endedAtHeight, | ||
| amount, | ||
| }); | ||
| expect(update).toEqual({ | ||
| ...defaultTradingRewardAggregation, | ||
| id: defaultTradingRewardAggregationId, | ||
| endedAt, | ||
| endedAtHeight, | ||
| amount, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The test case for updating an existing TradingRewardAggregation should verify the existence of the record before attempting an update to ensure the test is accurate.
it('Successfully updates an existing TradingRewardAggregation', async () => {
+ const existing = await TradingRewardAggregationTable.findById(defaultTradingRewardAggregationId);
+ expect(existing).toBeDefined();
await TradingRewardAggregationTable.create(defaultTradingRewardAggregation);
// ... rest of the test
});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.
| it('Successfully updates an existing TradingRewardAggregation', async () => { | |
| await TradingRewardAggregationTable.create(defaultTradingRewardAggregation); | |
| const amount: string = '100000.00'; | |
| const endedAt: string = '2021-01-01T00:00:00.000Z'; | |
| const endedAtHeight: string = '1000'; | |
| const update: | |
| TradingRewardAggregationFromDatabase | undefined = await TradingRewardAggregationTable.update({ | |
| id: defaultTradingRewardAggregationId, | |
| endedAt, | |
| endedAtHeight, | |
| amount, | |
| }); | |
| expect(update).toEqual({ | |
| ...defaultTradingRewardAggregation, | |
| id: defaultTradingRewardAggregationId, | |
| endedAt, | |
| endedAtHeight, | |
| amount, | |
| }); | |
| }); | |
| it('Successfully updates an existing TradingRewardAggregation', async () => { | |
| const existing = await TradingRewardAggregationTable.findById(defaultTradingRewardAggregationId); | |
| expect(existing).toBeDefined(); | |
| await TradingRewardAggregationTable.create(defaultTradingRewardAggregation); | |
| const amount: string = '100000.00'; | |
| const endedAt: string = '2021-01-01T00:00:00.000Z'; | |
| const endedAtHeight: string = '1000'; | |
| const update: | |
| TradingRewardAggregationFromDatabase | undefined = await TradingRewardAggregationTable.update({ | |
| id: defaultTradingRewardAggregationId, | |
| endedAt, | |
| endedAtHeight, | |
| amount, | |
| }); | |
| expect(update).toEqual({ | |
| ...defaultTradingRewardAggregation, | |
| id: defaultTradingRewardAggregationId, | |
| endedAt, | |
| endedAtHeight, | |
| amount, | |
| }); | |
| }); |
indexer/packages/postgres/__tests__/stores/trading-reward-aggregation-table.test.ts
Outdated
Show resolved
Hide resolved
Changelist
Test Plan
Unit tests
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.