fix(mentions): deleting or editing a mention should remove the mention (#3421)

* fix(mentions): deleting or editing a mention should remove the mention

* test(edit): add a test for mentions in edits

* test(delete): add test for deleting a message with a mention

protocol/common/message.go

@@ -555,7 +555,7 @@ func (m *Message) PrepareContent(identity string) error {
 	m.Links = visitor.links
 	// Leave it set if already set, as sometimes we might run this without
 	// an identity
-	if !m.Mentioned {
+	if !m.Mentioned || identity != "" {
 		m.Mentioned = visitor.mentioned
 	}
 	jsonParsedText, err := json.Marshal(parsedText)

protocol/messenger_delete_message_test.go

@@ -391,3 +391,62 @@ func (s *MessengerDeleteMessageSuite) TestDeleteImageMessageFirstThenMessage() {
 	s.Require().Len(state.Response.RemovedMessages(), 0)
 	s.Require().Nil(state.Response.Chats()[0].LastMessage)
 }
+
+func (s *MessengerDeleteMessageSuite) TestDeleteMessageWithAMention() {
+	theirMessenger := s.newMessenger()
+	_, err := theirMessenger.Start()
+	s.Require().NoError(err)
+
+	theirChat := CreateOneToOneChat("Their 1TO1", &s.privateKey.PublicKey, s.m.transport)
+	err = theirMessenger.SaveChat(theirChat)
+	s.Require().NoError(err)
+
+	ourChat := CreateOneToOneChat("Our 1TO1", &theirMessenger.identity.PublicKey, s.m.transport)
+	err = s.m.SaveChat(ourChat)
+	s.Require().NoError(err)
+
+	inputMessage := buildTestMessage(*theirChat)
+	inputMessage.Text = "text with a mention @" + common.PubkeyToHex(&s.privateKey.PublicKey)
+	sendResponse, err := theirMessenger.SendChatMessage(context.Background(), inputMessage)
+	s.NoError(err)
+	s.Require().Len(sendResponse.Messages(), 1)
+
+	messageID := sendResponse.Messages()[0].ID
+
+	response, err := WaitOnMessengerResponse(
+		s.m,
+		func(r *MessengerResponse) bool { return len(r.messages) == 1 },
+		"no messages",
+	)
+	s.Require().NoError(err)
+	s.Require().Len(response.Chats(), 1)
+	s.Require().Len(response.Messages(), 1)
+	// Receiver (us) is mentioned
+	s.Require().Equal(int(response.Chats()[0].UnviewedMessagesCount), 1)
+	s.Require().Equal(int(response.Chats()[0].UnviewedMentionsCount), 1)
+	s.Require().True(response.Messages()[0].Mentioned)
+
+	deleteMessage := DeleteMessage{
+		DeleteMessage: protobuf.DeleteMessage{
+			Clock:       2,
+			MessageType: protobuf.MessageType_ONE_TO_ONE,
+			MessageId:   messageID,
+			ChatId:      theirChat.ID,
+		},
+		From: common.PubkeyToHex(&theirMessenger.identity.PublicKey),
+	}
+
+	state := &ReceivedMessageState{
+		Response: &MessengerResponse{},
+	}
+
+	// Handle Delete first
+	err = s.m.HandleDeleteMessage(state, deleteMessage)
+
+	s.Require().NoError(err)
+	s.Require().Len(response.Chats(), 1)
+	s.Require().Len(response.Messages(), 1)
+	// Receiver (us) is  no longer mentioned
+	s.Require().Equal(int(response.Chats()[0].UnviewedMessagesCount), 0)
+	s.Require().Equal(int(response.Chats()[0].UnviewedMentionsCount), 0)
+}

protocol/messenger_edit_message_test.go

@@ -374,3 +374,109 @@ func (s *MessengerEditMessageSuite) TestEditGroupChatMessage() {
 	s.Require().NotEmpty(response.Messages()[0].EditedAt)
 	s.Require().False(response.Messages()[0].New)
 }
+
+func (s *MessengerEditMessageSuite) TestEditMessageWithMention() {
+	theirMessenger := s.newMessenger()
+	_, err := theirMessenger.Start()
+	s.Require().NoError(err)
+
+	theirChat := CreateOneToOneChat("Their 1TO1", &s.privateKey.PublicKey, s.m.transport)
+	err = theirMessenger.SaveChat(theirChat)
+	s.Require().NoError(err)
+
+	ourChat := CreateOneToOneChat("Our 1TO1", &theirMessenger.identity.PublicKey, s.m.transport)
+	err = s.m.SaveChat(ourChat)
+	s.Require().NoError(err)
+
+	inputMessage := buildTestMessage(*theirChat)
+	// Send first message with no mention
+	sendResponse, err := theirMessenger.SendChatMessage(context.Background(), inputMessage)
+	s.NoError(err)
+	s.Require().Len(sendResponse.Messages(), 1)
+
+	response, err := WaitOnMessengerResponse(
+		s.m,
+		func(r *MessengerResponse) bool { return len(r.messages) == 1 },
+		"no messages",
+	)
+	s.Require().NoError(err)
+	s.Require().Len(response.Chats(), 1)
+	s.Require().Len(response.Messages(), 1)
+	// Make sure there is no mention at first
+	s.Require().Equal(int(response.Chats()[0].UnviewedMessagesCount), 1)
+	s.Require().Equal(int(response.Chats()[0].UnviewedMentionsCount), 0)
+	s.Require().False(response.Messages()[0].Mentioned)
+
+	ogMessage := sendResponse.Messages()[0]
+
+	messageID, err := types.DecodeHex(ogMessage.ID)
+	s.Require().NoError(err)
+
+	// Edit the message and add a mention
+	editedText := "edited text @" + common.PubkeyToHex(&s.privateKey.PublicKey)
+	editedMessage := &requests.EditMessage{
+		ID:   messageID,
+		Text: editedText,
+	}
+
+	sendResponse, err = theirMessenger.EditMessage(context.Background(), editedMessage)
+
+	s.Require().NoError(err)
+	s.Require().Len(sendResponse.Messages(), 1)
+	s.Require().NotEmpty(sendResponse.Messages()[0].EditedAt)
+	s.Require().Equal(sendResponse.Messages()[0].Text, editedText)
+	s.Require().Len(sendResponse.Chats(), 1)
+	s.Require().NotNil(sendResponse.Chats()[0].LastMessage)
+	s.Require().NotEmpty(sendResponse.Chats()[0].LastMessage.EditedAt)
+	s.Require().False(sendResponse.Messages()[0].Mentioned) // Sender is still not mentioned
+
+	response, err = WaitOnMessengerResponse(
+		s.m,
+		func(r *MessengerResponse) bool { return len(r.messages) == 1 },
+		"no messages",
+	)
+	s.Require().NoError(err)
+
+	s.Require().Len(response.Chats(), 1)
+	s.Require().Len(response.Messages(), 1)
+	s.Require().NotEmpty(response.Messages()[0].EditedAt)
+	s.Require().False(response.Messages()[0].New)
+	// Receiver (us) is now mentioned
+	s.Require().Equal(int(response.Chats()[0].UnviewedMessagesCount), 1)
+	s.Require().Equal(int(response.Chats()[0].UnviewedMentionsCount), 1)
+	s.Require().True(response.Messages()[0].Mentioned)
+
+	// Edit the message again but remove the mention
+	editedText = "edited text no mention"
+	editedMessage = &requests.EditMessage{
+		ID:   messageID,
+		Text: editedText,
+	}
+
+	sendResponse, err = theirMessenger.EditMessage(context.Background(), editedMessage)
+
+	s.Require().NoError(err)
+	s.Require().Len(sendResponse.Messages(), 1)
+	s.Require().NotEmpty(sendResponse.Messages()[0].EditedAt)
+	s.Require().Equal(sendResponse.Messages()[0].Text, editedText)
+	s.Require().Len(sendResponse.Chats(), 1)
+	s.Require().NotNil(sendResponse.Chats()[0].LastMessage)
+	s.Require().NotEmpty(sendResponse.Chats()[0].LastMessage.EditedAt)
+	s.Require().False(sendResponse.Messages()[0].Mentioned) // Sender is still not mentioned
+
+	response, err = WaitOnMessengerResponse(
+		s.m,
+		func(r *MessengerResponse) bool { return len(r.messages) == 1 },
+		"no messages",
+	)
+	s.Require().NoError(err)
+
+	s.Require().Len(response.Chats(), 1)
+	s.Require().Len(response.Messages(), 1)
+	s.Require().NotEmpty(response.Messages()[0].EditedAt)
+	s.Require().False(response.Messages()[0].New)
+	// Receiver (us) is no longer mentioned
+	s.Require().Equal(int(response.Chats()[0].UnviewedMessagesCount), 1) // We still have an unread message though
+	s.Require().Equal(int(response.Chats()[0].UnviewedMentionsCount), 0)
+	s.Require().False(response.Messages()[0].Mentioned)
+}

protocol/messenger_handler.go

@@ -1496,38 +1496,52 @@ func (m *Messenger) HandleEditMessage(state *ReceivedMessageState, editMessage E
 		return m.persistence.SaveEdit(editMessage)
 	}
 
+	// applyEditMessage modifies the message. Changing the variable name to make it clearer
+	editedMessage := originalMessage
 	// Update message and return it
-	err = m.applyEditMessage(&editMessage.EditMessage, originalMessage)
+	err = m.applyEditMessage(&editMessage.EditMessage, editedMessage)
 	if err != nil {
 		return err
 	}
 
-	if chat.LastMessage != nil && chat.LastMessage.ID == originalMessage.ID {
+	if chat.LastMessage != nil && chat.LastMessage.ID == editedMessage.ID {
-		chat.LastMessage = originalMessage
+		chat.LastMessage = editedMessage
 		err := m.saveChat(chat)
 		if err != nil {
 			return err
 		}
 	}
-	responseTo, err := m.persistence.MessageByID(originalMessage.ResponseTo)
+	responseTo, err := m.persistence.MessageByID(editedMessage.ResponseTo)
 
 	if err != nil && err != common.ErrRecordNotFound {
 		return err
 	}
 
-	err = state.updateExistingActivityCenterNotification(m.identity.PublicKey, m, originalMessage, responseTo)
+	err = state.updateExistingActivityCenterNotification(m.identity.PublicKey, m, editedMessage, responseTo)
 	if err != nil {
 		return err
 	}
 
-	editedMessageHasMentions := originalMessage.Mentioned
+	editedMessageHasMentions := editedMessage.Mentioned
 
-	if editedMessageHasMentions && !originalMessageMentioned && !originalMessage.Seen {
+	needToSaveChat := false
+	if editedMessageHasMentions && !originalMessageMentioned && !editedMessage.Seen {
 		// Increase unviewed count when the edited message has a mention and didn't have one before
-		m.updateUnviewedCounts(chat, originalMessage.Mentioned || originalMessage.Replied)
+		chat.UnviewedMentionsCount++
+		needToSaveChat = true
+	} else if !editedMessageHasMentions && originalMessageMentioned && !editedMessage.Seen {
+		// Opposite of above, the message had a mention, but no longer does, so we reduce the count
+		chat.UnviewedMentionsCount--
+		needToSaveChat = true
+	}
+	if needToSaveChat {
+		err := m.saveChat(chat)
+		if err != nil {
+			return err
+		}
 	}
 
-	state.Response.AddMessage(originalMessage)
+	state.Response.AddMessage(editedMessage)
 
 	// pull updated messages
 	updatedMessages, err := m.persistence.MessagesByResponseTo(messageID)
@@ -1597,6 +1611,7 @@ func (m *Messenger) HandleDeleteMessage(state *ReceivedMessageState, deleteMessa
 		return err
 	}
 
+	unreadCountDecreased := false
 	for _, messageToDelete := range messagesToDelete {
 		messageToDelete.Deleted = true
 		messageToDelete.DeletedBy = deleteMessage.DeleteMessage.DeletedBy
@@ -1613,6 +1628,19 @@ func (m *Messenger) HandleDeleteMessage(state *ReceivedMessageState, deleteMessa
 			return err
 		}
 
+		// Reduce chat mention count and unread count if unread
+		if !messageToDelete.Seen && !unreadCountDecreased {
+			unreadCountDecreased = true
+			chat.UnviewedMessagesCount--
+			if messageToDelete.Mentioned || messageToDelete.Replied {
+				chat.UnviewedMentionsCount--
+			}
+			err := m.saveChat(chat)
+			if err != nil {
+				return err
+			}
+		}
+
 		state.Response.AddRemovedMessage(&RemovedMessage{MessageID: messageToDelete.ID, ChatID: chat.ID, DeletedBy: deleteMessage.DeleteMessage.DeletedBy})
 		state.Response.AddNotification(DeletedMessageNotification(messageToDelete.ID, chat))
 		state.Response.AddActivityCenterNotification(&ActivityCenterNotification{