Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

join fixups #19249

Merged
merged 10 commits into from
Sep 5, 2019
Merged

join fixups #19249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b513afd
join fixups
mmaxim Sep 3, 2019
fa63933
Merge branch 'master' into mike/HOTPOT-730
mmaxim Sep 4, 2019
fe08c0b
wip
mmaxim Sep 4, 2019
367eab4
fix test
mmaxim Sep 4, 2019
a16143c
Merge branch 'master' into mike/HOTPOT-730
mmaxim Sep 4, 2019
d2d3051
wip
mmaxim Sep 4, 2019
f330bd7
fix
mmaxim Sep 5, 2019
b72fbfd
Merge branch 'master' into mike/HOTPOT-730
mmaxim Sep 5, 2019
2b7219f
ahhhh
mmaxim Sep 5, 2019
6fe6854
comment
mmaxim Sep 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 38 additions & 20 deletions go/chat/uithreadloader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func NewUIThreadLoader(g *globals.Context) *UIThreadLoader {
}

func (t *UIThreadLoader) groupGeneric(ctx context.Context, uid gregor1.UID, msgs []chat1.MessageUnboxed,
matches func(msg chat1.MessageUnboxed) bool, makeCombined func([]chat1.MessageUnboxed) *chat1.MessageUnboxed) (res []chat1.MessageUnboxed) {
matches func(msg chat1.MessageUnboxed, grouped []chat1.MessageUnboxed) bool, makeCombined func([]chat1.MessageUnboxed) *chat1.MessageUnboxed) (res []chat1.MessageUnboxed) {
var grouped []chat1.MessageUnboxed
addGrouped := func() {
if len(grouped) == 0 {
Expand All @@ -59,14 +59,18 @@ func (t *UIThreadLoader) groupGeneric(ctx context.Context, uid gregor1.UID, msgs
grouped = nil
}
for _, msg := range msgs {
if msg.IsValid() {
if matches(msg) {
grouped = append(grouped, msg)
continue
}
if msg.IsValid() && matches(msg, grouped) {
grouped = append(grouped, msg)
continue
}
addGrouped()
res = append(res, msg)
// some match functions may depend on messages in grouped, so after we clear it
// this message might be a candidate to get grouped.
if msg.IsValid() && matches(msg, grouped) {
mmaxim marked this conversation as resolved.
Show resolved Hide resolved
grouped = append(grouped, msg)
} else {
res = append(res, msg)
}
}
addGrouped()
return res
Expand All @@ -76,11 +80,24 @@ func (t *UIThreadLoader) groupThreadView(ctx context.Context, uid gregor1.UID, t

// group JOIN/LEAVE messages
newMsgs := t.groupGeneric(ctx, uid, tv.Messages,
func(msg chat1.MessageUnboxed) bool {
func(msg chat1.MessageUnboxed, grouped []chat1.MessageUnboxed) bool {
body := msg.Valid().MessageBody
mtyp, err := body.MessageType()
return (err == nil && (mtyp == chat1.MessageType_JOIN ||
mtyp == chat1.MessageType_LEAVE) && !msg.Valid().ClientHeader.Sender.Eq(uid))
if err != nil {
return false
}
if !(mtyp == chat1.MessageType_JOIN || mtyp == chat1.MessageType_LEAVE) {
return false
}
if msg.Valid().ClientHeader.Sender.Eq(uid) {
return false
}
for _, g := range grouped {
joshblum marked this conversation as resolved.
Show resolved Hide resolved
if g.Valid().SenderUsername == msg.Valid().SenderUsername {
return false
}
}
return true
},
func(grouped []chat1.MessageUnboxed) *chat1.MessageUnboxed {
var joiners, leavers []string
Expand All @@ -103,16 +120,17 @@ func (t *UIThreadLoader) groupThreadView(ctx context.Context, uid gregor1.UID, t

var activeMap map[string]struct{}
// group BULKADDTOCONV system messages
newMsgs = t.groupGeneric(ctx, uid, newMsgs, func(msg chat1.MessageUnboxed) bool {
body := msg.Valid().MessageBody
mtyp, err := body.MessageType()
if err == nil && mtyp == chat1.MessageType_SYSTEM {
body := msg.Valid().MessageBody.System()
typ, err := body.SystemType()
return err == nil && typ == chat1.MessageSystemType_BULKADDTOCONV
}
return false
},
newMsgs = t.groupGeneric(ctx, uid, newMsgs,
func(msg chat1.MessageUnboxed, grouped []chat1.MessageUnboxed) bool {
body := msg.Valid().MessageBody
mtyp, err := body.MessageType()
if err == nil && mtyp == chat1.MessageType_SYSTEM {
body := msg.Valid().MessageBody.System()
typ, err := body.SystemType()
return err == nil && typ == chat1.MessageSystemType_BULKADDTOCONV
}
return false
},
func(grouped []chat1.MessageUnboxed) *chat1.MessageUnboxed {
var filteredUsernames, usernames []string
for _, j := range grouped {
Expand Down
40 changes: 24 additions & 16 deletions go/chat/uithreadloader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
func TestUIThreadLoaderGrouper(t *testing.T) {
useRemoteMock = false
defer func() { useRemoteMock = true }()
ctc := makeChatTestContext(t, "TestUIThreadLoaderGrouper", 5)
ctc := makeChatTestContext(t, "TestUIThreadLoaderGrouper", 6)
defer ctc.cleanup()

timeout := 2 * time.Second
Expand Down Expand Up @@ -90,9 +90,12 @@ func TestUIThreadLoaderGrouper(t *testing.T) {
lastLeave := consumeNewMsgRemote(t, listener0, chat1.MessageType_LEAVE)
consumeLeaveConv(t, listener1)

_, err = ctc.as(t, users[1]).chatLocalHandler().JoinConversationByIDLocal(ctx, conv.Id)
_, err = ctc.as(t, users[5]).chatLocalHandler().JoinConversationByIDLocal(ctx, conv.Id)
require.NoError(t, err)
consumeNewMsgRemote(t, listener0, chat1.MessageType_JOIN)
_, err = ctc.as(t, users[5]).chatLocalHandler().LeaveConversationLocal(ctx, conv.Id)
require.NoError(t, err)
consumeNewMsgRemote(t, listener0, chat1.MessageType_LEAVE)

require.NoError(t, tc.Context().ConvSource.Clear(ctx, conv.Id, uid))
_, err = tc.Context().ConvSource.GetMessages(ctx, convFull.Conv, uid,
Expand All @@ -115,30 +118,35 @@ func TestUIThreadLoaderGrouper(t *testing.T) {
case res := <-chatUI.ThreadCb:
require.False(t, res.Full)

require.Equal(t, 7, len(res.Thread.Messages))
require.Equal(t, 9, len(res.Thread.Messages))

require.True(t, res.Thread.Messages[0].IsPlaceholder())
require.True(t, res.Thread.Messages[1].IsPlaceholder())

require.Equal(t, chat1.MessageType_JOIN, res.Thread.Messages[2].GetMessageType())
require.Equal(t, 1, len(res.Thread.Messages[2].Valid().MessageBody.Join().Joiners))
require.Equal(t, 1, len(res.Thread.Messages[2].Valid().MessageBody.Join().Leavers))

require.Equal(t, chat1.MessageType_JOIN, res.Thread.Messages[1].GetMessageType())
require.Equal(t, 1, len(res.Thread.Messages[1].Valid().MessageBody.Join().Joiners))
require.Equal(t, 1, len(res.Thread.Messages[1].Valid().MessageBody.Join().Leavers))
require.Equal(t, chat1.MessageType_TEXT, res.Thread.Messages[3].GetMessageType())

require.Equal(t, chat1.MessageType_TEXT, res.Thread.Messages[2].GetMessageType())
require.Equal(t, chat1.MessageType_JOIN, res.Thread.Messages[4].GetMessageType())
require.Zero(t, len(res.Thread.Messages[4].Valid().MessageBody.Join().Joiners))
require.Equal(t, 1, len(res.Thread.Messages[4].Valid().MessageBody.Join().Leavers))

require.Equal(t, chat1.MessageType_JOIN, res.Thread.Messages[3].GetMessageType())
require.Equal(t, 2, len(res.Thread.Messages[3].Valid().MessageBody.Join().Joiners))
require.Equal(t, 1, len(res.Thread.Messages[3].Valid().MessageBody.Join().Leavers))
require.Equal(t, chat1.MessageType_JOIN, res.Thread.Messages[5].GetMessageType())
require.Equal(t, 2, len(res.Thread.Messages[5].Valid().MessageBody.Join().Joiners))
require.Zero(t, len(res.Thread.Messages[5].Valid().MessageBody.Join().Leavers))

require.Equal(t, chat1.MessageType_SYSTEM, res.Thread.Messages[4].GetMessageType())
bod := res.Thread.Messages[4].Valid().MessageBody.System()
require.Equal(t, chat1.MessageType_SYSTEM, res.Thread.Messages[6].GetMessageType())
bod := res.Thread.Messages[6].Valid().MessageBody.System()
sysTyp, err := bod.SystemType()
require.NoError(t, err)
require.Equal(t, chat1.MessageSystemType_BULKADDTOCONV, sysTyp)
require.Equal(t, 2, len(bod.Bulkaddtoconv().Usernames))

require.Equal(t, chat1.MessageType_JOIN, res.Thread.Messages[5].GetMessageType())
require.Zero(t, len(res.Thread.Messages[5].Valid().MessageBody.Join().Joiners))
require.Zero(t, len(res.Thread.Messages[5].Valid().MessageBody.Join().Leavers))
require.Equal(t, chat1.MessageType_JOIN, res.Thread.Messages[7].GetMessageType())
require.Zero(t, len(res.Thread.Messages[7].Valid().MessageBody.Join().Joiners))
require.Zero(t, len(res.Thread.Messages[7].Valid().MessageBody.Join().Leavers))

case <-time.After(timeout):
require.Fail(t, "no full cb")
Expand All @@ -148,7 +156,7 @@ func TestUIThreadLoaderGrouper(t *testing.T) {
select {
case res := <-chatUI.ThreadCb:
require.True(t, res.Full)
require.Equal(t, 5, len(res.Thread.Messages))
require.Equal(t, 7, len(res.Thread.Messages))
for _, msg := range res.Thread.Messages {
switch msg.GetMessageID() {
case lastLeave.GetMessageID():
Expand Down
2 changes: 1 addition & 1 deletion shared/chat/conversation/messages/system-joined/container.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const mapStateToProps = (state, {message}) => ({
_meta: Constants.getMeta(state, message.conversationIDKey),
author: message.author,
authorIsYou: state.config.username === message.author,
joiners: message.joiners,
joiners: !message.joiners.length && !message.leavers.length ? [message.author] : message.joiners,
mmaxim marked this conversation as resolved.
Show resolved Hide resolved
leavers: message.leavers,
timestamp: message.timestamp,
})
Expand Down