BUG: Out of order messages when creating 2 bill splits while offline

[neil-marcellini]

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. Go offline
2. From the global create menu +, select Split bill
3. Go through the prompt and select TWO or MORE participants that you have never split a bill with, and submit the request
4. Verify that the group chat is created with those participants and you are navigated to it (the whole chat should be greyed out)
5. Verify that the group chat has a message Split $X with and (this should be greyed out)
6. Verify that individual chats with each participant are created (these chats should be greyed out)
7. Verify that individual chats have the IOU message and preview (these messages should be greyed out)
8. From the group chat, tap + > Split bill and send another split bill request
9. Verify that the modal is dismissed
10. Verify that another split message shows up in the group chat (this message should be greyed out)
11. Verify that each individual chat with and show the new request and the total has been updated (these messages should be greyed out)
12. Turn on your internet connection and verify that the chats and messages are still the same, but are no longer greyed out.

Expected Result:

1. Messages in the split chats are always in order
2. After creating the second split while offline the chats in the LHN are grayed out

Actual Result:

1. Messages in the split chats are not always in order
2. After creating the second split while offline the chats in the LHN were no longer grayed out

Workaround:

Go back online and ignore the UI flaws.

Platform:

All

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

Version Number: v1.2.39-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
Email or phone of affected tester (no customers): N/A
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

split-offline-bugs.mp4

Expensify/Expensify Issue URL:
Issue reported by: @neil-marcellini
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1670980004459869

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d7aa95ed8ddc61fb
• Upwork Job ID: 1605621931009056768
• Last Price Increase: 2022-12-21

[neil-marcellini]

I put up a draft PR. I think another PR might have already fixed the issue where the chats appear out of order. I'll test that later and put the PR up for review if so.

[neil-marcellini]

Today I was working on my monthly issues so I didn't get to this.

[neil-marcellini]

Today I was working on my weekly issues so I didn't get to this.

[neil-marcellini]

I debugged the message ordering pretty thoroughly today. I'm not sure if it's worth fixing at the moment. Here's what I found.

I followed the offline test steps in the App PR which involves splitting a bill with 2 other users, requesting another split, viewing a 1:1 chat with the split, and then going back online. The first bill split was $1 per user, the next was $2 per user. I'm keeping track of the created timestamps on each report action because that's how they are currently sorted on the front end.

I request a bill split while offline and open one of the 1:1 chats. First I get
the created message and the $1 request. The created timestamps are
"2022-12-20 00:22:33.029",
"2022-12-20 00:22:33.029"

Then I request another split. I get the created, the $1, and the $2. The
timestamps are as follows.
"2022-12-20 00:22:33.029"
"2022-12-20 00:22:33.029",
"2022-12-20 00:22:54.451"

I go back online and start making API requests. First SplitBillAndOpenReport runs
for the first split. A Pusher response comes back with the created timestamp
from the server for the $1 IOU action. Now the messages are out of order; I get the
created, the $2, then the $1.
"2022-12-20 00:22:33.029",
"2022-12-20 00:22:54.451",
"2022-12-20 00:23:12.171"

Now the created action is out of order. I get the $2, the created, and the $1.
"2022-12-20 00:22:54.451",
"2022-12-20 00:23:12",
"2022-12-20 00:23:12.171"

So it's definitely odd here that the created timestamp is later than the $1
request, because those two should be created by the same api command.

Finally the values are sorted properly as created, $1, $2.
"2022-12-20 00:23:12",
"2022-12-20 00:23:12.171",
"2022-12-20 00:23:16.724"

The results above are a case of the "replay effect" where actions are replayed when the user goes back online.

If the created report action from the server came back first the results would look like this.

1. Both splits are created while offline. The order is created, $1, $2.
2. The user goes online and the first split is processed. The created action and the $1 IOU action are pushed together. The order is $2, created, $1.
3. Next the second split is processed and the order is correctly set to created, $1, $2. The replay effect will always cause a problem here even if the

[neil-marcellini]

Tomorrow I will test the PR to fix the grayed out state in the LHN and I'll put it up for review as a partial fix. I'm holding the out of order messages part on the issue to fix the replay effect.

[neil-marcellini]

The grayed out state fix is merged. This issue #13310 is the same problem so I'm going to close this one and let the problem get solved in the issue eventually.

[neil-marcellini]

Actually, I should leave this open until the related PR is deployed.

[neil-marcellini]

Here's the PR, GH doesn't seem to automatically link it right now #13602

[neil-marcellini]

Since this issue is locked the Bug Zero checklist didn't post, so here it is.

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@neil-marcellini] The PR that introduced the bug has been identified. Link to the PR:
  N/A graying out the chats in the side bar was not something that we decided to do until after the initial implementation.

• [@neil-marcellini] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  N/A

• [@neil-marcellini] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  N/A

• [@johncschuster] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here: https://github.com/Expensify/Expensify/issues/260938

[johncschuster]

@mananjadhav (C+ reviewer) can you apply for the Upwork job, here?

https://www.upwork.com/jobs/~01d7aa95ed8ddc61fb

[johncschuster]

This has been paid out! I'll work on the regression test shortly.

[neil-marcellini]

@johncschuster any update on this? It's been a month and I think Melvin didn't ping because the issue is locked. Also because the regressions steps weren't updated we have this issue as well #14560.

[Beamanator]

@johncschuster how did you comment on this last YEAR

Screen.Recording.2023-02-07.at.1.48.13.PM.mov

[johncschuster]

Oy! Sorry guys. I've been a bit behind on GH notifs. Working on the regression steps now!

[johncschuster]

Oh! It looks like the QA steps from the linked PR are already included in the TestSuite, here.

[johncschuster]

PR

TestRail

[neil-marcellini]

Ok cool. The other issue was not following these test steps as far as I can tell. Maybe we can make them more clear, or maybe they are good as is? What do you think @kbecciv?

[neil-marcellini]

We could update step 8 like so to describe the expected behavior regarding participants being grayed out on the second pending split.

Navigate to the created group chat and tap + > Split bill and send another split bill request (the participants with a pending split should be grayed out)

[johncschuster]

Great suggestion! I've created an issue for that proposed change, here.

[neil-marcellini]

🚀

[johncschuster]

Are we good to close this one out?

[neil-marcellini]

It looks like you already completed the payments so I updated the issue title. Let's wait until the regression test is actually updated and this issue is closed https://github.com/Expensify/Expensify/issues/260938. @johncschuster please ping the #qa channel in Slack. I find that's usually required to get someone on the issue quickly.

[MelvinBot]

@johncschuster, @neil-marcellini Whoops! This issue is 2 days overdue. Let's get this updated quick!

[neil-marcellini]

The regression test issue is closed now so this is done!