[CIS-974] Messages maintain correct order of the local time is off #1246
Conversation
VojtaStavik
changed the title
VojtaStavik
force-pushed
the
CIS-974-local-message-ordering
branch
from
dba55d2 to
fb7e84b
Compare
Codecov Report
@@ Coverage Diff @@
## main #1246 +/- ##
==========================================
+ Coverage 90.34% 91.52% +1.17%
==========================================
Files 174 219 +45
Lines 7067 9400 +2333
==========================================
+ Hits 6385 8603 +2218
- Misses 682 797 +115
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
| /// We use this value as the offset for the sorting key for unsent messages. Even if the local time of the device | ||
| /// is different from the server time, we want to be sure the newly sent message is always the last one. By applying | ||
| /// the same offset to all unsent message, we make sure the order of the messages is maintained. | ||
| static let localSortingKeyTimeOffset: TimeInterval = 24 * 60 * 60 |
There was a problem hiding this comment.
I don't think we should do this, in some cases the failed message will make no sense if re-ordered (in the meantime newer messages will be in the channel).
There was a problem hiding this comment.
Good point 👍 We can either ignore this edge case (all iPhones have the time synced remotely by default - you have to explicitly turn this off and then change the time), or we can just make sure the locallyCreatedAt is always bigger than the last message in the channel we locally have. WDYT?
| /// Makes sure the `defaultSortingKey` value is computed and set. | ||
| fileprivate func prepareDefaultSortKeyIfNeeded() { | ||
| let newSortingKey = locallyCreatedAt ?? createdAt | ||
| let newSortingKey = locallyCreatedAt.map { $0.addingTimeInterval(Self.localSortingKeyTimeOffset) } ?? createdAt |
There was a problem hiding this comment.
disclaimer: this is unrelated to your change and possible N/A
From a db perspective computing the sort key sounds is very inefficient (all rows/objects will get a computed value which will be used to sort, gets slower as you data set grows). You normally want your sort key to come from an indexed column
There was a problem hiding this comment.
The value is not computed on the fly. We add an additional column that's re-computed every time the object is changed (see the willSave method) and the db uses this value for sorting. This is a limitation of CoreData which doesn't allow sorting by more complex expression (see also channels where we sort by max(createdAt, lastMessageAt).
However, this column is not indexed atm and it should be. Fixing 👍
b-onc
force-pushed
the
CIS-974-local-message-ordering
branch
from
fb7e84b to
2242d28
Compare
[Fixes #1231]
We didn't reset
locallyCreatedAtvalue after the message was sent which was causing the wrong ordering of the messages when the local time was different from the server time.I also added an artificial time offset for the unsent messages sorting key such that the most recent message is always at the bottom of the list.