allow groups to have no display name (#5020)

relax conditions on groups by allowing them to have an empty display name.

---

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

- [x]  Yes, it's included

#### Type of change

- [x] 🐛 Bugfix

#### Issue(s)

* Fixes #4918

#### Test Plan

- [x] 💪 Manual
- [x]  Unit test
This commit is contained in:
Keepers 2024-01-12 12:16:35 -07:00 committed by GitHub
parent 722c69b3cc
commit 9aa32d2755
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 102 additions and 4 deletions

View File

@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed ### Fixed
- Retry transient 400 "invalidRequest" errors during onedrive & sharepoint backup. - Retry transient 400 "invalidRequest" errors during onedrive & sharepoint backup.
- Backup attachments associated with group mailbox items. - Backup attachments associated with group mailbox items.
- Groups and Teams backups no longer fail when a resource has no display name.
### Changed ### Changed
- When running `backup details` on an empty backup returns a more helpful error message. - When running `backup details` on an empty backup returns a more helpful error message.

View File

@ -2,12 +2,14 @@ package m365
import ( import (
"context" "context"
"strings"
"github.com/alcionai/clues" "github.com/alcionai/clues"
"github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/logger"
"github.com/alcionai/corso/src/pkg/services/m365/api" "github.com/alcionai/corso/src/pkg/services/m365/api"
) )
@ -141,14 +143,23 @@ func parseGroup(ctx context.Context, mg models.Groupable) (*Group, error) {
// parseGroup extracts information from `models.Teamable` we care about // parseGroup extracts information from `models.Teamable` we care about
func parseGroupFromTeamable(ctx context.Context, mg models.Teamable) (*Group, error) { func parseGroupFromTeamable(ctx context.Context, mg models.Teamable) (*Group, error) {
if mg.GetDisplayName() == nil { if len(ptr.Val(mg.GetId())) == 0 {
return nil, clues.New("group missing display name"). return nil, clues.New("group missing id").
With("group_id", ptr.Val(mg.GetId())) With("group_id", ptr.Val(mg.GetId()))
} }
if mg.GetDisplayName() == nil {
// logging just in case. This shouldn't cause any issues, because
// idname consumers should be prepared to use the ID in place of the
// display name (or some other standard display).
logger.Ctx(ctx).
With("group_id", ptr.Val(mg.GetId())).
Info("group missing display name")
}
u := &Group{ u := &Group{
ID: ptr.Val(mg.GetId()), ID: strings.Clone(ptr.Val(mg.GetId())),
DisplayName: ptr.Val(mg.GetDisplayName()), DisplayName: strings.Clone(ptr.Val(mg.GetDisplayName())),
IsTeam: true, IsTeam: true,
} }

View File

@ -5,10 +5,12 @@ import (
"github.com/alcionai/clues" "github.com/alcionai/clues"
"github.com/google/uuid" "github.com/google/uuid"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite" "github.com/stretchr/testify/suite"
"github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/internal/tester"
"github.com/alcionai/corso/src/internal/tester/tconfig" "github.com/alcionai/corso/src/internal/tester/tconfig"
"github.com/alcionai/corso/src/pkg/errs" "github.com/alcionai/corso/src/pkg/errs"
@ -149,3 +151,87 @@ func (suite *GroupsIntgSuite) TestSitesInGroup() {
assert.NoError(t, err, clues.ToCore(err)) assert.NoError(t, err, clues.ToCore(err))
assert.NotEmpty(t, sites) assert.NotEmpty(t, sites)
} }
// ---------------------------------------------------------------------------
// unit tests
// ---------------------------------------------------------------------------
type GroupsUnitSuite struct {
tester.Suite
}
func TestGroupsUnitSuite(t *testing.T) {
suite.Run(t, &GroupsUnitSuite{Suite: tester.NewUnitSuite(t)})
}
func (suite *GroupsUnitSuite) TestParseGroupFromTeamable() {
id := uuid.NewString()
name := uuid.NewString()
table := []struct {
name string
team func() models.Teamable
expectErr assert.ErrorAssertionFunc
expect Group
}{
{
name: "good team",
team: func() models.Teamable {
team := models.NewTeam()
team.SetId(ptr.To(id))
team.SetDisplayName(ptr.To(name))
return team
},
expectErr: assert.NoError,
expect: Group{
ID: id,
DisplayName: name,
IsTeam: true,
},
},
{
name: "no display name",
team: func() models.Teamable {
team := models.NewTeam()
team.SetId(ptr.To(id))
return team
},
expectErr: assert.NoError,
expect: Group{
ID: id,
DisplayName: "",
IsTeam: true,
},
},
{
name: "no id",
team: func() models.Teamable {
team := models.NewTeam()
team.SetDisplayName(ptr.To(name))
return team
},
expectErr: assert.Error,
expect: Group{},
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
ctx, flush := tester.NewContext(t)
defer flush()
result, err := parseGroupFromTeamable(ctx, test.team())
test.expectErr(t, err, clues.ToCore(err))
if err != nil {
assert.Nil(t, result)
} else {
assert.Equal(t, test.expect, *result)
}
})
}
}