fix up cli tabular display and locationRef (#4594)

#### Does this PR need a docs update or release note?

- [x] ⛔ No

#### Type of change

- [x] 🌻 Feature

#### Issue(s)

* #4536

#### Test Plan

- [x] 💪 Manual
- [x] ⚡ Unit test
- [x] 💚 E2E

src/cli/backup/groups_e2e_test.go

@@ -464,10 +464,6 @@ func runGroupsDetailsCmdTest(suite *PreparedBackupGroupsE2ESuite, category path.
 
 	t := suite.T()
 
-	if category == path.ConversationPostsCategory {
-		t.Skip("skipping conversation details test, see issue #4780")
-	}
-
 	ctx, flush := tester.NewContext(t)
 	ctx = config.SetViper(ctx, suite.dpnd.vpr)
 

src/cli/utils/groups_test.go

@@ -42,32 +42,28 @@ func (suite *GroupsUtilsSuite) TestIncludeGroupsRestoreDataSelectors() {
 		{
 			name:             "no inputs",
 			opts:             utils.GroupsOpts{},
-			// TODO: bump to 3 when we release conversations
-			expectIncludeLen: 2,
+			expectIncludeLen: 3,
 		},
 		{
 			name: "empty",
 			opts: utils.GroupsOpts{
 				Groups: empty,
 			},
-			// TODO: bump to 3 when we release conversations
-			expectIncludeLen: 2,
+			expectIncludeLen: 3,
 		},
 		{
 			name: "single inputs",
 			opts: utils.GroupsOpts{
 				Groups: single,
 			},
-			// TODO: bump to 3 when we release conversations
-			expectIncludeLen: 2,
+			expectIncludeLen: 3,
 		},
 		{
 			name: "multi inputs",
 			opts: utils.GroupsOpts{
 				Groups: multi,
 			},
-			// TODO: bump to 3 when we release conversations
-			expectIncludeLen: 2,
+			expectIncludeLen: 3,
 		},
 		// sharepoint
 		{
@@ -421,7 +417,7 @@ func (suite *GroupsUtilsSuite) TestAddGroupsCategories() {
 		{
 			name:           "none",
 			cats:           []string{},
-			expectScopeLen: 2,
+			expectScopeLen: 3,
 		},
 		{
 			name:           "libraries",
@@ -443,10 +439,9 @@ func (suite *GroupsUtilsSuite) TestAddGroupsCategories() {
 			cats: []string{
 				flags.DataLibraries,
 				flags.DataMessages,
-				// flags.DataConversations,
+				flags.DataConversations,
 			},
-			// TODO: bump to 3 when we include conversations in all data
-			expectScopeLen: 2,
+			expectScopeLen: 3,
 		},
 		{
 			name:           "bad inputs",

src/internal/m365/collection/groups/backup.go

@@ -69,7 +69,7 @@ func CreateCollections[C graph.GetIDer, I groupsItemer](
 
 	counter.Add(count.Channels, int64(len(containers)))
 
-	collections, err := populateCollections(
+	collections, err := populateCollections[C, I](
 		ctx,
 		qp,
 		bh,
@@ -213,6 +213,7 @@ func populateCollections[C graph.GetIDer, I groupsItemer](
 			qp.ProtectedResource.ID(),
 			added,
 			removed,
+			c,
 			statusUpdater)
 
 		collections[c.storageDirFolders.String()] = &edc

src/internal/m365/collection/groups/backup_test.go

@@ -39,6 +39,7 @@ import (
 
 var _ backupHandler[models.Channelable, models.ChatMessageable] = &mockBackupHandler{}
 
+//lint:ignore U1000 false linter issue due to generics
 type mockBackupHandler struct {
 	channels      []models.Channelable
 	messageIDs    []string
@@ -50,6 +51,14 @@ type mockBackupHandler struct {
 	doNotInclude  bool
 }
 
+//lint:ignore U1000 false linter issue due to generics
+func (bh mockBackupHandler) augmentItemInfo(
+	*details.GroupsInfo,
+	models.Channelable,
+) {
+	// no-op
+}
+
 func (bh mockBackupHandler) canMakeDeltaQueries() bool {
 	return true
 }
@@ -116,7 +125,8 @@ func (bh mockBackupHandler) canonicalPath(
 			false)
 }
 
-func (bh mockBackupHandler) GetItem(
+//lint:ignore U1000 false linter issue due to generics
+func (bh mockBackupHandler) getItem(
 	_ context.Context,
 	_ string,
 	_ path.Elements,

src/internal/m365/collection/groups/channel_handler.go

@@ -95,7 +95,8 @@ func (bh channelsBackupHandler) PathPrefix(tenantID string) (path.Path, error) {
 		false)
 }
 
-func (bh channelsBackupHandler) GetItem(
+//lint:ignore U1000 false linter issue due to generics
+func (bh channelsBackupHandler) getItem(
 	ctx context.Context,
 	groupID string,
 	containerIDs path.Elements,
@@ -104,6 +105,14 @@ func (bh channelsBackupHandler) GetItem(
 	return bh.ac.GetChannelMessage(ctx, groupID, containerIDs[0], messageID)
 }
 
+//lint:ignore U1000 false linter issue due to generics
+func (bh channelsBackupHandler) augmentItemInfo(
+	dgi *details.GroupsInfo,
+	c models.Channelable,
+) {
+	// no-op
+}
+
 func channelContainer(ch models.Channelable) container[models.Channelable] {
 	return container[models.Channelable]{
 		storageDirFolders:   path.Elements{ptr.Val(ch.GetId())},

src/internal/m365/collection/groups/collection.go

@@ -17,26 +17,29 @@ import (
 	"github.com/alcionai/corso/src/pkg/count"
 	"github.com/alcionai/corso/src/pkg/fault"
 	"github.com/alcionai/corso/src/pkg/logger"
+	"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
 )
 
-var _ data.BackupCollection = &Collection[groupsItemer]{}
+var _ data.BackupCollection = &Collection[graph.GetIDer, groupsItemer]{}
 
 const (
 	collectionChannelBufferSize = 1000
 	numberOfRetries             = 4
 )
 
-type Collection[I groupsItemer] struct {
+type Collection[C graph.GetIDer, I groupsItemer] struct {
 	data.BaseCollection
 	protectedResource string
 	stream            chan data.Item
 
+	contains container[C]
+
 	// added is a list of existing item IDs that were added to a container
 	added map[string]struct{}
 	// removed is a list of item IDs that were deleted from, or moved out, of a container
 	removed map[string]struct{}
 
-	getter getItemer[I]
+	getAndAugment getItemAndAugmentInfoer[C, I]
 
 	statusUpdater support.StatusUpdater
 }
@@ -47,18 +50,20 @@ type Collection[I groupsItemer] struct {
 // to be deleted.  If the prev path is nil, it is assumed newly created.
 // If both are populated, then state is either moved (if they differ),
 // or notMoved (if they match).
-func NewCollection[I groupsItemer](
+func NewCollection[C graph.GetIDer, I groupsItemer](
 	baseCol data.BaseCollection,
-	getter getItemer[I],
+	getAndAugment getItemAndAugmentInfoer[C, I],
 	protectedResource string,
 	added map[string]struct{},
 	removed map[string]struct{},
+	contains container[C],
 	statusUpdater support.StatusUpdater,
-) Collection[I] {
-	collection := Collection[I]{
+) Collection[C, I] {
+	collection := Collection[C, I]{
 		BaseCollection:    baseCol,
 		added:             added,
-		getter:            getter,
+		contains:          contains,
+		getAndAugment:     getAndAugment,
 		removed:           removed,
 		statusUpdater:     statusUpdater,
 		stream:            make(chan data.Item, collectionChannelBufferSize),
@@ -70,7 +75,7 @@ func NewCollection[I groupsItemer](
 
 // Items utility function to asynchronously execute process to fill data channel with
 // M365 exchange objects and returns the data channel
-func (col *Collection[I]) Items(ctx context.Context, errs *fault.Bus) <-chan data.Item {
+func (col *Collection[C, I]) Items(ctx context.Context, errs *fault.Bus) <-chan data.Item {
 	go col.streamItems(ctx, errs)
 	return col.stream
 }
@@ -79,7 +84,7 @@ func (col *Collection[I]) Items(ctx context.Context, errs *fault.Bus) <-chan dat
 // items() production
 // ---------------------------------------------------------------------------
 
-func (col *Collection[I]) streamItems(ctx context.Context, errs *fault.Bus) {
+func (col *Collection[C, I]) streamItems(ctx context.Context, errs *fault.Bus) {
 	var (
 		streamedItems int64
 		totalBytes    int64
@@ -145,7 +150,7 @@ func (col *Collection[I]) streamItems(ctx context.Context, errs *fault.Bus) {
 			writer := kjson.NewJsonSerializationWriter()
 			defer writer.Close()
 
-			item, info, err := col.getter.GetItem(
+			item, info, err := col.getAndAugment.getItem(
 				ctx,
 				col.protectedResource,
 				col.FullPath().Folders(),
@@ -157,6 +162,8 @@ func (col *Collection[I]) streamItems(ctx context.Context, errs *fault.Bus) {
 				return
 			}
 
+			col.getAndAugment.augmentItemInfo(info, col.contains.container)
+
 			if err := writer.WriteObjectValue("", item); err != nil {
 				err = clues.Wrap(err, "writing channel message to serializer").Label(fault.LabelForceNoBackupCreation)
 				el.AddRecoverable(ctx, err)
@@ -207,7 +214,7 @@ func (col *Collection[I]) streamItems(ctx context.Context, errs *fault.Bus) {
 
 // finishPopulation is a utility function used to close a Collection's data channel
 // and to send the status update through the channel.
-func (col *Collection[I]) finishPopulation(
+func (col *Collection[C, I]) finishPopulation(
 	ctx context.Context,
 	streamedItems, totalBytes int64,
 	err error,

src/internal/m365/collection/groups/collection_test.go

@@ -2,6 +2,7 @@ package groups
 
 import (
 	"bytes"
+	"context"
 	"io"
 	"testing"
 	"time"
@@ -12,9 +13,9 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/common/readers"
 	"github.com/alcionai/corso/src/internal/data"
-	"github.com/alcionai/corso/src/internal/m365/collection/groups/mock"
 	"github.com/alcionai/corso/src/internal/m365/support"
 	"github.com/alcionai/corso/src/internal/tester"
 	"github.com/alcionai/corso/src/pkg/backup/details"
@@ -117,7 +118,7 @@ func (suite *CollectionUnitSuite) TestNewCollection_state() {
 		suite.Run(test.name, func() {
 			t := suite.T()
 
-			c := NewCollection[models.ChatMessageable](
+			c := NewCollection[models.Channelable, models.ChatMessageable](
 				data.NewBaseCollection(
 					test.curr,
 					test.prev,
@@ -127,7 +128,9 @@ func (suite *CollectionUnitSuite) TestNewCollection_state() {
 					count.New()),
 				nil,
 				"g",
-				nil, nil,
+				nil,
+				nil,
+				container[models.Channelable]{},
 				nil)
 			assert.Equal(t, test.expect, c.State(), "collection state")
 			assert.Equal(t, test.curr, c.FullPath(), "full path")
@@ -137,6 +140,28 @@ func (suite *CollectionUnitSuite) TestNewCollection_state() {
 	}
 }
 
+type getAndAugmentChannelMessage struct {
+	Err error
+}
+
+//lint:ignore U1000 false linter issue due to generics
+func (m getAndAugmentChannelMessage) getItem(
+	_ context.Context,
+	_ string,
+	_ path.Elements,
+	itemID string,
+) (models.ChatMessageable, *details.GroupsInfo, error) {
+	msg := models.NewChatMessage()
+	msg.SetId(ptr.To(itemID))
+
+	return msg, &details.GroupsInfo{}, m.Err
+}
+
+//lint:ignore U1000 false linter issue due to generics
+func (getAndAugmentChannelMessage) augmentItemInfo(*details.GroupsInfo, models.Channelable) {
+	// no-op
+}
+
 func (suite *CollectionUnitSuite) TestCollection_streamItems() {
 	var (
 		t             = suite.T()
@@ -199,7 +224,7 @@ func (suite *CollectionUnitSuite) TestCollection_streamItems() {
 			ctx, flush := tester.NewContext(t)
 			defer flush()
 
-			col := &Collection[models.ChatMessageable]{
+			col := &Collection[models.Channelable, models.ChatMessageable]{
 				BaseCollection: data.NewBaseCollection(
 					fullPath,
 					nil,
@@ -208,8 +233,9 @@ func (suite *CollectionUnitSuite) TestCollection_streamItems() {
 					false,
 					count.New()),
 				added:         test.added,
+				contains:      container[models.Channelable]{},
 				removed:       test.removed,
-				getter:        mock.GetChannelMessage{},
+				getAndAugment: getAndAugmentChannelMessage{},
 				stream:        make(chan data.Item),
 				statusUpdater: statusUpdater,
 			}

src/internal/m365/collection/groups/conversation_handler.go

@@ -113,7 +113,8 @@ func (bh conversationsBackupHandler) PathPrefix(tenantID string) (path.Path, err
 		false)
 }
 
-func (bh conversationsBackupHandler) GetItem(
+//lint:ignore U1000 false linter issue due to generics
+func (bh conversationsBackupHandler) getItem(
 	ctx context.Context,
 	groupID string,
 	containerIDs path.Elements, // expects: [conversationID, threadID]
@@ -128,6 +129,14 @@ func (bh conversationsBackupHandler) GetItem(
 		api.CallConfig{})
 }
 
+//lint:ignore U1000 false linter issue due to generics
+func (bh conversationsBackupHandler) augmentItemInfo(
+	dgi *details.GroupsInfo,
+	c models.Conversationable,
+) {
+	dgi.Post.Topic = ptr.Val(c.GetTopic())
+}
+
 func conversationThreadContainer(
 	c models.Conversationable,
 	t models.ConversationThreadable,
@@ -136,9 +145,8 @@ func conversationThreadContainer(
 		storageDirFolders: path.Elements{ptr.Val(c.GetId()), ptr.Val(t.GetId())},
 		// microsoft UX doesn't display any sort of container name that would make a reasonable
 		// "location" for the posts in the conversation.  We may need to revisit this, perhaps
-		// the subject is sufficiently acceptable.  But at this time it's left empty so that
-		// we don't populate it with problematic data.
-		humanLocation:       path.Elements{},
+		// the subject (aka topic) is sufficiently acceptable.
+		humanLocation:       path.Elements{ptr.Val(c.GetTopic())},
 		canMakeDeltaQueries: false,
 		container:           c,
 	}

src/internal/m365/collection/groups/handlers.go

@@ -25,13 +25,25 @@ type backupHandler[C graph.GetIDer, I groupsItemer] interface {
 	getItemer[I]
 	getContainerser[C]
 	getContainerItemIDser
+	getItemAndAugmentInfoer[C, I]
 	includeContainerer[C]
 	canonicalPather
 	canMakeDeltaQuerieser
 }
 
+type getItemAndAugmentInfoer[C graph.GetIDer, I groupsItemer] interface {
+	getItemer[I]
+	augmentItemInfoer[C]
+}
+
+type augmentItemInfoer[C graph.GetIDer] interface {
+	// augmentItemInfo completes the groupInfo population with any data
+	// owned by the container and not accessible to the item.
+	augmentItemInfo(*details.GroupsInfo, C)
+}
+
 type getItemer[I groupsItemer] interface {
-	GetItem(
+	getItem(
 		ctx context.Context,
 		protectedResource string,
 		containerIDs path.Elements,

src/internal/m365/collection/groups/mock/getter.go

@@ -1,27 +0,0 @@
-package mock
-
-import (
-	"context"
-
-	"github.com/microsoftgraph/msgraph-sdk-go/models"
-
-	"github.com/alcionai/corso/src/internal/common/ptr"
-	"github.com/alcionai/corso/src/pkg/backup/details"
-	"github.com/alcionai/corso/src/pkg/path"
-)
-
-type GetChannelMessage struct {
-	Err error
-}
-
-func (m GetChannelMessage) GetItem(
-	_ context.Context,
-	_ string,
-	_ path.Elements,
-	itemID string,
-) (models.ChatMessageable, *details.GroupsInfo, error) {
-	msg := models.NewChatMessage()
-	msg.SetId(ptr.To(itemID))
-
-	return msg, &details.GroupsInfo{}, m.Err
-}

src/pkg/backup/details/groups.go

@@ -65,7 +65,7 @@ type ConversationPostInfo struct {
 	Creator   string    `json:"creator,omitempty"`
 	Preview   string    `json:"preview,omitempty"`
 	Size      int64     `json:"size,omitempty"`
-	Subject   string    `json:"subject,omitempty"`
+	Topic     string    `json:"topic,omitempty"`
 }
 
 type ChannelMessageInfo struct {
@@ -86,6 +86,8 @@ func (i GroupsInfo) Headers() []string {
 		return []string{"ItemName", "Library", "ParentPath", "Size", "Owner", "Created", "Modified"}
 	case GroupsChannelMessage:
 		return []string{"Message", "Channel", "Subject", "Replies", "Creator", "Created", "Last Reply"}
+	case GroupsConversationPost:
+		return []string{"Post", "Conversation", "Sender", "Created"}
 	}
 
 	return []string{}
@@ -112,7 +114,7 @@ func (i GroupsInfo) Values() []string {
 		}
 
 		return []string{
-			// html parsing may produce newlijnes, which we'll want to avoid
+			// html parsing may produce newlines, which we'll want to avoid
 			strings.ReplaceAll(i.Message.Preview, "\n", "\\n"),
 			i.ParentPath,
 			i.Message.Subject,
@@ -121,6 +123,14 @@ func (i GroupsInfo) Values() []string {
 			dttm.FormatToTabularDisplay(i.Message.CreatedAt),
 			lastReply,
 		}
+	case GroupsConversationPost:
+		return []string{
+			// html parsing may produce newlines, which we'll want to avoid
+			strings.ReplaceAll(i.Post.Preview, "\n", "\\n"),
+			i.Post.Topic,
+			i.Post.Creator,
+			dttm.FormatToTabularDisplay(i.Post.CreatedAt),
+		}
 	}
 
 	return []string{}

src/pkg/backup/details/groups_test.go

@@ -21,13 +21,20 @@ func TestGroupsUnitSuite(t *testing.T) {
 }
 
 func (suite *GroupsUnitSuite) TestGroupsPrintable() {
-	t := suite.T()
 	now := time.Now()
 	then := now.Add(time.Minute)
 
-	gi := details.GroupsInfo{
+	table := []struct {
+		name     string
+		info     details.GroupsInfo
+		expectHs []string
+		expectVs []string
+	}{
+		{
+			name: "channel message",
+			info: details.GroupsInfo{
 				ItemType:   details.GroupsChannelMessage,
-		ParentPath: "parentPath",
+				ParentPath: "parentpath",
 				Message: details.ChannelMessageInfo{
 					Preview:    "preview",
 					ReplyCount: 1,
@@ -38,21 +45,71 @@ func (suite *GroupsUnitSuite) TestGroupsPrintable() {
 				LastReply: details.ChannelMessageInfo{
 					CreatedAt: then,
 				},
-	}
-
-	expectVs := []string{
+			},
+			expectHs: []string{"Message", "Channel", "Subject", "Replies", "Creator", "Created", "Last Reply"},
+			expectVs: []string{
 				"preview",
-		"parentPath",
+				"parentpath",
 				"subject",
 				"1",
 				"creator",
 				dttm.FormatToTabularDisplay(now),
 				dttm.FormatToTabularDisplay(then),
+			},
+		},
+		{
+			name: "conversation post",
+			info: details.GroupsInfo{
+				ItemType: details.GroupsConversationPost,
+				Post: details.ConversationPostInfo{
+					Preview:   "preview",
+					Creator:   "creator",
+					CreatedAt: now,
+					Topic:     "topic",
+				},
+			},
+			expectHs: []string{"Post", "Conversation", "Sender", "Created"},
+			expectVs: []string{
+				"preview",
+				"topic",
+				"creator",
+				dttm.FormatToTabularDisplay(now),
+			},
+		},
+		{
+			name: "sharepoint library",
+			info: details.GroupsInfo{
+				ItemType:   details.SharePointLibrary,
+				ParentPath: "parentPath",
+				Created:    now,
+				Modified:   then,
+				DriveName:  "librarydrive",
+				ItemName:   "item",
+				Size:       42,
+				Owner:      "user",
+			},
+			expectHs: []string{"ItemName", "Library", "ParentPath", "Size", "Owner", "Created", "Modified"},
+			expectVs: []string{
+				"item",
+				"librarydrive",
+				"parentPath",
+				"42 B",
+				"user",
+				dttm.FormatToTabularDisplay(now),
+				dttm.FormatToTabularDisplay(then),
+			},
+		},
 	}
+	for _, test := range table {
+		suite.Run(test.name, func() {
+			t := suite.T()
 
-	hs := gi.Headers()
-	vs := gi.Values()
+			hs := test.info.Headers()
+			vs := test.info.Values()
 
 			assert.Equal(t, len(hs), len(vs))
-	assert.Equal(t, expectVs, vs)
+			assert.Equal(t, test.expectHs, hs)
+			assert.Equal(t, test.expectVs, vs)
+		})
+	}
 }

src/pkg/selectors/groups.go

@@ -217,9 +217,8 @@ func (s *groups) AllData() []GroupsScope {
 	scopes = append(
 		scopes,
 		makeScope[GroupsScope](GroupsLibraryFolder, Any()),
-		makeScope[GroupsScope](GroupsChannel, Any()))
-	// TODO: enable conversations in all-data backups
-	// makeScope[GroupsScope](GroupsConversation, Any()))
+		makeScope[GroupsScope](GroupsChannel, Any()),
+		makeScope[GroupsScope](GroupsConversation, Any()))
 
 	return scopes
 }

src/pkg/selectors/groups_test.go

@@ -245,9 +245,8 @@ func (suite *GroupsSelectorSuite) TestGroupsRestore_Reduce() {
 			},
 			expect: arr(
 				libItem, libItem2, libItem3,
-				chanItem, chanItem2, chanItem3),
-			// TODO: re-add when we release conversations
-			// convItem, convItem2, convItem3),
+				chanItem, chanItem2, chanItem3,
+				convItem, convItem2, convItem3),
 		},
 		{
 			name: "only match library item",

src/pkg/services/m365/api/conversations_test.go

@@ -96,7 +96,7 @@ func (suite *ConversationsAPIUnitSuite) TestConversationPostInfo() {
 						Preview:   "",
 						Size:      0,
 						// TODO: feed the subject in from the conversation
-						Subject: "",
+						Topic: "",
 					},
 				}