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

fix(subaccounts): correct type on SubAccountCreateParameters #940

Merged
merged 2 commits into from
Jun 20, 2024

Conversation

froggy1014
Copy link
Contributor

@froggy1014 froggy1014 commented Jun 18, 2024

Description

Current State (AS-IS)

/**
 * Type definition for parameters used to create a subaccount for a primary account.
 */
export type SubAccountCreateParameters = {
  /**
   * Name of the subaccount.
   */
  name: string;

  /**
   * API secret of the subaccount.
   */
  secret: string;

  /**
   * Flag indicating whether to use the primary account balance (true) or not (false).
   */
  usePrimaryAccountBalance: boolean;
};

Proposed Changes (TO-BE)

/**
 * Type definition for parameters used to create a subaccount for a primary account.
 */
export type SubAccountCreateParameters = {
  /**
   * Name of the subaccount.
   */
  name: string;

  /**
   * API secret of the subaccount.
   * - At least 8 characters and no more than 25 characters
   * - Contains at least 1 lower-case letter
   * - Contains at least 1 capital letter
   * - Contains at least 1 digit
   * - Must be unique
   *
   * If this parameter is not provided, a secret will be automatically generated and you can check it on the dashboard.
   */
  secret?: string;

  /**
   * Flag indicating whether to use the primary account balance (true) or not (false).
   * Default value is true.
   */
  usePrimaryAccountBalance?: boolean;
};

Reasons for the Changes

Optional Parameters:

Both secret and usePrimaryAccountBalance are optional in the actual HTTP request. Even if these parameters are not included in the request, it successfully creates the subaccount. Thus, the type definition should reflect their optional nature.

// 200 

const subAccount = await subAccountClient.createSubAccount({
        name: CLIENT_ID
} as SubAccountCreateParameters )

API Secret Validation:

The API secret has specific validation requirements:

  • Minimum of 8 characters and a maximum of 25 characters.
  • Must contain at least 1 lowercase letter, 1 uppercase letter, and 1 digit.
  • Must be unique.

These requirements are enforced in the Vonage dashboard but were not clear during testing in the development environment. Therefore, including these validation rules in the comments helps developers understand the constraints without having to rely on trial and error.

image

Test Adjustments:

The current tests might include invalid values for the secret. Updating the tests to align with the valid values is necessary to ensure the robustness of the test suite.

before after
'the new secret' 'The new secret1'

If the parameters were intentionally made non-optional to prevent user errors, let me know

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@manchuck manchuck merged commit f7e7531 into Vonage:3.x Jun 20, 2024
11 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.

None yet

2 participants