diff --git a/CHANGELOG.md b/CHANGELOG.md index f849a2758..c7bc7984a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Retry transient 400 "invalidRequest" errors during onedrive & sharepoint backup. - Backup attachments associated with group mailbox items. +- Groups and Teams backups no longer fail when a resource has no display name. ### Changed - When running `backup details` on an empty backup returns a more helpful error message. diff --git a/src/pkg/services/m365/groups.go b/src/pkg/services/m365/groups.go index a7e0cff6a..53092d5b8 100644 --- a/src/pkg/services/m365/groups.go +++ b/src/pkg/services/m365/groups.go @@ -2,12 +2,14 @@ package m365 import ( "context" + "strings" "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/pkg/fault" + "github.com/alcionai/corso/src/pkg/logger" "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 func parseGroupFromTeamable(ctx context.Context, mg models.Teamable) (*Group, error) { - if mg.GetDisplayName() == nil { - return nil, clues.New("group missing display name"). + if len(ptr.Val(mg.GetId())) == 0 { + return nil, clues.New("group missing id"). 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{ - ID: ptr.Val(mg.GetId()), - DisplayName: ptr.Val(mg.GetDisplayName()), + ID: strings.Clone(ptr.Val(mg.GetId())), + DisplayName: strings.Clone(ptr.Val(mg.GetDisplayName())), IsTeam: true, } diff --git a/src/pkg/services/m365/groups_test.go b/src/pkg/services/m365/groups_test.go index 1e0eb5e21..b67804aaa 100644 --- a/src/pkg/services/m365/groups_test.go +++ b/src/pkg/services/m365/groups_test.go @@ -5,10 +5,12 @@ import ( "github.com/alcionai/clues" "github.com/google/uuid" + "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "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/tconfig" "github.com/alcionai/corso/src/pkg/errs" @@ -149,3 +151,87 @@ func (suite *GroupsIntgSuite) TestSitesInGroup() { assert.NoError(t, err, clues.ToCore(err)) 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) + } + }) + } +}