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]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ]  No

#### Type of change

- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

* #4540

#### Test Plan

- [x] 💪 Manual
- [ ]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-10-25 19:16:01 -07:00 committed by GitHub
parent 2bc3c89885
commit e1f0794185
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 15 additions and 10 deletions

View File

@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased] (beta) ## [Unreleased] (beta)
### Fixed
- SharePoint backup would fail if any site had an empty display name
## [v0.14.2] (beta) - 2023-10-17
### Added ### Added
- Skips graph calls for expired item download URLs. - Skips graph calls for expired item download URLs.
- Export operation now shows the stats at the end of the run - Export operation now shows the stats at the end of the run

View File

@ -273,21 +273,20 @@ func ValidateSite(item models.Siteable) error {
return clues.New("missing webURL").With("site_id", clues.Hide(id)) 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. // personal (ie: oneDrive) sites have to be filtered out server-side.
if strings.Contains(wURL, PersonalSitePath) { if strings.Contains(wURL, PersonalSitePath) {
return clues.Stack(ErrKnownSkippableCase). return clues.Stack(ErrKnownSkippableCase).
With("site_id", clues.Hide(id), "site_web_url", clues.Hide(wURL)) With("site_id", clues.Hide(id), "site_web_url", clues.Hide(wURL))
} }
name := ptr.Val(item.GetDisplayName()) // Not checking for a name since it's possible to have sites without a name.
if len(name) == 0 { // 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. // The built-in site at "https://{tenant-domain}/search" never has a name.
if strings.HasSuffix(wURL, "/search") { if len(ptr.Val(item.GetDisplayName())) == 0 && strings.HasSuffix(wURL, "/search") {
return clues.Stack(ErrKnownSkippableCase). return clues.Stack(ErrKnownSkippableCase).
With("site_id", clues.Hide(id), "site_web_url", clues.Hide(wURL)) 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))
} }
return nil return nil

View File

@ -61,7 +61,7 @@ func (suite *SitesUnitSuite) TestValidateSite() {
s.SetWebUrl(ptr.To("sharepoint.com/sites/foo")) s.SetWebUrl(ptr.To("sharepoint.com/sites/foo"))
return s return s
}(), }(),
errCheck: assert.Error, errCheck: assert.NoError,
}, },
{ {
name: "Search site", name: "Search site",
@ -136,6 +136,7 @@ func (suite *SitesIntgSuite) TestGetAll() {
for _, site := range sites { 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()), api.PersonalSitePath, "must not return onedrive sites")
assert.NotContains(t, ptr.Val(site.GetWebUrl()), "sharepoint.com/search", "must not return search site")
} }
} }