Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
2.97k stars 2.48k forks source link

[$250] FAB - Nothing happens when Start chat is clicked after creating a room offline #40855

Open izarutskaya opened 1 week ago

izarutskaya commented 1 week ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 1.4.65-0 Reproducible in staging?: Y Reproducible in production?: Y Found when executing: https://github.com/Expensify/Expensify/issues/302054 Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Log in with a new account
  2. Create a workspace
  3. Go offline
  4. Create a new room
  5. Navigate to the LHN
  6. Navigate to FAB - Start chat

Expected Result:

Start chat modal should open.

Actual Result:

Nothing happens when Start chat is clicked after creating a room offline. On iOS and Android native, you can see it opening for a short time but it disappears after. On mWeb Safari, the page shakes after the tap.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/e4b393d6-5868-489c-ad9d-6e7e5ea296a4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0114d6fec2c1cc3911
  • Upwork Job ID: 1785002053872132096
  • Last Price Increase: 2024-04-29
melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @muttmuure (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

izarutskaya commented 1 week ago

We think this issue might be related to the #vip-vsb.

bernhardoj commented 1 week ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

When we open start chat again after creating a room while offline, nothing happen.

What is the root cause of that problem?

When we create a new room, it will optimistically set the loading to the new room form https://github.com/Expensify/App/blob/08b7ff41656a34373761fb2bd505cc0dfa64be7b/src/libs/actions/Report.ts#L1955-L1957

Because we are offline, the loading state is always true. When we reopen the page again, it will immediately close because of the below effect. https://github.com/Expensify/App/blob/08b7ff41656a34373761fb2bd505cc0dfa64be7b/src/pages/workspace/WorkspaceNewRoomPage.tsx#L146-L152

The (isOffline && isLoading) condition is true. When the page is mounted, we clear the loading state (set to false) https://github.com/Expensify/App/blob/08b7ff41656a34373761fb2bd505cc0dfa64be7b/src/pages/workspace/WorkspaceNewRoomPage.tsx#L128-L130

and this condition (wasLoading && !isLoading) is also becomes true.

What changes do you think we should make in order to solve the problem?

The useEffect is added (in https://github.com/Expensify/App/pull/31655) so it will only close the page when the request is successful, but we added back the code to close it immediately in Report.addPolicyReport, so it's really weird, I think it's a bad merge, so we can remove the dismiss code here. https://github.com/Expensify/App/blob/08b7ff41656a34373761fb2bd505cc0dfa64be7b/src/libs/actions/Report.ts#L2012-L2013

To fix this issue, we can set initWithStoredValues: false to the new room form state. This way, the data will always be empty initially. We don't want the previous state to affect the new page.

formState: {
    key: ONYXKEYS.FORMS.NEW_ROOM_FORM,
    initWithStoredValues: false,
},

Then, we can remove this effect https://github.com/Expensify/App/blob/08b7ff41656a34373761fb2bd505cc0dfa64be7b/src/pages/workspace/WorkspaceNewRoomPage.tsx#L128-L130

Or trigger it when it's unmounted instead.

NOTE: there is currently a bug in Onyx where initWithStoredValues doesn't work. To fix it we should initialize the onyx key only if initWithStoredValues is true.

-if (
+if (mapping.initWithStoredValues &&
    ((value !== undefined
        && !Onyx.hasPendingMergeForKey(key))
    || mapping.allowStaleData)
) {
melvin-bot[bot] commented 6 days ago

Job added to Upwork: https://www.upwork.com/jobs/~0114d6fec2c1cc3911

melvin-bot[bot] commented 6 days ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr (External)

muttmuure commented 4 days ago

@mollfpr there's a proposal above for you to take a look at

mollfpr commented 4 days ago

@bernhardoj Is there any conversation regarding the bug of initWithStoredValues? It should be critical if the property doesn't work.

bernhardoj commented 4 days ago

@mollfpr I'm not aware of any conversation of that