App icon indicating copy to clipboard operation
App copied to clipboard

[$1000] Web -Chat -Changes made offline will not be saved after connecting to an internet connation, error message displayed

Open kbecciv opened this issue 2 years ago • 30 comments

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 http://staging.new.expensify.com/
  2. Log in with your account
  3. Sign in to an existing one or create one #Room
  4. Disable the internet connection on the device
  5. Click on the header of the #Room
  6. Click Settings
  7. In field "Notify me about new messages" select an option "Immediately" or "Mute"
  8. Go back online
  9. Refresh the page
  10. Click on the header of the #Room
  11. Click Settings

Expected Result:

The changes made offline are still displayed after refreshing the page, no error messages.

Actual Result:

Changes made offline will not be saved after connecting to an internet connection and an error message is displayed.

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.2.2.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any expensifail accounts

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

Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/93399543/191131346-93af82ad-2ede-4587-87b8-1728dc2f8748.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

kbecciv avatar Sep 19 '22 22:09 kbecciv

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

melvin-bot[bot] avatar Sep 19 '22 22:09 melvin-bot[bot]

I can reproduce this. It seems like something changes the value in Picker and triggers two calls to updateNotificationPreference, one with the new value and a second one with the old value. We need to identify what's triggering that second call and prevent it from happening.

luacmartins avatar Sep 20 '22 14:09 luacmartins

This can be external.

luacmartins avatar Sep 20 '22 14:09 luacmartins

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

melvin-bot[bot] avatar Sep 20 '22 14:09 melvin-bot[bot]

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

melvin-bot[bot] avatar Sep 20 '22 14:09 melvin-bot[bot]

Current assignee @luacmartins is eligible for the External assigner, not assigning anyone new.

melvin-bot[bot] avatar Sep 20 '22 14:09 melvin-bot[bot]

Looking for proposals

luacmartins avatar Sep 23 '22 16:09 luacmartins

The save Button is specifically responsible for only changing the room name. The notification preference gets updated when it is picked.

getusha avatar Sep 24 '22 05:09 getusha

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

melvin-bot[bot] avatar Sep 24 '22 14:09 melvin-bot[bot]

@getusha correct! The issue here is that the notifications preference is reset when the user comes back online. I removed step 8 from the OP since it was unrelated as you pointed out!

luacmartins avatar Sep 26 '22 18:09 luacmartins

Still looking for proposals

luacmartins avatar Sep 29 '22 17:09 luacmartins

Still looking for proposals

Proposal

Here is pseudo code I approach this by using state. first should check if the user is online if user is online save the change else set the change on pending user back online? save the change.

getusha avatar Sep 29 '22 18:09 getusha

@getusha Please go through https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md if you are new here.

That proposal is unacceptable in its current state. A proposal should:

  1. Explain the root cause.
  2. Show us, how you will solve it in our app.
  3. It should fit into the codebase.
  4. Include technical details.

parasharrajat avatar Sep 29 '22 19:09 parasharrajat

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

melvin-bot[bot] avatar Oct 03 '22 06:10 melvin-bot[bot]

Moving this to weekly as we look for proposals

luacmartins avatar Oct 03 '22 19:10 luacmartins

We're close to doubling again - @luacmartins @parasharrajat do we have a cap in mind for this issue?

adelekennedy avatar Oct 10 '22 15:10 adelekennedy

@adelekennedy no caps. Let's keep doubling until we get it fixed!

luacmartins avatar Oct 11 '22 16:10 luacmartins

Proposal

Problem

I see this is BE bug. Even when online (don't disable network connection), this issue still display. As the below video, when I change notificationPreferences, it call API update successfully. But when I reload page, It call OpenAPI and the response from this API is wrong

https://user-images.githubusercontent.com/113963320/195778502-a04a8e66-0877-4eab-bc27-494c1dbc937f.mp4

Solution

Fix API in BE

tienifr avatar Oct 14 '22 06:10 tienifr

Thanks, @tienifr. It is indeed a backend issue. cc: @luacmartins

parasharrajat avatar Oct 14 '22 12:10 parasharrajat

Thanks for the investigation. I'll take a look at our backend.

luacmartins avatar Oct 14 '22 19:10 luacmartins

potentially a backend issue

adelekennedy avatar Oct 17 '22 16:10 adelekennedy

I'll take a look at this tomorrow!

luacmartins avatar Oct 19 '22 23:10 luacmartins

I think that this issue is related to https://github.com/Expensify/Expensify/issues/229696. The problem seems to be the order of events:

  1. App comes back online and we trigger ReconnectApp
  2. The server sends back the last know value, i.e. Daily. Since we had Mute saved locally, that value gets saved to Onyx and we add a UpdateReportNotificationPreference request with value Daily to the network queue
  3. We trigger the UpdateReportNotificationPreference with value Mute, that was added to the network queue while we were offline
  4. We trigger the UpdateReportNotificationPreference added to the queue in 2 and the value is set to Daily

As discussed in this thread, we should be triggering any write requests first, before any read requests.

@jasperhuangg are we tracking all issues that depend on this network queue sequence somewhere?

luacmartins avatar Oct 21 '22 18:10 luacmartins

Latest update above. I think this might be part of a bigger internal change to how we process the network queue.

luacmartins avatar Oct 24 '22 15:10 luacmartins

Issue not reproducible during KI retests. (First week)

mvtglobally avatar Oct 24 '22 18:10 mvtglobally

@luacmartins @parasharrajat if this is tied to the issue linked above should we remove the help wanted tag here? Are we actually looking for proposals?

adelekennedy avatar Oct 27 '22 15:10 adelekennedy

@adelekennedy thanks for bringing that up. That makes sense to me. I'll mark this as internal as well, since this will touch our API layer.

luacmartins avatar Oct 27 '22 16:10 luacmartins

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

melvin-bot[bot] avatar Oct 31 '22 07:10 melvin-bot[bot]

@luacmartins, @parasharrajat, @adelekennedy Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] avatar Oct 31 '22 08:10 melvin-bot[bot]