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

Feature: update donation summary hook with helpful form state values #7600

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

jonwaldstein
Copy link
Contributor

@jonwaldstein jonwaldstein commented Nov 1, 2024

Resolves #

Description

Context: Gateways need access to the following values:

  • Currency
  • Donation Amount for one-time (including base amount, fee recovered, event tix, etc.)
  • Subscription Amount & details for recurring (excluding some values like event tix, etc.)

We're currently putting a lot of responsibility on our gateways to understand our donation form values...

As our form has grown in complexity with new amount fields like event tickets, fee recovery, subscription details, and many different gateway implementations - it's been increasingly difficult to keep track of certain values across all fields.

Gateways are the most obvious place where this has been a challenge to maintain as they all are having to manually look up the total values for the donation amount, fees recovered, event tickets, subscription details etc.

While we do have our useWatch() hook from react-hook-form that allows us to find the raw values, it has become very redundant to manually look up the same values across all gateways while also accounting for the nuances of different variations.

This PR is intended to simplify and unify accessing our core donation form values.

This has been achieved by updating our useDonationSummary() hook with helpful form state values for more unification of common values.

The reason I chose to use the useDonationSummary() hook is because it's already being used across our fields / add-ons to sum total values making it a seamless update.

Here's what we are adding:

      const {state: {
            currency,
            donationAmount, 
            donationAmountMinor,
            donationAmountTotal,
            donationAmountTotalMinor,
            donationIsOneTime, 
            donationIsRecurring,
            subscriptionAmount,
            subscriptionAmountMinor,
            subscriptionPeriod,
            subscriptionFrequency,
        }} = window.givewp.form.hooks.useDonationSummary();

Scenario

  • Donor selects "$50"
  • Checks fee recovery of "$1.80"
  • Add 2 tix for "$10" total
        const {state: {
            currency,
            donationAmount, // 50
            donationAmountMinor, // 5000
            donationAmountTotal, //  61.80 (50 + 1.80 + 10 = 61.80)
            donationAmountTotalMinor, // 6180
            donationIsOneTime, // true
            donationIsRecurring, // false
            subscriptionAmount, // 51.80 (50 + 1.80 = 51.80) does not include tix
            subscriptionAmountMinor, // 5180
            subscriptionPeriod, //  undefined 
            subscriptionFrequency, // undefined 
        }} = window.givewp.form.hooks.useDonationSummary();

Scenario

  • Donor selects "$50"
  • Donor selects "Monthly"
  • Checks fee recovery of "$1.80"
  • Add 2 tix for "$10" total
        const {state: {
            currency,
            donationAmount, // 50
            donationAmountMinor, // 5000
            donationAmountTotal, //  61.80 (50 + 1.80 + 10 = 61.80)
            donationAmountTotalMinor, // 6180
            donationIsOneTime, // false
            donationIsRecurring, // true
            subscriptionAmount, // 51.80 (50 + 1.80 = 51.80) does not include tix
            subscriptionAmountMinor, // 5180
            subscriptionPeriod, //  "month"
            subscriptionFrequency, // 1 
        }} = window.givewp.form.hooks.useDonationSummary();

Example of how this could be used in gateways to clean up redundancy:

Stripe: before

Screenshot 2024-11-01 at 10 42 50 AM

Stripe: after

Screenshot 2024-11-01 at 11 42 54 AM Screenshot 2024-11-08 at 11 47 41 AM

PayPal before

Screenshot 2024-11-01 at 12 31 53 PM

PayPal after

Screenshot 2024-11-01 at 12 34 13 PM

Affects

This currently affects the Donation Summary field rendering.

WIP questions:

  • Which totals do recurring donation support? (amount: y, fee recovery: y/n, tickets: n)
  • In the visual donation summary field, should we account for when the total donation amount is different than subscription amount?
  • Additional fields: name, email, billing address, phone
  • change the hook name to something like useDonationFields(), useDonationFormFields()?

Visuals

Testing Instructions

  1. Test different form scenarios and make sure the donation summary field works as expected

WIP...

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

*
* @unreleased
*/
const dollarsToCents = (amount: string, currency: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I suggest amountToMinorUnit — as "dollars" and "cents" are distinct terms for certain currencies. 🤓

Comment on lines 24 to 41
const zeroDecimalCurrencies = [
'BIF',
'CLP',
'DJF',
'GNF',
'JPY',
'KMF',
'KRW',
'MGA',
'PYG',
'RWF',
'UGX',
'VND',
'VUV',
'XAF',
'XOF',
'XPF',
];
Copy link
Contributor

@JasonTheAdams JasonTheAdams Nov 1, 2024

Choose a reason for hiding this comment

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

@jonwaldstein I recommend using the following array to also identify 3-decimal currencies:

const nonTwoDecimalCurrencies = {
    'BIF': 0,
    'CLP': 0,
    'DJF': 0,
    'GNF': 0,
    'ISK': 0,
    'JPY': 0,
    'KMF': 0,
    'KRW': 0,
    'PYG': 0,
    'RWF': 0,
    'UGX': 0,
    'VND': 0,
    'VUV': 0,
    'XAF': 0,
    'XOF': 0,
    'XPF': 0,
    'BHD': 3,
    'IQD': 3,
    'JOD': 3,
    'KWD': 3,
    'LYD': 3,
    'OMR': 3,
    'TND': 3
};

Copy link
Contributor

@JasonTheAdams JasonTheAdams Nov 1, 2024

Choose a reason for hiding this comment

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

You could then adjust your math:

decimals = nonTwoDecimalCurrencies[currency] ?? 2;
return Math.round(parseFloat(amount) * (10 ** decimals)); 

@JasonTheAdams
Copy link
Contributor

I like the direction, @jonwaldstein! Thinking about the useDonationSummary() term, but I can appreciate why it's important to have an easy way for gateways to have this information.

@jonwaldstein
Copy link
Contributor Author

@glaubersilva would like to get your initial thoughts on this one too (as I continue to flesh this out) 😄

@glaubersilva
Copy link
Contributor

@jonwaldstein, as I mentioned another day, we have had problems related to retriving the total amount for event tickets. @jason and I discussed the possibility of adding a hidden field (like the fee recovery block does) to store the tickets total amount there.

But in the end, we decided to do it later in a subsequent PR due to the unknowns described here: https://lw.slack.com/archives/C04SLRDD9CK/p1717767332375629?thread_ts=1717619979.200869&cid=C04SLRDD9CK

So, if we plan to add the tickets total amount to the hook implemented on this PR, we need first take a look at the problems described in the thread linked above.

Also, a task that blocks moving the Events feature from Beta was created, in the comments of this task I added details about my discoveries at that time: https://stellarwp.atlassian.net/browse/GIVE-877?focusedCommentId=184630

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