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

[PAY-2158] Update purchase-content saga to account for purchase method #6737

Merged
merged 2 commits into from
Nov 17, 2023

Conversation

schottra
Copy link
Contributor

Description

This updates the logic in the purchase-content saga to rely on the purchaseMethod field to decide how to execute the purchase, instead of attempting to automatically decide based on existing balance.

  • Added required field for purchaseMethod in the action
  • Moved Onramp invocation into its own function, updated it to throw in all non-success cases
  • Updated onramp flow to always use the "total amount" instead of attempting to fund the difference
  • Added logic for existing balance and manual transfer cases to verify that the required balance exists. Throws a new error type in cases where we somehow end up with insufficient balance to complete the purchase
  • Updated the "cancel" flow to throw an error with a specific code. We handle that in the catch and dispatch the cancellation action with an early return. This makes it easier/cleaner to nest the onramp functionality.
  • Moved the types for vendor/purchaseMethod into the models folder so that we aren't importing from hooks in places where that feels weird.
  • Added purchaseMethod to our analytics payload

fixes PAY-2158

How Has This Been Tested?

Tested locally in Chrome against staging.

})
)
} else if (purchaseMethod === PurchaseMethod.EXISTING_BALANCE) {
if (purchaseMethod === PurchaseMethod.MANUAL_TRANSFER) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I flipped this conditional around since "existing balance" and "card" will follow the same flow, with manual funding requiring an additional step in the component before dispatching the purchaseContent action.

if (isBuyUSDCViaSolEnabled) {
yield* put(
buyCryptoViaSol({
// expects "friendly" amount, so dollars
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for later: This threw me off as the Stripe saga expects the value in cents and the buyCryptoViaSol sagas want it in dollars. Need to be careful when updating this code to use FixedDecimal to ensure that we don't pass the wrong order of magnitude. Likely we want all of the sagas to accept a USDC and convert it internally.
cc @rickyrombo

Copy link
Contributor

Choose a reason for hiding this comment

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

what do you think of USDC(BigInt(1000000)).toNumber() // 1.0 as a proposal? Right now the way to do this would be Number(USDC(BigInt(1000000)).toString())

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/rs-pay-2158-purchase-logic

@@ -234,62 +292,29 @@ function* doStartPurchaseContentFlow({
const extraAmountBN = new BN(extraAmount ?? 0).mul(BN_USDC_CENT_WEI)
const totalAmountDueCentsBN = priceBN.add(extraAmountBN) as BNUSDC
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started attempting to update some of this to FixedDecimal and it immediately became scary. So I chose to leave it alone for now until we can convert all of the code using formatting and calculation utilities (such as the ceilingBNUSDCToNearestCent() call below)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think keeping these as BNUSDC or UsdcWei makes sense while doing some math. I don't think we should have FixedDecimal's being math'ed on or passed around.

Worth noting though that every wallet.ts/formatUtils.ts file dealing with currency has an equivalency using FixedDecimal - you can find it in their respective tests:
https://github.com/AudiusProject/audius-protocol/blob/main/packages/common/src/utils/wallet.test.ts
https://github.com/AudiusProject/audius-protocol/blob/main/packages/common/src/utils/formatUtil.test.ts

@schottra schottra enabled auto-merge (squash) November 17, 2023 21:48
if (result.failed) {
yield* put(
purchaseContentFlowFailed({ error: result.failed.payload.error })
yield* call(purchaseUSDC, { amount: totalAmountDueCentsBN })
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on having purchaseUSDC be named doBuyUSDC? Purchase/buy are getting used pretty interchangeably

@schottra schottra merged commit c152f0e into main Nov 17, 2023
15 of 18 checks passed
@schottra schottra deleted the rs-pay-2158-purchase-logic branch November 17, 2023 22:01
audius-infra pushed a commit that referenced this pull request Nov 18, 2023
[a917c79] Fix MetaMask not working C-3386 (#6738) nicoback2
[58b4140] Implement sign up + sign in with Metamask C-3356 (#6732) nicoback2
[6956101] [PAY-2174] Support mobile web properly in add funds flows (#6740) Raymond Jacobson
[cd09949] Support FixedDecimal on mobile (#6739) Marcus Pasell
[cc6d1c7] [PAY-2173] Make payments page work nicely in mobile web (#6726) Raymond Jacobson
[a2e38c2] [PAY-2152] Bring add funds up to spec (web) (#6725) Raymond Jacobson
[48f90ca] [C-3383] Update Divider (#6728) Dylan Jeffers
[170634d] [C-3237] Sign Up Mobile Nav Header (#6730) JD Francis
[bcbe2d5] Tweak Harmony Icon Docs (#6733) JD Francis
[c152f0e] [PAY-2158] Update purchase-content saga to account for purchase method (#6737) Randy Schott
[bb4beae] Fix metamask help url (#6736) Isaac Solo
[4706195] [PAY-2156] Purchase flow web ui updates (#6718) Reed
[682e572] [PAY-2168] Add better typechecking for FixedDecimal (#6724) Marcus Pasell
[5e8f8b7] [C-3337] Fix table section end styles (#6731) Kyle Shanks
[e69bdf5] [C-3270] Mobile UI for create email page (#6701) JD Francis
[6a507f5] Bump version to 0.5.19 audius-infra
[47910e2] [PAY-2159] Make purchase summary collapsible on web (#6719) Randy Schott
[f7162e7] [C-3364] FollowButton (#6708) Andrew Mendelsohn
[d6cad0a] Update modal title and icon colors (#6721) Dylan Jeffers
[9b492af] Add accent color to text and icons (#6720) Dylan Jeffers
[41dc703] Upgrade wagmi to v2 (in /d)  (#6712) Theo Ilie
[5061bee] Rename /payandearn to /payments (#6717) Raymond Jacobson
[664a1aa] Fixed package-lock.json to not have broken bn types in @audius/fixed-decimal (#6716) Marcus Pasell
[c02dfb3] [C-3363] FollowArtistsPage initial layout (#6692) Andrew Mendelsohn
[5e64792] [C-3378] Improve web-app hierarchy (#6710) Dylan Jeffers
[bc48098] Patch verified user on creation (#6705) Isaac Solo
[f0d7a57] Fix icon color/layout in summary table (#6715) Raymond Jacobson
[96f418b] [PAY-2164] Update web nav usdc and audio (#6696) Saliou Diallo
[ed26ea6] [C-3353] Add WelcomeModal for the new sign up flow for both web and mobile web (#6707) Kyle Shanks
[026aead] [PAY-2154] Implement add funds modal (#6714) Raymond Jacobson
[68aa7b5] Bump version to 0.5.18 audius-infra
[c35bac5] [PROTO-1426] Display chain+GH versions with react-query (#6704) Theo Ilie
[19b532c] Fix protodash mobile nav services & governance (#6709) Theo Ilie
[e8ebbc6] Update docs for upload track mood (#6706) Isaac Solo
[27c8aa4] [PROTO-1398] Add audius-d sandbox-compatible startup to mediorum (#6693) Danny
[d584a9e] [C-3375] Update TextLink styles and examples (#6703) Dylan Jeffers
[5d237e8] [PAY-2161] Implement FilterButton in Harmony (#6697) Raymond Jacobson
[d6e8c15] Healthz explorer (#6664) Steve Perkins
[d16b941] Fix 10th track has 15s wait bug (#6702) Reed
[ca5bf83] [C-3362] Port Popup component to Harmony (#6685) Raymond Jacobson
[902e805] [PROTO-1430] Add external wallets to /d (fka /up) (#6695) Theo Ilie
[6947471] [C-3370]  Fix harmony types (#6694) Dylan Jeffers
[55f2957] Bump version to 0.5.17 audius-infra
[7ec407d] [PAY-2165][PAY-2163] Add payments and earnings to mobile (#6688) Saliou Diallo
[33e22f1] [C-3223] Add select-genre sign-up page (#6691) Dylan Jeffers
[5302154] Sales, Purchases, Withdrawals tables on Pay & Earn Page (#6684) Reed
[a5c1cf8] [C-3369] Add checkbox/radio types to SelectablePill (#6689) Dylan Jeffers
[9a83de5] Migrate release_date from string to date (#6655) Isaac Solo
[c7445f9] [PAY-2167] @audius/fixed-decimal: A data structure for fixed precision decimals (#6662) Marcus Pasell
[bdee772] Fix harmony docs sidebar-item (#6690) Dylan Jeffers
[5dffb42] Fix Flex prop leaking, Upgrade Box (#6686) Dylan Jeffers
[ae9e668] Hotfix double gzip on mobile for Content Nodes (#6687) Theo Ilie
[48154d5] Add pay-gated upload test and re-enable stems test (#6681) Raymond Jacobson
[bb0f2a4] Bump version to 0.5.16 audius-infra
[7937393] Add text-shadow, fix avatar className, add 2xs spacing (#6682) Dylan Jeffers
[0f0429a] Fix broken useAccountHasClaimableRewards (#6683) Reed
[e5da54d] [PROTO-1418] Add dynamically imported Audius libs to /up app (#6680) Theo Ilie
[5bb8a96] [PAY-1955] Fix purchases, sales, withdrawals text overflow (#6669) Saliou Diallo
[a75a1e5] [C-3294] Add Harmony PlainButton docs (#6678) Kyle Shanks
[445cec1] Fix typography header xl font-size and line-height (#6677) Dylan Jeffers
[e5d53dc] Ignore animations in Chromatic (#6676) Dylan Jeffers
[c91486c] [C-3278] Add TextLink component and docs (#6661) Andrew Mendelsohn
[9b4a017] Fix emotion support in web (#6675) Dylan Jeffers
[c474b2a] [Sign up Redesign] Sign up white screens footer (web); add shadow mid inver to Harmony C-3358 C-3359ted (#6671) nicoback2
[0b0aedd] [INF-546] Ensure that vite base is set properly in prod builds (#6674) Sebastian Klingler
[29f1466] [C-3244] Fix heading text clipping (#6670) Dylan Jeffers
[57b8ddc] Only announce prod client deploys (#6673) Raymond Jacobson
[ef80e97] Add back react-native symlink step in CI (#6672) Sebastian Klingler
[e52b8c2] Add no-constant-binary-expression eslint rule (#6666) Sebastian Klingler
[02b28d7] Fix trpc endpoints w/ vite (#6668) Sebastian Klingler
[c358e17] Disable ToS banner (#6665) Raymond Jacobson
[183434d] [PAY-2151] Create Pay & Earn Page (#6654) Reed
[e313506] Fix vite webworker basename issue (#6667) Raymond Jacobson
[3ded93c] [C-3274, C-3348] Add README to Harmony (#6663) Kyle Shanks
[5ff8e61] Bump version to 0.5.15 audius-infra
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants