Skip to content

Commit

Permalink
fix(pins): fix old messages not showing as pinned
Browse files Browse the repository at this point in the history
Fixes #16896

The problem was that we relied on the `SIGNAL_PINNED_MESSAGES_LOADED` event to apply the pinned status to messages, but that only happens at the start and a lot of messages are not loaded at start if they are not in the first 30 messages.

To fix this, I just added `pinnedBy` to the Message object in status-go. This way, we can easily tell straight from the MessageDto if a message is pinned and by whom.
  • Loading branch information
jrainville committed Jan 6, 2025
1 parent 37a06fc commit cdeaf56
Show file tree
Hide file tree
Showing 11 changed files with 30 additions and 26 deletions.
3 changes: 2 additions & 1 deletion src/app/modules/main/activity_center/module.nim
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ proc createMessageItemFromDto(self: Module, message: MessageDto, communityId: st
imagesAlbum.add(msg.image)
albumMessageIds.add(msg.id)

return msg_item_qobj.newMessageItem(msg_item.initItem(
return msg_item_qobj.newMessageItem(msg_item.initMessageItem(
message.id,
communityId, # we don't received community id via `activityCenterNotifications` api call
message.chatId,
Expand Down Expand Up @@ -144,6 +144,7 @@ proc createMessageItemFromDto(self: Module, message: MessageDto, communityId: st
message.deleted,
message.deletedBy,
deletedByContactDetails = ContactDetails(),
message.pinnedBy,
message.mentioned,
message.quotedMessage.`from`,
message.quotedMessage.text,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,6 @@ proc init*(self: Controller) =
return
self.delegate.newMessagesLoaded(args.messages, args.reactions)

self.events.on(SIGNAL_PINNED_MESSAGES_LOADED) do(e:Args):
let args = PinnedMessagesLoadedArgs(e)
if(self.chatId != args.chatId):
return
self.delegate.newPinnedMessagesLoaded(args.pinnedMessages)

self.events.on(SIGNAL_NEW_MESSAGE_RECEIVED) do(e: Args):
var args = MessagesArgs(e)
if(self.chatId != args.chatId):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ method updateChatFetchMoreMessages*(self: AccessInterface) {.base.} =
method newMessagesLoaded*(self: AccessInterface, messages: seq[MessageDto], reactions: seq[ReactionDto]) {.base.} =
raise newException(ValueError, "No implementation available")

method newPinnedMessagesLoaded*(self: AccessInterface, pinnedMessages: seq[PinnedMessageDto]) {.base.} =
raise newException(ValueError, "No implementation available")

method onReactionAdded*(self: AccessInterface, messageId: string, emojiId: int, reactionId: string) {.base.} =
raise newException(ValueError, "No implementation available")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ proc createMessageItemsFromMessageDtos(self: Module, messages: seq[MessageDto],
if message.transactionParameters.fromAddress != "":
isCurrentUser = self.currentUserWalletContainsAddress(message.transactionParameters.fromAddress)

var item = initItem(
var item = initMessageItem(
message.id,
message.chatId,
message.communityId,
Expand Down Expand Up @@ -193,6 +193,7 @@ proc createMessageItemsFromMessageDtos(self: Module, messages: seq[MessageDto],
message.deleted,
message.deletedBy,
deletedByContactDetails,
message.pinnedBy,
message.mentioned,
message.quotedMessage.`from`,
message.quotedMessage.text,
Expand Down Expand Up @@ -238,7 +239,7 @@ proc createMessageItemsFromMessageDtos(self: Module, messages: seq[MessageDto],

proc createFetchMoreMessagesItem(self: Module): Item =
let chatDto = self.controller.getChatDetails()
result = initItem(
result = initMessageItem(
FETCH_MORE_MESSAGES_MESSAGE_ID,
communityId = "",
chatId = "",
Expand Down Expand Up @@ -275,6 +276,7 @@ proc createFetchMoreMessagesItem(self: Module): Item =
deleted = false,
deletedBy = "",
deletedByContactDetails = ContactDetails(),
pinnedBy = "",
mentioned = false,
quotedMessageFrom = "",
quotedMessageText = "",
Expand Down Expand Up @@ -307,7 +309,7 @@ proc createChatIdentifierItem(self: Module): Item =
(chatName, smallImage, chatIcon) = self.controller.getOneToOneChatNameAndImage()
senderColorHash = sender.colorHash

result = initItem(
result = initMessageItem(
CHAT_IDENTIFIER_MESSAGE_ID,
communityId = "",
chatId = "",
Expand Down Expand Up @@ -344,6 +346,7 @@ proc createChatIdentifierItem(self: Module): Item =
deleted = false,
deletedBy = "",
deletedByContactDetails = ContactDetails(),
pinnedBy = "",
mentioned = false,
quotedMessageFrom = "",
quotedMessageText = "",
Expand Down Expand Up @@ -420,10 +423,6 @@ method newMessagesLoaded*(self: Module, messages: seq[MessageDto], reactions: se
self.initialMessagesLoaded = true
self.reevaluateViewLoadingState()

method newPinnedMessagesLoaded*(self: Module, pinnedMessages: seq[PinnedMessageDto]) =
for p in pinnedMessages:
self.onPinMessage(p.message.id, p.pinnedBy)

method messagesAdded*(self: Module, messages: seq[MessageDto]) =
let items = self.createMessageItemsFromMessageDtos(messages)

Expand Down
3 changes: 2 additions & 1 deletion src/app/modules/main/chat_section/chat_content/module.nim
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ proc buildPinnedMessageItem(self: Module, message: MessageDto, actionInitiatedBy
(transactionContract, transactionValue) = self.controller.getTransactionDetails(message)
if message.transactionParameters.fromAddress != "":
isCurrentUser = self.currentUserWalletContainsAddress(message.transactionParameters.fromAddress)
item = pinned_msg_item.initItem(
item = pinned_msg_item.initMessageItem(
message.id,
message.communityId,
message.chatId,
Expand Down Expand Up @@ -219,6 +219,7 @@ proc buildPinnedMessageItem(self: Module, message: MessageDto, actionInitiatedBy
message.deleted,
message.deletedBy,
deletedByContactDetails = ContactDetails(),
message.pinnedBy,
message.mentioned,
message.quotedMessage.`from`,
message.quotedMessage.text,
Expand Down
3 changes: 2 additions & 1 deletion src/app/modules/main/chat_section/module.nim
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ proc buildCommunityMemberMessageItem(self: Module, message: MessageDto): member_
(transactionContract, transactionValue) = self.controller.getTransactionDetails(message)
if message.transactionParameters.fromAddress != "":
isCurrentUser = self.currentUserWalletContainsAddress(message.transactionParameters.fromAddress)
return member_msg_item.initItem(
return member_msg_item.initMessageItem(
message.id,
message.communityId,
message.chatId,
Expand Down Expand Up @@ -200,6 +200,7 @@ proc buildCommunityMemberMessageItem(self: Module, message: MessageDto): member_
message.deleted,
message.deletedBy,
deletedByContactDetails = ContactDetails(),
message.pinnedBy,
message.mentioned,
message.quotedMessage.`from`,
message.quotedMessage.text,
Expand Down
9 changes: 7 additions & 2 deletions src/app/modules/shared_models/message_item.nim
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ type
bridgeName: string
paymentRequestModel: payment_request_model.Model

proc initItem*(
proc initMessageItem*(
id,
communityId,
chatId,
Expand Down Expand Up @@ -113,6 +113,7 @@ proc initItem*(
deleted: bool,
deletedBy: string,
deletedByContactDetails: ContactDetails,
pinnedBy: string,
mentioned: bool,
quotedMessageFrom: string,
quotedMessageText: string,
Expand Down Expand Up @@ -164,6 +165,9 @@ proc initItem*(
result.deleted = deleted
result.deletedBy = deletedBy
result.deletedByContactDetails = deletedByContactDetails
result.pinnedBy = pinnedBy
if pinnedBy != "":
result.pinned = true
result.links = links
result.linkPreviewModel = newLinkPreviewModel(linkPreviews)
result.emojiReactionsModel = newEmojiReactionsModel()
Expand Down Expand Up @@ -233,7 +237,7 @@ proc initItem*(
result.bridgeName = bridgeMessage.bridgeName

proc initNewMessagesMarkerItem*(clock, timestamp: int64): Item =
return initItem(
return initMessageItem(
id = "",
communityId = "",
chatId = "",
Expand Down Expand Up @@ -270,6 +274,7 @@ proc initNewMessagesMarkerItem*(clock, timestamp: int64): Item =
deleted = false,
deletedBy = "",
deletedByContactDetails = ContactDetails(),
pinnedBy = "",
mentioned = false,
quotedMessageFrom = "",
quotedMessageText = "",
Expand Down
9 changes: 6 additions & 3 deletions src/app/modules/shared_models/message_model.nim
Original file line number Diff line number Diff line change
Expand Up @@ -601,12 +601,15 @@ QtObject:
defer: index.delete
self.dataChanged(index, index, @[ModelRole.Reactions.int])

proc pinUnpinMessage*(self: Model, messageId: string, pin: bool, pinnedBy: string) =
proc pinUnpinMessage*(self: Model, messageId: string, pinned: bool, pinnedBy: string) =
let ind = self.findIndexForMessageId(messageId)
if(ind == -1):
if ind == -1:
return

if self.items[ind].pinned == pinned:
return

self.items[ind].pinned = pin
self.items[ind].pinned = pinned
self.items[ind].pinnedBy = pinnedBy

let index = self.createIndex(ind, 0, nil)
Expand Down
2 changes: 2 additions & 0 deletions src/app_service/service/message/dto/message.nim
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ type MessageDto* = object
deleted*: bool
deletedBy*: string
deletedForMe*: bool
pinnedBy*: string
transactionParameters*: TransactionParameters
mentioned*: bool
replied*: bool
Expand Down Expand Up @@ -277,6 +278,7 @@ proc toMessageDto*(jsonObj: JsonNode): MessageDto =
# The message was deleted by the sender itself
result.deletedBy = result.`from`
discard jsonObj.getProp("deletedForMe", result.deletedForMe)
discard jsonObj.getProp("pinnedBy", result.pinnedBy)
discard jsonObj.getProp("mentioned", result.mentioned)
discard jsonObj.getProp("replied", result.replied)

Expand Down
3 changes: 2 additions & 1 deletion test/nim/message_model_test.nim
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import app/modules/shared_models/message_item
import app/modules/shared_models/message_transaction_parameters_item

proc createTestMessageItem(id: string, clock: int64): Item =
return initItem(
return initMessageItem(
id = id,
communityId = "",
chatId = "",
Expand Down Expand Up @@ -46,6 +46,7 @@ proc createTestMessageItem(id: string, clock: int64): Item =
deleted = false,
deletedBy = "",
deletedByContactDetails = ContactDetails(),
pinnedBy = "",
mentioned = false,
quotedMessageFrom = "",
quotedMessageText = "",
Expand Down
2 changes: 1 addition & 1 deletion vendor/status-go
Submodule status-go updated 34 files
+2 −1 .github/workflows/pytest-lint.yml
+2 −1 _assets/scripts/run_functional_tests.sh
+18 −1 abi-spec/utils.go
+3 −0 abi-spec/utils_test.go
+1 −1 go.mod
+2 −2 go.sum
+15 −6 mobile/status.go
+5 −0 protocol/common/message.go
+11 −4 protocol/message_persistence.go
+17 −0 protocol/persistence_test.go
+15 −0 tests-functional/clients/services/accounts.py
+1 −1 tests-functional/clients/services/service.py
+11 −0 tests-functional/clients/services/settings.py
+40 −0 tests-functional/clients/services/wakuext.py
+3 −0 tests-functional/clients/services/wallet.py
+82 −79 tests-functional/clients/status_backend.py
+29 −32 tests-functional/tests/reliability/test_contact_request.py
+77 −0 tests-functional/tests/reliability/test_create_private_groups.py
+21 −30 tests-functional/tests/reliability/test_one_to_one_messages.py
+60 −21 tests-functional/tests/test_cases.py
+39 −45 tests-functional/tests/test_init_status_app.py
+4 −1 vendor/github.com/waku-org/go-waku/waku/v2/api/filter/filter_manager.go
+45 −10 vendor/github.com/waku-org/go-waku/waku/v2/api/missing/missing_messages.go
+2 −2 vendor/github.com/waku-org/go-waku/waku/v2/api/publish/message_sender.go
+8 −18 vendor/github.com/waku-org/go-waku/waku/v2/peermanager/peer_manager.go
+2 −0 vendor/github.com/waku-org/go-waku/waku/v2/peermanager/peer_selection.go
+12 −2 vendor/github.com/waku-org/go-waku/waku/v2/protocol/filter/client.go
+1 −1 vendor/github.com/waku-org/go-waku/waku/v2/protocol/filter/filter_health_check.go
+1 −1 vendor/github.com/waku-org/go-waku/waku/v2/protocol/filter/options.go
+5 −0 vendor/github.com/waku-org/go-waku/waku/v2/protocol/lightpush/waku_lightpush.go
+4 −0 vendor/github.com/waku-org/go-waku/waku/v2/protocol/store/client.go
+1 −1 vendor/modules.txt
+14 −5 wakuv2/waku.go
+4 −3 wakuv2/waku_test.go

0 comments on commit cdeaf56

Please sign in to comment.