App
App copied to clipboard
[$1000] Web -Chat -Changes made offline will not be saved after connecting to an internet connation, error message displayed
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:
- Navigate to http://staging.new.expensify.com/
- Log in with your account
- Sign in to an existing one or create one #Room
- Disable the internet connection on the device
- Click on the header of the #Room
- Click Settings
- In field "Notify me about new messages" select an option "Immediately" or "Mute"
- Go back online
- Refresh the page
- Click on the header of the #Room
- 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:
Triggered auto assignment to @luacmartins (Engineering
), see https://stackoverflow.com/c/expensify/questions/4319 for more details.
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.
This can be external.
Triggered auto assignment to @adelekennedy (External
), see https://stackoverflow.com/c/expensify/questions/8582 for more details.
Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External
)
Current assignee @luacmartins is eligible for the External assigner, not assigning anyone new.
Looking for proposals
The save Button is specifically responsible for only changing the room name. The notification preference gets updated when it is picked.
⚠️ 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.
@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!
Still looking for proposals
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 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:
- Explain the root cause.
- Show us, how you will solve it in our app.
- It should fit into the codebase.
- Include technical details.
@luacmartins, @parasharrajat, @adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Moving this to weekly as we look for proposals
We're close to doubling again - @luacmartins @parasharrajat do we have a cap in mind for this issue?
@adelekennedy no caps. Let's keep doubling until we get it fixed!
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
Thanks, @tienifr. It is indeed a backend issue. cc: @luacmartins
Thanks for the investigation. I'll take a look at our backend.
potentially a backend issue
I'll take a look at this tomorrow!
I think that this issue is related to https://github.com/Expensify/Expensify/issues/229696. The problem seems to be the order of events:
- App comes back online and we trigger ReconnectApp
- The server sends back the last know value, i.e.
Daily
. Since we hadMute
saved locally, that value gets saved to Onyx and we add aUpdateReportNotificationPreference
request with valueDaily
to the network queue - We trigger the
UpdateReportNotificationPreference
with valueMute
, that was added to the network queue while we were offline - We trigger the
UpdateReportNotificationPreference
added to the queue in 2 and the value is set toDaily
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?
Latest update above. I think this might be part of a bigger internal change to how we process the network queue.
Issue not reproducible during KI retests. (First week)
@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 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, @parasharrajat, @adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@luacmartins, @parasharrajat, @adelekennedy Whoops! This issue is 2 days overdue. Let's get this updated quick!