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

[CT-808] Add pagination for pnl endpoint #1529

Merged
merged 2 commits into from
May 17, 2024
Merged

[CT-808] Add pagination for pnl endpoint #1529

merged 2 commits into from
May 17, 2024

Conversation

dydxwill
Copy link
Contributor

@dydxwill dydxwill commented May 16, 2024

Changelist

Missing historical pnl data points because objects/response > 100. Add pagination for pnl endpoint.

Test Plan

Unit tested.

Author/Reviewer Checklist

  • If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
  • If the PR has breaking postgres changes to the indexer add the indexer-postgres-breaking label.
  • If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
  • If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
  • If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
  • Manually add any of the following labels: refactor, chore, bug.

Summary by CodeRabbit

  • New Features

    • Added pagination support to historical PnL data endpoints, including page, pageSize, totalResults, and offset fields in API responses.
  • Improvements

    • Enhanced the findAll function to support pagination, improving data retrieval efficiency for large datasets.
  • Documentation

    • Updated API documentation to reflect new pagination parameters and response structure for historical PnL data.
  • Chores

    • Aligned environment naming conventions in deployment scripts for consistency.

Copy link
Contributor

coderabbitai bot commented May 16, 2024

Walkthrough

The recent updates introduce pagination to the findAll function in the pnl-ticks-table.ts file, affecting its usage across multiple modules. This change necessitates adjustments in various test files, API documentation, and helper functions to accommodate the new pagination structure. Additionally, there are updates to environment naming in a deployment script and enhancements to API responses to include pagination details for historical PnL data.

Changes

Files Change Summary
.../postgres/__tests__/stores/pnl-ticks-table.test.ts Restructured pnlTicks assignments to destructure the results object from PnlTicksTable.findAll.
.../postgres/src/stores/pnl-ticks-table.ts Updated findAll to accept a page parameter, implementing pagination, and returning a PaginationFromDatabase object.
.../deploy-commit-to-env.sh Changed environment name from "public-testnet" to "testnet" while retaining the same account number.
.../comlink/public/api-documentation.md Added pagination fields (page, pageSize, totalResults, offset) in API responses for historical PnL data.
.../comlink/src/controllers/api/v4/historical-pnl-controller.ts Added pagination parameters and details in the returned data structure for historical PnL data.
.../comlink/src/types.ts Modified HistoricalPnlResponse to extend PaginationResponse, indicating changes in response structure for historical PnL data.
.../roundtable/__tests__/tasks/create-pnl-ticks.test.ts Modified pnlTicks assignments to destructure the results object from PnlTicksTable.findAll.
.../scripts/src/helpers/pnl-validation-helpers.ts Updated validatePnlForSubaccount function to destructure the returned object from PnlTicksTable.findAll to extract results.

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@dydxwill dydxwill changed the title Add pagination for pnl endpoint [CT-808] Add pagination for pnl endpoint May 16, 2024
Copy link

