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

Use currentBalance instead of availableBalance #5014

Merged
merged 2 commits into from
Sep 7, 2021

Conversation

madmax330
Copy link
Contributor

@madmax330 madmax330 commented Sep 2, 2021

Details

We're showing the wrong balance to the user. The currentBalance is the one we want the user to see. availableBalance is used on the backend to decide wether or not to authorize transactions.

cc @parasharrajat

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/173152
$ https://github.com/Expensify/Expensify/issues/173151

Tests

  • Log into an account with a wallet
  • Insert a queued reimbursement with the creditBankAccountID the wallet of the account you are logged in with
  • Make sure that you see the balance updated.
  • Check that available balance is 0 by running this snippet:
API.get({returnValueList: 'userWallet'}).done(({userWallet}) => {
            console.log(userWallet);
        }).fail((e) => {
            console.log(e);
        });

QA Steps

  • Request money from someone
  • Have them pay the IOU via bank account, not debit card
  • Make sure you see your balance change right after they paid the IOU

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screen Shot 2021-09-06 at 10 04 48 PM

Screen Shot 2021-09-06 at 10 04 53 PM

Screen Shot 2021-09-06 at 10 05 22 PM

Mobile Web

Desktop

iOS

Android

@madmax330 madmax330 self-assigned this Sep 2, 2021
@madmax330 madmax330 requested a review from a team as a code owner September 2, 2021 13:11
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@MelvinBot MelvinBot requested review from Dal-Papa and removed request for a team September 2, 2021 13:11
@madmax330
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@madmax330 madmax330 changed the title Use currentBalance instead of availableBalance [Hold on testing] Use currentBalance instead of availableBalance Sep 2, 2021
@parasharrajat
Copy link
Member

Ok. Thanks for making a clear distinction. I am working on one of the PR's which deal with this so I will update that.

Dal-Papa
Dal-Papa previously approved these changes Sep 2, 2021
Copy link

@Dal-Papa Dal-Papa left a comment

Choose a reason for hiding this comment

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

Code LGTM

@madmax330 madmax330 changed the title [Hold on testing] Use currentBalance instead of availableBalance Use currentBalance instead of availableBalance Sep 6, 2021
@madmax330
Copy link
Contributor Author

Updated, only tested on web since there's not actually a UI change.

@stitesExpensify
Copy link
Contributor

You stole my PR! lol jk. Updated your description to include that other issue and merged 👍

@OSBotify
Copy link
Contributor

OSBotify commented Sep 7, 2021

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

OSBotify commented Sep 8, 2021

🚀 Deployed to staging by @stitesExpensify in version: 1.0.94-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@isagoico
Copy link

isagoico commented Sep 9, 2021

@madmax330 Hello! is this testable by us? we tried by adding a fully verified account but the payment option doesn't appear.

@madmax330
Copy link
Contributor Author

No I put the internalQA label, but apparently it doesn't work the same here as it does for the web app. Sorry, we'll test this internally.

@roryabraham
Copy link
Contributor

Hey y'all, any update on the internal QA here? This is the last item on the checklist blocking a prod deploy.

@yuwenmemon
Copy link
Contributor

yuwenmemon commented Sep 9, 2021

Looks like @Jag96 and @kevinksullivan are doing the internal QA for this. What's the latest fellas?

@madmax330
Copy link
Contributor Author

No it's @joekaufmanexpensify and Kevin. Should be good to check off her soon

@yuwenmemon
Copy link
Contributor

🤦‍♂️ lol Sorry!

@madmax330
Copy link
Contributor Author

Hence the importance of the Ionatan rule 😆

@kevinksullivan
Copy link
Contributor

QA successful on staging - balance changed after paying an IOU.

@OSBotify
Copy link
Contributor

OSBotify commented Sep 9, 2021

🚀 Deployed to production by @yuwenmemon in version: 1.0.95-1 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

9 participants