-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Tracking] "Replay effect" when sequential actions are taken offline, and user subsequently comes online #12775
Comments
I created this issue last week, but it's honestly pretty annoying to have this in the queue given all the immediate work we have in front of us elsewhere. I want to believe that we can come back to this one and remember this issue/convo, though curious for your thoughts. |
It's a tracking issue, why does it bother you that exists? We don't have to work on it now... |
I'm with Ioni. I think it's fine, let's put the |
Nice, I'm good with that! |
Here's another manifestation of this convo. |
Still on hold for WAQ. |
I think that this issue with out of order messages when creating 2 bill splits while offline is another instance of this bug. Therefore, I'm going to hold it on this issue. |
Added that one to our OP of known issues. |
Thanks! I'm actually going to have that issue closed soon, and the part where the messages are out of order will be fixed in #13310. I'll update the description to link that instead. |
Still on hold and not something we're going to prioritize. |
We have this held on WAQ, so that should be removed now, but do we want to progress with finding a solution to this atm though? |
I vote that we start working on a fix now. It feels like a liability to have a bug in one of our most common offline patterns. |
I tend to agree we should figure out how we're going to plug this gap to improve the handling of adding/removing something offline. The "flashing" effect feels like a bug, though it is by design I believe, so technically it's still a |
yeah, kind of by design. |
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@chrispader as you know the Onyx bump was reverted. Since we need to repeat that Onyx bump PR, can you please upgrade the Node version in react-native-onyx to 20.10.0 and npm to 10.2.3, then do the same in E/App? |
yes, i can do that 👍 @roryabraham |
Merged the Node bump in Onyx |
A tad out of the loop here, is payment actually due? |
This issue has not been updated in over 15 days. @iwiznia, @JmillsExpensify, @trjExpensify, @neil-marcellini, @aimane-chnaif eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
@iwiznia, @JmillsExpensify, @trjExpensify, @neil-marcellini, @aimane-chnaif, this Monthly task hasn't been acted upon in 6 weeks; closing. If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead. |
C+ payment for reviewing PR is remaining |
@JmillsExpensify or @trjExpensify can you please reopen and handle payment? Thanks |
Dang, an issue from January? Payment summary as follows:
*$500 because this is an issue created prior to the switch to $250. Offer sent. |
Paid, closing! |
Problem
With Pattern B, offline creation events trigger a "pending" action that clears when the app comes back online. Similarly for deleting or removing content, a "strikethrough" action appears while offline and clears when the app comes back online. The pickle is that if a user takes a creation and deletion action while offline – whether that be uploading/removing an attachment, sending/deleting a message, adding/removing workspace members – content will flash or "replay" because actions in our app happen sequentially.
Here's an example of how this "replay" happens in practice:
However, again because of sequentiality, we see flashing in practice. The create action will run first, returning the data and showing the row. Next, the deleted action runs and we remove the row. Here's another pickle: when the creation action is run, we also send an email inviting the workspace member, but then that email is immediately invalidated. Confusing right?
tldr: When a user takes multiple write actions affecting the same Onyx key while offline and then goes back online, the actions are “replayed” which causes the UI to update unnecessarily and can cause flickering. We have already tracked 5 issues related to this problem in the App.
Solution
1. https://github.com/Expensify/Expensify/issues/270217 react-native-onyx: CreateNot necessary, Onyx already batches updates.Onyx.batchUpdate
which will do the same thing as Onyx.update, except that it won't notify subscribers until the last update is applied.2. https://github.com/Expensify/Expensify/issues/270219
2.1. App: Send the Pusher
socket_id
with all requests.2.2. App: [HOLD 1] While the SequentialQueue is processing, create a list of Onyx updates from each response. Once the SequentialQueue is empty, batch update Onyx.
3. https://github.com/Expensify/Expensify/issues/270220
3.1. Web-Expensify Send the Pusher
socket_id
with all Pusher events, preventing them from being delivered to that client.3.2. Web-Expensify [HOLD 2] Modify our api to return all Onyx updates in the response that were pushed to the requesting accountID.
Known Issues
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @trjExpensifyThe text was updated successfully, but these errors were encountered: