fix(cr)_: fix dismissing and then sending a CR (#6140)

Fixes https://github.com/status-im/status-desktop/issues/16817

There were two issues.
When dismissing a CR, then sending one back, it did mark the two contacts as mutual and showed the 1-1 chat. However, the message sent in the second/final CR was not shown in the first person's client.
Also, the AC notification for the first user didn't update, so it got stuck in a "pending" state.

Those two issues are fixed now with a test to confirm.
This commit is contained in:
Jonathan Rainville 2024-12-03 15:03:55 -05:00 committed by GitHub
parent 0794edc3db
commit 3db68c4d64
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 110 additions and 62 deletions

View File

@ -14,6 +14,29 @@ import (
const allFieldsForTableActivityCenterNotification = `id, timestamp, notification_type, chat_id, read, dismissed, accepted, message, author,
reply_message, community_id, membership_status, contact_verification_status, token_data, deleted, updated_at`
const selectActivityCenterNotificationsQuery = `SELECT
a.id,
a.timestamp,
a.notification_type,
a.chat_id,
a.community_id,
a.membership_status,
a.read,
a.accepted,
a.dismissed,
a.deleted,
a.message,
c.last_message,
a.reply_message,
a.contact_verification_status,
c.name,
a.author,
a.token_data,
a.updated_at,
a.installation_id
FROM activity_center_notifications a
LEFT JOIN chats c ON c.id = a.chat_id `
var emptyNotifications = make([]*ActivityCenterNotification, 0)
func (db sqlitePersistence) DeleteActivityCenterNotificationByID(id []byte, updatedAt uint64) error {
@ -697,34 +720,33 @@ func (db sqlitePersistence) GetActivityCenterNotificationsByID(ids []types.HexBy
return notifications, nil
}
func (db sqlitePersistence) GetActivityCenterNotificationByTypeAuthorAndChatID(acType ActivityCenterType, author string, chatID string) (*ActivityCenterNotification, error) {
if len(author) == 0 {
return nil, nil
}
if len(chatID) == 0 {
return nil, nil
}
// nolint: gosec
query := selectActivityCenterNotificationsQuery + `
WHERE a.notification_type = ?
AND a.author = ?
AND a.chat_id = ?
ORDER BY a.timestamp DESC
LIMIT 1`
row := db.db.QueryRow(query, acType, author, chatID)
notification, err := db.unmarshalActivityCenterNotificationRow(row)
if err == sql.ErrNoRows {
return nil, nil
}
return notification, err
}
// GetActivityCenterNotificationByID returns a notification by its ID even it's deleted logically
func (db sqlitePersistence) GetActivityCenterNotificationByID(id types.HexBytes) (*ActivityCenterNotification, error) {
row := db.db.QueryRow(`
SELECT
a.id,
a.timestamp,
a.notification_type,
a.chat_id,
a.community_id,
a.membership_status,
a.read,
a.accepted,
a.dismissed,
a.deleted,
a.message,
c.last_message,
a.reply_message,
a.contact_verification_status,
c.name,
a.author,
a.token_data,
a.updated_at,
a.installation_id
FROM activity_center_notifications a
LEFT JOIN chats c
ON
c.id = a.chat_id
WHERE a.id = ?`, id)
query := selectActivityCenterNotificationsQuery + `WHERE a.id = ?`
row := db.db.QueryRow(query, id)
notification, err := db.unmarshalActivityCenterNotificationRow(row)
if err == sql.ErrNoRows {
@ -1334,29 +1356,7 @@ func (db sqlitePersistence) ActiveContactRequestNotification(contactID string) (
// contact request per contact, but to avoid relying on the unpredictable
// behavior of the DB engine for sorting, we sort by notification.Timestamp
// DESC.
query := `
SELECT
a.id,
a.timestamp,
a.notification_type,
a.chat_id,
a.community_id,
a.membership_status,
a.read,
a.accepted,
a.dismissed,
a.deleted,
a.message,
c.last_message,
a.reply_message,
a.contact_verification_status,
c.name,
a.author,
a.token_data,
a.updated_at,
a.installation_id
FROM activity_center_notifications a
LEFT JOIN chats c ON c.id = a.chat_id
query := selectActivityCenterNotificationsQuery + `
WHERE a.author = ?
AND NOT a.deleted
AND NOT a.dismissed

View File

@ -31,7 +31,7 @@ func (s *MessengerContactRequestSuite) findFirstByContentType(messages []*common
return FindFirstByContentType(messages, contentType)
}
func (s *MessengerContactRequestSuite) sendContactRequest(request *requests.SendContactRequest, messenger *Messenger) {
func (s *MessengerContactRequestSuite) sendContactRequestWithState(request *requests.SendContactRequest, messenger *Messenger, requestState common.ContactRequestState, mutualState bool) *MessengerResponse {
s.logger.Info("sendContactRequest", zap.String("sender", messenger.IdentityPublicKeyString()), zap.String("receiver", request.ID))
// Send contact request
@ -53,7 +53,7 @@ func (s *MessengerContactRequestSuite) sendContactRequest(request *requests.Send
contactRequest := s.findFirstByContentType(resp.Messages(), protobuf.ChatMessage_CONTACT_REQUEST)
s.Require().NotNil(contactRequest)
s.Require().Equal(common.ContactRequestStatePending, contactRequest.ContactRequestState)
s.Require().Equal(requestState, contactRequest.ContactRequestState)
s.Require().Equal(request.Message, contactRequest.Text)
// Check pending notification
@ -66,12 +66,14 @@ func (s *MessengerContactRequestSuite) sendContactRequest(request *requests.Send
// Check contacts
s.Require().Len(resp.Contacts, 1)
contact := resp.Contacts[0]
s.Require().False(contact.mutual())
s.Require().Equal(mutualState, contact.mutual())
// Make sure it's not returned as coming from us
contactRequests, _, err := messenger.PendingContactRequests("", 10)
s.Require().NoError(err)
s.Require().Len(contactRequests, 0)
if len(contactRequests) > 0 {
s.Require().Equal(request.ID, contactRequests[0].LocalChatID)
}
// Make sure contact is added on the sender side
contacts := messenger.AddedContacts()
@ -81,6 +83,12 @@ func (s *MessengerContactRequestSuite) sendContactRequest(request *requests.Send
// Check contact's primary name matches notification's name
s.Require().Equal(resp.ActivityCenterNotifications()[0].Name, contacts[0].PrimaryName())
return resp
}
func (s *MessengerContactRequestSuite) sendContactRequest(request *requests.SendContactRequest, messenger *Messenger) *MessengerResponse {
return s.sendContactRequestWithState(request, messenger, common.ContactRequestStatePending, false)
}
func (s *MessengerContactRequestSuite) receiveContactRequest(messageText string, theirMessenger *Messenger) *common.Message {
@ -1197,11 +1205,7 @@ func (s *MessengerContactRequestSuite) TestBobSendsContactRequestAfterDecliningO
ID: aliceID,
Message: messageTextBob,
}
// Send contact request
resp, err := bob.SendContactRequest(context.Background(), requestFromBob)
s.Require().NoError(err)
s.Require().NotNil(resp)
resp := s.sendContactRequestWithState(requestFromBob, bob, common.ContactRequestStateAccepted, true)
// Check CR message, it should be accepted
s.Require().Len(resp.Messages(), 2)
@ -1223,6 +1227,31 @@ func (s *MessengerContactRequestSuite) TestBobSendsContactRequestAfterDecliningO
s.Require().Len(resp.Contacts, 1)
contact := resp.Contacts[0]
s.Require().True(contact.mutual())
resp, err := WaitOnMessengerResponse(
alice,
func(r *MessengerResponse) bool {
return len(r.Contacts) == 1 && len(r.Messages()) >= 2 && len(r.ActivityCenterNotifications()) == 1
},
"no messages",
)
s.Require().NoError(err)
s.Require().NotNil(resp)
contactRequest = s.findFirstByContentType(resp.Messages(), protobuf.ChatMessage_CONTACT_REQUEST)
s.Require().NotNil(contactRequest)
// Alice's contact request is marked as accepted
s.Require().Equal(common.ContactRequestStateAccepted, contactRequest.ContactRequestState)
s.Require().Equal(messageTextBob, contactRequest.Text)
// Check activity center notification is of the right type and is is the Accepted satte as well
s.Require().Len(resp.ActivityCenterNotifications(), 1)
s.Require().Equal(ActivityCenterNotificationTypeContactRequest, resp.ActivityCenterNotifications()[0].Type)
s.Require().Equal(contactRequest.ContactRequestState, resp.ActivityCenterNotifications()[0].Message.ContactRequestState)
s.Require().Equal(true, resp.ActivityCenterNotifications()[0].Read)
s.Require().Equal(false, resp.ActivityCenterNotifications()[0].Accepted)
s.Require().Equal(false, resp.ActivityCenterNotifications()[0].Dismissed)
}
func (s *MessengerContactRequestSuite) TestBuildContact() {

View File

@ -2374,6 +2374,7 @@ func (m *Messenger) handleChatMessage(state *ReceivedMessageState, forceSeen boo
m.logger.Warn("failed to add peersyncing message", zap.Error(err))
}
receivedAContactRequest := false
// If we receive some propagated state from someone who's not
// our paired device, we handle it
if receivedMessage.ContactRequestPropagatedState != nil && !isSyncMessage {
@ -2385,6 +2386,7 @@ func (m *Messenger) handleChatMessage(state *ReceivedMessageState, forceSeen boo
}
}
if result.newContactRequestReceived {
receivedAContactRequest = true
if contact.hasAddedUs() && !contact.mutual() {
receivedMessage.ContactRequestState = common.ContactRequestStatePending
@ -2407,9 +2409,26 @@ func (m *Messenger) handleChatMessage(state *ReceivedMessageState, forceSeen boo
}
state.Response.AddMessage(updateMessage)
err = m.createIncomingContactRequestNotification(contact, state, receivedMessage, true)
if err != nil {
return err
if !contact.mutual() {
// Only create the notification if we are not mutual yet
err = m.createIncomingContactRequestNotification(contact, state, receivedMessage, true)
if err != nil {
return err
}
} else {
// We already sent a contact request, so we can mark the old notification as Accepted
notification, err := m.persistence.GetActivityCenterNotificationByTypeAuthorAndChatID(ActivityCenterNotificationTypeContactRequest, m.myHexIdentity(), contact.ID)
if err != nil {
return err
}
if notification != nil && notification.Message.ContactRequestState != common.ContactRequestStateAccepted {
notification.Message.ContactRequestState = common.ContactRequestStateAccepted
notification.UpdatedAt = m.GetCurrentTimeInMillis()
err = m.addActivityCenterNotification(state.Response, notification, nil)
if err != nil {
return err
}
}
}
}
state.ModifiedContacts.Store(contact.ID, true)
@ -2429,7 +2448,7 @@ func (m *Messenger) handleChatMessage(state *ReceivedMessageState, forceSeen boo
chatContact.CustomizationColor = multiaccountscommon.IDToColorFallbackToBlue(receivedMessage.CustomizationColor)
}
if chatContact.mutual() || chatContact.dismissed() {
if (!receivedAContactRequest && chatContact.mutual()) || chatContact.dismissed() {
m.logger.Info("ignoring contact request message for a mutual or dismissed contact")
return nil
}