fixing bugs found during extensive manual tests (#4173)

fix up the following bugs:
* reinstate previousPath in the metadata retrieval
* implement DoNotMergeItems for real
* nil pointer protection for the 'from' property
* some additional logging

---

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

- [x] ⛔ No

#### Type of change

- [x] 🐛 Bugfix

#### Issue(s)

* #3989

#### Test Plan

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

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

@@ -251,14 +251,14 @@ func populateCollections(
 		"num_deltas_entries", len(deltaURLs),
 		"num_paths_entries", len(collections))
 
-	pathPrefix, err := path.Builder{}.ToServiceCategoryMetadataPath(
+	pathPrefix, err := path.BuildMetadata(
 		qp.TenantID,
 		qp.ProtectedResource.ID(),
 		path.GroupsService,
 		qp.Category,
 		false)
 	if err != nil {
-		return nil, clues.Wrap(err, "making metadata path")
+		return nil, clues.Wrap(err, "making metadata path prefix")
 	}
 
 	col, err := graph.MakeMetadataCollection(

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

@@ -133,8 +133,7 @@ func (col Collection) State() data.CollectionState {
 }
 
 func (col Collection) DoNotMergeItems() bool {
-	// TODO: depends on whether or not deltas are valid
-	return true
+	return col.doNotMergeItems
 }
 
 // ---------------------------------------------------------------------------

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

@@ -69,7 +69,15 @@ func parseMetadataCollections(
 
 				switch item.ID() {
 				case metadata.PreviousPathFileName:
-					// no-op at this time, previous paths not needed
+					if _, ok := found[category][metadata.PathKey]; ok {
+						return nil, false, clues.Wrap(clues.New(category.String()), "multiple versions of path metadata").WithClues(ctx)
+					}
+
+					for k, p := range m {
+						cdps.AddPath(k, p)
+					}
+
+					found[category][metadata.PathKey] = struct{}{}
 
 				case metadata.DeltaURLsFileName:
 					if _, ok := found[category][metadata.DeltaKey]; ok {
@@ -100,9 +108,16 @@ func parseMetadataCollections(
 		}, false, nil
 	}
 
-	// Do not remove entries that contain only a path or a delta, but not both.
-	// This condition is expected.  Channels only record their path.  Messages
-	// only record their deltas.
+	// Remove any entries that contain a path or a delta, but not both.
+	// That metadata is considered incomplete, and needs to incur a
+	// complete backup on the next run.
+	for _, dps := range cdp {
+		for k, dp := range dps {
+			if len(dp.Path) == 0 {
+				delete(dps, k)
+			}
+		}
+	}
 
 	return cdp, true, nil
 }

src/internal/operations/backup.go

@@ -365,7 +365,11 @@ func (op *BackupOperation) do(
 		return nil, clues.Wrap(err, "producing manifests and metadata")
 	}
 
-	ctx = clues.Add(ctx, "can_use_metadata", canUseMetadata)
+	ctx = clues.Add(
+		ctx,
+		"can_use_metadata", canUseMetadata,
+		"assist_bases", len(mans.UniqueAssistBases()),
+		"merge_bases", len(mans.MergeBases()))
 
 	if canUseMetadata {
 		lastBackupVersion = mans.MinBackupVersion()
@@ -711,6 +715,7 @@ func mergeDetails(
 
 	// Don't bother loading any of the base details if there's nothing we need to merge.
 	if bases == nil || dataFromBackup == nil || dataFromBackup.ItemsToMerge() == 0 {
+		logger.Ctx(ctx).Info("no base details to merge")
 		return nil
 	}
 

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

@@ -129,7 +129,7 @@ func (c Channels) GetChannelMessageIDsDelta(
 		}
 
 		for _, v := range vals {
-			if v.GetAdditionalData()[graph.AddtlDataRemoved] == nil {
+			if v.GetDeletedDateTime() == nil {
 				added[ptr.Val(v.GetId())] = struct{}{}
 			} else {
 				deleted[ptr.Val(v.GetId())] = struct{}{}

src/pkg/services/m365/sites.go

@@ -47,6 +47,25 @@ type Site struct {
 	OwnerID string
 }
 
+// SiteByID retrieves a specific site.
+func SiteByID(
+	ctx context.Context,
+	acct account.Account,
+	id string,
+) (*Site, error) {
+	ac, err := makeAC(ctx, acct, path.SharePointService)
+	if err != nil {
+		return nil, clues.Stack(err).WithClues(ctx)
+	}
+
+	s, err := ac.Sites().GetByID(ctx, id)
+	if err != nil {
+		return nil, clues.Stack(err)
+	}
+
+	return ParseSite(s), nil
+}
+
 // Sites returns a list of Sites in a specified M365 tenant
 func Sites(ctx context.Context, acct account.Account, errs *fault.Bus) ([]*Site, error) {
 	ac, err := makeAC(ctx, acct, path.SharePointService)

website/docs/support/known-issues.md

@@ -25,3 +25,5 @@ Below is a list of known Corso issues and limitations:
   not be inheritable by children.
 
 * Link shares with password protection can't be restored.
+
+* All replies in a Teams Conversation are removed from backup when the root message gets deleted.