Add offline message to IOU and Split bill and allow currency selection while offline by luacmartins · Pull Request #4019 · Expensify/App

luacmartins · 2021-07-13T22:52:27Z

cc @chiragsalian
cc @tgolen and @marcaaron because of changes to processNetworkRequestQueue

Currently, requests that are sent offline and should not be retried are hanging since the promises never resolve. This PR should solve that and allow users to go through the currency selection, as well as see an offline error message on the confirmation page.

Fixed Issues

$ #3792

Tests

Login to app
Kill internet connection
Click '+' and select Request Money
Verify that you can go through the flow and see Looks like you're offline. Please check your connection and try again. error message on the request confirmation page.
Dismiss the modal
Click '+' and select Split Bill
Verify that you can go through the flow and see Looks like you're offline. Please check your connection and try again. error message on the request confirmation page.

QA Steps

Steps above.

Tested On

Screenshots

Web

web.mov

Mobile Web

mobile-web.mov

Desktop

desktop.mov

iOS

ios.mov

Android

android.mov

marcaaron

Just to clarify, it seems like this PR is solving two separate issues?

Disallow creating IOUs while offline
Preventing requests to get a preferred currency while offline from "hanging" because promises that should not be retried never resolve.

The code changes for 1 look straightforward. I'd like to discuss the changes for 2 some more also wondering whether this can be solved by adding some kind of default currency as suggested here

src/libs/Network.js

src/libs/actions/PersonalDetails.js

src/libs/Network.js

luacmartins · 2021-07-15T15:38:03Z

Hey @marcaaron! Thank you so much for the awesome feedback - I really appreciate it!

Just to clarify, it seems like this PR is solving two separate issues?

Correct! The goal for 2 is to allow users to go through the IOU flow and get to the confirmation page while offline. We currently set a default currency, but the issue is that we never set isRetrievingCurrency back to false when we call fetchCurrencyPreferences

luacmartins · 2021-07-16T18:37:58Z

Updated! Changes to Network were reverted and the solution now implements a second selectedCurrency key.

src/pages/iou/IOUCurrencySelection.js

src/pages/iou/IOUModal.js

src/libs/actions/IOU.js

luacmartins · 2021-07-19T16:25:27Z

Updated with requested changes! I will create a PR in Web-E renaming preferredCurrency to localCurrency in the API.

src/libs/actions/IOU.js

src/libs/actions/PersonalDetails.js

src/pages/iou/IOUCurrencySelection.js

luacmartins · 2021-07-19T23:28:03Z

Updated!

Julesssss · 2021-07-22T15:28:32Z

Leaving this to @marcaaron, as he also requested changes

marcaaron

LGTM! Left some small comments

src/pages/iou/steps/IOUAmountPage.js

luacmartins · 2021-07-22T16:57:29Z

Updated!

OSBotify · 2021-07-22T17:32:20Z

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

Add offline message to IOU and Split bill and allow currency selection while offline (cherry picked from commit fd92e19)

Julesssss · 2021-07-23T16:10:09Z

I requested a CP for this PR as deploying it will fix this issue sooner. Until then, some users will not be able to make IOU requests.

Update: I just realized this wasn't necessary as we already had a PR on staging which made the API change 🤦

OSBotify · 2021-07-23T16:39:46Z

🚀 Deployed to staging in version: 1.0.79-4🚀

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

OSBotify · 2021-07-23T21:12:43Z

🚀 Deployed to production in version: 1.0.79-4🚀

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

OSBotify · 2021-07-23T21:34:34Z

🚀 Deployed to staging in version: 1.0.79-5🚀

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

OSBotify · 2021-07-28T21:08:02Z

🚀 Deployed to production in version: 1.0.80-2🚀

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

luacmartins self-assigned this Jul 13, 2021

luacmartins force-pushed the cmartins-offline-currency branch from 63d5d7b to de88d27 Compare July 14, 2021 21:57

luacmartins requested a review from a team July 14, 2021 22:23

luacmartins marked this pull request as ready for review July 14, 2021 22:23

MelvinBot requested review from MariaHCD and removed request for a team July 14, 2021 22:23

marcaaron requested changes Jul 15, 2021

View reviewed changes

src/libs/Network.js Outdated Show resolved Hide resolved

src/libs/Network.js Outdated Show resolved Hide resolved

src/libs/actions/PersonalDetails.js Outdated Show resolved Hide resolved

src/libs/Network.js Outdated Show resolved Hide resolved

luacmartins added 2 commits July 16, 2021 10:18

merge main

8d8745b

add selectedCurrency

8f10ad7

luacmartins force-pushed the cmartins-offline-currency branch from de88d27 to 8f10ad7 Compare July 16, 2021 18:31

luacmartins requested a review from a team as a code owner July 16, 2021 18:31

MelvinBot requested review from yuwenmemon and removed request for a team July 16, 2021 18:32

luacmartins removed the request for review from yuwenmemon July 16, 2021 18:40

marcaaron requested changes Jul 16, 2021

View reviewed changes

src/pages/iou/IOUCurrencySelection.js Outdated Show resolved Hide resolved

src/pages/iou/IOUModal.js Show resolved Hide resolved

src/pages/iou/IOUModal.js Outdated Show resolved Hide resolved

luacmartins added 2 commits July 16, 2021 17:14

clean up state and onyx from react components

68be9bb

fix props

707e505

Julesssss reviewed Jul 19, 2021

View reviewed changes

src/libs/actions/IOU.js Outdated Show resolved Hide resolved

Julesssss reviewed Jul 19, 2021

View reviewed changes

src/libs/actions/IOU.js Outdated Show resolved Hide resolved

luacmartins added 2 commits July 19, 2021 09:49

finish removing state

a2b7356

rename preferredCurrency to localCurrency

39b0d84

luacmartins changed the title ~~Add offline message to IOU and Split bill and allow currency selection while offline~~ [HOLD Web-E PR #31503] Add offline message to IOU and Split bill and allow currency selection while offline Jul 19, 2021

marcaaron requested changes Jul 19, 2021

View reviewed changes

src/libs/actions/IOU.js Show resolved Hide resolved

src/libs/actions/IOU.js Outdated Show resolved Hide resolved

src/libs/actions/PersonalDetails.js Outdated Show resolved Hide resolved

src/pages/iou/IOUCurrencySelection.js Outdated Show resolved Hide resolved

luacmartins added 3 commits July 19, 2021 16:57

remove onyx currency symbol key

72d127e

move selectedCurrency to IOU key

f1881e0

update setIOUSelectedCurrency

d8cfac2

Julesssss previously approved these changes Jul 22, 2021

View reviewed changes

marcaaron previously approved these changes Jul 22, 2021

View reviewed changes

src/pages/iou/steps/IOUAmountPage.js Outdated Show resolved Hide resolved

src/pages/iou/steps/IOUAmountPage.js Outdated Show resolved Hide resolved

doc comments and lodashGet

180bef2

luacmartins dismissed stale reviews from marcaaron and Julesssss via 180bef2 July 22, 2021 16:53

fix style

9144f80

marcaaron approved these changes Jul 22, 2021

View reviewed changes

marcaaron merged commit fd92e19 into main Jul 22, 2021

marcaaron deleted the cmartins-offline-currency branch July 22, 2021 17:31

github-actions bot pushed a commit that referenced this pull request Jul 23, 2021

Merge pull request #4019 from Expensify/cmartins-offline-currency

91f797b

Add offline message to IOU and Split bill and allow currency selection while offline (cherry picked from commit fd92e19)

OSBotify mentioned this pull request Jul 23, 2021

🍒 Cherry pick PR #4019 to staging 🍒 #4193

Merged

OSBotify mentioned this pull request Jul 23, 2021

Deploy Checklist: Expensify.cash 2021-07-23 #4198

Closed

60 tasks

This was referenced Jul 28, 2021

Deploy Checklist: Expensify.cash 2021-07-28 #4279

Closed

Deploy Checklist: Expensify.cash 2021-07-30 #4333

Closed

This was referenced Aug 6, 2021

Deploy Checklist: Expensify.cash 2021-08-06 #4447

Closed

Deploy Checklist: Expensify.cash 2021-08-11 #4554

Closed

OSBotify mentioned this pull request Aug 17, 2021

Deploy Checklist: New Expensify 2021-08-17 #4701

Closed

This was referenced Aug 25, 2021

Deploy Checklist: New Expensify 2021-08-25 #4828

Closed

Deploy Checklist: New Expensify 2021-09-01 #4976

Closed

This was referenced Sep 2, 2021

Deploy Checklist: New Expensify 2021-09-01 #5004

Closed

Deploy Checklist: New Expensify 2021-09-03 #5046

Closed

OSBotify mentioned this pull request Aug 16, 2022

Deploy Checklist: New Expensify 2022-08-04 #10253

Closed

Conversation

luacmartins commented Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Uh oh!

marcaaron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

luacmartins commented Jul 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luacmartins commented Jul 16, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

luacmartins commented Jul 19, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

luacmartins commented Jul 19, 2021

Uh oh!

Julesssss commented Jul 22, 2021

Uh oh!

marcaaron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

luacmartins commented Jul 22, 2021

Uh oh!

OSBotify commented Jul 22, 2021

Uh oh!

Julesssss commented Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OSBotify commented Jul 23, 2021

Uh oh!

OSBotify commented Jul 23, 2021

Uh oh!

OSBotify commented Jul 23, 2021

Uh oh!

OSBotify commented Jul 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

luacmartins commented Jul 13, 2021 •

edited

Loading

luacmartins commented Jul 15, 2021 •

edited

Loading

Julesssss commented Jul 23, 2021 •

edited

Loading