Journey card changes (keybase#20463)

* type comments

* comment

* Add a placeholder for rendering journey cards

* journeycard

* jenkins

* lint

* log

* disable lint singleCaseSwitch

* Revert "disable lint singleCaseSwitch"

This reverts commit 83fe6df.

Jenkinsfile

@@ -482,6 +482,7 @@ def testGo(prefix, packagesToTest) {
     println "Installing golangci-lint"
     dir("..") {
       retry(5) {
+        // This works with go1.12.12 but not go1.13.1 with an error containing "invalid pseudo-version"
         sh 'GO111MODULE=on go get github.com/golangci/golangci-lint/cmd/[email protected]'
       }
     }

go/chat/convsource.go

@@ -94,17 +94,14 @@ func (s *baseConversationSource) addConversationCards(ctx context.Context, uid g
 	cc := newJourneyCardChecker(s.G())
 	card, err := cc.Next(ctx, uid, conv, thread)
 	if err != nil {
-		s.Debug(ctx, "error getting next conversation card: %s", err)
+		s.Debug(ctx, "addConversationCards: error getting next conversation card: %s", err)
 		return
 	}
-
 	if card == nil {
-		s.Debug(ctx, "card is nil")
 		return
 	}
-
-	s.Debug(ctx, "got a card for this conversation: %+v", card)
-	thread.Messages = append(thread.Messages, chat1.NewMessageUnboxedWithJourneycard(*card))
+	s.Debug(ctx, "addConversationCards: got a card: %+v", card)
+	thread.Messages = append([]chat1.MessageUnboxed{chat1.NewMessageUnboxedWithJourneycard(*card)}, thread.Messages...)
 }
 
 func (s *baseConversationSource) postProcessThread(ctx context.Context, uid gregor1.UID,

go/chat/journey_card_checker.go

@@ -2,6 +2,7 @@ package chat
 
 import (
 	"context"
+	"runtime/debug"
 
 	"github.com/keybase/client/go/chat/globals"
 	"github.com/keybase/client/go/chat/utils"
@@ -22,10 +23,15 @@ func newJourneyCardChecker(g *globals.Context) *journeyCardChecker {
 	}
 }
 
-func (cc *journeyCardChecker) Next(ctx context.Context, uid gregor1.UID, conv *chat1.ConversationLocal, thread *chat1.ThreadView) (*chat1.MessageUnboxedJourneyCard, error) {
+func (cc *journeyCardChecker) Next(ctx context.Context, uid gregor1.UID, conv *chat1.ConversationLocal, thread *chat1.ThreadView) (*chat1.MessageUnboxedJourneycard, error) {
+	if !cc.G().GetEnv().GetDebugJourneycard() {
+		// Journey cards are gated by the client-side flag KEYBASE_DEBUG_JOURNEYCARD
+		return nil, nil
+	}
 	if conv == nil {
 		// if no verified conversation parameter, don't do anything
-		cc.Debug(ctx, "no conversation parameter")
+		// if this happens the first question will by "why?", hence the stack
+		cc.Debug(ctx, "no conversation parameter\n%v", string(debug.Stack()))
 		return nil, nil
 	}
 
@@ -40,31 +46,66 @@ func (cc *journeyCardChecker) Next(ctx context.Context, uid gregor1.UID, conv *c
 
 	if conv.GetMembersType() != chat1.ConversationMembersType_TEAM {
 		// currently, cards only exist in team conversations
-		cc.Debug(ctx, "not a team conversation")
 		return nil, 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 {
+		// No message found to use as base for ordinal.
+		return nil, nil
+	}
+
+	makeCard := func(cardType chat1.JourneycardType, highlightMsgID chat1.MessageID) (*chat1.MessageUnboxedJourneycard, error) {
+		return &chat1.MessageUnboxedJourneycard{
+			PrevID:         prevID,
+			Ordinal:        ordinal,
+			CardType:       cardType,
+			HighlightMsgID: highlightMsgID,
+		}, nil
+	}
+
 	// for testing, do special stuff based on channel name:
 	switch conv.Info.TopicName {
 	case "kb_cards_0_kb":
-		return &chat1.MessageUnboxedJourneyCard{CardType: chat1.MessageUnboxedJourneyCardType_WELCOME, Data: "{\"msg\": \"hello\"}"}, nil
+		return makeCard(chat1.JourneycardType_WELCOME, 0)
 	case "kb_cards_1_kb":
-		return &chat1.MessageUnboxedJourneyCard{CardType: chat1.MessageUnboxedJourneyCardType_POPULAR_CHANNELS, Data: "{\"msg\": \"Other popular channels:\", \"channels\": [\"public\", \"announcements\"]}"}, nil
+		return makeCard(chat1.JourneycardType_POPULAR_CHANNELS, 0)
 	case "kb_cards_2_kb":
-		return &chat1.MessageUnboxedJourneyCard{CardType: chat1.MessageUnboxedJourneyCardType_ADD_PEOPLE, Data: "{\"msg\": \"add some friends\"}"}, nil
+		return makeCard(chat1.JourneycardType_ADD_PEOPLE, 0)
 	case "kb_cards_3_kb":
-		return &chat1.MessageUnboxedJourneyCard{CardType: chat1.MessageUnboxedJourneyCardType_CREATE_CHANNELS, Data: "{\"msg\": \"create some channels\"}"}, nil
+		return makeCard(chat1.JourneycardType_CREATE_CHANNELS, 0)
 	case "kb_cards_4_kb":
-		return &chat1.MessageUnboxedJourneyCard{CardType: chat1.MessageUnboxedJourneyCardType_MSG_ATTENTION, Data: "{\"msg\": \"one of your messages got a lot of attention\", \"msgid\": 2}"}, nil
+		return makeCard(chat1.JourneycardType_MSG_ATTENTION, 3)
 	case "kb_cards_5_kb":
-		return &chat1.MessageUnboxedJourneyCard{CardType: chat1.MessageUnboxedJourneyCardType_USER_AWAY_FOR_LONG, Data: "{\"msg\": \"see what you missed\", \"lastread_msgid\": 2}"}, nil
+		return makeCard(chat1.JourneycardType_USER_AWAY_FOR_LONG, 0)
 	case "kb_cards_6_kb":
-		return &chat1.MessageUnboxedJourneyCard{CardType: chat1.MessageUnboxedJourneyCardType_CHANNEL_INACTIVE, Data: "{\"msg\": \"This channel isn't very active.  Revive it?\"}"}, nil
+		return makeCard(chat1.JourneycardType_CHANNEL_INACTIVE, 0)
 	case "kb_cards_7_kb":
-		return &chat1.MessageUnboxedJourneyCard{CardType: chat1.MessageUnboxedJourneyCardType_POPULAR_CHANNELS, Data: "{\"msg\": \"People haven't been talkative in a while. Perhaps post in another channel?\", \"channels\": [\"public\", \"announcements\", \"random\"]}"}, nil
+		return makeCard(chat1.JourneycardType_POPULAR_CHANNELS, 0)
 	}
-
-	cc.Debug(ctx, "no card needed")
-
 	return nil, nil
 }

go/chat/utils/utils.go

@@ -1354,7 +1354,16 @@ func PresentThreadView(ctx context.Context, g *globals.Context, uid gregor1.UID,
 }
 
 func computeOutboxOrdinal(obr chat1.OutboxRecord) float64 {
-	return float64(obr.Msg.ClientHeader.OutboxInfo.Prev) + float64(obr.Ordinal)/1000.0
+	return computeOrdinal(obr.Msg.ClientHeader.OutboxInfo.Prev, obr.Ordinal)
+}
+
+// Compute an "ordinal". There are two senses of "ordinal".
+// The service considers ordinals ints, like 3, which are the offset after some message ID.
+// The frontend considers ordinals floats like "180.03" where before the dot is
+// a message ID, and after the dot is a sub-position in thousandths.
+// This function translates from the service's sense to the frontend's sense.
+func computeOrdinal(messageID chat1.MessageID, serviceOrdinal int) (frontendOrdinal float64) {
+	return float64(messageID) + float64(serviceOrdinal)/1000.0
 }
 
 func PresentChannelNameMentions(ctx context.Context, crs []chat1.ChannelNameMention) (res []chat1.UIChannelNameMention) {
@@ -1786,6 +1795,16 @@ func PresentMessageUnboxed(ctx context.Context, g *globals.Context, rawMsg chat1
 		res = chat1.NewUIMessageWithError(rawMsg.Error())
 	case chat1.MessageUnboxedState_PLACEHOLDER:
 		res = chat1.NewUIMessageWithPlaceholder(rawMsg.Placeholder())
+	case chat1.MessageUnboxedState_JOURNEYCARD:
+		journeycard := rawMsg.Journeycard()
+		res = chat1.NewUIMessageWithJourneycard(chat1.UIMessageJourneycard{
+			Ordinal:        computeOrdinal(journeycard.PrevID, journeycard.Ordinal),
+			CardType:       journeycard.CardType,
+			HighlightMsgID: journeycard.HighlightMsgID,
+		})
+	default:
+		g.MetaContext(ctx).Debug("PresentMessageUnboxed: unhandled MessageUnboxedState: %v", state)
+		// res = zero values
 	}
 	return res
 }

go/client/chat_svc_handler.go

@@ -493,21 +493,6 @@ func (c *chatServiceHandler) formatMessages(ctx context.Context, messages []chat
 			continue
 		}
 
-		if st == chat1.MessageUnboxedState_JOURNEYCARD {
-			mc := m.Journeycard()
-			ret = append(ret, chat1.Message{
-				Msg: &chat1.MsgSummary{
-					Content: chat1.MsgContent{
-						TypeName: chat1.MessageUnboxedJourneyCardTypeRevMap[mc.CardType],
-						Text: &chat1.MessageText{
-							Body: mc.Data,
-						},
-					},
-				},
-			})
-			continue
-		}
-
 		// skip any PLACEHOLDER or OUTBOX messages
 		if st != chat1.MessageUnboxedState_VALID {
 			continue

go/libcmdline/cmdline.go

@@ -117,6 +117,9 @@ func (p CommandLine) GetDebug() (bool, bool) {
 	}
 	return p.GetBool("debug", true)
 }
+func (p CommandLine) GetDebugJourneycard() (bool, bool) {
+	return p.GetBool("debug-journeycard", true)
+}
 func (p CommandLine) GetDisplayRawUntrustedOutput() (bool, bool) {
 	return p.GetBool("display-raw-untrusted-output", true)
 }
@@ -745,6 +748,10 @@ func (p *CommandLine) PopulateApp(addHelp bool, extraFlags []cli.Flag) {
 			Name:  "vdebug",
 			Usage: "Verbose debugging; takes a comma-joined list of levels and tags",
 		},
+		cli.BoolFlag{
+			Name:  "debug-journeycard",
+			Usage: "Enable experimental journey cards",
+		},
 		cli.BoolFlag{
 			Name:  "disable-team-auditor",
 			Usage: "Disable auditing of teams",

go/libkb/config.go

@@ -24,6 +24,8 @@ type JSONConfigFile struct {
 	userConfigWrapper *UserConfigWrapper
 }
 
+var _ (ConfigReader) = (*JSONConfigFile)(nil)
+
 func NewJSONConfigFile(g *GlobalContext, s string) *JSONConfigFile {
 	return &JSONConfigFile{NewJSONFile(g, s, "config"), &UserConfigWrapper{}}
 }
@@ -569,6 +571,9 @@ func (f *JSONConfigFile) IsCertPinningEnabled() bool {
 func (f *JSONConfigFile) GetDebug() (bool, bool) {
 	return f.GetTopLevelBool("debug")
 }
+func (f *JSONConfigFile) GetDebugJourneycard() (bool, bool) {
+	return f.GetTopLevelBool("debug_journeycard")
+}
 func (f *JSONConfigFile) GetDisplayRawUntrustedOutput() (bool, bool) {
 	return f.GetTopLevelBool("display_raw_untrusted_output")
 }

go/libkb/env.go

@@ -155,9 +155,8 @@ func (n NullConfiguration) GetAllUserConfigs() (*UserConfig, []UserConfig, error
 	return nil, nil, nil
 }
 
-func (n NullConfiguration) GetDebug() (bool, bool) {
-	return false, false
-}
+func (n NullConfiguration) GetDebug() (bool, bool)            { return false, false }
+func (n NullConfiguration) GetDebugJourneycard() (bool, bool) { return false, false }
 func (n NullConfiguration) GetDisplayRawUntrustedOutput() (bool, bool) {
 	return false, false
 }
@@ -818,6 +817,14 @@ func (e *Env) GetProveBypass() bool {
 		func() (bool, bool) { return e.GetConfig().GetProveBypass() })
 }
 
+// GetDebugJourneycard enables experimental chat journey cards.
+func (e *Env) GetDebugJourneycard() bool {
+	return e.GetBool(false,
+		func() (bool, bool) { return e.cmd.GetDebugJourneycard() },
+		func() (bool, bool) { return e.getEnvBool("KEYBASE_DEBUG_JOURNEYCARD") },
+		func() (bool, bool) { return e.GetConfig().GetDebugJourneycard() })
+}
+
 func (e *Env) GetDebug() bool {
 	return e.GetBool(false,
 		func() (bool, bool) { return e.Test.GetDebug() },

go/libkb/interfaces.go

@@ -46,6 +46,7 @@ type configGetter interface {
 	GetConfigFilename() string
 	GetDbFilename() string
 	GetDebug() (bool, bool)
+	GetDebugJourneycard() (bool, bool)
 	GetDisplayRawUntrustedOutput() (bool, bool)
 	GetGpg() string
 	GetGpgHome() string