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

[FX-1376] Fetch rosetta-traffic-percentage to determine vault provider #258

Conversation

evanfreeze
Copy link
Contributor

@evanfreeze evanfreeze commented May 29, 2024

What

  • Checks rosetta-traffic-percentage in LaunchDarkly to determine vault provider, only checking vault-primary-traffic-percentage if we aren't using rosetta based on the initial flag check.
  • Changes the default third-party vault provider to be BT instead of VGS
  • Updates tests to reflect new behavior

Why

https://linear.app/joinforage/issue/FX-1376/fetch-rosetta-traffic-percentage-to-determine-whether-to-route-between

Test Plan

  • Added unit tests

How

Can be merged as-is, as far as I know?

internal fun computeVaultType(trafficPrimaryPercentFlag: Double): VaultType {
val randomNum = Math.random() * 100
return if (randomNum < trafficPrimaryPercentFlag) VaultType.BT_VAULT_TYPE else VaultType.VGS_VAULT_TYPE
}

internal fun shouldUseRosetta(rosettaPercentFlag: Double): Boolean {
val randomNum = Math.random() * 100
return randomNum < rosettaPercentFlag
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should be randomNum <= rosettaPercentFlag to favor rosetta in the unlikely case that randomNum = 100

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 34 to 37
internal fun computeVaultType(trafficPrimaryPercentFlag: Double): VaultType {
val randomNum = Math.random() * 100
return if (randomNum < trafficPrimaryPercentFlag) VaultType.BT_VAULT_TYPE else VaultType.VGS_VAULT_TYPE
}
Copy link
Contributor

Choose a reason for hiding this comment

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

can drop vault-primary-traffic-percentage entirely in favor of rosetta-traffic-percentage, as discussed in slack

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@evanfreeze evanfreeze marked this pull request as ready for review May 30, 2024 17:33
Copy link
Contributor

@djoksimo djoksimo left a comment

Choose a reason for hiding this comment

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

Nice stuff @evanfreeze!

@evanfreeze evanfreeze changed the base branch from main to evan/remove-vault-percentage-flag May 30, 2024 19:56
@evanfreeze evanfreeze merged commit 963cbf1 into evan/remove-vault-percentage-flag May 30, 2024
6 checks passed
@djoksimo djoksimo deleted the evan/fx-1376-fetch-rosetta-traffic-percentage-to-determine-whether-to branch May 30, 2024 21:04
djoksimo pushed a commit that referenced this pull request Jun 17, 2024
#258)

* add support for reading rosetta-traffic-percentage flag to LDManager

* update LDManager.getVaultProvider tests

* re-organize and combine lines

* remove `vault-primary-traffic-percentage` flag references entirely

* update LDManager tests

* add TODO comment to clean up vgsVaultWrapper

* clean up unused imports
djoksimo added a commit that referenced this pull request Jun 17, 2024
* [FX-1376] Fetch rosetta-traffic-percentage to determine vault provider (#258)

* add support for reading rosetta-traffic-percentage flag to LDManager

* update LDManager.getVaultProvider tests

* re-organize and combine lines

* remove `vault-primary-traffic-percentage` flag references entirely

* update LDManager tests

* add TODO comment to clean up vgsVaultWrapper

* clean up unused imports

* [FX-1368] Re-introduce the ForageVaultWrapper  (#260)

* add ForageResponseParser

* add ForagePinSubmitter

* add ForageVaultWrapper

* use ForageVaultWrapper in ForagePINEditText

* flip logic around to reduce unnecessary diff

* update comment

* Update forage-android/src/main/java/com/joinforage/forage/android/ecom/ui/ForagePINEditText.kt

Co-authored-by: Danilo Joksimovic <[email protected]>

* Rotate sample app value for customer-id

Force re-tokenization of test cards in mobile-qa-tets

* [FX-1451] Add unit tests for RosettaPinSubmitter and RosettaPinElement (#261)

* Add unit tests for ForagePinSubmitter

* Add RosettaPinElement tests!

* Add test for checking default text size

* Bring parity to corner styles

* Add comment about x-key in tests

* Fix tests from merging against Devin's changes

* Appease linter

---------

Co-authored-by: Danilo Joksimovic <[email protected]>
Co-authored-by: Danilo Joksimovic <[email protected]>

---------

Co-authored-by: Danilo Joksimovic <[email protected]>
Co-authored-by: Danilo Joksimovic <[email protected]>
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.

3 participants