Skip to content

Conversation

@danielpeng1
Copy link
Contributor

@danielpeng1 danielpeng1 commented Sep 25, 2025

Added optional clientConstants parameter to the BitGoAPI constructor, allowing users to provide/pass constants that override server-fetched values to the SDK.

  • Notably, modified fetchConstants() to merge server constants with client constants (client overrides take precedence).
  • Unit test in test/unit/bitgoAPI.ts passing in a constant under this flow.

Ticket: WP-5538

@danielpeng1 danielpeng1 self-assigned this Sep 25, 2025
@danielpeng1 danielpeng1 force-pushed the WP-5538/add-clients-constants-support branch from 0045af8 to aea1004 Compare September 25, 2025 17:29
@danielpeng1 danielpeng1 marked this pull request as ready for review October 1, 2025 03:53
@danielpeng1 danielpeng1 requested review from a team as code owners October 1, 2025 03:53
@pranavjain97 pranavjain97 requested a review from Copilot October 1, 2025 16:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for passing client constants to the BitGoAPI constructor, allowing developers to provide constants that override server-fetched values. This enables more flexible configuration and testing scenarios.

  • Adds an optional clientConstants parameter to the BitGoAPI constructor options
  • Modifies fetchConstants() method to handle caching logic and merge client/server constants
  • Updates test files to use the new client constants parameter instead of complex mocking

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
modules/sdk-api/src/types.ts Adds Constants type and clientConstants property to BitGoAPIOptions interface
modules/sdk-api/src/bitgoAPI.ts Implements client constants storage in constructor and updates fetchConstants caching logic
modules/sdk-api/test/unit/bitgoAPI.ts Adds comprehensive unit tests for constants caching and client constants functionality
modules/sdk-api/test/unit/v1/wallet.ts Replaces complex fetchConstants mocking with simpler clientConstants parameter usage

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@pranavjain97 pranavjain97 left a comment

Choose a reason for hiding this comment

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

Also, lets squash the commits please.

@danielpeng1 danielpeng1 force-pushed the WP-5538/add-clients-constants-support branch from 5b849ea to 7b85881 Compare October 2, 2025 19:40
Copy link
Contributor

@pranavjain97 pranavjain97 left a comment

Choose a reason for hiding this comment

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

lesgo!

@danielpeng1 danielpeng1 merged commit 9b2f238 into master Oct 3, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants