-
Notifications
You must be signed in to change notification settings - Fork 204
chore/auction-whitelist-new-endpoint #611
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
Conversation
📝 WalkthroughWalkthroughNew account auction status fetching capability added to the SDK. A public method Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant IndexerGrpcAuctionApi
participant gRPC Service
participant Transformer
Client->>IndexerGrpcAuctionApi: fetchAccountAuctionStatus(address, round)
IndexerGrpcAuctionApi->>IndexerGrpcAuctionApi: Create AuctionAccountStatusRequest
IndexerGrpcAuctionApi->>gRPC Service: auctionAccountStatus(request)
gRPC Service-->>IndexerGrpcAuctionApi: AuctionAccountStatusResponse
IndexerGrpcAuctionApi->>Transformer: auctionAccountStatusResponseToAuctionAccountStatus(response)
Transformer-->>IndexerGrpcAuctionApi: AccountAuctionStatus
IndexerGrpcAuctionApi-->>Client: AccountAuctionStatus
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
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
🧹 Nitpick comments (2)
packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.spec.ts (1)
142-162: Test will pass even when the API call fails.The test swallows exceptions with
console.error, so it will always pass regardless of whether the API call succeeds. This is consistent with other tests in this file, but consider:
- At minimum, adding
throw eafter logging to ensure failures are caught- Adding a test case that exercises the optional
roundparameter♻️ Suggested improvement
test('fetchAccountAuctionStatus', async () => { try { const response = await indexerGrpcAuctionApi.fetchAccountAuctionStatus({ address: 'inj17gkuet8f6pssxd8nycm3qr9d9y699rupv6397z', }) expect(response).toBeDefined() expect(response).toEqual( expect.objectContaining< ReturnType< typeof IndexerGrpcAuctionTransformer.auctionAccountStatusResponseToAuctionAccountStatus > >(response), ) } catch (e) { console.error( 'IndexerGrpcAuctionApi.fetchAccountAuctionStatus => ' + (e as any).message, ) + throw e } })packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.ts (1)
162-164: Redundant check:addressis a required parameter.Since
addressis defined as a required parameter in the function signature (address: string), theif (address)check is unnecessary. TypeScript already ensures it will be provided.♻️ Suggested simplification
- if (address) { - request.address = address - } + request.address = address
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/sdk-ts/package.jsonpackages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.spec.tspackages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.tspackages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.tspackages/sdk-ts/src/client/indexer/types/auction.ts
🧰 Additional context used
🧬 Code graph analysis (3)
packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.spec.ts (1)
packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts (1)
IndexerGrpcAuctionTransformer(22-166)
packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts (1)
packages/sdk-ts/src/client/indexer/types/auction.ts (1)
AccountAuctionStatus(71-73)
packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.ts (1)
packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts (1)
IndexerGrpcAuctionTransformer(22-166)
🔇 Additional comments (5)
packages/sdk-ts/package.json (1)
331-331: LGTM!The dependency bump to
@injectivelabs/[email protected]is required to support the newAuctionAccountStatusRequestandAuctionAccountStatusResponsetypes used by the newfetchAccountAuctionStatusAPI method.packages/sdk-ts/src/client/indexer/types/auction.ts (1)
71-73: LGTM!The new
AccountAuctionStatusinterface is well-defined and correctly exported for use by the transformer and API method.packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.ts (1)
149-174: LGTM!The new
fetchAccountAuctionStatusmethod follows the established patterns in this class. The gRPC call is properly typed, and the response is correctly transformed using the dedicated transformer method.packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts (2)
14-14: LGTM!The
AccountAuctionStatustype is correctly imported alongside related auction types.
159-165: LGTM!The transformer method follows the established pattern in this class, providing a clean transformation from the gRPC response to the domain model.
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.