fix(pinned events): make sure to initialize ordering before reloading events from disk#6466
fix(pinned events): make sure to initialize ordering before reloading events from disk#6466bnjbvr wants to merge 2 commits into
Conversation
…an be reused for other linked chunks Makes sense to me to have it in the persistence file, since it's reloading all the chunks quickly to set the initial ordering.
…disk This fixes a subtle bug, found by re-enabling the assertion disabled in github.com//pull/5332. Since the events ordering was never properly initialized, events couldn't be ordered against each other even after being added etc. This could result in the incorrect latest edit being shown in a pinned events timeline (maybe? it's hard to make a standalone test case for this, so I'll really on people re-enabling the ordering assertion back in the future).
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6466 +/- ##
==========================================
+ Coverage 89.88% 89.89% +0.01%
==========================================
Files 380 380
Lines 105023 105026 +3
Branches 105023 105026 +3
==========================================
+ Hits 94395 94416 +21
+ Misses 7009 6993 -16
+ Partials 3619 3617 -2 ☔ View full report in Codecov by Sentry. |
Hywan
left a comment
There was a problem hiding this comment.
Do we have a failing test to ensure this patch fixes it?
|
The answer is in the second commit's message:
|
|
Fwiw, I've tried to write a failing test for this since the previous message, but I've realized that the ordering might not even be used in the case of the pinned events, so this is only fixing the assertion re-enabled in the test anyways. In other words: there's very little chance that this affects real-world users, at this point. |
See the commit description. Honestly, even after writing it myself, I do find that API is a bit of a mess; relying on the linked chunk updates emitted by the same linked chunk might be too hacky for the event ordering to function properly overall. But first, let's fix the bug.
CHANGELOG.mdfiles.