use display name to look up groups (#4242)

Allows lookup of groups using their display name in addition to their ID.

---

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

- [x] ⛔ No

#### Type of change

- [x] 🌻 Feature

#### Issue(s)

* #3988 

#### Test Plan

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

src/internal/m365/graph/errors.go

@@ -96,6 +96,13 @@ var (
 	// when filenames collide in a @microsoft.graph.conflictBehavior=fail request.
 	ErrItemAlreadyExistsConflict = clues.New("item already exists")
 
+	// ErrMultipleResultsMatchIdentifier describes a situation where we're doing a lookup
+	// in some way other than by canonical url ID (ex: filtering, searching, etc).
+	// This error should only be returned if a unique result is an expected constraint
+	// of the call results.  If it's possible to opportunistically select one of the many
+	// replies, no error should get returned.
+	ErrMultipleResultsMatchIdentifier = clues.New("multiple results match the identifier")
+
 	// ErrServiceNotEnabled identifies that a resource owner does not have
 	// access to a given service.
 	ErrServiceNotEnabled = clues.New("service is not enabled for that resource owner")

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

@@ -141,7 +141,7 @@ func (c Contacts) GetContainerByName(
 	// Return an error if multiple container exist (unlikely) or if no container
 	// is found.
 	if len(gv) != 1 {
-		return nil, clues.New("unexpected number of folders returned").
+		return nil, clues.Stack(graph.ErrMultipleResultsMatchIdentifier).
 			With("returned_container_count", len(gv)).
 			WithClues(ctx)
 	}

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

@@ -2,9 +2,11 @@ package api
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/alcionai/clues"
 	msgraphgocore "github.com/microsoftgraph/msgraph-sdk-go-core"
+	"github.com/microsoftgraph/msgraph-sdk-go/groups"
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 
 	"github.com/alcionai/corso/src/internal/common/ptr"
@@ -70,7 +72,7 @@ func getGroups(
 	}
 
 	var (
-		groups = make([]models.Groupable, 0)
+		results = make([]models.Groupable, 0)
 		el      = errs.Local()
 	)
 
@@ -83,7 +85,7 @@ func getGroups(
 		if err != nil {
 			el.AddRecoverable(ctx, graph.Wrap(ctx, err, "validating groups"))
 		} else {
-			groups = append(groups, item)
+			results = append(results, item)
 		}
 
 		return true
@@ -93,9 +95,11 @@ func getGroups(
 		return nil, graph.Wrap(ctx, err, "iterating all groups")
 	}
 
-	return groups, el.Failure()
+	return results, el.Failure()
 }
 
+const filterGroupByDisplayNameQueryTmpl = "displayName eq '%s'"
+
 // GetID retrieves group by groupID.
 func (c Groups) GetByID(
 	ctx context.Context,
@@ -106,14 +110,49 @@ func (c Groups) GetByID(
 		return nil, err
 	}
 
-	resp, err := service.Client().Groups().ByGroupIdString(identifier).Get(ctx, nil)
+	ctx = clues.Add(ctx, "resource_identifier", identifier)
-	if err != nil {
-		err := graph.Wrap(ctx, err, "getting group by id")
 
-		return nil, err
+	var group models.Groupable
+
+	// prefer lookup by id, but fallback to lookup by display name,
+	// even in the case of a uuid, just in case the display name itself
+	// is a uuid.
+	if uuidRE.MatchString(identifier) {
+		group, err = service.
+			Client().
+			Groups().
+			ByGroupIdString(identifier).
+			Get(ctx, nil)
+		if err == nil {
+			return group, nil
 		}
 
-	return resp, graph.Stack(ctx, err).OrNil()
+		logger.CtxErr(ctx, err).Info("finding group by id, falling back to display name")
+	}
+
+	opts := &groups.GroupsRequestBuilderGetRequestConfiguration{
+		Headers: newEventualConsistencyHeaders(),
+		QueryParameters: &groups.GroupsRequestBuilderGetQueryParameters{
+			Filter: ptr.To(fmt.Sprintf(filterGroupByDisplayNameQueryTmpl, identifier)),
+		},
+	}
+
+	resp, err := service.Client().Groups().Get(ctx, opts)
+	if err != nil {
+		return nil, graph.Wrap(ctx, err, "finding group by display name")
+	}
+
+	vs := resp.GetValue()
+
+	if len(vs) == 0 {
+		return nil, clues.Stack(graph.ErrResourceOwnerNotFound).WithClues(ctx)
+	} else if len(vs) > 1 {
+		return nil, clues.Stack(graph.ErrMultipleResultsMatchIdentifier).WithClues(ctx)
+	}
+
+	group = vs[0]
+
+	return group, nil
 }
 
 // GetRootSite retrieves the root site for the group.
@@ -158,10 +197,10 @@ func ValidateGroup(item models.Groupable) error {
 	return nil
 }
 
-func OnlyTeams(ctx context.Context, groups []models.Groupable) []models.Groupable {
+func OnlyTeams(ctx context.Context, gs []models.Groupable) []models.Groupable {
 	var teams []models.Groupable
 
-	for _, g := range groups {
+	for _, g := range gs {
 		if IsTeam(ctx, g) {
 			teams = append(teams, g)
 		}

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

@@ -33,7 +33,7 @@ func (suite *GroupUnitSuite) TestValidateGroup() {
 	tests := []struct {
 		name           string
 		args           models.Groupable
-		errCheck       assert.ErrorAssertionFunc
+		expectErr      assert.ErrorAssertionFunc
 		errIsSkippable bool
 	}{
 		{
@@ -44,7 +44,7 @@ func (suite *GroupUnitSuite) TestValidateGroup() {
 				s.SetDisplayName(ptr.To("testgroup"))
 				return s
 			}(),
-			errCheck: assert.NoError,
+			expectErr: assert.NoError,
 		},
 		{
 			name: "No name",
@@ -53,7 +53,7 @@ func (suite *GroupUnitSuite) TestValidateGroup() {
 				s.SetId(ptr.To("id"))
 				return s
 			}(),
-			errCheck: assert.Error,
+			expectErr: assert.Error,
 		},
 		{
 			name: "No ID",
@@ -62,7 +62,7 @@ func (suite *GroupUnitSuite) TestValidateGroup() {
 				s.SetDisplayName(ptr.To("testgroup"))
 				return s
 			}(),
-			errCheck: assert.Error,
+			expectErr: assert.Error,
 		},
 	}
 
@@ -71,7 +71,7 @@ func (suite *GroupUnitSuite) TestValidateGroup() {
 			t := suite.T()
 
 			err := api.ValidateGroup(test.args)
-			test.errCheck(t, err, clues.ToCore(err))
+			test.expectErr(t, err, clues.ToCore(err))
 
 			if test.errIsSkippable {
 				assert.ErrorIs(t, err, api.ErrKnownSkippableCase)
@@ -111,29 +111,43 @@ func (suite *GroupsIntgSuite) TestGetAll() {
 }
 
 func (suite *GroupsIntgSuite) TestGroups_GetByID() {
+	t := suite.T()
+
+	ctx, flush := tester.NewContext(t)
+	defer flush()
+
 	var (
 		groupID   = suite.its.group.id
 		groupsAPI = suite.its.ac.Groups()
 	)
 
+	grp, err := groupsAPI.GetByID(ctx, groupID)
+	require.NoError(t, err, clues.ToCore(err))
+
 	table := []struct {
 		name      string
 		id        string
-		expectErr func(*testing.T, error)
+		expectErr assert.ErrorAssertionFunc
 	}{
 		{
 			name:      "valid id",
 			id:        groupID,
-			expectErr: func(t *testing.T, err error) {
+			expectErr: assert.NoError,
-				assert.NoError(t, err, clues.ToCore(err))
-			},
 		},
 		{
 			name:      "invalid id",
 			id:        uuid.NewString(),
-			expectErr: func(t *testing.T, err error) {
+			expectErr: assert.Error,
-				assert.Error(t, err, clues.ToCore(err))
 		},
+		{
+			name:      "valid display name",
+			id:        ptr.Val(grp.GetDisplayName()),
+			expectErr: assert.NoError,
+		},
+		{
+			name:      "invalid displayName",
+			id:        "jabberwocky",
+			expectErr: assert.Error,
 		},
 	}
 	for _, test := range table {
@@ -144,7 +158,7 @@ func (suite *GroupsIntgSuite) TestGroups_GetByID() {
 			defer flush()
 
 			_, err := groupsAPI.GetByID(ctx, test.id)
-			test.expectErr(t, err)
+			test.expectErr(t, err, clues.ToCore(err))
 		})
 	}
 }

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

@@ -169,7 +169,7 @@ func (c Mail) GetContainerByName(
 	// Return an error if multiple container exist (unlikely) or if no container
 	// is found.
 	if len(gv) != 1 {
-		return nil, clues.New("unexpected number of folders returned").
+		return nil, clues.Stack(graph.ErrMultipleResultsMatchIdentifier).
 			With("returned_container_count", len(gv)).
 			WithClues(ctx)
 	}

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

@@ -102,12 +102,14 @@ func (c Sites) GetAll(ctx context.Context, errs *fault.Bus) ([]models.Siteable,
 	return us, el.Failure()
 }
 
-const uuidRE = "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}"
+const uuidRETmpl = "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}"
+
+var uuidRE = regexp.MustCompile(uuidRETmpl)
 
 // matches a site ID, with or without a doman name.  Ex, either one of:
 // 10rqc2.sharepoint.com,deadbeef-0000-0000-0000-000000000000,beefdead-0000-0000-0000-000000000000
 // deadbeef-0000-0000-0000-000000000000,beefdead-0000-0000-0000-000000000000
-var siteIDRE = regexp.MustCompile(`(.+,)?` + uuidRE + "," + uuidRE)
+var siteIDRE = regexp.MustCompile(`(.+,)?` + uuidRETmpl + "," + uuidRETmpl)
 
 const sitesWebURLGetTemplate = "https://graph.microsoft.com/v1.0/sites/%s:/%s?$expand=drive"