Unifying the endpoints for contact flows (#3379)

* fix: Contact requests flows

fix: pending CR notification
fix: use CR as message with provided text

* fix: Remove legacy default contact request

* fix: Add test case sending CR after removal from contacts

* fix: refactor contact request tests to have common steps

* fix: activate chat on sender side after receiveing the CR

* chore: Return defaultContactRequestID function

* fix: force activate chat on reciever's side

* fix: ensure AC notification's name for CR notifiaction match contact's primary name

VERSION

@@ -1 +1 @@
-0.146.6
+0.147.0

protocol/communities_messenger_test.go

@@ -2312,7 +2312,7 @@ func (s *MessengerCommunitiesSuite) TestShareCommunity() {
 	s.Require().Len(response.Messages(), 1)
 
 	// Add bob to contacts so it does not go on activity center
-	bobPk := common.PubkeyToHex(&s.alice.identity.PublicKey)
+	bobPk := common.PubkeyToHex(&s.bob.identity.PublicKey)
 	request := &requests.AddContact{ID: bobPk}
 	_, err = s.alice.AddContact(context.Background(), request)
 	s.Require().NoError(err)

protocol/messenger_contact_verification_test.go

@@ -135,7 +135,7 @@ func (s *MessengerVerificationRequests) mutualContact(theirMessenger *Messenger)
 	resp, err = WaitOnMessengerResponse(
 		s.m,
 		func(r *MessengerResponse) bool {
-			return len(r.Contacts) == 1 && len(r.Messages()) == 2 && len(r.ActivityCenterNotifications()) == 1
+			return len(r.Contacts) == 1 && len(r.Messages()) == 1 && len(r.ActivityCenterNotifications()) == 1
 		},
 		"no messages",
 	)
@@ -148,18 +148,9 @@ func (s *MessengerVerificationRequests) mutualContact(theirMessenger *Messenger)
 
 	// Make sure the message is updated, sender s2de
 	s.Require().NotNil(resp)
-	s.Require().Len(resp.Messages(), 2)
+	s.Require().Len(resp.Messages(), 1)
-	var message *common.Message
+	s.Require().Equal(resp.Messages()[0].ID, contactRequests[0].ID)
-
+	s.Require().Equal(common.ContactRequestStateAccepted, resp.Messages()[0].ContactRequestState)
-	for _, m := range resp.Messages() {
-		if m.ID == contactRequests[0].ID {
-			message = m
-		}
-	}
-	s.Require().NotNil(message)
-
-	s.Require().Equal(message.ID, contactRequests[0].ID)
-	s.Require().Equal(common.ContactRequestStateAccepted, message.ContactRequestState)
 
 	// Make sure we consider them a mutual contact, sender side
 	mutualContacts = s.m.MutualContacts()
@@ -168,7 +159,6 @@ func (s *MessengerVerificationRequests) mutualContact(theirMessenger *Messenger)
 	// Check the contact state is correctly set
 	s.Require().Len(resp.Contacts, 1)
 	s.Require().True(resp.Contacts[0].mutual())
-
 }
 
 func (s *MessengerVerificationRequests) TestAcceptVerificationRequests() {

protocol/messenger_contacts.go

@@ -16,7 +16,7 @@ import (
 	"github.com/status-im/status-go/protocol/transport"
 )
 
-func (m *Messenger) acceptContactRequest(requestID string, syncing bool) (*MessengerResponse, error) {
+func (m *Messenger) acceptContactRequest(ctx context.Context, requestID string, syncing bool) (*MessengerResponse, error) {
 	contactRequest, err := m.persistence.MessageByID(requestID)
 	if err != nil {
 		m.logger.Error("could not find contact request message", zap.Error(err))
@@ -24,9 +24,30 @@ func (m *Messenger) acceptContactRequest(requestID string, syncing bool) (*Messe
 	}
 
 	m.logger.Info("acceptContactRequest")
-	// We send a contact update for compatibility with 0.90 desktop, once that's
+
-	// not an issue anymore, we can set the last bool flag to `false`
+	response, err := m.addContact(ctx, contactRequest.From, "", "", "", contactRequest.ID, "", syncing, false, false)
-	return m.addContact(contactRequest.From, "", "", "", contactRequest.ID, "", syncing, true, false)
+	if err != nil {
+		return nil, err
+	}
+
+	// Force activate chat
+	chat, ok := m.allChats.Load(contactRequest.From)
+	if !ok {
+		publicKey, err := common.HexToPubkey(contactRequest.From)
+		if err != nil {
+			return nil, err
+		}
+
+		chat = OneToOneFromPublicKey(publicKey, m.getTimesource())
+	}
+
+	chat.Active = true
+	if err := m.saveChat(chat); err != nil {
+		return nil, err
+	}
+	response.AddChat(chat)
+
+	return response, nil
 }
 
 func (m *Messenger) AcceptContactRequest(ctx context.Context, request *requests.AcceptContactRequest) (*MessengerResponse, error) {
@@ -35,7 +56,7 @@ func (m *Messenger) AcceptContactRequest(ctx context.Context, request *requests.
 		return nil, err
 	}
 
-	response, err := m.acceptContactRequest(request.ID.String(), false)
+	response, err := m.acceptContactRequest(ctx, request.ID.String(), false)
 	if err != nil {
 		return nil, err
 	}
@@ -89,6 +110,7 @@ func (m *Messenger) declineContactRequest(requestID string, syncing bool) (*Mess
 		return nil, err
 	}
 	if notification != nil {
+		notification.Name = contact.PrimaryName()
 		notification.Message = contactRequest
 		notification.Read = true
 		notification.Dismissed = true
@@ -133,7 +155,8 @@ func (m *Messenger) SendContactRequest(ctx context.Context, request *requests.Se
 		return nil, err
 	}
 
-	response, err := m.addContact(
+	return m.addContact(
+		ctx,
 		chatID,
 		"",
 		"",
@@ -142,50 +165,8 @@ func (m *Messenger) SendContactRequest(ctx context.Context, request *requests.Se
 		request.Message,
 		false,
 		false,
-		false,
+		true,
 	)
-	if err != nil {
-		return nil, err
-	}
-
-	publicKey, err := common.HexToPubkey(chatID)
-	if err != nil {
-		return nil, err
-	}
-
-	// A valid added chat is required.
-	_, ok := m.allChats.Load(chatID)
-	if !ok {
-		// Create a one to one chat and set active to false
-		chat := CreateOneToOneChat(chatID, publicKey, m.getTimesource())
-		chat.Active = false
-		err = m.initChatSyncFields(chat)
-		if err != nil {
-			return nil, err
-		}
-		err = m.saveChat(chat)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	chatMessage := &common.Message{}
-	chatMessage.ChatId = chatID
-	chatMessage.Text = request.Message
-	chatMessage.ContentType = protobuf.ChatMessage_CONTACT_REQUEST
-	chatMessage.ContactRequestState = common.ContactRequestStatePending
-
-	messageResponse, err := m.sendChatMessage(ctx, chatMessage)
-	if err != nil {
-		return nil, err
-	}
-
-	err = response.Merge(messageResponse)
-	if err != nil {
-		return nil, err
-	}
-
-	return response, nil
 }
 
 func (m *Messenger) updateAcceptedContactRequest(response *MessengerResponse, contactRequestID string) (*MessengerResponse, error) {
@@ -239,6 +220,7 @@ func (m *Messenger) updateAcceptedContactRequest(response *MessengerResponse, co
 	}
 
 	if notification != nil {
+		notification.Name = contact.PrimaryName()
 		notification.Message = contactRequest
 		notification.Read = true
 		notification.Accepted = true
@@ -256,7 +238,7 @@ func (m *Messenger) updateAcceptedContactRequest(response *MessengerResponse, co
 	return response, nil
 }
 
-func (m *Messenger) addContact(pubKey, ensName, nickname, displayName, contactRequestID string, contactRequestText string, syncing bool, sendContactUpdate bool, createOutgoingContactRequestNotification bool) (*MessengerResponse, error) {
+func (m *Messenger) addContact(ctx context.Context, pubKey, ensName, nickname, displayName, contactRequestID string, contactRequestText string, syncing bool, sendContactUpdate bool, createOutgoingContactRequestNotification bool) (*MessengerResponse, error) {
 	contact, err := m.BuildContact(&requests.BuildContact{PublicKey: pubKey})
 	if err != nil {
 		return nil, err
@@ -323,6 +305,7 @@ func (m *Messenger) addContact(pubKey, ensName, nickname, displayName, contactRe
 	if err != nil {
 		return nil, err
 	}
+
 	if err := m.saveChat(profileChat); err != nil {
 		return nil, err
 	}
@@ -403,13 +386,17 @@ func (m *Messenger) addContact(pubKey, ensName, nickname, displayName, contactRe
 	// Add outgoing contact request notification
 	if createOutgoingContactRequestNotification {
 		clock, timestamp := chat.NextClockAndTimestamp(m.transport)
-		contactRequest, err := m.generateContactRequest(clock, timestamp, contact, contactRequestText)
+		contactRequest, err := m.generateContactRequest(clock, timestamp, contact, contactRequestText, true)
 		if err != nil {
 			return nil, err
 		}
 
-		response.AddMessage(contactRequest)
+		messageResponse, err := m.sendChatMessage(ctx, contactRequest)
-		err = m.persistence.SaveMessages([]*common.Message{contactRequest})
+		if err != nil {
+			return nil, err
+		}
+
+		err = response.Merge(messageResponse)
 		if err != nil {
 			return nil, err
 		}
@@ -423,24 +410,27 @@ func (m *Messenger) addContact(pubKey, ensName, nickname, displayName, contactRe
 
 	// Add contact
 	response.AddContact(contact)
-
 	return response, nil
 }
 
-func (m *Messenger) generateContactRequest(clock uint64, timestamp uint64, contact *Contact, text string) (*common.Message, error) {
+func (m *Messenger) generateContactRequest(clock uint64, timestamp uint64, contact *Contact, text string, outgoing bool) (*common.Message, error) {
 	if contact == nil {
 		return nil, errors.New("contact cannot be nil")
 	}
 
 	contactRequest := &common.Message{}
+	contactRequest.ChatId = contact.ID
 	contactRequest.WhisperTimestamp = timestamp
 	contactRequest.Seen = true
 	contactRequest.Text = text
+	if outgoing {
+		contactRequest.From = m.myHexIdentity()
+	} else {
 		contactRequest.From = contact.ID
+	}
 	contactRequest.LocalChatID = contact.ID
 	contactRequest.ContentType = protobuf.ChatMessage_CONTACT_REQUEST
 	contactRequest.Clock = clock
-	contactRequest.ID = defaultContactRequestID(contact.ID)
 	if contact.mutual() {
 		contactRequest.ContactRequestState = common.ContactRequestStateAccepted
 	} else {
@@ -459,7 +449,9 @@ func (m *Messenger) generateOutgoingContactRequestNotification(contact *Contact,
 		Message:   contactRequest,
 		Timestamp: m.getTimesource().GetCurrentTime(),
 		ChatID:    contact.ID,
-		Read:      contactRequest.ContactRequestState == common.ContactRequestStateAccepted || contactRequest.ContactRequestState == common.ContactRequestStateDismissed,
+		Read: contactRequest.ContactRequestState == common.ContactRequestStateAccepted ||
+			contactRequest.ContactRequestState == common.ContactRequestStateDismissed ||
+			contactRequest.ContactRequestState == common.ContactRequestStatePending,
 		Accepted:  contactRequest.ContactRequestState == common.ContactRequestStateAccepted,
 		Dismissed: contactRequest.ContactRequestState == common.ContactRequestStateDismissed,
 	}
@@ -477,6 +469,7 @@ func (m *Messenger) AddContact(ctx context.Context, request *requests.AddContact
 	}
 
 	return m.addContact(
+		ctx,
 		id,
 		request.ENSName,
 		request.Nickname,
@@ -944,9 +937,6 @@ func (m *Messenger) AcceptLatestContactRequestForContact(ctx context.Context, re
 	if err != nil {
 		return nil, err
 	}
-	if contactRequestID == "" {
-		contactRequestID = defaultContactRequestID(request.ID.String())
-	}
 
 	return m.AcceptContactRequest(ctx, &requests.AcceptContactRequest{ID: types.Hex2Bytes(contactRequestID)})
 }
@@ -961,10 +951,6 @@ func (m *Messenger) DismissLatestContactRequestForContact(ctx context.Context, r
 		return nil, err
 	}
 
-	if contactRequestID == "" {
-		contactRequestID = defaultContactRequestID(request.ID.String())
-	}
-
 	return m.DeclineContactRequest(ctx, &requests.DeclineContactRequest{ID: types.Hex2Bytes(contactRequestID)})
 }
 

protocol/messenger_handler.go

@@ -270,8 +270,33 @@ func (m *Messenger) PendingNotificationContactRequest(contactID string) (*Activi
 	return m.persistence.ActiveContactRequestNotification(contactID)
 }
 
+func (m *Messenger) createContactRequestForContactUpdate(contact *Contact, messageState *ReceivedMessageState) (*common.Message, error) {
+	contactRequest, err := m.generateContactRequest(
+		messageState.CurrentMessageState.Message.Clock,
+		messageState.CurrentMessageState.WhisperTimestamp,
+		contact,
+		"Please add me to your contacts",
+		false,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	contactRequest.ID = defaultContactRequestID(contact.ID)
+
+	// save this message
+	messageState.Response.AddMessage(contactRequest)
+	err = m.persistence.SaveMessages([]*common.Message{contactRequest})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return contactRequest, nil
+}
+
 func (m *Messenger) createIncomingContactRequestNotification(contact *Contact, messageState *ReceivedMessageState, contactRequest *common.Message, createNewNotification bool) error {
-	if contactRequest != nil && contactRequest.ContactRequestState == common.ContactRequestStateAccepted {
+	if contactRequest.ContactRequestState == common.ContactRequestStateAccepted {
 		// Pull one from the db if there
 		notification, err := m.persistence.GetActivityCenterNotificationByID(types.FromHex(contactRequest.ID))
 		if err != nil {
@@ -279,6 +304,7 @@ func (m *Messenger) createIncomingContactRequestNotification(contact *Contact, m
 		}
 
 		if notification != nil {
+			notification.Name = contact.PrimaryName()
 			notification.Message = contactRequest
 			notification.Read = true
 			notification.Accepted = true
@@ -293,82 +319,6 @@ func (m *Messenger) createIncomingContactRequestNotification(contact *Contact, m
 		return nil
 	}
 
-	if contactRequest == nil || contactRequest.ContactRequestState == common.ContactRequestStatePending {
-		notification, err := m.PendingNotificationContactRequest(contact.ID)
-		if err != nil {
-			return err
-		}
-
-		// If there's already a notification, we will check whether is a default notification
-		// that has not been dismissed (nor accepted???)
-		// If it is, we replace it with a non-default, since it contains a message
-		if notification != nil {
-			// Check if it's the default notification
-			if notification.Message.ID == defaultContactRequestID(contact.ID) {
-				// Nothing to do, we already have a default notification
-				if contactRequest == nil {
-					return nil
-				}
-				// We first dismiss it in the database
-				err := m.persistence.DismissActivityCenterNotifications([]types.HexBytes{types.Hex2Bytes(notification.Message.ID)})
-				if err != nil {
-					return err
-				}
-				//  we mark the notification as dismissed & read
-				notification.Dismissed = true
-				notification.Read = true
-				// We remove it from the response, since the client has never seen it, better to just remove it
-				found := messageState.Response.RemoveActivityCenterNotification(notification.Message.ID)
-				// Otherwise, it means we have already passed it to the client, so we add it with a `dismissed` flag
-				// so it can clean up
-				if !found {
-					messageState.Response.AddActivityCenterNotification(notification)
-				}
-			}
-		}
-	}
-
-	// Legacy//ContactUpdate contact request
-	if contactRequest == nil {
-		if messageState.CurrentMessageState == nil || messageState.CurrentMessageState.MessageID == "" {
-			return errors.New("no available id")
-		}
-		// We use a known id so that we can check if already in the database
-		defaultID := defaultContactRequestID(contact.ID)
-
-		// Pull one from the db if there
-		notification, err := m.persistence.GetActivityCenterNotificationByID(types.FromHex(defaultID))
-		if err != nil {
-			return err
-		}
-
-		// if the notification is accepted, we clear it, as this one will replace it
-		if notification != nil && notification.Accepted {
-			err = m.persistence.DeleteActivityCenterNotification(types.FromHex(defaultID))
-			if err != nil {
-				return err
-			}
-		}
-
-		// generate request message
-		contactRequest, err = m.generateContactRequest(
-			messageState.CurrentMessageState.Message.Clock,
-			messageState.CurrentMessageState.WhisperTimestamp,
-			contact,
-			"Please add me to your contacts",
-		)
-		if err != nil {
-			return err
-		}
-
-		// save this message
-		messageState.Response.AddMessage(contactRequest)
-		err = m.persistence.SaveMessages([]*common.Message{contactRequest})
-		if err != nil {
-			return err
-		}
-	}
-
 	if !createNewNotification {
 		return nil
 	}
@@ -829,7 +779,7 @@ func (m *Messenger) handleAcceptContactRequestMessage(state *ReceivedMessageStat
 
 	// If the state has changed from non-mutual contact, to mutual contact
 	// we want to notify the user
-	if processingResponse.newContactRequestReceived && contact.mutual() {
+	if contact.mutual() {
 		// We set the chat as active, this is currently the expected behavior
 		// for mobile, it might change as we implement further the activity
 		// center
@@ -874,14 +824,7 @@ func (m *Messenger) handleAcceptContactRequestMessage(state *ReceivedMessageStat
 }
 
 func (m *Messenger) HandleAcceptContactRequest(state *ReceivedMessageState, message protobuf.AcceptContactRequest, senderID string) error {
-	// outgoing contact requests are created on the side of a sender
+	err := m.handleAcceptContactRequestMessage(state, message.Clock, message.Id, false)
-	err := m.handleAcceptContactRequestMessage(state, message.Clock, defaultContactRequestID(senderID), true)
-	if err != nil {
-		m.logger.Warn("could not accept contact request", zap.Error(err))
-	}
-
-	// legacy contact requests: the ones that are send with SendContactRequest
-	err = m.handleAcceptContactRequestMessage(state, message.Clock, message.Id, false)
 	if err != nil {
 		m.logger.Warn("could not accept contact request", zap.Error(err))
 	}
@@ -965,12 +908,17 @@ func (m *Messenger) HandleContactUpdate(state *ReceivedMessageState, message pro
 
 		}
 		if result.newContactRequestReceived {
-			err = m.createIncomingContactRequestNotification(contact, state, nil, true)
+			contactRequest, err := m.createContactRequestForContactUpdate(contact, state)
 			if err != nil {
 				return err
 			}
 
+			err = m.createIncomingContactRequestNotification(contact, state, contactRequest, true)
+			if err != nil {
+				return err
 			}
+		}
+
 		logger.Debug("handled propagated state", zap.Any("state after update", contact.ContactRequestPropagatedState()))
 		state.ModifiedContacts.Store(contact.ID, true)
 		state.AllContacts.Store(contact.ID, contact)