Skip to content

Commit

Permalink
journeycard changes (keybase#20677)
Browse files Browse the repository at this point in the history 
* journeycard changes

* remove debug

* lint

* fix test

* fix test

* lint
  • Loading branch information
@mlsteele
mlsteele authored Oct 30, 2019
1 parent 6eebe81 commit 4f31426
Show file tree
Hide file tree
Showing 5 changed files with 220 additions and 79 deletions.
20 changes: 16 additions & 4 deletions go/chat/convsource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,19 @@ func (s *baseConversationSource) addConversationCards(ctx context.Context, uid g
if card == nil {
return
}
thread.Messages = append([]chat1.MessageUnboxed{chat1.NewMessageUnboxedWithJourneycard(*card)}, thread.Messages...)
// Slot it in to the left of its prev.
addLeftOf := 0
for i := len(thread.Messages) - 1; i >= 0; i-- {
msgID := thread.Messages[i].GetMessageID()
if msgID != 0 && msgID >= card.PrevID {
addLeftOf = i
break
}
}
// Insert at index: https://github.com/golang/go/wiki/SliceTricks#insert
thread.Messages = append(thread.Messages, chat1.MessageUnboxed{})
copy(thread.Messages[addLeftOf+1:], thread.Messages[addLeftOf:])
thread.Messages[addLeftOf] = chat1.NewMessageUnboxedWithJourneycard(*card)
}

func (s *baseConversationSource) postProcessThread(ctx context.Context, uid gregor1.UID,
Expand Down Expand Up @@ -144,6 +156,9 @@ func (s *baseConversationSource) postProcessThread(ctx context.Context, uid greg
thread.Messages = utils.FilterExploded(conv, thread.Messages, s.boxer.clock.Now())
s.Debug(ctx, "postProcessThread: thread messages after explode filter: %d", len(thread.Messages))

// Add any conversation cards
s.addConversationCards(ctx, uid, conv.GetConvID(), verifiedConv, thread)

// Fetch outbox and tack onto the result
outbox := storage.NewOutbox(s.G(), uid)
if err = outbox.AppendToThread(ctx, conv.GetConvID(), thread); err != nil {
Expand All @@ -154,9 +169,6 @@ func (s *baseConversationSource) postProcessThread(ctx context.Context, uid greg
// Add attachment previews to pending messages
s.addPendingPreviews(ctx, thread)

// Add any conversation cards
s.addConversationCards(ctx, uid, conv.GetConvID(), verifiedConv, thread)

return nil
}

Expand Down
259 changes: 188 additions & 71 deletions go/chat/journey_card_checker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/keybase/client/go/libkb"
"github.com/keybase/client/go/protocol/chat1"
"github.com/keybase/client/go/protocol/gregor1"
"github.com/keybase/client/go/protocol/keybase1"
)

type JourneyCardChecker struct {
Expand Down Expand Up @@ -67,8 +68,12 @@ func (cc *JourneyCardChecker) PickCard(ctx context.Context, uid gregor1.UID,
}

var conv ConvEnough
var untrustedTeamRole keybase1.TeamRole
var tlfID chat1.TLFID
if convLocalOptional != nil {
conv = convLocalOptional
tlfID = convLocalOptional.Info.Triple.Tlfid
untrustedTeamRole = convLocalOptional.ReaderInfo.UntrustedTeamRole
} else {
convFromCache, err := utils.GetUnverifiedConv(ctx, cc.G(), uid, convID, types.InboxSourceDataSourceLocalOnly)
if err != nil {
Expand All @@ -79,12 +84,16 @@ func (cc *JourneyCardChecker) PickCard(ctx context.Context, uid gregor1.UID,
return nil, fmt.Errorf("conv LocalMetadata not found")
}
conv = convFromCache
tlfID = convFromCache.Conv.Metadata.IdTriple.Tlfid
if convFromCache.Conv.ReaderInfo != nil {
untrustedTeamRole = convFromCache.Conv.ReaderInfo.UntrustedTeamRole
}
}

inGeneralChannel := conv.GetTopicName() == globals.DefaultTeamTopic
if !(conv.GetTopicType() == chat1.TopicType_CHAT &&
conv.GetMembersType() == chat1.ConversationMembersType_TEAM &&
(conv.GetTopicName() == globals.DefaultTeamTopic || debug)) {
// Cards only exist in the general channel of team chats.
conv.GetMembersType() == chat1.ConversationMembersType_TEAM) {
// Cards only exist in team chats.
debugDebug(ctx, "conv not eligible for card: topicType:%v membersType:%v general:%v",
conv.GetTopicType(), conv.GetMembersType(), conv.GetTopicName() == globals.DefaultTeamTopic)
return nil, nil
Expand All @@ -101,58 +110,41 @@ func (cc *JourneyCardChecker) PickCard(ctx context.Context, uid gregor1.UID,

jcd := cc.getConvData(ctx, convID)

makeCard := func(cardType chat1.JourneycardType, highlightMsgID chat1.MessageID) (*chat1.MessageUnboxedJourneycard, error) {
pos := jcd.Positions[cardType]
makeCard := func(cardType chat1.JourneycardType, highlightMsgID chat1.MessageID, preferSavedPosition bool) (*chat1.MessageUnboxedJourneycard, error) {
// preferSavedPosition : If true, the card stays in the position it was previously seen. If false, the card goes at the bottom.
var pos *journeyCardPosition
if preferSavedPosition {
pos = jcd.Positions[cardType]
}
if pos == nil {
// Pick a message to use as the base for a frontend ordinal.
// Get out of the way of any other messages that have taken ordinal offsets of that message.
prevID := conv.MaxVisibleMsgID()
ordinal := 1 // offset within
for _, message := range thread.Messages {
if message.GetMessageID() > prevID {
prevID = message.GetMessageID()
ordinal = 1
}
state, err := message.State()
if err != nil {
continue
}
foundOrdinal := func(foundOrdinal int) {
if prevID == message.GetMessageID() && foundOrdinal > ordinal {
ordinal = foundOrdinal + 1
}
}
switch state {
case chat1.MessageUnboxedState_OUTBOX:
foundOrdinal(message.Outbox().Ordinal)
case chat1.MessageUnboxedState_JOURNEYCARD:
foundOrdinal(message.Journeycard().Ordinal)
}
}
if prevID == 0 {
debugDebug(ctx, "no message found to use as base for ordinal")
return nil, nil
}

pos = &journeyCardPosition{
PrevID: prevID,
Ordinal: ordinal,
PrevID: prevID,
}

go cc.savePosition(globals.BackgroundChatCtx(ctx, cc.G()), convID, cardType, *pos)

// TODO reserve ordinals. Otherwise outbox clobbers journey card ordinals. Something like this:
// go func() {
// ctx := globals.BackgroundChatCtx(ctx, cc.G())
// err := outbox.ReservePosition(ctx, convID, pos.PrevID, pos.Ordinal)
// if err != nil {
// cc.Debug(ctx, "ReservePosition error: %v", err)
// }
// }()
} else {
var foundPrev bool
for _, msg := range thread.Messages {
if msg.GetMessageID() == pos.PrevID {
foundPrev = true
break
}
}
// If the message that is being used as a prev is not found, omit the card.
// So that the card isn't presented at the edge of a far away page.
if !foundPrev {
return nil, nil
}
}
ordinal := 1 // Won't conflict with outbox messages since they are all <= outboxOrdinalStart.
return &chat1.MessageUnboxedJourneycard{
PrevID: pos.PrevID,
Ordinal: pos.Ordinal,
Ordinal: ordinal,
CardType: cardType,
HighlightMsgID: highlightMsgID,
}, nil
Expand All @@ -162,63 +154,189 @@ func (cc *JourneyCardChecker) PickCard(ctx context.Context, uid gregor1.UID,
// for testing, do special stuff based on channel name:
switch conv.GetTopicName() {
case "kb_cards_0_kb":
return makeCard(chat1.JourneycardType_WELCOME, 0)
return makeCard(chat1.JourneycardType_WELCOME, 0, false)
case "kb_cards_1_kb":
return makeCard(chat1.JourneycardType_POPULAR_CHANNELS, 0)
return makeCard(chat1.JourneycardType_POPULAR_CHANNELS, 0, false)
case "kb_cards_2_kb":
return makeCard(chat1.JourneycardType_ADD_PEOPLE, 0)
return makeCard(chat1.JourneycardType_ADD_PEOPLE, 0, false)
case "kb_cards_3_kb":
return makeCard(chat1.JourneycardType_CREATE_CHANNELS, 0)
return makeCard(chat1.JourneycardType_CREATE_CHANNELS, 0, false)
case "kb_cards_4_kb":
return makeCard(chat1.JourneycardType_MSG_ATTENTION, 3)
return makeCard(chat1.JourneycardType_MSG_ATTENTION, 3, false)
case "kb_cards_5_kb":
return makeCard(chat1.JourneycardType_USER_AWAY_FOR_LONG, 0)
return makeCard(chat1.JourneycardType_USER_AWAY_FOR_LONG, 0, false)
case "kb_cards_6_kb":
return makeCard(chat1.JourneycardType_CHANNEL_INACTIVE, 0)
return makeCard(chat1.JourneycardType_CHANNEL_INACTIVE, 0, false)
case "kb_cards_7_kb":
return makeCard(chat1.JourneycardType_MSG_NO_ANSWER, 0)
return makeCard(chat1.JourneycardType_MSG_NO_ANSWER, 0, false)
}
}

// TODO card type: WELCOME (interaction with existing system message) (persist its location, disappear when other cards come in)
// TODO factor out individual message filters. This func is getting a bit big. Also might help with dealing
// with the reversed priorities of cards that have already been shown. Maybe if no new cards trigger, show the
// latest already shown card.

// TODO card type: WELCOME (1) (interaction with existing system message) (persist its location, disappear when other cards come in)
// Condition: Only in #general channel

// Card type: MSG_NO_ANSWER (C)
// Gist: "People haven't been talkative in a while. Perhaps post in another channel? <list of channels>"
// Condition: The last visible message is old, was sent by the logged-in user, and was a long text message.
{
// If the latest message is eligible then show the card.
var eligibleMsg chat1.MessageID // maximum eligible msg
var preventerMsg chat1.MessageID // maximum preventer msg
save := func(msgID chat1.MessageID, eligible bool) {
if eligible {
if msgID > eligibleMsg {
eligibleMsg = msgID
}
} else {
if msgID > preventerMsg {
preventerMsg = msgID
}
}
}
msgscan:
for _, msg := range thread.Messages {
state, err := msg.State()
if err != nil {
continue
}
switch state {
case chat1.MessageUnboxedState_VALID:
msg.GetMessageID()
eligible := func() bool {
if !msg.IsValidFull() {
return false
}
if !msg.Valid().ClientHeader.Sender.Eq(uid) {
return false
}
switch msg.GetMessageType() {
case chat1.MessageType_TEXT:
const howLongIsLong = 40
const howOldIsOld = time.Hour * 24
isLong := (len(msg.Valid().MessageBody.Text().Body) >= howLongIsLong)
isOld := (cc.G().GetClock().Since(msg.Valid().ServerHeader.Ctime.Time()) >= howOldIsOld)
answer := isLong && isOld
return answer
default:
return false
}
}
if eligible() {
save(msg.GetMessageID(), true)
} else {
save(msg.GetMessageID(), false)
}
case chat1.MessageUnboxedState_ERROR:
save(msg.Error().MessageID, false)
case chat1.MessageUnboxedState_OUTBOX:
// If there's something in the outbox, don't show this card.
eligibleMsg = 0
preventerMsg = 9999
break msgscan
case chat1.MessageUnboxedState_PLACEHOLDER:
save(msg.Placeholder().MessageID, false)
case chat1.MessageUnboxedState_JOURNEYCARD:
save(msg.Journeycard().PrevID, false)
default:
debugDebug(ctx, "unrecognized message state: %v", state)
continue
}
}
show := eligibleMsg != 0 && eligibleMsg >= preventerMsg
if show {
return makeCard(chat1.JourneycardType_MSG_NO_ANSWER, 0, true)
}
}

// Card type: ADD_PEOPLE (3)
// Gist: "Do you know people interested in joining?"
// Condition: User is an admin.
// Condition: User has sent messages OR joined channels.
if untrustedTeamRole.IsAdminOrAbove() {
show := jcd.SentMessage || !inGeneralChannel
if !show {
// Figure whether the user is in other channels.
topicType := chat1.TopicType_CHAT
inbox, err := cc.G().InboxSource.ReadUnverified(ctx, uid, types.InboxSourceDataSourceLocalOnly, &chat1.GetInboxQuery{
TlfID: &tlfID,
// ConvIDs: []chat1.ConversationID{convID},
TopicType: &topicType,
MemberStatus: []chat1.ConversationMemberStatus{
chat1.ConversationMemberStatus_ACTIVE,
chat1.ConversationMemberStatus_REMOVED,
chat1.ConversationMemberStatus_LEFT,
chat1.ConversationMemberStatus_PREVIEW,
},
MembersTypes: []chat1.ConversationMembersType{chat1.ConversationMembersType_TEAM},
SummarizeMaxMsgs: true,
SkipBgLoads: true,
}, nil)
if err != nil {
debugDebug(ctx, "ReadUnverified error: %v", err)
} else {
debugDebug(ctx, "ReadUnverified found %v convs", len(inbox.ConvsUnverified))
for _, convOther := range inbox.ConvsUnverified {
if !convOther.GetConvID().Eq(convID) {
debugDebug(ctx, "ReadUnverified found alternate conv: %v", convOther.GetTopicName()) // xxx do not log this
show = true
break
}
}
}
}
if show {
return makeCard(chat1.JourneycardType_ADD_PEOPLE, 0, true)
}
}

timeSinceJoined := func(duration time.Duration) bool {
if jcd.JoinedTime == nil {
debugDebug(ctx, "missing joined time")
go cc.saveJoinedTime(globals.BackgroundChatCtx(ctx, cc.G()), convID, cc.G().GetClock().Now())
return false
}
return cc.G().GetClock().Since(jcd.JoinedTime.Time()) >= duration
}

// Card type: POPULAR_CHANNELS
// Gist: "You are in #general. Other popular channels in this team: diplomacy, sportsball"
// Condition: Only in #general channel
// Condition: The team has channels besides general.
// Condition: User has sent a first message OR a few days have passed since they joined the channel.
otherChannelsExist := conv.GetTeamType() == chat1.TeamType_COMPLEX
if otherChannelsExist {
if (inGeneralChannel || debug) && otherChannelsExist {
debugDebug(ctx, "other channels exist")
show := jcd.SentMessage
if !show {
if jcd.JoinedTime == nil {
debugDebug(ctx, "missing joined time")
go cc.saveJoinedTime(globals.BackgroundChatCtx(ctx, cc.G()), convID, cc.G().GetClock().Now())
} else {
cutoff := time.Hour * 24 * 2
if cc.G().GetClock().Since(jcd.JoinedTime.Time()) >= cutoff {
show = true
} else {
debugDebug(ctx, "not past time")
}
}
if !show && timeSinceJoined(time.Hour*24*2) {
show = true
}
if show {
return makeCard(chat1.JourneycardType_POPULAR_CHANNELS, 0)
return makeCard(chat1.JourneycardType_POPULAR_CHANNELS, 0, true)
}
}

// TODO card type: ADD_PEOPLE

// TODO card type: CREATE_CHANNELS
// Gist: "Go ahead and create #channels around topics you think are missing."
// Condition: A few weeks have passed.
// Condition: User has sent a message.
if jcd.SentMessage && timeSinceJoined(time.Hour*24*14) {
return makeCard(chat1.JourneycardType_CREATE_CHANNELS, 0, true)
}

// TODO card type: MSG_ATTENTION
// Gist: "One of your messages is getting a lot of attention! <pointer to message>"
// Condition: The logged-in user's message gets a lot of reacjis
// Condition: That message is above the fold.

// TODO card type: USER_AWAY_FOR_LONG
// Gist: "Long time no see.... Look at all the things you missed."

// TODO card type: CHANNEL_INACTIVE

// TODO card type: MSG_NO_ANSWER
// Gist: "Zzz... This channel hasn't been very active... Revive it?"

debugDebug(ctx, "no card at end of checks")
return nil, nil
Expand Down Expand Up @@ -308,8 +426,7 @@ func (cc *JourneyCardChecker) saveJoinedTime(ctx context.Context, convID chat1.C
}

type journeyCardPosition struct {
PrevID chat1.MessageID
Ordinal int
PrevID chat1.MessageID
}

// Storage for a single conversation's journey cards.
Expand Down
Loading

0 comments on commit 4f31426

Please sign in to comment.