linear bot commented May 16, 2024

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 430c208 and 82ab271.
Files ignored due to path filters (1)
  • indexer/services/comlink/public/swagger.json is excluded by !**/*.json
Files selected for processing (8)
  • indexer/packages/postgres/tests/stores/pnl-ticks-table.test.ts (4 hunks)
  • indexer/packages/postgres/src/stores/pnl-ticks-table.ts (3 hunks)
  • indexer/scripts/deploy-commit-to-env.sh (1 hunks)
  • indexer/services/comlink/public/api-documentation.md (4 hunks)
  • indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts (10 hunks)
  • indexer/services/comlink/src/types.ts (1 hunks)
  • indexer/services/roundtable/tests/tasks/create-pnl-ticks.test.ts (6 hunks)
  • indexer/services/scripts/src/helpers/pnl-validation-helpers.ts (1 hunks)
Additional Context Used
LanguageTool (114)
indexer/services/comlink/public/api-documentation.md (114)

Near line 2688: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...id="schemaperpetualpositionstatus"> </a...


Near line 2689: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...d="schema_PerpetualPositionStatus"> <...


Near line 2714: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...onSide <a id="to...


Near line 2715: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ..."> <a id="tocsp...


Near line 2739: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...IsoString <a id="tocSi...


Near line 2740: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ing"> <a id="tocsisos...


Near line 2757: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...maperpetualpositionresponseobject"> <a id="schema_PerpetualPositionResponseObj...


Near line 2758: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a_PerpetualPositionResponseObject"> <a id="tocSperpetualpositionresponseobject...


Near line 2807: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a id="schemaperpetualpositionsmap"> ...


Near line 2808: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... id="schema_PerpetualPositionsMap"> <a ...


Near line 2862: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...schemaassetpositionresponseobject"> <a id="schema_AssetPositionResponseObject"...


Near line 2863: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...chema_AssetPositionResponseObject"> </...


Near line 2890: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...p <a i...


Near line 2891: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...> <a id="...


Near line 2923: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...d="schemasubaccountresponseobject"> </...


Near line 2924: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...="schema_SubaccountResponseObject"> ...


Near line 3009: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...nse <a id=...


Near line 3010: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: .../a> <a id="to...


Near line 3094: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...d="schemaparentsubaccountresponse"> </...


Near line 3095: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...="schema_ParentSubaccountResponse"> ...


Near line 3185: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a id="schemaassetpositionresponse"> ...


Near line 3186: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... id="schema_AssetPositionResponse"> <a ...


Near line 3213: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...on <a id...


Near line 3214: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a> <a id="t...


Near line 3243: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <...


Near line 3244: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a id="schema_CandleResponseObject"> <a i...


Near line 3285: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...onse <a id="...


Near line 3286: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a id="toc...


Near line 3320: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a ...


Near line 3321: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a id=...


Near line 3342: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...us <a id...


Near line 3343: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a> <a id="t...


Near line 3370: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...on <a id...


Near line 3371: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a> <a id="t...


Near line 3398: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <...


Near line 3399: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a id="schema_ComplianceV2Response"> <a i...


Near line 3422: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...OrderSide <a id="tocSo...


Near line 3423: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ide"> <a id="tocsorde...


Near line 3447: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...Liquidity <a id="tocSl...


Near line 3448: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ity"> <a id="tocsliqu...


Near line 3472: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...# FillType <a id="tocSfi...


Near line 3473: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...type"> <a id="tocsfillt...


Near line 3500: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...rketType <a id="tocS...


Near line 3501: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...pe"> <a id="tocsmar...


Near line 3525: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a ...


Near line 3526: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a id=...


Near line 3571: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...sponse <a id="to...


Near line 3572: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ..."> <a id="tocsf...


Near line 3614: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...onse <a id="...


Near line 3615: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a id="toc...


Near line 3636: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...chemahistoricalblocktradingreward"> <a id="schema_HistoricalBlockTradingReward...


Near line 3637: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...hema_HistoricalBlockTradingReward"> <...


Near line 3660: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...oricalblocktradingrewardsresponse"> <a id="schema_HistoricalBlockTradingReward...


Near line 3661: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...oricalBlockTradingRewardsResponse"> <a id="tocShistoricalblocktradingrewardsre...


Near line 3686: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...mahistoricalfundingresponseobject"> <a id="schema_HistoricalFundingResponseObj...


Near line 3687: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a_HistoricalFundingResponseObject"> <a id="tocShistoricalfundingresponseobject...


Near line 3714: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...="schemahistoricalfundingresponse"> <...


Near line 3715: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ..."schema_HistoricalFundingResponse"> ...


Near line 3742: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... id="schemapnlticksresponseobject"> ...


Near line 3743: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...id="schema_PnlTicksResponseObject"> <a...


Near line 3776: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a id="schemahistoricalpnlresponse"> ...


Near line 3777: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... id="schema_HistoricalPnlResponse"> <a ...


Near line 3813: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ematradingrewardaggregationperiod"> <a id="schema_TradingRewardAggregationPeri...


Near line 3814: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ma_TradingRewardAggregationPeriod"> <a id="tocStradingrewardaggregationperiod"...


Near line 3839: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...istoricaltradingrewardaggregation"> <a id="schema_HistoricalTradingRewardAggre...


Near line 3840: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...istoricalTradingRewardAggregation"> <a id="tocShistoricaltradingrewardaggregat...


Near line 3869: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...tradingrewardaggregationsresponse"> <a id="schema_HistoricalTradingRewardAggre...


Near line 3870: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...TradingRewardAggregationsResponse"> <a id="tocShistoricaltradingrewardaggregat...


Near line 3898: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...schemaorderbookresponsepricelevel"> <a id="schema_OrderbookResponsePriceLevel"...


Near line 3899: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...chema_OrderbookResponsePriceLevel"> </...


Near line 3920: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...id="schemaorderbookresponseobject"> </a...


Near line 3921: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...d="schema_OrderbookResponseObject"> <...


Near line 3952: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...orce <a id="...


Near line 3953: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a id="toc...


Near line 3978: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...rStatus <a id="toc...


Near line 3979: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...s"> <a id="tocsor...


Near line 4006: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... id="schemabesteffortopenedstatus"> ...


Near line 4007: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...id="schema_BestEffortOpenedStatus"> <a...


Near line 4030: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...atus <a id="...


Near line 4031: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a id="toc...


Near line 4056: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...OrderType <a id="tocSo...


Near line 4057: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ype"> <a id="tocsorde...


Near line 4089: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a...


Near line 4090: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a id...


Near line 4153: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a id="schemaperpetualmarketstatus"> ...


Near line 4154: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... id="schema_PerpetualMarketStatus"> <a ...


Near line 4182: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a...


Near line 4183: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a id...


Near line 4207: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...hemaperpetualmarketresponseobject"> <a id="schema_PerpetualMarketResponseObjec...


Near line 4208: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ema_PerpetualMarketResponseObject"> ...


Near line 4267: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...id="schemaperpetualmarketresponse"> </a...


Near line 4268: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...d="schema_PerpetualMarketResponse"> <...


Near line 4335: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...="schemaperpetualpositionresponse"> <...


Near line 4336: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ..."schema_PerpetualPositionResponse"> ...


Near line 4374: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...id="schemasparklineresponseobject"> </a...


Near line 4375: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...d="schema_SparklineResponseObject"> <...


Near line 4399: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a...


Near line 4400: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a id...


Near line 4424: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...sponse <a id="to...


Near line 4425: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ..."> <a id="tocst...


Near line 4446: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...TradeType <a id="tocSt...


Near line 4447: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ype"> <a id="tocstrad...


Near line 4472: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a...


Near line 4473: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... <a id...


Near line 4504: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ponse <a id="t...


Near line 4505: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...> <a id="tocs...


Near line 4540: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...erType <a id="to...


Near line 4541: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ..."> <a id="tocst...


Near line 4567: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ... id="schematransferresponseobject"> ...


Near line 4568: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...id="schema_TransferResponseObject"> <a...


Near line 4613: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...se <a id...


Near line 4614: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...a> <a id="t...


Near line 4657: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...aparentsubaccounttransferresponse"> <a id="schema_ParentSubaccountTransferResp...


Near line 4658: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ..._ParentSubaccountTransferResponse"> <a id="tocSparentsubaccounttransferrespons...

ShellCheck (14)
indexer/scripts/deploy-commit-to-env.sh (14)

[warning] 41-41: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.


[info] 41-41: Double quote to prevent globbing and word splitting.


[warning] 43-43: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.


[info] 43-43: Use a ( subshell ) to avoid having to cd back.


[info] 49-49: Double quote to prevent globbing and word splitting.


[info] 49-49: Double quote to prevent globbing and word splitting.


[info] 49-49: Double quote to prevent globbing and word splitting.


[info] 51-51: Double quote to prevent globbing and word splitting.


[info] 52-52: Double quote to prevent globbing and word splitting.


[info] 54-54: Double quote to prevent globbing and word splitting.


[info] 54-54: Double quote to prevent globbing and word splitting.


[info] 54-54: Double quote to prevent globbing and word splitting.


[info] 56-56: Double quote to prevent globbing and word splitting.


[info] 56-56: Double quote to prevent globbing and word splitting.

Path-based Instructions (8)
indexer/scripts/deploy-commit-to-env.sh (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/scripts/src/helpers/pnl-validation-helpers.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/packages/postgres/src/stores/pnl-ticks-table.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/roundtable/__tests__/tasks/create-pnl-ticks.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/comlink/src/types.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/comlink/public/api-documentation.md (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

Additional comments not posted (44)
indexer/services/scripts/src/helpers/pnl-validation-helpers.ts (1)

178-178: LGTM! Ensure pagination is handled correctly in the calling code.

indexer/packages/postgres/__tests__/stores/pnl-ticks-table.test.ts (2)

61-61: LGTM! The tests correctly reflect the new pagination structure.


81-81: LGTM! The tests correctly reflect the new pagination structure.

indexer/packages/postgres/src/stores/pnl-ticks-table.ts (1)

133-166: LGTM! Ensure pagination is handled correctly in the calling code.

indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts (6)

49-49: Ensure the page parameter is validated.

Consider adding validation for the page parameter to ensure it is a positive integer.


53-62: Verify the correctness of the pagination handling.

Ensure that the pagination logic correctly handles the page parameter and returns the expected results.


98-100: Include pagination details in the response.

The return statement correctly includes pagination details such as pageSize, totalResults, and offset.


113-113: Ensure the page parameter is validated.

Consider adding validation for the page parameter to ensure it is a positive integer.


118-127: Verify the correctness of the pagination handling.

Ensure that the pagination logic correctly handles the page parameter and returns the expected results.


188-190: Include pagination details in the response.

The return statement correctly includes pagination details such as pageSize, totalResults, and offset.

indexer/services/roundtable/__tests__/tasks/create-pnl-ticks.test.ts (5)

101-103: LGTM! The test case correctly destructures the results from the pagination object.


146-148: LGTM! The test case correctly destructures the results from the pagination object.


198-200: LGTM! The test case correctly destructures the results from the pagination object.


250-252: LGTM! The test case correctly destructures the results from the pagination object.


299-301: LGTM! The test case correctly destructures the results from the pagination object.

indexer/services/comlink/src/types.ts (1)

202-204: LGTM! The HistoricalPnlResponse interface correctly extends PaginationResponse.

indexer/services/comlink/public/api-documentation.md (28)

1195-1195: The addition of the page parameter to the GetHistoricalPnl endpoint looks good.


1203-1205: The addition of pagination response fields (pageSize, totalResults, offset) to the GetHistoricalPnl endpoint looks good.


1284-1284: The addition of the page parameter to the GetHistoricalPnlForParentSubaccount endpoint looks good.


1292-1294: The addition of pagination response fields (pageSize, totalResults, offset) to the GetHistoricalPnlForParentSubaccount endpoint looks good.


Line range hint 1165-1165: The addition of the page parameter to the GetTrades endpoint looks good.


Line range hint 1173-1175: The addition of pagination response fields (pageSize, totalResults, offset) to the GetTrades endpoint looks good.


Line range hint 1165-1165: The addition of the page parameter to the GetTransfers endpoint looks good.


Line range hint 1173-1175: The addition of pagination response fields (pageSize, totalResults, offset) to the GetTransfers endpoint looks good.


1284-1284: The addition of the page parameter to the GetTransfersForParentSubaccount endpoint looks good.


1292-1294: The addition of pagination response fields (pageSize, totalResults, offset) to the GetTransfersForParentSubaccount endpoint looks good.


Line range hint 1165-1165: The addition of the page parameter to the GetFills endpoint looks good.


Line range hint 1173-1175: The addition of pagination response fields (pageSize, totalResults, offset) to the GetFills endpoint looks good.


1284-1284: The addition of the page parameter to the GetFillsForParentSubaccount endpoint looks good.


1292-1294: The addition of pagination response fields (pageSize, totalResults, offset) to the GetFillsForParentSubaccount endpoint looks good.


Line range hint 1165-1165: The addition of the page parameter to the ListOrders endpoint looks good.


Line range hint 1173-1175: The addition of pagination response fields (pageSize, totalResults, offset) to the ListOrders endpoint looks good.


1284-1284: The addition of the page parameter to the ListOrdersForParentSubaccount endpoint looks good.


1292-1294: The addition of pagination response fields (pageSize, totalResults, offset) to the ListOrdersForParentSubaccount endpoint looks good.


Line range hint 1165-1165: The addition of the page parameter to the GetHistoricalFunding endpoint looks good.


Line range hint 1173-1175: The addition of pagination response fields (pageSize, totalResults, offset) to the GetHistoricalFunding endpoint looks good.


Line range hint 1165-1165: The addition of the page parameter to the GetHistoricalBlockTradingRewards endpoint looks good.


Line range hint 1173-1175: The addition of pagination response fields (pageSize, totalResults, offset) to the GetHistoricalBlockTradingRewards endpoint looks good.


Line range hint 1165-1165: The addition of the page parameter to the GetHistoricalTradingRewardAggregations endpoint looks good.


Line range hint 1173-1175: The addition of pagination response fields (pageSize, totalResults, offset) to the GetHistoricalTradingRewardAggregations endpoint looks good.


Line range hint 1165-1165: The addition of the page parameter to the GetAssetPositions endpoint looks good.


Line range hint 1173-1175: The addition of pagination response fields (pageSize, totalResults, offset) to the GetAssetPositions endpoint looks good.


1284-1284: The addition of the page parameter to the GetAssetPositionsForParentSubaccount endpoint looks good.


1292-1294: The addition of pagination response fields (pageSize, totalResults, offset) to the GetAssetPositionsForParentSubaccount endpoint looks good.

@@ -23,7 +23,7 @@ case $env in
"dev4") account=525975847385;;
"dev5") account=917958511744;;
"staging") account=677285201534;;
"public-testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet
Copy link
Contributor

Choose a reason for hiding this comment

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

Add error handling for cd commands.

- cd services/$service
+ cd services/$service || exit

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.

Suggested change
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet

Double quote to prevent globbing and word splitting.

- cd services/$service
+ cd "services/$service"

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.

Suggested change
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet

Use a subshell to avoid having to cd back.

- cd services/$service
- pnpm build
- cd -
+ (cd "services/$service" && pnpm build)

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.

Suggested change
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet

Double quote to prevent globbing and word splitting.

- AWS_PROFILE=dydx-v4-$env aws ecr get-login-password --region ap-northeast-1 | docker login --username AWS --password-stdin $account.dkr.ecr.ap-northeast-1.amazonaws.com
+ AWS_PROFILE="dydx-v4-$env" aws ecr get-login-password --region ap-northeast-1 | docker login --username AWS --password-stdin "$account.dkr.ecr.ap-northeast-1.amazonaws.com"

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.

Suggested change
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet
AWS_PROFILE="dydx-v4-$env" aws ecr get-login-password --region ap-northeast-1 | docker login --username AWS --password-stdin "$account.dkr.ecr.ap-northeast-1.amazonaws.com"

Double quote to prevent globbing and word splitting.

- DOCKER_BUILDKIT=1 docker build \
-             --platform linux/amd64 \
-             -t $account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag \
-             -f $dockerfile \
-             --build-arg service=$service \
-             --build-arg NPM_TOKEN=$NPM_TOKEN .
+ DOCKER_BUILDKIT=1 docker build \
+             --platform linux/amd64 \
+             -t "$account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag" \
+             -f "$dockerfile" \
+             --build-arg service="$service" \
+             --build-arg NPM_TOKEN="$NPM_TOKEN" .

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.

Suggested change
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet
DOCKER_BUILDKIT=1 docker build \
--platform linux/amd64 \
-t "$account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag" \
-f "$dockerfile" \
--build-arg service="$service" \
--build-arg NPM_TOKEN="$NPM_TOKEN" .

Double quote to prevent globbing and word splitting.

- docker push $account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag
+ docker push "$account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag"

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.

Suggested change
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet
docker push "$account.dkr.ecr.ap-northeast-1.amazonaws.com/$env-indexer-$service:$tag"

Double quote to prevent globbing and word splitting.

- AWS_PROFILE=dydx-v4-$env orb deploy $service $tag
+ AWS_PROFILE="dydx-v4-$env" orb deploy "$service" "$tag"

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.

Suggested change
"testnet") account=013339450148;; # public testnet
"testnet") account=013339450148;; # public testnet

Comment on lines +46 to +50
page,
}: PnlTicksQueryConfig,
requiredFields: QueryableField[],
options: Options = DEFAULT_POSTGRES_OPTIONS,
): Promise<PnlTicksFromDatabase[]> {
): Promise<PaginationFromDatabase<PnlTicksFromDatabase>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Several files in the codebase still use the old function signature for PnlTicksTable.findAll.

  • Ensure that all instances of PnlTicksTable.findAll are updated to handle pagination correctly.

Locations to update:

  • validatePnlForSubaccount function
  • Test cases involving PnlTicksTable.findAll
  • Any other instances where PnlTicksTable.findAll is called without pagination parameters
Analysis chain

LGTM! Ensure pagination is handled correctly in the calling code.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify pagination handling in the calling code.

# Test: Search for the function usage. Expect: Proper handling of pagination.
rg --type typescript $'findAll'

Length of output: 65



Script:

#!/bin/bash
# Description: Verify pagination handling in the calling code.

# Find all TypeScript files and search for the `findAll` method usage within those files.
fd -e ts -e tsx -x rg 'findAll' {}

Length of output: 20249



Script:

#!/bin/bash
# Description: Extract context around the `PnlTicksTable.findAll` calls to verify pagination handling.

# Extract 5 lines before and after each `PnlTicksTable.findAll` call.
fd -e ts -e tsx -x rg -C 5 'PnlTicksTable.findAll' {}

Length of output: 3955

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits Files that changed from the base of the PR and between 82ab271 and b3a6d5a.
Files selected for processing (4)
  • indexer/packages/postgres/tests/stores/pnl-ticks-table.test.ts (5 hunks)
  • indexer/services/comlink/tests/controllers/api/v4/historical-pnl-controller.test.ts (1 hunks)
  • indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts (12 hunks)
  • indexer/services/comlink/src/types.ts (2 hunks)
Files not reviewed due to errors (1)
  • indexer/services/comlink/tests/controllers/api/v4/historical-pnl-controller.test.ts (no review received)
Files skipped from review as they are similar to previous changes (3)
  • indexer/packages/postgres/tests/stores/pnl-ticks-table.test.ts
  • indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts
  • indexer/services/comlink/src/types.ts
Additional Context Used
Path-based Instructions (1)
indexer/services/comlink/__tests__/controllers/api/v4/historical-pnl-controller.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

@dydxwill dydxwill merged commit a20347d into main May 17, 2024
11 checks passed
@dydxwill dydxwill deleted the wl/pnl_pagination branch May 17, 2024 13:11
@dydxwill
Copy link
Contributor Author

https://github.com/Mergifyio backport release/indexer/v5.x

Copy link
Contributor

mergify bot commented May 17, 2024

backport release/indexer/v5.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request May 17, 2024
* add pagination for pnl endpoint

* add unit tests

(cherry picked from commit a20347d)
@dydxwill
Copy link
Contributor Author

https://github.com/Mergifyio backport release/indexer/v5.x

Copy link
Contributor

mergify bot commented May 17, 2024

backport release/indexer/v5.x

✅ Backports have been created

dydxwill added a commit that referenced this pull request May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants