Skip to content

Commit

Permalink
fix(bridge)_: fix bridge replies not working when they come from Disc…
Browse files Browse the repository at this point in the history 
…ord (#5830)

Fixes status-im/status-desktop#16323

The problem was that the code expected to receive the Discord message ID, but the bridge is smart enough to return the Status message ID already, so there is no need to try and convert it.

There were also a couple issues in the bridge code itself.
  • Loading branch information
@jrainville
jrainville authored Sep 27, 2024
1 parent fc36a7e commit 2bbcce6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 80 deletions.
65 changes: 1 addition & 64 deletions protocol/message_persistence.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1580,11 +1580,6 @@ func (db sqlitePersistence) SaveMessages(messages []*common.Message) (err error)
if err != nil {
return
}
// handle replies
err = db.findAndUpdateReplies(tx, msg.GetBridgeMessage().MessageID, msg.ID)
if err != nil {
return
}
parentMessageID := msg.GetBridgeMessage().ParentMessageID
if parentMessageID != "" {
err = db.findAndUpdateRepliedTo(tx, parentMessageID, msg.ID)
Expand Down Expand Up @@ -2972,26 +2967,6 @@ func (db sqlitePersistence) GetCommunityMemberMessagesToDelete(member string, co
return result, nil
}

// Finds status messages id which are replies for bridgeMessageID
func (db sqlitePersistence) findStatusMessageIdsReplies(tx *sql.Tx, bridgeMessageID string) ([]string, error) {
rows, err := tx.Query(`SELECT user_messages_id FROM bridge_messages WHERE parent_message_id = ?`, bridgeMessageID)
if err != nil {
return []string{}, err
}
defer rows.Close()

var statusMessageIDs []string
for rows.Next() {
var statusMessageID string
err = rows.Scan(&statusMessageID)
if err != nil {
return []string{}, err
}
statusMessageIDs = append(statusMessageIDs, statusMessageID)
}
return statusMessageIDs, nil
}

func (db sqlitePersistence) FindStatusMessageIDForBridgeMessageID(messageID string) (string, error) {
rows, err := db.db.Query(`SELECT user_messages_id FROM bridge_messages WHERE message_id = ?`, messageID)
if err != nil {
Expand All @@ -3010,24 +2985,6 @@ func (db sqlitePersistence) FindStatusMessageIDForBridgeMessageID(messageID stri
return "", nil
}

func (db sqlitePersistence) findStatusMessageIDForBridgeMessageID(tx *sql.Tx, messageID string) (string, error) {
rows, err := tx.Query(`SELECT user_messages_id FROM bridge_messages WHERE message_id = ?`, messageID)
if err != nil {
return "", err
}
defer rows.Close()

if rows.Next() {
var statusMessageID string
err = rows.Scan(&statusMessageID)
if err != nil {
return "", err
}
return statusMessageID, nil
}
return "", nil
}

func (db sqlitePersistence) updateStatusMessagesWithResponse(tx *sql.Tx, statusMessagesToUpdate []string, responseValue string) error {
sql := "UPDATE user_messages SET response_to = ? WHERE id IN (?" + strings.Repeat(",?", len(statusMessagesToUpdate)-1) + ")" //nolint: gosec
stmt, err := tx.Prepare(sql)
Expand Down Expand Up @@ -3062,28 +3019,8 @@ func (db sqlitePersistence) updateBridgeMessageContent(tx *sql.Tx, bridgeMessage
return err
}

// Finds if there are any messages that are replies to that message (in case replies were received earlier)
func (db sqlitePersistence) findAndUpdateReplies(tx *sql.Tx, bridgeMessageID string, statusMessageID string) error {
replyMessageIds, err := db.findStatusMessageIdsReplies(tx, bridgeMessageID)
if err != nil {
return err
}
if len(replyMessageIds) == 0 {
return nil
}
return db.updateStatusMessagesWithResponse(tx, replyMessageIds, statusMessageID)
}

func (db sqlitePersistence) findAndUpdateRepliedTo(tx *sql.Tx, discordParentMessageID string, statusMessageID string) error {
// Finds status messages id which are replies for bridgeMessageID
repliedMessageID, err := db.findStatusMessageIDForBridgeMessageID(tx, discordParentMessageID)
if err != nil {
return err
}
if repliedMessageID == "" {
return nil
}
return db.updateStatusMessagesWithResponse(tx, []string{statusMessageID}, repliedMessageID)
return db.updateStatusMessagesWithResponse(tx, []string{statusMessageID}, discordParentMessageID)
}

func (db sqlitePersistence) GetCommunityMemberAllMessages(member string, communityID string) ([]*common.Message, error) {
Expand Down
32 changes: 16 additions & 16 deletions protocol/persistence_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1983,43 +1983,43 @@ func TestBridgeMessageReplies(t *testing.T) {

require.NoError(t, err)

err = insertMinimalBridgeMessage(p, "111", "1", "")
err = insertMinimalBridgeMessage(p, "message1", "discordId1", "")
require.NoError(t, err)

err = insertMinimalBridgeMessage(p, "222", "2", "1")
err = insertMinimalBridgeMessage(p, "message2", "discordId2", "message1")
require.NoError(t, err)

// "333 is not delivered yet"
// "message3 is not delivered yet"

// this is a reply to a message which was not delivered yet
err = insertMinimalBridgeMessage(p, "444", "4", "3")
err = insertMinimalBridgeMessage(p, "message4", "discordId4", "message3")
require.NoError(t, err)

// status message "222" should have reply_to = "111"
responseTo, err := messageResponseTo(p, "222")
// status message "message2" should have reply_to ="message1" because it's a discord message to another discord message
responseTo, err := messageResponseTo(p, "message2")
require.NoError(t, err)
require.Equal(t, "111", responseTo)
require.Equal(t, "message1", responseTo)

responseTo, err = messageResponseTo(p, "111")
responseTo, err = messageResponseTo(p, "message1")
require.NoError(t, err)
require.Equal(t, "", responseTo)

responseTo, err = messageResponseTo(p, "444")
responseTo, err = messageResponseTo(p, "message4")
require.NoError(t, err)
require.Equal(t, "", responseTo)
require.Equal(t, "message3", responseTo)

// receiving message for which "444" is replied to
err = insertMinimalBridgeMessage(p, "333", "3", "")
// receiving message for which "message4" is replied to
err = insertMinimalBridgeMessage(p, "message3", "discordId3", "")
require.NoError(t, err)

responseTo, err = messageResponseTo(p, "333")
responseTo, err = messageResponseTo(p, "message3")
require.NoError(t, err)
require.Equal(t, "", responseTo)

// now 444 is replied to 333
responseTo, err = messageResponseTo(p, "444")
// message4 is still replied to message3
responseTo, err = messageResponseTo(p, "message4")
require.NoError(t, err)
require.Equal(t, "333", responseTo)
require.Equal(t, "message3", responseTo)
}

func createAndSaveMessage(p *sqlitePersistence, id string, from string, deleted bool, communityID string) error {
Expand Down

0 comments on commit 2bbcce6

Please sign in to comment.