Navigating to enable-payments shows only the side bar content when first navigating to it. Refreshing shows an infinite spinner.

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Navigate to https://staging.new.expensify.com/enable-payments
2. Refresh the page

Expected Result:

Enable-payments sidebar should load

Actual Result:

An infinite loading spinner is displayed on the sidebar.

Workaround:

User has to log out and back in to be able to navigate succesfully to https://staging.new.expensify.com/enable-payments

Platform:

Where is this issue occurring?

• Web
• Mobile Web

Version Number: 1.0.82-2

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:

View all open jobs on Upwork

Upwork job

---

From @Jag96 https://expensify.slack.com/archives/C01GTK53T8Q/p1627944072091400

Navigating to https://staging.new.expensify.com/enable-payments only shows the sidebar content the first time that you navigate to it. After refreshing the page, it shows an infinite spinner until you sign out and sign back in. Same issue on main.

[MelvinBot]

Triggered auto assignment to @AndrewGable (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[AndrewGable]

I wasn't able to reproduce on the first try. Can you confirm this is still happening @isagoico ?

[isagoico]

Yep I'm still able to reproduce on my side

Grabacion.de.pantalla.2021-08-04.a.las.11.59.37.mov

[AndrewGable]

Ok great thank you, I now am able to reproduce. Attached is an additional error seen.

Uncaught (in promise) TypeError: Cannot read property '[email protected]' of undefined
    at Qw (PersonalDetails.js:199)
    at Zw (PersonalDetails.js:219)
    at Object.callback (AuthScreens.js:79)
    at Hr (Onyx.js:342)
    at Onyx.js:407

[MelvinBot]

Triggered auto assignment to @MitchExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[parasharrajat]

I think there is some issue with Onyx.merge. I checked that Onyx.merge is not triggering render for the EnablePayments component.
Props this.props.userWallet is not updating after API response is received in EnablePayments.

cc: @kidroca

[kidroca]

I think there is some issue with Onyx.merge. I checked that Onyx.merge is not triggering render for the EnablePayments component.
Props this.props.userWallet is not updating after API response is received in EnablePayments.

The only change that has happened to Onyx.merge is the one made in Onyx.set as set is used internally
When we're trying to set or merge a value where we have the exact same value in storage we bail from the update.
This will skip the call to keyChanged, because it didn't really change, and in turn will skip a re-render that might have happened before

Perhaps the API response is brining the same value for userWallet that we already have in storage and so it's not triggering a re-render?
IMO we shouldn't depend on the API response forcing a re-render through Onyx.merge (if the value didn't change)

[parasharrajat]

Oh, Yeah that is causing the issue.
My question is console.log(_.isEqual({a: 1, b: 2} , {b :2, a: 1})) is true. Is this intended to skip the update we do Onyx.merge with the second value.
cc: @marcaaron

[parasharrajat]

Coming from the Slack thread https://expensify.slack.com/archives/C01GTK53T8Q/p1628106963153400.
It is not too sure what should we do here in the case of standard implementation which is already there but will not work anymore due to changes to Onyx Architecture.

For now, I propose this.

Proposal

1. I will create a state variable for EnablePayments component loading.

2. In componentDidMount when the API call is finished, I will update the state loading to false.

   App/src/pages/EnablePayments/index.js

   Line 27 in 8c06c02

   fetchUserWallet();

3. In the render method, I will use this state variable to show/hide the loader.

4. Based on the current implementation, if the API call returns a value other than 200, the loader is shown forever. So to fix this as well. I think Showing a growl and going back is the best option.

[AndrewGable]

I like this proposal @parasharrajat 👍

Can you configure the Upwork job @MitchExpensify? Thanks!

[MitchExpensify]

Job created & @parasharrajat invited!

[marcaaron]

will not work anymore due to changes to Onyx Architecture.

Just wondering here.. should this not be regarded as a regression and we should address it in Onyx? Seems like some existing behavior was broken and we also know what broke it (issue). But we are kind of proposing to hack around it instead of address the regression and restore the existing behavior. Thoughts?

[parasharrajat]

I cc'ed you #4392 (comment) and then followed in slack but there was no conclusion to fix this in Onyx so I thought that the current approach needs to be changed. Then I realized that this is a bigger issue thus opened #4500. We can hold this up and Follow that #4500 to fix the breaking behavior.

[kidroca]

Why keeping a loading state is considered a hack solution?
Plenty of other actions with network calls would capture such.
The changes in Onyx revealed that this was missed here.

You will also get an infinite loading if data never arrives from the network, the request fails, or if you're offline
Keeping action state would address these items, while fixing Onyx would fix only the "happy path"

[MelvinBot]

@AndrewGable, @MitchExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[AndrewGable]

PR is in review