Skip to content

Commit

Permalink
fix!: ensure deleting message does not accidentally delete notificati…
Browse files Browse the repository at this point in the history
…ons that reference the message with last message
  • Loading branch information
seanstrom committed Aug 30, 2024
1 parent 82ba10e commit ba43762
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 10 deletions.
5 changes: 4 additions & 1 deletion protocol/activity_center_persistence.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ func (db sqlitePersistence) DeleteActivityCenterNotificationForMessage(chatID st

for _, notification := range notifications {
if notification.LastMessage != nil && notification.LastMessage.ID == messageID {
withNotification(notification)
switch notification.Type {
case ActivityCenterNotificationTypeNewOneToOne, ActivityCenterNotificationTypeNewPrivateGroupChat:
withNotification(notification)
}
}

if notification.Message != nil && notification.Message.ID == messageID {
Expand Down
118 changes: 110 additions & 8 deletions protocol/activity_center_persistence_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,17 +205,22 @@ func (s *ActivityCenterPersistenceTestSuite) Test_DeleteActivityCenterNotificati
}

// Test: soft delete the notifications that have Message.ID == messages[1].ID
// or LastMessage.ID == chat.LastMessage.
_, err = p.DeleteActivityCenterNotificationForMessage(chat.ID, messages[1].ID, currentMilliseconds())
s.Require().NoError(err)

for _, id := range []types.HexBytes{nID2, nID3} {
notif, err = p.GetActivityCenterNotificationByID(id)
s.Require().NoError(err, notif.ID)
s.Require().True(notif.Deleted, notif.ID)
s.Require().True(notif.Dismissed, notif.ID)
s.Require().True(notif.Read, notif.ID)
}
notif, err = p.GetActivityCenterNotificationByID(nID2)
s.Require().NoError(err, notif.ID)
s.Require().True(notif.Deleted, notif.ID)
s.Require().True(notif.Dismissed, notif.ID)
s.Require().True(notif.Read, notif.ID)

// Check that notifications with LastMessage.ID == messages[1].ID will remain.
// Unless the notification is of type NewOneToOne or NewPrivateGroupChat.
notif, err = p.GetActivityCenterNotificationByID(nID3)
s.Require().NoError(err)
s.Require().False(notif.Deleted, notif.ID)
s.Require().False(notif.Dismissed, notif.ID)
s.Require().False(notif.Read, notif.ID)

notif, err = p.GetActivityCenterNotificationByID(nID4)
s.Require().NoError(err)
Expand All @@ -228,6 +233,103 @@ func (s *ActivityCenterPersistenceTestSuite) Test_DeleteActivityCenterNotificati
s.Require().NoError(err)
}

func (s *ActivityCenterPersistenceTestSuite) Test_DeleteActivityCenterNotificationsForMessage_LastMessage() {
// docs: Create the temporary test-database that will be used to store chats, messages, and notifications
db, err := openTestDB()
s.Require().NoError(err)
p := newSQLitePersistence(db)

// docs: Create and save the public chat that will be used to group our test messages
chat := CreatePublicChat("test-chat", &testTimeSource{})
err = p.SaveChat(*chat)
s.Require().NoError(err)

// docs: Define multiple test messages for our chat so we can emulate a chat with a latest message.
messages := []*common.Message{
{
ID: "0x1",
ChatMessage: &protobuf.ChatMessage{},
LocalChatID: chat.ID,
},
{
ID: "0x2",
ChatMessage: &protobuf.ChatMessage{},
LocalChatID: chat.ID,
},
}
err = p.SaveMessages(messages)
s.Require().NoError(err)

chat.LastMessage = messages[1]
err = p.SaveChat(*chat)
s.Require().NoError(err)

chatMessages, _, err := p.MessageByChatID(chat.ID, "", 2)
s.Require().NoError(err)
s.Require().Len(chatMessages, 2)

// docs: Define multiple notifications of different types to emulate the removal of notifications when deleting a message
nID1 := types.HexBytes("1")
nID2 := types.HexBytes("2")
nID3 := types.HexBytes("3")
nID4 := types.HexBytes("4")

s.createNotifications(p, []*ActivityCenterNotification{
{
ID: nID1,
ChatID: chat.ID,
Type: ActivityCenterNotificationTypeMention,
Message: messages[0],
LastMessage: messages[1],
},
{
ID: nID2,
ChatID: chat.ID,
Type: ActivityCenterNotificationTypeMention,
Message: messages[1],
},
{
ID: nID3,
ChatID: chat.ID,
Type: ActivityCenterNotificationTypeNewOneToOne,
LastMessage: messages[1],
},
{
ID: nID4,
ChatID: chat.ID,
Type: ActivityCenterNotificationTypeNewPrivateGroupChat,
LastMessage: messages[1],
},
})

// Action: soft delete notifications related to a chat and message
_, err = p.DeleteActivityCenterNotificationForMessage(chat.ID, messages[1].ID, currentMilliseconds())
s.Require().NoError(err)

// Test: check that notifications unrelated to the message are not affected.
notif, err := p.GetActivityCenterNotificationByID(nID1)
s.Require().NoError(err)
s.Require().False(notif.Deleted, notif.ID)
s.Require().False(notif.Dismissed, notif.ID)
s.Require().False(notif.Read, notif.ID)

// Test: check notifications directly related to the message are soft deleted
notif, err = p.GetActivityCenterNotificationByID(nID2)
s.Require().NoError(err)
s.Require().True(notif.Deleted)
s.Require().True(notif.Dismissed)
s.Require().True(notif.Read)

// Test: check NewOneToOne or NewPrivateGroupChat notifications that are indirectly related to the message are soft deleted
for _, id := range []types.HexBytes{nID3, nID4} {
notif, err = p.GetActivityCenterNotificationByID(id)
s.Require().NoError(err)
s.Require().True(notif.Deleted)
s.Require().True(notif.Dismissed)
s.Require().True(notif.Read)
}
}

func (s *ActivityCenterPersistenceTestSuite) Test_AcceptActivityCenterNotificationsForInvitesFromUser() {
db, err := openTestDB()
s.Require().NoError(err)
Expand Down
2 changes: 1 addition & 1 deletion protocol/messenger_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ func (m *Messenger) createMessageNotification(chat *Chat, messageState *Received
notification := &ActivityCenterNotification{
ID: types.FromHex(chat.ID),
Name: chat.Name,
LastMessage: message,
Message: message,
Type: notificationType,
Author: messageState.CurrentMessageState.Contact.ID,
Timestamp: messageState.CurrentMessageState.WhisperTimestamp,
Expand Down

0 comments on commit ba43762

Please sign in to comment.