Simplify the Logic for Display Amount in Transactions

[SudharakaP]

Related to https://github.com/opencollective/opencollective-frontend/pull/7391#discussion_r812777066

This simplifies the logic for display amount in the transactions list.

[vercel[bot]]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

opencollective-frontend – ./

🔍 Inspect: https://vercel.com/opencollective/opencollective-frontend/BE4r4ReJ9hv9iW3ke9DSvFzgx9eW
✅ Preview: https://opencollective-frontend-git-refactor-disp-f26ae4-opencollective.vercel.app

opencollective-styleguide – ./

🔍 Inspect: https://vercel.com/opencollective/opencollective-styleguide/FefNBJQjG2gjxVtQqGpaUFMwYFsk
✅ Preview: https://opencollective-styleguide-git-refactor-di-6d1297-opencollective.vercel.app

[znarf]

The comment there doesn't give me enough guarantee to approve this PR: https://github.com/opencollective/opencollective-frontend/pull/7391#discussion_r813499925

Can we have a list of all cases we want to support?

Can we have a test account with screenshot before/after for all the cases?

[SudharakaP]

The comment there doesn't give me enough guarantee to approve this PR: #7391 (comment)

Can we have a list of all cases we want to support?

Can we have a test account with screenshot before/after for all the cases?

@znarf : Originally we had,

  if (isExpense) {
    return transaction.netAmount;
  } else if (isCredit && hasOrder) {
    // Credit from donations should display the full amount donated by the user
    return transaction.amount;
  } else if (transaction.isRefunded) {
    if ((isSelf && !transaction.isRefund) || (transaction.isRefund && isCredit)) {
      return transaction.netAmount;
    } else {
      return transaction.amount;
    }
  } else {
    return transaction.netAmount;
  }

what we are doing here is essentially removing the isRefunded part. So it becomes,

  if (isExpense) {
    return transaction.netAmount;
  } else if (isCredit && hasOrder) {
    // Credit from donations should display the full amount donated by the user
    return transaction.amount;
  } else {
    return transaction.netAmount;
  }

and isCredit && hasOrder only happens if the transaction is not an expense (it's a donation credit). So we could simplify it to,

  if (isCredit && hasOrder) {
    // Credit from donations should display the full amount donated by the user
    return transaction.amount;
  } else {
    return transaction.netAmount;
  }

Can we have a list of all cases we want to support?

Therefore since we are removing transaction.isRefunded we should test for all types of refunded transactions is my thinking and especially these cases below as they involve things we haven't tested before. For this purpose we could use latest transactions (which I've created on Feb 23rd) at, https://staging.opencollective.com/transactions-project-2/transactions. Here we have,

1. Added funds refund
2. Refund of expense from collective to itself.
3. Refund of added fund from collective to itself.

All other refund cases we have covered before with the same collective. As for before and after screenshots of refunds we have,

Before

After

Can we have a test account with screenshot before/after for all the cases?

I am using the test account https://staging.opencollective.com/transactions-project-2/transactions to test as it now has all types of refunds. Just to be completely sure I've also done a comparison of all the transactions of https://staging.opencollective.com/transactions-project-2/transactions vs all the transactions from this PR. And they also tally with each other.

Let me know if I've missed anything here of if you have any other concerns. 🙂