feat: remove explicit group chat join

relates: #status-im/status-desktop/5717
This commit is contained in:
Patryk Osmaczko 2022-05-20 12:53:28 +02:00 committed by Andrea Maria Piana
parent ed67cc2a6c
commit 5df5d6b519
10 changed files with 42 additions and 87 deletions

View File

@ -239,16 +239,6 @@ func (c *Chat) MembersAsPublicKeys() ([]*ecdsa.PublicKey, error) {
return stringSliceToPublicKeys(publicKeys) return stringSliceToPublicKeys(publicKeys)
} }
func (c *Chat) JoinedMembersAsPublicKeys() ([]*ecdsa.PublicKey, error) {
var publicKeys []string
for _, member := range c.Members {
if member.Joined {
publicKeys = append(publicKeys, member.ID)
}
}
return stringSliceToPublicKeys(publicKeys)
}
func (c *Chat) HasMember(memberID string) bool { func (c *Chat) HasMember(memberID string) bool {
for _, member := range c.Members { for _, member := range c.Members {
if memberID == member.ID { if memberID == member.ID {
@ -259,14 +249,14 @@ func (c *Chat) HasMember(memberID string) bool {
return false return false
} }
func (c *Chat) HasJoinedMember(memberID string) bool { func (c *Chat) RemoveMember(memberID string) {
for _, member := range c.Members { members := c.Members
if member.Joined && memberID == member.ID { c.Members = []ChatMember{}
return true for _, member := range members {
if memberID != member.ID {
c.Members = append(c.Members, member)
} }
} }
return false
} }
func (c *Chat) updateChatFromGroupMembershipChanges(g *v1protocol.Group) { func (c *Chat) updateChatFromGroupMembershipChanges(g *v1protocol.Group) {
@ -280,7 +270,6 @@ func (c *Chat) updateChatFromGroupMembershipChanges(g *v1protocol.Group) {
// Members // Members
members := g.Members() members := g.Members()
admins := g.Admins() admins := g.Admins()
joined := g.Joined()
chatMembers := make([]ChatMember, 0, len(members)) chatMembers := make([]ChatMember, 0, len(members))
for _, m := range members { for _, m := range members {
@ -288,7 +277,6 @@ func (c *Chat) updateChatFromGroupMembershipChanges(g *v1protocol.Group) {
ID: m, ID: m,
} }
chatMember.Admin = stringSliceContains(admins, m) chatMember.Admin = stringSliceContains(admins, m)
chatMember.Joined = stringSliceContains(joined, m)
chatMembers = append(chatMembers, chatMember) chatMembers = append(chatMembers, chatMember)
} }
c.Members = chatMembers c.Members = chatMembers
@ -350,8 +338,6 @@ type ChatMember struct {
ID string `json:"id"` ID string `json:"id"`
// Admin indicates if the member is an admin of the group chat // Admin indicates if the member is an admin of the group chat
Admin bool `json:"admin"` Admin bool `json:"admin"`
// Joined indicates if the member has joined the group chat
Joined bool `json:"joined"`
} }
func (c ChatMember) PublicKey() (*ecdsa.PublicKey, error) { func (c ChatMember) PublicKey() (*ecdsa.PublicKey, error) {

View File

@ -17,8 +17,7 @@ func init() {
defaultSystemMessagesTranslationSet := map[protobuf.MembershipUpdateEvent_EventType]string{ defaultSystemMessagesTranslationSet := map[protobuf.MembershipUpdateEvent_EventType]string{
protobuf.MembershipUpdateEvent_CHAT_CREATED: "{{from}} created the group {{name}}", protobuf.MembershipUpdateEvent_CHAT_CREATED: "{{from}} created the group {{name}}",
protobuf.MembershipUpdateEvent_NAME_CHANGED: "{{from}} changed the group's name to {{name}}", protobuf.MembershipUpdateEvent_NAME_CHANGED: "{{from}} changed the group's name to {{name}}",
protobuf.MembershipUpdateEvent_MEMBERS_ADDED: "{{from}} has invited {{members}}", protobuf.MembershipUpdateEvent_MEMBERS_ADDED: "{{from}} has added {{members}}",
protobuf.MembershipUpdateEvent_MEMBER_JOINED: "{{from}} joined the group",
protobuf.MembershipUpdateEvent_ADMINS_ADDED: "{{from}} has made {{members}} admin", protobuf.MembershipUpdateEvent_ADMINS_ADDED: "{{from}} has made {{members}} admin",
protobuf.MembershipUpdateEvent_MEMBER_REMOVED: "{{member}} left the group", protobuf.MembershipUpdateEvent_MEMBER_REMOVED: "{{member}} left the group",
protobuf.MembershipUpdateEvent_ADMIN_REMOVED: "{{member}} is not admin anymore", protobuf.MembershipUpdateEvent_ADMIN_REMOVED: "{{member}} is not admin anymore",
@ -50,9 +49,6 @@ func eventToSystemMessage(e v1protocol.MembershipUpdateEvent, translations *syst
} }
message, _ := translations.Load(protobuf.MembershipUpdateEvent_MEMBERS_ADDED) message, _ := translations.Load(protobuf.MembershipUpdateEvent_MEMBERS_ADDED)
text = tsprintf(message, map[string]string{"from": "@" + e.From, "members": strings.Join(memberMentions, ", ")}) text = tsprintf(message, map[string]string{"from": "@" + e.From, "members": strings.Join(memberMentions, ", ")})
case protobuf.MembershipUpdateEvent_MEMBER_JOINED:
message, _ := translations.Load(protobuf.MembershipUpdateEvent_MEMBER_JOINED)
text = tsprintf(message, map[string]string{"from": "@" + e.From})
case protobuf.MembershipUpdateEvent_ADMINS_ADDED: case protobuf.MembershipUpdateEvent_ADMINS_ADDED:
var memberMentions []string var memberMentions []string
for _, s := range e.Members { for _, s := range e.Members {
@ -93,8 +89,12 @@ func buildSystemMessages(events []v1protocol.MembershipUpdateEvent, translations
var messages []*common.Message var messages []*common.Message
for _, e := range events { for _, e := range events {
messages = append(messages, eventToSystemMessage(e, translations)) if e.Type == protobuf.MembershipUpdateEvent_MEMBER_JOINED {
// explicit join has been removed, ignore this event
continue
}
messages = append(messages, eventToSystemMessage(e, translations))
} }
return messages return messages

View File

@ -1988,6 +1988,7 @@ func (m *Messenger) AddAdminsToGroupChat(ctx context.Context, chatID string, mem
return m.addMessagesAndChat(chat, buildSystemMessages([]v1protocol.MembershipUpdateEvent{event}, m.systemMessagesTranslations), &response) return m.addMessagesAndChat(chat, buildSystemMessages([]v1protocol.MembershipUpdateEvent{event}, m.systemMessagesTranslations), &response)
} }
// Kept only for backward compatibility (auto-join), explicit join has been removed
func (m *Messenger) ConfirmJoiningGroup(ctx context.Context, chatID string) (*MessengerResponse, error) { func (m *Messenger) ConfirmJoiningGroup(ctx context.Context, chatID string) (*MessengerResponse, error) {
var response MessengerResponse var response MessengerResponse
@ -2051,9 +2052,11 @@ func (m *Messenger) leaveGroupChat(ctx context.Context, response *MessengerRespo
return nil, ErrChatNotFound return nil, ErrChatNotFound
} }
joined := chat.HasJoinedMember(common.PubkeyToHex(&m.identity.PublicKey)) amIMember := chat.HasMember(common.PubkeyToHex(&m.identity.PublicKey))
if amIMember {
chat.RemoveMember(common.PubkeyToHex(&m.identity.PublicKey))
if joined {
group, err := newProtocolGroupFromChat(chat) group, err := newProtocolGroupFromChat(chat)
if err != nil { if err != nil {
return nil, err return nil, err
@ -2314,22 +2317,11 @@ func (m *Messenger) dispatchMessage(ctx context.Context, spec common.RawMessage)
case ChatTypePrivateGroupChat: case ChatTypePrivateGroupChat:
logger.Debug("sending group message", zap.String("chatName", chat.Name)) logger.Debug("sending group message", zap.String("chatName", chat.Name))
if spec.Recipients == nil { if spec.Recipients == nil {
// Anything that is not a membership update message is only dispatched to joined users
// NOTE: I think here it might make sense to always invite to joined users apart from the
// initial message
if spec.MessageType != protobuf.ApplicationMetadataMessage_MEMBERSHIP_UPDATE_MESSAGE {
spec.Recipients, err = chat.JoinedMembersAsPublicKeys()
if err != nil {
return spec, err
}
} else {
spec.Recipients, err = chat.MembersAsPublicKeys() spec.Recipients, err = chat.MembersAsPublicKeys()
if err != nil { if err != nil {
return spec, err return spec, err
} }
} }
}
hasPairedDevices := m.hasPairedDevices() hasPairedDevices := m.hasPairedDevices()

View File

@ -143,7 +143,7 @@ func (m *Messenger) HandleMembershipUpdate(messageState *ReceivedMessageState, c
chat.updateChatFromGroupMembershipChanges(group) chat.updateChatFromGroupMembershipChanges(group)
wasUserAdded = !existingGroup.IsMember(ourKey) && wasUserAdded = !existingGroup.IsMember(ourKey) &&
updateGroup.IsMember(ourKey) group.IsMember(ourKey)
// Reactivate deleted group chat on re-invite from contact // Reactivate deleted group chat on re-invite from contact
chat.Active = chat.Active || (isActive && wasUserAdded) chat.Active = chat.Active || (isActive && wasUserAdded)
@ -187,7 +187,10 @@ func (m *Messenger) HandleMembershipUpdate(messageState *ReceivedMessageState, c
// Store in chats map as it might be a new one // Store in chats map as it might be a new one
messageState.AllChats.Store(chat.ID, chat) messageState.AllChats.Store(chat.ID, chat)
if waitingForApproval { // explicit join has been removed, mimic auto-join for backward compatibility
// no all cases are covered, e.g. if added to a group by non-contact
autoJoin := chat.Active && wasUserAdded
if autoJoin || waitingForApproval {
_, err = m.ConfirmJoiningGroup(context.Background(), chat.ID) _, err = m.ConfirmJoiningGroup(context.Background(), chat.ID)
if err != nil { if err != nil {
return err return err
@ -1536,22 +1539,20 @@ func (m *Messenger) matchChatEntity(chatEntity common.ChatEntity) (*Chat, error)
return nil, errors.New("received group chat chatEntity for non-existing chat") return nil, errors.New("received group chat chatEntity for non-existing chat")
} }
theirKeyHex := contactIDFromPublicKey(chatEntity.GetSigPubKey()) senderKeyHex := contactIDFromPublicKey(chatEntity.GetSigPubKey())
myKeyHex := contactIDFromPublicKey(&m.identity.PublicKey) myKeyHex := contactIDFromPublicKey(&m.identity.PublicKey)
var theyJoined bool senderIsMember := false
var iJoined bool iAmMember := false
for _, member := range chat.Members { for _, member := range chat.Members {
if member.ID == theirKeyHex && member.Joined { if member.ID == senderKeyHex {
theyJoined = true senderIsMember = true
} }
} if member.ID == myKeyHex {
for _, member := range chat.Members { iAmMember = true
if member.ID == myKeyHex && member.Joined {
iJoined = true
} }
} }
if theyJoined && iJoined { if senderIsMember && iAmMember {
return chat, nil return chat, nil
} }

View File

@ -81,13 +81,13 @@ func (s *EventToSystemMessageSuite) TestRun() {
Name: "members added event", Name: "members added event",
Event: v1protocol.NewMembersAddedEvent([]string{"a", "b", "c"}, 12), Event: v1protocol.NewMembersAddedEvent([]string{"a", "b", "c"}, 12),
From: "admin", From: "admin",
Expected: "@admin has invited @a, @b, @c", Expected: "@admin has added @a, @b, @c",
}, },
{ {
Name: "member joined event", Name: "member joined event",
Event: v1protocol.NewMemberJoinedEvent(12), Event: v1protocol.NewMemberJoinedEvent(12),
From: "admin", From: "admin",
Expected: "@admin joined the group", Expected: "", // joined events are deprecated
}, },
{ {
Name: "admins added event", Name: "admins added event",

View File

@ -1242,17 +1242,14 @@ func (s *MessengerSuite) TestChatPersistencePrivateGroupChat() {
{ {
ID: member1ID, ID: member1ID,
Admin: false, Admin: false,
Joined: true,
}, },
{ {
ID: member2ID, ID: member2ID,
Admin: true, Admin: true,
Joined: false,
}, },
{ {
ID: member3ID, ID: member3ID,
Admin: true, Admin: true,
Joined: true,
}, },
}, },
MembershipUpdates: []v1protocol.MembershipUpdateEvent{ MembershipUpdates: []v1protocol.MembershipUpdateEvent{

View File

@ -237,7 +237,6 @@ type Group struct {
events []MembershipUpdateEvent events []MembershipUpdateEvent
admins *stringSet admins *stringSet
members *stringSet members *stringSet
joined *stringSet
} }
func groupChatID(creator *ecdsa.PublicKey) string { func groupChatID(creator *ecdsa.PublicKey) string {
@ -265,7 +264,6 @@ func newGroup(chatID string, events []MembershipUpdateEvent) (*Group, error) {
events: events, events: events,
admins: newStringSet(), admins: newStringSet(),
members: newStringSet(), members: newStringSet(),
joined: newStringSet(),
} }
if err := g.init(); err != nil { if err := g.init(); err != nil {
return nil, err return nil, err
@ -394,10 +392,6 @@ func (g Group) Admins() []string {
return g.admins.List() return g.admins.List()
} }
func (g Group) Joined() []string {
return g.joined.List()
}
func (g *Group) ProcessEvents(events []MembershipUpdateEvent) error { func (g *Group) ProcessEvents(events []MembershipUpdateEvent) error {
for _, event := range events { for _, event := range events {
err := g.ProcessEvent(event) err := g.ProcessEvent(event)
@ -481,7 +475,6 @@ func (g *Group) processEvent(event MembershipUpdateEvent) {
case protobuf.MembershipUpdateEvent_CHAT_CREATED: case protobuf.MembershipUpdateEvent_CHAT_CREATED:
g.name = event.Name g.name = event.Name
g.members.Add(event.From) g.members.Add(event.From)
g.joined.Add(event.From)
g.admins.Add(event.From) g.admins.Add(event.From)
case protobuf.MembershipUpdateEvent_NAME_CHANGED: case protobuf.MembershipUpdateEvent_NAME_CHANGED:
g.name = event.Name g.name = event.Name
@ -493,10 +486,7 @@ func (g *Group) processEvent(event MembershipUpdateEvent) {
g.members.Add(event.Members...) g.members.Add(event.Members...)
case protobuf.MembershipUpdateEvent_MEMBER_REMOVED: case protobuf.MembershipUpdateEvent_MEMBER_REMOVED:
g.admins.Remove(event.Members[0]) g.admins.Remove(event.Members[0])
g.joined.Remove(event.Members[0])
g.members.Remove(event.Members[0]) g.members.Remove(event.Members[0])
case protobuf.MembershipUpdateEvent_MEMBER_JOINED:
g.joined.Add(event.From)
} }
} }

View File

@ -77,7 +77,6 @@ func TestGroupProcessEvent(t *testing.T) {
return Group{ return Group{
name: name, name: name,
admins: newStringSetFromSlice(admins), admins: newStringSetFromSlice(admins),
joined: newStringSetFromSlice(joined),
members: newStringSetFromSlice(members), members: newStringSetFromSlice(members),
} }
} }

View File

@ -301,7 +301,7 @@ func getChatMembers(sourceChat *protocol.Chat, community *communities.Community,
for _, m := range sourceChat.Members { for _, m := range sourceChat.Members {
result[m.ID] = Member{ result[m.ID] = Member{
Admin: m.Admin, Admin: m.Admin,
Joined: m.Joined, Joined: true,
} }
} }
return result, nil return result, nil

View File

@ -119,16 +119,6 @@ func (api *API) MakeAdmin(ctx context.Context, communityID types.HexBytes, chatI
}) })
} }
func (api *API) ConfirmJoiningGroup(ctx context.Context, communityID types.HexBytes, chatID string) (*GroupChatResponse, error) {
if len(communityID) != 0 {
return nil, ErrCommunitiesNotSupported
}
return api.execAndGetGroupChatResponse(func() (*protocol.MessengerResponse, error) {
return api.s.messenger.ConfirmJoiningGroup(ctx, chatID)
})
}
func (api *API) RenameChat(ctx context.Context, communityID types.HexBytes, chatID string, name string) (*GroupChatResponse, error) { func (api *API) RenameChat(ctx context.Context, communityID types.HexBytes, chatID string, name string) (*GroupChatResponse, error) {
if len(communityID) != 0 { if len(communityID) != 0 {
return nil, ErrCommunitiesNotSupported return nil, ErrCommunitiesNotSupported