Skip to content

Commit

Permalink
fix(chat/messages): set clock value for new messages marker
Browse files Browse the repository at this point in the history
New messages marker had a clock value of "0" before. Since all
messages are inserted based on the clock value, new messages marker
would cause other "0"-valued clock items to be inserted after it,
effectively making chat header being displayed in the middle of the
chat.

Setting new messages marker clock value to the clock of the message it
points to solves the issue.

fixes: #8955
  • Loading branch information
osmaczko committed Jan 11, 2023
1 parent fa6f0dd commit fe4e14f
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 22 deletions.
4 changes: 2 additions & 2 deletions src/app/modules/shared_models/message_item.nim
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ proc initItem*(
if attachment.contentType.contains("image"):
result.messageAttachments.add(attachment.localUrl)

proc initNewMessagesMarkerItem*(timestamp: int64): Item =
proc initNewMessagesMarkerItem*(clock, timestamp: int64): Item =
return initItem(
id = "",
communityId = "",
Expand All @@ -148,7 +148,7 @@ proc initNewMessagesMarkerItem*(timestamp: int64): Item =
messageContainsMentions = false,
seen = true,
timestamp = timestamp,
clock = 0,
clock = clock,
ContentType.NewMessagesMarker,
messageType = -1,
contactRequestState = 0,
Expand Down
2 changes: 1 addition & 1 deletion src/app/modules/shared_models/message_model.nim
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ QtObject:
defer: parentModelIndex.delete

self.beginInsertRows(parentModelIndex, position, position)
self.items.insert(initNewMessagesMarkerItem(self.items[index].timestamp), position)
self.items.insert(initNewMessagesMarkerItem(self.items[index].clock, self.items[index].timestamp), position)
self.endInsertRows()
self.countChanged()

Expand Down
185 changes: 166 additions & 19 deletions test/nim/message_model_test.nim
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ proc createTestMessageItem(id: string, clock: int64): Item =
seen = true,
timestamp = 0,
clock = clock,
ContentType.NewMessagesMarker,
ContentType.Unknown,
messageType = -1,
contactRequestState = 0,
sticker = "",
Expand All @@ -41,19 +41,23 @@ proc createTestMessageItem(id: string, clock: int64): Item =
mentioned = false
)

let message0_chatIdentifier = createTestMessageItem("chat-identifier", 0)
let message0_fetchMoreMessages = createTestMessageItem("fetch-more-messages", 0)
let message1 = createTestMessageItem("0xa", 1)
let message2 = createTestMessageItem("0xb", 2)
let message3 = createTestMessageItem("0xc", 3)
let message4 = createTestMessageItem("0xd", 3)
let message5 = createTestMessageItem("0xe", 4)

template checkOrder(model: Model) =
require(model.items.len == 5)
require(model.items.len == 7)
check(model.items[0].id == message5.id)
check(model.items[1].id == message4.id)
check(model.items[2].id == message3.id)
check(model.items[3].id == message2.id)
check(model.items[4].id == message1.id)
check(model.items[5].id == message0_fetchMoreMessages.id)
check(model.items[6].id == message0_chatIdentifier.id)

suite "empty model":
let model = newModel()
Expand All @@ -65,56 +69,60 @@ suite "empty model":
suite "inserting new messages":
setup:
let model = newModel()
model.appendItem(message0_fetchMoreMessages)
model.appendItem(message0_chatIdentifier)

test "insert same message twice":
model.insertItemBasedOnClock(message1)
check(model.rowCount() == 1)
check(model.rowCount() == 3)
model.insertItemBasedOnClock(message1)
check(model.rowCount() == 1)
check(model.rowCount() == 3)

test "insert in order":
model.insertItemBasedOnClock(message1)
check(model.rowCount() == 1)
check(model.rowCount() == 3)
model.insertItemBasedOnClock(message2)
check(model.rowCount() == 2)
check(model.rowCount() == 4)
model.insertItemBasedOnClock(message3)
check(model.rowCount() == 3)
check(model.rowCount() == 5)
model.insertItemBasedOnClock(message4)
check(model.rowCount() == 4)
check(model.rowCount() == 6)
model.insertItemBasedOnClock(message5)
check(model.rowCount() == 5)
check(model.rowCount() == 7)
checkOrder(model)

test "insert out of order":
model.insertItemBasedOnClock(message5)
check(model.rowCount() == 1)
check(model.rowCount() == 3)
model.insertItemBasedOnClock(message4)
check(model.rowCount() == 2)
check(model.rowCount() == 4)
model.insertItemBasedOnClock(message3)
check(model.rowCount() == 3)
check(model.rowCount() == 5)
model.insertItemBasedOnClock(message2)
check(model.rowCount() == 4)
check(model.rowCount() == 6)
model.insertItemBasedOnClock(message1)
check(model.rowCount() == 5)
check(model.rowCount() == 7)
checkOrder(model)

test "insert out of order (randomly)":
model.insertItemBasedOnClock(message3)
check(model.rowCount() == 1)
check(model.rowCount() == 3)
model.insertItemBasedOnClock(message1)
check(model.rowCount() == 2)
check(model.rowCount() == 4)
model.insertItemBasedOnClock(message4)
check(model.rowCount() == 3)
check(model.rowCount() == 5)
model.insertItemBasedOnClock(message2)
check(model.rowCount() == 4)
check(model.rowCount() == 6)
model.insertItemBasedOnClock(message5)
check(model.rowCount() == 5)
check(model.rowCount() == 7)
checkOrder(model)

# assumption: each append sequence is already sorted
suite "appending new messages":
setup:
let model = newModel()
model.appendItem(message0_fetchMoreMessages)
model.appendItem(message0_chatIdentifier)

test "append empty model":
model.appendItems(@[message5,
Expand Down Expand Up @@ -147,3 +155,142 @@ suite "appending new messages":
model.appendItems(@[message4,
message2])
checkOrder(model)

suite "new messages marker":
setup:
let model = newModel()
model.appendItem(message0_fetchMoreMessages)
model.appendItem(message0_chatIdentifier)

test "set new messages marker":
model.appendItems(@[message1,
message2])
require(model.items.len == 4)

# add new messages marker
model.setFirstUnseenMessageId(message2.id)
model.resetNewMessagesMarker()

require(model.items.len == 5)
check(model.items[0].id == message2.id)
check(model.items[1].contentType == ContentType.NewMessagesMarker)
check(model.items[2].id == message1.id)
check(model.items[3].id == message0_fetchMoreMessages.id)
check(model.items[4].id == message0_chatIdentifier.id)

test "remove new messages marker":
model.appendItems(@[message1,
message2])
require(model.items.len == 4)

# add new messages marker
model.setFirstUnseenMessageId(message2.id)
model.resetNewMessagesMarker()
require(model.items.len == 5)

# remove new messages marker
model.setFirstUnseenMessageId("")
model.resetNewMessagesMarker()
require(model.items.len == 4)
check(model.items[0].id == message2.id)
check(model.items[1].id == message1.id)
check(model.items[2].id == message0_fetchMoreMessages.id)
check(model.items[3].id == message0_chatIdentifier.id)

suite "simulations":
setup:
let model = newModel()
model.appendItem(message0_fetchMoreMessages)
model.appendItem(message0_chatIdentifier)

test "simulate messages loading":
# load first two messages
var loadedMessages: seq[Item]
loadedMessages.add(message5)
loadedMessages.add(message4)
model.removeItem(message0_fetchMoreMessages.id)
model.removeItem(message0_chatIdentifier.id)
loadedMessages.add(message0_fetchMoreMessages)
loadedMessages.add(message0_chatIdentifier)
model.appendItems(loadedMessages)

require(model.items.len == 4)
check(model.items[0].id == message5.id)
check(model.items[1].id == message4.id)
check(model.items[2].id == message0_fetchMoreMessages.id)
check(model.items[3].id == message0_chatIdentifier.id)

# set new messages marker
model.setFirstUnseenMessageId(message5.id)
model.resetNewMessagesMarker()

require(model.items.len == 5)
check(model.items[0].id == message5.id)
check(model.items[1].contentType == ContentType.NewMessagesMarker)
check(model.items[2].id == message4.id)
check(model.items[3].id == message0_fetchMoreMessages.id)
check(model.items[4].id == message0_chatIdentifier.id)

# load next two messages
loadedMessages = @[]
loadedMessages.add(message3)
loadedMessages.add(message2)
model.removeItem(message0_fetchMoreMessages.id)
model.removeItem(message0_chatIdentifier.id)
loadedMessages.add(message0_fetchMoreMessages)
loadedMessages.add(message0_chatIdentifier)
model.appendItems(loadedMessages)

require(model.items.len == 7)
check(model.items[0].id == message5.id)
check(model.items[1].contentType == ContentType.NewMessagesMarker)
check(model.items[2].id == message4.id)
check(model.items[3].id == message3.id)
check(model.items[4].id == message2.id)
check(model.items[5].id == message0_fetchMoreMessages.id)
check(model.items[6].id == message0_chatIdentifier.id)

# load last message
loadedMessages = @[]
loadedMessages.add(message1)
model.removeItem(message0_fetchMoreMessages.id)
model.removeItem(message0_chatIdentifier.id)
loadedMessages.add(message0_fetchMoreMessages)
loadedMessages.add(message0_chatIdentifier)
model.appendItems(loadedMessages)

require(model.items.len == 8)
check(model.items[0].id == message5.id)
check(model.items[1].contentType == ContentType.NewMessagesMarker)
check(model.items[2].id == message4.id)
check(model.items[3].id == message3.id)
check(model.items[4].id == message2.id)
check(model.items[5].id == message1.id)
check(model.items[6].id == message0_fetchMoreMessages.id)
check(model.items[7].id == message0_chatIdentifier.id)

test "simulate chat identifier update":
model.appendItems(@[message5,
message4,
message3,
message2,
message1])
checkOrder(model)

# set new messages marker
model.setFirstUnseenMessageId(message4.id)
model.resetNewMessagesMarker()

# update chat identifier
model.removeItem(message0_chatIdentifier.id)
model.appendItem(message0_chatIdentifier)

require(model.items.len == 8)
check(model.items[0].id == message5.id)
check(model.items[1].id == message4.id)
check(model.items[2].contentType == ContentType.NewMessagesMarker)
check(model.items[3].id == message3.id)
check(model.items[4].id == message2.id)
check(model.items[5].id == message1.id)
check(model.items[6].id == message0_fetchMoreMessages.id)
check(model.items[7].id == message0_chatIdentifier.id)

0 comments on commit fe4e14f

Please sign in to comment.