From e1f0794185fe3bb225e7ecb81900a99b87afea94 Mon Sep 17 00:00:00 2001
From: ashmrtn <3891298+ashmrtn@users.noreply.github.com>
Date: Wed, 25 Oct 2023 19:16:01 -0700
Subject: [PATCH] Remove site name validation check (#4541)

It's possible to have a site without a display name and this is causing unrelated backups to fail.

Need to do some testing for upstream consumers prior to merging

---

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

- [x] :white_check_mark: Yes, it's included
- [ ] :clock1: Yes, but in a later PR
- [ ] :no_entry: No

#### Type of change

- [ ] :sunflower: Feature
- [x] :bug: Bugfix
- [ ] :world_map: Documentation
- [ ] :robot: Supportability/Tests
- [ ] :computer: CI/Deployment
- [ ] :broom: Tech Debt/Cleanup

#### Issue(s)

* #4540

#### Test Plan

- [x] :muscle: Manual
- [ ] :zap: Unit test
- [ ] :green_heart: E2E
---
 CHANGELOG.md                            |  5 +++++
 src/pkg/services/m365/api/sites.go      | 17 ++++++++---------
 src/pkg/services/m365/api/sites_test.go |  3 ++-
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 06d7a09ed..8677a27b1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased] (beta)
 
+### Fixed
+- SharePoint backup would fail if any site had an empty display name
+
+## [v0.14.2] (beta) - 2023-10-17
+
 ### Added
 - Skips graph calls for expired item download URLs.
 - Export operation now shows the stats at the end of the run
diff --git a/src/pkg/services/m365/api/sites.go b/src/pkg/services/m365/api/sites.go
index 94abfb8c7..10735c19a 100644
--- a/src/pkg/services/m365/api/sites.go
+++ b/src/pkg/services/m365/api/sites.go
@@ -273,21 +273,20 @@ func ValidateSite(item models.Siteable) error {
 		return clues.New("missing webURL").With("site_id", clues.Hide(id))
 	}
 
+	// TODO(ashmrtn): Personal and search site checks should really be in the
+	// caller instead of the validation function.
 	// personal (ie: oneDrive) sites have to be filtered out server-side.
 	if strings.Contains(wURL, PersonalSitePath) {
 		return clues.Stack(ErrKnownSkippableCase).
 			With("site_id", clues.Hide(id), "site_web_url", clues.Hide(wURL))
 	}
 
-	name := ptr.Val(item.GetDisplayName())
-	if len(name) == 0 {
-		// the built-in site at "https://{tenant-domain}/search" never has a name.
-		if strings.HasSuffix(wURL, "/search") {
-			return clues.Stack(ErrKnownSkippableCase).
-				With("site_id", clues.Hide(id), "site_web_url", clues.Hide(wURL))
-		}
-
-		return clues.New("missing site display name").With("site_id", clues.Hide(id))
+	// Not checking for a name since it's possible to have sites without a name.
+	// Do check if it's the search site though since we want to filter that out.
+	// The built-in site at "https://{tenant-domain}/search" never has a name.
+	if len(ptr.Val(item.GetDisplayName())) == 0 && strings.HasSuffix(wURL, "/search") {
+		return clues.Stack(ErrKnownSkippableCase).
+			With("site_id", clues.Hide(id), "site_web_url", clues.Hide(wURL))
 	}
 
 	return nil
diff --git a/src/pkg/services/m365/api/sites_test.go b/src/pkg/services/m365/api/sites_test.go
index ada517c92..3f0d24554 100644
--- a/src/pkg/services/m365/api/sites_test.go
+++ b/src/pkg/services/m365/api/sites_test.go
@@ -61,7 +61,7 @@ func (suite *SitesUnitSuite) TestValidateSite() {
 				s.SetWebUrl(ptr.To("sharepoint.com/sites/foo"))
 				return s
 			}(),
-			errCheck: assert.Error,
+			errCheck: assert.NoError,
 		},
 		{
 			name: "Search site",
@@ -136,6 +136,7 @@ func (suite *SitesIntgSuite) TestGetAll() {
 
 	for _, site := range sites {
 		assert.NotContains(t, ptr.Val(site.GetWebUrl()), api.PersonalSitePath, "must not return onedrive sites")
+		assert.NotContains(t, ptr.Val(site.GetWebUrl()), "sharepoint.com/search", "must not return search site")
 	}
 }